[PATCH] fdtable: Remove the free_files field

An fdtable can either be embedded inside a files_struct or standalone (after
being expanded).  When an fdtable is being discarded after all RCU references
to it have expired, we must either free it directly, in the standalone case,
or free the files_struct it is contained within, in the embedded case.

Currently the free_files field controls this behavior, but we can get rid of
it entirely, as all the necessary information is already recorded.  We can
distinguish embedded and standalone fdtables using max_fds, and if it is
embedded we can divine the relevant files_struct using container_of().

Signed-off-by: Vadim Lobanov <vlobanov@speakeasy.net>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Al Viro <viro@zeniv.linux.org.uk>
Cc: Dipankar Sarma <dipankar@in.ibm.com>
Signed-off-by: Andrew Morton <akpm@osdl.org>
Signed-off-by: Linus Torvalds <torvalds@osdl.org>

diff --git a/fs/file.c b/fs/file.c
index fb3d2038dc215b9a8bb227a20ae3d4e4766213b7..17e6a55521e2017dcc37984e60ad3c44487426d2 100644 (file)
--- a/fs/file.c
+++ b/fs/file.c
@@ -91,7 +91,7 @@ static void free_fdtable_work(struct work_struct *work)
        }
 }
 
-static void free_fdtable_rcu(struct rcu_head *rcu)
+void free_fdtable_rcu(struct rcu_head *rcu)
 {
        struct fdtable *fdt = container_of(rcu, struct fdtable, rcu);
        int fdset_size, fdarray_size;
@@ -101,20 +101,15 @@ static void free_fdtable_rcu(struct rcu_head *rcu)
        fdset_size = fdt->max_fds / 8;
        fdarray_size = fdt->max_fds * sizeof(struct file *);
 
-       if (fdt->free_files) {
+       if (fdt->max_fds <= NR_OPEN_DEFAULT) {
                /*
-                * The this fdtable was embedded in the files structure
-                * and the files structure itself was getting destroyed.
-                * It is now safe to free the files structure.
+                * This fdtable is embedded in the files structure and that
+                * structure itself is getting destroyed.
                 */
-               kmem_cache_free(files_cachep, fdt->free_files);
+               kmem_cache_free(files_cachep,
+                               container_of(fdt, struct files_struct, fdtab));
                return;
        }
-       if (fdt->max_fds <= NR_OPEN_DEFAULT)
-               /*
-                * The fdtable was embedded
-                */
-               return;
        if (fdset_size <= PAGE_SIZE && fdarray_size <= PAGE_SIZE) {
                kfree(fdt->open_fds);
                kfree(fdt->close_on_exec);
@@ -132,12 +127,6 @@ static void free_fdtable_rcu(struct rcu_head *rcu)
        }
 }
 
-void free_fdtable(struct fdtable *fdt)
-{
-       if (fdt->free_files || fdt->max_fds > NR_OPEN_DEFAULT)
-               call_rcu(&fdt->rcu, free_fdtable_rcu);
-}
-
 /*
  * Expand the fdset in the files_struct.  Called with the files spinlock
  * held for write.
@@ -247,7 +236,6 @@ static struct fdtable *alloc_fdtable(int nr)
                goto out;
        fdt->fd = new_fds;
        fdt->max_fds = nfds;
-       fdt->free_files = NULL;
        return fdt;
 out:
        free_fdset(new_openset, nfds);
@@ -283,7 +271,8 @@ static int expand_fdtable(struct files_struct *files, int nr)
                /* Continue as planned */
                copy_fdtable(new_fdt, cur_fdt);
                rcu_assign_pointer(files->fdt, new_fdt);
-               free_fdtable(cur_fdt);
+               if (cur_fdt->max_fds > NR_OPEN_DEFAULT)
+                       call_rcu(&cur_fdt->rcu, free_fdtable_rcu);
        } else {
                /* Somebody else expanded, so undo our attempt */
                __free_fdtable(new_fdt);

diff --git a/include/linux/file.h b/include/linux/file.h
index 02be4012225b2d7dfe4e7cff18c42248347ab39c..319118f275b0dd8fcce3e8b1c2a11f71686911e5 100644 (file)
--- a/include/linux/file.h
+++ b/include/linux/file.h
@@ -32,7 +32,6 @@ struct fdtable {
        fd_set *close_on_exec;
        fd_set *open_fds;
        struct rcu_head rcu;
-       struct files_struct *free_files;
        struct fdtable *next;
 };
 
@@ -84,7 +83,7 @@ extern fd_set *alloc_fdset(int);
 extern void free_fdset(fd_set *, int);
 
 extern int expand_files(struct files_struct *, int nr);
-extern void free_fdtable(struct fdtable *fdt);
+extern void free_fdtable_rcu(struct rcu_head *rcu);
 extern void __init files_defer_init(void);
 
 static inline struct file * fcheck_files(struct files_struct *files, unsigned int fd)

diff --git a/include/linux/init_task.h b/include/linux/init_task.h
index 58c18daab65ddca4af1f2765a1c67b01c9bcf514..b5315150199e9528bbfa1b5fcfe64b669b199058 100644 (file)
--- a/include/linux/init_task.h
+++ b/include/linux/init_task.h
@@ -16,7 +16,6 @@
        .close_on_exec  = (fd_set *)&init_files.close_on_exec_init, \
        .open_fds       = (fd_set *)&init_files.open_fds_init,  \
        .rcu            = RCU_HEAD_INIT,                \
-       .free_files     = NULL,                         \
        .next           = NULL,                         \
 }
 

diff --git a/kernel/exit.c b/kernel/exit.c
index 5f77e76b4f97666c53e4a78d6213288bf88105a7..122fadb972fc2dd9ff3fda5cf15c3c0432a50007 100644 (file)
--- a/kernel/exit.c
+++ b/kernel/exit.c
@@ -466,11 +466,9 @@ void fastcall put_files_struct(struct files_struct *files)
                 * you can free files immediately.
                 */
                fdt = files_fdtable(files);
-               if (fdt == &files->fdtab)
-                       fdt->free_files = files;
-               else
+               if (fdt != &files->fdtab)
                        kmem_cache_free(files_cachep, files);
-               free_fdtable(fdt);
+               call_rcu(&fdt->rcu, free_fdtable_rcu);
        }
 }
 

diff --git a/kernel/fork.c b/kernel/fork.c
index aba595424f78e029c1d064d3989535a97b7a3078..d16c566eb645a6b7aae1bd7dd5a5489eeb3d3db9 100644 (file)
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -645,7 +645,6 @@ static struct files_struct *alloc_files(void)
        fdt->open_fds = (fd_set *)&newf->open_fds_init;
        fdt->fd = &newf->fd_array[0];
        INIT_RCU_HEAD(&fdt->rcu);
-       fdt->free_files = NULL;
        fdt->next = NULL;
        rcu_assign_pointer(newf->fdt, fdt);
 out: