On Tue, 28 Sept 2021 at 14:48, Chengguang Xu <cgxu519@xxxxxxxxxxxx> 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@xxxxxxxx>
> Signed-off-by: Chengguang Xu <cgxu519@xxxxxxxxxxxx>
Can you please add Fixes: and Cc: stable@xxxxxxxxxxxxxxx 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
