[patch] brd: support discard

[patch] brd: support discard

[Nick Piggin]

Hi Jens,

What do you think of this patch? Can you possibly merge it via your
block tree if it looks OK?

Thanks,
Nick
--

brd: support discard

Support discard requests in brd by zeroing or deleting the underlying backing
pages. This is simply to help with testing and documentation nature of
brd code.

Signed-off-by: Nick Piggin <npiggin@suse.de>
---
Index: linux-2.6/drivers/block/brd.c
===================================================================
--- linux-2.6.orig/drivers/block/brd.c
+++ linux-2.6/drivers/block/brd.c
@@ -133,6 +133,28 @@ static struct page *brd_insert_page(stru
 	return page;
 }
 
+static void brd_free_page(struct brd_device *brd, sector_t sector)
+{
+	struct page *page;
+	pgoff_t idx;
+
+	spin_lock(&brd->brd_lock);
+	idx = sector >> PAGE_SECTORS_SHIFT;
+	page = radix_tree_delete(&brd->brd_pages, idx);
+	spin_unlock(&brd->brd_lock);
+	if (page)
+		__free_page(page);
+}
+
+static void brd_zero_page(struct brd_device *brd, sector_t sector)
+{
+	struct page *page;
+
+	page = brd_lookup_page(brd, sector);
+	if (page)
+		clear_highpage(page);
+}
+
 /*
  * Free all backing store pages and radix tree. This must only be called when
  * there are no other users of the device.
@@ -189,6 +211,24 @@ static int copy_to_brd_setup(struct brd_
 	return 0;
 }
 
+static void discard_from_brd(struct brd_device *brd,
+			sector_t sector, size_t n)
+{
+	while (n >= PAGE_SIZE) {
+		/*
+		 * Don't want to actually discard pages here because
+		 * re-allocating the pages can result in writeback
+		 * deadlocks under heavy load.
+		 */
+		if (0)
+			brd_free_page(brd, sector);
+		else
+			brd_zero_page(brd, sector);
+		sector += PAGE_SIZE >> SECTOR_SHIFT;
+		n -= PAGE_SIZE;
+	}
+}
+
 /*
  * Copy n bytes from src to the brd starting at sector. Does not sleep.
  */
@@ -300,6 +340,12 @@ static int brd_make_request(struct reque
 						get_capacity(bdev->bd_disk))
 		goto out;
 
+	if (unlikely(bio_rw_flagged(bio, BIO_RW_DISCARD))) {
+		err = 0;
+		discard_from_brd(brd, sector, bio->bi_size);
+		goto out;
+	}
+
 	rw = bio_rw(bio);
 	if (rw == READA)
 		rw = READ;
@@ -320,7 +366,7 @@ out:
 }
 
 #ifdef CONFIG_BLK_DEV_XIP
-static int brd_direct_access (struct block_device *bdev, sector_t sector,
+static int brd_direct_access(struct block_device *bdev, sector_t sector,
 			void **kaddr, unsigned long *pfn)
 {
 	struct brd_device *brd = bdev->bd_disk->private_data;
@@ -437,6 +483,11 @@ static struct brd_device *brd_alloc(int
 	blk_queue_max_hw_sectors(brd->brd_queue, 1024);
 	blk_queue_bounce_limit(brd->brd_queue, BLK_BOUNCE_ANY);
 
+	brd->brd_queue->limits.discard_granularity = PAGE_SIZE;
+	brd->brd_queue->limits.max_discard_sectors = UINT_MAX;
+	brd->brd_queue->limits.discard_zeroes_data = 1;
+	queue_flag_set_unlocked(QUEUE_FLAG_DISCARD, brd->brd_queue);
+
 	disk = brd->brd_disk = alloc_disk(1 << part_shift);
 	if (!disk)
 		goto out_free_queue;

Re: [patch] brd: support discard

[Jens Axboe]

On Wed, May 26 2010, Nick Piggin wrote:
> Hi Jens,
> 
> What do you think of this patch? Can you possibly merge it via your
> block tree if it looks OK?

Sure, looks trivial enough (and correct, from a quick look through).

-- 
Jens Axboe

Re: [patch] brd: support discard

[Nick Piggin]

On Wed, May 26, 2010 at 03:40:06PM +0200, Jens Axboe wrote:
> On Wed, May 26 2010, Nick Piggin wrote:
> > Hi Jens,
> > 
> > What do you think of this patch? Can you possibly merge it via your
> > block tree if it looks OK?
> 
> Sure, looks trivial enough (and correct, from a quick look through).

Thanks. It passes some basic testing (via blockdev discard ioctl and
btrfs -odiscard mount) and it looks to be working properly.

Re: [patch] brd: support discard

[Kyle McMartin]

On Wed, May 26, 2010 at 11:33:42PM +1000, Nick Piggin wrote:
> +static void discard_from_brd(struct brd_device *brd,
> +			sector_t sector, size_t n)
> +{
> +	while (n >= PAGE_SIZE) {
> +		/*
> +		 * Don't want to actually discard pages here because
> +		 * re-allocating the pages can result in writeback
> +		 * deadlocks under heavy load.
> +		 */
> +		if (0)
> +			brd_free_page(brd, sector);
> +		else
> +			brd_zero_page(brd, sector);

That if (0) looks a little suspicious... ;-)

regards, Kyle