[PATCH v2 0/3] md: three misc changes

[PATCH v2 0/3] md: three misc changes

[Ming Lei]

This 1st patch fixes one issue introduced in the following two
commits:
	Fixes: f0250618361d(md: raid10: don't use bio's vec table to manage resync pages)
	Fixes: 98d30c5812c3(md: raid1: don't use bio's vec table to manage resync pages)

The 2nd one initializes bvec table of bio via bio_add_page() after bio_reset().

The 3rd one moves the common definitation and helpers into raid1-10.c. 

V2:
	- fix 'page_idx' increasement in patch 1
	- move raid1/raid10 common code to raid1-raid10.c, as suggested by Neil

Ming Lei (3):
  md: remove 'idx' from 'struct resync_pages'
  md: raid1/raid10: initialize bvec table via bio_add_page()
  md: raid1-10: move raid1/raid10 common code into raid1-10.c

 drivers/md/md.h       | 54 ----------------------------------
 drivers/md/raid1-10.c | 81 +++++++++++++++++++++++++++++++++++++++++++++++++++
 drivers/md/raid1.c    | 31 ++++----------------
 drivers/md/raid10.c   | 19 ++++--------
 4 files changed, 93 insertions(+), 92 deletions(-)
 create mode 100644 drivers/md/raid1-10.c

-- 
2.9.4

[PATCH v2 1/3] md: remove 'idx' from 'struct resync_pages'

[Ming Lei]

bio_add_page() won't fail for resync bio, and the page index for each
bio is same, so remove it.

More importantly the 'idx' of 'struct resync_pages' is initialized in
mempool allocator function, the current way is wrong since mempool is
only responsible for allocation, we can't use that for initialization.

Suggested-by: NeilBrown <neilb@suse.com>
Reported-by: NeilBrown <neilb@suse.com>
Fixes: f0250618361d(md: raid10: don't use bio's vec table to manage resync pages)
Fixes: 98d30c5812c3(md: raid1: don't use bio's vec table to manage resync pages)
Signed-off-by: Ming Lei <ming.lei@redhat.com>
---
 drivers/md/md.h     | 1 -
 drivers/md/raid1.c  | 6 +++---
 drivers/md/raid10.c | 6 +++---
 3 files changed, 6 insertions(+), 7 deletions(-)

diff --git a/drivers/md/md.h b/drivers/md/md.h
index 991f0fe2dcc6..2c780aa8d07f 100644
--- a/drivers/md/md.h
+++ b/drivers/md/md.h
@@ -736,7 +736,6 @@ static inline void mddev_check_write_zeroes(struct mddev *mddev, struct bio *bio
 
 /* for managing resync I/O pages */
 struct resync_pages {
-	unsigned	idx;	/* for get/put page from the pool */
 	void		*raid_bio;
 	struct page	*pages[RESYNC_PAGES];
 };
diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
index 3febfc8391fb..0896c772a560 100644
--- a/drivers/md/raid1.c
+++ b/drivers/md/raid1.c
@@ -170,7 +170,6 @@ static void * r1buf_pool_alloc(gfp_t gfp_flags, void *data)
 			resync_get_all_pages(rp);
 		}
 
-		rp->idx = 0;
 		rp->raid_bio = r1_bio;
 		bio->bi_private = rp;
 	}
@@ -2619,6 +2618,7 @@ static sector_t raid1_sync_request(struct mddev *mddev, sector_t sector_nr,
 	int good_sectors = RESYNC_SECTORS;
 	int min_bad = 0; /* number of sectors that are bad in all devices */
 	int idx = sector_to_idx(sector_nr);
+	int page_idx = 0;
 
 	if (!conf->r1buf_pool)
 		if (init_resync(conf))
@@ -2846,7 +2846,7 @@ static sector_t raid1_sync_request(struct mddev *mddev, sector_t sector_nr,
 			bio = r1_bio->bios[i];
 			rp = get_resync_pages(bio);
 			if (bio->bi_end_io) {
-				page = resync_fetch_page(rp, rp->idx++);
+				page = resync_fetch_page(rp, page_idx);
 
 				/*
 				 * won't fail because the vec table is big
@@ -2858,7 +2858,7 @@ static sector_t raid1_sync_request(struct mddev *mddev, sector_t sector_nr,
 		nr_sectors += len>>9;
 		sector_nr += len>>9;
 		sync_blocks -= (len>>9);
-	} while (get_resync_pages(r1_bio->bios[disk]->bi_private)->idx < RESYNC_PAGES);
+	} while (++page_idx < RESYNC_PAGES);
 
 	r1_bio->sectors = nr_sectors;
 
diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
index 5026e7ad51d3..fa8bcf04e791 100644
--- a/drivers/md/raid10.c
+++ b/drivers/md/raid10.c
@@ -221,7 +221,6 @@ static void * r10buf_pool_alloc(gfp_t gfp_flags, void *data)
 			resync_get_all_pages(rp);
 		}
 
-		rp->idx = 0;
 		rp->raid_bio = r10_bio;
 		bio->bi_private = rp;
 		if (rbio) {
@@ -2853,6 +2852,7 @@ static sector_t raid10_sync_request(struct mddev *mddev, sector_t sector_nr,
 	sector_t sectors_skipped = 0;
 	int chunks_skipped = 0;
 	sector_t chunk_mask = conf->geo.chunk_mask;
+	int page_idx = 0;
 
 	if (!conf->r10buf_pool)
 		if (init_resync(conf))
@@ -3355,7 +3355,7 @@ static sector_t raid10_sync_request(struct mddev *mddev, sector_t sector_nr,
 			break;
 		for (bio= biolist ; bio ; bio=bio->bi_next) {
 			struct resync_pages *rp = get_resync_pages(bio);
-			page = resync_fetch_page(rp, rp->idx++);
+			page = resync_fetch_page(rp, page_idx);
 			/*
 			 * won't fail because the vec table is big enough
 			 * to hold all these pages
@@ -3364,7 +3364,7 @@ static sector_t raid10_sync_request(struct mddev *mddev, sector_t sector_nr,
 		}
 		nr_sectors += len>>9;
 		sector_nr += len>>9;
-	} while (get_resync_pages(biolist)->idx < RESYNC_PAGES);
+	} while (++page_idx < RESYNC_PAGES);
 	r10_bio->sectors = nr_sectors;
 
 	while (biolist) {
-- 
2.9.4

[PATCH v2 2/3] md: raid1/raid10: initialize bvec table via bio_add_page()

[Ming Lei]

We will support multipage bvec soon, so initialize bvec
table using the standardy way instead of writing the
talbe directly. Otherwise it won't work any more once
multipage bvec is enabled.

Acked-by: Guoqing Jiang <gqjiang@suse.com>
Signed-off-by: Ming Lei <ming.lei@redhat.com>
---
 drivers/md/raid1-10.c | 19 +++++++++++++++++++
 drivers/md/raid1.c    | 18 ++++--------------
 drivers/md/raid10.c   |  6 ++++--
 3 files changed, 27 insertions(+), 16 deletions(-)
 create mode 100644 drivers/md/raid1-10.c

diff --git a/drivers/md/raid1-10.c b/drivers/md/raid1-10.c
new file mode 100644
index 000000000000..3adb5b9dc4b4
--- /dev/null
+++ b/drivers/md/raid1-10.c
@@ -0,0 +1,19 @@
+/* generally called after bio_reset() for reseting bvec */
+static void md_bio_reset_resync_pages(struct bio *bio, struct resync_pages *rp,
+			       int size)
+{
+	int idx = 0;
+
+	/* initialize bvec table again */
+	do {
+		struct page *page = resync_fetch_page(rp, idx);
+		int len = min_t(int, size, PAGE_SIZE);
+
+		/*
+		 * won't fail because the vec table is big
+		 * enough to hold all these pages
+		 */
+		bio_add_page(bio, page, len, 0);
+		size -= len;
+	} while (idx++ < RESYNC_PAGES && size > 0);
+}
diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
index 0896c772a560..fe86ab18961b 100644
--- a/drivers/md/raid1.c
+++ b/drivers/md/raid1.c
@@ -81,6 +81,8 @@ static void lower_barrier(struct r1conf *conf, sector_t sector_nr);
 #define raid1_log(md, fmt, args...)				\
 	do { if ((md)->queue) blk_add_trace_msg((md)->queue, "raid1 " fmt, ##args); } while (0)
 
+#include "raid1-10.c"
+
 /*
  * 'strct resync_pages' stores actual pages used for doing the resync
  *  IO, and it is per-bio, so make .bi_private points to it.
@@ -2085,10 +2087,7 @@ static void process_checks(struct r1bio *r1_bio)
 	/* Fix variable parts of all bios */
 	vcnt = (r1_bio->sectors + PAGE_SIZE / 512 - 1) >> (PAGE_SHIFT - 9);
 	for (i = 0; i < conf->raid_disks * 2; i++) {
-		int j;
-		int size;
 		blk_status_t status;
-		struct bio_vec *bi;
 		struct bio *b = r1_bio->bios[i];
 		struct resync_pages *rp = get_resync_pages(b);
 		if (b->bi_end_io != end_sync_read)
@@ -2097,8 +2096,6 @@ static void process_checks(struct r1bio *r1_bio)
 		status = b->bi_status;
 		bio_reset(b);
 		b->bi_status = status;
-		b->bi_vcnt = vcnt;
-		b->bi_iter.bi_size = r1_bio->sectors << 9;
 		b->bi_iter.bi_sector = r1_bio->sector +
 			conf->mirrors[i].rdev->data_offset;
 		b->bi_bdev = conf->mirrors[i].rdev->bdev;
@@ -2106,15 +2103,8 @@ static void process_checks(struct r1bio *r1_bio)
 		rp->raid_bio = r1_bio;
 		b->bi_private = rp;
 
-		size = b->bi_iter.bi_size;
-		bio_for_each_segment_all(bi, b, j) {
-			bi->bv_offset = 0;
-			if (size > PAGE_SIZE)
-				bi->bv_len = PAGE_SIZE;
-			else
-				bi->bv_len = size;
-			size -= PAGE_SIZE;
-		}
+		/* initialize bvec table again */
+		md_bio_reset_resync_pages(b, rp, r1_bio->sectors << 9);
 	}
 	for (primary = 0; primary < conf->raid_disks * 2; primary++)
 		if (r1_bio->bios[primary]->bi_end_io == end_sync_read &&
diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
index fa8bcf04e791..9952721e1cde 100644
--- a/drivers/md/raid10.c
+++ b/drivers/md/raid10.c
@@ -110,6 +110,8 @@ static void end_reshape(struct r10conf *conf);
 #define raid10_log(md, fmt, args...)				\
 	do { if ((md)->queue) blk_add_trace_msg((md)->queue, "raid10 " fmt, ##args); } while (0)
 
+#include "raid1-10.c"
+
 /*
  * 'strct resync_pages' stores actual pages used for doing the resync
  *  IO, and it is per-bio, so make .bi_private points to it.
@@ -2086,8 +2088,8 @@ static void sync_request_write(struct mddev *mddev, struct r10bio *r10_bio)
 		rp = get_resync_pages(tbio);
 		bio_reset(tbio);
 
-		tbio->bi_vcnt = vcnt;
-		tbio->bi_iter.bi_size = fbio->bi_iter.bi_size;
+		md_bio_reset_resync_pages(tbio, rp, fbio->bi_iter.bi_size);
+
 		rp->raid_bio = r10_bio;
 		tbio->bi_private = rp;
 		tbio->bi_iter.bi_sector = r10_bio->devs[i].addr;
-- 
2.9.4

[PATCH v2 3/3] md: raid1-10: move raid1/raid10 common code into raid1-10.c

[Ming Lei]

No function change, just move 'struct resync_pages' and related
helpers into raid1-10.c

Signed-off-by: Ming Lei <ming.lei@redhat.com>
---
 drivers/md/md.h       | 53 -------------------------------------------
 drivers/md/raid1-10.c | 62 +++++++++++++++++++++++++++++++++++++++++++++++++++
 drivers/md/raid1.c    |  9 --------
 drivers/md/raid10.c   |  9 --------
 4 files changed, 62 insertions(+), 71 deletions(-)

diff --git a/drivers/md/md.h b/drivers/md/md.h
index 2c780aa8d07f..004a21c214e8 100644
--- a/drivers/md/md.h
+++ b/drivers/md/md.h
@@ -729,57 +729,4 @@ static inline void mddev_check_write_zeroes(struct mddev *mddev, struct bio *bio
 	    !bdev_get_queue(bio->bi_bdev)->limits.max_write_zeroes_sectors)
 		mddev->queue->limits.max_write_zeroes_sectors = 0;
 }
-
-/* Maximum size of each resync request */
-#define RESYNC_BLOCK_SIZE (64*1024)
-#define RESYNC_PAGES ((RESYNC_BLOCK_SIZE + PAGE_SIZE-1) / PAGE_SIZE)
-
-/* for managing resync I/O pages */
-struct resync_pages {
-	void		*raid_bio;
-	struct page	*pages[RESYNC_PAGES];
-};
-
-static inline int resync_alloc_pages(struct resync_pages *rp,
-				     gfp_t gfp_flags)
-{
-	int i;
-
-	for (i = 0; i < RESYNC_PAGES; i++) {
-		rp->pages[i] = alloc_page(gfp_flags);
-		if (!rp->pages[i])
-			goto out_free;
-	}
-
-	return 0;
-
-out_free:
-	while (--i >= 0)
-		put_page(rp->pages[i]);
-	return -ENOMEM;
-}
-
-static inline void resync_free_pages(struct resync_pages *rp)
-{
-	int i;
-
-	for (i = 0; i < RESYNC_PAGES; i++)
-		put_page(rp->pages[i]);
-}
-
-static inline void resync_get_all_pages(struct resync_pages *rp)
-{
-	int i;
-
-	for (i = 0; i < RESYNC_PAGES; i++)
-		get_page(rp->pages[i]);
-}
-
-static inline struct page *resync_fetch_page(struct resync_pages *rp,
-					     unsigned idx)
-{
-	if (WARN_ON_ONCE(idx >= RESYNC_PAGES))
-		return NULL;
-	return rp->pages[idx];
-}
 #endif /* _MD_MD_H */
diff --git a/drivers/md/raid1-10.c b/drivers/md/raid1-10.c
index 3adb5b9dc4b4..9f2670b45f31 100644
--- a/drivers/md/raid1-10.c
+++ b/drivers/md/raid1-10.c
@@ -1,3 +1,65 @@
+/* Maximum size of each resync request */
+#define RESYNC_BLOCK_SIZE (64*1024)
+#define RESYNC_PAGES ((RESYNC_BLOCK_SIZE + PAGE_SIZE-1) / PAGE_SIZE)
+
+/* for managing resync I/O pages */
+struct resync_pages {
+	void		*raid_bio;
+	struct page	*pages[RESYNC_PAGES];
+};
+
+static inline int resync_alloc_pages(struct resync_pages *rp,
+				     gfp_t gfp_flags)
+{
+	int i;
+
+	for (i = 0; i < RESYNC_PAGES; i++) {
+		rp->pages[i] = alloc_page(gfp_flags);
+		if (!rp->pages[i])
+			goto out_free;
+	}
+
+	return 0;
+
+out_free:
+	while (--i >= 0)
+		put_page(rp->pages[i]);
+	return -ENOMEM;
+}
+
+static inline void resync_free_pages(struct resync_pages *rp)
+{
+	int i;
+
+	for (i = 0; i < RESYNC_PAGES; i++)
+		put_page(rp->pages[i]);
+}
+
+static inline void resync_get_all_pages(struct resync_pages *rp)
+{
+	int i;
+
+	for (i = 0; i < RESYNC_PAGES; i++)
+		get_page(rp->pages[i]);
+}
+
+static inline struct page *resync_fetch_page(struct resync_pages *rp,
+					     unsigned idx)
+{
+	if (WARN_ON_ONCE(idx >= RESYNC_PAGES))
+		return NULL;
+	return rp->pages[idx];
+}
+
+/*
+ * 'strct resync_pages' stores actual pages used for doing the resync
+ *  IO, and it is per-bio, so make .bi_private points to it.
+ */
+static inline struct resync_pages *get_resync_pages(struct bio *bio)
+{
+	return bio->bi_private;
+}
+
 /* generally called after bio_reset() for reseting bvec */
 static void md_bio_reset_resync_pages(struct bio *bio, struct resync_pages *rp,
 			       int size)
diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
index fe86ab18961b..8387eb1540cd 100644
--- a/drivers/md/raid1.c
+++ b/drivers/md/raid1.c
@@ -84,15 +84,6 @@ static void lower_barrier(struct r1conf *conf, sector_t sector_nr);
 #include "raid1-10.c"
 
 /*
- * 'strct resync_pages' stores actual pages used for doing the resync
- *  IO, and it is per-bio, so make .bi_private points to it.
- */
-static inline struct resync_pages *get_resync_pages(struct bio *bio)
-{
-	return bio->bi_private;
-}
-
-/*
  * for resync bio, r1bio pointer can be retrieved from the per-bio
  * 'struct resync_pages'.
  */
diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
index 9952721e1cde..e2617d0f37dc 100644
--- a/drivers/md/raid10.c
+++ b/drivers/md/raid10.c
@@ -113,15 +113,6 @@ static void end_reshape(struct r10conf *conf);
 #include "raid1-10.c"
 
 /*
- * 'strct resync_pages' stores actual pages used for doing the resync
- *  IO, and it is per-bio, so make .bi_private points to it.
- */
-static inline struct resync_pages *get_resync_pages(struct bio *bio)
-{
-	return bio->bi_private;
-}
-
-/*
  * for resync bio, r10bio pointer can be retrieved from the per-bio
  * 'struct resync_pages'.
  */
-- 
2.9.4

Re: [PATCH v2 3/3] md: raid1-10: move raid1/raid10 common code into raid1-10.c

[Coly Li]

On 2017/7/14 下午4:14, Ming Lei wrote:
> No function change, just move 'struct resync_pages' and related
> helpers into raid1-10.c

Please give me a hint, I don't find where raid1-10.c is ....

Thanks.

Coly

> 
> Signed-off-by: Ming Lei <ming.lei@redhat.com>
> ---
>  drivers/md/md.h       | 53 -------------------------------------------
>  drivers/md/raid1-10.c | 62 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  drivers/md/raid1.c    |  9 --------
>  drivers/md/raid10.c   |  9 --------
>  4 files changed, 62 insertions(+), 71 deletions(-)
> 
> diff --git a/drivers/md/md.h b/drivers/md/md.h
> index 2c780aa8d07f..004a21c214e8 100644
> --- a/drivers/md/md.h
> +++ b/drivers/md/md.h
> @@ -729,57 +729,4 @@ static inline void mddev_check_write_zeroes(struct mddev *mddev, struct bio *bio
>  	    !bdev_get_queue(bio->bi_bdev)->limits.max_write_zeroes_sectors)
>  		mddev->queue->limits.max_write_zeroes_sectors = 0;
>  }
> -
> -/* Maximum size of each resync request */
> -#define RESYNC_BLOCK_SIZE (64*1024)
> -#define RESYNC_PAGES ((RESYNC_BLOCK_SIZE + PAGE_SIZE-1) / PAGE_SIZE)
> -
> -/* for managing resync I/O pages */
> -struct resync_pages {
> -	void		*raid_bio;
> -	struct page	*pages[RESYNC_PAGES];
> -};
> -
> -static inline int resync_alloc_pages(struct resync_pages *rp,
> -				     gfp_t gfp_flags)
> -{
> -	int i;
> -
> -	for (i = 0; i < RESYNC_PAGES; i++) {
> -		rp->pages[i] = alloc_page(gfp_flags);
> -		if (!rp->pages[i])
> -			goto out_free;
> -	}
> -
> -	return 0;
> -
> -out_free:
> -	while (--i >= 0)
> -		put_page(rp->pages[i]);
> -	return -ENOMEM;
> -}
> -
> -static inline void resync_free_pages(struct resync_pages *rp)
> -{
> -	int i;
> -
> -	for (i = 0; i < RESYNC_PAGES; i++)
> -		put_page(rp->pages[i]);
> -}
> -
> -static inline void resync_get_all_pages(struct resync_pages *rp)
> -{
> -	int i;
> -
> -	for (i = 0; i < RESYNC_PAGES; i++)
> -		get_page(rp->pages[i]);
> -}
> -
> -static inline struct page *resync_fetch_page(struct resync_pages *rp,
> -					     unsigned idx)
> -{
> -	if (WARN_ON_ONCE(idx >= RESYNC_PAGES))
> -		return NULL;
> -	return rp->pages[idx];
> -}
>  #endif /* _MD_MD_H */
> diff --git a/drivers/md/raid1-10.c b/drivers/md/raid1-10.c
> index 3adb5b9dc4b4..9f2670b45f31 100644
> --- a/drivers/md/raid1-10.c
> +++ b/drivers/md/raid1-10.c
> @@ -1,3 +1,65 @@
> +/* Maximum size of each resync request */
> +#define RESYNC_BLOCK_SIZE (64*1024)
> +#define RESYNC_PAGES ((RESYNC_BLOCK_SIZE + PAGE_SIZE-1) / PAGE_SIZE)
> +
> +/* for managing resync I/O pages */
> +struct resync_pages {
> +	void		*raid_bio;
> +	struct page	*pages[RESYNC_PAGES];
> +};
> +
> +static inline int resync_alloc_pages(struct resync_pages *rp,
> +				     gfp_t gfp_flags)
> +{
> +	int i;
> +
> +	for (i = 0; i < RESYNC_PAGES; i++) {
> +		rp->pages[i] = alloc_page(gfp_flags);
> +		if (!rp->pages[i])
> +			goto out_free;
> +	}
> +
> +	return 0;
> +
> +out_free:
> +	while (--i >= 0)
> +		put_page(rp->pages[i]);
> +	return -ENOMEM;
> +}
> +
> +static inline void resync_free_pages(struct resync_pages *rp)
> +{
> +	int i;
> +
> +	for (i = 0; i < RESYNC_PAGES; i++)
> +		put_page(rp->pages[i]);
> +}
> +
> +static inline void resync_get_all_pages(struct resync_pages *rp)
> +{
> +	int i;
> +
> +	for (i = 0; i < RESYNC_PAGES; i++)
> +		get_page(rp->pages[i]);
> +}
> +
> +static inline struct page *resync_fetch_page(struct resync_pages *rp,
> +					     unsigned idx)
> +{
> +	if (WARN_ON_ONCE(idx >= RESYNC_PAGES))
> +		return NULL;
> +	return rp->pages[idx];
> +}
> +
> +/*
> + * 'strct resync_pages' stores actual pages used for doing the resync
> + *  IO, and it is per-bio, so make .bi_private points to it.
> + */
> +static inline struct resync_pages *get_resync_pages(struct bio *bio)
> +{
> +	return bio->bi_private;
> +}
> +
>  /* generally called after bio_reset() for reseting bvec */
>  static void md_bio_reset_resync_pages(struct bio *bio, struct resync_pages *rp,
>  			       int size)
> diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
> index fe86ab18961b..8387eb1540cd 100644
> --- a/drivers/md/raid1.c
> +++ b/drivers/md/raid1.c
> @@ -84,15 +84,6 @@ static void lower_barrier(struct r1conf *conf, sector_t sector_nr);
>  #include "raid1-10.c"
>  
>  /*
> - * 'strct resync_pages' stores actual pages used for doing the resync
> - *  IO, and it is per-bio, so make .bi_private points to it.
> - */
> -static inline struct resync_pages *get_resync_pages(struct bio *bio)
> -{
> -	return bio->bi_private;
> -}
> -
> -/*
>   * for resync bio, r1bio pointer can be retrieved from the per-bio
>   * 'struct resync_pages'.
>   */
> diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
> index 9952721e1cde..e2617d0f37dc 100644
> --- a/drivers/md/raid10.c
> +++ b/drivers/md/raid10.c
> @@ -113,15 +113,6 @@ static void end_reshape(struct r10conf *conf);
>  #include "raid1-10.c"
>  
>  /*
> - * 'strct resync_pages' stores actual pages used for doing the resync
> - *  IO, and it is per-bio, so make .bi_private points to it.
> - */
> -static inline struct resync_pages *get_resync_pages(struct bio *bio)
> -{
> -	return bio->bi_private;
> -}
> -
> -/*
>   * for resync bio, r10bio pointer can be retrieved from the per-bio
>   * 'struct resync_pages'.
>   */
> 

Re: [PATCH v2 3/3] md: raid1-10: move raid1/raid10 common code into raid1-10.c

[NeilBrown]

[-- Attachment #1: Type: text/plain, Size: 5466 bytes --]

On Fri, Jul 14 2017, Ming Lei wrote:

> No function change, just move 'struct resync_pages' and related
> helpers into raid1-10.c
>
> Signed-off-by: Ming Lei <ming.lei@redhat.com>

Thanks for doing this!
I'm quite happy with this approach - and with all patches in this
series.

Thanks,
NeilBrown


> ---
>  drivers/md/md.h       | 53 -------------------------------------------
>  drivers/md/raid1-10.c | 62 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  drivers/md/raid1.c    |  9 --------
>  drivers/md/raid10.c   |  9 --------
>  4 files changed, 62 insertions(+), 71 deletions(-)
>
> diff --git a/drivers/md/md.h b/drivers/md/md.h
> index 2c780aa8d07f..004a21c214e8 100644
> --- a/drivers/md/md.h
> +++ b/drivers/md/md.h
> @@ -729,57 +729,4 @@ static inline void mddev_check_write_zeroes(struct mddev *mddev, struct bio *bio
>  	    !bdev_get_queue(bio->bi_bdev)->limits.max_write_zeroes_sectors)
>  		mddev->queue->limits.max_write_zeroes_sectors = 0;
>  }
> -
> -/* Maximum size of each resync request */
> -#define RESYNC_BLOCK_SIZE (64*1024)
> -#define RESYNC_PAGES ((RESYNC_BLOCK_SIZE + PAGE_SIZE-1) / PAGE_SIZE)
> -
> -/* for managing resync I/O pages */
> -struct resync_pages {
> -	void		*raid_bio;
> -	struct page	*pages[RESYNC_PAGES];
> -};
> -
> -static inline int resync_alloc_pages(struct resync_pages *rp,
> -				     gfp_t gfp_flags)
> -{
> -	int i;
> -
> -	for (i = 0; i < RESYNC_PAGES; i++) {
> -		rp->pages[i] = alloc_page(gfp_flags);
> -		if (!rp->pages[i])
> -			goto out_free;
> -	}
> -
> -	return 0;
> -
> -out_free:
> -	while (--i >= 0)
> -		put_page(rp->pages[i]);
> -	return -ENOMEM;
> -}
> -
> -static inline void resync_free_pages(struct resync_pages *rp)
> -{
> -	int i;
> -
> -	for (i = 0; i < RESYNC_PAGES; i++)
> -		put_page(rp->pages[i]);
> -}
> -
> -static inline void resync_get_all_pages(struct resync_pages *rp)
> -{
> -	int i;
> -
> -	for (i = 0; i < RESYNC_PAGES; i++)
> -		get_page(rp->pages[i]);
> -}
> -
> -static inline struct page *resync_fetch_page(struct resync_pages *rp,
> -					     unsigned idx)
> -{
> -	if (WARN_ON_ONCE(idx >= RESYNC_PAGES))
> -		return NULL;
> -	return rp->pages[idx];
> -}
>  #endif /* _MD_MD_H */
> diff --git a/drivers/md/raid1-10.c b/drivers/md/raid1-10.c
> index 3adb5b9dc4b4..9f2670b45f31 100644
> --- a/drivers/md/raid1-10.c
> +++ b/drivers/md/raid1-10.c
> @@ -1,3 +1,65 @@
> +/* Maximum size of each resync request */
> +#define RESYNC_BLOCK_SIZE (64*1024)
> +#define RESYNC_PAGES ((RESYNC_BLOCK_SIZE + PAGE_SIZE-1) / PAGE_SIZE)
> +
> +/* for managing resync I/O pages */
> +struct resync_pages {
> +	void		*raid_bio;
> +	struct page	*pages[RESYNC_PAGES];
> +};
> +
> +static inline int resync_alloc_pages(struct resync_pages *rp,
> +				     gfp_t gfp_flags)
> +{
> +	int i;
> +
> +	for (i = 0; i < RESYNC_PAGES; i++) {
> +		rp->pages[i] = alloc_page(gfp_flags);
> +		if (!rp->pages[i])
> +			goto out_free;
> +	}
> +
> +	return 0;
> +
> +out_free:
> +	while (--i >= 0)
> +		put_page(rp->pages[i]);
> +	return -ENOMEM;
> +}
> +
> +static inline void resync_free_pages(struct resync_pages *rp)
> +{
> +	int i;
> +
> +	for (i = 0; i < RESYNC_PAGES; i++)
> +		put_page(rp->pages[i]);
> +}
> +
> +static inline void resync_get_all_pages(struct resync_pages *rp)
> +{
> +	int i;
> +
> +	for (i = 0; i < RESYNC_PAGES; i++)
> +		get_page(rp->pages[i]);
> +}
> +
> +static inline struct page *resync_fetch_page(struct resync_pages *rp,
> +					     unsigned idx)
> +{
> +	if (WARN_ON_ONCE(idx >= RESYNC_PAGES))
> +		return NULL;
> +	return rp->pages[idx];
> +}
> +
> +/*
> + * 'strct resync_pages' stores actual pages used for doing the resync
> + *  IO, and it is per-bio, so make .bi_private points to it.
> + */
> +static inline struct resync_pages *get_resync_pages(struct bio *bio)
> +{
> +	return bio->bi_private;
> +}
> +
>  /* generally called after bio_reset() for reseting bvec */
>  static void md_bio_reset_resync_pages(struct bio *bio, struct resync_pages *rp,
>  			       int size)
> diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
> index fe86ab18961b..8387eb1540cd 100644
> --- a/drivers/md/raid1.c
> +++ b/drivers/md/raid1.c
> @@ -84,15 +84,6 @@ static void lower_barrier(struct r1conf *conf, sector_t sector_nr);
>  #include "raid1-10.c"
>  
>  /*
> - * 'strct resync_pages' stores actual pages used for doing the resync
> - *  IO, and it is per-bio, so make .bi_private points to it.
> - */
> -static inline struct resync_pages *get_resync_pages(struct bio *bio)
> -{
> -	return bio->bi_private;
> -}
> -
> -/*
>   * for resync bio, r1bio pointer can be retrieved from the per-bio
>   * 'struct resync_pages'.
>   */
> diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
> index 9952721e1cde..e2617d0f37dc 100644
> --- a/drivers/md/raid10.c
> +++ b/drivers/md/raid10.c
> @@ -113,15 +113,6 @@ static void end_reshape(struct r10conf *conf);
>  #include "raid1-10.c"
>  
>  /*
> - * 'strct resync_pages' stores actual pages used for doing the resync
> - *  IO, and it is per-bio, so make .bi_private points to it.
> - */
> -static inline struct resync_pages *get_resync_pages(struct bio *bio)
> -{
> -	return bio->bi_private;
> -}
> -
> -/*
>   * for resync bio, r10bio pointer can be retrieved from the per-bio
>   * 'struct resync_pages'.
>   */
> -- 
> 2.9.4

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

Re: [PATCH v2 3/3] md: raid1-10: move raid1/raid10 common code into raid1-10.c

[Ming Lei]

On Sun, Jul 16, 2017 at 1:14 PM, Coly Li <colyli@suse.de> wrote:
> On 2017/7/14 下午4:14, Ming Lei wrote:
>> No function change, just move 'struct resync_pages' and related
>> helpers into raid1-10.c
>
> Please give me a hint, I don't find where raid1-10.c is ....

It is introduced in this patch, :-)


-- 
Ming Lei

Re: [PATCH v2 0/3] md: three misc changes

[Shaohua Li]

On Fri, Jul 14, 2017 at 04:14:41PM +0800, Ming Lei wrote:
> This 1st patch fixes one issue introduced in the following two
> commits:
> 	Fixes: f0250618361d(md: raid10: don't use bio's vec table to manage resync pages)
> 	Fixes: 98d30c5812c3(md: raid1: don't use bio's vec table to manage resync pages)
> 
> The 2nd one initializes bvec table of bio via bio_add_page() after bio_reset().
> 
> The 3rd one moves the common definitation and helpers into raid1-10.c. 

applied, thanks!
 
> V2:
> 	- fix 'page_idx' increasement in patch 1
> 	- move raid1/raid10 common code to raid1-raid10.c, as suggested by Neil
> 
> Ming Lei (3):
>   md: remove 'idx' from 'struct resync_pages'
>   md: raid1/raid10: initialize bvec table via bio_add_page()
>   md: raid1-10: move raid1/raid10 common code into raid1-10.c
> 
>  drivers/md/md.h       | 54 ----------------------------------
>  drivers/md/raid1-10.c | 81 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  drivers/md/raid1.c    | 31 ++++----------------
>  drivers/md/raid10.c   | 19 ++++--------
>  4 files changed, 93 insertions(+), 92 deletions(-)
>  create mode 100644 drivers/md/raid1-10.c
> 
> -- 
> 2.9.4
>