On Fri, Feb 26, 2016 at 9:08 PM, Verkamp, Daniel
<daniel.verkamp(a)intel.com> wrote:
> Hi Andrey,
>
> I am not sure what behavior you would like to see from probe when calling it again after attaching some controllers.  We can't re-probe/re-attach the same controller twice without detaching it in between - if it is already attached, that means the driver is already running, queues are initialized, etc. and we should not call the user's attach callback again.
>
> The spdk_nvme_probe() interface operates at the controller (PCIe device) level; namespace enumeration should be handled in the application layer in or after the attach callback.
>
> Can you clarify your desired behavior/use case in your app?
>

To my understanding, the goal of the probe(), from the user's
standpoint, is to get hold of the controller reference (and it's
basically the only way to accomplish this under the current API). If
I'm parsing multiple PCI inputs, the probe() semantics forces me into
the probed controller housekeeping on my side. As probe() (one can
safely assume) occurs in the initialization phase, it should be safe
to return the controller references back to the user even if the
controller has already been started, at user's discretion.

> We can possibly add another interface to enumerate all of the NVMe controllers that are currently attached to the userspace driver (the ones in the attached_ctrlrs list) if that would help.
>

That's pretty much what I have suggested.

Regards,
Andrey

> Thanks,
> -- Daniel
>
> P.S. I believe the return value from PCI enumeration is already handled; the intent is that even if nvme_pci_enumerate() returns a failure code, it may have found some controllers, and we want to initialize those even if other PCI functions failed to probe.  The return code is returned intact at the end of spdk_nvme_probe() so that the application can determine that an error occurred and take action if necessary.
>
> -----Original Message-----
> From: SPDK [mailto:spdk-bounces(a)lists.01.org] On Behalf Of Andrey Kuzmin
> Sent: Friday, February 26, 2016 10:52 AM
> To: spdk(a)lists.01.org
> Subject: [SPDK] Allow attachment to an already probed controller
>
> While I appreciatethe reasoning behind the current spdk_nvme_probe attach-once semantics, it still imposes an extra burden of the already probed controller tracking on the application that, for instance, seeks to attach to multiple namespaces under the same controller. The below patch provides an optional fix for the issue. Notice that it also fixes the bug of the error returned by the enumeration being ignored.
>
> Regards,
> Andrey
>
> diff --git a/lib/nvme/nvme.c b/lib/nvme/nvme.c index 63a316b..2a3ac64 100644
> --- a/lib/nvme/nvme.c
> +++ b/lib/nvme/nvme.c
> @@ -296,10 +296,32 @@ spdk_nvme_probe(void *cb_ctx, spdk_nvme_probe_cb probe_cb, spdk_nvme_attach_cb a
>
>      nvme_mutex_lock(&g_nvme_driver.lock);
>
> +    /*
> +     * First, check if the requested controller has already been started.
> +     */
> +    TAILQ_FOREACH(ctrlr, &g_nvme_driver.attached_ctrlrs, tailq) {
> +        if (!probe_cb(cb_ctx, ctrlr->devhandle))
> +            continue;
> +
> +        nvme_mutex_unlock(&g_nvme_driver.lock);
> +        attach_cb(cb_ctx, ctrlr->devhandle, ctrlr);
> +        return 0;
> +    }
> +
>      enum_ctx.probe_cb = probe_cb;
>      enum_ctx.cb_ctx = cb_ctx;
>
>      rc = nvme_pci_enumerate(nvme_enum_cb, &enum_ctx);
> +
> +    /*
> +     * Appreciate the error being returned
> +     * by the nvme_pci_enumerate, if any.
> +     */
> +    if (rc) {
> +        nvme_mutex_unlock(&g_nvme_driver.lock);
> +        return rc;
> +    }
> +
>      /*
>       * Keep going even if one or more nvme_attach() calls failed,
>       *  but maintain the value of rc to signal errors when we return.
> _______________________________________________
> SPDK mailing list
> SPDK(a)lists.01.org
> https://lists.01.org/mailman/listinfo/spdk