[adding Al, I think we really want him to review anything fget
related]
On Thu, Dec 13, 2018 at 10:56:35AM -0700, Jens Axboe wrote:
> Some uses cases repeatedly get and put references to the same file, but
> the only exposed interface is doing these one at the time. As each of
> these entail an atomic inc or dec on a shared structure, that cost can
> add up.
>
> Add fget_many(), which works just like fget(), except it takes an
> argument for how many references to get on the file. Ditto fput_many(),
> which can drop an arbitrary number of references to a file.
>
> Signed-off-by: Jens Axboe <axboe@xxxxxxxxx>
> ---
> fs/file.c | 15 ++++++++++-----
> fs/file_table.c | 10 ++++++++--
> include/linux/file.h | 2 ++
> include/linux/fs.h | 3 ++-
> 4 files changed, 22 insertions(+), 8 deletions(-)
>
> diff --git a/fs/file.c b/fs/file.c
> index 7ffd6e9d103d..ad9870edfd51 100644
> --- a/fs/file.c
> +++ b/fs/file.c
> @@ -676,7 +676,7 @@ void do_close_on_exec(struct files_struct *files)
> spin_unlock(&files->file_lock);
> }
>
> -static struct file *__fget(unsigned int fd, fmode_t mask)
> +static struct file *__fget(unsigned int fd, fmode_t mask, unsigned int refs)
> {
> struct files_struct *files = current->files;
> struct file *file;
> @@ -691,7 +691,7 @@ static struct file *__fget(unsigned int fd, fmode_t mask)
> */
> if (file->f_mode & mask)
> file = NULL;
> - else if (!get_file_rcu(file))
> + else if (!get_file_rcu_many(file, refs))
> goto loop;
> }
> rcu_read_unlock();
> @@ -699,15 +699,20 @@ static struct file *__fget(unsigned int fd, fmode_t mask)
> return file;
> }
>
> +struct file *fget_many(unsigned int fd, unsigned int refs)
> +{
> + return __fget(fd, FMODE_PATH, refs);
> +}
> +
> struct file *fget(unsigned int fd)
> {
> - return __fget(fd, FMODE_PATH);
> + return fget_many(fd, 1);
Can we just call __fget directly here? That is a little easier
to follow, and might actually generate better code if the compiler
is inliner challenged (which they often seem to be).
> +void fput(struct file *file)
> +{
> + fput_many(file, 1);
> +}
> +
> +
Double empty line here.
> +#define get_file_rcu_many(x, cnt) atomic_long_add_unless(&(x)->f_count, (cnt), 0)
This could use a line break to be easier readable and not spill over
80 chars. Otherwise this looks fine to me.
