On 07/25/2012 05:18 AM, Dave Chinner wrote:
> On Tue, Jul 24, 2012 at 10:01:59AM +0800, Wanlong Gao wrote:
>> On 07/24/2012 07:28 AM, Dave Chinner wrote:
>>> On Wed, Jul 11, 2012 at 02:29:05PM +0800, Wanlong Gao wrote:
>>>> @@ -208,175 +292,192 @@ xfs_parseargs(
>>>> if (!options)
>>>> goto done;
>>>>
>>>> - while ((this_char = strsep(&options, ",")) != NULL) {
>>>> - if (!*this_char)
>>>> + options = kstrdup(options, GFP_NOFS);
>>>
>>> GFP_NOFS allocation. Why is this GFP_NOFS, and all the other
>>> allocations GFP_KERNEL? If it is not safe to use GFP_KERNEL
>>> allocations here, then all of the above allocations need to be
>>> GFP_NOFS, too.
>>
>> Since strsep() will change the options, so we should make GFP_NOFS
>> safely to dup the orig options, but the parse functions can safely
>> use GFP_KERNEL.
>
> I unerstand why you are using kstrdup(). My question is why are you
> using GFP_NOFS here? What filesystem recursion deadlock are you
> tryin gto avoid here, and why don't the other allocations in the
> string parsing need to avoid the same problem as they are called
> from the same context?
>
> If there is no recursion deadlock (I can't see that there is one),
> then GFP_KERNEL is what you should be using here....
>
Sorry, I have no strong reason, so will use GFP_KERNEL instead.
Thanks,
Wanlong Gao
> Cheers,
>
> Dave.
>
_______________________________________________
xfs mailing list
xfs@xxxxxxxxxxx
http://oss.sgi.com/mailman/listinfo/xfs
