| Message ID | 20231009151026.66145-6-aleksander.lobakin@intel.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | ip_tunnel: convert __be16 tunnel flags to bitmaps | expand |
On Mon, Oct 09, 2023 at 05:10:17PM +0200, Alexander Lobakin wrote: > bitmap_size() is a pretty generic name and one may want to use it for > a generic bitmap API function. At the same time, its logic is not > "generic", i.e. it's not just `nbits -> size of bitmap in bytes` > converter as it would be expected from its name. > Add the prefix 'idset_' used throughout the file where the function > resides. At the first glance, this custom implementation just duplicates the generic one that you introduce in the following patch. If so, why don't you switch idset to just use generic bitmap_size()? > > Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@intel.com> > Signed-off-by: Alexander Lobakin <aleksander.lobakin@intel.com> > --- > idset_new() really wants its vmalloc() + memset() pair to be replaced > with vzalloc(). > --- > drivers/s390/cio/idset.c | 10 ++++++---- > 1 file changed, 6 insertions(+), 4 deletions(-) > > diff --git a/drivers/s390/cio/idset.c b/drivers/s390/cio/idset.c > index 45f9c0736be4..0a1105a483bf 100644 > --- a/drivers/s390/cio/idset.c > +++ b/drivers/s390/cio/idset.c > @@ -16,7 +16,7 @@ struct idset { > unsigned long bitmap[]; > }; > > -static inline unsigned long bitmap_size(int num_ssid, int num_id) > +static inline unsigned long idset_bitmap_size(int num_ssid, int num_id) > { > return BITS_TO_LONGS(num_ssid * num_id) * sizeof(unsigned long); > } > @@ -25,11 +25,12 @@ static struct idset *idset_new(int num_ssid, int num_id) > { > struct idset *set; > > - set = vmalloc(sizeof(struct idset) + bitmap_size(num_ssid, num_id)); > + set = vmalloc(sizeof(struct idset) + > + idset_bitmap_size(num_ssid, num_id)); > if (set) { > set->num_ssid = num_ssid; > set->num_id = num_id; > - memset(set->bitmap, 0, bitmap_size(num_ssid, num_id)); > + memset(set->bitmap, 0, idset_bitmap_size(num_ssid, num_id)); > } > return set; > } > @@ -41,7 +42,8 @@ void idset_free(struct idset *set) > > void idset_fill(struct idset *set) > { > - memset(set->bitmap, 0xff, bitmap_size(set->num_ssid, set->num_id)); > + memset(set->bitmap, 0xff, > + idset_bitmap_size(set->num_ssid, set->num_id)); > } > > static inline void idset_add(struct idset *set, int ssid, int id) > -- > 2.41.0
From: Yury Norov <yury.norov@gmail.com> Date: Mon, 9 Oct 2023 09:35:20 -0700 > On Mon, Oct 09, 2023 at 05:10:17PM +0200, Alexander Lobakin wrote: >> bitmap_size() is a pretty generic name and one may want to use it for >> a generic bitmap API function. At the same time, its logic is not >> "generic", i.e. it's not just `nbits -> size of bitmap in bytes` >> converter as it would be expected from its name. >> Add the prefix 'idset_' used throughout the file where the function >> resides. > > At the first glance, this custom implementation just duplicates the > generic one that you introduce in the following patch. If so, why > don't you switch idset to just use generic bitmap_size()? I didn't want to introduce any semantic changes, but good point, why not. > >> >> Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@intel.com> >> Signed-off-by: Alexander Lobakin <aleksander.lobakin@intel.com> [...] Thanks, Olek
diff --git a/drivers/s390/cio/idset.c b/drivers/s390/cio/idset.c index 45f9c0736be4..0a1105a483bf 100644 --- a/drivers/s390/cio/idset.c +++ b/drivers/s390/cio/idset.c @@ -16,7 +16,7 @@ struct idset { unsigned long bitmap[]; }; -static inline unsigned long bitmap_size(int num_ssid, int num_id) +static inline unsigned long idset_bitmap_size(int num_ssid, int num_id) { return BITS_TO_LONGS(num_ssid * num_id) * sizeof(unsigned long); } @@ -25,11 +25,12 @@ static struct idset *idset_new(int num_ssid, int num_id) { struct idset *set; - set = vmalloc(sizeof(struct idset) + bitmap_size(num_ssid, num_id)); + set = vmalloc(sizeof(struct idset) + + idset_bitmap_size(num_ssid, num_id)); if (set) { set->num_ssid = num_ssid; set->num_id = num_id; - memset(set->bitmap, 0, bitmap_size(num_ssid, num_id)); + memset(set->bitmap, 0, idset_bitmap_size(num_ssid, num_id)); } return set; } @@ -41,7 +42,8 @@ void idset_free(struct idset *set) void idset_fill(struct idset *set) { - memset(set->bitmap, 0xff, bitmap_size(set->num_ssid, set->num_id)); + memset(set->bitmap, 0xff, + idset_bitmap_size(set->num_ssid, set->num_id)); } static inline void idset_add(struct idset *set, int ssid, int id)