Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1760491Ab2FGJaG (ORCPT ); Thu, 7 Jun 2012 05:30:06 -0400 Received: from youngberry.canonical.com ([91.189.89.112]:33096 "EHLO youngberry.canonical.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755617Ab2FGJaD convert rfc822-to-8bit (ORCPT ); Thu, 7 Jun 2012 05:30:03 -0400 MIME-Version: 1.0 In-Reply-To: References: Date: Thu, 7 Jun 2012 17:30:00 +0800 Message-ID: Subject: Re: [PATCH] driver core: fix shutdown races with probe/remove From: Ming Lei To: Alan Stern Cc: Greg Kroah-Hartman , USB list , Kernel development list , Paul McKenney Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 8BIT Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2112 Lines: 53 On Wed, Jun 6, 2012 at 10:44 PM, Alan Stern wrote: >> > On the whole, it might be easier just to hold the device lock during >> > the shutdown call. >> >> Yes, it is easier, but it is not efficient because there are very less >> drivers which implemented .shutdown callback. Suppose there are some >> drivers which have no .shutdown but are holding device lock, ?extra >> time will be consumed in the lock acquiring, which may slow down 'power >> down'. > > The main reasons for holding the device lock are probe and remove > callbacks. ?In your patch, synchronize_srcu has to wait for those too. > So there won't be much extra time. There are differences between holding dev->lock(parent->lock) and using synchronize_srcu: - synchronize_srcu just wait for the ongoing probe/release to complete - holding dev->lock and parent->lock before shutdown means every dev->lock and parent->lock is to be acquired, so any drivers holding dev lock during device_shutdown will slow down 'power down' or 'reset'. - if the device has not .shutdown implemented, it is not necessary to acquire the lock and parent->lock, either of which may has been held in other places already. But after thinking about the problem further, the patch isn't good enough: - even probe/remove is disallowed during shutdown, device_add/ device_del can be completed and only probe and release things are bypassed, so .shutdown may see a deleted device from view of driver model. For example, pci_device_shutdown need to access its devres which may have been deleted by device_del already. - violate driver model's implicit rule: once device_release_driver (in device_del path) is completed, other callbacks for the device(driver) won't be seen or called by others So take the simply approach of holding lock to fix the races. Thanks, -- Ming Lei -- 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/