RE: [PATCH 081/117] Staging: hv: vmbus: Introduce a lock to protectthe ext field in hv_device

[KY Srinivasan]

> -----Original Message-----
> From: Greg KH [mailto:greg@xxxxxxxxx]
> Sent: Tuesday, August 23, 2011 10:58 PM
> To: KY Srinivasan
> Cc: gregkh@xxxxxxx; linux-kernel@xxxxxxxxxxxxxxx;
> devel@xxxxxxxxxxxxxxxxxxxxxx; virtualization@xxxxxxxxxxxxxx; Haiyang Zhang
> Subject: Re: [PATCH 081/117] Staging: hv: vmbus: Introduce a lock to protect the
> ext field in hv_device
> 
> On Wed, Aug 24, 2011 at 12:55:12AM +0000, KY Srinivasan wrote:
> >
> >
> > > -----Original Message-----
> > > From: Greg KH [mailto:greg@xxxxxxxxx]
> > > Sent: Tuesday, August 23, 2011 7:08 PM
> > > To: KY Srinivasan
> > > Cc: gregkh@xxxxxxx; linux-kernel@xxxxxxxxxxxxxxx;
> > > devel@xxxxxxxxxxxxxxxxxxxxxx; virtualization@xxxxxxxxxxxxxx; Haiyang Zhang
> > > Subject: Re: [PATCH 081/117] Staging: hv: vmbus: Introduce a lock to protect
> the
> > > ext field in hv_device
> > >
> > > On Fri, Jul 15, 2011 at 10:47:09AM -0700, K. Y. Srinivasan wrote:
> > > > The current mechanism for handling references in broken.
> > > > Introduce a lock to protect the ext field in hv_device.
> > >
> > > Why would that lock ever be needed?  How can things change to this
> > > pointer in different ways like you are thinking it could?  Doesn't the
> > > reference counting in the device itself handle this properly?
> >
> > This is to deal with a potential race condition between the driver being
> > unloaded and incoming traffic from the VMBUS side. The ext pointer is
> > device specific (either pointing to a storage or a network device) and what
> > we are protecting is the pointer being set to NULL from the unload side when
> > we might have a racing access from the interrupt side (on the incoming vmbus
> > traffic).
> 
> I still don't think this is needed at all, the drivers should not have
> to worry about this.  Something is wrong with the design if it is, as no
> other bus needs something like this, right?

This notion of reference counting has been in this code ever since I have been
working on this code; it just that I fixed some obvious holes/race conditions in the
logic to manage the reference counting. Firstly, this is not mandated by the vmbus and is 
used to support the protocol between the guest and host for storage, networking etc.
At least in the case of both block and networking, the client side driver code (running in the guest)
Initiates book keeping messages to the host that is not known to upper level code in Linux. So for
Instance, even though SCSI layer knows exactly what I/O's are pending and takes care of reference
counting the structures, it does not know about messages that have been sent by the lower level
storvsc driver code. If a racing unload were to happen, we could potentially dereference a NULL
ext pointer from the incoming packet processing context. The cases where this can happen are even
more prevalent on the networking side.

Regards,

K. Y

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/