Re: [PATCH] mm/sparsemem: fix race in accessing memory_section->usage

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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





[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Bugtraq]     [Linux OMAP]     [Linux MIPS]     [eCos]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux