* [PATCH 0/2] Can't grow size twice for a super1.0 array @ 2020-06-30 7:55 Xiao Ni 2020-06-30 7:55 ` [PATCH 1/2] super1.0 calculates max sectors in a wrong way Xiao Ni 2020-06-30 7:55 ` [PATCH 2/2] Don't need to reset superblock start address for super1.0 Xiao Ni 0 siblings, 2 replies; 8+ messages in thread From: Xiao Ni @ 2020-06-30 7:55 UTC (permalink / raw) To: linux-raid; +Cc: song, ncroxon Hi all This are the test steps: [root@storageqe-54 ~]# mdadm -CR /dev/md0 -l1 -n2 /dev/loop0 /dev/loop1 -e 1.0 --size=1G [root@storageqe-54 ~]# mdadm --wait /dev/md0 [root@storageqe-54 ~]# mdadm -G /dev/md0 --size=5G mdadm: component size of /dev/md0 has been set to 5242880K [root@storageqe-54 ~]# mdadm --wait /dev/md0 [root@storageqe-54 ~]# mdadm -G /dev/md0 --size=6G mdadm: Cannot set device size for /dev/md0: No space left on device loop0 7:0 0 7.8G 0 loop └─md0 9:0 0 5G 0 raid1 loop1 7:1 0 7.8G 0 loop └─md0 9:0 0 5G 0 raid1 The reason is that it calcluates the max usable space in a wrong way for super1.0 array. It uses rdev->sectors to calcuate the max usable space rather than the whole disk size. At first it grow size and it's successful. Because rdev->sectors is set to the whole disk size when creating raid device. super_1_load sets rdev->sectors to sb->data_size. In mdadm it sets sb->data_size to the whole disk size rathan than the raid device size. I think it's a wrong place too. Because of this error, it can grow size successfully at the first time. It re-sets rdev->sectors to real raid device after reshaping to the new size. So it can't grow size anymore. Xiao Ni (2): super1.0 calculates max sectors in a wrong way Don't need to reset superblock start address for super1.0 drivers/md/md.c | 27 +++++++++++++++++++++++---- 1 file changed, 23 insertions(+), 4 deletions(-) -- 2.7.5 ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 1/2] super1.0 calculates max sectors in a wrong way 2020-06-30 7:55 [PATCH 0/2] Can't grow size twice for a super1.0 array Xiao Ni @ 2020-06-30 7:55 ` Xiao Ni 2020-07-27 17:33 ` Song Liu 2020-06-30 7:55 ` [PATCH 2/2] Don't need to reset superblock start address for super1.0 Xiao Ni 1 sibling, 1 reply; 8+ messages in thread From: Xiao Ni @ 2020-06-30 7:55 UTC (permalink / raw) To: linux-raid; +Cc: song, ncroxon One super1.0 raid array wants to grow size, it needs to check the device max usable size. If the requested size is bigger than max usable size, it will return an error. Now it uses rdev->sectors for max usable size. If one disk is 500G and the raid device only uses the 100GB of this disk. rdev->sectors can't tell the real max usable size. The max usable size should be dev_size-(superblock_size+bitmap_size+badblock_size). Signed-off-by: Xiao Ni <xni@redhat.com> --- drivers/md/md.c | 26 +++++++++++++++++++++++--- 1 file changed, 23 insertions(+), 3 deletions(-) diff --git a/drivers/md/md.c b/drivers/md/md.c index f567f53..d2c5984 100644 --- a/drivers/md/md.c +++ b/drivers/md/md.c @@ -2183,10 +2183,30 @@ super_1_rdev_size_change(struct md_rdev *rdev, sector_t num_sectors) return 0; } else { /* minor version 0; superblock after data */ - sector_t sb_start; - sb_start = (i_size_read(rdev->bdev->bd_inode) >> 9) - 8*2; + sector_t sb_start, bm_space; + sector_t dev_size = i_size_read(rdev->bdev->bd_inode) >> 9; + + /* 8K is for superblock */ + sb_start = dev_size - 8*2; sb_start &= ~(sector_t)(4*2 - 1); - max_sectors = rdev->sectors + sb_start - rdev->sb_start; + + /* if the device is bigger than 8Gig, save 64k for bitmap usage, + * if bigger than 200Gig, save 128k + */ + if (dev_size < 64*2) + bm_space = 0; + else if (dev_size - 64*2 >= 200*1024*1024*2) + bm_space = 128*2; + else if (dev_size - 4*2 > 8*1024*1024*2) + bm_space = 64*2; + else + bm_space = 4*2; + + /* Space that can be used to store date needs to decrease superblock + * bitmap space and bad block space(4K) + */ + max_sectors = sb_start - bm_space - 4*2; + if (!num_sectors || num_sectors > max_sectors) num_sectors = max_sectors; rdev->sb_start = sb_start; -- 2.7.5 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] super1.0 calculates max sectors in a wrong way 2020-06-30 7:55 ` [PATCH 1/2] super1.0 calculates max sectors in a wrong way Xiao Ni @ 2020-07-27 17:33 ` Song Liu 2020-07-28 1:18 ` Xiao Ni 0 siblings, 1 reply; 8+ messages in thread From: Song Liu @ 2020-07-27 17:33 UTC (permalink / raw) To: Xiao Ni; +Cc: linux-raid, Nigel Croxon On Tue, Jun 30, 2020 at 12:55 AM Xiao Ni <xni@redhat.com> wrote: > > One super1.0 raid array wants to grow size, it needs to check the device max usable > size. If the requested size is bigger than max usable size, it will return an error. > > Now it uses rdev->sectors for max usable size. If one disk is 500G and the raid device > only uses the 100GB of this disk. rdev->sectors can't tell the real max usable size. > The max usable size should be dev_size-(superblock_size+bitmap_size+badblock_size). > > Signed-off-by: Xiao Ni <xni@redhat.com> Thanks for the fix! > --- > drivers/md/md.c | 26 +++++++++++++++++++++++--- > 1 file changed, 23 insertions(+), 3 deletions(-) > > diff --git a/drivers/md/md.c b/drivers/md/md.c > index f567f53..d2c5984 100644 > --- a/drivers/md/md.c > +++ b/drivers/md/md.c > @@ -2183,10 +2183,30 @@ super_1_rdev_size_change(struct md_rdev *rdev, sector_t num_sectors) > return 0; > } else { > /* minor version 0; superblock after data */ > - sector_t sb_start; > - sb_start = (i_size_read(rdev->bdev->bd_inode) >> 9) - 8*2; > + sector_t sb_start, bm_space; > + sector_t dev_size = i_size_read(rdev->bdev->bd_inode) >> 9; > + > + /* 8K is for superblock */ > + sb_start = dev_size - 8*2; > sb_start &= ~(sector_t)(4*2 - 1); > - max_sectors = rdev->sectors + sb_start - rdev->sb_start; > + > + /* if the device is bigger than 8Gig, save 64k for bitmap usage, > + * if bigger than 200Gig, save 128k > + */ > + if (dev_size < 64*2) > + bm_space = 0; > + else if (dev_size - 64*2 >= 200*1024*1024*2) > + bm_space = 128*2; > + else if (dev_size - 4*2 > 8*1024*1024*2) > + bm_space = 64*2; > + else > + bm_space = 4*2; Could you explain the handling of bitmap space? Thanks, Song ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] super1.0 calculates max sectors in a wrong way 2020-07-27 17:33 ` Song Liu @ 2020-07-28 1:18 ` Xiao Ni 2020-07-29 0:43 ` Song Liu 0 siblings, 1 reply; 8+ messages in thread From: Xiao Ni @ 2020-07-28 1:18 UTC (permalink / raw) To: Song Liu; +Cc: linux-raid, Nigel Croxon On 07/28/2020 01:33 AM, Song Liu wrote: > On Tue, Jun 30, 2020 at 12:55 AM Xiao Ni <xni@redhat.com> wrote: >> One super1.0 raid array wants to grow size, it needs to check the device max usable >> size. If the requested size is bigger than max usable size, it will return an error. >> >> Now it uses rdev->sectors for max usable size. If one disk is 500G and the raid device >> only uses the 100GB of this disk. rdev->sectors can't tell the real max usable size. >> The max usable size should be dev_size-(superblock_size+bitmap_size+badblock_size). >> >> Signed-off-by: Xiao Ni <xni@redhat.com> > Thanks for the fix! > >> --- >> drivers/md/md.c | 26 +++++++++++++++++++++++--- >> 1 file changed, 23 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/md/md.c b/drivers/md/md.c >> index f567f53..d2c5984 100644 >> --- a/drivers/md/md.c >> +++ b/drivers/md/md.c >> @@ -2183,10 +2183,30 @@ super_1_rdev_size_change(struct md_rdev *rdev, sector_t num_sectors) >> return 0; >> } else { >> /* minor version 0; superblock after data */ >> - sector_t sb_start; >> - sb_start = (i_size_read(rdev->bdev->bd_inode) >> 9) - 8*2; >> + sector_t sb_start, bm_space; >> + sector_t dev_size = i_size_read(rdev->bdev->bd_inode) >> 9; >> + >> + /* 8K is for superblock */ >> + sb_start = dev_size - 8*2; >> sb_start &= ~(sector_t)(4*2 - 1); >> - max_sectors = rdev->sectors + sb_start - rdev->sb_start; >> + >> + /* if the device is bigger than 8Gig, save 64k for bitmap usage, >> + * if bigger than 200Gig, save 128k >> + */ >> + if (dev_size < 64*2) >> + bm_space = 0; >> + else if (dev_size - 64*2 >= 200*1024*1024*2) >> + bm_space = 128*2; >> + else if (dev_size - 4*2 > 8*1024*1024*2) >> + bm_space = 64*2; >> + else >> + bm_space = 4*2; > Could you explain the handling of bitmap space? Hi Song This calculation of bitmap is decided by mdadm. In mdadm super1.c choose_bm_space function, it uses this way to calculate bitmap space. Do I need to add comments here to describe this? Something like "mdadm function choose_bm_space decides the bitmap space"? Regards Xiao ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] super1.0 calculates max sectors in a wrong way 2020-07-28 1:18 ` Xiao Ni @ 2020-07-29 0:43 ` Song Liu 2020-07-29 1:13 ` Xiao Ni 0 siblings, 1 reply; 8+ messages in thread From: Song Liu @ 2020-07-29 0:43 UTC (permalink / raw) To: Xiao Ni; +Cc: linux-raid, Nigel Croxon On Mon, Jul 27, 2020 at 6:18 PM Xiao Ni <xni@redhat.com> wrote: > > > > On 07/28/2020 01:33 AM, Song Liu wrote: > > On Tue, Jun 30, 2020 at 12:55 AM Xiao Ni <xni@redhat.com> wrote: > >> One super1.0 raid array wants to grow size, it needs to check the device max usable > >> size. If the requested size is bigger than max usable size, it will return an error. > >> > >> Now it uses rdev->sectors for max usable size. If one disk is 500G and the raid device > >> only uses the 100GB of this disk. rdev->sectors can't tell the real max usable size. > >> The max usable size should be dev_size-(superblock_size+bitmap_size+badblock_size). > >> > >> Signed-off-by: Xiao Ni <xni@redhat.com> > > Thanks for the fix! > > > >> --- > >> drivers/md/md.c | 26 +++++++++++++++++++++++--- > >> 1 file changed, 23 insertions(+), 3 deletions(-) > >> > >> diff --git a/drivers/md/md.c b/drivers/md/md.c > >> index f567f53..d2c5984 100644 > >> --- a/drivers/md/md.c > >> +++ b/drivers/md/md.c > >> @@ -2183,10 +2183,30 @@ super_1_rdev_size_change(struct md_rdev *rdev, sector_t num_sectors) > >> return 0; > >> } else { > >> /* minor version 0; superblock after data */ > >> - sector_t sb_start; > >> - sb_start = (i_size_read(rdev->bdev->bd_inode) >> 9) - 8*2; > >> + sector_t sb_start, bm_space; > >> + sector_t dev_size = i_size_read(rdev->bdev->bd_inode) >> 9; > >> + > >> + /* 8K is for superblock */ > >> + sb_start = dev_size - 8*2; > >> sb_start &= ~(sector_t)(4*2 - 1); > >> - max_sectors = rdev->sectors + sb_start - rdev->sb_start; > >> + > >> + /* if the device is bigger than 8Gig, save 64k for bitmap usage, > >> + * if bigger than 200Gig, save 128k > >> + */ > >> + if (dev_size < 64*2) > >> + bm_space = 0; > >> + else if (dev_size - 64*2 >= 200*1024*1024*2) > >> + bm_space = 128*2; > >> + else if (dev_size - 4*2 > 8*1024*1024*2) > >> + bm_space = 64*2; > >> + else > >> + bm_space = 4*2; > > Could you explain the handling of bitmap space? > Hi Song > > This calculation of bitmap is decided by mdadm. In mdadm super1.c > choose_bm_space function, > it uses this way to calculate bitmap space. Do I need to add comments > here to describe this? > Something like "mdadm function choose_bm_space decides the bitmap space"? Thanks for the explanation. I merged the two patches, made some changes, and applied it to md-next. Please let me know whether it looks good. Song ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] super1.0 calculates max sectors in a wrong way 2020-07-29 0:43 ` Song Liu @ 2020-07-29 1:13 ` Xiao Ni 2020-07-29 2:51 ` Song Liu 0 siblings, 1 reply; 8+ messages in thread From: Xiao Ni @ 2020-07-29 1:13 UTC (permalink / raw) To: Song Liu; +Cc: linux-raid, Nigel Croxon On 07/29/2020 08:43 AM, Song Liu wrote: >> Hi Song >> >> This calculation of bitmap is decided by mdadm. In mdadm super1.c >> choose_bm_space function, >> it uses this way to calculate bitmap space. Do I need to add comments >> here to describe this? >> Something like "mdadm function choose_bm_space decides the bitmap space"? > Thanks for the explanation. > > I merged the two patches, made some changes, and applied it to md-next. Please > let me know whether it looks good. > The function super_10_choose_bm_space can make people confused. All types of super1 calculate bitmap space in the same way (super1.0, 1.1 and 1.2). Could you change the function name to super_1_choose_bm_space? Thanks Xiao ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] super1.0 calculates max sectors in a wrong way 2020-07-29 1:13 ` Xiao Ni @ 2020-07-29 2:51 ` Song Liu 0 siblings, 0 replies; 8+ messages in thread From: Song Liu @ 2020-07-29 2:51 UTC (permalink / raw) To: Xiao Ni; +Cc: linux-raid, Nigel Croxon On Tue, Jul 28, 2020 at 6:15 PM Xiao Ni <xni@redhat.com> wrote: > > > > On 07/29/2020 08:43 AM, Song Liu wrote: > >> Hi Song > >> > >> This calculation of bitmap is decided by mdadm. In mdadm super1.c > >> choose_bm_space function, > >> it uses this way to calculate bitmap space. Do I need to add comments > >> here to describe this? > >> Something like "mdadm function choose_bm_space decides the bitmap space"? > > Thanks for the explanation. > > > > I merged the two patches, made some changes, and applied it to md-next. Please > > let me know whether it looks good. > > > The function super_10_choose_bm_space can make people confused. All > types of super1 calculate > bitmap space in the same way (super1.0, 1.1 and 1.2). Could you change > the function name to > super_1_choose_bm_space? Updated. Thanks! Song ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 2/2] Don't need to reset superblock start address for super1.0 2020-06-30 7:55 [PATCH 0/2] Can't grow size twice for a super1.0 array Xiao Ni 2020-06-30 7:55 ` [PATCH 1/2] super1.0 calculates max sectors in a wrong way Xiao Ni @ 2020-06-30 7:55 ` Xiao Ni 1 sibling, 0 replies; 8+ messages in thread From: Xiao Ni @ 2020-06-30 7:55 UTC (permalink / raw) To: linux-raid; +Cc: song, ncroxon The superblock start address doesn't change after creating raid device. It always at the end of the device(end_pos-8*2). Signed-off-by: Xiao Ni <xni@redhat.com> --- drivers/md/md.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/md/md.c b/drivers/md/md.c index d2c5984..9de9c0e 100644 --- a/drivers/md/md.c +++ b/drivers/md/md.c @@ -2209,7 +2209,6 @@ super_1_rdev_size_change(struct md_rdev *rdev, sector_t num_sectors) if (!num_sectors || num_sectors > max_sectors) num_sectors = max_sectors; - rdev->sb_start = sb_start; } sb = page_address(rdev->sb_page); sb->data_size = cpu_to_le64(num_sectors); -- 2.7.5 ^ permalink raw reply related [flat|nested] 8+ messages in thread
end of thread, other threads:[~2020-07-29 2:51 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-06-30 7:55 [PATCH 0/2] Can't grow size twice for a super1.0 array Xiao Ni 2020-06-30 7:55 ` [PATCH 1/2] super1.0 calculates max sectors in a wrong way Xiao Ni 2020-07-27 17:33 ` Song Liu 2020-07-28 1:18 ` Xiao Ni 2020-07-29 0:43 ` Song Liu 2020-07-29 1:13 ` Xiao Ni 2020-07-29 2:51 ` Song Liu 2020-06-30 7:55 ` [PATCH 2/2] Don't need to reset superblock start address for super1.0 Xiao Ni
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.