Re: [PATCH 2/7] res_counter: introduce ratelimiting attributes

[KAMEZAWA Hiroyuki]

On Sat, 18 Apr 2009 23:38:27 +0200
Andrea Righi <righi.andrea@xxxxxxxxx> wrote:

> Introduce attributes and functions in res_counter to implement throttling-based
> cgroup subsystems.
> 
> The following attributes have been added to struct res_counter:
>  * @policy:     the limiting policy / algorithm
>  * @capacity:   the maximum capacity of the resource
>  * @timestamp:  timestamp of the last accounted resource request
> 
> Currently the available policies are: token-bucket and leaky-bucket and the
> attribute @capacity is only used by token-bucket policy (to represent the
> bucket size).
> 
> The following function has been implemented to return the amount of time a
> cgroup should sleep to remain within the defined resource limits.
> 
>   unsigned long long
>   res_counter_ratelimit_sleep(struct res_counter *res, ssize_t val);
> 
> [ Note: only the interfaces needed by the cgroup IO controller are implemented
> right now ]
> 
> Signed-off-by: Andrea Righi <righi.andrea@xxxxxxxxx>
> ---
>  include/linux/res_counter.h |   69 +++++++++++++++++++++++++++++++----------
>  kernel/res_counter.c        |   72 +++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 124 insertions(+), 17 deletions(-)
> 
> diff --git a/include/linux/res_counter.h b/include/linux/res_counter.h
> index 4c5bcf6..9bed6af 100644
> --- a/include/linux/res_counter.h
> +++ b/include/linux/res_counter.h
> @@ -14,30 +14,36 @@
>   */
>  
>  #include <linux/cgroup.h>
> +#include <linux/jiffies.h>
>  
> -/*
> - * The core object. the cgroup that wishes to account for some
> - * resource may include this counter into its structures and use
> - * the helpers described beyond
> - */
> +/* The various policies that can be used for ratelimiting resources */
> +#define	RATELIMIT_LEAKY_BUCKET	0
> +#define	RATELIMIT_TOKEN_BUCKET	1
>  
> +/**
> + * struct res_counter - the core object to account cgroup resources
> + *
> + * @usage:	the current resource consumption level
> + * @max_usage:	the maximal value of the usage from the counter creation
> + * @limit:	the limit that usage cannot be exceeded
> + * @failcnt:	the number of unsuccessful attempts to consume the resource
> + * @policy:	the limiting policy / algorithm
> + * @capacity:	the maximum capacity of the resource
> + * @timestamp:	timestamp of the last accounted resource request
> + * @lock:	the lock to protect all of the above.
> + *		The routines below consider this to be IRQ-safe
> + *
> + * The cgroup that wishes to account for some resource may include this counter
> + * into its structures and use the helpers described beyond.
> + */
>  struct res_counter {
> -	/*
> -	 * the current resource consumption level
> -	 */
>  	unsigned long long usage;
> -	/*
> -	 * the maximal value of the usage from the counter creation
> -	 */
>  	unsigned long long max_usage;
> -	/*
> -	 * the limit that usage cannot exceed
> -	 */
>  	unsigned long long limit;
> -	/*
> -	 * the number of unsuccessful attempts to consume the resource
> -	 */
>  	unsigned long long failcnt;
> +	unsigned long long policy;
> +	unsigned long long capacity;
> +	unsigned long long timestamp;
>  
Andrea, sizeof(struct res_counter) is getting close to 128bytes. (maybe someone adds more)
Then, could you check "unsigned long or unsigned int" is allowed or not, again ?

It's very bad if cacheline of spinlock is different from data field, in future.

Thanks,
-Kame

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/