Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Wed, 17 Apr 2019 14:24:06 -0400 (EDT)
From:   Alan Stern <stern@rowland.harvard.edu>
To:     Raul Rangel <rrangel@chromium.org>
cc:     Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        <linux-usb@vger.kernel.org>, <groeck@chromium.org>,
        <oneukum@suse.com>, <djkurtz@chromium.org>, <zwisler@chromium.org>,
        Sebastian Andrzej Siewior <bigeasy@linutronix.de>,
        Martin Blumenstingl <martin.blumenstingl@googlemail.com>,
        Dmitry Torokhov <dtor@chromium.org>,
        Suwan Kim <suwan.kim027@gmail.com>,
        <linux-kernel@vger.kernel.org>,
        "Gustavo A. R. Silva" <gustavo@embeddedor.com>,
        Miquel Raynal <miquel.raynal@bootlin.com>,
        Johan Hovold <johan@kernel.org>,
        Mathias Nyman <mathias.nyman@linux.intel.com>
Subject: Re: [PATCH v2] usb/hcd: Send a uevent signaling that the host
 controller has died
In-Reply-To: <20190417175300.GA74282@google.com>
Message-ID: <Pine.LNX.4.44L0.1904171419260.1400-100000@iolanthe.rowland.org>
MIME-Version: 1.0
Content-Type: TEXT/PLAIN; charset=US-ASCII
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Wed, 17 Apr 2019, Raul Rangel wrote:

> > > Also where would be a good place to document this?
> > 
> > Documentation/ABI/ is a good start.
> I'll add something to Documentation/ABI/testing/xhci-uevent

Your patch will apply to all host controllers, not just xhci.  The
documentation filename should reflect this.  Perhaps "usb-uevent"?

> > Why do you need to lock something that is "dead"?  And why is the idr
> > lock the correct one here?
> We need to ensure that root_hub is not null. Though I'm not sure the
> lock is entirely necessary in this case. usb_remove_hcd stops the work
> item before it sets the rhdev to null. The reason I picked
> usb_bus_idr_lock was because it's the same lock that usb_remove_hcd uses
> when setting rhdev = NULL.
> 
> Alan, what do you think? Should I remove the lock?

You're both right; the lock isn't needed because the work is stopped
before the root hub gets removed.  Acquiring the lock doesn't do any
harm, but it isn't needed so you probably should remove it.  In fact, 
you don't even need to test for whether hcd->self.root_hub is 
non-NULL.

Alan Stern