On 01/31/2018 08:59 PM, Bart Van Assche wrote:
On Wed, 2018-01-31 at 17:48 -0200, Mauricio Faria de Oliveira wrote:
On 01/31/2018 05:06 PM, Bart Van Assche wrote:
Sorry but I think this patch introduces new race conditions. Have you
Can you detail the race conditions? As far as I can see, the only race
condition would be when an error handler is invoked very close in time
to the shutdown/unload handler, and as such miss the 'ioc->remove_host'
assignment (thus it continues running anyway, and hit the oops).
That's indeed the race I was referring to. [snip]
Okay. I'm not sure that point invalidates this patch; let me explain.
First, IMHO that problem already exists in mpt3sas as it is now.
There are 17 checks for 'ioc->remove_host' to return early, in the
same manner. AFAIK, those are subject to this same race condition.
$ grep -r 'ioc->remove_host[^=]*$' drivers/scsi/mpt3sas/ | wc -l
17
Second, I don't think this patch makes the driver any worse.
Even with the potential race condition (which already exists elsewhere
in the driver), it reduces the chance to hit an oops from _every time_
to only when the race condition occurs _and_ the error handler looses.
So, even not being completely safe (like other parts of the driver),
it still does help.
BTW, are you aware that your patch
does not seem to apply on Martin's tree (the fixes branch of
git://git.kernel.org/pub/scm/linux/kernel/git/mkp/scsi.git):
> [snip]
Can you rebase this patch on top of Martin's tree?
Yes, sure. I'll rebase, test, and send v2. Thanks for the pointer.
cheers,
Mauricio