All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andrew Morton <akpm@linux-foundation.org>
To: Chen Wandun <chenwandun@huawei.com>
Cc: <0x7f454c46@gmail.com>, <wangkefeng.wang@huawei.com>,
	<weiyongjun1@huawei.com>, <linux-mm@kvack.org>,
	<linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] mm/mremap: fix memory account on do_munmap() failure
Date: Tue, 20 Jul 2021 16:28:10 -0700	[thread overview]
Message-ID: <20210720162810.e4710eebca48e9dc8ce2fa4d@linux-foundation.org> (raw)
In-Reply-To: <20210717101942.120607-1-chenwandun@huawei.com>

On Sat, 17 Jul 2021 18:19:42 +0800 Chen Wandun <chenwandun@huawei.com> wrote:

> mremap will account the delta between new_len and old_len in
> vma_to_resize, and then call move_vma when expanding an existing
> memory mapping. In function move_vma, there are two scenarios when
> calling do_munmap:
> 1. move_page_tables from old_addr to new_addr success
> 2. move_page_tables from old_addr to new_addr fail
> 
> In first scenario, it should account old_len if do_munmap fail,
> because the delta has already been accounted.
> 
> In second scenario, new_addr/new_len will assign to old_addr/old_len
> if move_page_table fail, so do_munmap is try to unmap new_addr actually,
> if do_munmap fail, it should account the new_len, because error code
> will be return from move_vma, and delta will be unaccounted.
> What'more, because of new_len == old_len, so account old_len also is
> OK.
> 
> In summary, account old_len will be correct if do_munmap fail.

Sorry, but I'm having trouble following that description.  Dmitry, could
you please review this change and then assist in clarifying the
changelog text?

Also, could it be argued that we're doing this in the wrong place? 
Should it be the responsibility of do_munmap() to fix up the accounting
if it is going to return an error?  Rather than expecting the
do_munmap() caller to fix up do_munmap()'s mess?

Thirdly, is the comment in there true?  Does this accounting error only
occur due to ENOMEM?  If that is the case then I am inclined not to
backport this fix into -stable kernels, as the error is so unlikely to
be triggered.  Thoughts on this?


      reply	other threads:[~2021-07-20 23:38 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-17 10:19 [PATCH] mm/mremap: fix memory account on do_munmap() failure Chen Wandun
2021-07-20 23:28 ` Andrew Morton [this message]

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=20210720162810.e4710eebca48e9dc8ce2fa4d@linux-foundation.org \
    --to=akpm@linux-foundation.org \
    --cc=0x7f454c46@gmail.com \
    --cc=chenwandun@huawei.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=wangkefeng.wang@huawei.com \
    --cc=weiyongjun1@huawei.com \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.