From: Alexander Duyck <firstname.lastname@example.org> To: email@example.com Cc: firstname.lastname@example.org, Greg KH <email@example.com>, LKML <firstname.lastname@example.org>, email@example.com, firstname.lastname@example.org, email@example.com, firstname.lastname@example.org, email@example.com, firstname.lastname@example.org, Tejun Heo <email@example.com>, Andrew Morton <firstname.lastname@example.org> Subject: Re: [driver-core PATCH v4 4/6] driver core: Probe devices asynchronously instead of the driver Date: Thu, 18 Oct 2018 19:20:12 -0700 [thread overview] Message-ID: <CAKgT0Ucf-v3FNTA0z=3nPR-xS=0NYeqDjRfDW-KjDE5Edemail@example.com> (raw) In-Reply-To: <firstname.lastname@example.org> On Thu, Oct 18, 2018 at 1:15 PM Bart Van Assche <email@example.com> wrote: > > On Thu, 2018-10-18 at 12:38 -0700, Alexander Duyck wrote: > > Basically if somebody loads a driver the dev->driver becomes set. If a > > driver is removed it will clear dev->driver and set driver_data to > > 0/NULL. That is what I am using as a mutex to track it in conjunction > > with the device mutex. Basically if somebody attempts to attach a driver > > before we get there we just exit and don't attempt to load this driver. > > I don't think that the above matches your code. __device_attach() does not > set the dev->driver pointer before scheduling an asynchronous probe. Only > dev->driver_data gets set before the asynchonous probe is scheduled. Since > driver_detach() only iterates over devices that are in the per-driver klist > it will skip all devices for which an asynchronous probe has been scheduled > but __device_attach_async_helper() has not yet been called. My conclusion > remains that this patch does not prevent a driver pointer to become invalid > concurrently with __device_attach_async_helper() dereferencing the same > driver pointer. > > Bart. I see what you are talking about now. Actually I think this was an existing issue before my patch even came into play. Basically the code as it currently stands is device specific in terms of the attach and release code. I wonder if we shouldn't have the async_synchronize_full call in __device_release_driver moved down and into driver_detach before we even start the for loop. Assuming the driver is no longer associated with the bus that should flush out all devices so that we can then pull them out of the devices list at least. I may look at adding an additional bitflag to the device struct to indicate that it has a driver attach pending. Then for things like races between any attach and detach calls the logic becomes pretty straight forward. Attach will set the bit and provide driver data, detach will clear the bit and the driver data. If a driver loads in between it should clear the bit as well. I'll work on it over the next couple days and hopefully have something ready for testing/review early next week. Thanks. - Alex
next prev parent reply other threads:[~2018-10-19 2:20 UTC|newest] Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top 2018-10-15 15:09 [driver-core PATCH v4 0/6] Add NUMA aware async_schedule calls Alexander Duyck 2018-10-15 15:09 ` [driver-core PATCH v4 1/6] workqueue: Provide queue_work_node to queue work near a given NUMA node Alexander Duyck 2018-10-15 15:09 ` [driver-core PATCH v4 2/6] async: Add support for queueing on specific " Alexander Duyck 2018-10-15 15:09 ` [driver-core PATCH v4 3/6] device core: Consolidate locking and unlocking of parent and device Alexander Duyck 2018-10-18 7:46 ` Rafael J. Wysocki 2018-10-18 17:53 ` Bart Van Assche 2018-10-15 15:09 ` [driver-core PATCH v4 4/6] driver core: Probe devices asynchronously instead of the driver Alexander Duyck 2018-10-18 18:11 ` Bart Van Assche 2018-10-18 19:38 ` Alexander Duyck 2018-10-18 20:13 ` Bart Van Assche 2018-10-19 2:20 ` Alexander Duyck [this message] 2018-10-19 2:31 ` Bart Van Assche 2018-10-19 22:35 ` Alexander Duyck 2018-10-15 15:09 ` [driver-core PATCH v4 5/6] driver core: Attach devices on CPU local to device node Alexander Duyck 2018-10-15 15:09 ` [driver-core PATCH v4 6/6] PM core: Use new async_schedule_dev command Alexander Duyck
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to='CAKgT0Ucf-v3FNTA0z=3nPR-xS=0NYeqDjRfDW-KjDE5Edfirstname.lastname@example.org' \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --subject='Re: [driver-core PATCH v4 4/6] driver core: Probe devices asynchronously instead of the driver' \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).