On 2/26/2020 7:52 PM, Sagi Grimberg wrote:

quoted

quoted

quoted

As per NVMe-oF spec sq flow control is actually mandatory, and we 
should
be implementing it to avoid the controller to return a fatal status
error, and try to play nicely with controllers using sq flow control
to implement QoS.

Hannes,

Can you please clarify why the individual transports aren't sufficient
for this QoS feature you are talking about?

It's all about allocation of resources - whether mandated or not.

See Hannes' patch description, he talks about QoS, not a resource
problem. I am arguing that rate-limiting a specific host can be done
today.

QoS is an overloaded term. It's not just rate limiting, but is expending 
effort on doing an io is considered a type of QoS, with most devices 
assuming if I have it I'll work on it - not just park it. I don't want 
to waste memory on parking lots.

quoted

Current nvme/nvmeof model requires memory for entire sq size (well 
maybe sqsize -1) so that the host can send a sqe and there's a place 
to put it on the target.

That is not the case for either TCP or RDMA. TCP can not read from the
socket, and RDMA can not post a receive buffer, which will cause the
HW to issue rnr nak (receive not ready), and the host would be
effectively throttled. So there are no such memory requirement (not for
PCIe either).

yep - so a difference vs FC which doesn't have real "connections".

quoted

For a transport like FC, where there isn't dedicated memory allocated 
for the sq and each sqe arrives independently, there's a desire to 
guarantee only a small number of elements, then let the controller 
adjust sqhd up to open more of a window to allow more commands from a 
particular host.

I can have shared resources with both RDMA, TCP (and PCIe) and
implementing what you are saying is perfectly achievable.

quoted

When the subsystem is managing multiple controllers to perhaps many 
hosts, they may be all sharing the same set of receive buffers.

This is not specific to FC, see nvmet-rdma usage of srq for example, and
nvmet-tcp can have the same concept.

This is becoming a resource constrained issue which is different from
the patch description.

the only disagreement is terms - many consider resource allocation an 
element of QoS.

quoted

The controller would like to control the ebb and flow of what 
controllers/what queues use the available resources at any particular 
point in time based on host/controller QOS settings.

As most arrays are limited in the amount of memory they can dedicate 
to receive buffers/queues and as the rules to date require guaranteed 
allocation of sq size, the controllers have to pick how much they 
scale - how many associations/controllers vs how many queues per 
controller vs sq size per queue that they allow.

They should do that regardless, there is no point allowing lots of
queues if effectively the array supports very little right?

Agree - but in some cases, the scale that has to be reached has things 
reaching absurdly small levels.  Given that most things are bursty it 
would be better to give each enough to recognize when they start pushing 
load, then based on a priority scheme or some other metric, dole out the 
resources of the subsystem.  People view that policy choice as QoS.

quoted

The disablement of sqflow control doesn't help the "guarantee", it 
actually confirms it. There are a couple of issues in the nvmeof spec 
that conflict with removing the guarantee - namely - can a value 
other than 1 (or 0) be returned from the connect fabric command for 
sqhd ?  It can be argued that our implementation today, which returns 
1, which assumes an increment after connect at idx 0 is actually 
incorrect to the spec as the queue is to be created as part of 
connect and it is to be "empty" with sqhd=sqtail=0.     A poor mans 
implementation, which ignores this initial connect response issue in 
the spec, and tries to avoid dedicated resources, would live with 
allowing a storm of 1 sqsize worth per queue, expecting that it won't 
really happen in a complete burst, and sqhd can be controlled going 
back to the host such that further io can't be sent beyond 1 
sqsizes's worth unless the controller moves sqhd.

You lost me a bit, is the issue supporting QD=1 from a large number of
queues?

No.  What I'm trying to say is: to use sqhd for flow control, there are 
2 needs: a) the ability to control when sqhd increments, not inherently 
1:1 with a completion; and b) control of the initial value for sqhd.   
For (b), if you can't control it - then it says you always are in a 
startup condition where the host may send a full sqsize-1 worth of 
sqes.  What the device would have rather done is send an sqhd value at 
Connect completion such that the sqtail/sqhd computation shows a few 
"free" slots, not the full sqsize-1's worth.  The spec doesn't 
necessarily say that the device couldn't do that today although many 
people, based on the other "initialization" and "empty" queue statements 
believes it must be 0...  and... that is actually at odds with what we 
do today as we actually return 1 due to the "send it on the queue it 
creates" issue.  This should-it-be-0 or should-it-be-1 needs to be 
cleared up as well as can the controller return other values. I had 
proposed errata over a year ago for an ECN but has been constantly 
backburner-ed.

So the latter half of the paragraph was stating - as long as the 
controller has (a), it would live with not having (b), just assuming 
that the initial sqsize-1 credits worth won't be all in one storm and 
it's management of sqhd will resolve within the 1st iteration of the sq 
and it can manage solely with (a).

quoted

quoted

If we look at the transports landscape, each transport has a credit
mechanism that can throttle bulk data transfers. In FC exchanges the
target is in control pulling data from the host with xfer_ready,
In RDMA the target decides when to issue rdma_read, and in TCP the
target decides when to issue R2T.

it's not data flow, it's reception of io commands vs sq.

quoted

These are all credits that give the control to the target to
back-pressure the host. Now if the target doesn't want the host to send
more commands, it can throttle sending completions thus controlling the
pace.

that doesn't help the system. It only makes the io look like it takes 
a lot longer to complete.

Exactly, they will take longer to complete, meaning the host won't issue
more. Even if the host is not issuing the command, it is still
effectively taking the same time to complete.

Well an io that moves for 10us latency to 100us will definitely affect 
the application that issued it, which is why I don't agree with the 
approach.

quoted

At some point, the completion has to go back.

Yes.

quoted

quoted

I must say that returning BLK_STS_RESOURCE for host managed SQ_HEAD 
is a
bit awkward in my mind, but that just one's opinion, what do others 
have
to say?

it isn't a host-managed SQ_HEAD. it's a real implementation of 
acknowledging the sq flow control that was originally spec'd. We've 
always ignored this in lieu of cheating and setting the blk-mq 
request count, and hoped as we never sent less than a queue's worth 
there was never a reason for the controller to need to not increment 
sqhd on a 1by1 basis.    But the other issue with this is - we're 
wasting lots of sq space on the controller. We have 4, 10, 100 sqs 
but only send 1 sq's worth of io ?  why would a controller ever want 
to support a high queue count except for a back-to-back attachment to 
a fixed number of hosts (1 ?).

I agree, the controller can tell the host how many queues and max
queue depth, it can prevent the host from allocating lots of long
queues.

quoted

That's not reasonable for a real SAN device - so we're forcing them 
into small queue counts and small sq sizes so they don't waste memory 
yet can handle bunches of associations.

Sounds like this makes sense.

quoted

So in addition to this sqhd tracking that Hannes was proposing, for 
FC at least, as sq's are logical,

You describe this sqhd throttling as something that will happen a lot,
and in normal working points, so returning BLK_STS_RESOURCE having
blk-mq retrying over and over doesn't seem like an adequate solution
in my mind...

shrug - I don't know. but I don't know how you do otherwise. Retrying, 
usually with small intervals before retry will typically be good as most 
things will be in bursts.
Certainly open to other ideas that may influence blk-mq to avoid all the 
retries.

quoted

I was also going to look into supporting more than 1 sq's worth of ios.

What do you mean? where is more than 1 sq's worth of ios not supported?
You talking about for a single queue? Maybe I'm missing something...

Sorry my mistake. I was under the impression that tag_set.queue_depth 
was an absolute cap, spread across the hw queues, not a depth per hw queue.

-- james



_______________________________________________
linux-nvme mailing list
linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme