linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] fs/btrfs: Fix raid6 qstripe kmap'ing
@ 2021-01-28  6:15 ira.weiny
  2021-02-03 15:56 ` David Sterba
  0 siblings, 1 reply; 6+ messages in thread
From: ira.weiny @ 2021-01-28  6:15 UTC (permalink / raw)
  To: clm, josef, dsterba; +Cc: Ira Weiny, Miao Xie, linux-kernel, linux-fsdevel

From: Ira Weiny <ira.weiny@intel.com>

When a qstripe is required an extra page is allocated and mapped.  There
were 3 problems.

1) There is no reason to map the qstripe page more than 1 time if the
   number of bits set in rbio->dbitmap is greater than one.
2) There is no reason to map the parity page and unmap it each time
   through the loop.
3) There is no corresponding call of kunmap() for the qstripe page.

The page memory can continue to be reused with a single mapping on each
iteration by raid6_call.gen_syndrome() without remapping.  So map the
page for the duration of the loop.

Similarly, improve the algorithm by mapping the parity page just 1 time.

Fixes: 5a6ac9eacb49 ("Btrfs, raid56: support parity scrub on raid56")
To: Chris Mason <clm@fb.com>
To: Josef Bacik <josef@toxicpanda.com>
To: David Sterba <dsterba@suse.com>
Cc: Miao Xie <miaox@cn.fujitsu.com>
Signed-off-by: Ira Weiny <ira.weiny@intel.com>

---
This was found while replacing kmap() with kmap_local_page().  After
this patch unwinding all the mappings becomes pretty straight forward.

I'm not exactly sure I've worded this commit message intelligently.
Please forgive me if there is a better way to word it.
---
 fs/btrfs/raid56.c | 21 ++++++++++-----------
 1 file changed, 10 insertions(+), 11 deletions(-)

diff --git a/fs/btrfs/raid56.c b/fs/btrfs/raid56.c
index 93fbf87bdc8d..b8a39dad0f00 100644
--- a/fs/btrfs/raid56.c
+++ b/fs/btrfs/raid56.c
@@ -2363,16 +2363,21 @@ static noinline void finish_parity_scrub(struct btrfs_raid_bio *rbio,
 	SetPageUptodate(p_page);
 
 	if (has_qstripe) {
+		/* raid6, allocate and map temp space for the qstripe */
 		q_page = alloc_page(GFP_NOFS | __GFP_HIGHMEM);
 		if (!q_page) {
 			__free_page(p_page);
 			goto cleanup;
 		}
 		SetPageUptodate(q_page);
+		pointers[rbio->real_stripes] = kmap(q_page);
 	}
 
 	atomic_set(&rbio->error, 0);
 
+	/* map the parity stripe just once */
+	pointers[nr_data] = kmap(p_page);
+
 	for_each_set_bit(pagenr, rbio->dbitmap, rbio->stripe_npages) {
 		struct page *p;
 		void *parity;
@@ -2382,16 +2387,8 @@ static noinline void finish_parity_scrub(struct btrfs_raid_bio *rbio,
 			pointers[stripe] = kmap(p);
 		}
 
-		/* then add the parity stripe */
-		pointers[stripe++] = kmap(p_page);
-
 		if (has_qstripe) {
-			/*
-			 * raid6, add the qstripe and call the
-			 * library function to fill in our p/q
-			 */
-			pointers[stripe++] = kmap(q_page);
-
+			/* raid6, call the library function to fill in our p/q */
 			raid6_call.gen_syndrome(rbio->real_stripes, PAGE_SIZE,
 						pointers);
 		} else {
@@ -2412,12 +2409,14 @@ static noinline void finish_parity_scrub(struct btrfs_raid_bio *rbio,
 
 		for (stripe = 0; stripe < nr_data; stripe++)
 			kunmap(page_in_rbio(rbio, stripe, pagenr, 0));
-		kunmap(p_page);
 	}
 
+	kunmap(p_page);
 	__free_page(p_page);
-	if (q_page)
+	if (q_page) {
+		kunmap(q_page);
 		__free_page(q_page);
+	}
 
 writeback:
 	/*
-- 
2.28.0.rc0.12.gb6a658bd00c9


^ permalink raw reply related	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2021-02-05 23:51 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-28  6:15 [PATCH] fs/btrfs: Fix raid6 qstripe kmap'ing ira.weiny
2021-02-03 15:56 ` David Sterba
2021-02-04 15:26   ` David Sterba
2021-02-05  3:52     ` Ira Weiny
2021-02-05 15:34       ` David Sterba
2021-02-05 16:39         ` Ira Weiny

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).