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. Max
signature.asc
Description: OpenPGP digital signature
