| Message ID | 20240810020854.797814-4-yukuai1@huaweicloud.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | md/md-bitmap: introduce bitmap_operations | expand |
On Sat, 10 Aug 2024 10:08:31 +0800 Yu Kuai <yukuai1@huaweicloud.com> wrote: > From: Yu Kuai <yukuai3@huawei.com> > > So that the implementation won't be exposed, and it'll be possible > to invent a new bitmap by replacing bitmap_operations. I don't like repeating same commit message for few patches. > > Signed-off-by: Yu Kuai <yukuai3@huawei.com> > --- > drivers/md/md-bitmap.c | 6 +++--- > drivers/md/md-bitmap.h | 10 +++++++++- > 2 files changed, 12 insertions(+), 4 deletions(-) > > diff --git a/drivers/md/md-bitmap.c b/drivers/md/md-bitmap.c > index d731f7d4bbbb..9a9f0fe3ebd0 100644 > --- a/drivers/md/md-bitmap.c > +++ b/drivers/md/md-bitmap.c > @@ -1965,7 +1965,7 @@ static struct bitmap *bitmap_create(struct mddev > *mddev, int slot) return ERR_PTR(err); > } > > -int md_bitmap_load(struct mddev *mddev) > +static int bitmap_load(struct mddev *mddev) > { > int err = 0; > sector_t start = 0; > @@ -2021,7 +2021,6 @@ int md_bitmap_load(struct mddev *mddev) > out: > return err; > } > -EXPORT_SYMBOL_GPL(md_bitmap_load); > > /* caller need to free returned bitmap with md_bitmap_free() */ > struct bitmap *get_bitmap_from_slot(struct mddev *mddev, int slot) > @@ -2411,7 +2410,7 @@ location_store(struct mddev *mddev, const char *buf, > size_t len) } > > mddev->bitmap = bitmap; > - rv = md_bitmap_load(mddev); > + rv = bitmap_load(mddev); > if (rv) { > mddev->bitmap_info.offset = 0; > md_bitmap_destroy(mddev); > @@ -2710,6 +2709,7 @@ const struct attribute_group md_bitmap_group = { > > static struct bitmap_operations bitmap_ops = { > .create = bitmap_create, > + .load = bitmap_load, > }; > > void mddev_set_bitmap_ops(struct mddev *mddev) > diff --git a/drivers/md/md-bitmap.h b/drivers/md/md-bitmap.h > index a7cbf0c692fc..de7fbe5903dd 100644 > --- a/drivers/md/md-bitmap.h > +++ b/drivers/md/md-bitmap.h > @@ -236,6 +236,7 @@ struct bitmap { > > struct bitmap_operations { > struct bitmap* (*create)(struct mddev *mddev, int slot); > + int (*load)(struct mddev *mddev); > }; > > /* the bitmap API */ > @@ -250,7 +251,14 @@ static inline struct bitmap *md_bitmap_create(struct > mddev *mddev, int slot) return mddev->bitmap_ops->create(mddev, slot); > } > > -int md_bitmap_load(struct mddev *mddev); > +static inline int md_bitmap_load(struct mddev *mddev) > +{ > + if (!mddev->bitmap_ops->load) > + return -EOPNOTSUPP; > + > + return mddev->bitmap_ops->load(mddev); > +} At this point we have only on bitmpa op (at that probably won't change), so if we we have bitmap_ops assigned (mddev->bitmap_ops != NULL) then ->load is must have, hence I don't see a need for this wrapper. you probably made this to avoid changes across code. If yes, please mention it in commit message but I still would prefer to replace them all by calls to mddev->bitmap_ops->load(). Mariusz
Hi, 在 2024/08/13 15:07, Mariusz Tkaczyk 写道: > On Sat, 10 Aug 2024 10:08:31 +0800 > Yu Kuai <yukuai1@huaweicloud.com> wrote: > >> From: Yu Kuai <yukuai3@huawei.com> >> >> So that the implementation won't be exposed, and it'll be possible >> to invent a new bitmap by replacing bitmap_operations. > > I don't like repeating same commit message for few patches. > >> >> Signed-off-by: Yu Kuai <yukuai3@huawei.com> >> --- >> drivers/md/md-bitmap.c | 6 +++--- >> drivers/md/md-bitmap.h | 10 +++++++++- >> 2 files changed, 12 insertions(+), 4 deletions(-) >> >> diff --git a/drivers/md/md-bitmap.c b/drivers/md/md-bitmap.c >> index d731f7d4bbbb..9a9f0fe3ebd0 100644 >> --- a/drivers/md/md-bitmap.c >> +++ b/drivers/md/md-bitmap.c >> @@ -1965,7 +1965,7 @@ static struct bitmap *bitmap_create(struct mddev >> *mddev, int slot) return ERR_PTR(err); >> } >> >> -int md_bitmap_load(struct mddev *mddev) >> +static int bitmap_load(struct mddev *mddev) >> { >> int err = 0; >> sector_t start = 0; >> @@ -2021,7 +2021,6 @@ int md_bitmap_load(struct mddev *mddev) >> out: >> return err; >> } >> -EXPORT_SYMBOL_GPL(md_bitmap_load); >> >> /* caller need to free returned bitmap with md_bitmap_free() */ >> struct bitmap *get_bitmap_from_slot(struct mddev *mddev, int slot) >> @@ -2411,7 +2410,7 @@ location_store(struct mddev *mddev, const char *buf, >> size_t len) } >> >> mddev->bitmap = bitmap; >> - rv = md_bitmap_load(mddev); >> + rv = bitmap_load(mddev); >> if (rv) { >> mddev->bitmap_info.offset = 0; >> md_bitmap_destroy(mddev); >> @@ -2710,6 +2709,7 @@ const struct attribute_group md_bitmap_group = { >> >> static struct bitmap_operations bitmap_ops = { >> .create = bitmap_create, >> + .load = bitmap_load, >> }; >> >> void mddev_set_bitmap_ops(struct mddev *mddev) >> diff --git a/drivers/md/md-bitmap.h b/drivers/md/md-bitmap.h >> index a7cbf0c692fc..de7fbe5903dd 100644 >> --- a/drivers/md/md-bitmap.h >> +++ b/drivers/md/md-bitmap.h >> @@ -236,6 +236,7 @@ struct bitmap { >> >> struct bitmap_operations { >> struct bitmap* (*create)(struct mddev *mddev, int slot); >> + int (*load)(struct mddev *mddev); >> }; >> >> /* the bitmap API */ >> @@ -250,7 +251,14 @@ static inline struct bitmap *md_bitmap_create(struct >> mddev *mddev, int slot) return mddev->bitmap_ops->create(mddev, slot); >> } >> >> -int md_bitmap_load(struct mddev *mddev); >> +static inline int md_bitmap_load(struct mddev *mddev) >> +{ >> + if (!mddev->bitmap_ops->load) >> + return -EOPNOTSUPP; >> + >> + return mddev->bitmap_ops->load(mddev); >> +} > > At this point we have only on bitmpa op (at that probably won't change), so if > we we have bitmap_ops assigned (mddev->bitmap_ops != NULL) then ->load is must > have, hence I don't see a need for this wrapper. Yes, we must define the load op otherwise the bitmap_ops is meaningless. > > you probably made this to avoid changes across code. If yes, please mention it > in commit message but I still would prefer to replace them all by calls to > mddev->bitmap_ops->load(). I'll replace to use mddev->bitmap_ops->load() directly, and other places. And I'll keep this inline helper for some ops that is used for md-cluster, because I'm not planning to support md-cluster for the new bitmap first. Thanks, Kuai > > Mariusz > > > . >
diff --git a/drivers/md/md-bitmap.c b/drivers/md/md-bitmap.c index d731f7d4bbbb..9a9f0fe3ebd0 100644 --- a/drivers/md/md-bitmap.c +++ b/drivers/md/md-bitmap.c @@ -1965,7 +1965,7 @@ static struct bitmap *bitmap_create(struct mddev *mddev, int slot) return ERR_PTR(err); } -int md_bitmap_load(struct mddev *mddev) +static int bitmap_load(struct mddev *mddev) { int err = 0; sector_t start = 0; @@ -2021,7 +2021,6 @@ int md_bitmap_load(struct mddev *mddev) out: return err; } -EXPORT_SYMBOL_GPL(md_bitmap_load); /* caller need to free returned bitmap with md_bitmap_free() */ struct bitmap *get_bitmap_from_slot(struct mddev *mddev, int slot) @@ -2411,7 +2410,7 @@ location_store(struct mddev *mddev, const char *buf, size_t len) } mddev->bitmap = bitmap; - rv = md_bitmap_load(mddev); + rv = bitmap_load(mddev); if (rv) { mddev->bitmap_info.offset = 0; md_bitmap_destroy(mddev); @@ -2710,6 +2709,7 @@ const struct attribute_group md_bitmap_group = { static struct bitmap_operations bitmap_ops = { .create = bitmap_create, + .load = bitmap_load, }; void mddev_set_bitmap_ops(struct mddev *mddev) diff --git a/drivers/md/md-bitmap.h b/drivers/md/md-bitmap.h index a7cbf0c692fc..de7fbe5903dd 100644 --- a/drivers/md/md-bitmap.h +++ b/drivers/md/md-bitmap.h @@ -236,6 +236,7 @@ struct bitmap { struct bitmap_operations { struct bitmap* (*create)(struct mddev *mddev, int slot); + int (*load)(struct mddev *mddev); }; /* the bitmap API */ @@ -250,7 +251,14 @@ static inline struct bitmap *md_bitmap_create(struct mddev *mddev, int slot) return mddev->bitmap_ops->create(mddev, slot); } -int md_bitmap_load(struct mddev *mddev); +static inline int md_bitmap_load(struct mddev *mddev) +{ + if (!mddev->bitmap_ops->load) + return -EOPNOTSUPP; + + return mddev->bitmap_ops->load(mddev); +} + void md_bitmap_flush(struct mddev *mddev); void md_bitmap_destroy(struct mddev *mddev);