Re: [PATCH 10/10] blkcg: implement per-blkg request allocation

[Vivek Goyal <vgoyal-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>]

On Mon, Jun 04, 2012 at 12:08:17AM -0700, Tejun Heo wrote:
> Currently, request_queue has one request_list to allocate requests
> from regardless of blkcg of the IO being issued.  When the unified
> request pool is used up, cfq proportional IO limits become meaningless
> - whoever grabs the next request being freed wins the race regardless
> of the configured weights.
> 
> This can be easily demonstrated by creating a blkio cgroup w/ very low
> weight, put a program which can issue a lot of random direct IOs there
> and running a sequential IO from a different cgroup.  As soon as the
> request pool is used up, the sequential IO bandwidth crashes.
> 
> This patch implements per-blkg request_list.  Each blkg has its own
> request_list and any IO allocates its request from the matching blkg
> making blkcgs completely isolated in terms of request allocation.
> 
> * Root blkcg uses the request_list embedded in each request_queue,
>   which was renamed to @q->root_rl from @q->rq.  While making blkcg rl
>   handling a bit harier, this enables avoiding most overhead for root
>   blkcg.
> 
> * Queue fullness is properly per request_list but bdi isn't blkcg
>   aware yet, so congestion state currently just follows the root
>   blkcg.  As writeback isn't aware of blkcg yet, this works okay for
>   async congestion but readahead may get the wrong signals.  It's
>   better than blkcg completely collapsing with shared request_list but
>   needs to be improved with future changes.

Hi Tejun,

It might be worth to update Documentation/block/queue-sysfs.txt and update
nr_requests description to reflect that fact that it is now a per queue
per cgroup/group limit and not per queue limit.

Also it will be good to explicitly mention in changelog that after this
patch allowing non-priviliged users to create blkio cgroups is dangerous
as they can create tons of cgroups, doing IO in those cgroups and locking
non-reclaimable kernel memory.

Otherwise this patch and series looks good to me.

Acked-by: Vivek Goyal <vgoyal-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>

Thanks
Vivek