Messages in this thread | | | From | Guoqing Jiang <> | Subject | Re: [PATCH v2] md: Kill usage of page->index | Date | Fri, 15 Oct 2021 11:01:21 +0800 |
| |
On 10/14/21 10:30 PM, Kent Overstreet wrote: > On Thu, Oct 14, 2021 at 04:58:46PM +0800,heming.zhao@suse.com wrote: >> Hello all, >> >> The page->index takes an important role for cluster-md module. >> i.e, a two-node HA env, node A bitmap may be managed in first 4K area, then >> node B bitmap is in 8K area (the second page). this patch removes the index >> and fix/hardcode index with value 0, which will only operate first node bitmap. >> >> If this serial patches are important and must be merged in mainline, we should >> redesign code logic for the related code. >> >> Thanks, >> Heming > Can you look at and test the updated patch below? The more I look at the md > bitmap code the more it scares me. > > -- >8 -- > Subject: [PATCH] md: Kill usage of page->index > > As part of the struct page cleanups underway, we want to remove as much > usage of page->mapping and page->index as possible, as frequently they > are known from context - as they are here in the md bitmap code. > > Signed-off-by: Kent Overstreet<kent.overstreet@gmail.com> > --- > drivers/md/md-bitmap.c | 49 ++++++++++++++++++++---------------------- > drivers/md/md-bitmap.h | 1 + > 2 files changed, 24 insertions(+), 26 deletions(-) > > diff --git a/drivers/md/md-bitmap.c b/drivers/md/md-bitmap.c > index e29c6298ef..316e4cd5a7 100644 > --- a/drivers/md/md-bitmap.c > +++ b/drivers/md/md-bitmap.c > @@ -165,10 +165,8 @@ static int read_sb_page(struct mddev *mddev, loff_t offset, > > if (sync_page_io(rdev, target, > roundup(size, bdev_logical_block_size(rdev->bdev)), > - page, REQ_OP_READ, 0, true)) { > - page->index = index; > + page, REQ_OP_READ, 0, true)) > return 0; > - } > } > return -EIO; > } > @@ -209,7 +207,8 @@ static struct md_rdev *next_active_rdev(struct md_rdev *rdev, struct mddev *mdde > return NULL; > } > > -static int write_sb_page(struct bitmap *bitmap, struct page *page, int wait) > +static int write_sb_page(struct bitmap *bitmap, struct page *page, > + unsigned long index, int wait) > { > struct md_rdev *rdev; > struct block_device *bdev; > @@ -224,7 +223,7 @@ static int write_sb_page(struct bitmap *bitmap, struct page *page, int wait) > > bdev = (rdev->meta_bdev) ? rdev->meta_bdev : rdev->bdev; > > - if (page->index == store->file_pages-1) { > + if (index == store->file_pages-1) { > int last_page_size = store->bytes & (PAGE_SIZE-1); > if (last_page_size == 0) > last_page_size = PAGE_SIZE; > @@ -236,8 +235,7 @@ static int write_sb_page(struct bitmap *bitmap, struct page *page, int wait) > */ > if (mddev->external) { > /* Bitmap could be anywhere. */ > - if (rdev->sb_start + offset + (page->index > - * (PAGE_SIZE/512)) > + if (rdev->sb_start + offset + index * PAGE_SECTORS > > rdev->data_offset > && > rdev->sb_start + offset > @@ -247,7 +245,7 @@ static int write_sb_page(struct bitmap *bitmap, struct page *page, int wait) > } else if (offset < 0) { > /* DATA BITMAP METADATA */ > if (offset > - + (long)(page->index * (PAGE_SIZE/512)) > + + (long)(index * PAGE_SECTORS) > + size/512 > 0) > /* bitmap runs in to metadata */ > goto bad_alignment; > @@ -259,7 +257,7 @@ static int write_sb_page(struct bitmap *bitmap, struct page *page, int wait) > /* METADATA BITMAP DATA */ > if (rdev->sb_start > + offset > - + page->index*(PAGE_SIZE/512) + size/512 > + + index * PAGE_SECTORS + size/512 > > rdev->data_offset) > /* bitmap runs in to data */ > goto bad_alignment; > @@ -268,7 +266,7 @@ static int write_sb_page(struct bitmap *bitmap, struct page *page, int wait) > } > md_super_write(mddev, rdev, > rdev->sb_start + offset > - + page->index * (PAGE_SIZE/512), > + + index * PAGE_SECTORS, > size, > page); > } > @@ -285,12 +283,13 @@ 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) > +static void write_page(struct bitmap *bitmap, struct page *page, > + unsigned long index, int wait) > { > struct buffer_head *bh; > > if (bitmap->storage.file == NULL) { > - switch (write_sb_page(bitmap, page, wait)) { > + switch (write_sb_page(bitmap, page, index, wait)) { > case -EINVAL: > set_bit(BITMAP_WRITE_ERROR, &bitmap->flags); > } > @@ -399,7 +398,6 @@ static int read_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); > @@ -472,7 +470,7 @@ 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); > + write_page(bitmap, bitmap->storage.sb_page, bitmap->storage.sb_index, 1);
I guess it is fine for sb_page now.
[...]
> @@ -1027,7 +1024,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); > + write_page(bitmap, bitmap->storage.filemap[i], i, 0);
But for filemap page, I am not sure the above is correct.
> writing = 1; > } > } > @@ -1137,7 +1134,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); > + write_page(bitmap, page, index, 1);
Ditto.
> > ret = -EIO; > if (test_bit(BITMAP_WRITE_ERROR, > @@ -1336,7 +1333,7 @@ void md_bitmap_daemon_work(struct mddev *mddev) > if (bitmap->storage.filemap && > test_and_clear_page_attr(bitmap, j, > BITMAP_PAGE_NEEDWRITE)) { > - write_page(bitmap, bitmap->storage.filemap[j], 0); > + write_page(bitmap, bitmap->storage.filemap[j], j, 0);
Ditto.
> } > } > > diff --git a/drivers/md/md-bitmap.h b/drivers/md/md-bitmap.h > index cfd7395de8..6e820eea32 100644 > --- a/drivers/md/md-bitmap.h > +++ b/drivers/md/md-bitmap.h > @@ -207,6 +207,7 @@ struct bitmap { > * w/ filemap pages */ > unsigned long file_pages; /* number of pages in the file*/ > unsigned long bytes; /* total bytes in the bitmap */ > + unsigned long sb_index; /* sb page index */
I guess it resolve the issue for sb_page, and we might need to do similar things for filemap as well if I am not misunderstood.
Thanks, Guoqing
| |