Skip site navigation (1)Skip section navigation (2)
Date:      Sun, 13 Dec 2020 18:56:11 +0000
From:      Alexander Richardson <arichardson@freebsd.org>
To:        Mateusz Guzik <mjg@freebsd.org>
Cc:        src-committers <src-committers@freebsd.org>, svn-src-all <svn-src-all@freebsd.org>,  svn-src-head <svn-src-head@freebsd.org>
Subject:   Re: svn commit: r368609 - in head/sys: kern sys
Message-ID:  <CA%2BZ_v8oqbLgB4jZ2eMGoMmxAMeGf6uduXXMrTnpbrKZvsX1Yyw@mail.gmail.com>
In-Reply-To: <202012131806.0BDI6Puj082474@repo.freebsd.org>
References:  <202012131806.0BDI6Puj082474@repo.freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help
On Sun, 13 Dec 2020 at 18:06, Mateusz Guzik <mjg@freebsd.org> wrote:
>
> Author: mjg
> Date: Sun Dec 13 18:06:24 2020
> New Revision: 368609
> URL: https://svnweb.freebsd.org/changeset/base/368609
>
> Log:
>   fd: fix fdrop prediction when closing a fd
>
>   Most of the time this is the last reference, contrary to typical fdrop use.
>
> Modified:
>   head/sys/kern/kern_descrip.c
>   head/sys/sys/file.h
>
> Modified: head/sys/kern/kern_descrip.c
> ==============================================================================
> --- head/sys/kern/kern_descrip.c        Sun Dec 13 16:26:37 2020        (r368608)
> +++ head/sys/kern/kern_descrip.c        Sun Dec 13 18:06:24 2020        (r368609)
> @@ -2766,7 +2766,7 @@ closef(struct file *fp, struct thread *td)
>                         FILEDESC_XUNLOCK(fdp);
>                 }
>         }
> -       return (fdrop(fp, td));
> +       return (fdrop_close(fp, td));
>  }
>
>  /*
>
> Modified: head/sys/sys/file.h
> ==============================================================================
> --- head/sys/sys/file.h Sun Dec 13 16:26:37 2020        (r368608)
> +++ head/sys/sys/file.h Sun Dec 13 18:06:24 2020        (r368609)
> @@ -299,6 +299,17 @@ fhold(struct file *fp)
>         _error;                                                 \
>  })
>
> +#define        fdrop_close(fp, td)             ({                      \
> +       struct file *_fp;                                       \
> +       int _error;                                             \
> +                                                               \
> +       _error = 0;                                             \
> +       _fp = (fp);                                             \
> +       if (__predict_true(refcount_release(&_fp->f_count)))    \
> +               _error = _fdrop(_fp, td);                       \
> +       _error;                                                 \
> +})
> +
>  static __inline fo_rdwr_t      fo_read;
>  static __inline fo_rdwr_t      fo_write;
>  static __inline fo_truncate_t  fo_truncate;

Wouldn't this be more readable as a static __inline function (or if
you are concerned about it not being inlined, __always_inline)? Also
means you can drop the temporary _fp variable that's there to avoid
side-effects in macro args.

Regards,
Alex



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CA%2BZ_v8oqbLgB4jZ2eMGoMmxAMeGf6uduXXMrTnpbrKZvsX1Yyw>