Thread (6 messages) 6 messages, 2 authors, 2011-11-30

RE: [PATCH 1/1] Staging: hv: storvsc: Move the storage driver out of the staging area

From: KY Srinivasan <kys@microsoft.com>
Date: 2011-11-30 13:46:31
Also in: linux-scsi, lkml

-----Original Message-----
From: KY Srinivasan
Sent: Thursday, November 17, 2011 11:53 PM
To: 'James Bottomley'
Cc: gregkh@suse.de; linux-kernel@vger.kernel.org;
devel@linuxdriverproject.org; virtualization@lists.osdl.org; linux-
scsi@vger.kernel.org; ohering@suse.com; hch@infradead.org
Subject: RE: [PATCH 1/1] Staging: hv: storvsc: Move the storage driver out of the
staging area


quoted
-----Original Message-----
From: James Bottomley [mailto:James.Bottomley@HansenPartnership.com]
Sent: Thursday, November 17, 2011 1:27 PM
To: KY Srinivasan
Cc: gregkh@suse.de; linux-kernel@vger.kernel.org;
devel@linuxdriverproject.org; virtualization@lists.osdl.org; linux-
scsi@vger.kernel.org; ohering@suse.com; hch@infradead.org
Subject: Re: [PATCH 1/1] Staging: hv: storvsc: Move the storage driver out of
the
quoted
staging area

On Tue, 2011-11-08 at 10:13 -0800, K. Y. Srinivasan wrote:
quoted
The storage driver (storvsc_drv.c) handles all block storage devices
assigned to Linux guests hosted on Hyper-V. This driver has been in the
staging tree for a while and this patch moves it out of the staging area.
As per Greg's recommendation, this patch makes no changes to the
staging/hv
quoted
quoted
directory. Once the driver moves out of staging, we will cleanup the
staging/hv directory.

This patch includes all the patches that I have sent against the staging/hv
tree to address the comments I have gotten to date on this storage driver.
First comment is that it would have been easier to see the individual
patches for comment before you committed them.
I am not sure if the patches have been committed yet. All patches were sent
to various mailing lists and you were copied as well. In the future, I will include
the scsi mailing list in the set of lists I include for the staging patches.
Greg has checked in these patches now.
quoted
The way you did mempool isn't entirely right: the problem is that to
prevent a memory to I/O deadlock we need to ensure forward progress on
the drain device.  Just having 64 commands available to the host doesn't
necessarily achieve this because LUN1 could consume them all and starve
LUN0 which is the drain device leading to the deadlock, so the mempool
really needs to be per device using slave_alloc.
Presently, Linux on Hyper-V can only boot from IDE devices. While IDE devices are
handled via this stor driver, the way we handle them is a little different compared
to block devices configured as scsi devices:

For  IDE devices; we have a HBA per LUN. Given that for a long time the drain device will
be an IDE device, my current implementation does address the concern that you had raised 
with regards to deadlock avoidance. 
quoted
+static int storvsc_device_alloc(struct scsi_device *sdevice)
+{
+	/*
+	 * This enables luns to be located sparsely. Otherwise, we may not
+	 * discovered them.
+	 */
+	sdevice->sdev_bflags |= BLIST_SPARSELUN | BLIST_LARGELUN;
+	return 0;
+}

Looks bogus ... this should happen automatically for SCSI-3 devices ...
unless your hypervisor has some strange (and wrong) identification?  I
really think you want to use SCSI-3 because it will do report LUN
scanning, which consumes far fewer resources.
Done.
quoted
I still think you need to disable clustering and junk the bvec merge
function.  Your object seems to be to accumulate in page size multiples
(and not aggregate over this) ... that's what clustering is designed to
do.
Done. James, I am going to send you (and the scsi mailing list) the patches addressing 
your comments. I will also send out a consolidated patch for getting the driver out of 
staging as well. I would like to thank you for your help in cleaning up this driver.

Regards,

K. Y
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help