Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754106AbdCHSGP (ORCPT ); Wed, 8 Mar 2017 13:06:15 -0500 Received: from mail.linuxfoundation.org ([140.211.169.12]:34210 "EHLO mail.linuxfoundation.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753010AbdCHSGA (ORCPT ); Wed, 8 Mar 2017 13:06:00 -0500 Date: Wed, 8 Mar 2017 19:05:07 +0100 From: "gregkh@linuxfoundation.org" To: Ajay Kaher Cc: Alan Stern , "linux-usb@vger.kernel.org" , "linux-kernel@vger.kernel.org" , AMAN DEEP , HEMANSHU SRIVASTAVA Subject: Re: Subject: [PATCH v4] USB:Core: BugFix: Proper handling of Race Condition when two USB class drivers try to call init_usb_class simultaneously Message-ID: <20170308180507.GA11248@kroah.com> References: <20170307043537epcms5p7e4fcfc351c4f193e454c9ea2e271e562@epcms5p7> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20170307043537epcms5p7e4fcfc351c4f193e454c9ea2e271e562@epcms5p7> User-Agent: Mutt/1.8.0 (2017-02-23) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1950 Lines: 50 On Tue, Mar 07, 2017 at 04:35:37AM +0000, Ajay Kaher wrote: >   >   >   > > On Fri, 3 Mar 2017, Ajay Kaher wrote: > >  > > > > usb_class->kref is not accessible outside the file.c > > > > as usb_class is _static_ inside the file.c and > > > > pointer of usb_class->kref is not passed anywhere. > > > >  > > > > Hence as you wanted, there are no references of usb_class->kref > > > > other than taken by init_usb_class() and released by destroy_usb_class(). > > >  > > > Verified the code again, I hope my last comments clarifed the things > > > which came in your mind and helps you to accept the patch :) > >   > > Your main point is that usb_class->kref is accessed from only two > > points, both of which are protected by the new mutex.  This means there > > is no reason for the value to be a struct kref at all.  You should > > change it to an int (and change its name).  Leaving it as a kref will > > make readers wonder why it needs to be updated atomically. > > At many places in Linux kernel, instances of Kref have been used within > Mutex, SpinLock and don’t have any side effect. > > Making to int and handle (i.e. get/put) it within file.c seems > not good as we have Kref. Instead, we can have non_atomic version of kref. > We can discuss about non_atomic kref in another thread, if you are interested. > > > Also, why does destroy_usb_class() have that "if (usb_class) "test?  > > Isn't it true that usb_class can never be NULL there? > > Removed in Patch v4. > > thanks, > ajay kaher >   >    > Signed-off-by: Ajay Kaher >   Can you resend this in a format that I can apply it in? I suggest reading Documentation/SubmittingPatches. If you have any questions about the correct format, please let me know. Also add Alan's ack to it. thanks, greg k-h