On Mon, 2017-05-15 at 16:03 +0000, Bart Van Assche wrote: > On Mon, 2017-05-15 at 10:16 +0200, Martin Wilck wrote: > > On Fri, 2017-05-12 at 16:24 +0000, Bart Van Assche wrote: > > > Allowing races like the one this patch tries to address to exist > > > makes the ALUA code harder to maintain than necessary. Have you > > > considered to make alua_bus_detach() wait until ALUA work has > > > finished by using e.g. cancel_work_sync() or rcu_synchronize()? > > > > to be honest, no, I didn't consider this yet. The current kernel > > crashes with BUG() if an ALUA device is detached at an inopportune > > point in time (not just theoretically, we actually observed this). > > The > > goal of my patch was to fix this with minimum risk to introduce > > other > > problems. The addition in patch 4/4 was an attempt to address the > > concern you had expressed in your review of the v1 patch. > > > > I'm not opposed to try to find a better solution, but could we > > maybe > > get the fix for the BUG() (i.e. patch 3/4) applied in the first > > place? > > AFAICS it would not conflict with a solution like the one you > > suggested. > > Hello Martin, > > Sorry but I don't think it's a good idea to merge patch 3/4 in the > upstream > kernel. Even with that patch applied there is nothing that prevents > that > h->handler_data would be freed while alua_rtpg() is in progress and > hence > that h->sdev is a completely random pointer if alua_rtpg() is > executed > concurrently with alua_bus_detach(). Please do not try to paper over > race > conditions but fix these properly.
Hello Bart, please be assured that I'm not trying to paper over anything. Your concern about sdev->handler_data is justified. While I think that it's a separate issue from what my patches were supposed to address, let me see if I can come up with something more comprehensive. It will take time, though, until I fully comprehend the locking concept of scsi_dh_alua.c. Regards, Martin -- Dr. Martin Wilck <[email protected]>, Tel. +49 (0)911 74053 2107 SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton HRB 21284 (AG Nürnberg)
