On Wed, 2019-01-16 at 17:40 -0600, Benjamin Marzinski wrote: > On Fri, Jan 04, 2019 at 06:59:12PM +0100, Martin Wilck wrote: > > Cancel the other threads before taking vecs->lock. This avoids > > delays during shutdown caused e.g. by the checker thread holding > > the vecs lock. > > Before this change, multipathd was guaranteed that once a thread had > locked the vecs lock, and checked if it had been cancelled, it could > not > be cancelled after that until it unlocked the vecs lock. Undoing > this > guarantee will likely make it possible for multipathd to leak memory > where it wasn't possible before. Thanks for pointing that out. I wasn't aware of this guarantee. In my latest valgrind tests, valgrind reported no leaks, but multipathd was also not "clean" in the sense that every chunk of memory malloc()d had been explicitly free()d at exit. IIRC that hadn't been caused by any patches added recently. I haven't had time to look at that further, I was satisfied with no real leaks being reported. We do free the global data structures in "vecs" when we exit. So any possible leaks caused by this patch must be cases where temporary memory is allocated without proper pthread_cleanup_push(), or where a thread was cancelled between allocation of memory and setting a reference to it in the global data structures - e.g. between allocation of a path and adding it to vecs->pathvec. I haven't audited either class of leaks. I believe the first class should have been eliminated by earlier phthread_cancel audits. Fixing the second class would require designing some really clever helpers, I guess. But as argued above, I really don't think it matters much if it concerns only leaks-at-shutdown. I'll put this on my todo list, but not at the highest prio. For this RFC, we need to decide whether it's more important to be leak- free on shutdown, or to react quickly on shutdown requests. Martin -- Dr. Martin Wilck <mwilck@xxxxxxxx>, Tel. +49 (0)911 74053 2107 SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton HRB 21284 (AG Nürnberg) -- dm-devel mailing list dm-devel@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/dm-devel
