On 5/8/2015 9:47 PM, J. Bruce Fields wrote: > On Fri, May 08, 2015 at 02:40:31PM +1000, NeilBrown wrote: >> Thanks for this patch. It looks good! >> >> My only comment on the code is that I would really like to see a >> "path_get_pin()" and "path_put_unpin()" rather than open coding: >> >>> + dget(item->ek_path.dentry); >>> + pin_insert_group(&new->ek_pin, item->ek_path.mnt, NULL); >> >> and >> >>> + dput(key->ek_path.dentry); >>> + pin_remove(&key->ek_pin); >> >> >> But the question you raise is an important one: Exactly which filesystems >> should be allowed to be unmounted? >> This is a change in behaviour - is it one that people uniformly would want? >> >> The kernel doesn't currently know which file systems were explicitly listed >> in /etc/exports, and which were found by following a 'crossmnt'. >> It could guess and allow the unmounting of anything below a 'crossmnt', but I >> wouldn't be comfortable with that - it is error prone. >> >> mountd does know what is in /etc/exports, and could tell the kernel. >> For the expkey cache, we could always use path_get_pin. >> For the export cache (where flags are available) we could use path_get >> or path_get_pin depending on some new flag. >> >> I'm not really sure it is worth it. I would rather the filesystems could >> always be unmounted. But doing that could possibly break someone's work >> flow. Maybe. >> >> Or maybe I'm seeing problems where there aren't any. >> >> Anyone else have an opinion? > > The undisputed bug here was negative cache entries preventing unmount. > So most conservative might be just to purge negative entries. I'd like this, if the cache is valid, user should not be allowed to umount the filesystem. > > Otherwise, the only guarantees I think we've really had is that we won't > allow unmount if you hold any actual state on the filesystem (NLM locks, > NFSv4 locks, opens, or delegations). Those resources hold the reference of vfsmnt. > > If a filesystem is exported but no clients hold state on it, then it's > currently mostly chance whether the unmount succeeds or not. So we're > probably free to change the behavior in this case. I'd be inclined to > allow the unmount, but haven't thought this through carefully. If client mount a nfsserver succeed without holds state, nfs server umounts the exported filesystem, client also think the filesystem is valid, but it is umounted. > > It could also be useful to have the ability to force an unmount even in > the presence of locks. That's not a safe default, but an > "allow_force_unmount" export option might be useful. > > We might similarly be able to add some way for the kernel to distinguish > explicit exports from crossmnt-found exports, but I'm not seeing the use > case for that. Agree, I don't think we needs that right now. thanks, Kinglong Mee -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
