Re: [PATCH 09/13] bdi: Do not wait for cgwbs release in bdi_unregister()

[Tejun Heo <tj@kernel.org>]

Hello,

On Tue, Feb 21, 2017 at 06:09:54PM +0100, Jan Kara wrote:
> @@ -726,14 +718,6 @@ static void cgwb_bdi_destroy(struct backing_dev_info *bdi)
>  	}
>  
>  	spin_unlock_irq(&cgwb_lock);
> -
> -	/*
> -	 * All cgwb's and their congested states must be shutdown and
> -	 * released before returning.  Drain the usage counter to wait for
> -	 * all cgwb's and cgwb_congested's ever created on @bdi.
> -	 */
> -	atomic_dec(&bdi->usage_cnt);
> -	wait_event(cgwb_release_wait, !atomic_read(&bdi->usage_cnt));
>  }

Hmm... I'm not sure about wb_shutdown() synchronization.  If you look
at the function, it's allowed to be called multiple times but doesn't
synchronize the end of the operation.  With usage_cnt, it was okay
because cgwb_bdi_destroy() would have waited until everything is
finished via usage_cnt, but with that gone, we can have a race like
the following.

	A					B
 a cgroup gets removed
 a cgwb starts to get destroyed
 it starts wb_shutdown()
					bdi starts getting destroyed
					calls cgwb_bdi_destroy()
					calls wb_shutdown() on the same cgwb
					  but it returns because it lost to
 wb_shutdown() is still in progress	  A's wb_shutdown()
					bdi destruction proceeds
 Oops.

So, I think we need to make sure that wb_shutdown()'s are properly
synchronized from start to end to get rid of the usage_cnt waiting.

Thanks.

-- 
tejun