Re: [PATCH v1 1/1] treewide: Align match_string() with sysfs_match_string()

From: Yury Norov
Date: Tue Jun 11 2024 - 17:38:16 EST


On Mon, Jun 10, 2024 at 10:08:00AM +0200, Linus Walleij wrote:
> On Tue, Jun 4, 2024 at 9:46 AM Jani Nikula <jani.nikula@xxxxxxxxxxxxxxx> wrote:
>
> [Maybe slightly off-topic, ranty]
>
> > Why do we think it's a good idea to increase and normalize the use of
> > double-underscore function names across the kernel, like
> > __match_string() in this case? It should mean "reserved for the
> > implementation, not to be called directly".
> >
> > If it's to be used directly, it should be named accordingly, right?
>
> It's a huge mess. "__" prefix is just so ambiguous I think it just
> shouldn't be used or prolifierated, and it usually breaks Rusty Russells
> API rules times over.
>
> Consider __set_bit() from <linux/bitops.h>, used all over the place,
> in contrast with set_bit() for example, what does "__" represent in
> this context that makes __set_bit() different from set_bit()?
>
> It means "non-atomic"...
>
> How does a random contributor know this?
>
> Yeah, you guess it. By the token of "everybody knows that".
> (Grep, google, repeat for the number of contributors to the kernel.)
>
> I was considering to send a script to Torvalds to just change all
> this to set_bit_nonatomic() (etc) but was hesitating because that
> makes the name unambiguous but long. I think I stayed off it
> because changing stuff like that all over the place creates churn
> and churn is bad.

Hi Linus,

I think about renaming set_bit() stuff for atomicity at least twice
a year. But it would be over 15000 renames for set and clear_bit only:

yury:linux$ git grep -w -E "set_bit|clear_bit"|wc -l
12972
yury:linux$ git grep -w -E "__set_bit|__clear_bit"|wc -l
2914

Anyways, if someone is brave enough to do that... The main problem is
that set_bit() is heavily abused because people don't bother putting
the '__' thing, which leads to using atomic helpers where non-atomic
versions are enough.

So the right path for renaming would be:

s/set_bit/atomic_set_bit
s/__set_bit/nonatomic_set_bit
/* Wait for a full release cycle or two to let people get used, then */
#define set_bit nonatomic_set_bit

Thanks,
Yury

PS. Had to drop all maillists except for LKML and some random victims
because my gmail account doesn't allow more that 100 recipients. If
you guys know how to increase the limit, please advise.