On Thu, Aug 30, 2012 at 12:13:02PM -0700, Andrew Morton wrote:
>On Sat, 25 Aug 2012 17:47:50 +0800
>Gavin Shan <shangw@xxxxxxxxxxxxxxxxxx> wrote:
>
>> >> --- a/mm/mmu_notifier.c
>> >> +++ b/mm/mmu_notifier.c
>> >> @@ -192,22 +192,23 @@ static int do_mmu_notifier_register(struct mmu_notifier *mn,
>> >>
>> >> BUG_ON(atomic_read(&mm->mm_users) <= 0);
>> >>
>> >> - ret = -ENOMEM;
>> >> - mmu_notifier_mm = kmalloc(sizeof(struct mmu_notifier_mm), GFP_KERNEL);
>> >> - if (unlikely(!mmu_notifier_mm))
>> >> - goto out;
>> >> -
>> >> if (take_mmap_sem)
>> >> down_write(&mm->mmap_sem);
>> >> ret = mm_take_all_locks(mm);
>> >> if (unlikely(ret))
>> >> - goto out_cleanup;
>> >> + goto out;
>> >>
>> >> if (!mm_has_notifiers(mm)) {
>> >> + mmu_notifier_mm = kmalloc(sizeof(struct mmu_notifier_mm),
>> >> + GFP_ATOMIC);
>> >
>> >Why was the code switched to the far weaker GFP_ATOMIC? We can still
>> >perform sleeping allocations inside mmap_sem.
>> >
>>
>> Yes, we can perform sleeping while allocating memory, but we're holding
>> the "mmap_sem". GFP_KERNEL possiblly block somebody else who also waits
>> on mmap_sem for long time even though the case should be rare :-)
>
>GFP_ATOMIC allocations are unreliable. If the allocation attempt fails
>here, an entire kernel subsystem will have failed, quite probably
>requiring a reboot. It's a bad tradeoff.
>
Yep. Thanks, Andrew :-)
>Please fix this and retest. With lockdep enabled, of course.
>
>And please do not attempt to sneak changes like this into the kernel
>without even mentioning them in the changelog. If I hadn't have
>happened to notice this, we'd have ended up with a less reliable
>kernel.
>
I'll fix and rerest it.
Thanks,
Gavin
--
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>