On 8/28/23, Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx> wrote: > On Mon, 28 Aug 2023 at 11:04, Mateusz Guzik <mjguzik@xxxxxxxxx> wrote: >> >> Other files do it (e.g., see __copy_user_nocache), but I have no >> strong opinion myself. > > So the __copy_user_nocache() thing is a case of that second issue - > see my comment about "some sane visually sensible grouping" of the > numbers. > > Look closer, and you'll notice that they aren't actually sequential. > They are of the form XY where the X is the grouping, and Y is the > local number within that grouping. > > That case also comes with a fair amount of comments about each group > for the extable entries. > > But yes, we also do have a number of thos e"sequential labels". See > for example arch/x86/lib/getuser.S, where we then end up having all > the exception handling at the end because it is mostly shared across > cases. It's ugly. > > We also have a lot of ugly cases that probably shouldn't use numbers > at all, eg csum_partial(). I think that goes back to some darker age > when things like "assembly is so trivial that it doesn't need any > fancy explanatory labels for code" was ok. > > See also arch/x86/lib/memmove_64.S for similar horrors. I wonder if it > is a case of "use compiler to get almost the right code, then massage > things manually". Nasty, nasty. That should use legible names, not > random numbers. > > I also suspect some people really thought that the numbers need to be > unique, and just didn't know to use local numbering. > That was bad example, I meant stuff was already *unique* in other files and it is sequential in some of them. In the very func I'm modifying here there is 0/1 followed by 2/3 pair already, so it looked like the convention to follow. Anyhow this is bullshit detail I'm not going to argue about, you made your position clear and I see no problem adhering to it -- consider this bit patched in v2. Can we drop this aspect please ;) -- Mateusz Guzik <mjguzik gmail.com>
