All of lore.kernel.org
 help / color / mirror / Atom feed
From: Vivek Goyal <vgoyal@redhat.com>
To: Tejun Heo <tj@kernel.org>
Cc: linux-kernel@vger.kernel.org, jaxboe@fusionio.com
Subject: Re: [PATCH 2/2] blk-throttle: Take blkcg->lock while traversing blkcg->policy_list
Date: Thu, 20 Oct 2011 17:20:21 -0400	[thread overview]
Message-ID: <20111020212021.GA2841@redhat.com> (raw)
In-Reply-To: <20111020211140.GY25124@google.com>

On Thu, Oct 20, 2011 at 02:11:40PM -0700, Tejun Heo wrote:
> Hello,
> 
> On Thu, Oct 20, 2011 at 05:08:26PM -0400, Vivek Goyal wrote:
> > blkcg->policy_list is protected by blkcg->lock. Its not rcu protected
> > list. So even for readers, they need to take blkcg->lock. There are
> > few functions which were reading the list without taking lock. Fix it.
> > 
> > Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
> 
>  Acked-by: Tejun Heo <tj@kernel.org>
> 
> And yeap, some of rcu usages in cfq/iocg seem either incorrect or
> unnecessary.  Trying to clean up that now too.

Cool. I am waiting for the patches as I am curious to know what's incorrect
or unnecessary.

I noticed above two problems while trying to figure out how can we cleanup
the device rules when devices goes away. One place could be.

blk_throtl_exit() {
	blkiocg_del_blkio_group() {
		delete_any_policy_nodes_associated_with_this device;
	}
}

The only problem with this approach is that it will cleanup per device
weight rules also at elevator_exit() time which is not same as device
removal and one might device to bring CFQ back on device and we will
need the rules again.

cfq_exit_queue()
  cfq_release_cfq_groups
    cfq_blkiocg_del_blkio_group()
      blkiocg_del_blkio_group()

Thanks
Vivek

  reply	other threads:[~2011-10-20 21:20 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-10-20 21:08 [PATCH 1/2] blk-throttle: Free up policy node associated with deleted rule Vivek Goyal
2011-10-20 21:08 ` [PATCH 2/2] blk-throttle: Take blkcg->lock while traversing blkcg->policy_list Vivek Goyal
2011-10-20 21:11   ` Tejun Heo
2011-10-20 21:20     ` Vivek Goyal [this message]
2011-10-20 21:29       ` Tejun Heo
2011-10-21 12:10         ` Vivek Goyal
2011-10-25 14:13           ` Jens Axboe
2011-10-25 19:01             ` Vivek Goyal
2011-10-20 21:09 ` [PATCH 1/2] blk-throttle: Free up policy node associated with deleted rule Tejun Heo
2011-10-25 13:36 ` Vivek Goyal
2011-10-25 13:45   ` Jens Axboe

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20111020212021.GA2841@redhat.com \
    --to=vgoyal@redhat.com \
    --cc=jaxboe@fusionio.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=tj@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.