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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20090421101659.GF19637@balbir.in.ibm.com>
Date:	Tue, 21 Apr 2009 15:46:59 +0530
From:	Balbir Singh <balbir@...ux.vnet.ibm.com>
To:	Andrea Righi <righi.andrea@...il.com>
Cc:	KAMEZAWA Hiroyuki <kamezawa.hiroyu@...fujitsu.com>,
	randy.dunlap@...cle.com, Paul Menage <menage@...gle.com>,
	Carl Henrik Lunde <chlunde@...g.uio.no>,
	eric.rannaud@...il.com, paolo.valente@...more.it,
	fernando@....ntt.co.jp, dradford@...ehost.com, fchecconi@...il.com,
	agk@...rceware.org, subrata@...ux.vnet.ibm.com, axboe@...nel.dk,
	akpm@...ux-foundation.org, containers@...ts.linux-foundation.org,
	linux-kernel@...r.kernel.org, dave@...ux.vnet.ibm.com,
	matt@...ehost.com, roberto@...it.it, ngupta@...gle.com
Subject: Re: [PATCH 2/7] res_counter: introduce ratelimiting attributes

* Andrea Righi <righi.andrea@...il.com> [2009-04-21 11:55:26]:

> On Tue, Apr 21, 2009 at 09:15:34AM +0900, KAMEZAWA Hiroyuki wrote:
> > On Sat, 18 Apr 2009 23:38:27 +0200
> > Andrea Righi <righi.andrea@...il.com> 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@...il.com>
> > > ---
> > >  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.
> 
> Regarding the new attributes, policy can be surely an unsigned int or
> even less (now only 1 bit is used!), maybe we can just add an unsigned
> int flags, and encode also potential future informations there.
> 
> Moreover, are we sure we really need an unsigned long long for failcnt?
>

No we don't. But having it helps the members align well on a 8 byte
boundary. For all you know the compiler might do that anyway, unless
we pack the structure.

Why does policy need to be unsigned long long? Can't it be a boolean
for now? Token or leaky? We can consider unioning of some fields like
soft_limit when added along with the proposed fields.

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