Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754527Ab0DOO3x (ORCPT ); Thu, 15 Apr 2010 10:29:53 -0400 Received: from moutng.kundenserver.de ([212.227.17.8]:64768 "EHLO moutng.kundenserver.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753127Ab0DOO3v (ORCPT ); Thu, 15 Apr 2010 10:29:51 -0400 From: Arnd Bergmann To: dgilbert@interlog.com Subject: Re: [PATCH 1/2] [RFC] block: replace BKL with global mutex Date: Thu, 15 Apr 2010 16:29:44 +0200 User-Agent: KMail/1.12.2 (Linux/2.6.31-19-generic; KDE/4.3.2; x86_64; ; ) Cc: Jens Axboe , Vivek Goyal , Tejun Heo , Frederic Weisbecker , FUJITA Tomonori , "Martin K. Petersen" , Steven Rostedt , Ingo Molnar , John Kacur , linux-scsi@vger.kernel.org, linux-kernel@vger.kernel.org, Kai Makisara References: <1271277384-7627-1-git-send-email-arnd@arndb.de> <201004150911.45842.arnd@arndb.de> <4BC7117C.7090208@interlog.com> In-Reply-To: <4BC7117C.7090208@interlog.com> MIME-Version: 1.0 Content-Type: Text/Plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit Message-Id: <201004151629.45029.arnd@arndb.de> X-Provags-ID: V01U2FsdGVkX19Z225+Nm1PsHdaLc6svbQX1OUC8ap1NHUQ8W0 96gGp0/z6rrIGqy8fXdbHBekd7iCd9nP+baR1i04uwzb7OCBGj t6yMDgQTNyuI6jog3vlmw== Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2405 Lines: 56 On Thursday 15 April 2010, Douglas Gilbert wrote: > At the level of SCSI commands, tape device state assumptions > made by the st driver could be compromised by SCSI commands > sent by the sg driver. However the BKL was never meant > to address that concern. > > From the comment in st_open() [st.c] it would be using > nonseekable_open() as well but there are apps out there > that do lseek()s on its file descriptors. Not sure > how long nonseekable_open() has been in the sg driver > but no-one has complained to me about it. It's been there for a long time, at least since the start of the git history, and it's very likely correct this way. > > The most simple solution for this would be to let sg > > take both blkdev_mutex and the BKL, which of course > > feels like a step backwards. > > > > A better way is to get rid of the BKL in sg, which requires > > a better understanding of what it's actually protecting. > > It only gets it in the open and ioctl functions, which is a > > result of the pushdown from the respective file operations. > > Chances are that it's not needed at all, but that's really > > hard to tell. Can you shed some more light on this? > > The BKL is not used to protect any of the internal > objects within the sg driver. From memory it was added > in some large code sweep through, not unlike what you > are proposing now. The one in the open function was moved there when the BKL was moved out from vfs_open(), while the use in ioctl is implicit by never having been converted to unlocked_ioctl. I don't see anything that really needs BKL protection in sg_open, so that can probably just be killed. For sg_ioctl, at least the blk_trace_* and scsi_ioctl stuff is currently called with BKL held everywhere else (not in st_ioctl though) and may still need that. > So I would not be concerned about any kernel locking > interactions between the sg and st drivers. I have > added Kai Makisara (st maintainer) to the cc list. Ok. I've also checked st.c again and noticed that it doesn't use use the BKL in ioctl() but only in open(), which is very unlikely to race against anything in sg.c or the block subsystem. Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/