[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH v2 5/9] block: Pass unaligned discard requests t

From: Max Reitz
Subject: Re: [Qemu-devel] [PATCH v2 5/9] block: Pass unaligned discard requests to drivers
Date: Sat, 19 Nov 2016 23:05:14 +0100
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.4.0

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

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.


Attachment: signature.asc
Description: OpenPGP digital signature

reply via email to

[Prev in Thread] Current Thread [Next in Thread]