| Message ID | 7be4c6d7-4da1-43bb-b081-522a8339fd99@web.de (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | sysctl: Reduce dput(child) calls in proc_sys_fill_cache() | expand |
On Thu, Sep 26, 2024 at 10:20:34AM +0200, Markus Elfring wrote: > From: Markus Elfring <elfring@users.sourceforge.net> > Date: Thu, 26 Sep 2024 10:10:33 +0200 > > A dput(child) call was immediately used after an error pointer check > for a d_splice_alias() call in this function implementation. > Thus call such a function instead directly before the check. This message reads funny, please re-write for your v2. Here is how I would write it. " Replace two dput(child) calls with one that occurs immediately before the IS_ERR evaluation. This is ok because dput gets called regardless of the value returned by IS_ERR(res). " > > This issue was transformed by using the Coccinelle software. How long is the coccinelle script? If it is a reasonable size, can you please append it to the commit message. If in doubt of what "reasonable" means, just share it to the list before doing your V2. > > Signed-off-by: Markus Elfring <elfring@users.sourceforge.net> > --- > fs/proc/proc_sysctl.c | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/fs/proc/proc_sysctl.c b/fs/proc/proc_sysctl.c > index d11ebc055ce0..97547de58218 100644 > --- a/fs/proc/proc_sysctl.c > +++ b/fs/proc/proc_sysctl.c > @@ -698,11 +698,11 @@ static bool proc_sys_fill_cache(struct file *file, > res = d_splice_alias(inode, child); > d_lookup_done(child); > if (unlikely(res)) { > - if (IS_ERR(res)) { > - dput(child); > - return false; > - } > dput(child); > + > + if (IS_ERR(res)) > + return false; > + > child = res; > } > } > -- > 2.46.1 >
>> A dput(child) call was immediately used after an error pointer check >> for a d_splice_alias() call in this function implementation. >> Thus call such a function instead directly before the check. > This message reads funny, please re-write for your v2. Here is how I would write > it. > > " > Replace two dput(child) calls with one that occurs immediately before the IS_ERR > evaluation. This is ok because dput gets called regardless of the value returned > by IS_ERR(res). > " Do you prefer the mentioned macro name over the wording “error pointer check”? >> This issue was transformed by using the Coccinelle software. > How long is the coccinelle script? … A related script for the semantic patch language was presented already according to the clarification approach “Generalising a transformation with SmPL?”. https://lore.kernel.org/kernel-janitors/300b5d1a-ab88-4548-91d2-0792bc15e15e@web.de/ https://lkml.org/lkml/2024/9/14/464 https://sympa.inria.fr/sympa/arc/cocci/2024-09/msg00004.html Will further development ideas evolve accordingly? Regards, Markus
On Wed, Oct 23, 2024 at 02:10:57PM +0200, Markus Elfring wrote: > >> A dput(child) call was immediately used after an error pointer check > >> for a d_splice_alias() call in this function implementation. > >> Thus call such a function instead directly before the check. > > This message reads funny, please re-write for your v2. Here is how I would write > > it. > > > > " > > Replace two dput(child) calls with one that occurs immediately before the IS_ERR > > evaluation. This is ok because dput gets called regardless of the value returned > > by IS_ERR(res). > > " > > Do you prefer the mentioned macro name over the wording “error pointer check”? yes. > > > >> This issue was transformed by using the Coccinelle software. > > How long is the coccinelle script? … > > A related script for the semantic patch language was presented already according to > the clarification approach “Generalising a transformation with SmPL?”. > https://lore.kernel.org/kernel-janitors/300b5d1a-ab88-4548-91d2-0792bc15e15e@web.de/ > https://lkml.org/lkml/2024/9/14/464 > https://sympa.inria.fr/sympa/arc/cocci/2024-09/msg00004.html There where several scripts in these links but non of them where too long. Can you please append the one you used for this patch to the commit message. Thx
diff --git a/fs/proc/proc_sysctl.c b/fs/proc/proc_sysctl.c index d11ebc055ce0..97547de58218 100644 --- a/fs/proc/proc_sysctl.c +++ b/fs/proc/proc_sysctl.c @@ -698,11 +698,11 @@ static bool proc_sys_fill_cache(struct file *file, res = d_splice_alias(inode, child); d_lookup_done(child); if (unlikely(res)) { - if (IS_ERR(res)) { - dput(child); - return false; - } dput(child); + + if (IS_ERR(res)) + return false; + child = res; } }