Messages in this thread | | | Date | Fri, 18 Aug 2023 11:55:47 +0300 | From | Andy Shevchenko <> | Subject | Re: [PATCH v1 2/2] bitmap: Optimize memset() calls |
| |
On Fri, Aug 18, 2023 at 08:53:38AM +0200, Rasmus Villemoes wrote: > On 17/08/2023 18.54, Andy Shevchenko wrote:
...
> > +#if BITS_PER_LONG == 64 > > + memset64((uint64_t *)dst, 0, len); > > +#else > > + memset32((uint32_t *)dst, 0, len); > > +#endif > > } > > So _if_ this is worth it at all,
Yury, I believe you have a set of performance tests for bitmaps, perhaps you can give a try and see if we got a benefit here.
Yet, not all architectures have custom implementation of the optimized memset()/memcpy(). But at least ARM, x86 do.
> all those new '#if BITS_PER_LONG == 64' > suggests that we should instead have a new helper memset_long(), no? > > In fact, string.h already has that: > > static inline void *memset_l(unsigned long *p, unsigned long v, > __kernel_size_t n)
My grep skills suddenly degraded, my thoughts were exactly like yours, but I missed the _l instead of _long for which I grepped.
...
> Please just spell an all-ones long "~0UL", that also matches the > small_const case.
OK!
...
> > - memset(&dst[lim - off], 0, off*sizeof(unsigned long)); > > + bitmap_zero(&dst[lim - off], off); > > This... can't be right.
Indeed, I have in mind memset_long() and on the half-way replaced it with bitmap_zero().
> bitmap_zero() still takes an argument which is > the size in bits, while off is the whole number of words to shift. So if > called with say shift=128, we'd have off==2, and that bitmap_zero() > would, because bitmap_zero() rounds up to a whole number of words, end > up clearing just one word. > > Perhaps a chance to add some more test cases? Maybe we're not exercising > any of the "shift more than BITS_PER_LONG" logic. > > But rather than using bitmap_zero() here, forcing you to convert off to > a number of bits and then bitmap_zero to divide again, if you do this at > all, just change that memset() to memset_l().
Agree.
-- With Best Regards, Andy Shevchenko
| |