linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Liam Howlett <liam.howlett@oracle.com>
To: Lukas Bulwahn <lukas.bulwahn@gmail.com>
Cc: "Matthew Wilcox (Oracle)" <willy@infradead.org>,
	"maple-tree@lists.infradead.org" <maple-tree@lists.infradead.org>,
	Linux-MM <linux-mm@kvack.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	kernel-janitors <kernel-janitors@vger.kernel.org>
Subject: Re: Issue in count_mm_mlocked_page_nr() with commit afcf76df48f9 ("mm/mlock: use vma iterator and maple state instead of vma linked list")
Date: Wed, 15 Jun 2022 15:01:27 +0000	[thread overview]
Message-ID: <20220615150119.6uvfxpvlm3fxmjex@revolver> (raw)
In-Reply-To: <CAKXUXMxs3waRA8pveFQ2LRMF5imoHxjQ0zfLS_4vpfr2YSLYPw@mail.gmail.com>

* Lukas Bulwahn <lukas.bulwahn@gmail.com> [220614 07:33]:
> Dear Matthew, dear Liam,
> 
> The commit afcf76df48f9 ("mm/mlock: use vma iterator and maple state instead
> of vma linked list") refactors the function count_mm_mlocked_page_nr() in
> mm/mlock.c, such that the special case:
> 
>         if (mm == NULL)
>                 mm = current->mm;
> 
> is now dead code. Before the refactoring, after this if statement, the variable
> mm was used further in vma = find_vma(mm, start) in the following line, but now,
> with the introduction of VMA_ITERATOR(vmi, mm, start) before the if statement,
> the assignment within the if statement is simply ignored.
> 
> I assume that:
> - If the semantics of this function shall be preserved, the VMA_ITERATOR macro
> needs to move after the if statement.
> 
> - or, if the consideration of the special case on mm == NULL is actually
> obsolete with this refactoring, it could be simply deleted.

I think this code was never necessary.  This statement was added in the
same commit that added the only user of the function [1], which passes
in current->mm as the argument.

> 
> I will keep it up to you how this issue is correctly resolved.
> 
> This was discovered due to tracking all dead stores that clang-analyzer reports
> on an x86 defconfig. The underlying issue here is more severe than "just a dead
> store in the kernel code base", but it is probably easy for you to resolve.
> 

Thanks for the report.  This code should be dropped.

[1] Commit ID 0cf2f6f6dc605

Regards,
Liam


  reply	other threads:[~2022-06-15 15:01 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-06-14 11:33 Lukas Bulwahn
2022-06-15 15:01 ` Liam Howlett [this message]
  -- strict thread matches above, loose matches on Subject: below --
2022-06-14 11:29 Lukas Bulwahn

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=20220615150119.6uvfxpvlm3fxmjex@revolver \
    --to=liam.howlett@oracle.com \
    --cc=akpm@linux-foundation.org \
    --cc=kernel-janitors@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=lukas.bulwahn@gmail.com \
    --cc=maple-tree@lists.infradead.org \
    --cc=willy@infradead.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