On Wed, Apr 08, 2015 at 02:06:14PM +0800, Qu Wenruo wrote: > > > -------- Original Message -------- > Subject: [PATCH 2/3] Btrfs: unify subvol= and subvolid= mounting > From: Omar Sandoval <osan...@osandov.com> > To: Chris Mason <c...@fb.com>, Josef Bacik <jba...@fb.com>, David Sterba > <dste...@suse.cz>, <linux-bt...@vger.kernel.org> > Date: 2015年04月08日 13:34 > > > Currently, mounting a subvolume with subvolid= takes a different code > > path than mounting with subvol=. This isn't really a big deal except for > > the fact that mounts done with subvolid= or the default subvolume don't > > have a dentry that's connected to the dentry tree like in the subvol= > > case. To unify the code paths, when given subvolid= or using the default > > subvolume ID, translate it into a subvolume name by walking > > ROOT_BACKREFs in the root tree and INODE_REFs in the filesystem trees. > Oh, this patch is what I have tried long long ago, and want to do the > same thing, to show subvolume mount for btrfs. > > But it came to me that, superblock->show_path() is a better method to do it. > > You can implement btrfs_show_path() to allow mountinfo to get the > subvolume name from subvolid, and don't change the mount routine much.
The problem I see with the show_mount approach is related to the additional path lookup, memory allocation and locking. If the mountpoint dentry is the right on ,it's just a simple seq_dentry in show_options. OTOH, your patch takes subvol_sem that will block the callback if there's eg. a subvolume being deleted (that takes the write lock). This is not a lightweight operation nor an infrequent one. There are more write locks to subvol_sem. I'm not sure if I've ever sent this comment back to you, sorry if not. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/