Re: [PATCH] ovl: set overlayfs inode's a_ops->direct_IO properly

[Miklos Szeredi <miklos@xxxxxxxxxx>]

On Sun, Oct 03, 2021 at 10:41:34PM +0800, Chengguang Xu wrote:
> ---- 在 星期四, 2021-09-30 20:55:54 Miklos Szeredi <miklos@xxxxxxxxxx> 撰写 ----

>  > OVL_UPPERDATA is only set after ovl_get_inode() in all callers.  This
>  > needs to be moved into ovl_inode_init() before calling
>  > ovl_inode_set_aops() otherwise this won't work correctly for a copied
>  > up file.
>  > 
> 
> Hi Miklos,
> 
> I found it's not convenient to move setting OVL_UPPERDATA into ovl_inode_init() because

If you look at the logic of the thing, then it becomes quite simple.  See
following (untested) patch.

Thanks,
Miklos

---
 fs/overlayfs/dir.c       |    3 +--
 fs/overlayfs/export.c    |    5 ++---
 fs/overlayfs/inode.c     |    3 +++
 fs/overlayfs/namei.c     |    3 +--
 fs/overlayfs/overlayfs.h |    1 +
 5 files changed, 8 insertions(+), 7 deletions(-)

--- a/fs/overlayfs/dir.c
+++ b/fs/overlayfs/dir.c
@@ -264,6 +264,7 @@ static int ovl_instantiate(struct dentry
 	struct ovl_inode_params oip = {
 		.upperdentry = newdentry,
 		.newinode = inode,
+		.metacopy = false,
 	};
 
 	ovl_dir_modified(dentry->d_parent, false);
@@ -287,8 +288,6 @@ static int ovl_instantiate(struct dentry
 		inode = ovl_get_inode(dentry->d_sb, &oip);
 		if (IS_ERR(inode))
 			return PTR_ERR(inode);
-		if (inode == oip.newinode)
-			ovl_set_flag(OVL_UPPERDATA, inode);
 	} else {
 		WARN_ON(ovl_inode_real(inode) != d_inode(newdentry));
 		dput(newdentry);
--- a/fs/overlayfs/export.c
+++ b/fs/overlayfs/export.c
@@ -290,7 +290,8 @@ static struct dentry *ovl_obtain_alias(s
 	struct ovl_inode_params oip = {
 		.lowerpath = lowerpath,
 		.index = index,
-		.numlower = !!lower
+		.numlower = !!lower,
+		.metacopy = false, /* No NFS export support for metacopy yet */
 	};
 
 	/* We get overlay directory dentries with ovl_lookup_real() */
@@ -304,8 +305,6 @@ static struct dentry *ovl_obtain_alias(s
 		return ERR_CAST(inode);
 	}
 
-	if (upper)
-		ovl_set_flag(OVL_UPPERDATA, inode);
 
 	dentry = d_find_any_alias(inode);
 	if (dentry)
--- a/fs/overlayfs/inode.c
+++ b/fs/overlayfs/inode.c
@@ -1155,6 +1155,9 @@ struct inode *ovl_get_inode(struct super
 		ino = realinode->i_ino;
 		fsid = lowerpath->layer->fsid;
 	}
+	if (upperdentry && !oip->metacopy)
+		ovl_set_flag(OVL_UPPERDATA, inode);
+
 	ovl_fill_inode(inode, realinode->i_mode, realinode->i_rdev);
 	ovl_inode_init(inode, oip, ino, fsid);
 
--- a/fs/overlayfs/namei.c
+++ b/fs/overlayfs/namei.c
@@ -1093,14 +1093,13 @@ struct dentry *ovl_lookup(struct inode *
 			.redirect = upperredirect,
 			.lowerdata = (ctr > 1 && !d.is_dir) ?
 				      stack[ctr - 1].dentry : NULL,
+			.metacopy = uppermetacopy,
 		};
 
 		inode = ovl_get_inode(dentry->d_sb, &oip);
 		err = PTR_ERR(inode);
 		if (IS_ERR(inode))
 			goto out_free_oe;
-		if (upperdentry && !uppermetacopy)
-			ovl_set_flag(OVL_UPPERDATA, inode);
 	}
 
 	ovl_dentry_update_reval(dentry, upperdentry,
--- a/fs/overlayfs/overlayfs.h
+++ b/fs/overlayfs/overlayfs.h
@@ -507,6 +507,7 @@ struct ovl_inode_params {
 	struct dentry *upperdentry;
 	struct ovl_path *lowerpath;
 	bool index;
+	bool metacopy;
 	unsigned int numlower;
 	char *redirect;
 	struct dentry *lowerdata;