On 24 April 2012 17:19, Minchan Kim <minchan@xxxxxxxxxx> wrote:
> On 04/24/2012 03:13 PM, Nick Piggin wrote:
>
>> 2012/4/24 Minchan Kim <minchan@xxxxxxxxxx>:
>>> On 04/24/2012 02:16 PM, KAMEZAWA Hiroyuki wrote:
>>>
>>>> (2012/04/23 17:55), Minchan Kim wrote:
>>>>
>>>>> As I test some code, I found a problem about deadlock by lockdep.
>>>>> The reason I saw the message is __vmalloc calls map_vm_area which calls
>>>>> pud/pmd_alloc without gfp_t. so although we call __vmalloc with
>>>>> GFP_ATOMIC or GFP_NOIO, it ends up allocating pages with GFP_KERNEL.
>>>>> The should be a BUG. This patch fixes it by passing gfp_to to low page
>>>>> table allocate functions.
>>>>>
>>>>> Signed-off-by: Minchan Kim <minchan@xxxxxxxxxx>
>>>>
>>>>
>>>> Hmm ? vmalloc should support GFP_ATOMIC ?
>>>
>>>
>>> I'm not sure but alloc_large_system_hash already has used.
>>> And it's not specific on GFP_ATOMIC.
>>> We have to care of GFP_NOFS and GFP_NOIO to prevent deadlock on reclaim
>>> context.
>>> There are some places to use GFP_NOFS and we don't emit any warning
>>> message in case of that.
>>
>> What's the lockdep warning?
>
>
> It's just some private-test code, not-mainlined and lockdep warning is like this.
>
> [ INFO: inconsistent lock state ]
> 3.4.0-rc3-next-20120417+ #80 Not tainted
> ---------------------------------
> inconsistent {RECLAIM_FS-ON-W} -> {IN-RECLAIM_FS-R} usage.
>
> It seems test code calls vmalloc inside reclaim context so that it enters
> reclaim context, again by map_vm_area which allocates pages with GFP_KERNEL.
>
> Of course, I can avoid this problem by fixing the caller but during I look into
> this problem, found other places to use gfp_t with "context restriction".
>
>
>>
>
>> vmalloc was never supposed to use gfp flags for allocation "context"
>> restriction. I.e., it
>> was always supposed to have blocking, fs, and io capable allocation
>> context. The flags
>> were supposed to be a memory type modifier.
>
>
> You mean "zone modifiers"?
Yeah, things like that.
>> These different classes of flags is a bit of a problem and source of
>> confusion we have.
>> We should be doing more checks for them, of course.
>
>
> It might need some warning in __vmalloc and family which use gfp_t
> if the caller use context flags.
I think that would be a good idea.
>> I suspect you need to fix the caller?
>
>
> Hmm, there are several places to use GFP_NOIO and GFP_NOFS even, GFP_ATOMIC.
> I believe it's not trivial now.
They're all buggy then. Unfortunately not through any real fault of their own.
I would say add a bit of warnings and documentation, and see what can be done
about callers.
We should not take lightly the decision to make the API more permissive, because
as you can see it's more work for implementation. Making it ATOMIC safe is even
harder, requiring irqsafe locks and such, and it might be tricky for some
architectures.
Thanks,
Nick
--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>