Re: [PATCH 2/7] block: use blkdev_issue_discard inblk_ioctl_discard

[Christoph Hellwig]

Jens, can we put this one and possible Dave's treat discard requests
as writes patch in early for 2.6.32?  That'll make the discard patch
queue quite a bit smaller and will allow easier experimenting.

On Sat, Aug 29, 2009 at 07:03:34PM -0400, Christoph Hellwig wrote:
> blk_ioctl_discard duplicates large amounts of code from blkdev_issue_discard,
> the only difference between the two is that blkdev_issue_discard needs to
> send a barrier discard request and blk_ioctl_discard a non-barrier one,
> and blk_ioctl_discard needs to wait on the request.  To facilitates this
> add a flags argument to blkdev_issue_discard to control both aspects of the
> behaviour.  This will be very useful later on for using the waiting
> funcitonality for other callers.
> 
> Based on an earlier patch from Matthew Wilcox <matthew@xxxxxx>.
> 
> 
> Signed-off-by: Christoph Hellwig <hch@xxxxxx>
> 
> Index: linux-2.6/block/blk-barrier.c
> ===================================================================
> --- linux-2.6.orig/block/blk-barrier.c	2009-08-29 16:49:43.067370900 -0300
> +++ linux-2.6/block/blk-barrier.c	2009-08-29 17:43:30.407344330 -0300
> @@ -348,6 +348,9 @@ static void blkdev_discard_end_io(struct
>  		clear_bit(BIO_UPTODATE, &bio->bi_flags);
>  	}
>  
> +	if (bio->bi_private)
> +		complete(bio->bi_private);
> +
>  	bio_put(bio);
>  }
>  
> @@ -357,21 +360,20 @@ static void blkdev_discard_end_io(struct
>   * @sector:	start sector
>   * @nr_sects:	number of sectors to discard
>   * @gfp_mask:	memory allocation flags (for bio_alloc)
> + * @flags:	DISCARD_FL_* flags to control behaviour
>   *
>   * Description:
> - *    Issue a discard request for the sectors in question. Does not wait.
> + *    Issue a discard request for the sectors in question.
>   */
> -int blkdev_issue_discard(struct block_device *bdev,
> -			 sector_t sector, sector_t nr_sects, gfp_t gfp_mask)
> +int blkdev_issue_discard(struct block_device *bdev, sector_t sector,
> +		sector_t nr_sects, gfp_t gfp_mask, int flags)
>  {
> -	struct request_queue *q;
> -	struct bio *bio;
> +	DECLARE_COMPLETION_ONSTACK(wait);
> +	struct request_queue *q = bdev_get_queue(bdev);
> +	int type = flags & DISCARD_FL_BARRIER ?
> +		DISCARD_BARRIER : DISCARD_NOBARRIER;
>  	int ret = 0;
>  
> -	if (bdev->bd_disk == NULL)
> -		return -ENXIO;
> -
> -	q = bdev_get_queue(bdev);
>  	if (!q)
>  		return -ENXIO;
>  
> @@ -379,12 +381,14 @@ int blkdev_issue_discard(struct block_de
>  		return -EOPNOTSUPP;
>  
>  	while (nr_sects && !ret) {
> -		bio = bio_alloc(gfp_mask, 0);
> +		struct bio *bio = bio_alloc(gfp_mask, 0);
>  		if (!bio)
>  			return -ENOMEM;
>  
>  		bio->bi_end_io = blkdev_discard_end_io;
>  		bio->bi_bdev = bdev;
> +		if (flags & DISCARD_FL_WAIT)
> +			bio->bi_private = &wait;
>  
>  		bio->bi_sector = sector;
>  
> @@ -396,10 +400,13 @@ int blkdev_issue_discard(struct block_de
>  			bio->bi_size = nr_sects << 9;
>  			nr_sects = 0;
>  		}
> +
>  		bio_get(bio);
> -		submit_bio(DISCARD_BARRIER, bio);
> +		submit_bio(type, bio);
> +
> +		if (flags & DISCARD_FL_WAIT)
> +			wait_for_completion(&wait);
>  
> -		/* Check if it failed immediately */
>  		if (bio_flagged(bio, BIO_EOPNOTSUPP))
>  			ret = -EOPNOTSUPP;
>  		else if (!bio_flagged(bio, BIO_UPTODATE))
> Index: linux-2.6/block/ioctl.c
> ===================================================================
> --- linux-2.6.orig/block/ioctl.c	2009-08-29 16:49:43.075370172 -0300
> +++ linux-2.6/block/ioctl.c	2009-08-29 16:49:52.743341374 -0300
> @@ -112,22 +112,9 @@ static int blkdev_reread_part(struct blo
>  	return res;
>  }
>  
> -static void blk_ioc_discard_endio(struct bio *bio, int err)
> -{
> -	if (err) {
> -		if (err == -EOPNOTSUPP)
> -			set_bit(BIO_EOPNOTSUPP, &bio->bi_flags);
> -		clear_bit(BIO_UPTODATE, &bio->bi_flags);
> -	}
> -	complete(bio->bi_private);
> -}
> -
>  static int blk_ioctl_discard(struct block_device *bdev, uint64_t start,
>  			     uint64_t len)
>  {
> -	struct request_queue *q = bdev_get_queue(bdev);
> -	int ret = 0;
> -
>  	if (start & 511)
>  		return -EINVAL;
>  	if (len & 511)
> @@ -137,40 +124,8 @@ static int blk_ioctl_discard(struct bloc
>  
>  	if (start + len > (bdev->bd_inode->i_size >> 9))
>  		return -EINVAL;
> -
> -	if (!q->prepare_discard_fn)
> -		return -EOPNOTSUPP;
> -
> -	while (len && !ret) {
> -		DECLARE_COMPLETION_ONSTACK(wait);
> -		struct bio *bio;
> -
> -		bio = bio_alloc(GFP_KERNEL, 0);
> -
> -		bio->bi_end_io = blk_ioc_discard_endio;
> -		bio->bi_bdev = bdev;
> -		bio->bi_private = &wait;
> -		bio->bi_sector = start;
> -
> -		if (len > queue_max_hw_sectors(q)) {
> -			bio->bi_size = queue_max_hw_sectors(q) << 9;
> -			len -= queue_max_hw_sectors(q);
> -			start += queue_max_hw_sectors(q);
> -		} else {
> -			bio->bi_size = len << 9;
> -			len = 0;
> -		}
> -		submit_bio(DISCARD_NOBARRIER, bio);
> -
> -		wait_for_completion(&wait);
> -
> -		if (bio_flagged(bio, BIO_EOPNOTSUPP))
> -			ret = -EOPNOTSUPP;
> -		else if (!bio_flagged(bio, BIO_UPTODATE))
> -			ret = -EIO;
> -		bio_put(bio);
> -	}
> -	return ret;
> +	return blkdev_issue_discard(bdev, start, len, GFP_KERNEL,
> +				    DISCARD_FL_WAIT);
>  }
>  
>  static int put_ushort(unsigned long arg, unsigned short val)
> Index: linux-2.6/fs/btrfs/extent-tree.c
> ===================================================================
> --- linux-2.6.orig/fs/btrfs/extent-tree.c	2009-08-29 16:49:43.079341766 -0300
> +++ linux-2.6/fs/btrfs/extent-tree.c	2009-08-29 16:49:52.743341374 -0300
> @@ -1511,7 +1511,8 @@ static int remove_extent_backref(struct 
>  static void btrfs_issue_discard(struct block_device *bdev,
>  				u64 start, u64 len)
>  {
> -	blkdev_issue_discard(bdev, start >> 9, len >> 9, GFP_KERNEL);
> +	blkdev_issue_discard(bdev, start >> 9, len >> 9, GFP_KERNEL,
> +			     DISCARD_FL_BARRIER);
>  }
>  #endif
>  
> Index: linux-2.6/fs/gfs2/rgrp.c
> ===================================================================
> --- linux-2.6.orig/fs/gfs2/rgrp.c	2009-08-29 16:49:43.139342340 -0300
> +++ linux-2.6/fs/gfs2/rgrp.c	2009-08-29 16:49:52.747373241 -0300
> @@ -857,7 +857,8 @@ static void gfs2_rgrp_send_discards(stru
>  					goto start_new_extent;
>  				if ((start + nr_sects) != blk) {
>  					rv = blkdev_issue_discard(bdev, start,
> -							    nr_sects, GFP_NOFS);
> +							    nr_sects, GFP_NOFS,
> +							    DISCARD_FL_BARRIER);
>  					if (rv)
>  						goto fail;
>  					nr_sects = 0;
> @@ -871,7 +872,8 @@ start_new_extent:
>  		}
>  	}
>  	if (nr_sects) {
> -		rv = blkdev_issue_discard(bdev, start, nr_sects, GFP_NOFS);
> +		rv = blkdev_issue_discard(bdev, start, nr_sects, GFP_NOFS,
> +					 DISCARD_FL_BARRIER);
>  		if (rv)
>  			goto fail;
>  	}
> Index: linux-2.6/include/linux/blkdev.h
> ===================================================================
> --- linux-2.6.orig/include/linux/blkdev.h	2009-08-29 16:49:43.147343148 -0300
> +++ linux-2.6/include/linux/blkdev.h	2009-08-29 17:43:19.767342397 -0300
> @@ -977,15 +977,18 @@ static inline struct request *blk_map_qu
>  }
>  
>  extern int blkdev_issue_flush(struct block_device *, sector_t *);
> -extern int blkdev_issue_discard(struct block_device *,
> -				sector_t sector, sector_t nr_sects, gfp_t);
> +#define DISCARD_FL_WAIT		0x01	/* wait for completion */
> +#define DISCARD_FL_BARRIER	0x02	/* issue DISCARD_BARRIER request */
> +extern int blkdev_issue_discard(struct block_device *, sector_t sector,
> +		sector_t nr_sects, gfp_t, int flags);
>  
>  static inline int sb_issue_discard(struct super_block *sb,
>  				   sector_t block, sector_t nr_blocks)
>  {
>  	block <<= (sb->s_blocksize_bits - 9);
>  	nr_blocks <<= (sb->s_blocksize_bits - 9);
> -	return blkdev_issue_discard(sb->s_bdev, block, nr_blocks, GFP_KERNEL);
> +	return blkdev_issue_discard(sb->s_bdev, block, nr_blocks, GFP_KERNEL,
> +				    DISCARD_FL_BARRIER);
>  }
>  
>  extern int blk_verify_command(unsigned char *cmd, fmode_t has_write_perm);
> Index: linux-2.6/mm/swapfile.c
> ===================================================================
> --- linux-2.6.orig/mm/swapfile.c	2009-08-29 16:49:43.151342120 -0300
> +++ linux-2.6/mm/swapfile.c	2009-08-29 16:49:52.755377749 -0300
> @@ -161,7 +161,8 @@ static int discard_swap(struct swap_info
>  		}
>  
>  		err = blkdev_issue_discard(si->bdev, start_block,
> -						nr_blocks, GFP_KERNEL);
> +						nr_blocks, GFP_KERNEL,
> +						DISCARD_FL_BARRIER);
>  		if (err)
>  			break;
>  
> @@ -200,7 +201,8 @@ static void discard_swap_cluster(struct 
>  			start_block <<= PAGE_SHIFT - 9;
>  			nr_blocks <<= PAGE_SHIFT - 9;
>  			if (blkdev_issue_discard(si->bdev, start_block,
> -							nr_blocks, GFP_NOIO))
> +							nr_blocks, GFP_NOIO,
> +							DISCARD_FL_BARRIER))
>  				break;
>  		}
>  
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
---end quoted text---
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/