From f5a9867b7d72b12d9f9ceb6537ed156bf5f32318 Mon Sep 17 00:00:00 2001 From: Hans Petter Selasky <hselasky@FreeBSD.org> Date: Wed, 31 May 2017 12:02:59 +0000 Subject: [PATCH] Fixes for refcounting "struct linux_file" in the LinuxKPI. - Allow "struct linux_file" to be refcounted when its "_file" member is NULL by using its "f_count" field. The reference counts are transferred to the file structure when the file descriptor is installed. - Add missing vdrop() calls for error cases during open(). - Set the "_file" member of "struct linux_file" during open. This allows use of refcounting through get_file() and fput() with LinuxKPI character devices. MFC after: 1 week Sponsored by: Mellanox Technologies --- .../linuxkpi/common/include/linux/file.h | 31 +++++++++++++------ sys/compat/linuxkpi/common/include/linux/fs.h | 9 ++++++ sys/compat/linuxkpi/common/src/linux_compat.c | 21 +++++++++++-- 3 files changed, 49 insertions(+), 12 deletions(-) diff --git a/sys/compat/linuxkpi/common/include/linux/file.h b/sys/compat/linuxkpi/common/include/linux/file.h index 0661e70a5697..b8f20222459a 100644 --- a/sys/compat/linuxkpi/common/include/linux/file.h +++ b/sys/compat/linuxkpi/common/include/linux/file.h @@ -2,7 +2,7 @@ * Copyright (c) 2010 Isilon Systems, Inc. * Copyright (c) 2010 iX Systems, Inc. * Copyright (c) 2010 Panasas, Inc. - * Copyright (c) 2013-2015 Mellanox Technologies, Ltd. + * Copyright (c) 2013-2017 Mellanox Technologies, Ltd. * All rights reserved. * * Redistribution and use in source and binary forms, with or without @@ -60,19 +60,24 @@ linux_fget(unsigned int fd) return (struct linux_file *)file->f_data; } +extern void linux_file_free(struct linux_file *filp); + static inline void fput(struct linux_file *filp) { - if (filp->_file == NULL) { - kfree(filp); - return; - } - if (refcount_release(&filp->_file->f_count)) { - _fdrop(filp->_file, curthread); - kfree(filp); + if (refcount_release(filp->_file == NULL ? + &filp->f_count : &filp->_file->f_count)) { + linux_file_free(filp); } } +static inline unsigned int +file_count(struct linux_file *filp) +{ + return (filp->_file == NULL ? + filp->f_count : filp->_file->f_count); +} + static inline void put_unused_fd(unsigned int fd) { @@ -106,6 +111,10 @@ fd_install(unsigned int fd, struct linux_file *filp) } else { filp->_file = file; finit(file, filp->f_mode, DTYPE_DEV, filp, &linuxfileops); + + /* transfer reference count from "filp" to "file" */ + while (refcount_release(&filp->f_count) == 0) + refcount_acquire(&file->f_count); } /* drop the extra reference */ @@ -150,6 +159,8 @@ alloc_file(int mode, const struct file_operations *fops) filp = kzalloc(sizeof(*filp), GFP_KERNEL); if (filp == NULL) return (NULL); + + filp->f_count = 1; filp->f_op = fops; filp->f_mode = mode; @@ -171,7 +182,7 @@ static inline struct fd fdget(unsigned int fd) return (struct fd){f}; } -#define file linux_file -#define fget linux_fget +#define file linux_file +#define fget(...) linux_fget(__VA_ARGS__) #endif /* _LINUX_FILE_H_ */ diff --git a/sys/compat/linuxkpi/common/include/linux/fs.h b/sys/compat/linuxkpi/common/include/linux/fs.h index fd59fcf37f11..4c291fb66470 100644 --- a/sys/compat/linuxkpi/common/include/linux/fs.h +++ b/sys/compat/linuxkpi/common/include/linux/fs.h @@ -79,6 +79,7 @@ struct linux_file { struct selinfo f_selinfo; struct sigio *f_sigio; struct vnode *f_vnode; + volatile u_int f_count; }; #define file linux_file @@ -220,6 +221,14 @@ iminor(struct inode *inode) return (minor(dev2unit(inode->v_rdev))); } +static inline struct linux_file * +get_file(struct linux_file *f) +{ + + refcount_acquire(f->_file == NULL ? &f->f_count : &f->_file->f_count); + return (f); +} + static inline struct inode * igrab(struct inode *inode) { diff --git a/sys/compat/linuxkpi/common/src/linux_compat.c b/sys/compat/linuxkpi/common/src/linux_compat.c index 02df4e325186..cd355cd687d7 100644 --- a/sys/compat/linuxkpi/common/src/linux_compat.c +++ b/sys/compat/linuxkpi/common/src/linux_compat.c @@ -401,6 +401,20 @@ linux_file_dtor(void *cdp) kfree(filp); } +void +linux_file_free(struct linux_file *filp) +{ + if (filp->_file == NULL) { + kfree(filp); + } else { + /* + * The close method of the character device or file + * will free the linux_file structure: + */ + _fdrop(filp->_file, curthread); + } +} + static int linux_cdev_pager_populate(vm_object_t vm_obj, vm_pindex_t pidx, int fault_type, vm_prot_t max_prot, vm_pindex_t *first, vm_pindex_t *last) @@ -593,9 +607,12 @@ linux_dev_open(struct cdev *dev, int oflags, int devtype, struct thread *td) vhold(file->f_vnode); filp->f_vnode = file->f_vnode; linux_set_current(td); + filp->_file = file; + if (filp->f_op->open) { error = -filp->f_op->open(file->f_vnode, filp); if (error) { + vdrop(filp->f_vnode); kfree(filp); goto done; } @@ -603,6 +620,7 @@ linux_dev_open(struct cdev *dev, int oflags, int devtype, struct thread *td) error = devfs_set_cdevpriv(filp, linux_file_dtor); if (error) { filp->f_op->release(file->f_vnode, filp); + vdrop(filp->f_vnode); kfree(filp); } done: @@ -622,8 +640,7 @@ linux_dev_close(struct cdev *dev, int fflag, int devtype, struct thread *td) if ((error = devfs_get_cdevpriv((void **)&filp)) != 0) return (error); filp->f_flags = file->f_flag; - devfs_clear_cdevpriv(); - + devfs_clear_cdevpriv(); return (0); }