[RFC v1 0/3] Unifying fabrics drivers

Daniel Wagner dwagner at suse.de
Tue Mar 7 04:41:43 PST 2023


On Fri, Mar 03, 2023 at 03:13:53PM -0800, James Smart wrote:
> Initial reaction:
> The resulting nvmf_configure_admin_queue() routine does look nice. However,
>> I'm not sure the callback chain is all that easier to follow.

v1 is a bit too simple as I only tried to get nvmf_configure_admin_queue()
sorted out.

> I guess I'm most disappointed that more of the logic isn't actually being
> commonized.

v2 is moving the whole state machine which makes more sense.

> Examples:
> alloc_queue():
>   At least pass queue_size as an arg. I assume we should to match up
>   with the fabric connect routine.

I'll look into that.

> init_queue():
>   I'd like to see setting of ctrl.max_segments/hw_sectors in the common
>   code. Unfortunately, likely needs to be a fops call. At least it
>   makes sure its done at the same point in controller creation.

Okay, I suspect it needs a few iterations until we sorted all these details out.

> start_queue():
>   can't common code do the nvmf_connect_admin_queue() call ?
>      If its too tied to the LIVE bit - then perhaps the queue LIVE
>      bits should actually be getting set by the common code after
>      the successful start_queue - or has routines transport can call
>      from start/stop to set/clear it.
>   If LIVE bit can be dealt with - this whole block in rdma/tcp moves to
>   common code.

Good point, the LIVE bit is currently transport specific which it dosen't need
to be.

What would be the best strategy with this kind of change for review? Should I
add stuff ontop v2 or should I try to add the common code first and then migrate
the transports over? The current approach with moving and updating in the same
time is obviously a bit hard to review (and has a good chance to introduce
regressions).

> Also I'd really like to get rid of the "new" argument everywhere. It should
> just be a state flag set on the common ctrl.

Okay.

> I guess not bad, but not helping much (yet).

I posted a very early version to discuss the approach and get all parties to
agree on so it has a chance to get accepted.



More information about the Linux-nvme mailing list