linux-toradex.git/block/blk-cgroup.c, branch v3.6.10

blkcg: Fix use-after-free of q->root_blkg and q->root_rl.blkg

2012-11-05T08:56:48+00:00

commit 65635cbc37e011e71b208257a25e7c1078cd039b upstream.

blk_put_rl() does not call blkg_put() for q->root_rl because we
don't take request list reference on q->root_blkg.
However, if root_blkg is once attached then detached (freed),
blk_put_rl() is confused by the bogus pointer in q->root_blkg.

For example, with !CONFIG_BLK_DEV_THROTTLING &&
CONFIG_CFQ_GROUP_IOSCHED,
switching IO scheduler from cfq to deadline will cause system stall
after the following warning with 3.6:

> WARNING: at /work/build/linux/block/blk-cgroup.h:250
> blk_put_rl+0x4d/0x95()
> Modules linked in: bridge stp llc sunrpc acpi_cpufreq freq_table mperf
> ipt_REJECT nf_conntrack_ipv4 nf_defrag_ipv4
> Pid: 0, comm: swapper/0 Not tainted 3.6.0 #1
> Call Trace:
>    [] warn_slowpath_common+0x85/0x9d
>  [] warn_slowpath_null+0x1a/0x1c
>  [] blk_put_rl+0x4d/0x95
>  [] __blk_put_request+0xc3/0xcb
>  [] blk_finish_request+0x232/0x23f
>  [] ? blk_end_bidi_request+0x34/0x5d
>  [] blk_end_bidi_request+0x42/0x5d
>  [] blk_end_request+0x10/0x12
>  [] scsi_io_completion+0x207/0x4d5
>  [] scsi_finish_command+0xfa/0x103
>  [] scsi_softirq_done+0xff/0x108
>  [] blk_done_softirq+0x8d/0xa1
>  [] ?
>  generic_smp_call_function_single_interrupt+0x9f/0xd7
>  [] __do_softirq+0x102/0x213
>  [] ? lock_release_holdtime+0xb6/0xbb
>  [] ? raise_softirq_irqoff+0x9/0x3d
>  [] call_softirq+0x1c/0x30
>  [] do_softirq+0x4b/0xa3
>  [] irq_exit+0x53/0xd5
>  [] smp_call_function_single_interrupt+0x34/0x36
>  [] call_function_single_interrupt+0x6f/0x80
>    [] ? mwait_idle+0x94/0xcd
>  [] ? mwait_idle+0x8b/0xcd
>  [] cpu_idle+0xbb/0x114
>  [] rest_init+0xc1/0xc8
>  [] ? csum_partial_copy_generic+0x16c/0x16c
>  [] start_kernel+0x3d4/0x3e1
>  [] ? kernel_init+0x1f7/0x1f7
>  [] x86_64_start_reservations+0xb8/0xbd
>  [] x86_64_start_kernel+0x101/0x110

This patch clears q->root_blkg and q->root_rl.blkg when root blkg
is destroyed.

Signed-off-by: Jun'ichi Nomura 
Acked-by: Vivek Goyal 
Acked-by: Tejun Heo 
Signed-off-by: Jens Axboe 
Signed-off-by: Greg Kroah-Hartman

blkcg: implement per-blkg request allocation

2012-06-26T22:42:49+00:00

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.

* After this change, each block cgroup gets a full request pool making
  resource consumption of each cgroup higher.  This makes allowing
  non-root users to create cgroups less desirable; however, note that
  allowing non-root users to directly manage cgroups is already
  severely broken regardless of this patch - each block cgroup
  consumes kernel memory and skews IO weight (IO weights are not
  hierarchical).

v2: queue-sysfs.txt updated and patch description udpated as suggested
    by Vivek.

v3: blk_get_rl() wasn't checking error return from
    blkg_lookup_create() and may cause oops on lookup failure.  Fix it
    by falling back to root_rl on blkg lookup failures.  This problem
    was spotted by Rakesh Iyer .

v4: Updated to accomodate 458f27a982 "block: Avoid missed wakeup in
    request waitqueue".  blk_drain_queue() now wakes up waiters on all
    blkg->rl on the target queue.

Signed-off-by: Tejun Heo 
Acked-by: Vivek Goyal 
Cc: Wu Fengguang 
Signed-off-by: Jens Axboe

blkcg: inline bio_blkcg() and friends

2012-06-25T09:53:50+00:00

Make bio_blkcg() and friends inline.  They all are very simple and
used only in few places.

This patch is to prepare for further updates to request allocation
path.

Signed-off-by: Tejun Heo 
Acked-by: Vivek Goyal 
Signed-off-by: Jens Axboe

blkcg: make root blkcg allocation use %GFP_KERNEL

2012-06-25T09:53:46+00:00

Currently, blkcg_activate_policy() depends on %GFP_ATOMIC allocation
from __blkg_lookup_create() for root blkcg creation.  This could make
policy fail unnecessarily.

Make blkg_alloc() take @gfp_mask, __blkg_lookup_create() take an
optional @new_blkg for preallocated blkg, and blkcg_activate_policy()
preload radix tree and preallocate blkg with %GFP_KERNEL before trying
to create the root blkg.

v2: __blkg_lookup_create() was returning %NULL on blkg alloc failure
   instead of ERR_PTR() value.  Fixed.

Signed-off-by: Tejun Heo 
Acked-by: Vivek Goyal 
Signed-off-by: Jens Axboe

blkcg: __blkg_lookup_create() doesn't need radix preload

2012-06-25T09:53:45+00:00

There's no point in calling radix_tree_preload() if preloading doesn't
use more permissible GFP mask.  Drop preloading from
__blkg_lookup_create().

While at it, drop sparse locking annotation which no longer applies.

v2: Vivek pointed out the odd preload usage.  Instead of updating,
    just drop it.

Signed-off-by: Tejun Heo 
Acked-by: Vivek Goyal 
Signed-off-by: Jens Axboe

blkcg: drop local variable @q from blkg_destroy()

2012-06-06T06:35:31+00:00

blkg_destroy() caches @blkg->q in local variable @q.  While there are
two places which needs @blkg->q, only lockdep_assert_held() used the
local variable leading to unused local variable warning if lockdep is
configured out.  Drop the local variable and just use @blkg->q
directly.

Signed-off-by: Tejun Heo 
Reported-by: Rakesh Iyer 
Signed-off-by: Jens Axboe

blkcg: fix blkg_alloc() failure path

2012-06-04T08:03:21+00:00

When policy data allocation fails in the middle, blkg_alloc() invokes
blkg_free() to destroy the half constructed blkg.  This ends up
calling pd_exit_fn() on policy datas which didn't go through
pd_init_fn().  Fix it by making blkg_alloc() call pd_init_fn()
immediately after each policy data allocation.

Signed-off-by: Tejun Heo 
Acked-by: Vivek Goyal 
Signed-off-by: Jens Axboe

blkcg: use radix tree to index blkgs from blkcg

2012-04-20T08:06:40+00:00

blkg lookup is currently performed by traversing linked list anchored
at blkcg->blkg_list.  This is very unscalable and with blk-throttle
enabled and enough request queues on the system, this can get very
ugly quickly (blk-throttle performs look up on every bio submission).

This patch makes blkcg use radix tree to index blkgs combined with
simple last-looked-up hint.  This is mostly identical to how icqs are
indexed from ioc.

Note that because __blkg_lookup() may be invoked without holding queue
lock, hint is only updated from __blkg_lookup_create().  Due to cfq's
cfqq caching, this makes hint updates overly lazy.  This will be
improved with scheduled blkcg aware request allocation.

Signed-off-by: Tejun Heo 
Cc: Vivek Goyal 
Signed-off-by: Jens Axboe

blkcg: fix blkcg->css ref leak in __blkg_lookup_create()

2012-04-20T08:06:40+00:00

__blkg_lookup_create() leaked blkcg->css ref if blkg allocation
failed.  Fix it.

Signed-off-by: Tejun Heo 
Cc: Vivek Goyal 
Signed-off-by: Jens Axboe

blkcg: collapse blkcg_policy_ops into blkcg_policy

2012-04-20T08:06:17+00:00

There's no reason to keep blkcg_policy_ops separate.  Collapse it into
blkcg_policy.

This patch doesn't introduce any functional change.

Signed-off-by: Tejun Heo 
Cc: Vivek Goyal 
Signed-off-by: Jens Axboe