Re: [PATCH 2/6] bitops: always define asm-generic non-atomic bitops

[Alexander Lobakin]

From: Mark Rutland <mark.rutland@xxxxxxx>
Date: Mon, 6 Jun 2022 13:44:52 +0100

> On Mon, Jun 06, 2022 at 01:49:03PM +0200, Alexander Lobakin wrote:
> > Move generic non-atomic bitops from the asm-generic header which
> > gets included only when there are no architecture-specific
> > alternatives, to a separate independent file to make them always
> > available.
> > 
> > Signed-off-by: Alexander Lobakin <alexandr.lobakin@xxxxxxxxx>
> > ---
> >  .../asm-generic/bitops/generic-non-atomic.h   | 124 ++++++++++++++++++
> >  include/asm-generic/bitops/non-atomic.h       | 109 ++-------------
> >  2 files changed, 132 insertions(+), 101 deletions(-)
> >  create mode 100644 include/asm-generic/bitops/generic-non-atomic.h
> > 
> > diff --git a/include/asm-generic/bitops/generic-non-atomic.h b/include/asm-generic/bitops/generic-non-atomic.h
> > new file mode 100644
> > index 000000000000..7a60adfa6e7d
> > --- /dev/null
> > +++ b/include/asm-generic/bitops/generic-non-atomic.h
> > @@ -0,0 +1,124 @@
> > +/* SPDX-License-Identifier: GPL-2.0-only */
> > +#ifndef __ASM_GENERIC_BITOPS_GENERIC_NON_ATOMIC_H
> > +#define __ASM_GENERIC_BITOPS_GENERIC_NON_ATOMIC_H
> > +
> > +#include <linux/bits.h>
> > +
> > +#ifndef _LINUX_BITOPS_H
> > +#error only <linux/bitops.h> can be included directly
> > +#endif
> > +
> > +/*
> > + * Generic definitions for bit operations, should not be used in regular code
> > + * directly.
> > + */
> > +
> > +/**
> > + * gen___set_bit - Set a bit in memory
> > + * @nr: the bit to set
> > + * @addr: the address to start counting from
> > + *
> > + * Unlike set_bit(), this function is non-atomic and may be reordered.
> > + * If it's called on the same region of memory simultaneously, the effect
> > + * may be that only one operation succeeds.
> > + */
> > +static __always_inline void
> > +gen___set_bit(unsigned int nr, volatile unsigned long *addr)
> 
> Could we please use 'generic' rather than 'gen' as the prefix?
> 
> That'd match what we did for the generic atomic_*() and atomic64_*() functions
> in commits
> 
> * f8b6455a9d381fc5 ("locking/atomic: atomic: support ARCH_ATOMIC")
> * 1bdadf46eff6804a ("locking/atomic: atomic64: support ARCH_ATOMIC")
> 
> ... and it avoids any potential confusion with 'gen' meaning 'generated' or
> similar.

Sure! Thanks for giving the hint, I do agree it would look better
and will rename in v2.

> 
> Thanks,
> Mark.
> 
> > +{
> > +	unsigned long mask = BIT_MASK(nr);
> > +	unsigned long *p = ((unsigned long *)addr) + BIT_WORD(nr);
> > +
> > +	*p  |= mask;
> > +}

[...]

> > -- 
> > 2.36.1

Thanks,
Al