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

[Dave Chinner <david@fromorbit.com>]

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....

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs