Re: [PATCH V5] xfs: cleanup the mount options

[Wanlong Gao <gaowanlong@cn.fujitsu.com>]

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@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs