Can't seem to find a maintainer for init/* files

Can't seem to find a maintainer for init/* files

[Paulo Almeida]

[-- Attachment #1.1: Type: text/plain, Size: 994 bytes --]

Hi all,

I was reading the KernelJanitor/Todo webpage and found the printk-related
task that had to be done.

I eventually came across this piece of code that led me to 2 questions that
I couldn't answer myself
https://github.com/torvalds/linux/blame/master/init/do_mounts.c#L434-L455

1 - This specific code block has been around for quite some time and many
additions using the correct printk(KERN_* were made after it was written.
Does that mean that this code block is an exception and should be left
as-is for some technical reason? Or, people have somehow forgotten about it
and I finally found something to do? :)

2 - I took a look at the https://www.kernel.org/doc/linux/MAINTAINERS list
and their respective folders/repos and couldn't find out who would be the
person who I should eventually send the patches to. Is there any
"fallthrough" maintainer when there isn't one specified? (Please put me
straight if I'm not seeing things from the right angle)

Best regards,

Paulo Almeida

[-- Attachment #1.2: Type: text/html, Size: 1355 bytes --]

[-- Attachment #2: Type: text/plain, Size: 170 bytes --]

_______________________________________________
Kernelnewbies mailing list
Kernelnewbies@kernelnewbies.org
https://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies

Re: Can't seem to find a maintainer for init/* files

[Paulo Almeida]

[-- Attachment #1.1: Type: text/plain, Size: 1447 bytes --]

I believe I just found the answer for point number 2:

THE REST
M: Linus Torvalds <torvalds@linux-foundation.org>
L: linux-kernel@vger.kernel.org
Q: http://patchwork.kernel.org/project/LKML/list/
T: git git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
S: Buried alive in reporters
F: *
F: */


On Sat, Oct 19, 2019 at 10:33 AM Paulo Almeida <
paulo.miguel.almeida.rodenas@gmail.com> wrote:

> Hi all,
>
> I was reading the KernelJanitor/Todo webpage and found the printk-related
> task that had to be done.
>
> I eventually came across this piece of code that led me to 2 questions
> that I couldn't answer myself
> https://github.com/torvalds/linux/blame/master/init/do_mounts.c#L434-L455
>
> 1 - This specific code block has been around for quite some time and many
> additions using the correct printk(KERN_* were made after it was written.
> Does that mean that this code block is an exception and should be left
> as-is for some technical reason? Or, people have somehow forgotten about it
> and I finally found something to do? :)
>
> 2 - I took a look at the https://www.kernel.org/doc/linux/MAINTAINERS list
> and their respective folders/repos and couldn't find out who would be the
> person who I should eventually send the patches to. Is there any
> "fallthrough" maintainer when there isn't one specified? (Please put me
> straight if I'm not seeing things from the right angle)
>
> Best regards,
>
> Paulo Almeida
>
>

[-- Attachment #1.2: Type: text/html, Size: 2402 bytes --]

[-- Attachment #2: Type: text/plain, Size: 170 bytes --]

_______________________________________________
Kernelnewbies mailing list
Kernelnewbies@kernelnewbies.org
https://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies

Re: Can't seem to find a maintainer for init/* files

[Greg KH]

On Sat, Oct 19, 2019 at 10:33:00AM +1300, Paulo Almeida wrote:
> Hi all,
> 
> I was reading the KernelJanitor/Todo webpage and found the printk-related
> task that had to be done.
> 
> I eventually came across this piece of code that led me to 2 questions that
> I couldn't answer myself
> https://github.com/torvalds/linux/blame/master/init/do_mounts.c#L434-L455
> 
> 1 - This specific code block has been around for quite some time and many
> additions using the correct printk(KERN_* were made after it was written.
> Does that mean that this code block is an exception and should be left
> as-is for some technical reason? Or, people have somehow forgotten about it
> and I finally found something to do? :)
> 
> 2 - I took a look at the https://www.kernel.org/doc/linux/MAINTAINERS list
> and their respective folders/repos and couldn't find out who would be the
> person who I should eventually send the patches to. Is there any
> "fallthrough" maintainer when there isn't one specified? (Please put me
> straight if I'm not seeing things from the right angle)

Run scripts/get_maintainer.pl on any file and it will tell you where to
send changes to:
	$ ./scripts/get_maintainer.pl --file init/do_mounts.c
	Al Viro <viro@zeniv.linux.org.uk> (commit_signer:7/9=78%,authored:5/9=56%,added_lines:7/44=16%,removed_lines:27/32=84%)
	David Howells <dhowells@redhat.com> (commit_signer:3/9=33%,authored:2/9=22%,added_lines:5/44=11%,removed_lines:5/32=16%)
	Andrew Morton <akpm@linux-foundation.org> (commit_signer:2/9=22%)
	Nikolaus Voss <nikolaus.voss@loewensteinmedical.de> (commit_signer:1/9=11%,authored:1/9=11%,added_lines:31/44=70%)
	Thomas Gleixner <tglx@linutronix.de> (commit_signer:1/9=11%,authored:1/9=11%)
	linux-kernel@vger.kernel.org (open list)

But don't do janitorial cleanup on those files as a "first task".  That
is what the drivers/staging/ directory is for, don't mess with core
kernel files unless you have experience doing kernel changes, as the
learning curve needs to be gotten over first.

good luck!

greg k-h

_______________________________________________
Kernelnewbies mailing list
Kernelnewbies@kernelnewbies.org
https://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies

Re: Can't seem to find a maintainer for init/* files

[Paulo Almeida]

[-- Attachment #1.1: Type: text/plain, Size: 1407 bytes --]


> On 19/10/2019, at 11:32 AM, Greg KH <greg@kroah.com> wrote:
> 
> Run scripts/get_maintainer.pl on any file and it will tell you where to
> send changes to:
> 	$ ./scripts/get_maintainer.pl --file init/do_mounts.c
> 	Al Viro <viro@zeniv.linux.org.uk <mailto:viro@zeniv.linux.org.uk>> (commit_signer:7/9=78%,authored:5/9=56%,added_lines:7/44=16%,removed_lines:27/32=84%)
> 	David Howells <dhowells@redhat.com <mailto:dhowells@redhat.com>> (commit_signer:3/9=33%,authored:2/9=22%,added_lines:5/44=11%,removed_lines:5/32=16%)
> 	Andrew Morton <akpm@linux-foundation.org <mailto:akpm@linux-foundation.org>> (commit_signer:2/9=22%)
> 	Nikolaus Voss <nikolaus.voss@loewensteinmedical.de <mailto:nikolaus.voss@loewensteinmedical.de>> (commit_signer:1/9=11%,authored:1/9=11%,added_lines:31/44=70%)
> 	Thomas Gleixner <tglx@linutronix.de <mailto:tglx@linutronix.de>> (commit_signer:1/9=11%,authored:1/9=11%)
> 	linux-kernel@vger.kernel.org <mailto:linux-kernel@vger.kernel.org> (open list)
> 
> But don't do janitorial cleanup on those files as a "first task".  That
> is what the drivers/staging/ directory is for, don't mess with core
> kernel files unless you have experience doing kernel changes, as the
> learning curve needs to be gotten over first.

Hi Greg,

I appreciate your reply and I will take your piece of advice on what to do as the first task.

Best regards,

Paulo Almeida

[-- Attachment #1.2: Type: text/html, Size: 17901 bytes --]

[-- Attachment #2: Type: text/plain, Size: 170 bytes --]

_______________________________________________
Kernelnewbies mailing list
Kernelnewbies@kernelnewbies.org
https://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies

Re: Can't seem to find a maintainer for init/* files

[Valdis Klētnieks]

[-- Attachment #1.1: Type: text/plain, Size: 1989 bytes --]

On Sat, 19 Oct 2019 10:33:00 +1300, Paulo Almeida said:

> 1 - This specific code block has been around for quite some time and many
> additions using the correct printk(KERN_* were made after it was written.
> Does that mean that this code block is an exception and should be left
> as-is for some technical reason? Or, people have somehow forgotten about it
> and I finally found something to do? :)

There's a meta-consideration or two here to think about.

First, many maintainers are not thrilled with trivial patches to code,
especially checkpatch cleanups.  That's because those patches fall into two
major categories:

The patch is against code that's debugged and rock solid stable.  Most of
do_mounts.c is close to a decade old, and it's only being changed when it's
needed to add an actual feature (such as mounting by partition label in 2018
or mounting a CIFS filesystem this year).  And we *have* had what looked like
"trivial checkpatch cleanup" patches that were buggy and broke stuff.

The other category is "patches against code that's being worked on".  If it's
something that somebody else is working on, it can cause merge conflicts, which
make maintainers grumpy.  So the maintainer only wants to see those cleanups if
they're by the person who's working on the code, at the front of the patch
series, so that (presumably) they don't have merge commits and they've gotten
some compile and run testing.

The other big consideration is git.  Yes, git knows where and when every single
line of code came from.  That doesn't mean it's always easy to get it to cough
up information.

For example:   'git blame init/do_mounts.c'.  That tells you where each line came from.
Now... imagine a commit that did a spaces-to-tabs cleanup on lines 249 to 257.
git blame' now lists the cleanup commit, not the 6 commits that added the original code.

Exercise for the reader:  Determine the easiest way to get 'git blame' to show you
the original 6 commits rather than the cleanup.

[-- Attachment #1.2: Type: application/pgp-signature, Size: 832 bytes --]

[-- Attachment #2: Type: text/plain, Size: 170 bytes --]

_______________________________________________
Kernelnewbies mailing list
Kernelnewbies@kernelnewbies.org
https://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies

Re: Can't seem to find a maintainer for init/* files

[Paulo Miguel Almeida]

On Fri, Oct 18, 2019 at 07:25:12PM -0400, Valdis Klētnieks wrote:
> On Sat, 19 Oct 2019 10:33:00 +1300, Paulo Almeida said:
> 
> > 1 - This specific code block has been around for quite some time and many
> > additions using the correct printk(KERN_* were made after it was written.
> > Does that mean that this code block is an exception and should be left
> > as-is for some technical reason? Or, people have somehow forgotten about it
> > and I finally found something to do? :)
> 
> There's a meta-consideration or two here to think about.
> 
> First, many maintainers are not thrilled with trivial patches to code,
> especially checkpatch cleanups.  That's because those patches fall into two
> major categories:
> 
> The patch is against code that's debugged and rock solid stable.  Most of
> do_mounts.c is close to a decade old, and it's only being changed when it's
> needed to add an actual feature (such as mounting by partition label in 2018
> or mounting a CIFS filesystem this year).  And we *have* had what looked like
> "trivial checkpatch cleanup" patches that were buggy and broke stuff.
> 
> The other category is "patches against code that's being worked on".  If it's
> something that somebody else is working on, it can cause merge conflicts, which
> make maintainers grumpy.  So the maintainer only wants to see those cleanups if
> they're by the person who's working on the code, at the front of the patch
> series, so that (presumably) they don't have merge commits and they've gotten
> some compile and run testing.
> 

That makes perfect sense. 

> The other big consideration is git.  Yes, git knows where and when every single
> line of code came from.  That doesn't mean it's always easy to get it to cough
> up information.

I must confess that prior to this "challenge" of yours, I hadn't realised how tricky
git can become to get it to do certain things.  

> 
> For example:   'git blame init/do_mounts.c'.  That tells you where each line came from.
> Now... imagine a commit that did a spaces-to-tabs cleanup on lines 249 to 257.
> git blame' now lists the cleanup commit, not the 6 commits that added the original code.
> 
> Exercise for the reader:  Determine the easiest way to get 'git blame' to show you
> the original 6 commits rather than the cleanup.

Considering that I'm technically a reader :) I'm gonna start with my solution and
if anyone else wants to add to it (or replace it altogether) feel free to do so.

What I got so far is:

L_START=249
L_FINISH=257
FILE=init/do_mounts.c; 

for commit in $(git log --format='%h' --no-patch -L$L_START,$L_FINISH:$FILE | cat | tail -n +2)
do 	
    echo "Showing commit: $commit of file: $FILE"
    git blame -n -L$L_START,$L_FINISH $commit -- $FILE
done

PS.: (For future readers)  That may take a while.

I feel like this isn't right per se as the "git log -L" doesn't support "--follow" 
argument that would impact our ability to make it cough up the info we wanted in case the
file had been renamed. However, on the other hand, it would (to my understanding) make
its best effort to keep track of those lines in the log history. (Correct me if I'm wrong)

In a second approach:
I tried making "git log" to list all the commits this particular file was involved in
(so I could use --follow) but I ended up with loads of commits that change other sections 
of the file (not the lines we were looking for) and because of that I didn't feel like I 
was meeting the "'git blame' to show you the original 6 commits rather than the 
cleanup" rule.

@Valdis I hope I'm not going off-topic but could you shed some light on this?

Last but not least, I appriciate the meta-consideration points you brought up.

Best Regards,

Paulo Almeida



_______________________________________________
Kernelnewbies mailing list
Kernelnewbies@kernelnewbies.org
https://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies

Re: Can't seem to find a maintainer for init/* files

[Valdis Klētnieks]

[-- Attachment #1.1: Type: text/plain, Size: 775 bytes --]

On Tue, 22 Oct 2019 01:37:33 +1300, Paulo Miguel Almeida said:

> In a second approach:
> I tried making "git log" to list all the commits this particular file was involved in
> (so I could use --follow) but I ended up with loads of commits that change other sections
> of the file (not the lines we were looking for) and because of that I didn't feel like I
> was meeting the "'git blame' to show you the original 6 commits rather than the
> cleanup" rule.

Hint 1:  'git log' supports --no-merges which can simplify things sometimes.

Hint 2:   When specifying a commit, there is a ~ operator that can be used to advantage here.

So once you figure out which commit did the whitespace patching, it's easy to get
git blame to cough up what the tree looked like just before.

[-- Attachment #1.2: Type: application/pgp-signature, Size: 832 bytes --]

[-- Attachment #2: Type: text/plain, Size: 170 bytes --]

_______________________________________________
Kernelnewbies mailing list
Kernelnewbies@kernelnewbies.org
https://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies