linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: ebiederm@xmission.com (Eric W. Biederman)
To: Ingo Molnar <mingo@kernel.org>
Cc: Greg KH <gregkh@linuxfoundation.org>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	Peter Zijlstra <a.p.zijlstra@chello.nl>,
	Andrew Morton <akpm@linux-foundation.org>,
	Borislav Petkov <bp@alien8.de>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Jonathan Corbet <corbet@lwn.net>
Subject: Re: [Tree, v2] De-clutter the top level directory, move ipc/ => kernel/ipc/, samples/ => Documentation/samples/ and sound/ => drivers/sound/
Date: Wed, 25 Sep 2019 18:00:39 -0500	[thread overview]
Message-ID: <87sgok3rs8.fsf@x220.int.ebiederm.org> (raw)
In-Reply-To: <20190924184159.GA47127@gmail.com> (Ingo Molnar's message of "Tue, 24 Sep 2019 20:41:59 +0200")

Ingo Molnar <mingo@kernel.org> writes:

> * Eric W. Biederman <ebiederm@xmission.com> wrote:
>
>> Ingo Molnar <mingo@kernel.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


  reply	other threads:[~2019-09-25 23:01 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-09-17 19:26 [GIT PULL] Kselftest update for Linux 5.4-rc1 Shuah Khan
2019-09-20 16:17 ` Linus Torvalds
2019-09-20 16:26   ` Randy Dunlap
     [not found]   ` <CAKRRn-edxk9Du70A27V=d3Na73fh=fVvGEVsQRGROrQm05YRrA@mail.gmail.com>
2019-09-20 16:35     ` Brendan Higgins
2019-09-20 16:51       ` Linus Torvalds
2019-09-20 18:03         ` Brendan Higgins
2019-09-20 18:14           ` Linus Torvalds
2019-09-20 18:16             ` Brendan Higgins
2019-09-20 18:06         ` Shuah Khan
2019-09-22 11:25         ` Ingo Molnar
2019-09-22 11:52           ` Greg KH
2019-09-23 14:44             ` Shuah Khan
2019-09-23 19:43               ` Ingo Molnar
2019-09-23 19:52                 ` Randy Dunlap
2019-09-23 20:29                   ` Shuah Khan
2019-09-23 20:53                     ` Ingo Molnar
2019-09-23 21:11                       ` Shuah Khan
2019-09-23 20:08             ` [RFC, Tree] De-clutter the top level directory, move ipc/ => kernel/ipc/, samples/ => Documentation/samples/ and sound/ => drivers/sound/ Ingo Molnar
2019-09-24 11:31               ` [Tree, v2] " Ingo Molnar
2019-09-24 15:28                 ` Eric W. Biederman
2019-09-24 18:41                   ` Ingo Molnar
2019-09-25 23:00                     ` Eric W. Biederman [this message]
2019-09-23 23:54           ` [GIT PULL] Kselftest update for Linux 5.4-rc1 Tim.Bird
2019-09-24  8:07             ` Ingo Molnar
2019-09-26 12:52           ` David Sterba
2019-09-27 13:52           ` Pavel Machek
2019-10-03  9:08           ` Masahiro Yamada

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87sgok3rs8.fsf@x220.int.ebiederm.org \
    --to=ebiederm@xmission.com \
    --cc=a.p.zijlstra@chello.nl \
    --cc=akpm@linux-foundation.org \
    --cc=bp@alien8.de \
    --cc=corbet@lwn.net \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@kernel.org \
    --cc=tglx@linutronix.de \
    --cc=torvalds@linux-foundation.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).