On Thu, 2015-04-16 at 14:16 +0200, Mateusz Guzik wrote:
> Hi,
>
> Currently obtaining a new file descriptor results in locking fdtable
> twice - once in order to reserve a slot and second time to fill it.
>
> Hack below gets rid of the second lock usage.
>
> It gives me a ~30% speedup (~300k ops -> ~400k ops) in a microbenchmark
> where 16 threads create a pipe (2 fds) and call 2 * close.
>
> Results are fluctuating and even with the patch sometimes drop to around
> ~300k ops. However, without the patch they never get higher.
>
> I can come up with a better benchmark if that's necessary.
Please push a patch with this test program alone, it will serve as a
baseline.
I discussed with Al about this problem in LKS 2014 in Chicago.
I am pleased to see you are working on this !
Please find one comment enclosed.
>
> Comments?
>
> ==============================================
>
> Subject: [PATCH] fs: use a sequence counter instead of file_lock in fd_install
>
> Because the lock is not held, it is possible that fdtable will be
> reallocated as we fill it.
>
> RCU is used to guarantee the old table is not freed just in case we
> happen to write to it (which is harmless).
>
> sequence counter is used to ensure we updated the right table.
>
> Signed-off-by: Mateusz Guzik <mguzik@xxxxxxxxxx>
> ---
> fs/file.c | 24 +++++++++++++++++++-----
> include/linux/fdtable.h | 5 +++++
> 2 files changed, 24 insertions(+), 5 deletions(-)
> void fd_install(unsigned int fd, struct file *file)
> diff --git a/include/linux/fdtable.h b/include/linux/fdtable.h
> index 230f87b..9e41765 100644
> --- a/include/linux/fdtable.h
> +++ b/include/linux/fdtable.h
> @@ -12,6 +12,7 @@
> #include <linux/types.h>
> #include <linux/init.h>
> #include <linux/fs.h>
> +#include <linux/seqlock.h>
>
> #include <linux/atomic.h>
>
> @@ -47,6 +48,7 @@ struct files_struct {
> * read mostly part
> */
> atomic_t count;
> + seqcount_t fdt_seqcount;
You put fdt_seqcount in the 'read mostly part' of 'struct files_struct',
please move it in the 'written part'
> struct fdtable __rcu *fdt;
> struct fdtable fdtab;
> /*
> @@ -69,6 +71,9 @@ struct dentry;
> #define files_fdtable(files) \
> rcu_dereference_check_fdtable((files), (files)->fdt)
>
> +#define files_fdtable_seq(files) \
> + rcu_dereference((files)->fdt)
> +
> /*
> * The caller must ensure that fd table isn't shared or hold rcu or file lock
> */
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
