Re: [PATCH 086/117] Staging: hv: storvsc: Leverage the spinlock to manage ref_cnt

On Wed, Aug 24, 2011 at 12:58:36AM +0000, KY Srinivasan wrote:

quoted

-----Original Message-----
From: Greg KH [mailto:greg@kroah.com]
Sent: Tuesday, August 23, 2011 7:10 PM
To: KY Srinivasan
Cc: gregkh@suse.de; linux-kernel@vger.kernel.org;
devel@linuxdriverproject.org; virtualization@lists.osdl.org; Haiyang Zhang
Subject: Re: [PATCH 086/117] Staging: hv: storvsc: Leverage the spinlock to
manage ref_cnt

On Fri, Jul 15, 2011 at 10:47:14AM -0700, K. Y. Srinivasan wrote:

quoted

Now that we have a spin lock protecting access to the stor device pointer,
use it manage the reference count as well.

Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
---
 drivers/staging/hv/hyperv_storage.h |    8 ++++----
 drivers/staging/hv/storvsc.c        |   10 +++++-----
 2 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/staging/hv/hyperv_storage.h

b/drivers/staging/hv/hyperv_storage.h

quoted

index 53b65be..d946211 100644

--- a/drivers/staging/hv/hyperv_storage.h
+++ b/drivers/staging/hv/hyperv_storage.h

@@ -265,7 +265,7 @@ struct storvsc_device {
 	struct hv_device *device;

 	/* 0 indicates the device is being destroyed */
-	atomic_t ref_count;
+	int	 ref_count;

Is this really needed?  Can't you rely on the reference count of the
hv_device itself?

We don't have a reference count on the hv_device

Wait, why not?  You shure better have a reference count on that device
if you have a pointer to it, if not, you have a bug, and that needs to
be fixed.  Please reread Documentation/CodingStyle for details.

and this count is taken to deal with racing unloads and incoming
traffic on the channel from the host. 

Is this something that all other storage drivers have to do?  If not,
then you shouldn't be doing that as well.

greg k-h