linux-kernel - Re: [PATCH] PCI/DOE: Remove asynchronous task support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Tue, 22 Nov 2022 12:12:15 -0800
From:   Dan Williams <dan.j.williams@...el.com>
To:     Lukas Wunner <lukas@...ner.de>,
        Jonathan Cameron <Jonathan.Cameron@...wei.com>
CC:     <ira.weiny@...el.com>, Dan Williams <dan.j.williams@...el.com>,
        "Bjorn Helgaas" <bhelgaas@...gle.com>,
        Gregory Price <gregory.price@...verge.com>,
        "Li, Ming" <ming4.li@...el.com>,
        Vishal Verma <vishal.l.verma@...el.com>,
        Alison Schofield <alison.schofield@...el.com>,
        <linux-cxl@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
        <linux-pci@...r.kernel.org>
Subject: Re: [PATCH] PCI/DOE: Remove asynchronous task support

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@...el.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.