LKML Archive on lore.kernel.org
 help / color / Atom feed
From: Michal Hocko <mhocko@suse.com>
To: Oscar Salvador <osalvador@suse.de>
Cc: Mike Kravetz <mike.kravetz@oracle.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Vlastimil Babka <vbabka@suse.cz>,
	David Hildenbrand <david@redhat.com>,
	Muchun Song <songmuchun@bytedance.com>,
	linux-mm@kvack.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v7 3/7] mm,hugetlb: Clear HPageFreed outside of the lock
Date: Wed, 14 Apr 2021 12:32:58 +0200
Message-ID: <YHbE2q/Otrdx1cgK@dhcp22.suse.cz> (raw)
In-Reply-To: <20210414100147.GD20886@linux>

On Wed 14-04-21 12:01:47, Oscar Salvador wrote:
> On Wed, Apr 14, 2021 at 10:28:33AM +0200, Michal Hocko wrote:
> > You are right it doesn't do it there. But all struct pages, even those
> > that are allocated by the bootmem allocator should initialize its struct
> > pages. They would be poisoned otherwise, right? I would have to look at
> > the exact code path but IIRC this should be around the time bootmem
> > allocator state transitions to the page allocator.
> 
> Ok, you are right.
> struct pages are initialized a bit earlier through:
> 
> start_kernel
>  setup_arch
>   paging_init
>    zone_sizes_init
>     free_area_init
>      free_area_init_node
>       free_area_init_core
>        memmap_init_zone
>         memmap_init_range
>          __init_single_page
> 
> While the allocation of bootmem hugetlb happens
> 
> start_kernel
>  parse_args
>   ...
>    hugepages_setup
>     ...
>      hugetlb_hstate_alloc_pages
>       __alloc_bootmem_huge_page
> 
> which is after the setup_arch() call.

Thanks for pulling those paths. It is always painful to crawl that code.

> So by the time we get the page from __alloc_bootmem_huge_page(), fields are
> zeroed.
> I thought we might get in trouble because memblock_alloc_try_nid_raw() calls
> page_init_poison() which poisons the chunk with 0xff,e.g:
> 
> [    1.955471] boot: ffffffffffffffff ffffffffffffffff ffffffffffffffff ffffffffffffffff
> [    1.955476] boot: ffffffffffffffff ffffffffffffffff ffffffffffffffff ffffffffffffffff
> 
>  but it seems that does not the memmap struct page.

Well, to be precise it does the very same thing with memamp struct pages
but that is before the initialization code you have pointed out above.
In this context it just poisons the allocated content which is the GB
page storage.

> I checked, and when we get there in __alloc_bootmem_huge_page, page->private is
> still zeroed, so I guess it should be safe to assume that we do not really need
> to clear the flag in __prep_new_huge_page() routine?

It would be quite nasty if the struct pages content would be undefined.
Maybe that is possible but then I would rather stick the initialization
into __alloc_bootmem_huge_page.

Thanks!
-- 
Michal Hocko
SUSE Labs

  parent reply index

Thread overview: 40+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-04-13 10:47 [PATCH v7 0/7] Make alloc_contig_range handle Hugetlb pages Oscar Salvador
2021-04-13 10:47 ` [PATCH v7 1/7] mm,page_alloc: Bail out earlier on -ENOMEM in alloc_contig_migrate_range Oscar Salvador
2021-04-13 17:00   ` Mike Kravetz
2021-04-13 10:47 ` [PATCH v7 2/7] mm,compaction: Let isolate_migratepages_{range,block} return error codes Oscar Salvador
2021-04-13 17:52   ` Mike Kravetz
2021-04-14 11:54   ` David Hildenbrand
2021-04-15  8:42     ` Oscar Salvador
2021-04-13 10:47 ` [PATCH v7 3/7] mm,hugetlb: Clear HPageFreed outside of the lock Oscar Salvador
2021-04-13 13:23   ` Michal Hocko
2021-04-13 21:19     ` Mike Kravetz
2021-04-14  6:04       ` Michal Hocko
2021-04-14  7:41         ` Oscar Salvador
2021-04-14  8:28           ` Michal Hocko
2021-04-14 10:01             ` Oscar Salvador
2021-04-14 10:03               ` Oscar Salvador
2021-04-14 10:32               ` Michal Hocko [this message]
2021-04-14 10:49                 ` Oscar Salvador
2021-04-14 11:09                   ` Michal Hocko
2021-04-14 12:02                     ` David Hildenbrand
2021-04-14 16:45                   ` Mike Kravetz
2021-04-13 10:47 ` [PATCH v7 4/7] mm,hugetlb: Split prep_new_huge_page functionality Oscar Salvador
2021-04-13 13:24   ` Michal Hocko
2021-04-13 13:26     ` Michal Hocko
2021-04-13 21:33   ` Mike Kravetz
2021-04-14  4:59     ` Oscar Salvador
2021-04-14 12:15       ` David Hildenbrand
2021-04-14 17:03       ` Mike Kravetz
2021-04-13 10:47 ` [PATCH v7 5/7] mm: Make alloc_contig_range handle free hugetlb pages Oscar Salvador
2021-04-13 13:40   ` Michal Hocko
2021-04-15  8:29     ` Oscar Salvador
2021-04-13 22:29   ` Mike Kravetz
2021-04-14  4:54     ` Oscar Salvador
2021-04-14 12:26   ` David Hildenbrand
2021-04-15  8:28     ` Oscar Salvador
2021-04-13 10:47 ` [PATCH v7 6/7] mm: Make alloc_contig_range handle in-use " Oscar Salvador
2021-04-13 22:48   ` Mike Kravetz
2021-04-14  4:52     ` Oscar Salvador
2021-04-14 17:07       ` Mike Kravetz
2021-04-13 10:47 ` [PATCH v7 7/7] mm,page_alloc: Drop unnecessary checks from pfn_range_valid_contig Oscar Salvador
2021-04-13 22:53   ` Mike Kravetz

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=YHbE2q/Otrdx1cgK@dhcp22.suse.cz \
    --to=mhocko@suse.com \
    --cc=akpm@linux-foundation.org \
    --cc=david@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mike.kravetz@oracle.com \
    --cc=osalvador@suse.de \
    --cc=songmuchun@bytedance.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

LKML Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/lkml/0 lkml/git/0.git
	git clone --mirror https://lore.kernel.org/lkml/1 lkml/git/1.git
	git clone --mirror https://lore.kernel.org/lkml/2 lkml/git/2.git
	git clone --mirror https://lore.kernel.org/lkml/3 lkml/git/3.git
	git clone --mirror https://lore.kernel.org/lkml/4 lkml/git/4.git
	git clone --mirror https://lore.kernel.org/lkml/5 lkml/git/5.git
	git clone --mirror https://lore.kernel.org/lkml/6 lkml/git/6.git
	git clone --mirror https://lore.kernel.org/lkml/7 lkml/git/7.git
	git clone --mirror https://lore.kernel.org/lkml/8 lkml/git/8.git
	git clone --mirror https://lore.kernel.org/lkml/9 lkml/git/9.git
	git clone --mirror https://lore.kernel.org/lkml/10 lkml/git/10.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 lkml lkml/ https://lore.kernel.org/lkml \
		linux-kernel@vger.kernel.org
	public-inbox-index lkml

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-kernel


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git