Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1350762

Re: [PATCH 2/3] block: require write_same and discard requests align to logical block size

Show all headers | View raw

On Fri, Mar 4, 2016 at 4:56 PM, Darrick J. Wong <darrick.wong@oracle.com> wrote:
>
> +       bs_mask = (bdev_logical_block_size(bdev) >> 9) - 1;
> +       if ((sector & bs_mask) || ((sector + nr_sects) & bs_mask))
> +               return -EINVAL;

This test may _work_, but it's kind of crazily overly complicated.

The sane test would be just "are the start and length aligned":

        if ((sector & bs_mask) || (nr_sects & bs_mask))
                return -EINVAL;

and the *smart* test is simpler still, and asks "are there invalid
bits in either the start or the length":

        if ((sector | nr_sects) & bs_mask)
                return -EINVAL:

I suspect either of these would be fine, and the compiler may even
notice that there's the smart way of doing it.

The compiler *might* even notice that the original version can be
simplified and generate sane code.

But I think that original version is not only overly complicated, it's
also actually less obvious than the simpler versions, if only because
the whole conditional is so big that you have to actively parse it.

That last shortest form is actually so simple that I think it's the
easiest to understand too - the conditional is simply so small that it
doesn't take a lot of effort to see what it does.

            Linus

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

[PATCH v6 0/3] fallocate for block devices to provide zero-out "Darrick J. Wong" <darrick.wong@oracle.com> - 2016-03-05 02:00 +0100
  [PATCH 3/3] block: implement (some of) fallocate for block devices "Darrick J. Wong" <darrick.wong@oracle.com> - 2016-03-05 02:00 +0100
    Re: [PATCH 3/3] block: implement (some of) fallocate for block devices Linus Torvalds <torvalds@linux-foundation.org> - 2016-03-05 04:10 +0100
      Re: [PATCH 3/3] block: implement (some of) fallocate for block  devices Christoph Hellwig <hch@infradead.org> - 2016-03-05 22:00 +0100
    Re: [PATCH 3/3] block: implement (some of) fallocate for block devices Linus Torvalds <torvalds@linux-foundation.org> - 2016-03-05 04:20 +0100
      Re: [PATCH 3/3] block: implement (some of) fallocate for block  devices Christoph Hellwig <hch@infradead.org> - 2016-03-05 22:00 +0100
  [PATCH 1/3] block: invalidate the page cache when issuing  BLKZEROOUT. "Darrick J. Wong" <darrick.wong@oracle.com> - 2016-03-05 02:00 +0100
    Re: [PATCH 1/3] block: invalidate the page cache when issuing  BLKZEROOUT. Christoph Hellwig <hch@infradead.org> - 2016-03-15 08:40 +0100
  [PATCH 2/3] block: require write_same and discard requests align to  logical block size "Darrick J. Wong" <darrick.wong@oracle.com> - 2016-03-05 02:00 +0100
    Re: [PATCH 2/3] block: require write_same and discard requests align  to logical block size Linus Torvalds <torvalds@linux-foundation.org> - 2016-03-05 04:10 +0100
    Re: [PATCH 2/3] block: require write_same and discard requests align  to logical block size Christoph Hellwig <hch@infradead.org> - 2016-03-15 08:40 +0100
  Re: [PATCH v6 0/3] fallocate for block devices to provide zero-out Linus Torvalds <torvalds@linux-foundation.org> - 2016-03-05 04:20 +0100

csiph-web