Re: [PATCH] PCI/DOE: Remove asynchronous task support
From: Dan Williams
Date: Tue Nov 22 2022 - 15:12:29 EST
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@xxxxxxxxx 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.