From: "Kirill A. Shutemov" <kirill@shutemov.name>
To: Wei Yang <richardw.yang@linux.intel.com>
Cc: hannes@cmpxchg.org, mhocko@kernel.org, vdavydov.dev@gmail.com,
akpm@linux-foundation.org, cgroups@vger.kernel.org,
linux-mm@kvack.org, linux-kernel@vger.kernel.org,
kirill.shutemov@linux.intel.com, yang.shi@linux.alibaba.com,
alexander.duyck@gmail.com, rientjes@google.com
Subject: Re: [Patch v2] mm: thp: grab the lock before manipulation defer list
Date: Mon, 13 Jan 2020 01:57:18 +0300 [thread overview]
Message-ID: <20200112225718.5vqzezfclacujyx3@box> (raw)
In-Reply-To: <20200112022858.GA17733@richard>
On Sun, Jan 12, 2020 at 10:28:58AM +0800, Wei Yang wrote:
> On Sat, Jan 11, 2020 at 03:03:52AM +0300, Kirill A. Shutemov wrote:
> >On Thu, Jan 09, 2020 at 10:30:54PM +0800, Wei Yang wrote:
> >> As all the other places, we grab the lock before manipulate the defer list.
> >> Current implementation may face a race condition.
> >>
> >> For example, the potential race would be:
> >>
> >> CPU1 CPU2
> >> mem_cgroup_move_account split_huge_page_to_list
> >> !list_empty
> >> lock
> >> !list_empty
> >> list_del
> >> unlock
> >> lock
> >> # !list_empty might not hold anymore
> >> list_del_init
> >> unlock
> >
> >I don't think this particular race is possible. Both parties take page
> >lock before messing with deferred queue, but anytway:
>
> I am afraid not. Page lock is per page, while defer queue is per pgdate or
> memcg.
>
> It is possible two page in the same pgdate or memcg grab page lock
> respectively and then access the same defer queue concurrently.
Look closer on the list_empty() argument. It's list_head local to the
page. Too different pages can be handled in parallel without any problem
in this particular scenario. As long as we as we modify it under the lock.
Said that, page lock here was somewhat accidential and I still belive we
need to move the check under the lock anyway.
--
Kirill A. Shutemov
next prev parent reply other threads:[~2020-01-12 22:57 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-01-09 14:30 Wei Yang
2020-01-09 18:52 ` David Rientjes
2020-01-09 18:58 ` Michal Hocko
2020-01-11 0:03 ` Kirill A. Shutemov
2020-01-12 2:28 ` Wei Yang
2020-01-12 22:57 ` Kirill A. Shutemov [this message]
2020-01-13 0:44 ` Wei Yang
2020-01-13 7:36 ` Kirill A. Shutemov
2020-01-13 8:23 ` Wei Yang
2020-01-14 9:31 ` Michal Hocko
2020-01-14 10:31 ` Kirill A. Shutemov
2020-01-14 10:59 ` Kirill A. Shutemov
2020-01-14 20:57 ` David Rientjes
2020-01-15 1:19 ` Wei Yang
2020-01-15 1:07 ` Wei Yang
2020-01-15 3:26 ` David Rientjes
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=20200112225718.5vqzezfclacujyx3@box \
--to=kirill@shutemov.name \
--cc=akpm@linux-foundation.org \
--cc=alexander.duyck@gmail.com \
--cc=cgroups@vger.kernel.org \
--cc=hannes@cmpxchg.org \
--cc=kirill.shutemov@linux.intel.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=mhocko@kernel.org \
--cc=richardw.yang@linux.intel.com \
--cc=rientjes@google.com \
--cc=vdavydov.dev@gmail.com \
--cc=yang.shi@linux.alibaba.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox