Linux-Raid Archives on lore.kernel.org
 help / color / Atom feed
* [PATCH 1/2] [md/bitmap] md_bitmap_read_sb use wrong bitmap blocks
@ 2020-10-03 16:11 Zhao Heming
  2020-10-03 16:11 ` [PATCH 2/2] [md/bitmap] md_bitmap_get_counter return wrong blocks in some cases Zhao Heming
  2020-10-03 16:26 ` [PATCH 1/2] [md/bitmap] md_bitmap_read_sb use wrong bitmap blocks heming.zhao
  0 siblings, 2 replies; 4+ messages in thread
From: Zhao Heming @ 2020-10-03 16:11 UTC (permalink / raw)
  To: linux-raid, song; +Cc: Zhao Heming

The patched code is used to get chunks number, should use
round-up div to replace current sector_div.
The same code is in md_bitmap_resize():
```
chunks = DIV_ROUND_UP_SECTOR_T(blocks, 1 << chunkshift);
```

Signed-off-by: Zhao Heming <heming.zhao@suse.com>
---
 drivers/md/md-bitmap.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/md/md-bitmap.c b/drivers/md/md-bitmap.c
index 593fe15..1efd2b4 100644
--- a/drivers/md/md-bitmap.c
+++ b/drivers/md/md-bitmap.c
@@ -605,7 +605,7 @@ static int md_bitmap_read_sb(struct bitmap *bitmap)
 	if (bitmap->cluster_slot >= 0) {
 		sector_t bm_blocks = bitmap->mddev->resync_max_sectors;
 
-		sector_div(bm_blocks,
+		DIV_ROUND_UP_SECTOR_T(bm_blocks,
 			   bitmap->mddev->bitmap_info.chunksize >> 9);
 		/* bits to bytes */
 		bm_blocks = ((bm_blocks+7) >> 3) + sizeof(bitmap_super_t);
-- 
1.8.3.1


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

* [PATCH 2/2] [md/bitmap] md_bitmap_get_counter return wrong blocks in some cases
  2020-10-03 16:11 [PATCH 1/2] [md/bitmap] md_bitmap_read_sb use wrong bitmap blocks Zhao Heming
@ 2020-10-03 16:11 ` Zhao Heming
  2020-10-03 16:26 ` [PATCH 1/2] [md/bitmap] md_bitmap_read_sb use wrong bitmap blocks heming.zhao
  1 sibling, 0 replies; 4+ messages in thread
From: Zhao Heming @ 2020-10-03 16:11 UTC (permalink / raw)
  To: linux-raid, song; +Cc: Zhao Heming

md_bitmap_get_counter() has code:

```
    if (bitmap->bp[page].hijacked ||
        bitmap->bp[page].map == NULL)
        csize = ((sector_t)1) << (bitmap->chunkshift +
                      PAGE_COUNTER_SHIFT - 1);
```

the minus 1 is wrong, this branch should report 2048 bits of space.
with "-1" action, this only report 1024 bit of space.

this bug code returns wrong blocks, but it doesn't inflence bitmap logic:
1. most callers focus this function return value (the counter of offset),
   not the parameter blocks.
2. the bug is only triggered when hijacked is true or map is NULL.
   the hijacked true condition is very rare.
   the "map == null" only true when array is creating or resizing.
3. even the caller gets wrong blocks, current code makes caller just to
   call md_bitmap_get_counter() one more time.

Signed-off-by: Zhao Heming <heming.zhao@suse.com>
---
 drivers/md/md-bitmap.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/md/md-bitmap.c b/drivers/md/md-bitmap.c
index 1efd2b4..145b3b2 100644
--- a/drivers/md/md-bitmap.c
+++ b/drivers/md/md-bitmap.c
@@ -1367,7 +1367,7 @@ static bitmap_counter_t *md_bitmap_get_counter(struct bitmap_counts *bitmap,
 	if (bitmap->bp[page].hijacked ||
 	    bitmap->bp[page].map == NULL)
 		csize = ((sector_t)1) << (bitmap->chunkshift +
-					  PAGE_COUNTER_SHIFT - 1);
+					  PAGE_COUNTER_SHIFT);
 	else
 		csize = ((sector_t)1) << bitmap->chunkshift;
 	*blocks = csize - (offset & (csize - 1));
-- 
1.8.3.1


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

* Re: [PATCH 1/2] [md/bitmap] md_bitmap_read_sb use wrong bitmap blocks
  2020-10-03 16:11 [PATCH 1/2] [md/bitmap] md_bitmap_read_sb use wrong bitmap blocks Zhao Heming
  2020-10-03 16:11 ` [PATCH 2/2] [md/bitmap] md_bitmap_get_counter return wrong blocks in some cases Zhao Heming
@ 2020-10-03 16:26 ` heming.zhao
  2020-10-05  7:44   ` Song Liu
  1 sibling, 1 reply; 4+ messages in thread
From: heming.zhao @ 2020-10-03 16:26 UTC (permalink / raw)
  To: linux-raid, song

very sorry for my mistake.

the patch should be change from:
```
-		sector_div(bm_blocks,
+		DIV_ROUND_UP_SECTOR_T(bm_blocks,
   			   bitmap->mddev->bitmap_info.chunksize >> 9);
```

to
```
-               sector_div(bm_blocks,
-                          bitmap->mddev->bitmap_info.chunksize >> 9);
+               bm_blocks = DIV_ROUND_UP_SECTOR_T(bm_blocks,
+                          (bitmap->mddev->bitmap_info.chunksize >> 9));
```

If my patch would be accepted, I will send v2 patch including above lines.


On 10/4/20 12:11 AM, Zhao Heming wrote:
> The patched code is used to get chunks number, should use
> round-up div to replace current sector_div.
> The same code is in md_bitmap_resize():
> ```
> chunks = DIV_ROUND_UP_SECTOR_T(blocks, 1 << chunkshift);
> ```
> 
> Signed-off-by: Zhao Heming <heming.zhao@suse.com>
> ---
>   drivers/md/md-bitmap.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/md/md-bitmap.c b/drivers/md/md-bitmap.c
> index 593fe15..1efd2b4 100644
> --- a/drivers/md/md-bitmap.c
> +++ b/drivers/md/md-bitmap.c
> @@ -605,7 +605,7 @@ static int md_bitmap_read_sb(struct bitmap *bitmap)
>   	if (bitmap->cluster_slot >= 0) {
>   		sector_t bm_blocks = bitmap->mddev->resync_max_sectors;
>   
> -		sector_div(bm_blocks,
> +		DIV_ROUND_UP_SECTOR_T(bm_blocks,
>   			   bitmap->mddev->bitmap_info.chunksize >> 9);
>   		/* bits to bytes */
>   		bm_blocks = ((bm_blocks+7) >> 3) + sizeof(bitmap_super_t);
> 


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

* Re: [PATCH 1/2] [md/bitmap] md_bitmap_read_sb use wrong bitmap blocks
  2020-10-03 16:26 ` [PATCH 1/2] [md/bitmap] md_bitmap_read_sb use wrong bitmap blocks heming.zhao
@ 2020-10-05  7:44   ` Song Liu
  0 siblings, 0 replies; 4+ messages in thread
From: Song Liu @ 2020-10-05  7:44 UTC (permalink / raw)
  To: heming.zhao; +Cc: linux-raid

On Sat, Oct 3, 2020 at 9:26 AM heming.zhao@suse.com
<heming.zhao@suse.com> wrote:
>
> very sorry for my mistake.
>
> the patch should be change from:
> ```
> -               sector_div(bm_blocks,
> +               DIV_ROUND_UP_SECTOR_T(bm_blocks,
>                            bitmap->mddev->bitmap_info.chunksize >> 9);
> ```
>
> to
> ```
> -               sector_div(bm_blocks,
> -                          bitmap->mddev->bitmap_info.chunksize >> 9);
> +               bm_blocks = DIV_ROUND_UP_SECTOR_T(bm_blocks,
> +                          (bitmap->mddev->bitmap_info.chunksize >> 9));
> ```
>
> If my patch would be accepted, I will send v2 patch including above lines.

Please send v2 of the patch. Please also CC Guoqing in v2.

Thanks,
Song

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

end of thread, back to index

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-03 16:11 [PATCH 1/2] [md/bitmap] md_bitmap_read_sb use wrong bitmap blocks Zhao Heming
2020-10-03 16:11 ` [PATCH 2/2] [md/bitmap] md_bitmap_get_counter return wrong blocks in some cases Zhao Heming
2020-10-03 16:26 ` [PATCH 1/2] [md/bitmap] md_bitmap_read_sb use wrong bitmap blocks heming.zhao
2020-10-05  7:44   ` Song Liu

Linux-Raid Archives on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-raid/0 linux-raid/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-raid linux-raid/ https://lore.kernel.org/linux-raid \
		linux-raid@vger.kernel.org
	public-inbox-index linux-raid

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-raid


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git