Ming Lei [off-list ref] writes:

CAUTION: This email originated from outside of Western Digital. Do not click on
links or open attachments unless you recognize the sender and know that the
content is safe.


On Fri, Nov 18, 2022 at 10:41:31AM +0100, Andreas Hindborg wrote:

quoted

Ming Lei [off-list ref] writes:

quoted

CAUTION: This email originated from outside of Western Digital. Do not click on
links or open attachments unless you recognize the sender and know that the
content is safe.


On Fri, Nov 18, 2022 at 01:35:29PM +0900, Damien Le Moal wrote:

quoted

On 11/18/22 13:12, Ming Lei wrote:
[...]

quoted

quoted

quoted

You can only assign it to zoned write request, but you still have to check
the sequence inside each zone, right? Then why not just check LBAs in
each zone simply?

We would need to know the zone map, which is not otherwise required.
Then we would need to track the write pointer for each open zone for
each queue, so that we can stall writes that are not issued at the write
pointer. This is in effect all zones, because we cannot track when zones
are implicitly closed. Then, if different queues are issuing writes to

Can you explain "implicitly closed" state a bit?

From https://zonedstorage.io/docs/introduction/zoned-storage, only the
following words are mentioned about closed state:

    ```Conversely, implicitly or explicitly opened zoned can be transitioned to the
    closed state using the CLOSE ZONE command.```

When a write is issued to an empty or closed zone, the drive will
automatically transition the zone into the implicit open state. This is
called implicit open because the host did not (explicitly) issue an open
zone command.

When there are too many implicitly open zones, the drive may choose to
close one of the implicitly opened zone to implicitly open the zone that
is a target for a write command.

Simple in a nutshell. This is done so that the drive can work with a
limited set of resources needed to handle open zones, that is, zones that
are being written. There are some more nasty details to all this with
limits on the number of open zones and active zones that a zoned drive may
have.

OK, thanks for the clarification about implicitly closed, but I
understand this close can't change the zone's write pointer.

You are right, it does not matter if the zone is implicitly closed, I
was mistaken. But we still have to track the write pointer of every zone
in open or active state, otherwise we cannot know if a write that arrive
to a zone with no outstanding IO is actually at the write pointer, or
whether we need to hold it.

quoted

quoted

quoted

zone info can be cached in the mapping(hash table)(zone sector is the key, and zone
info is the value), which can be implemented as one LRU style. If any zone
info isn't hit in the mapping table, ioctl(BLKREPORTZONE) can be called for
obtaining the zone info.

quoted

the same zone, we need to sync across queues. Userspace may have
synchronization in place to issue writes with multiple threads while
still hitting the write pointer.

You can trust mq-dealine, which guaranteed that write IO is sent to ->queue_rq()
in order, no matter MQ or SQ.

Yes, it could be issue from multiple queues for ublksrv, which doesn't sync
among multiple queues.

But per-zone re-order still can solve the issue, just need one lock
for each zone to cover the MQ re-order.

That lock is already there and using it, mq-deadline will never dispatch
more than one write per zone at any time. This is to avoid write
reordering. So multi queue or not, for any zone, there is no possibility
of having writes reordered.

oops, I miss the single queue depth point per zone, so ublk won't break
zoned write at all, and I agree order of batch IOs is one problem, but
not hard to solve.

The current implementation _does_ break zoned write because it reverses
batched writes. But if it is an easy fix, that is cool :)

Please look at Damien's comment:

quoted

quoted

That lock is already there and using it, mq-deadline will never dispatch
more than one write per zone at any time. This is to avoid write
reordering. So multi queue or not, for any zone, there is no possibility
of having writes reordered.

For zoned write, mq-deadline is used to limit at most one inflight write
for each zone.

So can you explain a bit how the current implementation breaks zoned
write?

Like Damien wrote in another email, mq-deadline will only impose
ordering for requests submitted in batch. The flow we have is the
following:

 - Userspace sends requests to ublk gendisk
 - Requests go through block layer and is _not_ reordered when using
   mq-deadline. They may be split.
 - Requests hit ublk_drv and ublk_drv will reverse order of _all_
   batched up requests (including split requests).
 - ublk_drv sends request to ublksrv in _reverse_ order.
 - ublksrv sends requests _not_ batched up to target device.
 - Requests that enter mq-deadline at the same time are reordered in LBA
   order, that is all good.
 - Requests that enter the kernel in different batches are not reordered
   in LBA order and end up missing the write pointer. This is bad.

So, ublk_drv is not functional for zoned storage as is. Either we have
to fix up the ordering in userspace in ublksrv, and that _will_ have a
performance impact. Or we fix the bug in ublk_drv that causes batched
requests to be _reversed_.

Thanks,
Andreas