Re: [PATCH] block: Ensure that a request queue is dissociated from the cgroup controller

2018-04-13 Thread t...@kernel.org
Hello, Bart. On Thu, Apr 12, 2018 at 10:40:52PM +, Bart Van Assche wrote: > Is something like the patch below perhaps what you had in mind? Yeah, exactly. It'd be really great to have the lockdep asserts add to the right places. Thanks. -- tejun

Re: [PATCH] block: Ensure that a request queue is dissociated from the cgroup controller

2018-04-12 Thread Bart Van Assche
On Thu, 2018-04-12 at 12:09 -0700, t...@kernel.org wrote: > On Thu, Apr 12, 2018 at 06:56:26PM +, Bart Van Assche wrote: > > On Thu, 2018-04-12 at 11:11 -0700, t...@kernel.org wrote: > > > * Right now, blk_queue_enter/exit() doesn't have any annotations. > > > IOW, there's no way for paths

Re: [PATCH] block: Ensure that a request queue is dissociated from the cgroup controller

2018-04-12 Thread t...@kernel.org
Hello, On Thu, Apr 12, 2018 at 06:56:26PM +, Bart Van Assche wrote: > On Thu, 2018-04-12 at 11:11 -0700, t...@kernel.org wrote: > > * Right now, blk_queue_enter/exit() doesn't have any annotations. > > IOW, there's no way for paths which need enter locked to actually > > assert that. If

Re: [PATCH] block: Ensure that a request queue is dissociated from the cgroup controller

2018-04-12 Thread Bart Van Assche
On Thu, 2018-04-12 at 11:11 -0700, t...@kernel.org wrote: > * Right now, blk_queue_enter/exit() doesn't have any annotations. > IOW, there's no way for paths which need enter locked to actually > assert that. If we're gonna protect more things with queue > enter/exit, it gotta be annotated.

Re: [PATCH] block: Ensure that a request queue is dissociated from the cgroup controller

2018-04-12 Thread t...@kernel.org
Hello, On Thu, Apr 12, 2018 at 04:29:09PM +, Bart Van Assche wrote: > Any code that submits a bio or request needs blk_queue_enter() / > blk_queue_exit() anyway. Please have a look at the following commit - you will > see that that commit reduces the number of blk_queue_enter() / >

Re: [PATCH] block: Ensure that a request queue is dissociated from the cgroup controller

2018-04-12 Thread Bart Van Assche
On Thu, 2018-04-12 at 09:12 -0700, t...@kernel.org wrote: > > Did you perhaps mean blkg_lookup_create()? That function has one caller, > > namely blkcg_bio_issue_check(). The only caller of that function is > > generic_make_request_checks(). A patch was posted on the linux-block mailing > > list

Re: [PATCH] block: Ensure that a request queue is dissociated from the cgroup controller

2018-04-12 Thread t...@kernel.org
Hello, On Thu, Apr 12, 2018 at 04:03:52PM +, Bart Van Assche wrote: > On Thu, 2018-04-12 at 08:37 -0700, Tejun Heo wrote: > > On Thu, Apr 12, 2018 at 08:09:17AM -0600, Bart Van Assche wrote: > > > I have retested hotunplugging by rerunning the srp-test software. It > > > seems like you

Re: [PATCH] block: Ensure that a request queue is dissociated from the cgroup controller

2018-04-12 Thread Bart Van Assche
On Thu, 2018-04-12 at 08:37 -0700, Tejun Heo wrote: > On Thu, Apr 12, 2018 at 08:09:17AM -0600, Bart Van Assche wrote: > > I have retested hotunplugging by rerunning the srp-test software. It > > seems like you overlooked that this patch does not remove the > > blkcg_exit_queue() call from

Re: [PATCH] block: Ensure that a request queue is dissociated from the cgroup controller

2018-04-12 Thread Tejun Heo
Hello, Bart. On Thu, Apr 12, 2018 at 08:09:17AM -0600, Bart Van Assche wrote: > I have retested hotunplugging by rerunning the srp-test software. It > seems like you overlooked that this patch does not remove the > blkcg_exit_queue() call from blk_cleanup_queue()? If a device is > hotunplugged it

Re: [PATCH] block: Ensure that a request queue is dissociated from the cgroup controller

2018-04-12 Thread Bart Van Assche
On 04/12/18 07:51, Tejun Heo wrote: On Wed, Apr 11, 2018 at 07:58:52PM -0600, Bart Van Assche wrote: Several block drivers call alloc_disk() followed by put_disk() if something fails before device_add_disk() is called without calling blk_cleanup_queue(). Make sure that also for this scenario a

Re: [PATCH] block: Ensure that a request queue is dissociated from the cgroup controller

2018-04-12 Thread t...@kernel.org
On Thu, Apr 12, 2018 at 06:58:21AM -0700, t...@kernel.org wrote: > Cool, can we just factor out the queue lock from those drivers? It's > just really messy to be switching locks like we do in the cleanup > path. So, looking at a couple drivers, it looks like all we'd need is a struct which

Re: [PATCH] block: Ensure that a request queue is dissociated from the cgroup controller

2018-04-12 Thread t...@kernel.org
Hello, On Thu, Apr 12, 2018 at 03:56:51PM +0200, h...@lst.de wrote: > On Thu, Apr 12, 2018 at 06:48:12AM -0700, t...@kernel.org wrote: > > > Which sounds like a very good reason not to use a driver controller > > > lock for internals like blkcq. > > > > > > In fact splitting the lock used for

Re: [PATCH] block: Ensure that a request queue is dissociated from the cgroup controller

2018-04-12 Thread h...@lst.de
On Thu, Apr 12, 2018 at 06:48:12AM -0700, t...@kernel.org wrote: > > Which sounds like a very good reason not to use a driver controller > > lock for internals like blkcq. > > > > In fact splitting the lock used for synchronizing access to queue > > fields from the driver controller lock used to

Re: [PATCH] block: Ensure that a request queue is dissociated from the cgroup controller

2018-04-12 Thread Tejun Heo
On Wed, Apr 11, 2018 at 07:58:52PM -0600, Bart Van Assche wrote: > Several block drivers call alloc_disk() followed by put_disk() if > something fails before device_add_disk() is called without calling > blk_cleanup_queue(). Make sure that also for this scenario a request > queue is dissociated

Re: [PATCH] block: Ensure that a request queue is dissociated from the cgroup controller

2018-04-12 Thread t...@kernel.org
Hello, On Thu, Apr 12, 2018 at 03:14:40PM +0200, h...@lst.de wrote: > > At least the SCSI ULP drivers drop the last reference to a disk after > > the blk_cleanup_queue() call. As explained in the description of commit > > a063057d7c73, removing a request queue from blkcg must happen before > >

Re: [PATCH] block: Ensure that a request queue is dissociated from the cgroup controller

2018-04-12 Thread h...@lst.de
On Thu, Apr 12, 2018 at 11:52:11AM +, Bart Van Assche wrote: > On Thu, 2018-04-12 at 07:34 +0200, Christoph Hellwig wrote: > > On Wed, Apr 11, 2018 at 07:58:52PM -0600, Bart Van Assche wrote: > > > Several block drivers call alloc_disk() followed by put_disk() if > > > something fails before

Re: [PATCH] block: Ensure that a request queue is dissociated from the cgroup controller

2018-04-12 Thread Bart Van Assche
On Thu, 2018-04-12 at 07:34 +0200, Christoph Hellwig wrote: > On Wed, Apr 11, 2018 at 07:58:52PM -0600, Bart Van Assche wrote: > > Several block drivers call alloc_disk() followed by put_disk() if > > something fails before device_add_disk() is called without calling > > blk_cleanup_queue(). Make

Re: [PATCH] block: Ensure that a request queue is dissociated from the cgroup controller

2018-04-11 Thread Christoph Hellwig
On Wed, Apr 11, 2018 at 07:58:52PM -0600, Bart Van Assche wrote: > Several block drivers call alloc_disk() followed by put_disk() if > something fails before device_add_disk() is called without calling > blk_cleanup_queue(). Make sure that also for this scenario a request > queue is dissociated

Re: [PATCH] block: Ensure that a request queue is dissociated from the cgroup controller

2018-04-11 Thread Alexandru Moise
On Thu, Apr 12, 2018 at 06:20:34AM +0200, Alexandru Moise wrote: > On Wed, Apr 11, 2018 at 07:58:52PM -0600, Bart Van Assche wrote: > > Several block drivers call alloc_disk() followed by put_disk() if > > something fails before device_add_disk() is called without calling > > blk_cleanup_queue().

Re: [PATCH] block: Ensure that a request queue is dissociated from the cgroup controller

2018-04-11 Thread Alexandru Moise
On Wed, Apr 11, 2018 at 07:58:52PM -0600, Bart Van Assche wrote: > Several block drivers call alloc_disk() followed by put_disk() if > something fails before device_add_disk() is called without calling > blk_cleanup_queue(). Make sure that also for this scenario a request > queue is dissociated

Re: [PATCH] block: Ensure that a request queue is dissociated from the cgroup controller

2018-04-11 Thread Alexandru Moise
On Wed, Apr 11, 2018 at 07:58:52PM -0600, Bart Van Assche wrote: > Several block drivers call alloc_disk() followed by put_disk() if > something fails before device_add_disk() is called without calling > blk_cleanup_queue(). Make sure that also for this scenario a request > queue is dissociated