Re: [PATCH v7 3/7] mm,hugetlb: Clear HPageFreed outside of the lock

2021-04-14 Thread Mike Kravetz
On 4/14/21 3:49 AM, Oscar Salvador wrote: > On Wed, Apr 14, 2021 at 12:32:58PM +0200, Michal Hocko wrote: >> 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 allo

Re: [PATCH v7 3/7] mm,hugetlb: Clear HPageFreed outside of the lock

2021-04-14 Thread David Hildenbrand
On 14.04.21 13:09, Michal Hocko wrote: On Wed 14-04-21 12:49:53, Oscar Salvador wrote: On Wed, Apr 14, 2021 at 12:32:58PM +0200, Michal Hocko wrote: [...] 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 w

Re: [PATCH v7 3/7] mm,hugetlb: Clear HPageFreed outside of the lock

2021-04-14 Thread Michal Hocko
On Wed 14-04-21 12:49:53, Oscar Salvador wrote: > On Wed, Apr 14, 2021 at 12:32:58PM +0200, Michal Hocko wrote: [...] > > > 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

Re: [PATCH v7 3/7] mm,hugetlb: Clear HPageFreed outside of the lock

2021-04-14 Thread Oscar Salvador
On Wed, Apr 14, 2021 at 12:32:58PM +0200, Michal Hocko wrote: > 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.

Re: [PATCH v7 3/7] mm,hugetlb: Clear HPageFreed outside of the lock

2021-04-14 Thread Michal Hocko
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 o

Re: [PATCH v7 3/7] mm,hugetlb: Clear HPageFreed outside of the lock

2021-04-14 Thread Oscar Salvador
On Wed, Apr 14, 2021 at 12:01:47PM +0200, Oscar Salvador wrote: > but it seems that does not the memmap struct page. that sould read as "but it seems that that does not affect the memmap struct page" -- Oscar Salvador SUSE L3

Re: [PATCH v7 3/7] mm,hugetlb: Clear HPageFreed outside of the lock

2021-04-14 Thread Oscar Salvador
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 cod

Re: [PATCH v7 3/7] mm,hugetlb: Clear HPageFreed outside of the lock

2021-04-14 Thread Michal Hocko
On Wed 14-04-21 09:41:32, Oscar Salvador wrote: > On Wed, Apr 14, 2021 at 08:04:21AM +0200, Michal Hocko wrote: > > On Tue 13-04-21 14:19:03, Mike Kravetz wrote: > > > On 4/13/21 6:23 AM, Michal Hocko wrote: > > > The only place where page->private may not be initialized is when we do > > > allocat

Re: [PATCH v7 3/7] mm,hugetlb: Clear HPageFreed outside of the lock

2021-04-14 Thread Oscar Salvador
On Wed, Apr 14, 2021 at 08:04:21AM +0200, Michal Hocko wrote: > On Tue 13-04-21 14:19:03, Mike Kravetz wrote: > > On 4/13/21 6:23 AM, Michal Hocko wrote: > > The only place where page->private may not be initialized is when we do > > allocations at boot time from memblock. In this case, we will ad

Re: [PATCH v7 3/7] mm,hugetlb: Clear HPageFreed outside of the lock

2021-04-13 Thread Michal Hocko
On Tue 13-04-21 14:19:03, Mike Kravetz wrote: > On 4/13/21 6:23 AM, Michal Hocko wrote: > > On Tue 13-04-21 12:47:43, Oscar Salvador wrote: [...] > > Or do we need it for giga pages which are not allocated by the page > > allocator? If yes then moving it to prep_compound_gigantic_page would be > >

Re: [PATCH v7 3/7] mm,hugetlb: Clear HPageFreed outside of the lock

2021-04-13 Thread Mike Kravetz
On 4/13/21 6:23 AM, Michal Hocko wrote: > On Tue 13-04-21 12:47:43, Oscar Salvador wrote: >> Currently, the clearing of the flag is done under the lock, but this >> is unnecessary as we just allocated the page and we did not give it >> away yet, so no one should be messing with it. >> >> Also, this

Re: [PATCH v7 3/7] mm,hugetlb: Clear HPageFreed outside of the lock

2021-04-13 Thread Michal Hocko
On Tue 13-04-21 12:47:43, Oscar Salvador wrote: > Currently, the clearing of the flag is done under the lock, but this > is unnecessary as we just allocated the page and we did not give it > away yet, so no one should be messing with it. > > Also, this helps making clear that here the lock is only