---- 在 星期四, 2021-09-30 20:55:54 Miklos Szeredi <miklos@xxxxxxxxxx> 撰写 ----
> 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.
>
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
