race between nvme device creation and discovery?

[Hannes Reinecke]

On 2/2/24 23:16, Daniel Wagner wrote:
> I am trying to figure out why some of the blktests fail randomly when
> running with FC as transport. This failure only appear when the
> autoconnect is running in the background. A clear indication we still
> have some sort of interference with it.
> 
> nvme/030 fails a bit more often then the rest, and it might just because
> it issues several 'nvme discover' commands, many other tests only a one.
> 
> When a test fails, it fails with
> 
>    failed to lookup subsystem for controller nvme0
> 
> which is from libnvme when it iterates over sysfs to gather infos.
> 
>          subsysname = nvme_ctrl_lookup_subsystem_name(r, name);
>          if (!subsysname) {
>                  nvme_msg(r, LOG_ERR,
>                           "failed to lookup subsystem for controller %s\n",
>                           name);
>                  errno = ENXIO;
>                  return NULL;
>          }
> 
> My current theory is when a new controller isa dded is not atomic from
> the POV userland and thus libnvme is able to observe a situation when
> there is controller but the matching subsystem is not yet visible.
> 
> So something like:
> 
>    nvme_init_ctrl
>      cdev_device_add
> 
>    // libnvme iterates over sysfs
> 
>    nvme_init_ctrl_finish
>      nvme_init_identify
>        nvme_init_subsystem
>           device_add          // nvme-subsys%d
>           sysfs_create_link   // subsys->dev -> ctrl-device
> 
> Does this any sense? And if so what could be done? Should we add some
> retry logic to libnvme?
> 

Hehe. Good old sysfs.
This is a common issue with sysfs, and we've even had a retry loop in 
udev back in them days to avoid these kind of things.

Point is, uevent will be sent out with device_add(), causing udev to 
run, running udev rules, and eventually call into libnvme to scan the 
device. But as you rightly pointed out, the sysfs link is only created
_after_ the event has been sent, so there's a race window during which
libnvme will fails to read the link, landing us with the scenario above.

While we could add a retry logic to libnvme, I'm not really convinced
this is a good idea; in the end, who's to tell how long we should wait?
A second? Several seconds? A minute? Several minutes?
Also not that sysfs_create_link() has a return code, so the link might
not be created at all ...

A possibly better way here would be to suppress uevents on device_add(),
and only send out events once the device is fully set up, ie just before
the 'return 0'.

Let me see if I can whip up a patch ...

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), GF: Ivo Totev, Andrew McDonald,
Werner Knoblich