lkml.org 
[lkml]   [2012]   [Mar]   [27]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 2/2] block: split discard into aligned requests
On Wed, Mar 14, 2012 at 06:12:28PM +0100, Paolo Bonzini wrote:
> When a disk has a relatively large discard_granularity or small
> max_discard_sectors, discards are not split with optimal alignment.
> In the limit case of discard_granularity == max_discard_sectors, all
> requests might be aligned incorrectly, so in fact you might end up
> with no discarded logical blocks at all.
>
> One example that helps showing how the patch works is with
> discard_granularity == 64, max_discard_sectors == 128. A request that is
> submitted for 256 sectors 2..257 will be split in two: 2..129, 130..257.
> However, only 2 aligned blocks out of 3 are included in the request;
> 128..191 may be left intact and not discarded. With this patch, the
> first request will be truncated to ensure good alignment of what's left,
> and the split will be 2..127, 128..255, 256..257. discard_alignment is
> also taken into account.
>
> At most one extra request will be introduced, because the first request
> will be reduced by at most granularity-1 sectors, and granularity
> must be less than max_discard_sectors. Subsequent requests will run
> on round_down(max_discard_sectors, granularity) sectors, as in the
> current code.
>
> Cc: Jens Axboe <axboe@kernel.dk>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
> block/blk-lib.c | 32 ++++++++++++++++++++++----------
> 1 files changed, 22 insertions(+), 10 deletions(-)
>
> diff --git a/block/blk-lib.c b/block/blk-lib.c
> index d4bb160..837ce96 100644
> --- a/block/blk-lib.c
> +++ b/block/blk-lib.c
> @@ -44,7 +44,7 @@ int blkdev_issue_discard(struct block_device *bdev, sector_t sector,
> struct request_queue *q = bdev_get_queue(bdev);
> int type = REQ_WRITE | REQ_DISCARD;
> unsigned int max_discard_sectors;
> - unsigned int granularity;
> + unsigned int granularity, alignment, mask;
> struct bio_batch bb;
> struct bio *bio;
> int ret = 0;
> @@ -57,10 +57,12 @@ int blkdev_issue_discard(struct block_device *bdev, sector_t sector,
>
> /* Zero-sector (unknown) and one-sector granularities are the same. */
> granularity = max(q->limits.discard_granularity >> 9, 1U);
> + mask = granularity - 1;

Given that ~ (aligment -1 is a common pattern I thin kthe code would be
better readable without the mask variable.

> + * If splitting a request, and the next starting sector would be
> + * misaligned, stop the discard at the previous aligned sector.
> + */
> + end_sect = sector + req_sects;
> + if (req_sects < nr_sects && (end_sect & mask) != alignment) {
> + end_sect = round_down(end_sect - alignment, granularity) + alignment;

Please avoid overlong lines.

Otherwise the changes look good to me,


Reviewed-by: Christoph Hellwig <hch@lst.de>


\
 
 \ /
  Last update: 2012-03-27 11:09    [W:0.070 / U:0.160 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site