[PATCH] nvme: re-read ANA log on NS CHANGED AEN

[Hannes Reinecke]

On 12/4/20 6:37 PM, Keith Busch wrote:
> On Fri, Dec 04, 2020 at 05:32:21PM +0100, Hannes Reinecke wrote:
>> On 12/4/20 4:20 PM, Keith Busch wrote:
>>> The scan work can be started from other contexts too, so there are some
>>> unfortunate races here if one of those other contexts clears ctrl->events
>>> before the ANA work gets to see it, which then gets you back to having
>>> unusable namespaces when that scan work uses a stale ana log.
>>>
>> Maybe this on top?
> 
> It does shrink the race, but doesn't eliminate it if scan work starts
> after the bit is set in ctrl->events, but before the ana work is queued.
> 
> I've another question, though. You mentioned this approach for recovery
> reasons. Whether you read the ana log here or where I suggested, there's
> no recovery occuring if the ana log read fails: your patch just queues
> the scan work that won't find the matching ana group id, which is the
> same result if refreshing the ana log inline with the namespace scan
> also fails. Is their a difference I'm not seeing?
> 
My point was that with your approach we're doing I/O in the middle of a 
non-recoverable section.
While the result of the actual I/O probably don't matter much (as you 
rightly pointed out), it's the side effect of an I/O error which I'm 
worried about.
If we hit an I/O error it means that error recovery _will_ have started.
Which means that we cannot rely on the remaining part of that function 
to be executed properly, as the controller might have been torn down 
already. So my worry here is that we're left with a halfway-initialized 
block device and have no way to recover that.
In queued approach I've suggested an I/O error on reading the ANA log 
would cause the scan to be aborted, which is perfectly fine as it'll be 
redone at the end of the error recovery anyway.
At the expense of always having to read the ANA log before scanning, 
I'll admit.
But maybe there's another way, by lifting the check for the ANA 
information prior to setting up the block device ... lemme check.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                Kernel Storage Architect
hare at suse.de                              +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer