From mboxrd@z Thu Jan 1 00:00:00 1970 From: Tejun Heo Subject: Re: Race condition between "read CFQ stats" and "block device shutdown" Date: Thu, 26 Sep 2013 12:30:29 -0400 Message-ID: <20130926163029.GD32391@mtj.dyndns.org> References: <5226D661.7070301@suse.de> <20130904160723.GC26609@mtj.dyndns.org> <20130926135443.GC2480@htj.dyndns.org> <5244423A.2050107@suse.de> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Return-path: Received: from mail-qa0-f45.google.com ([209.85.216.45]:55206 "EHLO mail-qa0-f45.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752353Ab3IZQac (ORCPT ); Thu, 26 Sep 2013 12:30:32 -0400 Content-Disposition: inline In-Reply-To: Sender: linux-scsi-owner@vger.kernel.org List-Id: linux-scsi@vger.kernel.org To: Anatol Pomozov Cc: Hannes Reinecke , Cgroups , Jens Axboe , linux-scsi@vger.kernel.org Hello, On Thu, Sep 26, 2013 at 09:23:19AM -0700, Anatol Pomozov wrote: > We use modified version of "sbull" block device driver from GKH book. > We use it for testing block device startup/shutdown path + CFQ > manipulation. > > The sbull driver uses function > blk_init_queue(..., &dev->qlock); > > it passes lock as a second parameter and function makes the lock > swapping. According to your information passing lock to > blk_init_queue() considered as an "old non recommended way" so we > should modify our driver and avoid doing this. I'll take a look. I wonder whether we can make request_queue record the passed in lock and grab it in addition to the queue lock instead of replacing it when invoking the request function. This would mean more overhead for the broken drivers but given how legacy they are at this point, I don't think that's gonna be a big problem. if that's likely to work, we may as well just push all the extra locking to the legacy drivers so that their request functions take the extra lock and then we can remove the spinlock argument completely. I'm not sure whether the locking in request path is the only thing necessary tho. Thanks. -- tejun