diff mbox series

ovl: clarify dget/dput in ovl_cleanup()

Message ID 20241022155513.303860-1-mszeredi@redhat.com (mailing list archive)
State New
Headers show
Series ovl: clarify dget/dput in ovl_cleanup() | expand

Commit Message

Miklos Szeredi Oct. 22, 2024, 3:55 p.m. UTC
Add a comment explaining the reason for the seemingly pointless extra
reference.

Suggested-by: Al Viro <viro@zeniv.linux.org.uk>
Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
---
 fs/overlayfs/dir.c | 4 ++++
 1 file changed, 4 insertions(+)

Comments

Miklos Szeredi Oct. 22, 2024, 4:04 p.m. UTC | #1
On Tue, 22 Oct 2024 at 17:56, Miklos Szeredi <mszeredi@redhat.com> wrote:
>
> Add a comment explaining the reason for the seemingly pointless extra
> reference.
>
> Suggested-by: Al Viro <viro@zeniv.linux.org.uk>
> Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
> ---
>  fs/overlayfs/dir.c | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c
> index ab65e98a1def..9e97f7dffd90 100644
> --- a/fs/overlayfs/dir.c
> +++ b/fs/overlayfs/dir.c
> @@ -28,6 +28,10 @@ int ovl_cleanup(struct ovl_fs *ofs, struct inode *wdir, struct dentry *wdentry)
>  {
>         int err;
>
> +       /*
> +        * Cached negative upper dentries are generally not useful, so grab a
> +        * ref to the victim to keep it from turning negative.
> +        */

In fact an explicit d_drop() after the fact would have exactly the
same effect, so maybe that would be cleaner...

Thanks,
Miklos
Amir Goldstein Oct. 23, 2024, 4:01 p.m. UTC | #2
On Tue, Oct 22, 2024 at 6:04 PM Miklos Szeredi <miklos@szeredi.hu> wrote:
>
> On Tue, 22 Oct 2024 at 17:56, Miklos Szeredi <mszeredi@redhat.com> wrote:
> >
> > Add a comment explaining the reason for the seemingly pointless extra
> > reference.
> >
> > Suggested-by: Al Viro <viro@zeniv.linux.org.uk>
> > Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
> > ---
> >  fs/overlayfs/dir.c | 4 ++++
> >  1 file changed, 4 insertions(+)
> >
> > diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c
> > index ab65e98a1def..9e97f7dffd90 100644
> > --- a/fs/overlayfs/dir.c
> > +++ b/fs/overlayfs/dir.c
> > @@ -28,6 +28,10 @@ int ovl_cleanup(struct ovl_fs *ofs, struct inode *wdir, struct dentry *wdentry)
> >  {
> >         int err;
> >
> > +       /*
> > +        * Cached negative upper dentries are generally not useful, so grab a
> > +        * ref to the victim to keep it from turning negative.
> > +        */
>
> In fact an explicit d_drop() after the fact would have exactly the
> same effect, so maybe that would be cleaner...
>

Agree.

Thanks,
Amir.
diff mbox series

Patch

diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c
index ab65e98a1def..9e97f7dffd90 100644
--- a/fs/overlayfs/dir.c
+++ b/fs/overlayfs/dir.c
@@ -28,6 +28,10 @@  int ovl_cleanup(struct ovl_fs *ofs, struct inode *wdir, struct dentry *wdentry)
 {
 	int err;
 
+	/*
+	 * Cached negative upper dentries are generally not useful, so grab a
+	 * ref to the victim to keep it from turning negative.
+	 */
 	dget(wdentry);
 	if (d_is_dir(wdentry))
 		err = ovl_do_rmdir(ofs, wdir, wdentry);