Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755852AbZJKOhL (ORCPT ); Sun, 11 Oct 2009 10:37:11 -0400 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1752383AbZJKOhH (ORCPT ); Sun, 11 Oct 2009 10:37:07 -0400 Received: from earthlight.etchedpixels.co.uk ([81.2.110.250]:48815 "EHLO www.etchedpixels.co.uk" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1753977AbZJKOhG (ORCPT ); Sun, 11 Oct 2009 10:37:06 -0400 Date: Sun, 11 Oct 2009 15:37:09 +0100 From: Alan Cox To: John Kacur Cc: linux-kernel@vger.kernel.org, Thomas Gleixner , Frederic Weisbecker , Ingo Molnar , Christoph Hellwig , Jonathan Corbet , Andrew Morton , Vincent Sanders , linux-scsi@vger.kernel.org Subject: Re: [PATCH RFC] [PATCH] drivers/scsi/ch.c: Remove BKL in ch_open Message-ID: <20091011153709.4438cf9c@lxorguk.ukuu.org.uk> In-Reply-To: References: X-Mailer: Claws Mail 3.7.2 (GTK+ 2.14.7; x86_64-redhat-linux-gnu) Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1443 Lines: 47 On Sun, 11 Oct 2009 15:45:16 +0200 (CEST) John Kacur wrote: [linux-scsi cc'd] > Locking in ch_open is covered by the spin_lock, it serializes the calls > to idr_find and scsi_device_get. The BKL appears redundant to me here. I'm not so sure. In fact there are some quite umm interesting questions about this code, and some of them are shared with other modules too. Consider the following sequence CPU1 CPU2 register_chrdev ok open device takes lock scsi_register_driver error unregister_chrdev error unload ?????? We don't allocate any idr entries in open so the paths don't seem to leak and the module itself looks correct. Looking at the bigger picture however I am not sure what the module loader is trying to do in kernel/module.c with the code at if (ret < 0) { /* Init routine failed: abort. Try to protect us from buggy refcounters. */ mod->state = MODULE_STATE_GOING; synchronize_sched(); module_put(mod); blocking_notifier_call_chain(&module_notify_list, but it doesn't seem to be sufficient for what may go on ? Rusty ? -- 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/