On Fri, 2013-06-07 at 00:34 +0400, Glauber Costa wrote:
> We currently use a compile-time constant to size the node array for the
> list_lru structure. Due to this, we don't need to allocate any memory at
> initialization time. But as a consequence, the structures that contain
> embedded list_lru lists can become way too big (the superblock for
> instance contains two of them).
>
> This patch aims at ameliorating this situation by dynamically allocating
> the node arrays with the firmware provided nr_node_ids.
>
> Signed-off-by: Glauber Costa <glommer@xxxxxxxxxx>
> Cc: Dave Chinner <dchinner@xxxxxxxxxx>
> Cc: Mel Gorman <mgorman@xxxxxxx>
> ---
> fs/super.c | 11 +++++++++--
> fs/xfs/xfs_buf.c | 6 +++++-
> fs/xfs/xfs_qm.c | 10 ++++++++--
> include/linux/list_lru.h | 13 ++-----------
> mm/list_lru.c | 14 +++++++++++++-
> 5 files changed, 37 insertions(+), 17 deletions(-)
>
> diff --git a/fs/super.c b/fs/super.c
> index 85a6104..1b6ef7b 100644
> --- a/fs/super.c
> +++ b/fs/super.c
> @@ -199,8 +199,12 @@ static struct super_block *alloc_super(struct file_system_type *type, int flags)
> INIT_HLIST_NODE(&s->s_instances);
> INIT_HLIST_BL_HEAD(&s->s_anon);
> INIT_LIST_HEAD(&s->s_inodes);
> - list_lru_init(&s->s_dentry_lru);
> - list_lru_init(&s->s_inode_lru);
> +
> + if (list_lru_init(&s->s_dentry_lru))
> + goto err_out;
> + if (list_lru_init(&s->s_inode_lru))
> + goto err_out_dentry_lru;
> +
> INIT_LIST_HEAD(&s->s_mounts);
> init_rwsem(&s->s_umount);
> lockdep_set_class(&s->s_umount, &type->s_umount_key);
> @@ -240,6 +244,9 @@ static struct super_block *alloc_super(struct file_system_type *type, int flags)
> }
> out:
> return s;
> +
> +err_out_dentry_lru:
> + list_lru_destroy(&s->s_dentry_lru);
> err_out:
> security_sb_free(s);
> #ifdef CONFIG_SMP
It seems we also need to call list_lru_destroy() in destroy_super()?
like below:
-----------
diff --git a/fs/super.c b/fs/super.c
index b79e732..06ee3af 100644
--- a/fs/super.c
+++ b/fs/super.c
@@ -269,6 +269,8 @@ err_out:
*/
static inline void destroy_super(struct super_block *s)
{
+ list_lru_destroy(&s->s_inode_lru);
+ list_lru_destroy(&s->s_dentry_lru);
#ifdef CONFIG_SMP
free_percpu(s->s_files);
#endif
-----------
Thanks, Zhong
> diff --git a/fs/xfs/xfs_buf.c b/fs/xfs/xfs_buf.c
> index c3f8ea9..9c2b656 100644
> --- a/fs/xfs/xfs_buf.c
> +++ b/fs/xfs/xfs_buf.c
> @@ -1591,6 +1591,7 @@ xfs_free_buftarg(
> struct xfs_mount *mp,
> struct xfs_buftarg *btp)
> {
> + list_lru_destroy(&btp->bt_lru);
> unregister_shrinker(&btp->bt_shrinker);
>
> if (mp->m_flags & XFS_MOUNT_BARRIER)
> @@ -1665,9 +1666,12 @@ xfs_alloc_buftarg(
> if (!btp->bt_bdi)
> goto error;
>
> - list_lru_init(&btp->bt_lru);
> if (xfs_setsize_buftarg_early(btp, bdev))
> goto error;
> +
> + if (list_lru_init(&btp->bt_lru))
> + goto error;
> +
> btp->bt_shrinker.count_objects = xfs_buftarg_shrink_count;
> btp->bt_shrinker.scan_objects = xfs_buftarg_shrink_scan;
> btp->bt_shrinker.seeks = DEFAULT_SEEKS;
> diff --git a/fs/xfs/xfs_qm.c b/fs/xfs/xfs_qm.c
> index bd6c12a..b840000 100644
> --- a/fs/xfs/xfs_qm.c
> +++ b/fs/xfs/xfs_qm.c
> @@ -781,11 +781,18 @@ xfs_qm_init_quotainfo(
>
> qinf = mp->m_quotainfo = kmem_zalloc(sizeof(xfs_quotainfo_t), KM_SLEEP);
>
> + if ((error = list_lru_init(&qinf->qi_lru))) {
> + kmem_free(qinf);
> + mp->m_quotainfo = NULL;
> + return error;
> + }
> +
> /*
> * See if quotainodes are setup, and if not, allocate them,
> * and change the superblock accordingly.
> */
> if ((error = xfs_qm_init_quotainos(mp))) {
> + list_lru_destroy(&qinf->qi_lru);
> kmem_free(qinf);
> mp->m_quotainfo = NULL;
> return error;
> @@ -795,8 +802,6 @@ xfs_qm_init_quotainfo(
> INIT_RADIX_TREE(&qinf->qi_gquota_tree, GFP_NOFS);
> mutex_init(&qinf->qi_tree_lock);
>
> - list_lru_init(&qinf->qi_lru);
> -
> /* mutex used to serialize quotaoffs */
> mutex_init(&qinf->qi_quotaofflock);
>
> @@ -884,6 +889,7 @@ xfs_qm_destroy_quotainfo(
> qi = mp->m_quotainfo;
> ASSERT(qi != NULL);
>
> + list_lru_destroy(&qi->qi_lru);
> unregister_shrinker(&qi->qi_shrinker);
>
> if (qi->qi_uquotaip) {
> diff --git a/include/linux/list_lru.h b/include/linux/list_lru.h
> index 2fe13e1..ff57503 100644
> --- a/include/linux/list_lru.h
> +++ b/include/linux/list_lru.h
> @@ -27,20 +27,11 @@ struct list_lru_node {
> } ____cacheline_aligned_in_smp;
>
> struct list_lru {
> - /*
> - * Because we use a fixed-size array, this struct can be very big if
> - * MAX_NUMNODES is big. If this becomes a problem this is fixable by
> - * turning this into a pointer and dynamically allocating this to
> - * nr_node_ids. This quantity is firwmare-provided, and still would
> - * provide room for all nodes at the cost of a pointer lookup and an
> - * extra allocation. Because that allocation will most likely come from
> - * a different slab cache than the main structure holding this
> - * structure, we may very well fail.
> - */
> - struct list_lru_node node[MAX_NUMNODES];
> + struct list_lru_node *node;
> nodemask_t active_nodes;
> };
>
> +void list_lru_destroy(struct list_lru *lru);
> int list_lru_init(struct list_lru *lru);
>
> /**
> diff --git a/mm/list_lru.c b/mm/list_lru.c
> index 2822817..700d322 100644
> --- a/mm/list_lru.c
> +++ b/mm/list_lru.c
> @@ -8,6 +8,7 @@
> #include <linux/module.h>
> #include <linux/mm.h>
> #include <linux/list_lru.h>
> +#include <linux/slab.h>
>
> bool list_lru_add(struct list_lru *lru, struct list_head *item)
> {
> @@ -162,9 +163,14 @@ unsigned long list_lru_dispose_all(struct list_lru *lru,
> int list_lru_init(struct list_lru *lru)
> {
> int i;
> + size_t size = sizeof(*lru->node) * nr_node_ids;
> +
> + lru->node = kzalloc(size, GFP_KERNEL);
> + if (!lru->node)
> + return -ENOMEM;
>
> nodes_clear(lru->active_nodes);
> - for (i = 0; i < MAX_NUMNODES; i++) {
> + for (i = 0; i < nr_node_ids; i++) {
> spin_lock_init(&lru->node[i].lock);
> INIT_LIST_HEAD(&lru->node[i].list);
> lru->node[i].nr_items = 0;
> @@ -172,3 +178,9 @@ int list_lru_init(struct list_lru *lru)
> return 0;
> }
> EXPORT_SYMBOL_GPL(list_lru_init);
> +
> +void list_lru_destroy(struct list_lru *lru)
> +{
> + kfree(lru->node);
> +}
> +EXPORT_SYMBOL_GPL(list_lru_destroy);
--
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
