On 04/29/2014 05:41 AM, Davidlohr Bueso wrote:
> On Mon, 2014-04-28 at 16:57 -0700, Linus Torvalds wrote:
>> On Mon, Apr 28, 2014 at 4:11 PM, Andrew Morton
>> <akpm@xxxxxxxxxxxxxxxxxxxx> wrote:
>>>
>>> unuse_mm() leaves current->mm at NULL so we'd hear about it pretty
>>> quickly if a user task was running use_mm/unuse_mm.
>>
>> Yes.
>>
>>> I think so. Maybe it's time to cook up a debug patch for Srivatsa to
>>> use? Dump the vma cache when the bug hits, or wire up some trace
>>> points. Or perhaps plain old printks - it seems to be happening pretty
>>> early in boot.
>>
>> Well, I think Srivatsa has only seen it once, and wasn't able to
>> reproduce it, so we'd have to make it happen more first.
>>
>>> Are there additional sanity checks we can perform at cache addition
>>> time?
>>
>> I wouldn't really expect it to happen at cache addition time, since
>> that's really quite simple. There's only one caller of
>> vmacache_update(), namely find_vma(). And vmacache_update() does the
>> same sanity check that vmacache lookup does (ie check that the
>> passed-on mm is the current thread mm, and that we're not a kernel
>> thread).
>
> Agreed.
>
>> I'd be more inclined to think it's a missing invalidate, but I can
>> only think of two reasons to invalidate:
>>
>> - the vma itself went away from the mm, got free'd/reused, and so
>> vm_mm changes..
>>
>> But then we'd have to remove it from the rb-tree, and both callers
>> of vma_rb_erase() do a vmacache_invalidate()
>
> Right, if this were the case, -next never would have allowed it.
>
>> - the mm of a thread changed
>>
>> This is exec, use_mm(), and fork() (and fork really only just
>> because we copy the vmacache).
>>
>> exec and fork do that "vmacache_flush(tsk)", which is why I was
>> looking at use_mm().
>
> Here's a patch to remove treating kthreads specially. Not sure how
> easily it would be to test since Srivatsa only ran into it once and I
> see no other users complaining.
>
I guess I'll hold off on testing this fix until I get to reproduce
the bug more reliably..
Regards,
Srivatsa S. Bhat
> diff --git a/mm/mmu_context.c b/mm/mmu_context.c
> index f802c2d..41445bb 100644
> --- a/mm/mmu_context.c
> +++ b/mm/mmu_context.c
> @@ -4,6 +4,7 @@
> */
>
> #include <linux/mm.h>
> +#include <linux/vmacache.h>
> #include <linux/mmu_context.h>
> #include <linux/export.h>
> #include <linux/sched.h>
> @@ -29,6 +30,7 @@ void use_mm(struct mm_struct *mm)
> tsk->active_mm = mm;
> }
> tsk->mm = mm;
> + vmacache_flush(tsk);
> switch_mm(active_mm, mm, tsk);
> task_unlock(tsk);
> #ifdef finish_arch_post_lock_switch
> diff --git a/mm/vmacache.c b/mm/vmacache.c
> index 1037a3ba..04009d3 100644
> --- a/mm/vmacache.c
> +++ b/mm/vmacache.c
> @@ -36,13 +36,10 @@ void vmacache_flush_all(struct mm_struct *mm)
> * get_user_pages()->find_vma(). The vmacache is task-local and this
> * task's vmacache pertains to a different mm (ie, its own). There is
> * nothing we can do here.
> - *
> - * Also handle the case where a kernel thread has adopted this mm via use_mm().
> - * That kernel thread's vmacache is not applicable to this mm.
> */
> static bool vmacache_valid_mm(struct mm_struct *mm)
> {
> - return current->mm == mm && !(current->flags & PF_KTHREAD);
> + return current->mm == mm;
> }
>
> void vmacache_update(unsigned long addr, struct vm_area_struct *newvma)
>
>
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@xxxxxxxxx. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>