linux-kernel - RE: Re: ftrace/module: Move ftrace_release_mod() to ddebug

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Tue, 12 Dec 2017 03:49:17 +0000
From:   AMIT SAHRAWAT <a.sahrawat@...sung.com>
To:     Steven Rostedt <rostedt@...dmis.org>,
        NAMIT GUPTA <gupta.namit@...sung.com>
CC:     "mingo@...hat.com" <mingo@...hat.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        PANKAJ MISHRA <pankaj.m@...sung.com>
Subject: RE: Re: ftrace/module: Move ftrace_release_mod() to ddebug_cleanup
 label

Hi Steven,

>> ftrace_module_init happen after dynamic_debug_setup, it is desired that
>> cleanup should be called after this label however in current implementation
>> it is called in free module label,ie:even though ftrace in not initialized,
>> from so many fail case ftrace_release_mod() will be called and unnecessary
>> traverse the whole list.
>> In below patch we moved ftrace_release_mod() from free_module label to
>> ddebug_cleanup label. that is the best possible location, other solution
>> is to make new label to ftrace_release_mod() but since ftrace_module_init()
>> is not return with minimum changes it should be in ddebug_cleanup label.
>> 
>> 
>> Signed-off-by: Namit Gupta <gupta.namit@...sung.com>
>> Reviewed-by: Amit Sahrawat <a.sahrawat@...sung.com>
> 
>Linus really looks down on this type of "Reviewed-by" tags. They are
>meaningless. When a patch first shows up on the mailing list, it should
>never have a "Reviewed-by" tag to it. Because to the maintainers, it
>has not had a chance to be reviewed. We don't care about internal
>company reviews. What should have happened, was this email goes to the
>mailing list, and then Amit can reply to that email with a
>"Reviewed-by", and any comments that Amit may have had. One reason that
>we dislike internal reviews, is that we don't know what was discussed.
>Since the review discussion is hidden, so should the tag.
> 

I fully agree to what you say and the overall perspective for the open source.
When i consider the situation, it was reviewed for our internal issue fixing and adopting 
to our kernel. And with respect to "internal reviews" it ended there itself.

After adopting it was realized to push this to open source as well.
Which as a policy is correct, but the submission method went wrong.
May be it is first patch from Namit and i did not notice due to our mail client.

>Since the email was missing a "[PATCH]" in the subject, that means Amit
>missed that too, and also takes the blame.

Yes, i did not notice that in my mailbox as well. 
It can just be put to my skipping of this mail as the code was finalized for internal purpose,
i did not notice that the external patch is having "Reviewed-by" and obviously the subject as you mentioned
[PATCH] was missing from it. I am aware of the guidelines and method for contribution, this particular patch submission can be put to my ignorance.
So, I take the blame for this as well.

Thanks & Regards,
Amit Sahrawat