linux-kernel - Re: [PATCH v2] mm/oom_kill.c: remove the unmatched comments

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <20201027151156.GA4336@rlk>
Date:   Tue, 27 Oct 2020 23:11:56 +0800
From:   Hui Su <sh_def@....com>
To:     Michal Hocko <mhocko@...e.com>, akpm@...ux-foundation.org,
        linux-mm@...ck.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] mm/oom_kill.c: remove the unmatched comments

On Tue, Oct 27, 2020 at 03:58:14PM +0100, Michal Hocko wrote:
> On Tue 27-10-20 22:45:29, Hui Su wrote:
> > is_dump_unreclaim_slabs() just check whether nr_unreclaimable
> > slabs amount is greater than user memory, not match witch comment.
> 
> As I've tried to explain, the comment is not explaining what the
> function does but how it should be used. It is not a kerneldoc afterall.
> So it is a good match. I can see how that might confuse somebody so I am
> not against changing this but the changelog shouldn't really be
> confusing on its own. What do you think about the following instead.
> 

Hi, Michal:

Thanks for your fast reply, your changlog is much more accurate.

And should i resend a patch V3 use the changlog below?

Thanks.

> "
> Comment for is_dump_unreclaim_slabs is not really clear whether it is
> meant to instruct how to use the function or whether it is an outdated
> information of the past implementation of the function. it doesn't realy
> help that is_dump_unreclaim_slabs is hard to grasp on its own.
> Rename the helper to should_dump_unreclaim_slabs which should make it
> clear what it is meant to do and drop the comment as the purpose should
> be pretty evident now.
> "
>