Re: [PATCH v7 7/7] mm/madvise: allow KSM hints for remote API
From: Michal Hocko <mhocko@kernel.org>
Date: 2020-03-09 16:03:40
Also in:
linux-mm, lkml
On Mon 09-03-20 16:42:43, Vlastimil Babka wrote:
On 3/9/20 4:19 PM, Oleksandr Natalenko wrote:quoted
On Mon, Mar 09, 2020 at 04:08:15PM +0100, Michal Hocko wrote:quoted
On Mon 09-03-20 14:11:17, Oleksandr Natalenko wrote:quoted
On Fri, Mar 06, 2020 at 05:08:18PM +0100, Vlastimil Babka wrote:[...]quoted
quoted
Dunno, it's nice to react to signals quickly, for any proces that gets them, no?So, do you mean something like this? ===diff --git a/mm/ksm.c b/mm/ksm.c index 363ec8189561..b39c237cfcf4 100644 --- a/mm/ksm.c +++ b/mm/ksm.c@@ -849,7 +849,8 @@ static int unmerge_ksm_pages(struct vm_area_struct *vma, for (addr = start; addr < end && !err; addr += PAGE_SIZE) { if (ksm_test_exit(vma->vm_mm)) break; - if (signal_pending(current)) + if (signal_pending(current) || + signal_pending(rcu_dereference(vma->vm_mm->owner))) err = -ERESTARTSYS; else err = break_ksm(vma, addr);===This is broken because mm might be attached to different tasks. AFAIU this check is meant to allow quick backoff of the _calling_ process so that it doesn't waste time when the context is killed already. I do not understand why should we care about any other context here? What is the actual problem this would solve?I agree with you, but still trying to understand what does Vlastimil mean :).Well you wondered if we should stop caring about current, and I said that probably wouldn't be nice. As for caring about the other task, patch 3/7 does that for (MADV_COLD|MADV_PAGEOUT) so I just pointed out that the KSM case doesn't. AFAIU if we don't check the signals, we might be blocking the killed task from exiting?
I would have to double check but I do not think this would be a problem because the remote task should take mmget to prevent address space to vanish under its feet. That should also rule out the exclusive mmap_sem usage from the exit path. -- Michal Hocko SUSE Labs