All of lore.kernel.org
 help / color / mirror / Atom feed
From: Li Zhong <zhong@linux.vnet.ibm.com>
To: Alan Stern <stern@rowland.harvard.edu>
Cc: Johan Hovold <jhovold@gmail.com>, Tejun Heo <tj@kernel.org>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org,
	rafael.j.wysocki@intel.com
Subject: Re: [PATCH] USB: serial: fix sysfs-attribute removal deadlock
Date: Mon, 28 Apr 2014 09:58:52 +0800	[thread overview]
Message-ID: <1398650332.3046.67.camel@ThinkPad-T5421> (raw)
In-Reply-To: <Pine.LNX.4.44L0.1404250954360.1140-100000@iolanthe.rowland.org>

On Fri, 2014-04-25 at 09:59 -0400, Alan Stern wrote:
> On Fri, 25 Apr 2014, Li Zhong wrote:
> 
> > > No, this isn't self removal. The driver-attribute (not device-attribute)
> > > store operation simply grabs a lock that is also held while the driver
> > > is being deregistered at module unload. Taking a reference to the module
> > > in this case will prevent deregistration while store is running.
> > > 
> > > But it seems like this can be solved for usb-serial by simply not
> > > holding the lock while deregistering.
> > 
> > I didn't look carefully about this lock. 
> > 
> > But I'm not sure whether there are such requirements for driver
> > attributes:
> > 
> > some lock needs be grabbed in the driver attributes store callbacks, and
> > the same lock also needs to be grabbed during driver unregister. 
> 
> In this case, the lock does _not_ need to be grabbed during driver 
> unregister.  The driver grabs the lock, but it doesn't need to.

OK.

> 
> > If we have such requirements currently or in the future, I think they
> > could all be solved by breaking active protection after get the module
> > reference.
> 
> No!  That would be very bad.
> 
> Unloading modules is quite different from unbinding drivers.  After the
> driver is unbound, its attribute callback routines can continue to run.  
> But after a driver module has been unloaded, its attribute callback 
> routines _cannot_ run because they aren't present in memory any more.
> 
> If we allowed a module to be unloaded while one of its callbacks was 
> running (because active protection was broken), imagine what would 
> happen...

I don't think the module could be unloaded after we increased the module
reference counter. 

Thanks, Zhong

> 
> Alan Stern
> 



  reply	other threads:[~2014-04-28  1:59 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-04-23  9:32 [PATCH] USB: serial: fix sysfs-attribute removal deadlock Johan Hovold
2014-04-23 14:19 ` Tejun Heo
2014-04-24  8:29   ` Li Zhong
2014-04-24 14:35     ` Tejun Heo
2014-04-24 14:52       ` Johan Hovold
2014-04-25  2:16         ` Li Zhong
2014-04-25 10:15           ` Johan Hovold
2014-04-28  0:39             ` Li Zhong
2014-05-02 15:20               ` Tejun Heo
2014-04-25 13:59           ` Alan Stern
2014-04-28  1:58             ` Li Zhong [this message]
2014-04-25  2:15       ` Li Zhong
2014-04-25 13:54         ` Alan Stern
2014-04-25 15:13           ` Johan Hovold
2014-04-28  1:55           ` Li Zhong

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1398650332.3046.67.camel@ThinkPad-T5421 \
    --to=zhong@linux.vnet.ibm.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=jhovold@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=rafael.j.wysocki@intel.com \
    --cc=stern@rowland.harvard.edu \
    --cc=tj@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.