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:   Wed, 22 Jun 2022 15:57:34 -0700
From:   Dan Williams <dan.j.williams@intel.com>
To:     Ira Weiny <ira.weiny@intel.com>,
        Dan Williams <dan.j.williams@intel.com>
CC:     Bjorn Helgaas <bhelgaas@google.com>,
        Jonathan Cameron <Jonathan.Cameron@huawei.com>,
        Alison Schofield <alison.schofield@intel.com>,
        Vishal Verma <vishal.l.verma@intel.com>,
        Dave Jiang <dave.jiang@intel.com>,
        Ben Widawsky <bwidawsk@kernel.org>,
        <linux-kernel@vger.kernel.org>, <linux-cxl@vger.kernel.org>,
        <linux-pci@vger.kernel.org>
Subject: Re: [PATCH V11 3/8] PCI: Create PCI library functions in support of
 DOE mailboxes.
Message-ID: <62b39e5eead1_32f38a294b6@dwillia2-xfh.notmuch>
References: <20220610202259.3544623-1-ira.weiny@intel.com>
 <20220610202259.3544623-4-ira.weiny@intel.com>
 <62ad06a643ddf_844b1294f@dwillia2-xfh.notmuch>
 <YrObe+AdfziEPlYB@iweiny-desk3>
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <YrObe+AdfziEPlYB@iweiny-desk3>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?clEhDvDEweZcQyNeOdQ2j6dZER+qMO67fOYLgLw4I0JT2QYZCi9xjzUw3Psu?=
 =?us-ascii?Q?JOeuhILimbpoGWGaK+7g9s+Vjm2igiuV1GLhrj8plOxajMi7WNo0lS83mFqQ?=
 =?us-ascii?Q?xsgKCHRUb1ICZwo8yi5CglIcFqTougRugfB3Ehm/OHP/aHUpgEC3dgdKMfd4?=
 =?us-ascii?Q?j36KESKW5NsDrraxtXPsbk1LFyrb+vFN4g0sY+qAn5KfK+8PV6LDFfn0m/Dr?=
 =?us-ascii?Q?IdneiSEU7l8VBMVT0goayhAYVl5QuM3h67T40ta4dRpJF7QKfnD3cI2HsgSs?=
 =?us-ascii?Q?KDfzP20P4866GsXOqwyguhUzPKwQzrCl916De6kIUE/fLR5bfhYFJPY23pLK?=
 =?us-ascii?Q?J3tMxvdq/DWD42xUTaAltWfJPfqP4rJZZVyAMgGG+XIaaxeMI6+rOclph6LU?=
 =?us-ascii?Q?IoWm2DpIjQfutrbQXncIlcPJ2PR4D/m2M41Xx88xCs14iDv3D2uBq6ZV/0Oq?=
 =?us-ascii?Q?qyfSpGoOkNcJSVwTwhnQOXucCHZeC3+WhbOxRdnhXH/AgrSEZ7q6LuSwWcVj?=
 =?us-ascii?Q?LGc9zRRqKZiiQo+1kMaY8V4ned/3zMMVJfGcpT3Z5I49fbaxvPZJmTMxN6wQ?=
 =?us-ascii?Q?OI61VXGOodb9asdmMOp8Faxj/KZvkDH9LDmSdjXqKDTkkG+yqv3KIL5T/L8+?=
 =?us-ascii?Q?FmReQ+w7a3HxdcCkgEPW7Ulkt7GbfpWZfvu0p5GK1Nc3gEEZdGSqzJpOw67j?=
 =?us-ascii?Q?vh8Q8eUMgwywE8Rfqh0sf9e+aNES8djS3UTzou+nssWjCKWcQZ1WSkDY9T7W?=
 =?us-ascii?Q?AG+QF01HaEF0D/YLP3eFoKhnmNvf9Wzt3wsHd19KVm9UByypk4oZX50vywqs?=
 =?us-ascii?Q?EuBb8Qpg1LAgoKBeaV9n5pI/baXzxUwgGMIQ36BZM8Y1qA2GBGmXDLgLqgsu?=
 =?us-ascii?Q?tqC4haYCqb9SGoun81YSLEMJPMcSniS86m1rScpg/B9v7MTkC3bc5lJhn9WS?=
 =?us-ascii?Q?N2+9TGK8Ay4Aii6BoTC/ymJFk0fUrzy9wwhFFVkvIyykTvCsrjdZ1FVpcj6f?=
 =?us-ascii?Q?VFlUj45Mr5CBJAT2W+KF6i0YkDeWnmW4jUvcjOO28jU4I8LmDGNSyyhZUHqO?=
 =?us-ascii?Q?fP+MoIAnd/pYA6VSbecjF6+BtD1T6WHwAzY8v2NbQoA2tdGLTiP8QQTFpDwi?=
 =?us-ascii?Q?9bc/RfILAQT9DDX6iWbufSIgIrx/dldQGmbhcwZRUZHPioPk/FaxS/S5Jzev?=
 =?us-ascii?Q?x11sE7aKC7A3tH2o/kSj0LMDm0sw2j5gqiEs29ogfXZgxWzzgxpcAuu5pjLT?=
 =?us-ascii?Q?x+ZET6SbaUczqhX8O+DpM53Eh57Ax4zGtp1DGC3FL2nReQtkn3Shn35Nc3Sf?=
 =?us-ascii?Q?uWFKIdsbiQ2M4Eg5geTSN4UFIyLG6lW1lnJYWhGhc4mJgA9CeDe8KKpnLb3B?=
 =?us-ascii?Q?/ta88+OyXC54TygOChG6TyE+VGRG2EzFGVVtAoehjz6BjGzPjqPBgj0k1mew?=
 =?us-ascii?Q?nM23P+ZXz+lO8abD8FPJeHnuct5mE+Jcf1p4wox2SN8pe+CoMeI7btI0Zd5d?=
 =?us-ascii?Q?sq9jUvel95pgc1Jho4ZSduogPbDk9BOrD11m/lodpwR1ki3wbOmBwN7e1yxu?=
 =?us-ascii?Q?LqsEvPpyYli3JJSUegVl2zHPRZF8WYYShFDpYGBQz5WhewyZ7fhVjvEWxhJM?=
 =?us-ascii?Q?Eqmvpc0QslJqbXW09Hyk5J/0+SXFToblDmrorbTdZmzqrVluoNI6V3kykUAJ?=
 =?us-ascii?Q?0fwtHrWtnImjhd5x6Hb4L0Ndn4Kgk/9WLpu355BXZFwQoXsmX6rwiDMklyNZ?=
 =?us-ascii?Q?lBVxA6OBhPmwTq4mi1ZaMn+GIDyS5w8=3D?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 5a86d2e4-4131-4607-1b23-08da54a29947
X-MS-Exchange-CrossTenant-AuthSource: MWHPR1101MB2126.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 22 Jun 2022 22:57:36.0449
 (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: ET/R9zcHAi1V7qUpbLTGYbSol5oaD/+pcR/h9Yiqich00iLn8QRWCSG8ZvI8lsbqnDM4GZkQVLidAg0Rmd02vVCsointMo4+/ZTb21upweM=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BN9PR11MB5244
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-5.0 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_MED,
        RCVD_IN_MSPIKE_H3,RCVD_IN_MSPIKE_WL,SPF_HELO_NONE,SPF_NONE,
        T_SCC_BODY_TEXT_LINE 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

Ira Weiny wrote:
> On Fri, Jun 17, 2022 at 03:56:38PM -0700, Dan Williams wrote:
[..]
> > > +static int pci_doe_discovery(struct pci_doe_mb *doe_mb, u8 *index, u16 *vid,
> > > +			     u8 *protocol)
> > > +{
> > > +	u32 request_pl = FIELD_PREP(PCI_DOE_DATA_OBJECT_DISC_REQ_3_INDEX,
> > > +				    *index);
> > > +	u32 response_pl;
> > > +	DECLARE_COMPLETION_ONSTACK(c);
> > > +	struct pci_doe_task task = {
> > > +		.prot.vid = PCI_VENDOR_ID_PCI_SIG,
> > > +		.prot.type = PCI_DOE_PROTOCOL_DISCOVERY,
> > > +		.request_pl = &request_pl,
> > > +		.request_pl_sz = sizeof(request_pl),
> > > +		.response_pl = &response_pl,
> > > +		.response_pl_sz = sizeof(response_pl),
> > > +		.complete = pci_doe_task_complete,
> > > +		.private = &c,
> > > +	};
> > > +	int ret;
> > > +
> > > +	ret = pci_doe_submit_task(doe_mb, &task);
> > > +	if (ret < 0)
> > > +		return ret;
> > > +
> > > +	wait_for_completion(&c);
> > 
> > Another place where the need for a completion can be replaced with
> > flush_work().
> 
> No not here.  While this call is internal it is actually acting like an
> external caller.  This specific wait is for that response to get back.
> 
> This pattern was specifically asked for by you.  Previously Jonathan had a
> synchronous call which took care of this but you said let all callers just
> handle it themselves.  So all callers submit a task and if they want to wait
> for the response they have to do so themselves.

Ah, true I remember that. The nice thing about a doing your own
wait_for_completion() like this is that you can make it
wait_for_completion_interruptible() to give up on the DOE if it gets
stalled. However, if you have a work item per-task and you're willing to
do an uninterruptible sleep, then flush_work(&task->work) is identical.