[dm-devel] [PATCH 2/3] dm-thin: fix discard support

Vivek Goyal vgoyal at redhat.com
Tue Jul 17 13:26:05 UTC 2012


On Mon, Jul 16, 2012 at 02:35:18PM -0400, Mikulas Patocka wrote:
> dm-thin: fix discard support
> 
> There is a bug in dm_thin regarding processing discards.
> When dm-thin receives a discard request with size equal to block size
> that is not aligned on block size boundary, io_overlaps_block returns
> true, process_discard treats this discard as a full block discard,
  ^^^^
> deletes the full block - the result is that some data that shouldn't be
> discarded are discarded.

Looking at io_overlaps_block(), it looks like it will return false (and
not true) for bios which are not aligned to block size boundary.

static int io_overlaps_block(struct pool *pool, struct bio *bio)
{
        return !(bio->bi_sector & pool->offset_mask) &&
                (bio->bi_size == (pool->sectors_per_block << SECTOR_SHIFT));

}

Hence for block which crosses block size boundary, we should be sending
discard down for partial block as per the current code and no harm should
be done?


> 
> This patch sets the variable "ti->split_discard_requests", so that
> device mapper core splits discard requests on a block boundary.
> 
> Consequently, a discard request that spans multiple blocks is never sent
> to dm-thin. The patch also removes some code in process_discard that
> deals with discards that span multiple blocks.
> 
> Signed-off-by: Mikulas Patocka <mpatocka at redhat.com>
> 
> ---
>  drivers/md/dm-thin.c |   18 +++++++-----------
>  1 file changed, 7 insertions(+), 11 deletions(-)
> 
> Index: linux-3.5-rc6-fast/drivers/md/dm-thin.c
> ===================================================================
> --- linux-3.5-rc6-fast.orig/drivers/md/dm-thin.c	2012-07-16 18:46:18.000000000 +0200
> +++ linux-3.5-rc6-fast/drivers/md/dm-thin.c	2012-07-16 20:07:19.000000000 +0200
> @@ -1246,17 +1246,10 @@ static void process_discard(struct thin_
>  			}
>  		} else {
>  			/*
> -			 * This path is hit if people are ignoring
> -			 * limits->discard_granularity.  It ignores any
> -			 * part of the discard that is in a subsequent
> -			 * block.
> +			 * The dm makes sure that the discard doesn't span
> +			 * a block boundary. So we submit the discard
> +			 * to the appropriate block.
>  			 */
> -			sector_t offset = pool->sectors_per_block_shift >= 0 ?
> -			      bio->bi_sector & (pool->sectors_per_block - 1) :
> -			      bio->bi_sector - block * pool->sectors_per_block;
> -			unsigned remaining = (pool->sectors_per_block - offset) << SECTOR_SHIFT;
> -			bio->bi_size = min(bio->bi_size, remaining);
> -

So previous code will also send down partial block discard and this code
will also send down partial discard. So nothing has changed from
functionality point of view?

Thanks
Vivek




More information about the dm-devel mailing list