Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Tue, 22 Nov 2022 12:12:15 -0800
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
Message-ID: <637d2d1f52828_12cdff294c0@dwillia2-xfh.jf.intel.com.notmuch>
References: <20221119222527.1799836-1-ira.weiny@intel.com>
 <20221121111925.00003eed@Huawei.com>
 <20221122192859.GA20515@wunner.de>
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <20221122192859.GA20515@wunner.de>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?yrXzntmraIQaKchzSVzXhilVzMiJfx1m4TDYEQSoZx44nvsSaREHDh1TxFNR?=
 =?us-ascii?Q?QUC/zH3VucmP9mnyvPR/CyXseDrk/hH9o8DKleUELon8PYGuTdto6D6y9Qx8?=
 =?us-ascii?Q?7OnFMz+YKlOZ9yInppb8uHrDZ2TaR8rFyN01O2M3veaNOjhBRHykxIYPXWWv?=
 =?us-ascii?Q?ILZ0ZiXmy+LM6QqC//t3dsZK/h7Wh2AyKegB8tm5TlyacwH/5wvchMFZJcCf?=
 =?us-ascii?Q?CNfAlEGGdcpJb7FK6ZvD7BQzqCbZ1l2VjBQWM0CGZbuHzirUd9a5jnifcN4F?=
 =?us-ascii?Q?0Mr6Fwt3ZkbHpTzzKCujZWtQe2v60afq/g1aT3LqL+KUv2OVhFVM4lontgsA?=
 =?us-ascii?Q?A7iuElCVBw5Lbri8owkqzTUOJUxkAQS8JhrcUC6S+vEwrsv3b6CFHY4ktbYK?=
 =?us-ascii?Q?MYET5T8YpEbU7J6lAuKflRhqlLAB6GsQSCYB1NjP5jVwmJxdfA+5yRvCc/M3?=
 =?us-ascii?Q?2I+0klT84bZ7ajKSH+zPs0gEvOYNo19uEj1ko3N9tiXq+dNq9wJbDu7HEtrp?=
 =?us-ascii?Q?7c+KprkFrh8gHZzjR3V9yKGaRU9Hw99WrLgnwqfNRcnJia1FVwDPC1XQJpMT?=
 =?us-ascii?Q?SIRDsTOSr/UpEiDiBmVRjNwXCsjm3HwZ3vF/x8WlipYf/4SJq1WEgQw8Urov?=
 =?us-ascii?Q?B7U1zbk9FcS75LoMdgIUFFaEXVHazbl+D12gbOXTusHLZc+huCDJ4IRVlJ65?=
 =?us-ascii?Q?WmxJ3hwhdz5cLHdw/+v6+GyviqTiNtB0OMvpS4WjTZge4owfv3AOpLqRdB0d?=
 =?us-ascii?Q?e19F7cBN/6HELv+NIqrlFRsR+D7Pn69qKGyraxKbsdURtqoJyo7mbJJ21PI4?=
 =?us-ascii?Q?hcL79TFVVGbGjS7KUUB22G0NBItgqHHwaLyOSGhiTwUb2wEtD0AycEnLdwEB?=
 =?us-ascii?Q?LGAvNocILpU5Kz7k9zDmHg9JP1+pe+SX+5x/+W8tCtU2wLiN+0ZizJftZC7I?=
 =?us-ascii?Q?n8ZOrxP5ZZBdHOXZlBOmcAt2kI9HLUvzqxaaXNteEjlf/EPR4EXCf3Z48laJ?=
 =?us-ascii?Q?vuKRptyFptpli4uXHM9sOLCnCrNwqXT2076prdiK0pGIQEN7yZtVBBPCCQWE?=
 =?us-ascii?Q?dDwRrEhMMdsJ/n9fpG0IOhm5CvmQ06G50XcH9qN1abOgnkaotmG8bUORKB54?=
 =?us-ascii?Q?4N8rjdtcLOp1JwNX6tO8ODmKOXVOVYCMtPjcW6VZxqM6d3aB0K0Nc0w81wBD?=
 =?us-ascii?Q?iDPgDs62Yi7Wk9dapw527mfkXcjYE//lRsgyDTi1bkvn0VTEg/AMoWkQsJtG?=
 =?us-ascii?Q?YI4KR2uBvaslsyhGF22xY8oYgLjxnjXTcE1cTTIe7OH8caXrmUBTH/2LWAPL?=
 =?us-ascii?Q?pF9b4kgeov0tCAam9fCZeF8jS30xpMWHeJ9w7u8q8BWpcLtSGm6diSDp7PZ1?=
 =?us-ascii?Q?r+VMiJfgVLpBRZoccaDzJTVAxqZO3uXgIs31czy8AKe5IpAyWdY+2iXkADGH?=
 =?us-ascii?Q?ZSr1ySoaoHRNiNI0PR+WWz2nQwCvOK4KLcQMvn7Fp++QyEBLeWYK5lROYW/b?=
 =?us-ascii?Q?gNUPXtrfPm2Z8S0xUE8O7X8MX+BIubwBFQ4OKTS6LJZFoewHFD5ftIjjgMF9?=
 =?us-ascii?Q?VSggP0UxSHfuSYpexDXOufL6ULkFJ1OO3u3lmRrSgC5u8J0vSKnm5/6nEZ15?=
 =?us-ascii?Q?gw=3D=3D?=
X-MS-Exchange-CrossTenant-Network-Message-Id: ba603353-1852-46b0-c2ef-08daccc5dab4
X-MS-Exchange-CrossTenant-AuthSource: MWHPR1101MB2126.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 22 Nov 2022 20:12:18.8435
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: 2czxweVLOFiwF1XQ2BOsyEqKc6qZWgMdkunUwVyX65Ip9vRuN9Oh8liEWK76nHoxeDRqthQrdRf6zZ21c9ZYaoVGj/tykbXVXGYW40t9sf8=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CY5PR11MB6163
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,SPF_HELO_NONE,
        SPF_NONE autolearn=ham autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

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.