diff mbox series

[3/4] brd: enable discard

Message ID alpine.LRH.2.02.2209160459470.543@file01.intranet.prod.int.rdu2.redhat.com (mailing list archive)
State New, archived
Headers show
Series brd: implement discard | expand

Commit Message

Mikulas Patocka Sept. 16, 2022, 9 a.m. UTC
This patch implements discard in the brd driver. We use RCU to free the
page, so that if there are any concurrent readers or writes, they won't
touch the page after it is freed.

Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>

---
 drivers/block/brd.c |   40 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 40 insertions(+)

Comments

Christoph Hellwig Sept. 20, 2022, 7:39 a.m. UTC | #1
> @@ -289,6 +308,23 @@ static void brd_submit_bio(struct bio *b
>  	struct bio_vec bvec;
>  	struct bvec_iter iter;
>  
> +	if (bio_op(bio) == REQ_OP_DISCARD) {
> +		sector_t len = bio_sectors(bio);
> +		sector_t front_pad = -sector & (PAGE_SECTORS - 1);
> +		sector += front_pad;
> +		if (unlikely(len <= front_pad))
> +			goto endio;
> +		len -= front_pad;
> +		len = round_down(len, PAGE_SECTORS);
> +		while (len) {
> +			brd_free_page(brd, sector);
> +			sector += PAGE_SECTORS;
> +			len -= PAGE_SECTORS;
> +			cond_resched();
> +		}
> +		goto endio;
> +	}
> +
>  	bio_for_each_segment(bvec, bio, iter) {

Please add separate helpers to each type of IO and just make the
main submit_bio method a dispatch on the types instead of this
spaghetti code.

> +	disk->queue->limits.discard_granularity = PAGE_SIZE;
> +	blk_queue_max_discard_sectors(disk->queue, UINT_MAX);

We'll probably want an opt in for this new feature.
Mikulas Patocka Sept. 20, 2022, 5:47 p.m. UTC | #2
On Tue, 20 Sep 2022, Christoph Hellwig wrote:

> > @@ -289,6 +308,23 @@ static void brd_submit_bio(struct bio *b
> >  	struct bio_vec bvec;
> >  	struct bvec_iter iter;
> >  
> > +	if (bio_op(bio) == REQ_OP_DISCARD) {
> > +		sector_t len = bio_sectors(bio);
> > +		sector_t front_pad = -sector & (PAGE_SECTORS - 1);
> > +		sector += front_pad;
> > +		if (unlikely(len <= front_pad))
> > +			goto endio;
> > +		len -= front_pad;
> > +		len = round_down(len, PAGE_SECTORS);
> > +		while (len) {
> > +			brd_free_page(brd, sector);
> > +			sector += PAGE_SECTORS;
> > +			len -= PAGE_SECTORS;
> > +			cond_resched();
> > +		}
> > +		goto endio;
> > +	}
> > +
> >  	bio_for_each_segment(bvec, bio, iter) {
> 
> Please add separate helpers to each type of IO and just make the
> main submit_bio method a dispatch on the types instead of this
> spaghetti code.
> 
> > +	disk->queue->limits.discard_granularity = PAGE_SIZE;
> > +	blk_queue_max_discard_sectors(disk->queue, UINT_MAX);
> 
> We'll probably want an opt in for this new feature.

OK. I addressed these concerns and I'll send a second version of the patch 
set.

Mikulas
diff mbox series

Patch

Index: linux-2.6/drivers/block/brd.c
===================================================================
--- linux-2.6.orig/drivers/block/brd.c
+++ linux-2.6/drivers/block/brd.c
@@ -112,6 +112,25 @@  static bool brd_insert_page(struct brd_d
 	return true;
 }
 
+static void brd_free_page_rcu(struct rcu_head *head)
+{
+	struct page *page = container_of(head, struct page, rcu_head);
+	__free_page(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)
+		call_rcu(&page->rcu_head, brd_free_page_rcu);
+}
+
 /*
  * Free all backing store pages and radix tree. This must only be called when
  * there are no other users of the device.
@@ -289,6 +308,23 @@  static void brd_submit_bio(struct bio *b
 	struct bio_vec bvec;
 	struct bvec_iter iter;
 
+	if (bio_op(bio) == REQ_OP_DISCARD) {
+		sector_t len = bio_sectors(bio);
+		sector_t front_pad = -sector & (PAGE_SECTORS - 1);
+		sector += front_pad;
+		if (unlikely(len <= front_pad))
+			goto endio;
+		len -= front_pad;
+		len = round_down(len, PAGE_SECTORS);
+		while (len) {
+			brd_free_page(brd, sector);
+			sector += PAGE_SECTORS;
+			len -= PAGE_SECTORS;
+			cond_resched();
+		}
+		goto endio;
+	}
+
 	bio_for_each_segment(bvec, bio, iter) {
 		unsigned int len = bvec.bv_len;
 		int err;
@@ -306,6 +342,7 @@  static void brd_submit_bio(struct bio *b
 		sector += len >> SECTOR_SHIFT;
 	}
 
+endio:
 	bio_endio(bio);
 }
 
@@ -409,6 +446,9 @@  static int brd_alloc(int i)
 	 */
 	blk_queue_physical_block_size(disk->queue, PAGE_SIZE);
 
+	disk->queue->limits.discard_granularity = PAGE_SIZE;
+	blk_queue_max_discard_sectors(disk->queue, UINT_MAX);
+
 	/* Tell the block layer that this is not a rotational device */
 	blk_queue_flag_set(QUEUE_FLAG_NONROT, disk->queue);
 	blk_queue_flag_clear(QUEUE_FLAG_ADD_RANDOM, disk->queue);