Re: [PATCH] md/md-llbitmap: grow the page cache in place for reshape
Su Yue <[email protected]>
| Newsgroups | gmane.linux.kernel,gmane.linux.raid |
|---|---|
| Message-ID | <[email protected]> |
On Fri 05 Jun 2026 at 17:15, Yu Kuai <[email protected]> wrote: > From: Yu Kuai <[email protected]> > > Use the page-control helpers to grow llbitmap's cached pages in > place > for resize and later reshape preparation, instead of rebuilding > the > whole cache. > > Signed-off-by: Yu Kuai <[email protected]> > --- > drivers/md/md-llbitmap.c | 139 > +++++++++++++++++++++++++++++++++++---- > 1 file changed, 127 insertions(+), 12 deletions(-) > > diff --git a/drivers/md/md-llbitmap.c b/drivers/md/md-llbitmap.c > index 2f2896fe4d6f..91d3dec43d48 100644 > --- a/drivers/md/md-llbitmap.c > +++ b/drivers/md/md-llbitmap.c > @@ -414,10 +414,23 @@ static char > state_machine[BitStateCount][BitmapActionCount] = { > [BitmapActionClearUnwritten] = BitUnwritten, > }, > }; > > static void __llbitmap_flush(struct mddev *mddev); > +static void llbitmap_flush(struct mddev *mddev); > +static void llbitmap_update_sb(void *data); > + > +static void llbitmap_resize_chunks(struct mddev *mddev, > sector_t blocks, > + unsigned long *chunksize, > + unsigned long *chunks) > NIT: I would like call it llbitmap_calculate_chunks. > +{ > + *chunks = DIV_ROUND_UP_SECTOR_T(blocks, *chunksize); > + while (*chunks > mddev->bitmap_info.space << SECTOR_SHIFT) { > + *chunksize = *chunksize << 1; > + *chunks = DIV_ROUND_UP_SECTOR_T(blocks, *chunksize); > + } > +} > > static enum llbitmap_state llbitmap_read(struct llbitmap > *llbitmap, loff_t pos) > { > unsigned int idx; > unsigned int offset; > @@ -653,10 +666,52 @@ static unsigned int > llbitmap_reserved_pages(struct llbitmap *llbitmap) > { > return DIV_ROUND_UP(llbitmap->mddev->bitmap_info.space << > SECTOR_SHIFT, > PAGE_SIZE); > } > > +static int llbitmap_expand_pages(struct llbitmap *llbitmap, > + unsigned long chunks) > +{ > + struct llbitmap_page_ctl **pctl; > + unsigned int old_nr_pages = llbitmap->nr_pages; > + unsigned int nr_pages = llbitmap_used_pages(llbitmap, chunks); > + int i; > + int ret; > + > + if (nr_pages <= old_nr_pages) > + return 0; > + > + pctl = kcalloc(nr_pages, sizeof(*pctl), GFP_KERNEL); > + if (!pctl) > + return -ENOMEM; > + > + if (llbitmap->pctl) > + memcpy(pctl, llbitmap->pctl, > + array_size(old_nr_pages, sizeof(*pctl))); > + > + for (i = old_nr_pages; i < nr_pages; i++) { > + pctl[i] = llbitmap_alloc_page_ctl(llbitmap, i); > + if (IS_ERR(pctl[i])) > + goto err_alloc_ptr; > + } > + > + kfree(llbitmap->pctl); > + llbitmap->pctl = pctl; > + llbitmap->nr_pages = nr_pages; > + return 0; > + > +err_alloc_ptr: > + ret = PTR_ERR(pctl[i]); > + for (i--; i >= (int)old_nr_pages; i--) { > Confused about why not just declare i as an unsigned int? -- Su > + __free_page(pctl[i]->page); > + percpu_ref_exit(&pctl[i]->active); > + kfree(pctl[i]); > + } > + kfree(pctl); > + return ret; > +} > + > static int llbitmap_alloc_pages(struct llbitmap *llbitmap) > { > unsigned int used_pages = llbitmap_used_pages(llbitmap, > llbitmap->chunks); > unsigned int nr_pages = max(used_pages, > llbitmap_reserved_pages(llbitmap)); > int i; > @@ -728,10 +783,38 @@ static bool llbitmap_zero_all_disks(struct > llbitmap *llbitmap) > } > > return true; > } > > +static void llbitmap_mark_range(struct llbitmap *llbitmap, > + unsigned long start, > + unsigned long end, > + enum llbitmap_state state) > +{ > + while (start <= end) { > + llbitmap_write(llbitmap, state, start); > + start++; > + } > +} > + > +static int llbitmap_prepare_resize(struct llbitmap *llbitmap, > + unsigned long old_chunks, > + unsigned long new_chunks, > + unsigned long cache_chunks) > +{ > + int ret; > + > + llbitmap_flush(llbitmap->mddev); > + ret = llbitmap_expand_pages(llbitmap, cache_chunks); > + if (ret) > + return ret; > + if (new_chunks > old_chunks) > + llbitmap_mark_range(llbitmap, old_chunks, new_chunks - 1, > + BitUnwritten); > + return 0; > +} > + > static void llbitmap_init_state(struct llbitmap *llbitmap) > { > struct mddev *mddev = llbitmap->mddev; > enum llbitmap_state state = BitUnwritten; > unsigned long i; > @@ -1024,14 +1107,14 @@ static int llbitmap_read_sb(struct > llbitmap *llbitmap) > pr_err("md/llbitmap: %s: chunksize not a power of 2", > mdname(mddev)); > goto out_put_page; > } > > - if (chunksize < > DIV_ROUND_UP_SECTOR_T(mddev->resync_max_sectors, > + if (chunksize < DIV_ROUND_UP_SECTOR_T(sync_size, > mddev->bitmap_info.space << > SECTOR_SHIFT)) { > pr_err("md/llbitmap: %s: chunksize too small %lu < %llu / > %lu", > - mdname(mddev), chunksize, > mddev->resync_max_sectors, > + mdname(mddev), chunksize, sync_size, > mddev->bitmap_info.space); > goto out_put_page; > } > > daemon_sleep = le32_to_cpu(sb->daemon_sleep); > @@ -1169,28 +1252,60 @@ static int llbitmap_create(struct mddev > *mddev) > } > > static int llbitmap_resize(struct mddev *mddev, sector_t > blocks, int chunksize) > { > struct llbitmap *llbitmap = mddev->bitmap; > + sector_t old_blocks = llbitmap->sync_size; > + unsigned long old_chunks = llbitmap->chunks; > unsigned long chunks; > + unsigned long cache_chunks; > + int ret = 0; > + unsigned long bitmap_chunksize; > + bool reshape; > > if (chunksize == 0) > chunksize = llbitmap->chunksize; > > - /* If there is enough space, leave the chunksize unchanged. */ > - chunks = DIV_ROUND_UP_SECTOR_T(blocks, chunksize); > - while (chunks > mddev->bitmap_info.space << SECTOR_SHIFT) { > - chunksize = chunksize << 1; > - chunks = DIV_ROUND_UP_SECTOR_T(blocks, chunksize); > - } > + bitmap_chunksize = chunksize; > + llbitmap_resize_chunks(mddev, blocks, &bitmap_chunksize, > &chunks); > > - llbitmap->chunkshift = ffz(~chunksize); > - llbitmap->chunksize = chunksize; > - llbitmap->chunks = chunks; > - llbitmap->sync_size = blocks; > + reshape = mddev->delta_disks || mddev->new_level != > mddev->level || > + mddev->new_layout != mddev->layout || > + mddev->new_chunk_sectors != mddev->chunk_sectors; > + if (!reshape && bitmap_chunksize != llbitmap->chunksize) > + return -EOPNOTSUPP; > + if (blocks == old_blocks && chunks == llbitmap->chunks) > + return 0; > + > + mutex_lock(&mddev->bitmap_info.mutex); > > + cache_chunks = reshape ? max(old_chunks, chunks) : chunks; > + ret = llbitmap_prepare_resize(llbitmap, old_chunks, chunks, > cache_chunks); > + if (ret) > + goto out; > + > + if (reshape) { > + llbitmap->reshape_sync_size = blocks; > + llbitmap->reshape_chunksize = bitmap_chunksize; > + llbitmap->reshape_chunks = chunks; > + llbitmap->chunks = max(old_chunks, chunks); > + } else { > + if (blocks < old_blocks && chunks < old_chunks) > + llbitmap_mark_range(llbitmap, chunks, old_chunks - 1, > + BitUnwritten); > + mddev->bitmap_info.chunksize = bitmap_chunksize; > + llbitmap->chunks = chunks; > + llbitmap->sync_size = blocks; > + llbitmap_update_sb(llbitmap); > + } > + __llbitmap_flush(mddev); > + mutex_unlock(&mddev->bitmap_info.mutex); > return 0; > + > +out: > + mutex_unlock(&mddev->bitmap_info.mutex); > + return ret; > } > > static int llbitmap_load(struct mddev *mddev) > { > enum llbitmap_action action = BitmapActionReload;