| Message ID | 1483479039-5255-1-git-send-email-fabf@skynet.be (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Jan 03, 2017 at 10:30:39PM +0100, Fabian Frederick wrote: > Add standard functions making AFFS work with NFS. > > Functions based on ext4 implementation. > Tested on loop device. How the hell is that supposed to work with cold dcache? You don't have ->get_parent() there at all... There *IS* a reference to parent directory in those suckers - not the same kind as in normal unix filesystems (".." is not a directory entry there - it's all fake), but it's doable. be32_to_cpu(AFFS_TAIL(sb, bh)->parent) would be the inumber you need, where bh is the inode block of directory. So it can be done, but not in this form. NAK for the time being... -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> On 03 January 2017 at 23:29 Al Viro <viro@ZenIV.linux.org.uk> wrote: > > > On Tue, Jan 03, 2017 at 10:30:39PM +0100, Fabian Frederick wrote: > > Add standard functions making AFFS work with NFS. > > > > Functions based on ext4 implementation. > > Tested on loop device. > > How the hell is that supposed to work with cold dcache? You don't have > ->get_parent() there at all... > > There *IS* a reference to parent directory in those suckers - not the same > kind as in normal unix filesystems (".." is not a directory entry there - > it's all fake), but it's doable. be32_to_cpu(AFFS_TAIL(sb, bh)->parent) > would be the inumber you need, where bh is the inode block of directory. > > So it can be done, but not in this form. NAK for the time being... I worked on that function declared optional in Documentation/filesystems/nfs/Exporting but was unable to test it. Regards, Fabian -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> On 03 January 2017 at 23:29 Al Viro <viro@ZenIV.linux.org.uk> wrote: > > > On Tue, Jan 03, 2017 at 10:30:39PM +0100, Fabian Frederick wrote: > > Add standard functions making AFFS work with NFS. > > > > Functions based on ext4 implementation. > > Tested on loop device. > > How the hell is that supposed to work with cold dcache? You don't have > ->get_parent() there at all... > > There *IS* a reference to parent directory in those suckers - not the same > kind as in normal unix filesystems (".." is not a directory entry there - > it's all fake), but it's doable. be32_to_cpu(AFFS_TAIL(sb, bh)->parent) > would be the inumber you need, where bh is the inode block of directory. > > So it can be done, but not in this form. NAK for the time being... I tried with the following function: static struct dentry *affs_get_parent(struct dentry *child) { struct inode *parent; struct buffer_head *bh; bh = affs_bread(child->d_sb, d_inode(child)->i_ino); if (IS_ERR(bh)) return ERR_CAST(bh); parent = affs_iget(child->d_sb, be32_to_cpu(AFFS_TAIL(child->d_sb, bh)->parent)); brelse(bh); if (IS_ERR(parent)) return ERR_CAST(parent); return d_obtain_alias(parent); } but after creating a new file and echo 3 > /proc/sys/vm/drop_caches on server, client directory requires some seconds before being updated or gives "ls: cannot open directory '.': Stale file handle" the first time and correct directory when trying again... Do you know how I could improve this ? Regards, Fabian -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Jan 04, 2017 at 06:53:31AM +0100, Fabian Frederick wrote: > > > > On 03 January 2017 at 23:29 Al Viro <viro@ZenIV.linux.org.uk> wrote: > > > > > > On Tue, Jan 03, 2017 at 10:30:39PM +0100, Fabian Frederick wrote: > > > Add standard functions making AFFS work with NFS. > > > > > > Functions based on ext4 implementation. > > > Tested on loop device. > > > > How the hell is that supposed to work with cold dcache? You don't have > > ->get_parent() there at all... > > > > There *IS* a reference to parent directory in those suckers - not the same > > kind as in normal unix filesystems (".." is not a directory entry there - > > it's all fake), but it's doable. be32_to_cpu(AFFS_TAIL(sb, bh)->parent) > > would be the inumber you need, where bh is the inode block of directory. > > > > So it can be done, but not in this form. NAK for the time being... > > I worked on that function declared optional in > Documentation/filesystems/nfs/Exporting > but was unable to test it. If we're going to reject patches that don't implement get_parent (and I think we should), then we should replace "optional but strongly recommended" there by just "mandatory". Any objections? --b. -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Jan 13, 2017 at 12:39:12PM -0500, J. Bruce Fields wrote: > If we're going to reject patches that don't implement get_parent (and I > think we should), then we should replace "optional but strongly > recommended" there by just "mandatory". Any objections? In theory yes - we'll just need an exception (or dummy implementation) for in-memory file systems like tmpfs. -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Jan 13, 2017 at 10:52:54AM -0800, Christoph Hellwig wrote: > On Fri, Jan 13, 2017 at 12:39:12PM -0500, J. Bruce Fields wrote: > > If we're going to reject patches that don't implement get_parent (and I > > think we should), then we should replace "optional but strongly > > recommended" there by just "mandatory". Any objections? > > In theory yes - we'll just need an exception (or dummy implementation) > for in-memory file systems like tmpfs. Hm, so does get_parent just never get called for tmpfs because the parent's always there already? Should be easy enough to document that exception. --b. -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Jan 13, 2017 at 02:03:57PM -0500, J. Bruce Fields wrote: > On Fri, Jan 13, 2017 at 10:52:54AM -0800, Christoph Hellwig wrote: > > On Fri, Jan 13, 2017 at 12:39:12PM -0500, J. Bruce Fields wrote: > > > If we're going to reject patches that don't implement get_parent (and I > > > think we should), then we should replace "optional but strongly > > > recommended" there by just "mandatory". Any objections? > > > > In theory yes - we'll just need an exception (or dummy implementation) > > for in-memory file systems like tmpfs. > > Hm, so does get_parent just never get called for tmpfs because the > parent's always there already? As the matter of fact, tmpfs *does* have a dummy ->get_parent(). FWIW, the situation right now looks so: 1) most of the local filesystems: get_parent/fh_to_dentry/fh_to_parent. efs, exofs, ext2, ext4, f2fs, fat, jffs2, jfs, ntfs, squashfs, udf, ufs are that way. 2) ditto, but with non-default ->encode_fh() (that's basically those who need more than 32-bit inumber to get to an inode). Often enough those have non-default ->get_name() as well - default one relies upon in-core ->i_ino matching whatever getdents(2) puts into d_ino; besides, there might be a faster way to search. Ones without ->get_name(): fat_nostale, fuse, isofs, nilfs2, ocfs2, reiserfs. Ones with: lustre, btrfs, ceph, gfs2. 3) shmem - essentially as (2), but there ->get_parent() is a dummy and so's ->fh_to_parent(), the latter represented by NULL rather than as an explicit stub. 4) orangefs - no ->fh_to_parent, no ->get_parent. Not sure if orangefs has a way to get khandle of parent by that of directory (i.e. if it can open a disconnected directory by khandle and then obtain a khandle of parent from it). They do include khandle of directory into on-the-wire fhandle, but never actually use that part. Might be blind copying of what other fs are doing, might be something they plan to do but hadn't gotten around to. If it's the latter, that's yet another class 2 (and it'll definitely need a ->get_name() then). 5) befs - ->fh_to_dentry/->fh_to_parent, no ->get_parent. IIRC, they simply don't have a way to find the parent by directory. 6) cifs - stub ->get_parent(), NULL everything else, impossible to use and what the hell is fs/cifs/export.c is doing in the tree, anyway? -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Jan 13, 2017 at 07:57:06PM +0000, Al Viro wrote: > On Fri, Jan 13, 2017 at 02:03:57PM -0500, J. Bruce Fields wrote: > > On Fri, Jan 13, 2017 at 10:52:54AM -0800, Christoph Hellwig wrote: > > > On Fri, Jan 13, 2017 at 12:39:12PM -0500, J. Bruce Fields wrote: > > > > If we're going to reject patches that don't implement get_parent (and I > > > > think we should), then we should replace "optional but strongly > > > > recommended" there by just "mandatory". Any objections? > > > > > > In theory yes - we'll just need an exception (or dummy implementation) > > > for in-memory file systems like tmpfs. > > > > Hm, so does get_parent just never get called for tmpfs because the > > parent's always there already? > > As the matter of fact, tmpfs *does* have a dummy ->get_parent(). > FWIW, the situation right now looks so: Thanks for the summary. > 1) most of the local filesystems: get_parent/fh_to_dentry/fh_to_parent. > efs, exofs, ext2, ext4, f2fs, fat, jffs2, jfs, ntfs, squashfs, udf, ufs are > that way. > 2) ditto, but with non-default ->encode_fh() (that's basically those > who need more than 32-bit inumber to get to an inode). Often enough those > have non-default ->get_name() as well - default one relies upon in-core > ->i_ino matching whatever getdents(2) puts into d_ino; besides, there might > be a faster way to search. Ones without ->get_name(): fat_nostale, fuse, > isofs, nilfs2, ocfs2, reiserfs. Ones with: lustre, btrfs, ceph, gfs2. > 3) shmem - essentially as (2), but there ->get_parent() is > a dummy and so's ->fh_to_parent(), the latter represented by NULL rather > than as an explicit stub. > 4) orangefs - no ->fh_to_parent, no ->get_parent. Not sure if > orangefs has a way to get khandle of parent by that of directory (i.e. > if it can open a disconnected directory by khandle and then obtain > a khandle of parent from it). They do include khandle of directory into > on-the-wire fhandle, but never actually use that part. Might be blind > copying of what other fs are doing, might be something they plan to do > but hadn't gotten around to. If it's the latter, that's yet another class > 2 (and it'll definitely need a ->get_name() then). > 5) befs - ->fh_to_dentry/->fh_to_parent, no ->get_parent. IIRC, > they simply don't have a way to find the parent by directory. > 6) cifs - stub ->get_parent(), NULL everything else, impossible > to use and what the hell is fs/cifs/export.c is doing in the tree, anyway? Dunno, I thought I tried to remove it once some time ago, but my memory may be wrong. It seems like nothing more than a trap for the unwary. --b. -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/fs/affs/affs.h b/fs/affs/affs.h index efe6839..1b55428 100644 --- a/fs/affs/affs.h +++ b/fs/affs/affs.h @@ -162,6 +162,7 @@ extern void affs_free_bitmap(struct super_block *sb); /* namei.c */ +extern const struct export_operations affs_export_ops; extern int affs_hash_name(struct super_block *sb, const u8 *name, unsigned int len); extern struct dentry *affs_lookup(struct inode *dir, struct dentry *dentry, unsigned int); extern int affs_unlink(struct inode *dir, struct dentry *dentry); diff --git a/fs/affs/namei.c b/fs/affs/namei.c index 29186d2..04c3156f 100644 --- a/fs/affs/namei.c +++ b/fs/affs/namei.c @@ -9,6 +9,7 @@ */ #include "affs.h" +#include <linux/exportfs.h> typedef int (*toupper_t)(int); @@ -465,3 +466,42 @@ affs_rename(struct inode *old_dir, struct dentry *old_dentry, affs_brelse(bh); return retval; } + +static struct inode *affs_nfs_get_inode(struct super_block *sb, u64 ino, + u32 generation) +{ + struct inode *inode; + + if (!affs_validblock(sb, ino)) + return ERR_PTR(-ESTALE); + + inode = affs_iget(sb, ino); + if (IS_ERR(inode)) + return ERR_CAST(inode); + + if (generation && inode->i_generation != generation) { + iput(inode); + return ERR_PTR(-ESTALE); + } + + return inode; +} + +static struct dentry *affs_fh_to_dentry(struct super_block *sb, struct fid *fid, + int fh_len, int fh_type) +{ + return generic_fh_to_dentry(sb, fid, fh_len, fh_type, + affs_nfs_get_inode); +} + +static struct dentry *affs_fh_to_parent(struct super_block *sb, struct fid *fid, + int fh_len, int fh_type) +{ + return generic_fh_to_parent(sb, fid, fh_len, fh_type, + affs_nfs_get_inode); +} + +const struct export_operations affs_export_ops = { + .fh_to_dentry = affs_fh_to_dentry, + .fh_to_parent = affs_fh_to_parent, +}; diff --git a/fs/affs/super.c b/fs/affs/super.c index d638486..98bd952 100644 --- a/fs/affs/super.c +++ b/fs/affs/super.c @@ -507,6 +507,7 @@ static int affs_fill_super(struct super_block *sb, void *data, int silent) return -ENOMEM; } + sb->s_export_op = &affs_export_ops; pr_debug("s_flags=%lX\n", sb->s_flags); return 0; }
Add standard functions making AFFS work with NFS. Functions based on ext4 implementation. Tested on loop device. Signed-off-by: Fabian Frederick <fabf@skynet.be> --- fs/affs/affs.h | 1 + fs/affs/namei.c | 40 ++++++++++++++++++++++++++++++++++++++++ fs/affs/super.c | 1 + 3 files changed, 42 insertions(+)