On Thu, Sep 16, 2021 at 09:10:57AM +0200, Martin Wilck wrote: > On Wed, 2021-09-15 at 17:20 -0500, Benjamin Marzinski wrote: > > On Fri, Sep 10, 2021 at 01:40:50PM +0200, mwilck@xxxxxxxx wrote: > > > From: Martin Wilck <mwilck@xxxxxxxx> > > > > > > uevents listed on merge_node must be cleaned up, too. uevents > > > cancelled while being serviced and temporary queues, likewise. > > > The global uevq must be cleaned out in the uevent listener thread, > > > because it might have added events after the dispatcher thread > > > had already finished. > > > > > > > There's nothing wrong with this, but for the global list, wouldn't it > > be > > easier to just wait till after cleanup_child() calls > > cleanup_threads(), > > and then call cleanup_global_uevq(). That way you know nothing else > > is > > running. > > That would be possible. If I understand your proposal correctly, that > would require calling cleanup_global_uevq() from cleanup_child(), i.e. > from the main process. Currently uevq and the functions handling it are > static and only visible in uevent.c. > > By taking the lock in cleanup_global_uevq() and calling it from the > listener on cleanup, I can be sure that the dispatcher won't see any > more uevents if it hasn't terminated yet, and that no no events will be > added to the queue after cleanup. So I think with this patch I can also > be certain that uevq is cleaned up for good, without a need to export > the cleanup function, and without enforcing a certain order of thread > shutdowns on exit. Do you disagree? Yeah, that's why I said that there's nothing wrong with the patch. It just seemed more self-evidently correct to do it after stopping all the threads. But, regardless Reviewed-by: Benjamin Marzinski <bmarzins@xxxxxxxxxx> > > Thanks, > Martin > -- dm-devel mailing list dm-devel@xxxxxxxxxx https://listman.redhat.com/mailman/listinfo/dm-devel
