Message-ID: <87sgok3rs8.fsf@x220.int.ebiederm.org>
Date:   Wed, 25 Sep 2019 18:00:39 -0500
From:   ebiederm@...ssion.com (Eric W. Biederman)
To:     Ingo Molnar <mingo@...nel.org>
Cc:     Greg KH <gregkh@...uxfoundation.org>,
        Linus Torvalds <torvalds@...ux-foundation.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Peter Zijlstra <a.p.zijlstra@...llo.nl>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Borislav Petkov <bp@...en8.de>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Jonathan Corbet <corbet@....net>
Subject: Re: [Tree, v2] De-clutter the top level directory, move ipc/ => kernel/ipc/, samples/ => Documentation/samples/ and sound/ => drivers/sound/

Ingo Molnar <mingo@...nel.org> writes:

> * Eric W. Biederman <ebiederm@...ssion.com> wrote:
>
>> Ingo Molnar <mingo@...nel.org> writes:
>> 
>> >  - Split it into finer grained steps (3 instead of 2 patches per 
>> >    movement), for easier review and bisection testing:
>> >
>> >      toplevel: Move ipc/ to kernel/ipc/: move the files
>> >      toplevel: Move ipc/ to kernel/ipc/: adjust the build system
>> >      toplevel: Move ipc/ to kernel/ipc/: adjust comments and documentation
>> 
>> Can we not mess with ipc/ please.
>> 
>> I know that will mess with my muscle memory and I don't see the point.
>> Especially as long as it is named ipc and not sysvipc.
>> 
>> A half cleanup really looks worse than a real cleanup.
>> 
>> SysV IPC really is a side car on the kernel and on unix in general
>> and having the directory structure reflect that seems completely sensible.
>
> While such objections are I think valid for scripts/, the situation is 
> very different for ipc/:
>
>  - ipc/ is a tiny subsystem of just ~9k lines of code, and it's in the 
>    top level directory for historical reasons.
>
>  - ipc/ is an established subsystem of old interfaces with comparatively 
>    very few changes these days: there were just about 16 commits added in 
>    the last 12 months that changed the code and had 'ipc' in the title. 
>    Somewhat ironically, the biggest commit of all was a "system call 
>    renaming" commit:
>
>      275f22148e87: ipc: rename old-style shmctl/semctl/msgctl syscalls
>      14 files changed, 137 insertions(+), 62 deletions(-)
>
>    Many of the remaining 15 commits were simple in nature - and there 
>    were 9 different authors, i.e. the per author frequency of changes for 
>    ipc/ is even lower: around one per year for those 9 developers who are 
>    interested in ipc/ changes. I doubt there's much muscle memory even 
>    for them as a result.
>

*snort*  Given how long some of those changes took to review.  No they
were not all simple.   Maybe the final result was but the amount of work
was not simple.
 
>  - The 'muscle memory' argument has to be weighted by probability of 
>    interest (linecount), probability of change (frequency of commits) and 
>    number of authors. These factors add up to a very low change frequency 
>    for ipc/. We've moved and consolidated much, much bigger and higher 
>    frequency subsystems in the recent past, such as kernel/sched/ or 
>    kernel/locking/.

With I presume with the consent and the input of the developers of those
subsystems.  Quite frankly digging into kernel/sched to understand what
is happening right now is a real mess.  It has not made the code any
easier to understand (from my outside perspective).  But if the
developers of the subsystem are fine with it that is their lookout.

>  - The ipc/ location is arguably random and idiosynchratic - it's a 
>    leftover from old times nobody really bothered to clean up - but that 
>    fact shouldn't be a permanent barrier IMO.

Most of the kernel interfaces are random and idiosyncratic.  Most of the
files and most of their contents as well.  That doesn't make them wrong.
It does mean that changing them for the sake of change just increases
the cognitive load on people who have to look after things like that for
no reason.

I figure I have some say in the matter because I am one of the
developers doing some of that looking after.

>  - While uncluttering the top level directory for aesthethic and 
>    documentation reasons is one technical factor, there's another reason 
>    for my ipc/ movement: I have the secret hope to be able to move init/ 
>    to kernel/init/ as well, in which case there's a big muscle memory 
>    advantage for the future: 'i<tab>' would expand to include/ in a 
>    single step! :-) Now *that* is perhaps the highest frequency muscle 
>    memory location in the kernel. ;-)


> Or looking at it from another angle: if we applied your ipc/ benchmark 
> then basically almost *nothing* could be moved from the toplevel 
> directory or any other location, pretty much *ever*.

Please if it ain't broke don't fix it.


On the other hand we still have a terrible mess from the introduction of
threads into the kernel in the 2.5 era.  The cleanup code for tasks is
in terrible shape.  By terrible shape I mean we have data that should be
in signal_struct sill in task_struct, and we have code in release_task
(called when we reap (aka wait on a zombie)) that can very reasonably be
run much sooner, and free up resources in a timely manner.  The worst
part are zombie thread group leaders that are essentially untested in
practice but are a recurring source of errors in kernel code.

Want to help me clean up that code and get rid of zombie thread group
leaders?

Especially if you aren't interested in helping please stay away from the
code I feel responsible for so I can focus on picking up the messes like
that.

Eric

Powered by blists - more mailing lists