Re: [PATCH 1/1] process_madvise.2: Add process_madvise man page

2021-01-25 Thread Michal Hocko
ssure. This is a non-destructive operation. Other than that, looks good to me from the content POV. Thanks! -- Michal Hocko SUSE Labs

Re: [PATCH v4 2/4] mm: failfast mode with __GFP_NORETRY in alloc_contig_range

2021-01-25 Thread Michal Hocko
On Mon 25-01-21 14:12:02, Michal Hocko wrote: > On Thu 21-01-21 09:55:00, Minchan Kim wrote: > > Contiguous memory allocation can be stalled due to waiting > > on page writeback and/or page lock which causes unpredictable > > delay. It's a unavoidable cost for

Re: [PATCH v4 1/4] mm: cma: introduce gfp flag in cma_alloc instead of no_warn

2021-01-25 Thread Michal Hocko
mory region for which the allocation is performed. > * @count: Requested number of pages. > * @align: Requested alignment of pages (in PAGE_SIZE order). > - * @no_warn: Avoid printing message about failed allocation > + * @gfp_mask: GFP mask to use during the cma allocation. Call out s

Re: [PATCH] Revert "mm: memcontrol: avoid workload stalls when lowering memory.high"

2021-01-25 Thread Michal Hocko
ain. > > Fixes: 536d3bf261a2 ("mm: memcontrol: avoid workload stalls when lowering > memory.high") > Cc: # 5.8+ > Reported-by: Tejun Heo > Signed-off-by: Johannes Weiner Acked-by: Michal Hocko Thanks for extending the changelog to describe the scenario in a mo

Re: [PATCH] mm/filemap: Adding missing mem_cgroup_uncharge() to __add_to_page_cache_locked()

2021-01-25 Thread Michal Hocko
On Mon 25-01-21 13:23:58, Waiman Long wrote: > On 1/25/21 1:14 PM, Michal Hocko wrote: [...] > > With the proposed simplification by Willy > > Acked-by: Michal Hocko > > Thank for the ack. However, I am a bit confused about what you mean by > simplification. There is

Re: [PATCH] mm/filemap: Adding missing mem_cgroup_uncharge() to __add_to_page_cache_locked()

2021-01-25 Thread Michal Hocko
On Mon 25-01-21 17:41:19, Michal Hocko wrote: > On Mon 25-01-21 16:25:06, Matthew Wilcox wrote: > > On Mon, Jan 25, 2021 at 05:03:28PM +0100, Michal Hocko wrote: > > > On Mon 25-01-21 10:57:54, Waiman Long wrote: > > > > On 1/25/21 4:28 AM, Michal Hocko wrote: >

Re: [PATCH v16 06/11] mm: introduce memfd_secret system call to create "secret" memory areas

2021-01-25 Thread Michal Hocko
, MAP_SIZE, PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0); I do not see any access control or permission model for this feature. Is this feature generally safe to anybody? -- Michal Hocko SUSE Labs

Re: [PATCH v16 08/11] secretmem: add memcg accounting

2021-01-25 Thread Michal Hocko
hen this is not a slab owned memory? Why do you use the kmem accounting API when __GFP_ACCOUNT should give you the same without this details? -- Michal Hocko SUSE Labs

Re: [PATCH] mm/filemap: Adding missing mem_cgroup_uncharge() to __add_to_page_cache_locked()

2021-01-25 Thread Michal Hocko
On Mon 25-01-21 16:25:06, Matthew Wilcox wrote: > On Mon, Jan 25, 2021 at 05:03:28PM +0100, Michal Hocko wrote: > > On Mon 25-01-21 10:57:54, Waiman Long wrote: > > > On 1/25/21 4:28 AM, Michal Hocko wrote: > > > > On Sun 24-01-21 23:24:41, Waiman Long wrote: > >

Re: [PATCH 1/3] kvfree_rcu: Allocate a page for a single argument

2021-01-25 Thread Michal Hocko
away might become a future burden to make any changes in this aspect later on due to lack of exact reasoning. General rule of thumb for __GFP_RETRY_MAYFAIL is really try as hard as it can get without being really disruptive (like OOM killing something). And your wording didn't really give me that impression. -- Michal Hocko SUSE Labs

Re: [PATCH] mm/filemap: Adding missing mem_cgroup_uncharge() to __add_to_page_cache_locked()

2021-01-25 Thread Michal Hocko
On Mon 25-01-21 10:57:54, Waiman Long wrote: > On 1/25/21 4:28 AM, Michal Hocko wrote: > > On Sun 24-01-21 23:24:41, Waiman Long wrote: > > > The commit 3fea5a499d57 ("mm: memcontrol: convert page > > > cache to a new mem_cgroup_charge() API") introduced a bu

Re: [PATCH 1/3] kvfree_rcu: Allocate a page for a single argument

2021-01-25 Thread Michal Hocko
emory allocator. __GFP_RETRY_MAYFAIL can be quite heavy. It is effectively the most heavy way to allocate without triggering the OOM killer. Is this really what you need/want? Is __GFP_NORETRY too weak? -- Michal Hocko SUSE Labs

Re: [PATCH v4 2/4] mm: failfast mode with __GFP_NORETRY in alloc_contig_range

2021-01-25 Thread Michal Hocko
68,7 @@ int alloc_contig_range(unsigned long start, unsigned > long end, > .nr_migratepages = 0, > .order = -1, > .zone = page_zone(pfn_to_page(start)), > - .mode = MIGRATE_SYNC, > + .mode = gfp_mask & __GFP_NORETRY ? MIGRATE_ASYN

Re: [PATCH v4 3/5] mm: Teach pfn_to_online_page() about ZONE_DEVICE section collisions

2021-01-20 Thread Michal Hocko
) for a full ZONE_DEVICE section returns > false. > > Because the collision case is rare, and for simplicity, the > SECTION_TAINT_ZONE_DEVICE flag is never cleared once set. > > Fixes: ba72b4c8cf60 ("mm/sparsemem: support sub-section hotplug") > Cc: Andrew Morton > Reported-by: Mi

Re: [PATCH v4 2/5] mm: Teach pfn_to_online_page() to consider subsection validity

2021-01-20 Thread Michal Hocko
tially offline memory (e.g. part of ZONE_DEVICE) then pfn_to_online_page would lead to a false positive on some pfns. Fix this by adding pfn_section_valid check which is subsection aware. " > > Fixes: b13bc35193d9 ("mm/hotplug: invalid PFNs from pfn_to_online_page()") >

Re: [PATCH v4 1/5] mm: Move pfn_to_online_page() out of line

2021-01-20 Thread Michal Hocko
On Wed 13-01-21 16:43:16, Dan Williams wrote: > pfn_to_online_page() is already too large to be a macro or an inline > function. In anticipation of further logic changes / growth, move it out > of line. > > No functional change, just code movement. > > Reported-by: Michal

Re: [PATCH] mm: memcontrol: skip propagate percpu vmstat values to current memcg

2021-01-20 Thread Michal Hocko
y > considered a hot path. It takes global locks and such... This is not the first time when an (micro)optimization is posted without any data showing benefit or otherwise appealing justification. Sigh. -- Michal Hocko SUSE Labs

Re: [PATCH] swap: Check nrexceptional of swap cache before being freed

2021-01-20 Thread Michal Hocko
On Wed 20-01-21 15:54:04, Huang, Ying wrote: > Michal Hocko writes: > > > On Wed 20-01-21 15:27:11, Huang Ying wrote: > >> To catch the error in updating the swap cache shadow entries or their > >> count. > > > > What is the error? > > There

Re: [PATCH v2] mm: page_counter: relayout structure to reduce false sharing

2021-01-20 Thread Michal Hocko
3924 +0.1% 3929fio.write_bw_MBps Thanks for making results easier to read and understand. > [1].https://lore.kernel.org/lkml/20201102091543.GM31092@shao2-debian/ > Signed-off-by: Feng Tang > Reviewed-by: Roman Gushchin > Cc: Johannes Weiner > Cc: Micha

Re: [PATCH] swap: Check nrexceptional of swap cache before being freed

2021-01-19 Thread Michal Hocko
Huang, Ying" > Cc: Minchan Kim > Cc: Joonsoo Kim , > Cc: Johannes Weiner , > Cc: Vlastimil Babka , Hugh Dickins , > Cc: Mel Gorman , > Cc: Michal Hocko , > Cc: Dan Williams , > Cc: Christoph Hellwig , Ilya Dryomov , > --- > mm/swap_state.c | 7 ++- > 1 file

Re: SLUB: percpu partial object count is highly inaccurate, causing some memory wastage and maybe also worse tail latencies?

2021-01-18 Thread Michal Hocko
On Mon 18-01-21 15:46:43, Cristopher Lameter wrote: > On Mon, 18 Jan 2021, Michal Hocko wrote: > > > > Hm this would be similar to recommending a periodical echo > drop_caches > > > operation. We actually discourage from that (and yeah, some tools do > > > th

Re: [PATCH] mm: memcontrol: prevent starvation when writing memory.high

2021-01-18 Thread Michal Hocko
On Fri 15-01-21 11:20:50, Johannes Weiner wrote: > On Wed, Jan 13, 2021 at 03:46:54PM +0100, Michal Hocko wrote: > > On Tue 12-01-21 11:30:11, Johannes Weiner wrote: > > > When a value is written to a cgroup's memory.high control file, the > > > write() context firs

Re: SLUB: percpu partial object count is highly inaccurate, causing some memory wastage and maybe also worse tail latencies?

2021-01-18 Thread Michal Hocko
should respond to memory > pressure and not OOM prematurely by itself, including SLUB. Absolutely agreed! Partial caches are a very deep internal implementation detail of the allocator and admin has no bussiness into fiddling with that. This would only lead to more harm than good. Comparision to drop_caches is really exact! -- Michal Hocko SUSE Labs

Re: [PATCH v6 3/5] mm: hugetlb: fix a race between freeing and dissolving the page

2021-01-18 Thread Michal Hocko
("mm: memory-hotplug: enable memory hotplug to handle > hugepage") > Signed-off-by: Muchun Song > Reviewed-by: Mike Kravetz > Cc: sta...@vger.kernel.org Acked-by: Michal Hocko Thanks! > --- > mm/hugetlb.c | 39 +++ > 1 file

Re: [PATCH v2] mm, oom: Fix a comment in dump_task

2021-01-18 Thread Michal Hocko
On Fri 15-01-21 22:23:14, Tang Yizhou wrote: > If p is a kthread, it will be checked in oom_unkillable_task() so > we can delete the corresponding comment. > > Signed-off-by: Tang Yizhou > Cc: David Rientjes > Cc: KOSAKI Motohiro > Cc: Shakeel Butt > Cc: Michal Hocko

Re: [External] Re: [PATCH v5 3/5] mm: hugetlb: fix a race between freeing and dissolving the page

2021-01-14 Thread Michal Hocko
On Thu 14-01-21 21:47:36, Muchun Song wrote: > On Thu, Jan 14, 2021 at 9:20 PM Michal Hocko wrote: [...] > > > @@ -1770,6 +1789,28 @@ int dissolve_free_huge_page(struct page *page) > > > int nid = page_to_nid(head); > > > if (h->fre

Re: [PATCH v5 3/5] mm: hugetlb: fix a race between freeing and dissolving the page

2021-01-14 Thread Michal Hocko
e that the waiter might get blocked on the spin lock while the concurrent freeing is happening. But maybe you wanted to avoid exactly this contention? Please put your thinking into the changelog. > + > + goto retry; > + } > + > /* >* Move PageHWPoison flag from head page to the raw error page, >* which makes any subpages rather than the error page reusable. > -- > 2.11.0 -- Michal Hocko SUSE Labs

Re: [PATCH] mm: memcontrol: prevent starvation when writing memory.high

2021-01-13 Thread Michal Hocko
t; + try_to_free_mem_cgroup_pages(memcg, nr_pages - high, > + GFP_KERNEL, true); > > - if (!reclaimed && !nr_retries--) > + if (!nr_retries--) > break; > } > > -- > 2.30.0 > -- Michal Hocko SUSE Labs

Re: [PATCH v2 1/1] mm/madvise: replace ptrace attach requirement for process_madvise

2021-01-13 Thread Michal Hocko
On Tue 12-01-21 09:51:24, Suren Baghdasaryan wrote: > On Tue, Jan 12, 2021 at 9:45 AM Oleg Nesterov wrote: > > > > On 01/12, Michal Hocko wrote: > > > > > > On Mon 11-01-21 09:06:22, Suren Baghdasaryan wrote: > > > > > > > What we want is th

Re: [PATCH v2 1/1] mm/madvise: replace ptrace attach requirement for process_madvise

2021-01-13 Thread Michal Hocko
On Tue 12-01-21 10:12:03, Suren Baghdasaryan wrote: > On Mon, Jan 11, 2021 at 11:46 PM Michal Hocko wrote: > > > > On Mon 11-01-21 09:06:22, Suren Baghdasaryan wrote: > > > process_madvise currently requires ptrace attach capability. > > > PTRACE_MODE_ATTACH g

Re: [External] Re: [PATCH v4 4/6] mm: hugetlb: retry dissolve page when hitting race

2021-01-13 Thread Michal Hocko
In this case, the cpu_relax() can > make sense. Right? The system would have to be under serious stress for WQ to clog. I do not expect there would be nothing runable at that stage. Possible? Maybe but if that matters than a short sleep would be more preferable than cpu_relax. -- Michal Hocko SUSE Labs

Re: [External] Re: [PATCH v4 4/6] mm: hugetlb: retry dissolve page when hitting race

2021-01-13 Thread Michal Hocko
On Wed 13-01-21 19:11:06, Muchun Song wrote: > On Wed, Jan 13, 2021 at 6:38 PM Michal Hocko wrote: [...] > > > I just want the fix patch to be small enough. > > > So I do the retry in this patch. If you do not agree with this. I > > > will fold this into the prev

Re: [External] Re: [PATCH v4 4/6] mm: hugetlb: retry dissolve page when hitting race

2021-01-13 Thread Michal Hocko
On Wed 13-01-21 18:14:55, Muchun Song wrote: > On Wed, Jan 13, 2021 at 5:33 PM Michal Hocko wrote: > > > > On Wed 13-01-21 13:22:07, Muchun Song wrote: > > > There is a race between dissolve_free_huge_page() and put_page(). > > > Theoretically, we should return -

Re: [PATCH v4 4/6] mm: hugetlb: retry dissolve page when hitting race

2021-01-13 Thread Michal Hocko
); > + cpu_relax(); > + } > + goto retry; OK, so you have done the retry here. Please fold it into the previous patch. Also do we need cpu_relax on top of cond_resched as well? > + } > > /* >* Move PageHWPoison flag from head page to the raw error page, > -- > 2.11.0 -- Michal Hocko SUSE Labs

Re: [PATCH v4 3/6] mm: hugetlb: fix a race between freeing and dissolving the page

2021-01-13 Thread Michal Hocko
is dissolved. > + */ > + if (unlikely(!PageHugeFreed(head))) > + goto out; I believe we have agreed to retry for this temporary state. > + > /* > * Move PageHWPoison flag from head page to the raw error page, >* which makes any subpages rather than the error page reusable. > -- > 2.11.0 -- Michal Hocko SUSE Labs

Re: [PATCH v3 1/6] mm: migrate: do not migrate HugeTLB page whose refcount is one

2021-01-12 Thread Michal Hocko
On Tue 12-01-21 15:41:02, David Hildenbrand wrote: > On 12.01.21 15:23, Michal Hocko wrote: > > On Tue 12-01-21 13:16:45, Michal Hocko wrote: > > [...] > >> Well, currently pool pages are not migrateable but you are right that > >> this is likely something that

Re: [PATCH v3 1/6] mm: migrate: do not migrate HugeTLB page whose refcount is one

2021-01-12 Thread Michal Hocko
On Tue 12-01-21 13:16:45, Michal Hocko wrote: [...] > Well, currently pool pages are not migrateable but you are right that > this is likely something that we will need to look into in the future > and this optimization would stand in the way. After some more thinking I believe I was wr

Re: [External] Re: [PATCH v3 3/6] mm: hugetlb: fix a race between freeing and dissolving the page

2021-01-12 Thread Michal Hocko
On Tue 12-01-21 19:43:21, Muchun Song wrote: > On Tue, Jan 12, 2021 at 7:17 PM Michal Hocko wrote: > > > > On Tue 12-01-21 18:13:02, Muchun Song wrote: > > > On Tue, Jan 12, 2021 at 6:02 PM Michal Hocko wrote: > > > > > > >

Re: [PATCH v3 1/6] mm: migrate: do not migrate HugeTLB page whose refcount is one

2021-01-12 Thread Michal Hocko
On Tue 12-01-21 12:34:01, David Hildenbrand wrote: > On 12.01.21 12:27, Michal Hocko wrote: > > On Tue 12-01-21 12:11:21, David Hildenbrand wrote: > >> On 12.01.21 12:00, David Hildenbrand wrote: > >>> On 10.01.21 13:40, Muchun Song wrote: > >>>> I

Re: [PATCH v3 1/6] mm: migrate: do not migrate HugeTLB page whose refcount is one

2021-01-12 Thread Michal Hocko
This is an interesting point. But do we try to migrate hugetlb pages in alloc_contig_range? isolate_migratepages_block !PageLRU need to be marked as PageMovable AFAICS. This would be quite easy to implement but a more fundamental question is whether we really want to mess with existing pools for alloc_contig_range. Anyway you are quite right that this change has more side effects than it is easy to see while it doesn't really bring any major advantage other than the code consistency. -- Michal Hocko SUSE Labs

Re: [External] Re: [PATCH v3 3/6] mm: hugetlb: fix a race between freeing and dissolving the page

2021-01-12 Thread Michal Hocko
On Tue 12-01-21 18:13:02, Muchun Song wrote: > On Tue, Jan 12, 2021 at 6:02 PM Michal Hocko wrote: > > > > On Sun 10-01-21 20:40:14, Muchun Song wrote: > > [...] > > > @@ -1770,6 +1788,14 @@ int dissolve_free_huge_page(struct page *page) > > >

Re: [External] Re: [PATCH v3 4/6] mm: hugetlb: add return -EAGAIN for dissolve_free_huge_page

2021-01-12 Thread Michal Hocko
On Tue 12-01-21 18:49:17, Muchun Song wrote: > On Tue, Jan 12, 2021 at 6:06 PM Michal Hocko wrote: > > > > On Tue 12-01-21 17:51:05, Muchun Song wrote: > > > On Tue, Jan 12, 2021 at 4:33 PM Michal Hocko wrote: > > > > > > > > On Mon 11-01-21 17:2

Re: [External] Re: [PATCH v3 4/6] mm: hugetlb: add return -EAGAIN for dissolve_free_huge_page

2021-01-12 Thread Michal Hocko
On Tue 12-01-21 17:51:05, Muchun Song wrote: > On Tue, Jan 12, 2021 at 4:33 PM Michal Hocko wrote: > > > > On Mon 11-01-21 17:20:51, Mike Kravetz wrote: > > > On 1/10/21 4:40 AM, Muchun Song wrote: > > > > There is a race between dissolve_free_huge_page()

Re: [PATCH v3 3/6] mm: hugetlb: fix a race between freeing and dissolving the page

2021-01-12 Thread Michal Hocko
ages rather than the error page reusable. > -- > 2.11.0 -- Michal Hocko SUSE Labs

Re: [PATCH v3 2/6] mm: hugetlbfs: fix cannot migrate the fallocated HugeTLB page

2021-01-12 Thread Michal Hocko
ure would be unexpected and wrong. > > Only export set_page_huge_active, just leave clear_page_huge_active > as static. Because there are no external users. > > Fixes: 70c3547e36f5 (hugetlbfs: add hugetlbfs_fallocate()) > Signed-off-by: Muchun Song > Cc: sta...@vger.kernel.org

Re: [PATCH v3 1/6] mm: migrate: do not migrate HugeTLB page whose refcount is one

2021-01-12 Thread Michal Hocko
> Reviewed-by: Mike Kravetz > Acked-by: Yang Shi Acked-by: Michal Hocko > --- > mm/migrate.c | 6 ++ > 1 file changed, 6 insertions(+) > > diff --git a/mm/migrate.c b/mm/migrate.c > index 4385f2fb5d18..a6631c4eb6a6 100644 > --- a/mm/migrate.c > +++ b/mm/migr

Re: [PATCH v3 4/6] mm: hugetlb: add return -EAGAIN for dissolve_free_huge_page

2021-01-12 Thread Michal Hocko
this code has been actually tested and the condition triggered. Besides that I have requested to have an explanation of why blocking on the WQ is safe and that hasn't happened. -- Michal Hocko SUSE Labs

Re: [PATCH v2 1/1] mm/madvise: replace ptrace attach requirement for process_madvise

2021-01-11 Thread Michal Hocko
217,6 +1227,7 @@ SYSCALL_DEFINE5(process_madvise, int, pidfd, const > struct iovec __user *, vec, > if (ret == 0) > ret = total_len - iov_iter_count(&iter); > > +release_mm: > mmput(mm); > release_task: > put_task_struct(task); > -- > 2.30.0.284.gd98b1dd5eaa7-goog > -- Michal Hocko SUSE Labs

Re: [PATCH v3] proc_sysctl: fix oops caused by incorrect command parameters.

2021-01-11 Thread Michal Hocko
leted? > > I think the individual logs are very useful and should be retained. Yes, other sysfs specific error messages are likely useful. I just fail to see why a missing value should be handled here when there is an existing handling in the caller. Not sure whether a complete shadow reporting in process_sysctl_arg is a deliberate decision or not. Vlastimil? Anyway one way or the other, all I care about is to have a reporting in place because this shouldn't be a silent failure. -- Michal Hocko SUSE Labs

Re: [PATCH] mm: vmscan: support equal reclaim for anon and file pages

2021-01-11 Thread Michal Hocko
scan_balance = SCAN_FILE; > goto out; > } > -- > Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, > a Linux Foundation Collaborative Project > -- Michal Hocko SUSE Labs

Re: [PATCH 0/1] mm: restore full accuracy in COW page reuse

2021-01-11 Thread Michal Hocko
s. The later would be a hard failure for the migration. -- Michal Hocko SUSE Labs

Re: [PATCH v2] proc_sysctl: fix oops caused by incorrect command parameters.

2021-01-11 Thread Michal Hocko
> mounting it when no > > sysctl log for patch3: > Setting sysctl args: `' invalid for parameter `hung_task_panic' [...] > When process_sysctl_arg() is called, the param parameter may not be the > sysctl parameter. > > Patch3 or patch4, which is better? Patch3 -- Michal Hocko SUSE Labs

Re: [PATCH v2] proc_sysctl: fix oops caused by incorrect command parameters.

2021-01-08 Thread Michal Hocko
On Fri 08-01-21 11:56:33, Kees Cook wrote: > On Fri, Jan 08, 2021 at 12:47:18PM +0100, Michal Hocko wrote: > > On Fri 08-01-21 18:01:52, Xiaoming Ni wrote: > > > On 2021/1/8 17:21, Michal Hocko wrote: > > > > On Fri 08-01-21 10:33:39, Xiaoming Ni wrote: > > >

Re: Very slow unlockall()

2021-01-08 Thread Michal Hocko
system :) > > Original report is https://gitlab.com/cryptsetup/cryptsetup/-/issues/617 > but this is apparently a kernel issue, just amplified by usage of > munlockall(). Which kernel version do you see this with? Have older releases worked better? -- Michal Hocko SUSE Labs

Re: [PATCH] mm: memcg: add swapcache stat for memcg v2

2021-01-08 Thread Michal Hocko
On Wed 06-01-21 08:42:39, Shakeel Butt wrote: > On Wed, Jan 6, 2021 at 6:53 AM Michal Hocko wrote: > > > > On Thu 31-12-20 18:39:55, Shakeel Butt wrote: > > > This patch adds swapcache stat for the cgroup v2. The swapcache > > > represents the memory that is acco

Re: [External] Re: [PATCH v2 3/6] mm: hugetlb: fix a race between freeing and dissolving the page

2021-01-08 Thread Michal Hocko
On Fri 08-01-21 19:52:54, Muchun Song wrote: > On Fri, Jan 8, 2021 at 7:44 PM Michal Hocko wrote: > > > > On Fri 08-01-21 18:08:57, Muchun Song wrote: > > > On Fri, Jan 8, 2021 at 5:31 PM Michal Hocko wrote: > > > > > > > > On Fri 08-01-21 17:0

Re: [PATCH v2] proc_sysctl: fix oops caused by incorrect command parameters.

2021-01-08 Thread Michal Hocko
On Fri 08-01-21 18:01:52, Xiaoming Ni wrote: > On 2021/1/8 17:21, Michal Hocko wrote: > > On Fri 08-01-21 10:33:39, Xiaoming Ni wrote: > > > The process_sysctl_arg() does not check whether val is empty before > > > invoking strlen(val). If the command line

Re: [External] Re: [PATCH v2 3/6] mm: hugetlb: fix a race between freeing and dissolving the page

2021-01-08 Thread Michal Hocko
On Fri 08-01-21 18:08:57, Muchun Song wrote: > On Fri, Jan 8, 2021 at 5:31 PM Michal Hocko wrote: > > > > On Fri 08-01-21 17:01:03, Muchun Song wrote: > > > On Fri, Jan 8, 2021 at 4:43 PM Michal Hocko wrote: > > > > > > > > On Thu 07-01-21 23:11

Re: [External] Re: [PATCH v2 3/6] mm: hugetlb: fix a race between freeing and dissolving the page

2021-01-08 Thread Michal Hocko
On Fri 08-01-21 17:01:03, Muchun Song wrote: > On Fri, Jan 8, 2021 at 4:43 PM Michal Hocko wrote: > > > > On Thu 07-01-21 23:11:22, Muchun Song wrote: [..] > > > But I find a tricky problem to solve. See free_huge_page(). > > > If we are in non-task context, w

Re: [PATCH v2] proc_sysctl: fix oops caused by incorrect command parameters.

2021-01-08 Thread Michal Hocko
@@ static int process_sysctl_arg(char *param, char *val, > loff_t pos = 0; > ssize_t wret; > > + if (!val) { > + pr_err("Missing param value! Expected '%s=...value...'\n", > param); > + return 0; > + } Shouldn't you retu

Re: [External] Re: [PATCH v2 3/6] mm: hugetlb: fix a race between freeing and dissolving the page

2021-01-08 Thread Michal Hocko
On Thu 07-01-21 23:11:22, Muchun Song wrote: > On Thu, Jan 7, 2021 at 10:11 PM Michal Hocko wrote: > > > > On Thu 07-01-21 20:59:33, Muchun Song wrote: > > > On Thu, Jan 7, 2021 at 8:38 PM Michal Hocko wrote: > > [...] > > > > Right. Can we simply back

Re: [PATCH v2 3/6] mm: hugetlb: fix a race between freeing and dissolving the page

2021-01-08 Thread Michal Hocko
On Thu 07-01-21 16:52:19, Mike Kravetz wrote: > On 1/7/21 12:40 AM, Michal Hocko wrote: > > On Wed 06-01-21 12:58:29, Mike Kravetz wrote: > >> On 1/6/21 8:56 AM, Michal Hocko wrote: > >>> On Wed 06-01-21 16:47:36, Muchun Song wrote: > >>>> There

Re: [External] Re: [PATCH v2 3/6] mm: hugetlb: fix a race between freeing and dissolving the page

2021-01-07 Thread Michal Hocko
On Thu 07-01-21 20:59:33, Muchun Song wrote: > On Thu, Jan 7, 2021 at 8:38 PM Michal Hocko wrote: [...] > > Right. Can we simply back off in the dissolving path when ref count is > > 0 && PageHuge() if list_empty(page->lru)? Is there any other scenario > > when th

Re: [External] Re: [PATCH v2 3/6] mm: hugetlb: fix a race between freeing and dissolving the page

2021-01-07 Thread Michal Hocko
On Thu 07-01-21 19:38:00, Muchun Song wrote: > On Thu, Jan 7, 2021 at 7:18 PM Michal Hocko wrote: > > > > On Thu 07-01-21 16:53:13, Muchun Song wrote: > > > On Thu, Jan 7, 2021 at 4:41 PM Michal Hocko wrote: > > > > > > > > On Thu 07-01-21 13:3

Re: [External] Re: [PATCH v2 1/6] mm: migrate: do not migrate HugeTLB page whose refcount is one

2021-01-07 Thread Michal Hocko
On Thu 07-01-21 19:24:59, Muchun Song wrote: > On Thu, Jan 7, 2021 at 7:16 PM Michal Hocko wrote: > > > > On Thu 07-01-21 10:52:21, Muchun Song wrote: > > > On Thu, Jan 7, 2021 at 12:13 AM Michal Hocko wrote: > > > > > > > > On Wed 06-01-21 16:47:

Re: [External] Re: [PATCH v2 4/6] mm: hugetlb: add return -EAGAIN for dissolve_free_huge_page

2021-01-07 Thread Michal Hocko
On Thu 07-01-21 17:01:16, Muchun Song wrote: > On Thu, Jan 7, 2021 at 4:39 PM Michal Hocko wrote: > > > > On Thu 07-01-21 11:11:41, Muchun Song wrote: > > > On Thu, Jan 7, 2021 at 1:07 AM Michal Hocko wrote: > > > > > > > > On Wed 06-0

Re: [External] Re: [PATCH v2 3/6] mm: hugetlb: fix a race between freeing and dissolving the page

2021-01-07 Thread Michal Hocko
On Thu 07-01-21 16:53:13, Muchun Song wrote: > On Thu, Jan 7, 2021 at 4:41 PM Michal Hocko wrote: > > > > On Thu 07-01-21 13:39:38, Muchun Song wrote: > > > On Thu, Jan 7, 2021 at 12:56 AM Michal Hocko wrote: > > > > > > > > On Wed 06-01-21 16:47:

Re: [External] Re: [PATCH v2 1/6] mm: migrate: do not migrate HugeTLB page whose refcount is one

2021-01-07 Thread Michal Hocko
On Thu 07-01-21 10:52:21, Muchun Song wrote: > On Thu, Jan 7, 2021 at 12:13 AM Michal Hocko wrote: > > > > On Wed 06-01-21 16:47:34, Muchun Song wrote: > > > If the refcount is one when it is migrated, it means that the page > > > was freed from under us. So we ar

Re: [External] Re: [PATCH v2 3/6] mm: hugetlb: fix a race between freeing and dissolving the page

2021-01-07 Thread Michal Hocko
On Thu 07-01-21 13:39:38, Muchun Song wrote: > On Thu, Jan 7, 2021 at 12:56 AM Michal Hocko wrote: > > > > On Wed 06-01-21 16:47:36, Muchun Song wrote: > > > There is a race condition between __free_huge_page() > > > and dissolve_free_huge_page(). > > >

Re: [PATCH v2 3/6] mm: hugetlb: fix a race between freeing and dissolving the page

2021-01-07 Thread Michal Hocko
On Wed 06-01-21 12:58:29, Mike Kravetz wrote: > On 1/6/21 8:56 AM, Michal Hocko wrote: > > On Wed 06-01-21 16:47:36, Muchun Song wrote: > >> There is a race condition between __free_huge_page() > >> and dissolve_free_huge_page(). > >> >

Re: [External] Re: [PATCH v2 4/6] mm: hugetlb: add return -EAGAIN for dissolve_free_huge_page

2021-01-07 Thread Michal Hocko
On Thu 07-01-21 11:11:41, Muchun Song wrote: > On Thu, Jan 7, 2021 at 1:07 AM Michal Hocko wrote: > > > > On Wed 06-01-21 16:47:37, Muchun Song wrote: > > > When dissolve_free_huge_page() races with __free_huge_page(), we can > > > do a retry. Because the race w

Re: [PATCH v2 2/6] mm: hugetlbfs: fix cannot migrate the fallocated HugeTLB page

2021-01-07 Thread Michal Hocko
On Wed 06-01-21 13:07:40, Mike Kravetz wrote: > On 1/6/21 12:02 PM, Michal Hocko wrote: > > On Wed 06-01-21 11:30:25, Mike Kravetz wrote: > >> On 1/6/21 8:35 AM, Michal Hocko wrote: > >>> On Wed 06-01-21 16:47:35, Muchun Song wrote: > >>>>

Re: [PATCH] mm/mmap: replace if (cond) BUG() with BUG_ON()

2021-01-07 Thread Michal Hocko
the change shouldn't be based solely on the script complains. Really, what is the point of changing an existing if (cond) BUG into BUG_ON? Fewer lines? Taste? Code consistency? -- Michal Hocko SUSE Labs

Re: [PATCH v2 2/6] mm: hugetlbfs: fix cannot migrate the fallocated HugeTLB page

2021-01-06 Thread Michal Hocko
On Wed 06-01-21 11:30:25, Mike Kravetz wrote: > On 1/6/21 8:35 AM, Michal Hocko wrote: > > On Wed 06-01-21 16:47:35, Muchun Song wrote: > >> Because we only can isolate a active page via isolate_huge_page() > >> and hugetlbfs_fallocate() forget to mark it as active,

Re: [PATCH v2 6/6] mm: hugetlb: remove VM_BUG_ON_PAGE from page_huge_active

2021-01-06 Thread Michal Hocko
ll trigger a BUG_ON which is in the > page_huge_active() when CONFIG_DEBUG_VM is enabled. Just remove the > VM_BUG_ON_PAGE. > > Fixes: 7e1f049efb86 ("mm: hugetlb: cleanup using paeg_huge_active()") > Signed-off-by: Muchun Song > Reviewed-by: Mike Kravetz Acked-by: Mich

Re: [PATCH v2 5/6] mm: hugetlb: fix a race between isolating and freeing page

2021-01-06 Thread Michal Hocko
igger a BUG_ON on CPU0. Because > it is already freed to the buddy allocator. > > Fixes: c8721bbbdd36 ("mm: memory-hotplug: enable memory hotplug to handle > hugepage") > Signed-off-by: Muchun Song > Reviewed-by: Mike Kravetz Acked-by: Michal Hocko Thanks! > --- &g

Re: [PATCH v2 4/6] mm: hugetlb: add return -EAGAIN for dissolve_free_huge_page

2021-01-06 Thread Michal Hocko
etlb_lock); > + > + /* > + * If the freeing of the HugeTLB page is put on a work queue, we should > + * flush the work before retrying. > + */ > + if (unlikely(rc == -EAGAIN)) > + flush_work(&free_hpage_work); Is it safe to wait for the work to finish from this context? -- Michal Hocko SUSE Labs

Re: [PATCH v2 3/6] mm: hugetlb: fix a race between freeing and dissolving the page

2021-01-06 Thread Michal Hocko
0) > goto out; > + > + /* > + * We should make sure that the page is already on the free list > + * when it is dissolved. > + */ > + if (unlikely(!PageHugeFreed(head))) > + goto out; > + > /* >* Move PageHWPoison flag from head page to the raw error page, >* which makes any subpages rather than the error page reusable. > -- > 2.11.0 -- Michal Hocko SUSE Labs

Re: [PATCH v2 2/6] mm: hugetlbfs: fix cannot migrate the fallocated HugeTLB page

2021-01-06 Thread Michal Hocko
ive(struct page *page) > } > > /* never called for tail page */ > -static void set_page_huge_active(struct page *page) > +void set_page_huge_active(struct page *page) > { > VM_BUG_ON_PAGE(!PageHeadHuge(page), page); > SetPagePrivate(&page[1]); > -- > 2.11.0 -- Michal Hocko SUSE Labs

Re: [PATCH v2 1/6] mm: migrate: do not migrate HugeTLB page whose refcount is one

2021-01-06 Thread Michal Hocko
On Wed 06-01-21 16:47:34, Muchun Song wrote: > If the refcount is one when it is migrated, it means that the page > was freed from under us. So we are done and do not need to migrate. Is this common enough that it would warrant the explicit check for each migration? -- Michal Hocko SUSE Labs

Re: [PATCH 1/6] mm: migrate: do not migrate HugeTLB page whose refcount is one

2021-01-06 Thread Michal Hocko
On Wed 06-01-21 17:11:39, Michal Hocko wrote: > On Mon 04-01-21 14:58:38, Muchun Song wrote: > > If the refcount is one when it is migrated, it means that the page > > was freed from under us. So we are done and do not need to migrate. > > Is this common enough that it would

Re: [PATCH 1/6] mm: migrate: do not migrate HugeTLB page whose refcount is one

2021-01-06 Thread Michal Hocko
gt; + } > + > new_hpage = get_new_page(hpage, private); > if (!new_hpage) > return -ENOMEM; > -- > 2.11.0 -- Michal Hocko SUSE Labs

Re: [PATCH 3/6] hugetlb: add free page reporting support

2021-01-06 Thread Michal Hocko
to do the zeroying? I do remember Alexander pushing back against that and so you should better have a very strong arguments to proceed that way. I am pretty sure there are more questions to come when more details are uncovered. -- Michal Hocko SUSE Labs

Re: [PATCH] mm: memcg: add swapcache stat for memcg v2

2021-01-06 Thread Michal Hocko
workloads > depending on memsw usage and limit to v2' memory and swap counters. Could you expand a bit more on why memsw usage is important even in cgroup v2 land? How are you going to use the approximated value? I am not really objecting to adding this counter. We do export it for the global case and having a memcg view sounds useful for analysis. -- Michal Hocko SUSE Labs

Re: [PATCH] mm: Teach pfn_to_online_page() about ZONE_DEVICE section collisions

2021-01-06 Thread Michal Hocko
On Wed 06-01-21 12:22:57, David Hildenbrand wrote: > On 06.01.21 11:42, Michal Hocko wrote: > > On Wed 06-01-21 10:56:19, David Hildenbrand wrote: > > [...] > >> Note that this is not sufficient in the general case. I already > >> mentioned that we effectivel

Re: [PATCH] mm: Teach pfn_to_online_page() about ZONE_DEVICE section collisions

2021-01-06 Thread Michal Hocko
e pmem reserved space cannot be fully initialized. On the other hand making sure that pfn_to_online_page sounds like the right thing to do. And having an explicit check for zone device there in a slow path makes sense to me. -- Michal Hocko SUSE Labs

Re: [PATCH] mm: Teach pfn_to_online_page() about ZONE_DEVICE section collisions

2021-01-06 Thread Michal Hocko
es: ba72b4c8cf60 ("mm/sparsemem: support sub-section hotplug") > Cc: Andrew Morton > Reported-by: Michal Hocko > Reported-by: David Hildenbrand > Signed-off-by: Dan Williams [...] > +static int zone_id(const struct zone *zone) > +{ > + struct pglist_d

Re: [PATCH] mm: migrate: initialize err in do_migrate_pages

2021-01-05 Thread Michal Hocko
c: Zi Yan > Cc: Yang Shi > Cc: Jan Kara > Cc: Matthew Wilcox > Cc: Mel Gorman > Cc: Michal Hocko > Cc: Song Liu > Cc: Andrew Morton > Signed-off-by: Jan Stancek Acked-by: Michal Hocko Thanks! > --- > mm/mempolicy.c | 2 +- > 1 file changed, 1 in

Re: [RFC v2 PATCH 4/4] mm: pre zero out free pages to speed up page allocation for __GFP_ZERO

2021-01-05 Thread Michal Hocko
On Tue 05-01-21 10:29:45, David Hildenbrand wrote: > On 05.01.21 10:20, Michal Hocko wrote: [...] > > A global knob with all or nothing sounds like an easier to use and > > maintain solution to me. > > I mean, that brings me back to my original suggestion: just use > huge

Re: uninitialized pmem struct pages

2021-01-05 Thread Michal Hocko
On Tue 05-01-21 10:13:49, David Hildenbrand wrote: > On 05.01.21 10:05, Michal Hocko wrote: > > On Tue 05-01-21 00:57:43, Dan Williams wrote: > >> On Tue, Jan 5, 2021 at 12:42 AM Michal Hocko wrote: > >>> > >>> On Tue 05-01-21 00:27:34, Dan Williams wro

Re: [RFC v2 PATCH 4/4] mm: pre zero out free pages to speed up page allocation for __GFP_ZERO

2021-01-05 Thread Michal Hocko
quickly if those files ended up over-sized somehow. We can probably allow MADV_FREE on shmem but that would require an exclusively mapped page. Shared case is really tricky because of silent data corruption in uncoordinated userspace. -- Michal Hocko SUSE Labs

Re: uninitialized pmem struct pages

2021-01-05 Thread Michal Hocko
On Tue 05-01-21 00:57:43, Dan Williams wrote: > On Tue, Jan 5, 2021 at 12:42 AM Michal Hocko wrote: > > > > On Tue 05-01-21 00:27:34, Dan Williams wrote: > > > On Tue, Jan 5, 2021 at 12:17 AM Michal Hocko wrote: > > > > > > > > On Tue 05-01-21 09:0

Re: uninitialized pmem struct pages

2021-01-05 Thread Michal Hocko
On Tue 05-01-21 00:27:34, Dan Williams wrote: > On Tue, Jan 5, 2021 at 12:17 AM Michal Hocko wrote: > > > > On Tue 05-01-21 09:01:00, Michal Hocko wrote: > > > On Mon 04-01-21 16:44:52, David Hildenbrand wrote: > > > > On 04.01.21 16:43, David Hildenbran

Re: uninitialized pmem struct pages

2021-01-05 Thread Michal Hocko
On Tue 05-01-21 09:01:00, Michal Hocko wrote: > On Mon 04-01-21 16:44:52, David Hildenbrand wrote: > > On 04.01.21 16:43, David Hildenbrand wrote: > > > On 04.01.21 16:33, Michal Hocko wrote: > > >> On Mon 04-01-21 16:15:23, David Hildenbrand wrote: > > >&

Re: uninitialized pmem struct pages

2021-01-05 Thread Michal Hocko
On Mon 04-01-21 16:44:52, David Hildenbrand wrote: > On 04.01.21 16:43, David Hildenbrand wrote: > > On 04.01.21 16:33, Michal Hocko wrote: > >> On Mon 04-01-21 16:15:23, David Hildenbrand wrote: > >>> On 04.01.21 16:10, Michal Hocko wrote: > >> [...] >

Re: uninitialized pmem struct pages

2021-01-04 Thread Michal Hocko
broken data or BUG_ON with debugging enabled (which is not the case for many production users). Or are we talking about different bugs here? -- Michal Hocko SUSE Labs

Re: uninitialized pmem struct pages

2021-01-04 Thread Michal Hocko
e. The "pgmap" field is actually at > an offset of 8 bytes within the memmap ... I haven't noticed this is an offset into struct page. This is indeed weird because the structure is much larger than struct page. But maybe it reuses storage of several struct pages in a row. Or maybe it is not pgmap but something else that shares the offset in the structure. -- Michal Hocko SUSE Labs

Re: uninitialized pmem struct pages

2021-01-04 Thread Michal Hocko
On Mon 04-01-21 21:33:06, Dan Williams wrote: > On Mon, Jan 4, 2021 at 7:59 AM Michal Hocko wrote: [...] > > Not sure what exactly you are asking for but crash says > > crash> kmem -p 606 > > PAGE PHYSICAL MAPPING INDEX CNT FLAGS > >

Re: [PATCH] mm/page_alloc: remove the static for local variable node_order

2021-01-04 Thread Michal Hocko
nit code) but even if we could, what would be an advantage. This code is called very seldom with a very shallow stacks so using the stack allocation sounds like the easiest thing to do. > (what blocks node and cpu hotplug in there??) Memory hotplug is excluded by the caller when it matters (e.g. no locking for the early init). -- Michal Hocko SUSE Labs

Re: uninitialized pmem struct pages

2021-01-04 Thread Michal Hocko
On Mon 04-01-21 16:43:49, David Hildenbrand wrote: > On 04.01.21 16:33, Michal Hocko wrote: > > On Mon 04-01-21 16:15:23, David Hildenbrand wrote: > >> On 04.01.21 16:10, Michal Hocko wrote: > > [...] > >> Do the physical addresses you see fall into the same secti

<    1   2   3   4   5   6   7   8   9   10   >