On Tue, Apr 18, 2023 at 10:55 AM Alexander Larsson <alexl@xxxxxxxxxx> wrote:
>
> On Sat, 2023-04-08 at 19:43 +0300, Amir Goldstein wrote:
> > The lower stacks of all the ovl inode aliases should be identical
> > and there is redundant information in ovl_entry and ovl_inode.
> >
> > Move lowerstack into ovl_inode and keep only the OVL_E_FLAGS
> > per overlay dentry.
> >
> > Following patches will deduplicate redundant ovl_inode fields.
> >
> > Note that for a negative dentry, OVL_E(dentry) can now be NULL,
> > so it is imporatnt to use the ovl_numlower() accessor.
> >
> > Signed-off-by: Amir Goldstein <amir73il@xxxxxxxxx>
>
>
> Reviewed-by: Alexander Larsson <alexl@xxxxxxxxxx>
>
> > ---
> > fs/overlayfs/dir.c | 2 +-
> > fs/overlayfs/export.c | 22 ++++++++++++----------
> > fs/overlayfs/inode.c | 8 ++++----
> > fs/overlayfs/namei.c | 5 ++---
> > fs/overlayfs/overlayfs.h | 6 ++++--
> > fs/overlayfs/ovl_entry.h | 36 ++++++++++++++++++------------------
> > fs/overlayfs/super.c | 18 ++++--------------
> > fs/overlayfs/util.c | 8 ++++----
> > 8 files changed, 49 insertions(+), 56 deletions(-)
> >
> > diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c
> > index 9be52d8013c8..92bdcedfaaec 100644
> > --- a/fs/overlayfs/dir.c
> > +++ b/fs/overlayfs/dir.c
> > @@ -269,7 +269,7 @@ static int ovl_instantiate(struct dentry *dentry,
> > struct inode *inode,
> >
> > ovl_dir_modified(dentry->d_parent, false);
> > ovl_dentry_set_upper_alias(dentry);
> > - ovl_dentry_init_reval(dentry, newdentry);
> > + ovl_dentry_init_reval(dentry, newdentry, NULL);
> >
> > if (!hardlink) {
> > /*
> > diff --git a/fs/overlayfs/export.c b/fs/overlayfs/export.c
> > index ddb546627749..d4caf57c8e17 100644
> > --- a/fs/overlayfs/export.c
> > +++ b/fs/overlayfs/export.c
> > @@ -286,7 +286,7 @@ static struct dentry *ovl_obtain_alias(struct
> > super_block *sb,
> > struct dentry *lower = lowerpath ? lowerpath->dentry : NULL;
> > struct dentry *upper = upper_alias ?: index;
> > struct dentry *dentry;
> > - struct inode *inode;
> > + struct inode *inode = NULL;
> > struct ovl_entry *oe;
> > struct ovl_inode_params oip = {
> > .lowerpath = lowerpath,
> > @@ -298,9 +298,19 @@ static struct dentry *ovl_obtain_alias(struct
> > super_block *sb,
> > if (d_is_dir(upper ?: lower))
> > return ERR_PTR(-EIO);
> >
> > + oe = ovl_alloc_entry(!!lower);
> > + if (!oe)
> > + goto nomem;
>
> You goto nomem here, but that will dput(dentry), and dentry is not
> initialized yet. I think you should just return an error directly here.
>
Good catch!
Thanks for the review.
Amir.
