Re: [RFC 4/5] security: smack: add kmem_cache for smack_rule allocations

[Casey Schaufler <casey@schaufler-ca.com>]

On 6/13/2013 8:29 AM, Tomasz Stanislawski wrote:
> On ARM, sizeof(struct smack_rule)==20. Allocation by kmalloc() uses a
> 32-byte-long chunk to allocate 20 bytes. Just ask ksize().  It means that 40%
> of memory is simply wasted for padding bytes.
>
> The problem is fixed in this patch by using kmem_cache. The cache allocates
> struct smack_rule using 24-byte-long chunks according to ksize(). This reduces
> amount of used memory by 25%.

I'm not opposed to this change, but could I see some performance
numbers to justify it? In particular, I'm concerned about the rules
load impact.

> Signed-off-by: Tomasz Stanislawski <t.stanislaws@samsung.com>
> ---
>  security/smack/smack.h     |    3 +++
>  security/smack/smack_lsm.c |   11 ++++++++++-
>  security/smack/smackfs.c   |    2 +-
>  3 files changed, 14 insertions(+), 2 deletions(-)
>
> diff --git a/security/smack/smack.h b/security/smack/smack.h
> index 8ad3095..38ba673 100644
> --- a/security/smack/smack.h
> +++ b/security/smack/smack.h
> @@ -233,6 +233,9 @@ extern struct mutex	smack_known_lock;
>  extern struct list_head smack_known_list;
>  extern struct list_head smk_netlbladdr_list;
>  
> +/* Cache for fast and thrifty allocations */
> +extern struct kmem_cache *smack_rule_cache;
> +
>  extern struct security_operations smack_ops;
>  
>  /*
> diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
> index d52c780..7aa696a 100644
> --- a/security/smack/smack_lsm.c
> +++ b/security/smack/smack_lsm.c
> @@ -3564,6 +3564,9 @@ static __init void init_smack_known_list(void)
>  	list_add(&smack_known_web.list, &smack_known_list);
>  }
>  
> +/* KMEM caches for fast and thrifty allocations */
> +struct kmem_cache *smack_rule_cache;
> +
>  /**
>   * smack_init - initialize the smack system
>   *
> @@ -3577,10 +3580,16 @@ static __init int smack_init(void)
>  	if (!security_module_enable(&smack_ops))
>  		return 0;
>  
> +	smack_rule_cache = KMEM_CACHE(smack_rule, 0);
> +	if (!smack_rule_cache)
> +		return -ENOMEM;
> +
>  	tsp = new_task_smack(smack_known_floor.smk_known,
>  				smack_known_floor.smk_known, GFP_KERNEL);
> -	if (tsp == NULL)
> +	if (tsp == NULL) {
> +		kmem_cache_destroy(smack_rule_cache);
>  		return -ENOMEM;
> +	}
>  
>  	printk(KERN_INFO "Smack:  Initializing.\n");
>  
> diff --git a/security/smack/smackfs.c b/security/smack/smackfs.c
> index e8c57f3..c08b1ec 100644
> --- a/security/smack/smackfs.c
> +++ b/security/smack/smackfs.c
> @@ -217,7 +217,7 @@ static int smk_set_access(struct smack_parsed_rule *srp,
>  	}
>  
>  	if (found == 0) {
> -		sp = kzalloc(sizeof(*sp), GFP_KERNEL);
> +		sp = kmem_cache_zalloc(smack_rule_cache, GFP_KERNEL);
>  		if (sp == NULL) {
>  			rc = -ENOMEM;
>  			goto out;