linux-kernel - Re: [PATCH 1/2] driver core: Introduce device_link_wait

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <cdf0a9facd95a2b7ee618e6130dedb9aabf4ed09.camel@gmail.com>
Date: Fri, 23 Feb 2024 11:45:02 +0100
From: Nuno Sá <noname.nuno@...il.com>
To: Herve Codina <herve.codina@...tlin.com>, Saravana Kannan
	 <saravanak@...gle.com>
Cc: Greg Kroah-Hartman <gregkh@...uxfoundation.org>, "Rafael J. Wysocki"
 <rafael@...nel.org>, Rob Herring <robh+dt@...nel.org>, Frank Rowand
 <frowand.list@...il.com>, Lizhi Hou <lizhi.hou@....com>, Max Zhen
 <max.zhen@....com>, Sonal Santan <sonal.santan@....com>, Stefano Stabellini
 <stefano.stabellini@...inx.com>, Jonathan Cameron
 <Jonathan.Cameron@...wei.com>,  linux-kernel@...r.kernel.org,
 devicetree@...r.kernel.org, Allan Nielsen <allan.nielsen@...rochip.com>,
 Horatiu Vultur <horatiu.vultur@...rochip.com>,  Steen Hegelund
 <steen.hegelund@...rochip.com>, Thomas Petazzoni
 <thomas.petazzoni@...tlin.com>, Android Kernel Team
 <kernel-team@...roid.com>
Subject: Re: [PATCH 1/2] driver core: Introduce device_link_wait_removal()

On Fri, 2024-02-23 at 10:11 +0100, Herve Codina wrote:
> Hi Saravana,
> 
> On Tue, 20 Feb 2024 16:31:13 -0800
> Saravana Kannan <saravanak@...gle.com> wrote:
> 
> ...
> 
> > > +void device_link_wait_removal(void)
> > > +{
> > > +       /*
> > > +        * devlink removal jobs are queued in the dedicated work queue.
> > > +        * To be sure that all removal jobs are terminated, ensure that
> > > any
> > > +        * scheduled work has run to completion.
> > > +        */
> > > +       drain_workqueue(fw_devlink_wq);  
> > 
> > Is there a reason this needs to be drain_workqueu() instead of
> > flush_workqueue(). Drain is a stronger guarantee than we need in this
> > case. All we are trying to make sure is that all the device link
> > remove work queued so far have completed.
> 
> I used drain_workqueue() because drain_workqueue() allows for jobs already
> present in a workqueue to re-queue a job and drain_workqueue() will wait
> also for this new job completion.
> 
> I think flush_workqueue() doesn't wait for this chain queueing.
> 
> In our case, my understanding was that device_link_release_fn() calls
> put_device() for the consumer and the supplier.
> If refcounts reaches zero, devlink_dev_release() can be called again
> and re-queue a job.
> 

Looks sensible. The only doubt (that Saravana mays know better) is that I'm not
sure put_device() on a supplier or consumer can actually lead to
devlink_dev_release(). AFAIU, a consumer or a supplier should not be a device
from the devlink class. Hence, looking at device_release(), I'm not sure it can
happen unless for some odd reason someone is messing with devlinks in .remove()
or .type->remove().

- Nuno Sá