linux-kernel - Re: [PATCH 5/8] blkcg: make sure blkg_lookup() returns %NULL if @q is bypassing

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: <20120413160053.GE26383@redhat.com>
Date:	Fri, 13 Apr 2012 12:00:53 -0400
From:	Vivek Goyal <vgoyal@...hat.com>
To:	Tejun Heo <tj@...nel.org>
Cc:	axboe@...nel.dk, ctalbott@...gle.com, rni@...gle.com,
	linux-kernel@...r.kernel.org, cgroups@...r.kernel.org,
	containers@...ts.linux-foundation.org
Subject: Re: [PATCH 5/8] blkcg: make sure blkg_lookup() returns %NULL if @q
 is bypassing

On Thu, Apr 12, 2012 at 04:29:37PM -0700, Tejun Heo wrote:

[..]
>   * In bypass mode, only the dispatch FIFO queue of @q is used.  This
>   * function makes @q enter bypass mode and drains all requests which were
>   * throttled or issued before.  On return, it's guaranteed that no request
> - * is being throttled or has ELVPRIV set.
> + * is being throttled or has ELVPRIV set and blk_queue_bypass() is %true
> + * inside queue or RCU read lock.
>   */
>  void blk_queue_bypass_start(struct request_queue *q)
>  {
> @@ -426,6 +427,7 @@ void blk_queue_bypass_start(struct request_queue *q)
>  	spin_unlock_irq(q->queue_lock);
>  
>  	blk_drain_queue(q, false);
> +	synchronize_rcu();

Hi Tejun,

I guess this synchronize_rcu() needs some comments here to make it clear
what it meant for. IIUC, you are protecting against policy data (stats
update) which happen under rcu in throttling code? You want to make sure
all these updaters are done before you go ahead with
activation/deactivation of a policy.

Well, I am wondering if CFQ is policy being activated/deactivated why
should we try to drain other policie's requests. Can't one continue
to work without draining all the throttled requests. We probably just
need to make sure new groups are not created.

Anyway, this is not a big deal. I was more concerned about other polices
losing configuration while one policy is changing. And that is fixed
with this patchset. It is nice to see all that in place update_root
go away. That was confusing.

Thanks
Vivek
--
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/