[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v3 17/22] block: Switch discard length bounds to
From: |
Kevin Wolf |
Subject: |
Re: [Qemu-devel] [PATCH v3 17/22] block: Switch discard length bounds to byte-based |
Date: |
Fri, 24 Jun 2016 16:29:51 +0200 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
Am 24.06.2016 um 16:15 hat Eric Blake geschrieben:
> On 06/24/2016 12:43 AM, Fam Zheng wrote:
> > On Thu, 06/23 16:37, Eric Blake wrote:
> >> Sector-based limits are awkward to think about; in our on-going
> >> quest to move to byte-based interfaces, convert max_discard and
> >> discard_alignment. Rename them, using 'pdiscard' as an aid to
> >> track which remaining discard interfaces need conversion, and so
> >> that the compiler will help us catch the change in semantics
> >> across any rebased code. The BlockLimits type is now completely
> >> byte-based; and in iscsi.c, sector_limits_lun2qemu() is no
> >> longer needed.
> >>
> >> pdiscard_alignment is made unsigned (we use power-of-2 alignments
> >> as bitmasks, where unsigned is easier to think about) while
> >> leaving max_pdiscard signed (since we still have an 'int'
> >> interface); this is comparable to what commit cf081fc did for
> >> write zeroes limits. We may later want to make everything an
> >> unsigned 64-bit limit - but that requires a bigger code audit.
> >>
>
> >> - /* optimal alignment for discard requests in sectors */
> >> - int64_t discard_alignment;
> >> + /* optimal alignment for discard requests in bytes, must be power
> >> + * of 2, less than max_discard if that is set, and multiple of
> >
> > s/max_discard/max_pdiscard/
>
> Maintainer could touch it up on pull request.
Okay, no problem.
> >> /* align request */
> >> - if (bs->bl.discard_alignment &&
> >> - num >= bs->bl.discard_alignment &&
> >> - sector_num % bs->bl.discard_alignment) {
> >> - if (num > bs->bl.discard_alignment) {
> >> - num = bs->bl.discard_alignment;
> >> + if (discard_alignment &&
> >> + num >= discard_alignment &&
> >> + sector_num % discard_alignment) {
> >> + if (num > discard_alignment) {
> >> + num = discard_alignment;
> >> }
> >> - num -= sector_num % bs->bl.discard_alignment;
> >> + num -= sector_num % discard_alignment;
> >
> > Or just
> >
> > num = discard_alignment - sector_num % discard_alignment;
> >
> > without the if.
> >
>
> Sure. It all gets simplified later when I switch to bdrv_co_pdiscard().
> Up to the maintainer.
This is an actual code change and not a bug fix, so I'll leave this one
alone. We can always have a follow-up patch, but as you say your other
work will simplify it anyway, I guess that's not necessary.
Kevin
pgpkdiOjuorZU.pgp
Description: PGP signature
- [Qemu-devel] [PATCH v3 07/22] scsi: Advertise limits by blocksize, not 512, (continued)
- [Qemu-devel] [PATCH v3 07/22] scsi: Advertise limits by blocksize, not 512, Eric Blake, 2016/06/23
- [Qemu-devel] [PATCH v3 01/22] block: Tighter assertions on bdrv_aligned_pwritev(), Eric Blake, 2016/06/23
- [Qemu-devel] [PATCH v3 13/22] block: Set request_alignment during .bdrv_refresh_limits(), Eric Blake, 2016/06/23
- [Qemu-devel] [PATCH v3 10/22] iscsi: Set request_alignment during .bdrv_refresh_limits(), Eric Blake, 2016/06/23
- [Qemu-devel] [PATCH v3 15/22] block: Switch transfer length bounds to byte-based, Eric Blake, 2016/06/23
- [Qemu-devel] [PATCH v3 17/22] block: Switch discard length bounds to byte-based, Eric Blake, 2016/06/23
- [Qemu-devel] [PATCH v3 14/22] block: Set default request_alignment during bdrv_refresh_limits(), Eric Blake, 2016/06/23
- [Qemu-devel] [PATCH v3 21/22] block: Fix error message style, Eric Blake, 2016/06/23
- [Qemu-devel] [PATCH v3 03/22] block: Fix harmless off-by-one in bdrv_aligned_preadv(), Eric Blake, 2016/06/23
- [Qemu-devel] [PATCH v3 12/22] raw-win32: Set request_alignment during .bdrv_refresh_limits(), Eric Blake, 2016/06/23
- [Qemu-devel] [PATCH v3 11/22] qcow2: Set request_alignment during .bdrv_refresh_limits(), Eric Blake, 2016/06/23
- [Qemu-devel] [PATCH v3 22/22] block: Use bool as appropriate for BDS members, Eric Blake, 2016/06/23
- [Qemu-devel] [PATCH v3 19/22] block: Split bdrv_merge_limits() from bdrv_refresh_limits(), Eric Blake, 2016/06/23