| Message ID | 20240713023917.3967269-2-yu.ma@intel.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | fs/file.c: optimize the critical section of file_lock in | expand |
On Fri 12-07-24 22:39:15, Yu Ma wrote: > alloc_fd() has a sanity check inside to make sure the struct file mapping to the > allocated fd is NULL. Remove this sanity check since it can be assured by > exisitng zero initilization and NULL set when recycling fd. Meanwhile, add > likely/unlikely and expand_file() call avoidance to reduce the work under > file_lock. > > Reviewed-by: Tim Chen <tim.c.chen@linux.intel.com> > Signed-off-by: Yu Ma <yu.ma@intel.com> Looks good to me. Feel free to add: Reviewed-by: Jan Kara <jack@suse.cz> Honza > --- > fs/file.c | 33 ++++++++++++++------------------- > 1 file changed, 14 insertions(+), 19 deletions(-) > > diff --git a/fs/file.c b/fs/file.c > index a3b72aa64f11..e1b9d6df7941 100644 > --- a/fs/file.c > +++ b/fs/file.c > @@ -515,7 +515,7 @@ static int alloc_fd(unsigned start, unsigned end, unsigned flags) > if (fd < files->next_fd) > fd = files->next_fd; > > - if (fd < fdt->max_fds) > + if (likely(fd < fdt->max_fds)) > fd = find_next_fd(fdt, fd); > > /* > @@ -523,19 +523,21 @@ static int alloc_fd(unsigned start, unsigned end, unsigned flags) > * will limit the total number of files that can be opened. > */ > error = -EMFILE; > - if (fd >= end) > + if (unlikely(fd >= end)) > goto out; > > - error = expand_files(files, fd); > - if (error < 0) > - goto out; > + if (unlikely(fd >= fdt->max_fds)) { > + error = expand_files(files, fd); > + if (error < 0) > + goto out; > > - /* > - * If we needed to expand the fs array we > - * might have blocked - try again. > - */ > - if (error) > - goto repeat; > + /* > + * If we needed to expand the fs array we > + * might have blocked - try again. > + */ > + if (error) > + goto repeat; > + } > > if (start <= files->next_fd) > files->next_fd = fd + 1; > @@ -546,13 +548,6 @@ static int alloc_fd(unsigned start, unsigned end, unsigned flags) > else > __clear_close_on_exec(fd, fdt); > error = fd; > -#if 1 > - /* Sanity check */ > - if (rcu_access_pointer(fdt->fd[fd]) != NULL) { > - printk(KERN_WARNING "alloc_fd: slot %d not NULL!\n", fd); > - rcu_assign_pointer(fdt->fd[fd], NULL); > - } > -#endif > > out: > spin_unlock(&files->file_lock); > @@ -618,7 +613,7 @@ void fd_install(unsigned int fd, struct file *file) > rcu_read_unlock_sched(); > spin_lock(&files->file_lock); > fdt = files_fdtable(files); > - BUG_ON(fdt->fd[fd] != NULL); > + WARN_ON(fdt->fd[fd] != NULL); > rcu_assign_pointer(fdt->fd[fd], file); > spin_unlock(&files->file_lock); > return; > -- > 2.43.0 >
diff --git a/fs/file.c b/fs/file.c index a3b72aa64f11..e1b9d6df7941 100644 --- a/fs/file.c +++ b/fs/file.c @@ -515,7 +515,7 @@ static int alloc_fd(unsigned start, unsigned end, unsigned flags) if (fd < files->next_fd) fd = files->next_fd; - if (fd < fdt->max_fds) + if (likely(fd < fdt->max_fds)) fd = find_next_fd(fdt, fd); /* @@ -523,19 +523,21 @@ static int alloc_fd(unsigned start, unsigned end, unsigned flags) * will limit the total number of files that can be opened. */ error = -EMFILE; - if (fd >= end) + if (unlikely(fd >= end)) goto out; - error = expand_files(files, fd); - if (error < 0) - goto out; + if (unlikely(fd >= fdt->max_fds)) { + error = expand_files(files, fd); + if (error < 0) + goto out; - /* - * If we needed to expand the fs array we - * might have blocked - try again. - */ - if (error) - goto repeat; + /* + * If we needed to expand the fs array we + * might have blocked - try again. + */ + if (error) + goto repeat; + } if (start <= files->next_fd) files->next_fd = fd + 1; @@ -546,13 +548,6 @@ static int alloc_fd(unsigned start, unsigned end, unsigned flags) else __clear_close_on_exec(fd, fdt); error = fd; -#if 1 - /* Sanity check */ - if (rcu_access_pointer(fdt->fd[fd]) != NULL) { - printk(KERN_WARNING "alloc_fd: slot %d not NULL!\n", fd); - rcu_assign_pointer(fdt->fd[fd], NULL); - } -#endif out: spin_unlock(&files->file_lock); @@ -618,7 +613,7 @@ void fd_install(unsigned int fd, struct file *file) rcu_read_unlock_sched(); spin_lock(&files->file_lock); fdt = files_fdtable(files); - BUG_ON(fdt->fd[fd] != NULL); + WARN_ON(fdt->fd[fd] != NULL); rcu_assign_pointer(fdt->fd[fd], file); spin_unlock(&files->file_lock); return;