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
ght 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
ontig_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_ASYNC : MIGRATE_SYNC, &

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

2021-01-20 Thread Michal Hocko
tion() 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
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's n

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

2021-01-20 Thread Michal Hocko
aking 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: Michal Hocko Acked-by: Michal Hocko Thanks! > --- > Changelogs: >

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 > > >

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 first

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
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 changed, 39 insertions(+) > > diff --git

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
o 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
y_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
*/ > + 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 so

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: > >>>&g

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

2021-01-12 Thread Michal Hocko
s 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
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
ailure 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/migrate.c

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

2021-01-12 Thread Michal Hocko
ign 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
@@ SYSCALL_DEFINE5(process_madvise, int, pidfd, const > struct iovec __user *, vec, > if (ret == 0) > ret = total_len - iov_iter_count(); > > +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
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
oid > 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
> > 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

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 c

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 return an error her

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.

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
ased 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 act

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

2021-01-06 Thread Michal Hocko
will 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
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! > --- > mm/hugetlb.c | 4 ++-- > 1 file changed, 2 i

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

2021-01-06 Thread Michal Hocko
; out: > spin_unlock(_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(_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
> @@ -1770,6 +1800,14 @@ int dissolve_free_huge_page(struct page *page) > int nid = page_to_nid(head); > if (h->free_huge_pages - h->resv_huge_pages == 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
t 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([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
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
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
rved 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
a72b4c8cf60 ("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_data *

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

2021-01-05 Thread Michal Hocko
ra > 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 insertion(+), 1 deletion(-) > > diff --g

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
es 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
en 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
t;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
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

Re: [PATCH 1/2] mm: page_counter: relayout structure to reduce false sharing

2021-01-04 Thread Michal Hocko
On Mon 04-01-21 22:44:02, Feng Tang wrote: > On Mon, Jan 04, 2021 at 03:11:40PM +0100, Michal Hocko wrote: > > On Mon 04-01-21 21:34:45, Feng Tang wrote: > > > Hi Michal, > > > > > > On Mon, Jan 04, 2021 at 02:03:57PM +0100, Michal Hocko wrote: > > >

Re: uninitialized pmem struct pages

2021-01-04 Thread Michal Hocko
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 section as boot > memory? Or what's around these addresses? Yes I am getting a garbage for the first struct page belonging to the pme

Re: uninitialized pmem struct pages

2021-01-04 Thread Michal Hocko
On Mon 04-01-21 15:51:35, David Hildenbrand wrote: > On 04.01.21 15:26, Michal Hocko wrote: > > On Mon 04-01-21 11:45:39, David Hildenbrand wrote: [] > >> One instance where this is still an issue is > >> mm/memory-failure.c:memory_failure() and > >> mm/mem

Re: uninitialized pmem struct pages

2021-01-04 Thread Michal Hocko
On Mon 04-01-21 11:45:39, David Hildenbrand wrote: > On 04.01.21 11:03, Michal Hocko wrote: > > Hi, > > back in March [1] you have recommended 53cdc1cb29e8 > > ("drivers/base/memory.c: indicate all memory blocks as removable") to be > > backported to stable tre

Re: [PATCH 1/2] mm: page_counter: relayout structure to reduce false sharing

2021-01-04 Thread Michal Hocko
On Mon 04-01-21 21:34:45, Feng Tang wrote: > Hi Michal, > > On Mon, Jan 04, 2021 at 02:03:57PM +0100, Michal Hocko wrote: > > On Tue 29-12-20 22:35:13, Feng Tang wrote: > > > When checking a memory cgroup related performance regression [1], > > > from the perf

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