linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Amir Goldstein <amir73il@gmail.com>
To: Miklos Szeredi <mszeredi@redhat.com>
Cc: overlayfs <linux-unionfs@vger.kernel.org>,
	linux-fsdevel <linux-fsdevel@vger.kernel.org>,
	linux-kernel <linux-kernel@vger.kernel.org>,
	Al Viro <viro@zeniv.linux.org.uk>
Subject: Re: [PATCH 05/11] ovl: return dentry from ovl_create_real()
Date: Tue, 29 May 2018 18:24:06 +0300	[thread overview]
Message-ID: <CAOQ4uxh0XUgOZ-tZ_1jTxBLFAdWv53kamh-1=hTuLkVpEVA08w@mail.gmail.com> (raw)
In-Reply-To: <20180529144143.16378-6-mszeredi@redhat.com>

On Tue, May 29, 2018 at 5:41 PM, Miklos Szeredi <mszeredi@redhat.com> wrote:
> From: Amir Goldstein <amir73il@gmail.com>
>
> Al Viro suggested to simplify callers of ovl_create_real() by
> returning the created dentry (or ERR_PTR) from ovl_create_real().
> This prep patch makes Al's patch change less callers.
>
> Also used ovl_create_temp() in ovl_create_index() instead of calling
> ovl_do_mkdir() directly, so now all callers of ovl_do_mkdir() are routed
> through ovl_create_real(), which paves the way for Al's fix for non-hashed
> result from vfs_mkdir().
>

It's good that you split this patch from ovl_create_temp() patch, but
the commit message it utterly not related to the patch anymore and
possibly should retain Al's authorship cause there is no relic of my
work in this patch.

Thanks,
Amir.

> Suggested-by: Al Viro <viro@zeniv.linux.org.uk>
> Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
> ---
>  fs/overlayfs/copy_up.c   | 15 ++++--------
>  fs/overlayfs/dir.c       | 62 +++++++++++++++++++++++-------------------------
>  fs/overlayfs/overlayfs.h |  4 ++--
>  fs/overlayfs/super.c     |  7 +++---
>  4 files changed, 40 insertions(+), 48 deletions(-)
>
> diff --git a/fs/overlayfs/copy_up.c b/fs/overlayfs/copy_up.c
> index 5f4c78b1bbeb..d3e9c1eeb7a4 100644
> --- a/fs/overlayfs/copy_up.c
> +++ b/fs/overlayfs/copy_up.c
> @@ -502,19 +502,12 @@ static int ovl_get_tmpfile(struct ovl_copy_up_ctx *c, struct dentry **tempp)
>
>         if (c->tmpfile) {
>                 temp = ovl_do_tmpfile(c->workdir, c->stat.mode);
> -               if (IS_ERR(temp))
> -                       goto temp_err;
>         } else {
> -               temp = ovl_lookup_temp(c->workdir);
> -               if (IS_ERR(temp))
> -                       goto temp_err;
> -
> -               err = ovl_create_real(d_inode(c->workdir), temp, &cattr);
> -               if (err) {
> -                       dput(temp);
> -                       goto out;
> -               }
> +               temp = ovl_create_real(d_inode(c->workdir),
> +                                      ovl_lookup_temp(c->workdir), &cattr);
>         }
> +       if (IS_ERR(temp))
> +               goto temp_err;
>         err = 0;
>         *tempp = temp;
>  out:
> diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c
> index 0fb3ef85f298..425ddb098c4a 100644
> --- a/fs/overlayfs/dir.c
> +++ b/fs/overlayfs/dir.c
> @@ -114,13 +114,17 @@ int ovl_cleanup_and_whiteout(struct dentry *workdir, struct inode *dir,
>         goto out;
>  }
>
> -int ovl_create_real(struct inode *dir, struct dentry *newdentry,
> -                   struct ovl_cattr *attr)
> +struct dentry *ovl_create_real(struct inode *dir, struct dentry *newdentry,
> +                              struct ovl_cattr *attr)
>  {
>         int err;
>
> +       if (IS_ERR(newdentry))
> +               return newdentry;
> +
> +       err = -ESTALE;
>         if (newdentry->d_inode)
> -               return -ESTALE;
> +               goto out;
>
>         if (attr->hardlink) {
>                 err = ovl_do_link(attr->hardlink, dir, newdentry);
> @@ -157,7 +161,12 @@ int ovl_create_real(struct inode *dir, struct dentry *newdentry,
>                  */
>                 err = -ENOENT;
>         }
> -       return err;
> +out:
> +       if (err) {
> +               dput(newdentry);
> +               return ERR_PTR(err);
> +       }
> +       return newdentry;
>  }
>
>  static int ovl_set_opaque_xerr(struct dentry *dentry, struct dentry *upper,
> @@ -224,14 +233,14 @@ static int ovl_create_upper(struct dentry *dentry, struct inode *inode,
>                 attr->mode &= ~current_umask();
>
>         inode_lock_nested(udir, I_MUTEX_PARENT);
> -       newdentry = lookup_one_len(dentry->d_name.name, upperdir,
> -                                  dentry->d_name.len);
> +       newdentry = ovl_create_real(udir,
> +                                   lookup_one_len(dentry->d_name.name,
> +                                                  upperdir,
> +                                                  dentry->d_name.len),
> +                                   attr);
>         err = PTR_ERR(newdentry);
>         if (IS_ERR(newdentry))
>                 goto out_unlock;
> -       err = ovl_create_real(udir, newdentry, attr);
> -       if (err)
> -               goto out_dput;
>
>         if (ovl_type_merge(dentry->d_parent) && d_is_dir(newdentry)) {
>                 /* Setting opaque here is just an optimization, allow to fail */
> @@ -239,9 +248,7 @@ static int ovl_create_upper(struct dentry *dentry, struct inode *inode,
>         }
>
>         ovl_instantiate(dentry, inode, newdentry, !!attr->hardlink);
> -       newdentry = NULL;
> -out_dput:
> -       dput(newdentry);
> +       err = 0;
>  out_unlock:
>         inode_unlock(udir);
>         return err;
> @@ -280,15 +287,12 @@ static struct dentry *ovl_clear_empty(struct dentry *dentry,
>         if (upper->d_parent->d_inode != udir)
>                 goto out_unlock;
>
> -       opaquedir = ovl_lookup_temp(workdir);
> +       opaquedir = ovl_create_real(wdir, ovl_lookup_temp(workdir),
> +                                   OVL_CATTR(stat.mode));
>         err = PTR_ERR(opaquedir);
>         if (IS_ERR(opaquedir))
>                 goto out_unlock;
>
> -       err = ovl_create_real(wdir, opaquedir, OVL_CATTR(stat.mode));
> -       if (err)
> -               goto out_dput;
> -
>         err = ovl_copy_xattr(upper, opaquedir);
>         if (err)
>                 goto out_cleanup;
> @@ -318,7 +322,6 @@ static struct dentry *ovl_clear_empty(struct dentry *dentry,
>
>  out_cleanup:
>         ovl_cleanup(wdir, opaquedir);
> -out_dput:
>         dput(opaquedir);
>  out_unlock:
>         unlock_rename(workdir, upperdir);
> @@ -379,20 +382,16 @@ static int ovl_create_over_whiteout(struct dentry *dentry, struct inode *inode,
>         if (err)
>                 goto out;
>
> -       newdentry = ovl_lookup_temp(workdir);
> -       err = PTR_ERR(newdentry);
> -       if (IS_ERR(newdentry))
> -               goto out_unlock;
> -
>         upper = lookup_one_len(dentry->d_name.name, upperdir,
>                                dentry->d_name.len);
>         err = PTR_ERR(upper);
>         if (IS_ERR(upper))
> -               goto out_dput;
> +               goto out_unlock;
>
> -       err = ovl_create_real(wdir, newdentry, cattr);
> -       if (err)
> -               goto out_dput2;
> +       newdentry = ovl_create_real(wdir, ovl_lookup_temp(workdir), cattr);
> +       err = PTR_ERR(newdentry);
> +       if (IS_ERR(newdentry))
> +               goto out_dput;
>
>         /*
>          * mode could have been mutilated due to umask (e.g. sgid directory)
> @@ -439,11 +438,9 @@ static int ovl_create_over_whiteout(struct dentry *dentry, struct inode *inode,
>                         goto out_cleanup;
>         }
>         ovl_instantiate(dentry, inode, newdentry, hardlink);
> -       newdentry = NULL;
> -out_dput2:
> -       dput(upper);
> +       err = 0;
>  out_dput:
> -       dput(newdentry);
> +       dput(upper);
>  out_unlock:
>         unlock_rename(workdir, upperdir);
>  out:
> @@ -455,7 +452,8 @@ static int ovl_create_over_whiteout(struct dentry *dentry, struct inode *inode,
>
>  out_cleanup:
>         ovl_cleanup(wdir, newdentry);
> -       goto out_dput2;
> +       dput(newdentry);
> +       goto out_dput;
>  }
>
>  static int ovl_create_or_link(struct dentry *dentry, struct inode *inode,
> diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h
> index aa8286419133..6bbde513e068 100644
> --- a/fs/overlayfs/overlayfs.h
> +++ b/fs/overlayfs/overlayfs.h
> @@ -358,8 +358,8 @@ struct ovl_cattr {
>
>  #define OVL_CATTR(m) (&(struct ovl_cattr) { .mode = (m) })
>
> -int ovl_create_real(struct inode *dir, struct dentry *newdentry,
> -                   struct ovl_cattr *attr);
> +struct dentry *ovl_create_real(struct inode *dir, struct dentry *newdentry,
> +                              struct ovl_cattr *attr);
>  int ovl_cleanup(struct inode *dir, struct dentry *dentry);
>
>  /* copy_up.c */
> diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
> index 286d36772e9c..704b37311467 100644
> --- a/fs/overlayfs/super.c
> +++ b/fs/overlayfs/super.c
> @@ -611,9 +611,10 @@ static struct dentry *ovl_workdir_create(struct ovl_fs *ofs,
>                         goto retry;
>                 }
>
> -               err = ovl_create_real(dir, work, OVL_CATTR(attr.ia_mode));
> -               if (err)
> -                       goto out_dput;
> +               work = ovl_create_real(dir, work, OVL_CATTR(attr.ia_mode));
> +               err = PTR_ERR(work);
> +               if (IS_ERR(work))
> +                       goto out_err;
>
>                 /*
>                  * Try to remove POSIX ACL xattrs from workdir.  We are good if:
> --
> 2.14.3
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-unionfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

  reply	other threads:[~2018-05-29 15:24 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-05-29 14:41 [PATCH 00/11] overlayfs fixes Miklos Szeredi
2018-05-29 14:41 ` [PATCH 01/11] ovl: update documentation for unionmount-testsuite Miklos Szeredi
2018-05-29 14:41 ` [PATCH 02/11] ovl: remove WARN_ON() real inode attributes mismatch Miklos Szeredi
2018-05-29 14:41 ` [PATCH 03/11] ovl: strip debug argument from ovl_do_ helpers Miklos Szeredi
2018-05-29 14:41 ` [PATCH 04/11] ovl: struct cattr cleanups Miklos Szeredi
2018-05-29 14:41 ` [PATCH 05/11] ovl: return dentry from ovl_create_real() Miklos Szeredi
2018-05-29 15:24   ` Amir Goldstein [this message]
2018-05-30  8:28     ` Miklos Szeredi
2018-05-29 14:41 ` [PATCH 06/11] ovl: create helper ovl_create_temp() Miklos Szeredi
2018-05-29 14:41 ` [PATCH 07/11] ovl: make ovl_create_real() cope with vfs_mkdir() safely Miklos Szeredi
2018-05-29 15:29   ` Amir Goldstein
2018-05-30  8:18     ` Miklos Szeredi
2018-05-29 14:41 ` [PATCH 08/11] ovl: clean up copy-up error paths Miklos Szeredi
2018-05-29 14:41 ` [PATCH 09/11] vfs: factor out inode_insert5() Miklos Szeredi
2018-06-10  5:49   ` Al Viro
2018-06-10  6:02     ` Al Viro
2018-06-11  9:15       ` Miklos Szeredi
2018-06-11 11:32         ` Miklos Szeredi
2018-06-11 16:43           ` Al Viro
2018-06-12 12:38             ` Miklos Szeredi
2018-05-29 14:41 ` [PATCH 10/11] ovl: Pass argument to ovl_get_inode() in a structure Miklos Szeredi
2018-05-29 14:41 ` [PATCH 11/11] ovl: use inode_insert5() to hash a newly created inode Miklos Szeredi

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAOQ4uxh0XUgOZ-tZ_1jTxBLFAdWv53kamh-1=hTuLkVpEVA08w@mail.gmail.com' \
    --to=amir73il@gmail.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-unionfs@vger.kernel.org \
    --cc=mszeredi@redhat.com \
    --cc=viro@zeniv.linux.org.uk \
    --subject='Re: [PATCH 05/11] ovl: return dentry from ovl_create_real()' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).