From: Dan Williams <dan.j.williams@intel.com>
To: Lukas Wunner <lukas@wunner.de>,
Jonathan Cameron <Jonathan.Cameron@huawei.com>
Cc: <ira.weiny@intel.com>, Dan Williams <dan.j.williams@intel.com>,
"Bjorn Helgaas" <bhelgaas@google.com>,
Gregory Price <gregory.price@memverge.com>,
"Li, Ming" <ming4.li@intel.com>,
Vishal Verma <vishal.l.verma@intel.com>,
Alison Schofield <alison.schofield@intel.com>,
<linux-cxl@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
<linux-pci@vger.kernel.org>
Subject: Re: [PATCH] PCI/DOE: Remove asynchronous task support
Date: Tue, 22 Nov 2022 12:12:15 -0800 [thread overview]
Message-ID: <637d2d1f52828_12cdff294c0@dwillia2-xfh.jf.intel.com.notmuch> (raw)
In-Reply-To: <20221122192859.GA20515@wunner.de>
Lukas Wunner wrote:
> On Mon, Nov 21, 2022 at 11:19:25AM +0000, Jonathan Cameron wrote:
> > On Sat, 19 Nov 2022 14:25:27 -0800 ira.weiny@intel.com wrote:
> > > Initially, it was anticipated that DOE tasks were going to need to be
> > > submitted asynchronously and the code was designed thusly. Many
> > > alternatives were discussed to fix the work initialization issue.[2]
> > >
> > > However, all current users submit tasks synchronously and this has
> > > therefore become an unneeded maintenance burden. Remove the extra
> > > maintenance burden by replacing asynchronous task submission with
> > > a synchronous wait function.[3]
> [...]
> > @Lukas, I assume we don't care about the async support for SPDM going forwards?
>
> We don't. However:
>
> While I wouldn't have put in the asynchronous support in the first place,
> now that it exists, it wouldn't delete it either.
>
> I would just keep it internal to doe.c and only expose a synchronous
> API call, which does the pci_doe_task allocation internally on the
> stack, uses the appropriate INIT_WORK variant and waits for completion.
This was my first instinct as well, but after typing it up for a bit
came to the conclusion the design would need a bigger rework.
The problem is having the work items in the task vs having a separate
command-queue where tasks are submitted, like block / usb and other
drivers that take command submissions.
> Actually I was going to do just that... I'm working on the DOE code
> but the ongoing patch submissions make things difficult for me
> because I have to shoot at a moving target.
>
> The simplest solution would probably just be the object_is_on_stack()
> check and the second simplest would be the synchronous API call outlined
> above.
The explicit separation of INIT_WORK() and INIT_WORK_ONSTACK() serves a
purpose. It makes it clear that the work context is scoped to submission
function. By hiding the difference it hides bugs where submitters get
the other async setup details of the submission wrong.
next prev parent reply other threads:[~2022-11-22 20:12 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-11-19 22:25 [PATCH] PCI/DOE: Remove asynchronous task support ira.weiny
2022-11-21 1:39 ` Li, Ming
2022-11-21 2:07 ` Li, Ming
2022-11-21 22:59 ` Ira Weiny
2022-11-22 9:46 ` Jonathan Cameron
2022-11-22 15:55 ` Ira Weiny
2022-11-21 2:01 ` Zhuo, Qiuxu
2022-11-21 11:07 ` Jonathan Cameron
2022-11-21 14:17 ` Zhuo, Qiuxu
2022-11-21 17:41 ` Jonathan Cameron
2022-11-22 19:48 ` Lukas Wunner
2022-11-21 11:19 ` Jonathan Cameron
2022-11-22 19:28 ` Lukas Wunner
2022-11-22 20:12 ` Dan Williams [this message]
2022-11-21 15:24 ` Dan Williams
2022-11-21 17:19 ` Davidlohr Bueso
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=637d2d1f52828_12cdff294c0@dwillia2-xfh.jf.intel.com.notmuch \
--to=dan.j.williams@intel.com \
--cc=Jonathan.Cameron@huawei.com \
--cc=alison.schofield@intel.com \
--cc=bhelgaas@google.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=ming4.li@intel.com \
--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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox