On 18.11.2016 02:13, Eric Blake wrote:
On 11/17/2016 05:44 PM, Max Reitz wrote:
Since the SCSI specification says nothing about a minimum
discard granularity, and only documents the preferred
alignment, it is best if the block layer gives the driver
every bit of information about discard requests, rather than
rounding it to alignment boundaries early.
Is this series supposed to address this issue? Because if so, I fail to
see where it does. If the device advertises 15 MB as the discard
granularity, then the iscsi driver will still drop all discard requests
that are not aligned to 15 MB boundaries, no?
I don't have access to the device in question, so I'm hoping Peter
chimes in (oops, how'd I miss him on original CC?). Here's all the more
he said on v1:
https://lists.gnu.org/archive/html/qemu-devel/2016-11/msg02223.html
My gut feel, however, is that the iscsi code is NOT rounding
Why are you relying on your gut feel when you can simply look into the
code in question? As a matter of fact, I know that you have looked at
the very piece of code in question because you touch it in patch 4.
Now that I looked at it again, though, I see my mistake, though: I
assumed that is_byte_request_lun_aligned() would check whether the
request is aligned to the advertised discard granularity. Of course, it
does not, though, it only checks whether it's aligned to the device's
block_size.
So with the device in question, block_size is something like, say, 1 MB
and the pdiscard_alignment is 15 MB. With your series, the block layer
will first split off the head of an unaligned discard request (with the
rest being aligned to the pdiscard_alignment) and then again the head
off that head (with regards to the request_alignment).
The iscsi driver will discard the head of the head (which isn't aligned
to the request_alignment), but pass the part of the head that is aligned
to request_alignment and of course pass everything that's aligned to
pdiscard_alignment, too.
(And the same then happens for the tail.)
OK, now I see.
(the qcow2
code rounded, but that's different); the regression happened in 2.7
because the block layer also started rounding, and this patch gets the
block layer rounding out of the way. If nothing changed in the iscsi
code in the meantime, then the iscsi code should now (once again) be
discarding all sizes, regardless of the 15M advertisement.
Well, all sizes that are aligned to the request_alignment.
Meanwhile, I _did_ test this patch with blkdebug (see 9/9), as well as
on a slightly modified NBD client that forced 15M alignments, and for
those cases, it definitely made the difference on whether all bytes were
passed (spread across several calls), vs. just the aligned bytes in the
middle of a request larger than 15M.
The only difference is that it's now the iscsi driver that drops the
request instead of the generic block layer.
If the iscsi driver was ever dropping it in the first place.
It wasn't dropping them, it was asserting that it was dropped; yes, my
mistake for thinking that is_byte_request_lun_aligned() would check
whether the request is aligned to pdiscard_alignment.