On Tue, 2023-01-17 at 13:52 -0800, Bart Van Assche wrote: > On 1/17/23 13:48, Martin Wilck wrote: > > Yes, that was my suggestion. Just defer the scsi_device_put() call > > in > > alua_rtpg_queue() in the case where the actual RTPG handler is not > > queued. I won't have time for that before next week though. > > Hi Martin, > > Do you agree that the call trace shared by Steffen is not sufficient > to > conclude that this change is necessary? Hmm, I suppose I missed your point... to re-iterate my thinking: 1 alua_queue_rtpg() must take a ref to the sdev before queueing work, whether or not the caller already has one 2 queue_delayed_work() can fail 3 if queue_delayed_work() fails, alua_queue_rtpg() must drop the ref it just took 4 BUT (and this is what I guess I missed) this ref can't be the last one dropped, because the caller of alua_rtpg_queue() must still hold a reference. And scsi_device_put() only sleeps if the last ref is dropped. Therefore the issue in Steffen's call stack should indeed be fixed just by removing the might_sleep(). If all callers callers of alua_rtpg_queue() must hold an sdev reference (I believe they do), we can indeed remove the might_sleep() entirely. Is this correct reasoning, and what you meant previously? If yes, I agree, and I apologize for not realizing it in the first place. But I think this is subtle enough to deserve a comment in the code. Thanks Martin
