From: Bjorn Helgaas <helgaas@kernel.org>
To: Lukas Wunner <lukas@wunner.de>
Cc: ira.weiny@intel.com, Dan Williams <dan.j.williams@intel.com>,
Bjorn Helgaas <bhelgaas@google.com>,
Gregory Price <gregory.price@memverge.com>,
Jonathan Cameron <Jonathan.Cameron@huawei.com>,
Alison Schofield <alison.schofield@intel.com>,
Vishal Verma <vishal.l.verma@intel.com>,
Ben Widawsky <bwidawsk@kernel.org>,
linux-cxl@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-pci@vger.kernel.org
Subject: Re: [PATCH] PCI/doe: Fix work struct declaration
Date: Wed, 16 Nov 2022 12:20:37 -0600 [thread overview]
Message-ID: <20221116182037.GA1127308@bhelgaas> (raw)
In-Reply-To: <20221116100939.GA32050@wunner.de>
On Wed, Nov 16, 2022 at 11:09:39AM +0100, Lukas Wunner wrote:
> On Mon, Nov 14, 2022 at 05:19:43PM -0800, ira.weiny@intel.com wrote:
> > From: Ira Weiny <ira.weiny@intel.com>
> >
> > The callers of pci_doe_submit_task() allocate the pci_doe_task on the
> > stack. This causes the work structure to be allocated on the stack
> > without pci_doe_submit_task() knowing. Work item initialization needs
> > to be done with either INIT_WORK_ONSTACK() or INIT_WORK() depending on
> > how the work item is allocated.
> >
> > Jonathan suggested creating doe task allocation macros such as
> > DECLARE_CDAT_DOE_TASK_ONSTACK().[1] The issue with this is the work
> > function is not known to the callers and must be initialized correctly.
> >
> > A follow up suggestion was to have an internal 'pci_doe_work' item
> > allocated by pci_doe_submit_task().[2] This requires an allocation which
> > could restrict the context where tasks are used.
> >
> > Compromise with an intermediate step to initialize the task struct with
> > a new call pci_doe_init_task() which must be called prior to submit
> > task.
> >
> > [1] https://lore.kernel.org/linux-cxl/20221014151045.24781-1-Jonathan.Cameron@huawei.com/T/#m88a7f50dcce52f30c8bf5c3dcc06fa9843b54a2d
> > [2] https://lore.kernel.org/linux-cxl/20221014151045.24781-1-Jonathan.Cameron@huawei.com/T/#m63c636c5135f304480370924f4d03c00357be667
>
> We have object_is_on_stack(), included from <linux/sched/task_stack.h>.
>
> So you could just autosense in pci_doe_submit_task() whether
> pci_doe_task is on the stack and call the appropriate INIT_WORK
> variant.
Nifty, I had no idea object_is_on_stack() existed, thank you!
I wonder if there's an opportunity to use object_is_on_stack()
somewhere in the INIT_WORK() path to find usage mistakes.
Adding it in pci_doe_submit_task() would add some complexity, so I'm
not sure whether it's worth adding it unless we actually have uses for
both cases.
Bjorn
next prev parent reply other threads:[~2022-11-16 18:20 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-11-15 1:19 [PATCH] PCI/doe: Fix work struct declaration ira.weiny
2022-11-15 11:13 ` Jonathan Cameron
2022-11-15 19:44 ` Bjorn Helgaas
2022-11-15 20:18 ` Ira Weiny
2022-11-15 20:41 ` Bjorn Helgaas
2022-11-15 20:54 ` Ira Weiny
2022-11-15 22:12 ` Bjorn Helgaas
2022-11-16 10:09 ` Lukas Wunner
2022-11-16 18:20 ` Bjorn Helgaas [this message]
2022-11-16 20:57 ` Ira Weiny
2022-11-16 21:10 ` Dan Williams
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20221116182037.GA1127308@bhelgaas \
--to=helgaas@kernel.org \
--cc=Jonathan.Cameron@huawei.com \
--cc=alison.schofield@intel.com \
--cc=bhelgaas@google.com \
--cc=bwidawsk@kernel.org \
--cc=dan.j.williams@intel.com \
--cc=gregory.price@memverge.com \
--cc=ira.weiny@intel.com \
--cc=linux-cxl@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pci@vger.kernel.org \
--cc=lukas@wunner.de \
--cc=vishal.l.verma@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.