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

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

[Chengguang Xu]

Loop device checks the ability of DIRECT-IO by checking
a_ops->direct_IO of inode, in order to avoid this kind of
false detection we set a_ops->direct_IO for overlayfs inode
only when underlying inode really has DIRECT-IO ability.

Reported-by: Huang Jianan <huangjianan@oppo.com>
Signed-off-by: Chengguang Xu <cgxu519@mykernel.net>
---
 fs/overlayfs/dir.c       |  2 ++
 fs/overlayfs/inode.c     |  4 ++--
 fs/overlayfs/overlayfs.h |  1 +
 fs/overlayfs/util.c      | 14 ++++++++++++++
 4 files changed, 19 insertions(+), 2 deletions(-)

diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c
index 1fefb2b8960e..32a60f9e3f9e 100644
--- a/fs/overlayfs/dir.c
+++ b/fs/overlayfs/dir.c
@@ -648,6 +648,8 @@ static int ovl_create_object(struct dentry *dentry, int mode, dev_t rdev,
 	/* Did we end up using the preallocated inode? */
 	if (inode != d_inode(dentry))
 		iput(inode);
+	else
+		ovl_inode_set_aops(inode);
 
 out_drop_write:
 	ovl_drop_write(dentry);
diff --git a/fs/overlayfs/inode.c b/fs/overlayfs/inode.c
index 832b17589733..a7a327e4f790 100644
--- a/fs/overlayfs/inode.c
+++ b/fs/overlayfs/inode.c
@@ -659,7 +659,7 @@ static const struct inode_operations ovl_special_inode_operations = {
 	.update_time	= ovl_update_time,
 };
 
-static const struct address_space_operations ovl_aops = {
+const struct address_space_operations ovl_aops = {
 	/* For O_DIRECT dentry_open() checks f_mapping->a_ops->direct_IO */
 	.direct_IO		= noop_direct_IO,
 };
@@ -786,6 +786,7 @@ void ovl_inode_init(struct inode *inode, struct ovl_inode_params *oip,
 	ovl_copyattr(realinode, inode);
 	ovl_copyflags(realinode, inode);
 	ovl_map_ino(inode, ino, fsid);
+	ovl_inode_set_aops(inode);
 }
 
 static void ovl_fill_inode(struct inode *inode, umode_t mode, dev_t rdev)
@@ -802,7 +803,6 @@ static void ovl_fill_inode(struct inode *inode, umode_t mode, dev_t rdev)
 	case S_IFREG:
 		inode->i_op = &ovl_file_inode_operations;
 		inode->i_fop = &ovl_file_operations;
-		inode->i_mapping->a_ops = &ovl_aops;
 		break;
 
 	case S_IFDIR:
diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h
index 3894f3347955..976c9d634293 100644
--- a/fs/overlayfs/overlayfs.h
+++ b/fs/overlayfs/overlayfs.h
@@ -349,6 +349,7 @@ bool ovl_is_metacopy_dentry(struct dentry *dentry);
 char *ovl_get_redirect_xattr(struct ovl_fs *ofs, struct dentry *dentry,
 			     int padding);
 int ovl_sync_status(struct ovl_fs *ofs);
+void ovl_inode_set_aops(struct inode *inode);
 
 static inline void ovl_set_flag(unsigned long flag, struct inode *inode)
 {
diff --git a/fs/overlayfs/util.c b/fs/overlayfs/util.c
index f48284a2a896..33535dbee1c3 100644
--- a/fs/overlayfs/util.c
+++ b/fs/overlayfs/util.c
@@ -1060,3 +1060,17 @@ int ovl_sync_status(struct ovl_fs *ofs)
 
 	return errseq_check(&mnt->mnt_sb->s_wb_err, ofs->errseq);
 }
+
+extern const struct address_space_operations ovl_aops;
+void ovl_inode_set_aops(struct inode *inode)
+{
+	struct inode *realinode;
+
+	if (!S_ISREG(inode->i_mode))
+		return;
+
+	realinode = ovl_inode_realdata(inode);
+	if (realinode && realinode->i_mapping && realinode->i_mapping->a_ops &&
+	    realinode->i_mapping->a_ops->direct_IO)
+		inode->i_mapping->a_ops = &ovl_aops;
+}
-- 
2.27.0

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

[Huang Jianan]

This patch can ensure that loop devices based on erofs and overlayfs 
can't set dio through __loop_update_dio.

Tested-by: Huang Jianan <huangjianan@oppo.com>

Thanks,
Jianan

在 2021/9/28 20:47, Chengguang Xu 写道:
> Loop device checks the ability of DIRECT-IO by checking
> a_ops->direct_IO of inode, in order to avoid this kind of
> false detection we set a_ops->direct_IO for overlayfs inode
> only when underlying inode really has DIRECT-IO ability.
>
> Reported-by: Huang Jianan <huangjianan@oppo.com>
> Signed-off-by: Chengguang Xu <cgxu519@mykernel.net>
> ---
>   fs/overlayfs/dir.c       |  2 ++
>   fs/overlayfs/inode.c     |  4 ++--
>   fs/overlayfs/overlayfs.h |  1 +
>   fs/overlayfs/util.c      | 14 ++++++++++++++
>   4 files changed, 19 insertions(+), 2 deletions(-)
>
> diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c
> index 1fefb2b8960e..32a60f9e3f9e 100644
> --- a/fs/overlayfs/dir.c
> +++ b/fs/overlayfs/dir.c
> @@ -648,6 +648,8 @@ static int ovl_create_object(struct dentry *dentry, int mode, dev_t rdev,
>   	/* Did we end up using the preallocated inode? */
>   	if (inode != d_inode(dentry))
>   		iput(inode);
> +	else
> +		ovl_inode_set_aops(inode);
>   
>   out_drop_write:
>   	ovl_drop_write(dentry);
> diff --git a/fs/overlayfs/inode.c b/fs/overlayfs/inode.c
> index 832b17589733..a7a327e4f790 100644
> --- a/fs/overlayfs/inode.c
> +++ b/fs/overlayfs/inode.c
> @@ -659,7 +659,7 @@ static const struct inode_operations ovl_special_inode_operations = {
>   	.update_time	= ovl_update_time,
>   };
>   
> -static const struct address_space_operations ovl_aops = {
> +const struct address_space_operations ovl_aops = {
>   	/* For O_DIRECT dentry_open() checks f_mapping->a_ops->direct_IO */
>   	.direct_IO		= noop_direct_IO,
>   };
> @@ -786,6 +786,7 @@ void ovl_inode_init(struct inode *inode, struct ovl_inode_params *oip,
>   	ovl_copyattr(realinode, inode);
>   	ovl_copyflags(realinode, inode);
>   	ovl_map_ino(inode, ino, fsid);
> +	ovl_inode_set_aops(inode);
>   }
>   
>   static void ovl_fill_inode(struct inode *inode, umode_t mode, dev_t rdev)
> @@ -802,7 +803,6 @@ static void ovl_fill_inode(struct inode *inode, umode_t mode, dev_t rdev)
>   	case S_IFREG:
>   		inode->i_op = &ovl_file_inode_operations;
>   		inode->i_fop = &ovl_file_operations;
> -		inode->i_mapping->a_ops = &ovl_aops;
>   		break;
>   
>   	case S_IFDIR:
> diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h
> index 3894f3347955..976c9d634293 100644
> --- a/fs/overlayfs/overlayfs.h
> +++ b/fs/overlayfs/overlayfs.h
> @@ -349,6 +349,7 @@ bool ovl_is_metacopy_dentry(struct dentry *dentry);
>   char *ovl_get_redirect_xattr(struct ovl_fs *ofs, struct dentry *dentry,
>   			     int padding);
>   int ovl_sync_status(struct ovl_fs *ofs);
> +void ovl_inode_set_aops(struct inode *inode);
>   
>   static inline void ovl_set_flag(unsigned long flag, struct inode *inode)
>   {
> diff --git a/fs/overlayfs/util.c b/fs/overlayfs/util.c
> index f48284a2a896..33535dbee1c3 100644
> --- a/fs/overlayfs/util.c
> +++ b/fs/overlayfs/util.c
> @@ -1060,3 +1060,17 @@ int ovl_sync_status(struct ovl_fs *ofs)
>   
>   	return errseq_check(&mnt->mnt_sb->s_wb_err, ofs->errseq);
>   }
> +
> +extern const struct address_space_operations ovl_aops;
> +void ovl_inode_set_aops(struct inode *inode)
> +{
> +	struct inode *realinode;
> +
> +	if (!S_ISREG(inode->i_mode))
> +		return;
> +
> +	realinode = ovl_inode_realdata(inode);
> +	if (realinode && realinode->i_mapping && realinode->i_mapping->a_ops &&
> +	    realinode->i_mapping->a_ops->direct_IO)
> +		inode->i_mapping->a_ops = &ovl_aops;
> +}

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

[Chengguang Xu]

在 2021/9/30 14:52, Huang Jianan 写道:
> This patch can ensure that loop devices based on erofs and overlayfs 
> can't set dio through __loop_update_dio.
>
> Tested-by: Huang Jianan <huangjianan@oppo.com>

HI Jianan,

Thanks for the test!



>
> Thanks,
> Jianan
>
> 在 2021/9/28 20:47, Chengguang Xu 写道:
>> Loop device checks the ability of DIRECT-IO by checking
>> a_ops->direct_IO of inode, in order to avoid this kind of
>> false detection we set a_ops->direct_IO for overlayfs inode
>> only when underlying inode really has DIRECT-IO ability.
>>
>> Reported-by: Huang Jianan <huangjianan@oppo.com>
>> Signed-off-by: Chengguang Xu <cgxu519@mykernel.net>
>> ---
>>   fs/overlayfs/dir.c       |  2 ++
>>   fs/overlayfs/inode.c     |  4 ++--
>>   fs/overlayfs/overlayfs.h |  1 +
>>   fs/overlayfs/util.c      | 14 ++++++++++++++
>>   4 files changed, 19 insertions(+), 2 deletions(-)
>>
>> diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c
>> index 1fefb2b8960e..32a60f9e3f9e 100644
>> --- a/fs/overlayfs/dir.c
>> +++ b/fs/overlayfs/dir.c
>> @@ -648,6 +648,8 @@ static int ovl_create_object(struct dentry 
>> *dentry, int mode, dev_t rdev,
>>       /* Did we end up using the preallocated inode? */
>>       if (inode != d_inode(dentry))
>>           iput(inode);
>> +    else
>> +        ovl_inode_set_aops(inode);
>>     out_drop_write:
>>       ovl_drop_write(dentry);
>> diff --git a/fs/overlayfs/inode.c b/fs/overlayfs/inode.c
>> index 832b17589733..a7a327e4f790 100644
>> --- a/fs/overlayfs/inode.c
>> +++ b/fs/overlayfs/inode.c
>> @@ -659,7 +659,7 @@ static const struct inode_operations 
>> ovl_special_inode_operations = {
>>       .update_time    = ovl_update_time,
>>   };
>>   -static const struct address_space_operations ovl_aops = {
>> +const struct address_space_operations ovl_aops = {
>>       /* For O_DIRECT dentry_open() checks 
>> f_mapping->a_ops->direct_IO */
>>       .direct_IO        = noop_direct_IO,
>>   };
>> @@ -786,6 +786,7 @@ void ovl_inode_init(struct inode *inode, struct 
>> ovl_inode_params *oip,
>>       ovl_copyattr(realinode, inode);
>>       ovl_copyflags(realinode, inode);
>>       ovl_map_ino(inode, ino, fsid);
>> +    ovl_inode_set_aops(inode);
>>   }
>>     static void ovl_fill_inode(struct inode *inode, umode_t mode, 
>> dev_t rdev)
>> @@ -802,7 +803,6 @@ static void ovl_fill_inode(struct inode *inode, 
>> umode_t mode, dev_t rdev)
>>       case S_IFREG:
>>           inode->i_op = &ovl_file_inode_operations;
>>           inode->i_fop = &ovl_file_operations;
>> -        inode->i_mapping->a_ops = &ovl_aops;
>>           break;
>>         case S_IFDIR:
>> diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h
>> index 3894f3347955..976c9d634293 100644
>> --- a/fs/overlayfs/overlayfs.h
>> +++ b/fs/overlayfs/overlayfs.h
>> @@ -349,6 +349,7 @@ bool ovl_is_metacopy_dentry(struct dentry *dentry);
>>   char *ovl_get_redirect_xattr(struct ovl_fs *ofs, struct dentry 
>> *dentry,
>>                    int padding);
>>   int ovl_sync_status(struct ovl_fs *ofs);
>> +void ovl_inode_set_aops(struct inode *inode);
>>     static inline void ovl_set_flag(unsigned long flag, struct inode 
>> *inode)
>>   {
>> diff --git a/fs/overlayfs/util.c b/fs/overlayfs/util.c
>> index f48284a2a896..33535dbee1c3 100644
>> --- a/fs/overlayfs/util.c
>> +++ b/fs/overlayfs/util.c
>> @@ -1060,3 +1060,17 @@ int ovl_sync_status(struct ovl_fs *ofs)
>>         return errseq_check(&mnt->mnt_sb->s_wb_err, ofs->errseq);
>>   }
>> +
>> +extern const struct address_space_operations ovl_aops;
>> +void ovl_inode_set_aops(struct inode *inode)
>> +{
>> +    struct inode *realinode;
>> +
>> +    if (!S_ISREG(inode->i_mode))
>> +        return;
>> +
>> +    realinode = ovl_inode_realdata(inode);
>> +    if (realinode && realinode->i_mapping && 
>> realinode->i_mapping->a_ops &&
>> +        realinode->i_mapping->a_ops->direct_IO)
>> +        inode->i_mapping->a_ops = &ovl_aops;
>> +}
>

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

[Miklos Szeredi]

On Thu, 30 Sept 2021 at 08:52, Huang Jianan <huangjianan@oppo.com> wrote:
>
> This patch can ensure that loop devices based on erofs and overlayfs
> can't set dio through __loop_update_dio.

So does this mean that you tested the "loop on overlayfs on erofs"
setup and it works?

Thanks,
Miklos

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

[Huang Jianan]

在 2021/9/30 16:02, Miklos Szeredi 写道:
> On Thu, 30 Sept 2021 at 08:52, Huang Jianan <huangjianan@oppo.com> wrote:
>> This patch can ensure that loop devices based on erofs and overlayfs
>> can't set dio through __loop_update_dio.
> So does this mean that you tested the "loop on overlayfs on erofs"
> setup and it works?
Yes, I think this has always been able to work normally. But recently we
found  that the direct_IO flag will be set incorrectly in the Android apex
scenario, which leads to oops.

With this patch, Android apex can work normally through loop on overlayfs
on erofs using bufferd IO.

Thanks,
Jianan
> Thanks,
> Miklos

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

[Miklos Szeredi]

On Tue, 28 Sept 2021 at 14:48, Chengguang Xu <cgxu519@mykernel.net> wrote:
>
> Loop device checks the ability of DIRECT-IO by checking
> a_ops->direct_IO of inode, in order to avoid this kind of
> false detection we set a_ops->direct_IO for overlayfs inode
> only when underlying inode really has DIRECT-IO ability.
>
> Reported-by: Huang Jianan <huangjianan@oppo.com>
> Signed-off-by: Chengguang Xu <cgxu519@mykernel.net>

Can you please add  Fixes: and  Cc: stable@vger.kernel.org tags?

> ---
>  fs/overlayfs/dir.c       |  2 ++
>  fs/overlayfs/inode.c     |  4 ++--
>  fs/overlayfs/overlayfs.h |  1 +
>  fs/overlayfs/util.c      | 14 ++++++++++++++
>  4 files changed, 19 insertions(+), 2 deletions(-)
>
> diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c
> index 1fefb2b8960e..32a60f9e3f9e 100644
> --- a/fs/overlayfs/dir.c
> +++ b/fs/overlayfs/dir.c
> @@ -648,6 +648,8 @@ static int ovl_create_object(struct dentry *dentry, int mode, dev_t rdev,
>         /* Did we end up using the preallocated inode? */
>         if (inode != d_inode(dentry))
>                 iput(inode);
> +       else
> +               ovl_inode_set_aops(inode);

This is too late, since the dentry was instantiated and can be found
through a cached lookup already.

Anyway, I think this can be dropped, since ovl_inode_init() should be
called for inodes preallocated by ovl_create_object() as well:
inode_insert5() will set I_NEW on the preallocated inode.

It is interesting that ovl_fill_inode() will be called a second time
on the preallocated inode.  This is something that should probably be
cleaned up, but that's a separate patch.

>
>  out_drop_write:
>         ovl_drop_write(dentry);
> diff --git a/fs/overlayfs/inode.c b/fs/overlayfs/inode.c
> index 832b17589733..a7a327e4f790 100644
> --- a/fs/overlayfs/inode.c
> +++ b/fs/overlayfs/inode.c
> @@ -659,7 +659,7 @@ static const struct inode_operations ovl_special_inode_operations = {
>         .update_time    = ovl_update_time,
>  };
>
> -static const struct address_space_operations ovl_aops = {
> +const struct address_space_operations ovl_aops = {
>         /* For O_DIRECT dentry_open() checks f_mapping->a_ops->direct_IO */
>         .direct_IO              = noop_direct_IO,
>  };
> @@ -786,6 +786,7 @@ void ovl_inode_init(struct inode *inode, struct ovl_inode_params *oip,
>         ovl_copyattr(realinode, inode);
>         ovl_copyflags(realinode, inode);
>         ovl_map_ino(inode, ino, fsid);
> +       ovl_inode_set_aops(inode);

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.

Thanks,
Miklos

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

[Chengguang Xu]

---- 在 星期四, 2021-09-30 20:55:54 Miklos Szeredi <miklos@szeredi.hu> 撰写 ----
 > On Tue, 28 Sept 2021 at 14:48, Chengguang Xu <cgxu519@mykernel.net> wrote:
 > >
 > > Loop device checks the ability of DIRECT-IO by checking
 > > a_ops->direct_IO of inode, in order to avoid this kind of
 > > false detection we set a_ops->direct_IO for overlayfs inode
 > > only when underlying inode really has DIRECT-IO ability.
 > >
 > > Reported-by: Huang Jianan <huangjianan@oppo.com>
 > > Signed-off-by: Chengguang Xu <cgxu519@mykernel.net>
 > 
 > Can you please add  Fixes: and  Cc: stable@vger.kernel.org tags?
 > 
 > > ---
 > >  fs/overlayfs/dir.c       |  2 ++
 > >  fs/overlayfs/inode.c     |  4 ++--
 > >  fs/overlayfs/overlayfs.h |  1 +
 > >  fs/overlayfs/util.c      | 14 ++++++++++++++
 > >  4 files changed, 19 insertions(+), 2 deletions(-)
 > >
 > > diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c
 > > index 1fefb2b8960e..32a60f9e3f9e 100644
 > > --- a/fs/overlayfs/dir.c
 > > +++ b/fs/overlayfs/dir.c
 > > @@ -648,6 +648,8 @@ static int ovl_create_object(struct dentry *dentry, int mode, dev_t rdev,
 > >         /* Did we end up using the preallocated inode? */
 > >         if (inode != d_inode(dentry))
 > >                 iput(inode);
 > > +       else
 > > +               ovl_inode_set_aops(inode);
 > 
 > This is too late, since the dentry was instantiated and can be found
 > through a cached lookup already.
 > 
 > Anyway, I think this can be dropped, since ovl_inode_init() should be
 > called for inodes preallocated by ovl_create_object() as well:
 > inode_insert5() will set I_NEW on the preallocated inode.
 > 
 > It is interesting that ovl_fill_inode() will be called a second time
 > on the preallocated inode.  This is something that should probably be
 > cleaned up, but that's a separate patch.
 > 
 > >
 > >  out_drop_write:
 > >         ovl_drop_write(dentry);
 > > diff --git a/fs/overlayfs/inode.c b/fs/overlayfs/inode.c
 > > index 832b17589733..a7a327e4f790 100644
 > > --- a/fs/overlayfs/inode.c
 > > +++ b/fs/overlayfs/inode.c
 > > @@ -659,7 +659,7 @@ static const struct inode_operations ovl_special_inode_operations = {
 > >         .update_time    = ovl_update_time,
 > >  };
 > >
 > > -static const struct address_space_operations ovl_aops = {
 > > +const struct address_space_operations ovl_aops = {
 > >         /* For O_DIRECT dentry_open() checks f_mapping->a_ops->direct_IO */
 > >         .direct_IO              = noop_direct_IO,
 > >  };
 > > @@ -786,6 +786,7 @@ void ovl_inode_init(struct inode *inode, struct ovl_inode_params *oip,
 > >         ovl_copyattr(realinode, inode);
 > >         ovl_copyflags(realinode, inode);
 > >         ovl_map_ino(inode, ino, fsid);
 > > +       ovl_inode_set_aops(inode);
 > 
 > 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
we should detect different conditions for different callers. How about calling  ovl_inode_set_aops()
after setting OVL_UPPERDATA?


Thanks,
Chengguang

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

[Miklos Szeredi]

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

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

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

[Chengguang Xu]

 ---- 在 星期四, 2021-10-07 20:09:59 Miklos Szeredi <miklos@szeredi.hu> 撰写 ----
 > On Sun, Oct 03, 2021 at 10:41:34PM +0800, Chengguang Xu wrote:
 > > ---- 在 星期四, 2021-09-30 20:55:54 Miklos Szeredi <miklos@szeredi.hu> 撰写 ----
 > 
 > >  > 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.
 > 

Hi Miklos,

Okay, thanks for the suggestion. I'll check fot it.

Thanks,
Chengguang