From: David Hildenbrand <david@redhat.com>
To: Charan Teja Kalla <quic_charante@quicinc.com>,
Pavan Kondeti <quic_pkondeti@quicinc.com>
Cc: akpm@linux-foundation.org, osalvador@suse.de,
dan.j.williams@intel.com, vbabka@suse.cz,
mgorman@techsingularity.net, aneesh.kumar@linux.ibm.com,
linux-mm@kvack.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] mm/sparsemem: fix race in accessing memory_section->usage
Date: Tue, 17 Oct 2023 16:53:46 +0200 [thread overview]
Message-ID: <17a890e2-a87b-446e-9088-bd608504378b@redhat.com> (raw)
In-Reply-To: <b4a2bbe8-4181-68ad-1a6c-463fbe64d8f8@quicinc.com>
On 17.10.23 16:10, Charan Teja Kalla wrote:
> Thanks Pavan!!
>
> On 10/16/2023 4:03 PM, Pavan Kondeti wrote:
>>> Fix this issue by the below steps:
>>> a) Clear SECTION_HAS_MEM_MAP before freeing the ->usage.
>>> b) RCU protected read side critical section will either return NULL when
>>> SECTION_HAS_MEM_MAP is cleared or can successfully access ->usage.
>>> c) Synchronize the rcu on the write side and free the ->usage. No
>>> attempt will be made to access ->usage after this as the
>>> SECTION_HAS_MEM_MAP is cleared thus valid_section() return false.
>>>
>>> Since the section_deactivate() is a rare operation and will come in the
>>> hot remove path, impact of synchronize_rcu() should be negligble.
>> struct mem_section_usage has other field like pageblock_flags. Do we
>> need to protect its readers with RCU? Also can we annotate usage field
>> in struct mem_section with __rcu and use RCU accessors like
>> rcu_dereference() while using memsection::usage field?
>
> Good question about the pageblock_flags!! I think we rely on the
> get_pageblock_bitmap() to read the ms->usage->pageblock_flags by passing
> struct page*.
>
> 1) All the functions that I have come across calling
> get_pageblock_bitmap()/get_pfnblock_flags_mask() passing the page* which
> it get from buddy. I think we are safe here as the device pages(from
> which the problem is coming will never be onlined/added to buddy)
>
> 2) There are functions in compaction which directly operate on the pfn's
> through pfn_to_online_page(). As for device pages, it is going to return
> NULL, I think we are safe here too.
>
> 3) alloc_contig_range() which also operate on the pfn's directly, and
> TMK, we will not pass the [start , end) values of this function to
> contains the hole/offlined pages. I think we are safe here too.
>
> May be David/other reviewers can help in commenting if there are some
> mistakes above.
Sound reasonable to me; most PFN walkers shouldn't deal with pageblock
flags. alloc_contig_range() is certainly interesting, I suspect it's
fine but we better double-check.
--
Cheers,
David / dhildenb
next prev parent reply other threads:[~2023-10-17 14:54 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-10-13 13:04 [PATCH] mm/sparsemem: fix race in accessing memory_section->usage Charan Teja Kalla
2023-10-14 22:25 ` Andrew Morton
2023-10-16 8:23 ` David Hildenbrand
2023-10-16 13:38 ` Charan Teja Kalla
2023-10-16 22:34 ` Andrew Morton
2023-10-18 7:52 ` David Hildenbrand
2023-10-16 10:33 ` Pavan Kondeti
2023-10-17 14:10 ` Charan Teja Kalla
2023-10-17 14:53 ` David Hildenbrand [this message]
2023-10-25 21:35 ` Andrew Morton
2023-10-26 7:00 ` David Hildenbrand
2023-10-26 7:18 ` Charan Teja Kalla
2024-01-15 18:44 ` Alexander Potapenko
2024-01-15 20:34 ` Marco Elver
2024-01-17 19:18 ` Marco Elver
2024-01-18 9:01 ` Alexander Potapenko
2024-01-18 9:43 ` Marco Elver
2024-01-25 13:20 ` Paul E. McKenney
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=17a890e2-a87b-446e-9088-bd608504378b@redhat.com \
--to=david@redhat.com \
--cc=akpm@linux-foundation.org \
--cc=aneesh.kumar@linux.ibm.com \
--cc=dan.j.williams@intel.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=mgorman@techsingularity.net \
--cc=osalvador@suse.de \
--cc=quic_charante@quicinc.com \
--cc=quic_pkondeti@quicinc.com \
--cc=vbabka@suse.cz \
/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;
as well as URLs for NNTP newsgroup(s).