deprecate md bitmap file support

deprecate md bitmap file support

[Christoph Hellwig]

Hi Song,

the md bitmap file support is very problematic in how it bypasses the
file system for file access.  I looked into fixing it in preparation
for making buffer_head optionals but had to give up because it is so
convoluted.  This series includes the cleanups I've started which seem
useful even for the internal bitmap support, then makes the bitmap file
support conditional and adds a deprecation warning.

Diffstat:
 Kconfig     |   10 +
 md-bitmap.c |  339 ++++++++++++++++++++++++++++++------------------------------
 md-bitmap.h |    1 
 md.c        |    9 +
 4 files changed, 191 insertions(+), 168 deletions(-)

[PATCH 01/11] md-bitmap: set BITMAP_WRITE_ERROR in write_sb_page

[Christoph Hellwig]

Set BITMAP_WRITE_ERROR directly in write_sb_page instead of propagating
the error to the caller and setting it there.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/md/md-bitmap.c | 21 ++++++++-------------
 1 file changed, 8 insertions(+), 13 deletions(-)

diff --git a/drivers/md/md-bitmap.c b/drivers/md/md-bitmap.c
index 1ff712889a3b36..d8469720fac23f 100644
--- a/drivers/md/md-bitmap.c
+++ b/drivers/md/md-bitmap.c
@@ -279,22 +279,20 @@ static int __write_sb_page(struct md_rdev *rdev, struct bitmap *bitmap,
 	return 0;
 }
 
-static int write_sb_page(struct bitmap *bitmap, struct page *page, int wait)
+static void write_sb_page(struct bitmap *bitmap, struct page *page, int wait)
 {
-	struct md_rdev *rdev;
 	struct mddev *mddev = bitmap->mddev;
-	int ret;
 
 	do {
-		rdev = NULL;
+		struct md_rdev *rdev = NULL;
+
 		while ((rdev = next_active_rdev(rdev, mddev)) != NULL) {
-			ret = __write_sb_page(rdev, bitmap, page);
-			if (ret)
-				return ret;
+			if (__write_sb_page(rdev, bitmap, page) < 0) {
+				set_bit(BITMAP_WRITE_ERROR, &bitmap->flags);
+				return;
+			}
 		}
 	} while (wait && md_super_wait(mddev) < 0);
-
-	return 0;
 }
 
 static void md_bitmap_file_kick(struct bitmap *bitmap);
@@ -306,10 +304,7 @@ static void write_page(struct bitmap *bitmap, struct page *page, int wait)
 	struct buffer_head *bh;
 
 	if (bitmap->storage.file == NULL) {
-		switch (write_sb_page(bitmap, page, wait)) {
-		case -EINVAL:
-			set_bit(BITMAP_WRITE_ERROR, &bitmap->flags);
-		}
+		write_sb_page(bitmap, page, wait);
 	} else {
 
 		bh = page_buffers(page);
-- 
2.39.2

Re: [PATCH 01/11] md-bitmap: set BITMAP_WRITE_ERROR in write_sb_page

[Hannes Reinecke]

On 6/15/23 08:48, Christoph Hellwig wrote:
> Set BITMAP_WRITE_ERROR directly in write_sb_page instead of propagating
> the error to the caller and setting it there.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>   drivers/md/md-bitmap.c | 21 ++++++++-------------
>   1 file changed, 8 insertions(+), 13 deletions(-)
> 
Reviewed-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                Kernel Storage Architect
hare@suse.de                              +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Ivo Totev, Andrew
Myers, Andrew McDonald, Martje Boudien Moerman

Re: [PATCH 01/11] md-bitmap: set BITMAP_WRITE_ERROR in write_sb_page

[Johannes Thumshirn]

Looks good,
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>

Re: [PATCH 01/11] md-bitmap: set BITMAP_WRITE_ERROR in write_sb_page

[Himanshu Madhani]

> On Jun 14, 2023, at 11:48 PM, Christoph Hellwig <hch@lst.de> wrote:
> 
> Set BITMAP_WRITE_ERROR directly in write_sb_page instead of propagating
> the error to the caller and setting it there.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
> drivers/md/md-bitmap.c | 21 ++++++++-------------
> 1 file changed, 8 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/md/md-bitmap.c b/drivers/md/md-bitmap.c
> index 1ff712889a3b36..d8469720fac23f 100644
> --- a/drivers/md/md-bitmap.c
> +++ b/drivers/md/md-bitmap.c
> @@ -279,22 +279,20 @@ static int __write_sb_page(struct md_rdev *rdev, struct bitmap *bitmap,
> return 0;
> }
> 
> -static int write_sb_page(struct bitmap *bitmap, struct page *page, int wait)
> +static void write_sb_page(struct bitmap *bitmap, struct page *page, int wait)
> {
> - struct md_rdev *rdev;
> struct mddev *mddev = bitmap->mddev;
> - int ret;
> 
> do {
> - rdev = NULL;
> + struct md_rdev *rdev = NULL;
> +
> while ((rdev = next_active_rdev(rdev, mddev)) != NULL) {
> - ret = __write_sb_page(rdev, bitmap, page);
> - if (ret)
> - return ret;
> + if (__write_sb_page(rdev, bitmap, page) < 0) {
> + set_bit(BITMAP_WRITE_ERROR, &bitmap->flags);
> + return;
> + }
> }
> } while (wait && md_super_wait(mddev) < 0);
> -
> - return 0;
> }
> 
> static void md_bitmap_file_kick(struct bitmap *bitmap);
> @@ -306,10 +304,7 @@ static void write_page(struct bitmap *bitmap, struct page *page, int wait)
> struct buffer_head *bh;
> 
> if (bitmap->storage.file == NULL) {
> - switch (write_sb_page(bitmap, page, wait)) {
> - case -EINVAL:
> - set_bit(BITMAP_WRITE_ERROR, &bitmap->flags);
> - }
> + write_sb_page(bitmap, page, wait);
> } else {
> 
> bh = page_buffers(page);
> -- 
> 2.39.2
> 

Reviewed-by: Himanshu Madhani <himanshu.madhani@oracle.com>

-- 
Himanshu Madhani Oracle Linux Engineering

[PATCH 02/11] md-bitmap: initialize variables at declaration time in md_bitmap_file_unmap

[Christoph Hellwig]

Just a small tidyup to prepare for bigger changes.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/md/md-bitmap.c | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/drivers/md/md-bitmap.c b/drivers/md/md-bitmap.c
index d8469720fac23f..0b2d8933cbc75e 100644
--- a/drivers/md/md-bitmap.c
+++ b/drivers/md/md-bitmap.c
@@ -842,14 +842,10 @@ static int md_bitmap_storage_alloc(struct bitmap_storage *store,
 
 static void md_bitmap_file_unmap(struct bitmap_storage *store)
 {
-	struct page **map, *sb_page;
-	int pages;
-	struct file *file;
-
-	file = store->file;
-	map = store->filemap;
-	pages = store->file_pages;
-	sb_page = store->sb_page;
+	struct file *file = store->file;
+	struct page *sb_page = store->sb_page;
+	struct page **map = store->filemap;
+	int pages = store->file_pages;
 
 	while (pages--)
 		if (map[pages] != sb_page) /* 0 is sb_page, release it below */
-- 
2.39.2

Re: [PATCH 02/11] md-bitmap: initialize variables at declaration time in md_bitmap_file_unmap

[Hannes Reinecke]

On 6/15/23 08:48, Christoph Hellwig wrote:
> Just a small tidyup to prepare for bigger changes.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>   drivers/md/md-bitmap.c | 12 ++++--------
>   1 file changed, 4 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/md/md-bitmap.c b/drivers/md/md-bitmap.c
> index d8469720fac23f..0b2d8933cbc75e 100644
> --- a/drivers/md/md-bitmap.c
> +++ b/drivers/md/md-bitmap.c
> @@ -842,14 +842,10 @@ static int md_bitmap_storage_alloc(struct bitmap_storage *store,
>   
>   static void md_bitmap_file_unmap(struct bitmap_storage *store)
>   {
> -	struct page **map, *sb_page;
> -	int pages;
> -	struct file *file;
> -
> -	file = store->file;
> -	map = store->filemap;
> -	pages = store->file_pages;
> -	sb_page = store->sb_page;
> +	struct file *file = store->file;
> +	struct page *sb_page = store->sb_page;
> +	struct page **map = store->filemap;
> +	int pages = store->file_pages;
>   
>   	while (pages--)
>   		if (map[pages] != sb_page) /* 0 is sb_page, release it below */
Reviewed-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                Kernel Storage Architect
hare@suse.de                              +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Ivo Totev, Andrew
Myers, Andrew McDonald, Martje Boudien Moerman

Re: [PATCH 02/11] md-bitmap: initialize variables at declaration time in md_bitmap_file_unmap

[Johannes Thumshirn]

Looks good,
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>

Re: [PATCH 02/11] md-bitmap: initialize variables at declaration time in md_bitmap_file_unmap

[Himanshu Madhani]

> On Jun 14, 2023, at 11:48 PM, Christoph Hellwig <hch@lst.de> wrote:
> 
> Just a small tidyup to prepare for bigger changes.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
> drivers/md/md-bitmap.c | 12 ++++--------
> 1 file changed, 4 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/md/md-bitmap.c b/drivers/md/md-bitmap.c
> index d8469720fac23f..0b2d8933cbc75e 100644
> --- a/drivers/md/md-bitmap.c
> +++ b/drivers/md/md-bitmap.c
> @@ -842,14 +842,10 @@ static int md_bitmap_storage_alloc(struct bitmap_storage *store,
> 
> static void md_bitmap_file_unmap(struct bitmap_storage *store)
> {
> - struct page **map, *sb_page;
> - int pages;
> - struct file *file;
> -
> - file = store->file;
> - map = store->filemap;
> - pages = store->file_pages;
> - sb_page = store->sb_page;
> + struct file *file = store->file;
> + struct page *sb_page = store->sb_page;
> + struct page **map = store->filemap;
> + int pages = store->file_pages;
> 
> while (pages--)
> if (map[pages] != sb_page) /* 0 is sb_page, release it below */
> -- 
> 2.39.2
> 

Reviewed-by: Himanshu Madhani <himanshu.madhani@oracle.com>

-- 
Himanshu Madhani Oracle Linux Engineering

[PATCH 03/11] md-bitmap: use %pD to print the file name in md_bitmap_file_kick

[Christoph Hellwig]

Don't bother allocating an extra buffer in the I/O failure handler and
instead use the printk built-in format to print the last 4 path name
components.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/md/md-bitmap.c | 12 ++----------
 1 file changed, 2 insertions(+), 10 deletions(-)

diff --git a/drivers/md/md-bitmap.c b/drivers/md/md-bitmap.c
index 0b2d8933cbc75e..e4b466522d4e74 100644
--- a/drivers/md/md-bitmap.c
+++ b/drivers/md/md-bitmap.c
@@ -870,21 +870,13 @@ static void md_bitmap_file_unmap(struct bitmap_storage *store)
  */
 static void md_bitmap_file_kick(struct bitmap *bitmap)
 {
-	char *path, *ptr = NULL;
-
 	if (!test_and_set_bit(BITMAP_STALE, &bitmap->flags)) {
 		md_bitmap_update_sb(bitmap);
 
 		if (bitmap->storage.file) {
-			path = kmalloc(PAGE_SIZE, GFP_KERNEL);
-			if (path)
-				ptr = file_path(bitmap->storage.file,
-					     path, PAGE_SIZE);
-
-			pr_warn("%s: kicking failed bitmap file %s from array!\n",
-				bmname(bitmap), IS_ERR(ptr) ? "" : ptr);
+			pr_warn("%s: kicking failed bitmap file %pD4 from array!\n",
+				bmname(bitmap), bitmap->storage.file);
 
-			kfree(path);
 		} else
 			pr_warn("%s: disabling internal bitmap due to errors\n",
 				bmname(bitmap));
-- 
2.39.2

Re: [PATCH 03/11] md-bitmap: use %pD to print the file name in md_bitmap_file_kick

[Hannes Reinecke]

On 6/15/23 08:48, Christoph Hellwig wrote:
> Don't bother allocating an extra buffer in the I/O failure handler and
> instead use the printk built-in format to print the last 4 path name
> components.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>   drivers/md/md-bitmap.c | 12 ++----------
>   1 file changed, 2 insertions(+), 10 deletions(-)
> 
Reviewed-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                Kernel Storage Architect
hare@suse.de                              +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Ivo Totev, Andrew
Myers, Andrew McDonald, Martje Boudien Moerman

Re: [PATCH 03/11] md-bitmap: use %pD to print the file name in md_bitmap_file_kick

[Johannes Thumshirn]

Looks good,
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>

Re: [PATCH 03/11] md-bitmap: use %pD to print the file name in md_bitmap_file_kick

[Himanshu Madhani]

> On Jun 14, 2023, at 11:48 PM, Christoph Hellwig <hch@lst.de> wrote:
> 
> Don't bother allocating an extra buffer in the I/O failure handler and
> instead use the printk built-in format to print the last 4 path name
> components.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
> drivers/md/md-bitmap.c | 12 ++----------
> 1 file changed, 2 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/md/md-bitmap.c b/drivers/md/md-bitmap.c
> index 0b2d8933cbc75e..e4b466522d4e74 100644
> --- a/drivers/md/md-bitmap.c
> +++ b/drivers/md/md-bitmap.c
> @@ -870,21 +870,13 @@ static void md_bitmap_file_unmap(struct bitmap_storage *store)
>  */
> static void md_bitmap_file_kick(struct bitmap *bitmap)
> {
> - char *path, *ptr = NULL;
> -
> if (!test_and_set_bit(BITMAP_STALE, &bitmap->flags)) {
> md_bitmap_update_sb(bitmap);
> 
> if (bitmap->storage.file) {
> - path = kmalloc(PAGE_SIZE, GFP_KERNEL);
> - if (path)
> - ptr = file_path(bitmap->storage.file,
> -     path, PAGE_SIZE);
> -
> - pr_warn("%s: kicking failed bitmap file %s from array!\n",
> - bmname(bitmap), IS_ERR(ptr) ? "" : ptr);
> + pr_warn("%s: kicking failed bitmap file %pD4 from array!\n",
> + bmname(bitmap), bitmap->storage.file);
> 
> - kfree(path);
> } else
> pr_warn("%s: disabling internal bitmap due to errors\n",
> bmname(bitmap));
> -- 
> 2.39.2
> 

Reviewed-by: Himanshu Madhani <himanshu.madhani@oracle.com>

-- 
Himanshu Madhani Oracle Linux Engineering

[PATCH 04/11] md-bitmap: split file writes into a separate helper

[Christoph Hellwig]

Split the file write code out of write_page into a separate helper.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/md/md-bitmap.c | 48 +++++++++++++++++++++---------------------
 1 file changed, 24 insertions(+), 24 deletions(-)

diff --git a/drivers/md/md-bitmap.c b/drivers/md/md-bitmap.c
index e4b466522d4e74..46fbcfc9d1fcac 100644
--- a/drivers/md/md-bitmap.c
+++ b/drivers/md/md-bitmap.c
@@ -296,33 +296,22 @@ static void write_sb_page(struct bitmap *bitmap, struct page *page, int wait)
 }
 
 static void md_bitmap_file_kick(struct bitmap *bitmap);
-/*
- * write out a page to a file
- */
-static void write_page(struct bitmap *bitmap, struct page *page, int wait)
-{
-	struct buffer_head *bh;
 
-	if (bitmap->storage.file == NULL) {
-		write_sb_page(bitmap, page, wait);
-	} else {
-
-		bh = page_buffers(page);
-
-		while (bh && bh->b_blocknr) {
-			atomic_inc(&bitmap->pending_writes);
-			set_buffer_locked(bh);
-			set_buffer_mapped(bh);
-			submit_bh(REQ_OP_WRITE | REQ_SYNC, bh);
-			bh = bh->b_this_page;
-		}
+static void write_file_page(struct bitmap *bitmap, struct page *page, int wait)
+{
+	struct buffer_head *bh = page_buffers(page);
 
-		if (wait)
-			wait_event(bitmap->write_wait,
-				   atomic_read(&bitmap->pending_writes)==0);
+	while (bh && bh->b_blocknr) {
+		atomic_inc(&bitmap->pending_writes);
+		set_buffer_locked(bh);
+		set_buffer_mapped(bh);
+		submit_bh(REQ_OP_WRITE | REQ_SYNC, bh);
+		bh = bh->b_this_page;
 	}
-	if (test_bit(BITMAP_WRITE_ERROR, &bitmap->flags))
-		md_bitmap_file_kick(bitmap);
+
+	if (wait)
+		wait_event(bitmap->write_wait,
+			   atomic_read(&bitmap->pending_writes) == 0);
 }
 
 static void end_bitmap_write(struct buffer_head *bh, int uptodate)
@@ -429,6 +418,17 @@ static int read_page(struct file *file, unsigned long index,
  * bitmap file superblock operations
  */
 
+/*
+ * write out a page to a file
+ */
+static void write_page(struct bitmap *bitmap, struct page *page, int wait)
+{
+	if (bitmap->storage.file)
+		write_file_page(bitmap, page, wait);
+	else
+		write_sb_page(bitmap, page, wait);
+}
+
 /*
  * md_bitmap_wait_writes() should be called before writing any bitmap
  * blocks, to ensure previous writes, particularly from
-- 
2.39.2

Re: [PATCH 04/11] md-bitmap: split file writes into a separate helper

[Hannes Reinecke]

On 6/15/23 08:48, Christoph Hellwig wrote:
> Split the file write code out of write_page into a separate helper.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>   drivers/md/md-bitmap.c | 48 +++++++++++++++++++++---------------------
>   1 file changed, 24 insertions(+), 24 deletions(-)
> 
Reviewed-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                Kernel Storage Architect
hare@suse.de                              +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Ivo Totev, Andrew
Myers, Andrew McDonald, Martje Boudien Moerman

Re: [PATCH 04/11] md-bitmap: split file writes into a separate helper

[Johannes Thumshirn]

Looks good,
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>

Re: [PATCH 04/11] md-bitmap: split file writes into a separate helper

[Himanshu Madhani]

> On Jun 14, 2023, at 11:48 PM, Christoph Hellwig <hch@lst.de> wrote:
> 
> Split the file write code out of write_page into a separate helper.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
> drivers/md/md-bitmap.c | 48 +++++++++++++++++++++---------------------
> 1 file changed, 24 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/md/md-bitmap.c b/drivers/md/md-bitmap.c
> index e4b466522d4e74..46fbcfc9d1fcac 100644
> --- a/drivers/md/md-bitmap.c
> +++ b/drivers/md/md-bitmap.c
> @@ -296,33 +296,22 @@ static void write_sb_page(struct bitmap *bitmap, struct page *page, int wait)
> }
> 
> static void md_bitmap_file_kick(struct bitmap *bitmap);
> -/*
> - * write out a page to a file
> - */
> -static void write_page(struct bitmap *bitmap, struct page *page, int wait)
> -{
> - struct buffer_head *bh;
> 
> - if (bitmap->storage.file == NULL) {
> - write_sb_page(bitmap, page, wait);
> - } else {
> -
> - bh = page_buffers(page);
> -
> - while (bh && bh->b_blocknr) {
> - atomic_inc(&bitmap->pending_writes);
> - set_buffer_locked(bh);
> - set_buffer_mapped(bh);
> - submit_bh(REQ_OP_WRITE | REQ_SYNC, bh);
> - bh = bh->b_this_page;
> - }
> +static void write_file_page(struct bitmap *bitmap, struct page *page, int wait)
> +{
> + struct buffer_head *bh = page_buffers(page);
> 
> - if (wait)
> - wait_event(bitmap->write_wait,
> -   atomic_read(&bitmap->pending_writes)==0);
> + while (bh && bh->b_blocknr) {
> + atomic_inc(&bitmap->pending_writes);
> + set_buffer_locked(bh);
> + set_buffer_mapped(bh);
> + submit_bh(REQ_OP_WRITE | REQ_SYNC, bh);
> + bh = bh->b_this_page;
> }
> - if (test_bit(BITMAP_WRITE_ERROR, &bitmap->flags))
> - md_bitmap_file_kick(bitmap);
> +
> + if (wait)
> + wait_event(bitmap->write_wait,
> +   atomic_read(&bitmap->pending_writes) == 0);
> }
> 
> static void end_bitmap_write(struct buffer_head *bh, int uptodate)
> @@ -429,6 +418,17 @@ static int read_page(struct file *file, unsigned long index,
>  * bitmap file superblock operations
>  */
> 
> +/*
> + * write out a page to a file
> + */
> +static void write_page(struct bitmap *bitmap, struct page *page, int wait)
> +{
> + if (bitmap->storage.file)
> + write_file_page(bitmap, page, wait);
> + else
> + write_sb_page(bitmap, page, wait);
> +}
> +
> /*
>  * md_bitmap_wait_writes() should be called before writing any bitmap
>  * blocks, to ensure previous writes, particularly from
> -- 
> 2.39.2
> 

Reviewed-by: Himanshu Madhani <himanshu.madhani@oracle.com>

-- 
Himanshu Madhani Oracle Linux Engineering

[PATCH 05/11] md-bitmap: rename read_page to read_file_page

[Christoph Hellwig]

Make the difference to read_sb_page clear.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/md/md-bitmap.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/md/md-bitmap.c b/drivers/md/md-bitmap.c
index 46fbcfc9d1fcac..fa0f6ca7b61b0b 100644
--- a/drivers/md/md-bitmap.c
+++ b/drivers/md/md-bitmap.c
@@ -348,10 +348,8 @@ static void free_buffers(struct page *page)
  * This usage is similar to how swap files are handled, and allows us
  * to write to a file with no concerns of memory allocation failing.
  */
-static int read_page(struct file *file, unsigned long index,
-		     struct bitmap *bitmap,
-		     unsigned long count,
-		     struct page *page)
+static int read_file_page(struct file *file, unsigned long index,
+		struct bitmap *bitmap, unsigned long count, struct page *page)
 {
 	int ret = 0;
 	struct inode *inode = file_inode(file);
@@ -632,7 +630,7 @@ static int md_bitmap_read_sb(struct bitmap *bitmap)
 		loff_t isize = i_size_read(bitmap->storage.file->f_mapping->host);
 		int bytes = isize > PAGE_SIZE ? PAGE_SIZE : isize;
 
-		err = read_page(bitmap->storage.file, 0,
+		err = read_file_page(bitmap->storage.file, 0,
 				bitmap, bytes, sb_page);
 	} else {
 		err = read_sb_page(bitmap->mddev,
@@ -1141,7 +1139,7 @@ static int md_bitmap_init_from_disk(struct bitmap *bitmap, sector_t start)
 				count = PAGE_SIZE;
 			page = store->filemap[index];
 			if (file)
-				ret = read_page(file, index, bitmap,
+				ret = read_file_page(file, index, bitmap,
 						count, page);
 			else
 				ret = read_sb_page(
-- 
2.39.2

Re: [PATCH 05/11] md-bitmap: rename read_page to read_file_page

[Hannes Reinecke]

On 6/15/23 08:48, Christoph Hellwig wrote:
> Make the difference to read_sb_page clear.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>   drivers/md/md-bitmap.c | 10 ++++------
>   1 file changed, 4 insertions(+), 6 deletions(-)
> 
Reviewed-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                Kernel Storage Architect
hare@suse.de                              +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Ivo Totev, Andrew
Myers, Andrew McDonald, Martje Boudien Moerman

Re: [PATCH 05/11] md-bitmap: rename read_page to read_file_page

[Johannes Thumshirn]

Looks good,
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>

Re: [PATCH 05/11] md-bitmap: rename read_page to read_file_page

[Himanshu Madhani]

> On Jun 14, 2023, at 11:48 PM, Christoph Hellwig <hch@lst.de> wrote:
> 
> Make the difference to read_sb_page clear.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
> drivers/md/md-bitmap.c | 10 ++++------
> 1 file changed, 4 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/md/md-bitmap.c b/drivers/md/md-bitmap.c
> index 46fbcfc9d1fcac..fa0f6ca7b61b0b 100644
> --- a/drivers/md/md-bitmap.c
> +++ b/drivers/md/md-bitmap.c
> @@ -348,10 +348,8 @@ static void free_buffers(struct page *page)
>  * This usage is similar to how swap files are handled, and allows us
>  * to write to a file with no concerns of memory allocation failing.
>  */
> -static int read_page(struct file *file, unsigned long index,
> -     struct bitmap *bitmap,
> -     unsigned long count,
> -     struct page *page)
> +static int read_file_page(struct file *file, unsigned long index,
> + struct bitmap *bitmap, unsigned long count, struct page *page)
> {
> int ret = 0;
> struct inode *inode = file_inode(file);
> @@ -632,7 +630,7 @@ static int md_bitmap_read_sb(struct bitmap *bitmap)
> loff_t isize = i_size_read(bitmap->storage.file->f_mapping->host);
> int bytes = isize > PAGE_SIZE ? PAGE_SIZE : isize;
> 
> - err = read_page(bitmap->storage.file, 0,
> + err = read_file_page(bitmap->storage.file, 0,
> bitmap, bytes, sb_page);
> } else {
> err = read_sb_page(bitmap->mddev,
> @@ -1141,7 +1139,7 @@ static int md_bitmap_init_from_disk(struct bitmap *bitmap, sector_t start)
> count = PAGE_SIZE;
> page = store->filemap[index];
> if (file)
> - ret = read_page(file, index, bitmap,
> + ret = read_file_page(file, index, bitmap,
> count, page);
> else
> ret = read_sb_page(
> -- 
> 2.39.2
> 

Reviewed-by: Himanshu Madhani <himanshu.madhani@oracle.com>

-- 
Himanshu Madhani Oracle Linux Engineering

[PATCH 06/11] md-bitmap: refactor md_bitmap_init_from_disk

[Christoph Hellwig]

Split the confusing loop in md_bitmap_init_from_disk that iterates over
all chunks but also needs to read and map the pages into three separate
loops: one that iterates over the pages to read them, a second optional
one to iterate over the pages to mark them invalid if the bitmaps are
out of date, and a final one that actually iterates over the chunks.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/md/md-bitmap.c | 142 ++++++++++++++++++++---------------------
 1 file changed, 70 insertions(+), 72 deletions(-)

diff --git a/drivers/md/md-bitmap.c b/drivers/md/md-bitmap.c
index fa0f6ca7b61b0b..1f71683b417981 100644
--- a/drivers/md/md-bitmap.c
+++ b/drivers/md/md-bitmap.c
@@ -1065,33 +1065,31 @@ void md_bitmap_unplug_async(struct bitmap *bitmap)
 EXPORT_SYMBOL(md_bitmap_unplug_async);
 
 static void md_bitmap_set_memory_bits(struct bitmap *bitmap, sector_t offset, int needed);
-/* * bitmap_init_from_disk -- called at bitmap_create time to initialize
- * the in-memory bitmap from the on-disk bitmap -- also, sets up the
- * memory mapping of the bitmap file
- * Special cases:
- *   if there's no bitmap file, or if the bitmap file had been
- *   previously kicked from the array, we mark all the bits as
- *   1's in order to cause a full resync.
+
+/*
+ * Initialize the in-memory bitmap from the on-disk bitmap and set up the memory
+ * mapping of the bitmap file.
+ *
+ * Special case: If there's no bitmap file, or if the bitmap file had been
+ * previously kicked from the array, we mark all the bits as 1's in order to
+ * cause a full resync.
  *
  * We ignore all bits for sectors that end earlier than 'start'.
- * This is used when reading an out-of-date bitmap...
+ * This is used when reading an out-of-date bitmap.
  */
 static int md_bitmap_init_from_disk(struct bitmap *bitmap, sector_t start)
 {
-	unsigned long i, chunks, index, oldindex, bit, node_offset = 0;
-	struct page *page = NULL;
-	unsigned long bit_cnt = 0;
-	struct file *file;
-	unsigned long offset;
-	int outofdate;
-	int ret = -ENOSPC;
-	void *paddr;
+	bool outofdate = test_bit(BITMAP_STALE, &bitmap->flags);
+	struct mddev *mddev = bitmap->mddev;
+	unsigned long chunks = bitmap->counts.chunks;
 	struct bitmap_storage *store = &bitmap->storage;
+	struct file *file = store->file;
+	unsigned long node_offset = 0;
+	unsigned long bit_cnt = 0;
+	unsigned long i;
+	int ret;
 
-	chunks = bitmap->counts.chunks;
-	file = store->file;
-
-	if (!file && !bitmap->mddev->bitmap_info.offset) {
+	if (!file && !mddev->bitmap_info.offset) {
 		/* No permanent bitmap - fill with '1s'. */
 		store->filemap = NULL;
 		store->file_pages = 0;
@@ -1106,10 +1104,6 @@ static int md_bitmap_init_from_disk(struct bitmap *bitmap, sector_t start)
 		return 0;
 	}
 
-	outofdate = test_bit(BITMAP_STALE, &bitmap->flags);
-	if (outofdate)
-		pr_warn("%s: bitmap file is out of date, doing full recovery\n", bmname(bitmap));
-
 	if (file && i_size_read(file->f_mapping->host) < store->bytes) {
 		pr_warn("%s: bitmap file too short %lu < %lu\n",
 			bmname(bitmap),
@@ -1118,65 +1112,70 @@ static int md_bitmap_init_from_disk(struct bitmap *bitmap, sector_t start)
 		goto err;
 	}
 
-	oldindex = ~0L;
-	offset = 0;
-	if (!bitmap->mddev->bitmap_info.external)
-		offset = sizeof(bitmap_super_t);
-
-	if (mddev_is_clustered(bitmap->mddev))
+	if (mddev_is_clustered(mddev))
 		node_offset = bitmap->cluster_slot * (DIV_ROUND_UP(store->bytes, PAGE_SIZE));
 
-	for (i = 0; i < chunks; i++) {
-		int b;
-		index = file_page_index(&bitmap->storage, i);
-		bit = file_page_offset(&bitmap->storage, i);
-		if (index != oldindex) { /* this is a new page, read it in */
-			int count;
-			/* unmap the old page, we're done with it */
-			if (index == store->file_pages-1)
-				count = store->bytes - index * PAGE_SIZE;
-			else
-				count = PAGE_SIZE;
-			page = store->filemap[index];
-			if (file)
-				ret = read_file_page(file, index, bitmap,
-						count, page);
-			else
-				ret = read_sb_page(
-					bitmap->mddev,
-					bitmap->mddev->bitmap_info.offset,
-					page,
-					index + node_offset, count);
-
-			if (ret)
-				goto err;
+	for (i = 0; i < store->file_pages; i++) {
+		struct page *page = store->filemap[i];
+		int count;
 
-			oldindex = index;
+		/* unmap the old page, we're done with it */
+		if (i == store->file_pages - 1)
+			count = store->bytes - i * PAGE_SIZE;
+		else
+			count = PAGE_SIZE;
 
-			if (outofdate) {
-				/*
-				 * if bitmap is out of date, dirty the
-				 * whole page and write it out
-				 */
-				paddr = kmap_atomic(page);
-				memset(paddr + offset, 0xff,
-				       PAGE_SIZE - offset);
-				kunmap_atomic(paddr);
-				write_page(bitmap, page, 1);
+		if (file)
+			ret = read_file_page(file, i, bitmap, count, page);
+		else
+			ret = read_sb_page(mddev, mddev->bitmap_info.offset,
+					   page, i + node_offset, count);
+		if (ret)
+			goto err;
+	}
+
+	if (outofdate) {
+		pr_warn("%s: bitmap file is out of date, doing full recovery\n",
+			bmname(bitmap));
+
+		for (i = 0; i < store->file_pages; i++) {
+			struct page *page = store->filemap[i];
+			unsigned long offset = 0;
+			void *paddr;
+	
+			if (i == 0 && !mddev->bitmap_info.external)
+				offset = sizeof(bitmap_super_t);
+
+			/*
+			 * If the bitmap is out of date, dirty the whole page
+			 * and write it out
+			 */
+			paddr = kmap_atomic(page);
+			memset(paddr + offset, 0xff, PAGE_SIZE - offset);
+			kunmap_atomic(paddr);
 
+			write_page(bitmap, page, 1);
+			if (test_bit(BITMAP_WRITE_ERROR, &bitmap->flags)) {
 				ret = -EIO;
-				if (test_bit(BITMAP_WRITE_ERROR,
-					     &bitmap->flags))
-					goto err;
+				goto err;
 			}
 		}
+	}
+
+	for (i = 0; i < chunks; i++) {
+		struct page *page = filemap_get_page(&bitmap->storage, i);
+		unsigned long bit = file_page_offset(&bitmap->storage, i);
+		void *paddr;
+		bool was_set;
+
 		paddr = kmap_atomic(page);
 		if (test_bit(BITMAP_HOSTENDIAN, &bitmap->flags))
-			b = test_bit(bit, paddr);
+			was_set = test_bit(bit, paddr);
 		else
-			b = test_bit_le(bit, paddr);
+			was_set = test_bit_le(bit, paddr);
 		kunmap_atomic(paddr);
-		if (b) {
+
+		if (was_set) {
 			/* if the disk bit is set, set the memory bit */
 			int needed = ((sector_t)(i+1) << bitmap->counts.chunkshift
 				      >= start);
@@ -1185,7 +1184,6 @@ static int md_bitmap_init_from_disk(struct bitmap *bitmap, sector_t start)
 						  needed);
 			bit_cnt++;
 		}
-		offset = 0;
 	}
 
 	pr_debug("%s: bitmap initialized from disk: read %lu pages, set %lu of %lu bits\n",
-- 
2.39.2

Re: [PATCH 06/11] md-bitmap: refactor md_bitmap_init_from_disk

[kernel test robot]

Hi Christoph,

kernel test robot noticed the following build warnings:

[auto build test WARNING on song-md/md-next]
[also build test WARNING on device-mapper-dm/for-next linus/master v6.4-rc6 next-20230615]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Christoph-Hellwig/md-bitmap-initialize-variables-at-declaration-time-in-md_bitmap_file_unmap/20230615-154928
base:   git://git.kernel.org/pub/scm/linux/kernel/git/song/md.git md-next
patch link:    https://lore.kernel.org/r/20230615064840.629492-7-hch%40lst.de
patch subject: [PATCH 06/11] md-bitmap: refactor md_bitmap_init_from_disk
config: x86_64-rhel-8.3-rust (https://download.01.org/0day-ci/archive/20230616/202306160552.smw0qbmb-lkp@intel.com/config)
compiler: clang version 15.0.7 (https://github.com/llvm/llvm-project.git 8dfdcc7b7bf66834a761bd8de445840ef68e4d1a)
reproduce (this is a W=1 build):
        mkdir -p ~/bin
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        git remote add song-md git://git.kernel.org/pub/scm/linux/kernel/git/song/md.git
        git fetch song-md md-next
        git checkout song-md/md-next
        b4 shazam https://lore.kernel.org/r/20230615064840.629492-7-hch@lst.de
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang ~/bin/make.cross W=1 O=build_dir ARCH=x86_64 olddefconfig
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang ~/bin/make.cross W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash drivers/md/

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202306160552.smw0qbmb-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> drivers/md/md-bitmap.c:1107:6: warning: variable 'ret' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized]
           if (file && i_size_read(file->f_mapping->host) < store->bytes) {
               ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/md/md-bitmap.c:1198:9: note: uninitialized use occurs here
           return ret;
                  ^~~
   drivers/md/md-bitmap.c:1107:2: note: remove the 'if' if its condition is always false
           if (file && i_size_read(file->f_mapping->host) < store->bytes) {
           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/md/md-bitmap.c:1090:9: note: initialize the variable 'ret' to silence this warning
           int ret;
                  ^
                   = 0
   1 warning generated.


vim +1107 drivers/md/md-bitmap.c

5479b6ae3886b9 drivers/md/md-bitmap.c Christoph Hellwig 2023-06-15  1068  
5479b6ae3886b9 drivers/md/md-bitmap.c Christoph Hellwig 2023-06-15  1069  /*
5479b6ae3886b9 drivers/md/md-bitmap.c Christoph Hellwig 2023-06-15  1070   * Initialize the in-memory bitmap from the on-disk bitmap and set up the memory
5479b6ae3886b9 drivers/md/md-bitmap.c Christoph Hellwig 2023-06-15  1071   * mapping of the bitmap file.
5479b6ae3886b9 drivers/md/md-bitmap.c Christoph Hellwig 2023-06-15  1072   *
5479b6ae3886b9 drivers/md/md-bitmap.c Christoph Hellwig 2023-06-15  1073   * Special case: If there's no bitmap file, or if the bitmap file had been
5479b6ae3886b9 drivers/md/md-bitmap.c Christoph Hellwig 2023-06-15  1074   * previously kicked from the array, we mark all the bits as 1's in order to
5479b6ae3886b9 drivers/md/md-bitmap.c Christoph Hellwig 2023-06-15  1075   * cause a full resync.
6a07997fc34ac1 drivers/md/bitmap.c    NeilBrown         2005-09-09  1076   *
6a07997fc34ac1 drivers/md/bitmap.c    NeilBrown         2005-09-09  1077   * We ignore all bits for sectors that end earlier than 'start'.
5479b6ae3886b9 drivers/md/md-bitmap.c Christoph Hellwig 2023-06-15  1078   * This is used when reading an out-of-date bitmap.
32a7627cf3a353 drivers/md/bitmap.c    NeilBrown         2005-06-21  1079   */
e64e4018d57271 drivers/md/md-bitmap.c Andy Shevchenko   2018-08-01  1080  static int md_bitmap_init_from_disk(struct bitmap *bitmap, sector_t start)
32a7627cf3a353 drivers/md/bitmap.c    NeilBrown         2005-06-21  1081  {
5479b6ae3886b9 drivers/md/md-bitmap.c Christoph Hellwig 2023-06-15  1082  	bool outofdate = test_bit(BITMAP_STALE, &bitmap->flags);
5479b6ae3886b9 drivers/md/md-bitmap.c Christoph Hellwig 2023-06-15  1083  	struct mddev *mddev = bitmap->mddev;
5479b6ae3886b9 drivers/md/md-bitmap.c Christoph Hellwig 2023-06-15  1084  	unsigned long chunks = bitmap->counts.chunks;
1ec885cdd01a9a drivers/md/bitmap.c    NeilBrown         2012-05-22  1085  	struct bitmap_storage *store = &bitmap->storage;
5479b6ae3886b9 drivers/md/md-bitmap.c Christoph Hellwig 2023-06-15  1086  	struct file *file = store->file;
5479b6ae3886b9 drivers/md/md-bitmap.c Christoph Hellwig 2023-06-15  1087  	unsigned long node_offset = 0;
5479b6ae3886b9 drivers/md/md-bitmap.c Christoph Hellwig 2023-06-15  1088  	unsigned long bit_cnt = 0;
5479b6ae3886b9 drivers/md/md-bitmap.c Christoph Hellwig 2023-06-15  1089  	unsigned long i;
5479b6ae3886b9 drivers/md/md-bitmap.c Christoph Hellwig 2023-06-15  1090  	int ret;
32a7627cf3a353 drivers/md/bitmap.c    NeilBrown         2005-06-21  1091  
5479b6ae3886b9 drivers/md/md-bitmap.c Christoph Hellwig 2023-06-15  1092  	if (!file && !mddev->bitmap_info.offset) {
ef99bf480de9bd drivers/md/bitmap.c    NeilBrown         2012-05-22  1093  		/* No permanent bitmap - fill with '1s'. */
1ec885cdd01a9a drivers/md/bitmap.c    NeilBrown         2012-05-22  1094  		store->filemap = NULL;
1ec885cdd01a9a drivers/md/bitmap.c    NeilBrown         2012-05-22  1095  		store->file_pages = 0;
ef99bf480de9bd drivers/md/bitmap.c    NeilBrown         2012-05-22  1096  		for (i = 0; i < chunks ; i++) {
ef99bf480de9bd drivers/md/bitmap.c    NeilBrown         2012-05-22  1097  			/* if the disk bit is set, set the memory bit */
40cffcc0e8f9f6 drivers/md/bitmap.c    NeilBrown         2012-05-22  1098  			int needed = ((sector_t)(i+1) << (bitmap->counts.chunkshift)
ef99bf480de9bd drivers/md/bitmap.c    NeilBrown         2012-05-22  1099  				      >= start);
e64e4018d57271 drivers/md/md-bitmap.c Andy Shevchenko   2018-08-01  1100  			md_bitmap_set_memory_bits(bitmap,
40cffcc0e8f9f6 drivers/md/bitmap.c    NeilBrown         2012-05-22  1101  						  (sector_t)i << bitmap->counts.chunkshift,
ef99bf480de9bd drivers/md/bitmap.c    NeilBrown         2012-05-22  1102  						  needed);
ef99bf480de9bd drivers/md/bitmap.c    NeilBrown         2012-05-22  1103  		}
ef99bf480de9bd drivers/md/bitmap.c    NeilBrown         2012-05-22  1104  		return 0;
ef99bf480de9bd drivers/md/bitmap.c    NeilBrown         2012-05-22  1105  	}
32a7627cf3a353 drivers/md/bitmap.c    NeilBrown         2005-06-21  1106  
d1244cb062750b drivers/md/bitmap.c    NeilBrown         2012-05-22 @1107  	if (file && i_size_read(file->f_mapping->host) < store->bytes) {
ec0cc226854a79 drivers/md/bitmap.c    NeilBrown         2016-11-02  1108  		pr_warn("%s: bitmap file too short %lu < %lu\n",
32a7627cf3a353 drivers/md/bitmap.c    NeilBrown         2005-06-21  1109  			bmname(bitmap),
32a7627cf3a353 drivers/md/bitmap.c    NeilBrown         2005-06-21  1110  			(unsigned long) i_size_read(file->f_mapping->host),
d1244cb062750b drivers/md/bitmap.c    NeilBrown         2012-05-22  1111  			store->bytes);
4ad1366376bfef drivers/md/bitmap.c    NeilBrown         2007-07-17  1112  		goto err;
32a7627cf3a353 drivers/md/bitmap.c    NeilBrown         2005-06-21  1113  	}
bc7f77de2cd817 drivers/md/bitmap.c    NeilBrown         2005-06-21  1114  
5479b6ae3886b9 drivers/md/md-bitmap.c Christoph Hellwig 2023-06-15  1115  	if (mddev_is_clustered(mddev))
b97e92574c0bf3 drivers/md/bitmap.c    Goldwyn Rodrigues 2014-06-06  1116  		node_offset = bitmap->cluster_slot * (DIV_ROUND_UP(store->bytes, PAGE_SIZE));
b97e92574c0bf3 drivers/md/bitmap.c    Goldwyn Rodrigues 2014-06-06  1117  
5479b6ae3886b9 drivers/md/md-bitmap.c Christoph Hellwig 2023-06-15  1118  	for (i = 0; i < store->file_pages; i++) {
5479b6ae3886b9 drivers/md/md-bitmap.c Christoph Hellwig 2023-06-15  1119  		struct page *page = store->filemap[i];
d785a06a0b9d0c drivers/md/bitmap.c    NeilBrown         2006-06-26  1120  		int count;
5479b6ae3886b9 drivers/md/md-bitmap.c Christoph Hellwig 2023-06-15  1121  
32a7627cf3a353 drivers/md/bitmap.c    NeilBrown         2005-06-21  1122  		/* unmap the old page, we're done with it */
5479b6ae3886b9 drivers/md/md-bitmap.c Christoph Hellwig 2023-06-15  1123  		if (i == store->file_pages - 1)
5479b6ae3886b9 drivers/md/md-bitmap.c Christoph Hellwig 2023-06-15  1124  			count = store->bytes - i * PAGE_SIZE;
d785a06a0b9d0c drivers/md/bitmap.c    NeilBrown         2006-06-26  1125  		else
d785a06a0b9d0c drivers/md/bitmap.c    NeilBrown         2006-06-26  1126  			count = PAGE_SIZE;
5479b6ae3886b9 drivers/md/md-bitmap.c Christoph Hellwig 2023-06-15  1127  
27581e5ae01f77 drivers/md/bitmap.c    NeilBrown         2012-05-22  1128  		if (file)
5479b6ae3886b9 drivers/md/md-bitmap.c Christoph Hellwig 2023-06-15  1129  			ret = read_file_page(file, i, bitmap, count, page);
27581e5ae01f77 drivers/md/bitmap.c    NeilBrown         2012-05-22  1130  		else
5479b6ae3886b9 drivers/md/md-bitmap.c Christoph Hellwig 2023-06-15  1131  			ret = read_sb_page(mddev, mddev->bitmap_info.offset,
5479b6ae3886b9 drivers/md/md-bitmap.c Christoph Hellwig 2023-06-15  1132  					   page, i + node_offset, count);
27581e5ae01f77 drivers/md/bitmap.c    NeilBrown         2012-05-22  1133  		if (ret)
4ad1366376bfef drivers/md/bitmap.c    NeilBrown         2007-07-17  1134  			goto err;
5479b6ae3886b9 drivers/md/md-bitmap.c Christoph Hellwig 2023-06-15  1135  	}
32a7627cf3a353 drivers/md/bitmap.c    NeilBrown         2005-06-21  1136  
32a7627cf3a353 drivers/md/bitmap.c    NeilBrown         2005-06-21  1137  	if (outofdate) {
5479b6ae3886b9 drivers/md/md-bitmap.c Christoph Hellwig 2023-06-15  1138  		pr_warn("%s: bitmap file is out of date, doing full recovery\n",
5479b6ae3886b9 drivers/md/md-bitmap.c Christoph Hellwig 2023-06-15  1139  			bmname(bitmap));
5479b6ae3886b9 drivers/md/md-bitmap.c Christoph Hellwig 2023-06-15  1140  
5479b6ae3886b9 drivers/md/md-bitmap.c Christoph Hellwig 2023-06-15  1141  		for (i = 0; i < store->file_pages; i++) {
5479b6ae3886b9 drivers/md/md-bitmap.c Christoph Hellwig 2023-06-15  1142  			struct page *page = store->filemap[i];
5479b6ae3886b9 drivers/md/md-bitmap.c Christoph Hellwig 2023-06-15  1143  			unsigned long offset = 0;
5479b6ae3886b9 drivers/md/md-bitmap.c Christoph Hellwig 2023-06-15  1144  			void *paddr;
5479b6ae3886b9 drivers/md/md-bitmap.c Christoph Hellwig 2023-06-15  1145  	
5479b6ae3886b9 drivers/md/md-bitmap.c Christoph Hellwig 2023-06-15  1146  			if (i == 0 && !mddev->bitmap_info.external)
5479b6ae3886b9 drivers/md/md-bitmap.c Christoph Hellwig 2023-06-15  1147  				offset = sizeof(bitmap_super_t);
5479b6ae3886b9 drivers/md/md-bitmap.c Christoph Hellwig 2023-06-15  1148  
32a7627cf3a353 drivers/md/bitmap.c    NeilBrown         2005-06-21  1149  			/*
5479b6ae3886b9 drivers/md/md-bitmap.c Christoph Hellwig 2023-06-15  1150  			 * If the bitmap is out of date, dirty the whole page
5479b6ae3886b9 drivers/md/md-bitmap.c Christoph Hellwig 2023-06-15  1151  			 * and write it out
32a7627cf3a353 drivers/md/bitmap.c    NeilBrown         2005-06-21  1152  			 */
b2f46e68825648 drivers/md/bitmap.c    Cong Wang         2011-11-28  1153  			paddr = kmap_atomic(page);
5479b6ae3886b9 drivers/md/md-bitmap.c Christoph Hellwig 2023-06-15  1154  			memset(paddr + offset, 0xff, PAGE_SIZE - offset);
b2f46e68825648 drivers/md/bitmap.c    Cong Wang         2011-11-28  1155  			kunmap_atomic(paddr);
4ad1366376bfef drivers/md/bitmap.c    NeilBrown         2007-07-17  1156  
5479b6ae3886b9 drivers/md/md-bitmap.c Christoph Hellwig 2023-06-15  1157  			write_page(bitmap, page, 1);
5479b6ae3886b9 drivers/md/md-bitmap.c Christoph Hellwig 2023-06-15  1158  			if (test_bit(BITMAP_WRITE_ERROR, &bitmap->flags)) {
4ad1366376bfef drivers/md/bitmap.c    NeilBrown         2007-07-17  1159  				ret = -EIO;
4ad1366376bfef drivers/md/bitmap.c    NeilBrown         2007-07-17  1160  				goto err;
32a7627cf3a353 drivers/md/bitmap.c    NeilBrown         2005-06-21  1161  			}
32a7627cf3a353 drivers/md/bitmap.c    NeilBrown         2005-06-21  1162  		}
5479b6ae3886b9 drivers/md/md-bitmap.c Christoph Hellwig 2023-06-15  1163  	}
5479b6ae3886b9 drivers/md/md-bitmap.c Christoph Hellwig 2023-06-15  1164  
5479b6ae3886b9 drivers/md/md-bitmap.c Christoph Hellwig 2023-06-15  1165  	for (i = 0; i < chunks; i++) {
5479b6ae3886b9 drivers/md/md-bitmap.c Christoph Hellwig 2023-06-15  1166  		struct page *page = filemap_get_page(&bitmap->storage, i);
5479b6ae3886b9 drivers/md/md-bitmap.c Christoph Hellwig 2023-06-15  1167  		unsigned long bit = file_page_offset(&bitmap->storage, i);
5479b6ae3886b9 drivers/md/md-bitmap.c Christoph Hellwig 2023-06-15  1168  		void *paddr;
5479b6ae3886b9 drivers/md/md-bitmap.c Christoph Hellwig 2023-06-15  1169  		bool was_set;
5479b6ae3886b9 drivers/md/md-bitmap.c Christoph Hellwig 2023-06-15  1170  
b2f46e68825648 drivers/md/bitmap.c    Cong Wang         2011-11-28  1171  		paddr = kmap_atomic(page);
b405fe91e50c60 drivers/md/bitmap.c    NeilBrown         2012-05-22  1172  		if (test_bit(BITMAP_HOSTENDIAN, &bitmap->flags))
5479b6ae3886b9 drivers/md/md-bitmap.c Christoph Hellwig 2023-06-15  1173  			was_set = test_bit(bit, paddr);
bd926c63b7a684 drivers/md/bitmap.c    NeilBrown         2005-11-08  1174  		else
5479b6ae3886b9 drivers/md/md-bitmap.c Christoph Hellwig 2023-06-15  1175  			was_set = test_bit_le(bit, paddr);
b2f46e68825648 drivers/md/bitmap.c    Cong Wang         2011-11-28  1176  		kunmap_atomic(paddr);
5479b6ae3886b9 drivers/md/md-bitmap.c Christoph Hellwig 2023-06-15  1177  
5479b6ae3886b9 drivers/md/md-bitmap.c Christoph Hellwig 2023-06-15  1178  		if (was_set) {
32a7627cf3a353 drivers/md/bitmap.c    NeilBrown         2005-06-21  1179  			/* if the disk bit is set, set the memory bit */
40cffcc0e8f9f6 drivers/md/bitmap.c    NeilBrown         2012-05-22  1180  			int needed = ((sector_t)(i+1) << bitmap->counts.chunkshift
db305e507d5544 drivers/md/bitmap.c    NeilBrown         2009-05-07  1181  				      >= start);
e64e4018d57271 drivers/md/md-bitmap.c Andy Shevchenko   2018-08-01  1182  			md_bitmap_set_memory_bits(bitmap,
40cffcc0e8f9f6 drivers/md/bitmap.c    NeilBrown         2012-05-22  1183  						  (sector_t)i << bitmap->counts.chunkshift,
db305e507d5544 drivers/md/bitmap.c    NeilBrown         2009-05-07  1184  						  needed);
32a7627cf3a353 drivers/md/bitmap.c    NeilBrown         2005-06-21  1185  			bit_cnt++;
32a7627cf3a353 drivers/md/bitmap.c    NeilBrown         2005-06-21  1186  		}
32a7627cf3a353 drivers/md/bitmap.c    NeilBrown         2005-06-21  1187  	}
32a7627cf3a353 drivers/md/bitmap.c    NeilBrown         2005-06-21  1188  
ec0cc226854a79 drivers/md/bitmap.c    NeilBrown         2016-11-02  1189  	pr_debug("%s: bitmap initialized from disk: read %lu pages, set %lu of %lu bits\n",
1ec885cdd01a9a drivers/md/bitmap.c    NeilBrown         2012-05-22  1190  		 bmname(bitmap), store->file_pages,
d1244cb062750b drivers/md/bitmap.c    NeilBrown         2012-05-22  1191  		 bit_cnt, chunks);
32a7627cf3a353 drivers/md/bitmap.c    NeilBrown         2005-06-21  1192  
4ad1366376bfef drivers/md/bitmap.c    NeilBrown         2007-07-17  1193  	return 0;
4ad1366376bfef drivers/md/bitmap.c    NeilBrown         2007-07-17  1194  
4ad1366376bfef drivers/md/bitmap.c    NeilBrown         2007-07-17  1195   err:
ec0cc226854a79 drivers/md/bitmap.c    NeilBrown         2016-11-02  1196  	pr_warn("%s: bitmap initialisation failed: %d\n",
4ad1366376bfef drivers/md/bitmap.c    NeilBrown         2007-07-17  1197  		bmname(bitmap), ret);
32a7627cf3a353 drivers/md/bitmap.c    NeilBrown         2005-06-21  1198  	return ret;
32a7627cf3a353 drivers/md/bitmap.c    NeilBrown         2005-06-21  1199  }
32a7627cf3a353 drivers/md/bitmap.c    NeilBrown         2005-06-21  1200  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH 06/11] md-bitmap: refactor md_bitmap_init_from_disk

[Song Liu]

On Thu, Jun 15, 2023 at 3:01 PM kernel test robot <lkp@intel.com> wrote:
>
> Hi Christoph,
>
> kernel test robot noticed the following build warnings:
>
> [auto build test WARNING on song-md/md-next]
> [also build test WARNING on device-mapper-dm/for-next linus/master v6.4-rc6 next-20230615]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use '--base' as documented in
> https://git-scm.com/docs/git-format-patch#_base_tree_information]
>
> url:    https://github.com/intel-lab-lkp/linux/commits/Christoph-Hellwig/md-bitmap-initialize-variables-at-declaration-time-in-md_bitmap_file_unmap/20230615-154928
> base:   git://git.kernel.org/pub/scm/linux/kernel/git/song/md.git md-next
> patch link:    https://lore.kernel.org/r/20230615064840.629492-7-hch%40lst.de
> patch subject: [PATCH 06/11] md-bitmap: refactor md_bitmap_init_from_disk
> config: x86_64-rhel-8.3-rust (https://download.01.org/0day-ci/archive/20230616/202306160552.smw0qbmb-lkp@intel.com/config)
> compiler: clang version 15.0.7 (https://github.com/llvm/llvm-project.git 8dfdcc7b7bf66834a761bd8de445840ef68e4d1a)
> reproduce (this is a W=1 build):
>         mkdir -p ~/bin
>         wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
>         chmod +x ~/bin/make.cross
>         git remote add song-md git://git.kernel.org/pub/scm/linux/kernel/git/song/md.git
>         git fetch song-md md-next
>         git checkout song-md/md-next
>         b4 shazam https://lore.kernel.org/r/20230615064840.629492-7-hch@lst.de
>         # save the config file
>         mkdir build_dir && cp config build_dir/.config
>         COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang ~/bin/make.cross W=1 O=build_dir ARCH=x86_64 olddefconfig
>         COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang ~/bin/make.cross W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash drivers/md/
>
> If you fix the issue in a separate patch/commit (i.e. not just a new version of
> the same patch/commit), kindly add following tags
> | Reported-by: kernel test robot <lkp@intel.com>
> | Closes: https://lore.kernel.org/oe-kbuild-all/202306160552.smw0qbmb-lkp@intel.com/

I fixed this one, and applied the set to md-next.

Thanks,
Song

>
> All warnings (new ones prefixed by >>):
>
> >> drivers/md/md-bitmap.c:1107:6: warning: variable 'ret' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized]
>            if (file && i_size_read(file->f_mapping->host) < store->bytes) {
>                ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>    drivers/md/md-bitmap.c:1198:9: note: uninitialized use occurs here
>            return ret;
>                   ^~~
>    drivers/md/md-bitmap.c:1107:2: note: remove the 'if' if its condition is always false
>            if (file && i_size_read(file->f_mapping->host) < store->bytes) {
>            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>    drivers/md/md-bitmap.c:1090:9: note: initialize the variable 'ret' to silence this warning
>            int ret;
>                   ^
>                    = 0
>    1 warning generated.
>
>
> vim +1107 drivers/md/md-bitmap.c
>
> 5479b6ae3886b9 drivers/md/md-bitmap.c Christoph Hellwig 2023-06-15  1068
> 5479b6ae3886b9 drivers/md/md-bitmap.c Christoph Hellwig 2023-06-15  1069  /*
> 5479b6ae3886b9 drivers/md/md-bitmap.c Christoph Hellwig 2023-06-15  1070   * Initialize the in-memory bitmap from the on-disk bitmap and set up the memory
> 5479b6ae3886b9 drivers/md/md-bitmap.c Christoph Hellwig 2023-06-15  1071   * mapping of the bitmap file.
> 5479b6ae3886b9 drivers/md/md-bitmap.c Christoph Hellwig 2023-06-15  1072   *
> 5479b6ae3886b9 drivers/md/md-bitmap.c Christoph Hellwig 2023-06-15  1073   * Special case: If there's no bitmap file, or if the bitmap file had been
> 5479b6ae3886b9 drivers/md/md-bitmap.c Christoph Hellwig 2023-06-15  1074   * previously kicked from the array, we mark all the bits as 1's in order to
> 5479b6ae3886b9 drivers/md/md-bitmap.c Christoph Hellwig 2023-06-15  1075   * cause a full resync.
> 6a07997fc34ac1 drivers/md/bitmap.c    NeilBrown         2005-09-09  1076   *
> 6a07997fc34ac1 drivers/md/bitmap.c    NeilBrown         2005-09-09  1077   * We ignore all bits for sectors that end earlier than 'start'.
> 5479b6ae3886b9 drivers/md/md-bitmap.c Christoph Hellwig 2023-06-15  1078   * This is used when reading an out-of-date bitmap.
> 32a7627cf3a353 drivers/md/bitmap.c    NeilBrown         2005-06-21  1079   */
> e64e4018d57271 drivers/md/md-bitmap.c Andy Shevchenko   2018-08-01  1080  static int md_bitmap_init_from_disk(struct bitmap *bitmap, sector_t start)
> 32a7627cf3a353 drivers/md/bitmap.c    NeilBrown         2005-06-21  1081  {
> 5479b6ae3886b9 drivers/md/md-bitmap.c Christoph Hellwig 2023-06-15  1082        bool outofdate = test_bit(BITMAP_STALE, &bitmap->flags);
> 5479b6ae3886b9 drivers/md/md-bitmap.c Christoph Hellwig 2023-06-15  1083        struct mddev *mddev = bitmap->mddev;
> 5479b6ae3886b9 drivers/md/md-bitmap.c Christoph Hellwig 2023-06-15  1084        unsigned long chunks = bitmap->counts.chunks;
> 1ec885cdd01a9a drivers/md/bitmap.c    NeilBrown         2012-05-22  1085        struct bitmap_storage *store = &bitmap->storage;
> 5479b6ae3886b9 drivers/md/md-bitmap.c Christoph Hellwig 2023-06-15  1086        struct file *file = store->file;
> 5479b6ae3886b9 drivers/md/md-bitmap.c Christoph Hellwig 2023-06-15  1087        unsigned long node_offset = 0;
> 5479b6ae3886b9 drivers/md/md-bitmap.c Christoph Hellwig 2023-06-15  1088        unsigned long bit_cnt = 0;
> 5479b6ae3886b9 drivers/md/md-bitmap.c Christoph Hellwig 2023-06-15  1089        unsigned long i;
> 5479b6ae3886b9 drivers/md/md-bitmap.c Christoph Hellwig 2023-06-15  1090        int ret;
> 32a7627cf3a353 drivers/md/bitmap.c    NeilBrown         2005-06-21  1091
> 5479b6ae3886b9 drivers/md/md-bitmap.c Christoph Hellwig 2023-06-15  1092        if (!file && !mddev->bitmap_info.offset) {
> ef99bf480de9bd drivers/md/bitmap.c    NeilBrown         2012-05-22  1093                /* No permanent bitmap - fill with '1s'. */
> 1ec885cdd01a9a drivers/md/bitmap.c    NeilBrown         2012-05-22  1094                store->filemap = NULL;
> 1ec885cdd01a9a drivers/md/bitmap.c    NeilBrown         2012-05-22  1095                store->file_pages = 0;
> ef99bf480de9bd drivers/md/bitmap.c    NeilBrown         2012-05-22  1096                for (i = 0; i < chunks ; i++) {
> ef99bf480de9bd drivers/md/bitmap.c    NeilBrown         2012-05-22  1097                        /* if the disk bit is set, set the memory bit */
> 40cffcc0e8f9f6 drivers/md/bitmap.c    NeilBrown         2012-05-22  1098                        int needed = ((sector_t)(i+1) << (bitmap->counts.chunkshift)
> ef99bf480de9bd drivers/md/bitmap.c    NeilBrown         2012-05-22  1099                                      >= start);
> e64e4018d57271 drivers/md/md-bitmap.c Andy Shevchenko   2018-08-01  1100                        md_bitmap_set_memory_bits(bitmap,
> 40cffcc0e8f9f6 drivers/md/bitmap.c    NeilBrown         2012-05-22  1101                                                  (sector_t)i << bitmap->counts.chunkshift,
> ef99bf480de9bd drivers/md/bitmap.c    NeilBrown         2012-05-22  1102                                                  needed);
> ef99bf480de9bd drivers/md/bitmap.c    NeilBrown         2012-05-22  1103                }
> ef99bf480de9bd drivers/md/bitmap.c    NeilBrown         2012-05-22  1104                return 0;
> ef99bf480de9bd drivers/md/bitmap.c    NeilBrown         2012-05-22  1105        }
> 32a7627cf3a353 drivers/md/bitmap.c    NeilBrown         2005-06-21  1106
> d1244cb062750b drivers/md/bitmap.c    NeilBrown         2012-05-22 @1107        if (file && i_size_read(file->f_mapping->host) < store->bytes) {
> ec0cc226854a79 drivers/md/bitmap.c    NeilBrown         2016-11-02  1108                pr_warn("%s: bitmap file too short %lu < %lu\n",
> 32a7627cf3a353 drivers/md/bitmap.c    NeilBrown         2005-06-21  1109                        bmname(bitmap),
> 32a7627cf3a353 drivers/md/bitmap.c    NeilBrown         2005-06-21  1110                        (unsigned long) i_size_read(file->f_mapping->host),
> d1244cb062750b drivers/md/bitmap.c    NeilBrown         2012-05-22  1111                        store->bytes);
> 4ad1366376bfef drivers/md/bitmap.c    NeilBrown         2007-07-17  1112                goto err;
> 32a7627cf3a353 drivers/md/bitmap.c    NeilBrown         2005-06-21  1113        }
> bc7f77de2cd817 drivers/md/bitmap.c    NeilBrown         2005-06-21  1114
> 5479b6ae3886b9 drivers/md/md-bitmap.c Christoph Hellwig 2023-06-15  1115        if (mddev_is_clustered(mddev))
> b97e92574c0bf3 drivers/md/bitmap.c    Goldwyn Rodrigues 2014-06-06  1116                node_offset = bitmap->cluster_slot * (DIV_ROUND_UP(store->bytes, PAGE_SIZE));
> b97e92574c0bf3 drivers/md/bitmap.c    Goldwyn Rodrigues 2014-06-06  1117
> 5479b6ae3886b9 drivers/md/md-bitmap.c Christoph Hellwig 2023-06-15  1118        for (i = 0; i < store->file_pages; i++) {
> 5479b6ae3886b9 drivers/md/md-bitmap.c Christoph Hellwig 2023-06-15  1119                struct page *page = store->filemap[i];
> d785a06a0b9d0c drivers/md/bitmap.c    NeilBrown         2006-06-26  1120                int count;
> 5479b6ae3886b9 drivers/md/md-bitmap.c Christoph Hellwig 2023-06-15  1121
> 32a7627cf3a353 drivers/md/bitmap.c    NeilBrown         2005-06-21  1122                /* unmap the old page, we're done with it */
> 5479b6ae3886b9 drivers/md/md-bitmap.c Christoph Hellwig 2023-06-15  1123                if (i == store->file_pages - 1)
> 5479b6ae3886b9 drivers/md/md-bitmap.c Christoph Hellwig 2023-06-15  1124                        count = store->bytes - i * PAGE_SIZE;
> d785a06a0b9d0c drivers/md/bitmap.c    NeilBrown         2006-06-26  1125                else
> d785a06a0b9d0c drivers/md/bitmap.c    NeilBrown         2006-06-26  1126                        count = PAGE_SIZE;
> 5479b6ae3886b9 drivers/md/md-bitmap.c Christoph Hellwig 2023-06-15  1127
> 27581e5ae01f77 drivers/md/bitmap.c    NeilBrown         2012-05-22  1128                if (file)
> 5479b6ae3886b9 drivers/md/md-bitmap.c Christoph Hellwig 2023-06-15  1129                        ret = read_file_page(file, i, bitmap, count, page);
> 27581e5ae01f77 drivers/md/bitmap.c    NeilBrown         2012-05-22  1130                else
> 5479b6ae3886b9 drivers/md/md-bitmap.c Christoph Hellwig 2023-06-15  1131                        ret = read_sb_page(mddev, mddev->bitmap_info.offset,
> 5479b6ae3886b9 drivers/md/md-bitmap.c Christoph Hellwig 2023-06-15  1132                                           page, i + node_offset, count);
> 27581e5ae01f77 drivers/md/bitmap.c    NeilBrown         2012-05-22  1133                if (ret)
> 4ad1366376bfef drivers/md/bitmap.c    NeilBrown         2007-07-17  1134                        goto err;
> 5479b6ae3886b9 drivers/md/md-bitmap.c Christoph Hellwig 2023-06-15  1135        }
> 32a7627cf3a353 drivers/md/bitmap.c    NeilBrown         2005-06-21  1136
> 32a7627cf3a353 drivers/md/bitmap.c    NeilBrown         2005-06-21  1137        if (outofdate) {
> 5479b6ae3886b9 drivers/md/md-bitmap.c Christoph Hellwig 2023-06-15  1138                pr_warn("%s: bitmap file is out of date, doing full recovery\n",
> 5479b6ae3886b9 drivers/md/md-bitmap.c Christoph Hellwig 2023-06-15  1139                        bmname(bitmap));
> 5479b6ae3886b9 drivers/md/md-bitmap.c Christoph Hellwig 2023-06-15  1140
> 5479b6ae3886b9 drivers/md/md-bitmap.c Christoph Hellwig 2023-06-15  1141                for (i = 0; i < store->file_pages; i++) {
> 5479b6ae3886b9 drivers/md/md-bitmap.c Christoph Hellwig 2023-06-15  1142                        struct page *page = store->filemap[i];
> 5479b6ae3886b9 drivers/md/md-bitmap.c Christoph Hellwig 2023-06-15  1143                        unsigned long offset = 0;
> 5479b6ae3886b9 drivers/md/md-bitmap.c Christoph Hellwig 2023-06-15  1144                        void *paddr;
> 5479b6ae3886b9 drivers/md/md-bitmap.c Christoph Hellwig 2023-06-15  1145
> 5479b6ae3886b9 drivers/md/md-bitmap.c Christoph Hellwig 2023-06-15  1146                        if (i == 0 && !mddev->bitmap_info.external)
> 5479b6ae3886b9 drivers/md/md-bitmap.c Christoph Hellwig 2023-06-15  1147                                offset = sizeof(bitmap_super_t);
> 5479b6ae3886b9 drivers/md/md-bitmap.c Christoph Hellwig 2023-06-15  1148
> 32a7627cf3a353 drivers/md/bitmap.c    NeilBrown         2005-06-21  1149                        /*
> 5479b6ae3886b9 drivers/md/md-bitmap.c Christoph Hellwig 2023-06-15  1150                         * If the bitmap is out of date, dirty the whole page
> 5479b6ae3886b9 drivers/md/md-bitmap.c Christoph Hellwig 2023-06-15  1151                         * and write it out
> 32a7627cf3a353 drivers/md/bitmap.c    NeilBrown         2005-06-21  1152                         */
> b2f46e68825648 drivers/md/bitmap.c    Cong Wang         2011-11-28  1153                        paddr = kmap_atomic(page);
> 5479b6ae3886b9 drivers/md/md-bitmap.c Christoph Hellwig 2023-06-15  1154                        memset(paddr + offset, 0xff, PAGE_SIZE - offset);
> b2f46e68825648 drivers/md/bitmap.c    Cong Wang         2011-11-28  1155                        kunmap_atomic(paddr);
> 4ad1366376bfef drivers/md/bitmap.c    NeilBrown         2007-07-17  1156
> 5479b6ae3886b9 drivers/md/md-bitmap.c Christoph Hellwig 2023-06-15  1157                        write_page(bitmap, page, 1);
> 5479b6ae3886b9 drivers/md/md-bitmap.c Christoph Hellwig 2023-06-15  1158                        if (test_bit(BITMAP_WRITE_ERROR, &bitmap->flags)) {
> 4ad1366376bfef drivers/md/bitmap.c    NeilBrown         2007-07-17  1159                                ret = -EIO;
> 4ad1366376bfef drivers/md/bitmap.c    NeilBrown         2007-07-17  1160                                goto err;
> 32a7627cf3a353 drivers/md/bitmap.c    NeilBrown         2005-06-21  1161                        }
> 32a7627cf3a353 drivers/md/bitmap.c    NeilBrown         2005-06-21  1162                }
> 5479b6ae3886b9 drivers/md/md-bitmap.c Christoph Hellwig 2023-06-15  1163        }
> 5479b6ae3886b9 drivers/md/md-bitmap.c Christoph Hellwig 2023-06-15  1164
> 5479b6ae3886b9 drivers/md/md-bitmap.c Christoph Hellwig 2023-06-15  1165        for (i = 0; i < chunks; i++) {
> 5479b6ae3886b9 drivers/md/md-bitmap.c Christoph Hellwig 2023-06-15  1166                struct page *page = filemap_get_page(&bitmap->storage, i);
> 5479b6ae3886b9 drivers/md/md-bitmap.c Christoph Hellwig 2023-06-15  1167                unsigned long bit = file_page_offset(&bitmap->storage, i);
> 5479b6ae3886b9 drivers/md/md-bitmap.c Christoph Hellwig 2023-06-15  1168                void *paddr;
> 5479b6ae3886b9 drivers/md/md-bitmap.c Christoph Hellwig 2023-06-15  1169                bool was_set;
> 5479b6ae3886b9 drivers/md/md-bitmap.c Christoph Hellwig 2023-06-15  1170
> b2f46e68825648 drivers/md/bitmap.c    Cong Wang         2011-11-28  1171                paddr = kmap_atomic(page);
> b405fe91e50c60 drivers/md/bitmap.c    NeilBrown         2012-05-22  1172                if (test_bit(BITMAP_HOSTENDIAN, &bitmap->flags))
> 5479b6ae3886b9 drivers/md/md-bitmap.c Christoph Hellwig 2023-06-15  1173                        was_set = test_bit(bit, paddr);
> bd926c63b7a684 drivers/md/bitmap.c    NeilBrown         2005-11-08  1174                else
> 5479b6ae3886b9 drivers/md/md-bitmap.c Christoph Hellwig 2023-06-15  1175                        was_set = test_bit_le(bit, paddr);
> b2f46e68825648 drivers/md/bitmap.c    Cong Wang         2011-11-28  1176                kunmap_atomic(paddr);
> 5479b6ae3886b9 drivers/md/md-bitmap.c Christoph Hellwig 2023-06-15  1177
> 5479b6ae3886b9 drivers/md/md-bitmap.c Christoph Hellwig 2023-06-15  1178                if (was_set) {
> 32a7627cf3a353 drivers/md/bitmap.c    NeilBrown         2005-06-21  1179                        /* if the disk bit is set, set the memory bit */
> 40cffcc0e8f9f6 drivers/md/bitmap.c    NeilBrown         2012-05-22  1180                        int needed = ((sector_t)(i+1) << bitmap->counts.chunkshift
> db305e507d5544 drivers/md/bitmap.c    NeilBrown         2009-05-07  1181                                      >= start);
> e64e4018d57271 drivers/md/md-bitmap.c Andy Shevchenko   2018-08-01  1182                        md_bitmap_set_memory_bits(bitmap,
> 40cffcc0e8f9f6 drivers/md/bitmap.c    NeilBrown         2012-05-22  1183                                                  (sector_t)i << bitmap->counts.chunkshift,
> db305e507d5544 drivers/md/bitmap.c    NeilBrown         2009-05-07  1184                                                  needed);
> 32a7627cf3a353 drivers/md/bitmap.c    NeilBrown         2005-06-21  1185                        bit_cnt++;
> 32a7627cf3a353 drivers/md/bitmap.c    NeilBrown         2005-06-21  1186                }
> 32a7627cf3a353 drivers/md/bitmap.c    NeilBrown         2005-06-21  1187        }
> 32a7627cf3a353 drivers/md/bitmap.c    NeilBrown         2005-06-21  1188
> ec0cc226854a79 drivers/md/bitmap.c    NeilBrown         2016-11-02  1189        pr_debug("%s: bitmap initialized from disk: read %lu pages, set %lu of %lu bits\n",
> 1ec885cdd01a9a drivers/md/bitmap.c    NeilBrown         2012-05-22  1190                 bmname(bitmap), store->file_pages,
> d1244cb062750b drivers/md/bitmap.c    NeilBrown         2012-05-22  1191                 bit_cnt, chunks);
> 32a7627cf3a353 drivers/md/bitmap.c    NeilBrown         2005-06-21  1192
> 4ad1366376bfef drivers/md/bitmap.c    NeilBrown         2007-07-17  1193        return 0;
> 4ad1366376bfef drivers/md/bitmap.c    NeilBrown         2007-07-17  1194
> 4ad1366376bfef drivers/md/bitmap.c    NeilBrown         2007-07-17  1195   err:
> ec0cc226854a79 drivers/md/bitmap.c    NeilBrown         2016-11-02  1196        pr_warn("%s: bitmap initialisation failed: %d\n",
> 4ad1366376bfef drivers/md/bitmap.c    NeilBrown         2007-07-17  1197                bmname(bitmap), ret);
> 32a7627cf3a353 drivers/md/bitmap.c    NeilBrown         2005-06-21  1198        return ret;
> 32a7627cf3a353 drivers/md/bitmap.c    NeilBrown         2005-06-21  1199  }
> 32a7627cf3a353 drivers/md/bitmap.c    NeilBrown         2005-06-21  1200
>
> --
> 0-DAY CI Kernel Test Service
> https://github.com/intel/lkp-tests/wiki

Re: [PATCH 06/11] md-bitmap: refactor md_bitmap_init_from_disk

[Christoph Hellwig]

On Thu, Jun 15, 2023 at 11:53:49PM -0700, Song Liu wrote:
> I fixed this one, and applied the set to md-next.

Thanks!

[PATCH 07/11] md-bitmap: cleanup read_sb_page

[Christoph Hellwig]

Convert read_sb_page to the normal kernel coding style, calculate the
target sector only once, and add a local iosize variable to make the call
to sync_page_io more readable.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/md/md-bitmap.c | 23 +++++++++++------------
 1 file changed, 11 insertions(+), 12 deletions(-)

diff --git a/drivers/md/md-bitmap.c b/drivers/md/md-bitmap.c
index 1f71683b417981..f4bff2dfe2fd8f 100644
--- a/drivers/md/md-bitmap.c
+++ b/drivers/md/md-bitmap.c
@@ -139,26 +139,25 @@ static void md_bitmap_checkfree(struct bitmap_counts *bitmap, unsigned long page
  */
 
 /* IO operations when bitmap is stored near all superblocks */
+
+/* choose a good rdev and read the page from there */
 static int read_sb_page(struct mddev *mddev, loff_t offset,
-			struct page *page,
-			unsigned long index, int size)
+		struct page *page, unsigned long index, int size)
 {
-	/* choose a good rdev and read the page from there */
 
+	sector_t sector = offset + index * (PAGE_SIZE / SECTOR_SIZE);
 	struct md_rdev *rdev;
-	sector_t target;
 
 	rdev_for_each(rdev, mddev) {
-		if (! test_bit(In_sync, &rdev->flags)
-		    || test_bit(Faulty, &rdev->flags)
-		    || test_bit(Bitmap_sync, &rdev->flags))
-			continue;
+		u32 iosize = roundup(size, bdev_logical_block_size(rdev->bdev));
 
-		target = offset + index * (PAGE_SIZE/512);
+		if (!test_bit(In_sync, &rdev->flags) ||
+		    test_bit(Faulty, &rdev->flags) ||
+		    test_bit(Bitmap_sync, &rdev->flags))
+			continue;
 
-		if (sync_page_io(rdev, target,
-				 roundup(size, bdev_logical_block_size(rdev->bdev)),
-				 page, REQ_OP_READ, true)) {
+		if (sync_page_io(rdev, sector, iosize, page, REQ_OP_READ,
+				true)) {
 			page->index = index;
 			return 0;
 		}
-- 
2.39.2

Re: [PATCH 07/11] md-bitmap: cleanup read_sb_page

[Hannes Reinecke]

On 6/15/23 08:48, Christoph Hellwig wrote:
> Convert read_sb_page to the normal kernel coding style, calculate the
> target sector only once, and add a local iosize variable to make the call
> to sync_page_io more readable.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>   drivers/md/md-bitmap.c | 23 +++++++++++------------
>   1 file changed, 11 insertions(+), 12 deletions(-)
> 
Reviewed-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes

Re: [PATCH 07/11] md-bitmap: cleanup read_sb_page

[Johannes Thumshirn]

Looks good,
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>

Re: [PATCH 07/11] md-bitmap: cleanup read_sb_page

[Himanshu Madhani]

> On Jun 14, 2023, at 11:48 PM, Christoph Hellwig <hch@lst.de> wrote:
> 
> Convert read_sb_page to the normal kernel coding style, calculate the
> target sector only once, and add a local iosize variable to make the call
> to sync_page_io more readable.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
> drivers/md/md-bitmap.c | 23 +++++++++++------------
> 1 file changed, 11 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/md/md-bitmap.c b/drivers/md/md-bitmap.c
> index 1f71683b417981..f4bff2dfe2fd8f 100644
> --- a/drivers/md/md-bitmap.c
> +++ b/drivers/md/md-bitmap.c
> @@ -139,26 +139,25 @@ static void md_bitmap_checkfree(struct bitmap_counts *bitmap, unsigned long page
>  */
> 
> /* IO operations when bitmap is stored near all superblocks */
> +
> +/* choose a good rdev and read the page from there */
> static int read_sb_page(struct mddev *mddev, loff_t offset,
> - struct page *page,
> - unsigned long index, int size)
> + struct page *page, unsigned long index, int size)
> {
> - /* choose a good rdev and read the page from there */
> 
> + sector_t sector = offset + index * (PAGE_SIZE / SECTOR_SIZE);
> struct md_rdev *rdev;
> - sector_t target;
> 
> rdev_for_each(rdev, mddev) {
> - if (! test_bit(In_sync, &rdev->flags)
> -    || test_bit(Faulty, &rdev->flags)
> -    || test_bit(Bitmap_sync, &rdev->flags))
> - continue;
> + u32 iosize = roundup(size, bdev_logical_block_size(rdev->bdev));
> 
> - target = offset + index * (PAGE_SIZE/512);
> + if (!test_bit(In_sync, &rdev->flags) ||
> +    test_bit(Faulty, &rdev->flags) ||
> +    test_bit(Bitmap_sync, &rdev->flags))
> + continue;
> 
> - if (sync_page_io(rdev, target,
> - roundup(size, bdev_logical_block_size(rdev->bdev)),
> - page, REQ_OP_READ, true)) {
> + if (sync_page_io(rdev, sector, iosize, page, REQ_OP_READ,
> + true)) {
> page->index = index;
> return 0;
> }
> -- 
> 2.39.2
> 

Reviewed-by: Himanshu Madhani <himanshu.madhani@oracle.com>

-- 
Himanshu Madhani Oracle Linux Engineering

[PATCH 08/11] md-bitmap: account for mddev->bitmap_info.offset in read_sb_page

[Christoph Hellwig]

Diretly apply mddev->bitmap_info.offset to the sector number to read
instead of doing that in both callers.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/md/md-bitmap.c | 17 ++++++++---------
 1 file changed, 8 insertions(+), 9 deletions(-)

diff --git a/drivers/md/md-bitmap.c b/drivers/md/md-bitmap.c
index f4bff2dfe2fd8f..39ff75cc7a16ac 100644
--- a/drivers/md/md-bitmap.c
+++ b/drivers/md/md-bitmap.c
@@ -145,7 +145,8 @@ static int read_sb_page(struct mddev *mddev, loff_t offset,
 		struct page *page, unsigned long index, int size)
 {
 
-	sector_t sector = offset + index * (PAGE_SIZE / SECTOR_SIZE);
+	sector_t sector = mddev->bitmap_info.offset + offset +
+		index * (PAGE_SIZE / SECTOR_SIZE);
 	struct md_rdev *rdev;
 
 	rdev_for_each(rdev, mddev) {
@@ -593,7 +594,7 @@ static int md_bitmap_read_sb(struct bitmap *bitmap)
 	unsigned long sectors_reserved = 0;
 	int err = -EINVAL;
 	struct page *sb_page;
-	loff_t offset = bitmap->mddev->bitmap_info.offset;
+	loff_t offset = 0;
 
 	if (!bitmap->storage.file && !bitmap->mddev->bitmap_info.offset) {
 		chunksize = 128 * 1024 * 1024;
@@ -620,7 +621,7 @@ static int md_bitmap_read_sb(struct bitmap *bitmap)
 		bm_blocks = ((bm_blocks+7) >> 3) + sizeof(bitmap_super_t);
 		/* to 4k blocks */
 		bm_blocks = DIV_ROUND_UP_SECTOR_T(bm_blocks, 4096);
-		offset = bitmap->mddev->bitmap_info.offset + (bitmap->cluster_slot * (bm_blocks << 3));
+		offset = bitmap->cluster_slot * (bm_blocks << 3);
 		pr_debug("%s:%d bm slot: %d offset: %llu\n", __func__, __LINE__,
 			bitmap->cluster_slot, offset);
 	}
@@ -632,10 +633,8 @@ static int md_bitmap_read_sb(struct bitmap *bitmap)
 		err = read_file_page(bitmap->storage.file, 0,
 				bitmap, bytes, sb_page);
 	} else {
-		err = read_sb_page(bitmap->mddev,
-				   offset,
-				   sb_page,
-				   0, sizeof(bitmap_super_t));
+		err = read_sb_page(bitmap->mddev, offset, sb_page, 0,
+				   sizeof(bitmap_super_t));
 	}
 	if (err)
 		return err;
@@ -1127,8 +1126,8 @@ static int md_bitmap_init_from_disk(struct bitmap *bitmap, sector_t start)
 		if (file)
 			ret = read_file_page(file, i, bitmap, count, page);
 		else
-			ret = read_sb_page(mddev, mddev->bitmap_info.offset,
-					   page, i + node_offset, count);
+			ret = read_sb_page(mddev, 0, page, i + node_offset,
+					   count);
 		if (ret)
 			goto err;
 	}
-- 
2.39.2

Re: [PATCH 08/11] md-bitmap: account for mddev->bitmap_info.offset in read_sb_page

[Hannes Reinecke]

On 6/15/23 08:48, Christoph Hellwig wrote:
> Diretly apply mddev->bitmap_info.offset to the sector number to read
> instead of doing that in both callers.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>   drivers/md/md-bitmap.c | 17 ++++++++---------
>   1 file changed, 8 insertions(+), 9 deletions(-)
> 
Reviewed-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes

Re: [PATCH 08/11] md-bitmap: account for mddev->bitmap_info.offset in read_sb_page

[Himanshu Madhani]

> On Jun 14, 2023, at 11:48 PM, Christoph Hellwig <hch@lst.de> wrote:
> 
> Diretly apply mddev->bitmap_info.offset to the sector number to read
> instead of doing that in both callers.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
> drivers/md/md-bitmap.c | 17 ++++++++---------
> 1 file changed, 8 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/md/md-bitmap.c b/drivers/md/md-bitmap.c
> index f4bff2dfe2fd8f..39ff75cc7a16ac 100644
> --- a/drivers/md/md-bitmap.c
> +++ b/drivers/md/md-bitmap.c
> @@ -145,7 +145,8 @@ static int read_sb_page(struct mddev *mddev, loff_t offset,
> struct page *page, unsigned long index, int size)
> {
> 
> - sector_t sector = offset + index * (PAGE_SIZE / SECTOR_SIZE);
> + sector_t sector = mddev->bitmap_info.offset + offset +
> + index * (PAGE_SIZE / SECTOR_SIZE);
> struct md_rdev *rdev;
> 
> rdev_for_each(rdev, mddev) {
> @@ -593,7 +594,7 @@ static int md_bitmap_read_sb(struct bitmap *bitmap)
> unsigned long sectors_reserved = 0;
> int err = -EINVAL;
> struct page *sb_page;
> - loff_t offset = bitmap->mddev->bitmap_info.offset;
> + loff_t offset = 0;
> 
> if (!bitmap->storage.file && !bitmap->mddev->bitmap_info.offset) {
> chunksize = 128 * 1024 * 1024;
> @@ -620,7 +621,7 @@ static int md_bitmap_read_sb(struct bitmap *bitmap)
> bm_blocks = ((bm_blocks+7) >> 3) + sizeof(bitmap_super_t);
> /* to 4k blocks */
> bm_blocks = DIV_ROUND_UP_SECTOR_T(bm_blocks, 4096);
> - offset = bitmap->mddev->bitmap_info.offset + (bitmap->cluster_slot * (bm_blocks << 3));
> + offset = bitmap->cluster_slot * (bm_blocks << 3);
> pr_debug("%s:%d bm slot: %d offset: %llu\n", __func__, __LINE__,
> bitmap->cluster_slot, offset);
> }
> @@ -632,10 +633,8 @@ static int md_bitmap_read_sb(struct bitmap *bitmap)
> err = read_file_page(bitmap->storage.file, 0,
> bitmap, bytes, sb_page);
> } else {
> - err = read_sb_page(bitmap->mddev,
> -   offset,
> -   sb_page,
> -   0, sizeof(bitmap_super_t));
> + err = read_sb_page(bitmap->mddev, offset, sb_page, 0,
> +   sizeof(bitmap_super_t));
> }
> if (err)
> return err;
> @@ -1127,8 +1126,8 @@ static int md_bitmap_init_from_disk(struct bitmap *bitmap, sector_t start)
> if (file)
> ret = read_file_page(file, i, bitmap, count, page);
> else
> - ret = read_sb_page(mddev, mddev->bitmap_info.offset,
> -   page, i + node_offset, count);
> + ret = read_sb_page(mddev, 0, page, i + node_offset,
> +   count);
> if (ret)
> goto err;
> }
> -- 
> 2.39.2
> 

Reviewed-by: Himanshu Madhani <himanshu.madhani@oracle.com>

-- 
Himanshu Madhani Oracle Linux Engineering

[PATCH 09/11] md-bitmap: don't use ->index for pages backing the bitmap file

[Christoph Hellwig]

The md driver allocates pages for storing the bitmap file data, which
are not page cache pages, and then stores the page granularity file
offset in page->index, which is a field that isn't really valid except
for page cache pages.

Use a separate index for the superblock, and use the scheme used at
read size to recalculate the index for the bitmap pages instead.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/md/md-bitmap.c | 65 ++++++++++++++++++++++++------------------
 drivers/md/md-bitmap.h |  1 +
 2 files changed, 39 insertions(+), 27 deletions(-)

diff --git a/drivers/md/md-bitmap.c b/drivers/md/md-bitmap.c
index 39ff75cc7a16ac..ed402f4dad182d 100644
--- a/drivers/md/md-bitmap.c
+++ b/drivers/md/md-bitmap.c
@@ -157,11 +157,8 @@ static int read_sb_page(struct mddev *mddev, loff_t offset,
 		    test_bit(Bitmap_sync, &rdev->flags))
 			continue;
 
-		if (sync_page_io(rdev, sector, iosize, page, REQ_OP_READ,
-				true)) {
-			page->index = index;
+		if (sync_page_io(rdev, sector, iosize, page, REQ_OP_READ, true))
 			return 0;
-		}
 	}
 	return -EIO;
 }
@@ -225,18 +222,19 @@ static unsigned int bitmap_io_size(unsigned int io_size, unsigned int opt_size,
 }
 
 static int __write_sb_page(struct md_rdev *rdev, struct bitmap *bitmap,
-			   struct page *page)
+			   unsigned long pg_index, struct page *page)
 {
 	struct block_device *bdev;
 	struct mddev *mddev = bitmap->mddev;
 	struct bitmap_storage *store = &bitmap->storage;
 	loff_t sboff, offset = mddev->bitmap_info.offset;
-	sector_t ps, doff;
+	sector_t ps = pg_index * PAGE_SIZE / SECTOR_SIZE;
 	unsigned int size = PAGE_SIZE;
 	unsigned int opt_size = PAGE_SIZE;
+	sector_t doff;
 
 	bdev = (rdev->meta_bdev) ? rdev->meta_bdev : rdev->bdev;
-	if (page->index == store->file_pages - 1) {
+	if (pg_index == store->file_pages - 1) {
 		unsigned int last_page_size = store->bytes & (PAGE_SIZE - 1);
 
 		if (last_page_size == 0)
@@ -245,7 +243,6 @@ static int __write_sb_page(struct md_rdev *rdev, struct bitmap *bitmap,
 		opt_size = optimal_io_size(bdev, last_page_size, size);
 	}
 
-	ps = page->index * PAGE_SIZE / SECTOR_SIZE;
 	sboff = rdev->sb_start + offset;
 	doff = rdev->data_offset;
 
@@ -279,7 +276,8 @@ static int __write_sb_page(struct md_rdev *rdev, struct bitmap *bitmap,
 	return 0;
 }
 
-static void write_sb_page(struct bitmap *bitmap, struct page *page, int wait)
+static void write_sb_page(struct bitmap *bitmap, unsigned long pg_index,
+			  struct page *page, bool wait)
 {
 	struct mddev *mddev = bitmap->mddev;
 
@@ -287,7 +285,7 @@ static void write_sb_page(struct bitmap *bitmap, struct page *page, int wait)
 		struct md_rdev *rdev = NULL;
 
 		while ((rdev = next_active_rdev(rdev, mddev)) != NULL) {
-			if (__write_sb_page(rdev, bitmap, page) < 0) {
+			if (__write_sb_page(rdev, bitmap, pg_index, page) < 0) {
 				set_bit(BITMAP_WRITE_ERROR, &bitmap->flags);
 				return;
 			}
@@ -397,7 +395,6 @@ static int read_file_page(struct file *file, unsigned long index,
 		blk_cur++;
 		bh = bh->b_this_page;
 	}
-	page->index = index;
 
 	wait_event(bitmap->write_wait,
 		   atomic_read(&bitmap->pending_writes)==0);
@@ -419,12 +416,21 @@ static int read_file_page(struct file *file, unsigned long index,
 /*
  * write out a page to a file
  */
-static void write_page(struct bitmap *bitmap, struct page *page, int wait)
+static void filemap_write_page(struct bitmap *bitmap, unsigned long pg_index,
+			       bool wait)
 {
-	if (bitmap->storage.file)
+	struct bitmap_storage *store = &bitmap->storage;
+	struct page *page = store->filemap[pg_index];
+
+	if (mddev_is_clustered(bitmap->mddev)) {
+		pg_index += bitmap->cluster_slot *
+			DIV_ROUND_UP(store->bytes, PAGE_SIZE);
+	}
+
+	if (store->file)
 		write_file_page(bitmap, page, wait);
 	else
-		write_sb_page(bitmap, page, wait);
+		write_sb_page(bitmap, pg_index, page, wait);
 }
 
 /*
@@ -481,7 +487,12 @@ void md_bitmap_update_sb(struct bitmap *bitmap)
 	sb->sectors_reserved = cpu_to_le32(bitmap->mddev->
 					   bitmap_info.space);
 	kunmap_atomic(sb);
-	write_page(bitmap, bitmap->storage.sb_page, 1);
+
+	if (bitmap->storage.file)
+		write_file_page(bitmap, bitmap->storage.sb_page, 1);
+	else
+		write_sb_page(bitmap, bitmap->storage.sb_index,
+			      bitmap->storage.sb_page, 1);
 }
 EXPORT_SYMBOL(md_bitmap_update_sb);
 
@@ -533,7 +544,7 @@ static int md_bitmap_new_disk_sb(struct bitmap *bitmap)
 	bitmap->storage.sb_page = alloc_page(GFP_KERNEL | __GFP_ZERO);
 	if (bitmap->storage.sb_page == NULL)
 		return -ENOMEM;
-	bitmap->storage.sb_page->index = 0;
+	bitmap->storage.sb_index = 0;
 
 	sb = kmap_atomic(bitmap->storage.sb_page);
 
@@ -810,7 +821,7 @@ static int md_bitmap_storage_alloc(struct bitmap_storage *store,
 	if (store->sb_page) {
 		store->filemap[0] = store->sb_page;
 		pnum = 1;
-		store->sb_page->index = offset;
+		store->sb_index = offset;
 	}
 
 	for ( ; pnum < num_pages; pnum++) {
@@ -819,7 +830,6 @@ static int md_bitmap_storage_alloc(struct bitmap_storage *store,
 			store->file_pages = pnum;
 			return -ENOMEM;
 		}
-		store->filemap[pnum]->index = pnum + offset;
 	}
 	store->file_pages = pnum;
 
@@ -924,6 +934,7 @@ static void md_bitmap_file_set_bit(struct bitmap *bitmap, sector_t block)
 	void *kaddr;
 	unsigned long chunk = block >> bitmap->counts.chunkshift;
 	struct bitmap_storage *store = &bitmap->storage;
+	unsigned long index = file_page_index(store, chunk);
 	unsigned long node_offset = 0;
 
 	if (mddev_is_clustered(bitmap->mddev))
@@ -941,9 +952,9 @@ static void md_bitmap_file_set_bit(struct bitmap *bitmap, sector_t block)
 	else
 		set_bit_le(bit, kaddr);
 	kunmap_atomic(kaddr);
-	pr_debug("set file bit %lu page %lu\n", bit, page->index);
+	pr_debug("set file bit %lu page %lu\n", bit, index);
 	/* record page number so it gets flushed to disk when unplug occurs */
-	set_page_attr(bitmap, page->index - node_offset, BITMAP_PAGE_DIRTY);
+	set_page_attr(bitmap, index - node_offset, BITMAP_PAGE_DIRTY);
 }
 
 static void md_bitmap_file_clear_bit(struct bitmap *bitmap, sector_t block)
@@ -953,6 +964,7 @@ static void md_bitmap_file_clear_bit(struct bitmap *bitmap, sector_t block)
 	void *paddr;
 	unsigned long chunk = block >> bitmap->counts.chunkshift;
 	struct bitmap_storage *store = &bitmap->storage;
+	unsigned long index = file_page_index(store, chunk);
 	unsigned long node_offset = 0;
 
 	if (mddev_is_clustered(bitmap->mddev))
@@ -968,8 +980,8 @@ static void md_bitmap_file_clear_bit(struct bitmap *bitmap, sector_t block)
 	else
 		clear_bit_le(bit, paddr);
 	kunmap_atomic(paddr);
-	if (!test_page_attr(bitmap, page->index - node_offset, BITMAP_PAGE_NEEDWRITE)) {
-		set_page_attr(bitmap, page->index - node_offset, BITMAP_PAGE_PENDING);
+	if (!test_page_attr(bitmap, index - node_offset, BITMAP_PAGE_NEEDWRITE)) {
+		set_page_attr(bitmap, index - node_offset, BITMAP_PAGE_PENDING);
 		bitmap->allclean = 0;
 	}
 }
@@ -1021,7 +1033,7 @@ void md_bitmap_unplug(struct bitmap *bitmap)
 							  "md bitmap_unplug");
 			}
 			clear_page_attr(bitmap, i, BITMAP_PAGE_PENDING);
-			write_page(bitmap, bitmap->storage.filemap[i], 0);
+			filemap_write_page(bitmap, i, false);
 			writing = 1;
 		}
 	}
@@ -1152,7 +1164,7 @@ static int md_bitmap_init_from_disk(struct bitmap *bitmap, sector_t start)
 			memset(paddr + offset, 0xff, PAGE_SIZE - offset);
 			kunmap_atomic(paddr);
 
-			write_page(bitmap, page, 1);
+			filemap_write_page(bitmap, i, true);
 			if (test_bit(BITMAP_WRITE_ERROR, &bitmap->flags)) {
 				ret = -EIO;
 				goto err;
@@ -1373,9 +1385,8 @@ void md_bitmap_daemon_work(struct mddev *mddev)
 			break;
 		if (bitmap->storage.filemap &&
 		    test_and_clear_page_attr(bitmap, j,
-					     BITMAP_PAGE_NEEDWRITE)) {
-			write_page(bitmap, bitmap->storage.filemap[j], 0);
-		}
+					     BITMAP_PAGE_NEEDWRITE))
+			filemap_write_page(bitmap, j, false);
 	}
 
  done:
diff --git a/drivers/md/md-bitmap.h b/drivers/md/md-bitmap.h
index 8a3788c9bfef85..bb9eb418780a62 100644
--- a/drivers/md/md-bitmap.h
+++ b/drivers/md/md-bitmap.h
@@ -201,6 +201,7 @@ struct bitmap {
 		struct file *file;		/* backing disk file */
 		struct page *sb_page;		/* cached copy of the bitmap
 						 * file superblock */
+		unsigned long sb_index;
 		struct page **filemap;		/* list of cache pages for
 						 * the file */
 		unsigned long *filemap_attr;	/* attributes associated
-- 
2.39.2

Re: [PATCH 09/11] md-bitmap: don't use ->index for pages backing the bitmap file

[Hannes Reinecke]

On 6/15/23 08:48, Christoph Hellwig wrote:
> The md driver allocates pages for storing the bitmap file data, which
> are not page cache pages, and then stores the page granularity file
> offset in page->index, which is a field that isn't really valid except
> for page cache pages.
> 
Hmm. Willy told me that ->index is free to use if you're not dealing 
with page cache pages (brd does the same thing).

Cheers,

Hannes

[PATCH 10/11] md: make bitmap file support optional

[Christoph Hellwig]

The support for write intent bitmaps in files on an external files in md
is a hot mess that abuses ->bmap to map file offsets into physical device
objects, and also abuses buffer_heads in a creative way.

Make this code optional so that MD can be built into future kernels
without buffer_head support, and so that we can eventually deprecate it.

Note this does not affect the internal bitmap support, which has none of
the problems.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/md/Kconfig     | 10 ++++++++++
 drivers/md/md-bitmap.c | 15 +++++++++++++++
 drivers/md/md.c        |  7 +++++++
 3 files changed, 32 insertions(+)

diff --git a/drivers/md/Kconfig b/drivers/md/Kconfig
index b0a22e99bade37..9712ab9bcba52e 100644
--- a/drivers/md/Kconfig
+++ b/drivers/md/Kconfig
@@ -50,6 +50,16 @@ config MD_AUTODETECT
 
 	  If unsure, say Y.
 
+config MD_BITMAP_FILE
+	bool "MD bitmap file support"
+	default y
+	help
+	  If you say Y here, support for write intent bitmaps in files on an
+	  external file system is enabled.  This is an alternative to the internal
+	  bitmaps near the MD superblock, and very problematic code that abuses
+	  various kernel APIs and can only work with files on a file system not
+	  actually sitting on the MD device.
+
 config MD_LINEAR
 	tristate "Linear (append) mode (deprecated)"
 	depends on BLK_DEV_MD
diff --git a/drivers/md/md-bitmap.c b/drivers/md/md-bitmap.c
index ed402f4dad182d..1e29088b1f081a 100644
--- a/drivers/md/md-bitmap.c
+++ b/drivers/md/md-bitmap.c
@@ -295,6 +295,7 @@ static void write_sb_page(struct bitmap *bitmap, unsigned long pg_index,
 
 static void md_bitmap_file_kick(struct bitmap *bitmap);
 
+#ifdef CONFIG_MD_BITMAP_FILE
 static void write_file_page(struct bitmap *bitmap, struct page *page, int wait)
 {
 	struct buffer_head *bh = page_buffers(page);
@@ -408,6 +409,20 @@ static int read_file_page(struct file *file, unsigned long index,
 		       ret);
 	return ret;
 }
+#else /* CONFIG_MD_BITMAP_FILE */
+static void write_file_page(struct bitmap *bitmap, struct page *page, int wait)
+{
+}
+static int read_file_page(struct file *file, unsigned long index,
+		struct bitmap *bitmap, unsigned long count, struct page *page)
+{
+	return -EIO;
+}
+static void free_buffers(struct page *page)
+{
+	put_page(page);
+}
+#endif /* CONFIG_MD_BITMAP_FILE */
 
 /*
  * bitmap file superblock operations
diff --git a/drivers/md/md.c b/drivers/md/md.c
index cf3733c90c47ed..c9fcefaf9c073b 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -7020,6 +7020,13 @@ static int set_bitmap_file(struct mddev *mddev, int fd)
 
 		if (mddev->bitmap || mddev->bitmap_info.file)
 			return -EEXIST; /* cannot add when bitmap is present */
+
+		if (!IS_ENABLED(CONFIG_MD_BITMAP_FILE)) {
+			pr_warn("%s: bitmap files not supported by this kernel\n",
+				mdname(mddev));
+			return -EINVAL;
+		}
+
 		f = fget(fd);
 
 		if (f == NULL) {
-- 
2.39.2

Re: [PATCH 10/11] md: make bitmap file support optional

[Hannes Reinecke]

On 6/15/23 08:48, Christoph Hellwig wrote:
> The support for write intent bitmaps in files on an external files in md
> is a hot mess that abuses ->bmap to map file offsets into physical device
> objects, and also abuses buffer_heads in a creative way.
> 
> Make this code optional so that MD can be built into future kernels
> without buffer_head support, and so that we can eventually deprecate it.
> 
> Note this does not affect the internal bitmap support, which has none of
> the problems.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>   drivers/md/Kconfig     | 10 ++++++++++
>   drivers/md/md-bitmap.c | 15 +++++++++++++++
>   drivers/md/md.c        |  7 +++++++
>   3 files changed, 32 insertions(+)
> 
Reviewed-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes

Re: [PATCH 10/11] md: make bitmap file support optional

[Himanshu Madhani]

> On Jun 14, 2023, at 11:48 PM, Christoph Hellwig <hch@lst.de> wrote:
> 
> The support for write intent bitmaps in files on an external files in md
> is a hot mess that abuses ->bmap to map file offsets into physical device
> objects, and also abuses buffer_heads in a creative way.
> 
> Make this code optional so that MD can be built into future kernels
> without buffer_head support, and so that we can eventually deprecate it.
> 
> Note this does not affect the internal bitmap support, which has none of
> the problems.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
> drivers/md/Kconfig     | 10 ++++++++++
> drivers/md/md-bitmap.c | 15 +++++++++++++++
> drivers/md/md.c        |  7 +++++++
> 3 files changed, 32 insertions(+)
> 
> diff --git a/drivers/md/Kconfig b/drivers/md/Kconfig
> index b0a22e99bade37..9712ab9bcba52e 100644
> --- a/drivers/md/Kconfig
> +++ b/drivers/md/Kconfig
> @@ -50,6 +50,16 @@ config MD_AUTODETECT
> 
>  If unsure, say Y.
> 
> +config MD_BITMAP_FILE
> + bool "MD bitmap file support"
> + default y
> + help
> +  If you say Y here, support for write intent bitmaps in files on an
> +  external file system is enabled.  This is an alternative to the internal
> +  bitmaps near the MD superblock, and very problematic code that abuses
> +  various kernel APIs and can only work with files on a file system not
> +  actually sitting on the MD device.
> +
> config MD_LINEAR
> tristate "Linear (append) mode (deprecated)"
> depends on BLK_DEV_MD
> diff --git a/drivers/md/md-bitmap.c b/drivers/md/md-bitmap.c
> index ed402f4dad182d..1e29088b1f081a 100644
> --- a/drivers/md/md-bitmap.c
> +++ b/drivers/md/md-bitmap.c
> @@ -295,6 +295,7 @@ static void write_sb_page(struct bitmap *bitmap, unsigned long pg_index,
> 
> static void md_bitmap_file_kick(struct bitmap *bitmap);
> 
> +#ifdef CONFIG_MD_BITMAP_FILE
> static void write_file_page(struct bitmap *bitmap, struct page *page, int wait)
> {
> struct buffer_head *bh = page_buffers(page);
> @@ -408,6 +409,20 @@ static int read_file_page(struct file *file, unsigned long index,
>       ret);
> return ret;
> }
> +#else /* CONFIG_MD_BITMAP_FILE */
> +static void write_file_page(struct bitmap *bitmap, struct page *page, int wait)
> +{
> +}
> +static int read_file_page(struct file *file, unsigned long index,
> + struct bitmap *bitmap, unsigned long count, struct page *page)
> +{
> + return -EIO;
> +}
> +static void free_buffers(struct page *page)
> +{
> + put_page(page);
> +}
> +#endif /* CONFIG_MD_BITMAP_FILE */
> 
> /*
>  * bitmap file superblock operations
> diff --git a/drivers/md/md.c b/drivers/md/md.c
> index cf3733c90c47ed..c9fcefaf9c073b 100644
> --- a/drivers/md/md.c
> +++ b/drivers/md/md.c
> @@ -7020,6 +7020,13 @@ static int set_bitmap_file(struct mddev *mddev, int fd)
> 
> if (mddev->bitmap || mddev->bitmap_info.file)
> return -EEXIST; /* cannot add when bitmap is present */
> +
> + if (!IS_ENABLED(CONFIG_MD_BITMAP_FILE)) {
> + pr_warn("%s: bitmap files not supported by this kernel\n",
> + mdname(mddev));
> + return -EINVAL;
> + }
> +
> f = fget(fd);
> 
> if (f == NULL) {
> -- 
> 2.39.2
> 

Reviewed-by: Himanshu Madhani <himanshu.madhani@oracle.com>

-- 
Himanshu Madhani Oracle Linux Engineering

[PATCH 11/11] md: deprecate bitmap file support

[Christoph Hellwig]

The support for bitmaps on files is a very bad idea abusing various kernel
APIs, and fundamentally requires the file to not be on the actual array
without a way to check that this is actually the case.  Add a deprecation
warning to see if we might be able to eventually drop it.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/md/Kconfig | 2 +-
 drivers/md/md.c    | 2 ++
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/md/Kconfig b/drivers/md/Kconfig
index 9712ab9bcba52e..444517d1a2336a 100644
--- a/drivers/md/Kconfig
+++ b/drivers/md/Kconfig
@@ -51,7 +51,7 @@ config MD_AUTODETECT
 	  If unsure, say Y.
 
 config MD_BITMAP_FILE
-	bool "MD bitmap file support"
+	bool "MD bitmap file support (deprecated)"
 	default y
 	help
 	  If you say Y here, support for write intent bitmaps in files on an
diff --git a/drivers/md/md.c b/drivers/md/md.c
index c9fcefaf9c073b..d04a91295edf9d 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -7026,6 +7026,8 @@ static int set_bitmap_file(struct mddev *mddev, int fd)
 				mdname(mddev));
 			return -EINVAL;
 		}
+		pr_warn("%s: using deprecated bitmap file support\n",
+			mdname(mddev));
 
 		f = fget(fd);
 
-- 
2.39.2

Re: [PATCH 11/11] md: deprecate bitmap file support

[Hannes Reinecke]

On 6/15/23 08:48, Christoph Hellwig wrote:
> The support for bitmaps on files is a very bad idea abusing various kernel
> APIs, and fundamentally requires the file to not be on the actual array
> without a way to check that this is actually the case.  Add a deprecation
> warning to see if we might be able to eventually drop it.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>   drivers/md/Kconfig | 2 +-
>   drivers/md/md.c    | 2 ++
>   2 files changed, 3 insertions(+), 1 deletion(-)
> 
Reviewed-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes

Re: [PATCH 11/11] md: deprecate bitmap file support

[Himanshu Madhani]

> On Jun 14, 2023, at 11:48 PM, Christoph Hellwig <hch@lst.de> wrote:
> 
> The support for bitmaps on files is a very bad idea abusing various kernel
> APIs, and fundamentally requires the file to not be on the actual array
> without a way to check that this is actually the case.  Add a deprecation
> warning to see if we might be able to eventually drop it.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
> drivers/md/Kconfig | 2 +-
> drivers/md/md.c    | 2 ++
> 2 files changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/md/Kconfig b/drivers/md/Kconfig
> index 9712ab9bcba52e..444517d1a2336a 100644
> --- a/drivers/md/Kconfig
> +++ b/drivers/md/Kconfig
> @@ -51,7 +51,7 @@ config MD_AUTODETECT
>  If unsure, say Y.
> 
> config MD_BITMAP_FILE
> - bool "MD bitmap file support"
> + bool "MD bitmap file support (deprecated)"
> default y
> help
>  If you say Y here, support for write intent bitmaps in files on an
> diff --git a/drivers/md/md.c b/drivers/md/md.c
> index c9fcefaf9c073b..d04a91295edf9d 100644
> --- a/drivers/md/md.c
> +++ b/drivers/md/md.c
> @@ -7026,6 +7026,8 @@ static int set_bitmap_file(struct mddev *mddev, int fd)
> mdname(mddev));
> return -EINVAL;
> }
> + pr_warn("%s: using deprecated bitmap file support\n",
> + mdname(mddev));
> 
> f = fget(fd);
> 
> -- 
> 2.39.2
> 

Reviewed-by: Himanshu Madhani <himanshu.madhani@oracle.com>

-- 
Himanshu Madhani Oracle Linux Engineering

Re: [PATCH 11/11] md: deprecate bitmap file support

[Himanshu Madhani]

> On Jun 14, 2023, at 11:48 PM, Christoph Hellwig <hch@lst.de> wrote:
> 
> The support for bitmaps on files is a very bad idea abusing various kernel
> APIs, and fundamentally requires the file to not be on the actual array
> without a way to check that this is actually the case.  Add a deprecation
> warning to see if we might be able to eventually drop it.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
> drivers/md/Kconfig | 2 +-
> drivers/md/md.c    | 2 ++
> 2 files changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/md/Kconfig b/drivers/md/Kconfig
> index 9712ab9bcba52e..444517d1a2336a 100644
> --- a/drivers/md/Kconfig
> +++ b/drivers/md/Kconfig
> @@ -51,7 +51,7 @@ config MD_AUTODETECT
>  If unsure, say Y.
> 
> config MD_BITMAP_FILE
> - bool "MD bitmap file support"
> + bool "MD bitmap file support (deprecated)"
> default y
> help
>  If you say Y here, support for write intent bitmaps in files on an
> diff --git a/drivers/md/md.c b/drivers/md/md.c
> index c9fcefaf9c073b..d04a91295edf9d 100644
> --- a/drivers/md/md.c
> +++ b/drivers/md/md.c
> @@ -7026,6 +7026,8 @@ static int set_bitmap_file(struct mddev *mddev, int fd)
> mdname(mddev));
> return -EINVAL;
> }
> + pr_warn("%s: using deprecated bitmap file support\n",
> + mdname(mddev));
> 
> f = fget(fd);
> 
> -- 
> 2.39.2
> 

Reviewed-by: Himanshu Madhani <himanshu.madhani@oracle.com>

-- 
Himanshu Madhani Oracle Linux Engineering

Re: deprecate md bitmap file support

[Mariusz Tkaczyk]

On Thu, 15 Jun 2023 08:48:29 +0200
Christoph Hellwig <hch@lst.de> wrote:

> Hi Song,
> 
> the md bitmap file support is very problematic in how it bypasses the
> file system for file access.  I looked into fixing it in preparation
> for making buffer_head optionals but had to give up because it is so
> convoluted.  This series includes the cleanups I've started which seem
> useful even for the internal bitmap support, then makes the bitmap file
> support conditional and adds a deprecation warning.
> 
Hi Christoph,
I think that it is worthy to make mdadm aware of that. For example, by requiring
"--force" to make the volume with bitmap file now.

Thanks,
Mariusz

Re: deprecate md bitmap file support

[Christoph Hellwig]

On Thu, Jun 15, 2023 at 10:05:57AM +0200, Mariusz Tkaczyk wrote:
> Hi Christoph,
> I think that it is worthy to make mdadm aware of that. For example, by requiring
> "--force" to make the volume with bitmap file now.

Sounds reasonable to me.  I'll look into it.