xfs
[Top] [All Lists]

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

To: Dave Chinner <david@xxxxxxxxxxxxx>
Subject: Re: [PATCH V5] xfs: cleanup the mount options
From: Wanlong Gao <gaowanlong@xxxxxxxxxxxxxx>
Date: Wed, 25 Jul 2012 09:09:57 +0800
Cc: xfs@xxxxxxxxxxx, Ben Myers <bpm@xxxxxxx>, Christoph Hellwig <hch@xxxxxxxxxxxxx>, Zach Brown <zab@xxxxxxxxx>
In-reply-to: <20120724211857.GO23387@dastard>
Organization: Fujitsu
References: <20120711022609.GX19223@dastard> <1341988145-12994-1-git-send-email-gaowanlong@xxxxxxxxxxxxxx> <20120723232813.GL23387@dastard> <500E0217.90405@xxxxxxxxxxxxxx> <20120724211857.GO23387@dastard>
Reply-to: gaowanlong@xxxxxxxxxxxxxx
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:14.0) Gecko/20120717 Thunderbird/14.0
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.
> 

<Prev in Thread] Current Thread [Next in Thread>