> -----Original Message----- > From: James Bottomley [mailto:James.Bottomley@xxxxxxxxxxxxxxxxxxxxx] > Sent: Monday, October 17, 2011 10:00 AM > To: KY Srinivasan > Cc: gregkh@xxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; > devel@xxxxxxxxxxxxxxxxxxxxxx; virtualization@xxxxxxxxxxxxxx; ohering@xxxxxxxx; > linux-scsi@xxxxxxxxxxxxxxx; hch@xxxxxxxxxxxxx; Haiyang Zhang > Subject: RE: [PATCH 1/1] Staging: hv: storvsc: Move the storage driver out of > staging > > On Sun, 2011-10-16 at 03:01 +0000, KY Srinivasan wrote: > > > > > -----Original Message----- > > > From: James Bottomley [mailto:James.Bottomley@xxxxxxxxxxxxxxxxxxxxx] > > > Sent: Saturday, October 15, 2011 5:27 PM > > > To: KY Srinivasan > > > Cc: gregkh@xxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; > > > devel@xxxxxxxxxxxxxxxxxxxxxx; virtualization@xxxxxxxxxxxxxx; > ohering@xxxxxxxx; > > > linux-scsi@xxxxxxxxxxxxxxx; hch@xxxxxxxxxxxxx; Haiyang Zhang > > > Subject: Re: [PATCH 1/1] Staging: hv: storvsc: Move the storage driver out of > > > staging > > > > > > On Fri, 2011-10-14 at 23:28 -0700, K. Y. Srinivasan wrote: > > > > In preparation for moving the storage driver out of staging, seek community > > > > review of the storage driver code. > > > > > > That's not exactly a very descriptive commit message for me to put into > > > SCSI. It doesn't have to be huge, just something like "driver to enable > > > hyperv windows guest on Linux" or something. > > > > Sorry about the commit message. I will have a more descriptive message in the > next > > submission. > > > > > > > > > > > > drivers/scsi/Kconfig | 7 + > > > > drivers/scsi/Makefile | 3 + > > > > drivers/scsi/storvsc_drv.c | 1480 > > > ++++++++++++++++++++++++++++++++++++++ > > > > drivers/staging/hv/Kconfig | 6 - > > > > drivers/staging/hv/Makefile | 2 - > > > > drivers/staging/hv/storvsc_drv.c | 1480 -------------------------------------- > > > > 6 files changed, 1490 insertions(+), 1488 deletions(-) > > > > > > What tree is this against? The hv/storvsc_drv.c in upstream only has > > > > > > jejb@dabdike> wc -l drivers/staging/hv/storvsc_drv.c > > > 792 drivers/staging/hv/storvsc_drv.c > > > > > > i.e. whatever you're sending is double the length (and obviously I have > > > trouble applying the patch. > > > > This patch moves the file from drivers/staging/hv/ directory to the > > drivers/scsi directory; hence double the length. > > No, that's not it. Look again: the storvsc_drv.c in staging is removing > 1480 lines, but in git head, this file is only 792 lines long ... is > there an alternative tree with the rest in? This patch is against Greg's staging tree where recently I merged all of the storage drivers into a single driver. This change may not have percolated up yet - Greg has checked this into his staging tree though. As I deal with the review comments now, do you want me to get these changes applied first to Greg's tree before you would consider moving this driver to drivers/scsi directory, or could you move the driver as it is in Greg's tree under staging and I could give you patches against the moved driver. > > The point I'm making is that the staging file you're modifying isn't the > one I see in Linus' git head, so which git tree is it in (I assume it's > somewhere waiting for the merge window)? > > [...] > > > The bounce buffer handling for discontiguous sg lists is a complete > > > nightmare. And can't you just fix the hypervisor instead of pulling > > > this level of nastiness in the driver, I mean really, the last piece of > > > real SCSI hardware that failed this badly was produced in the dark ages. > > > > The restrictions (as they are) are really from the windows host side > > and for what it is > > worth, I have never seen this code triggered at least for I/O > > initiated by the file system. > > > > > > > Assuming the answer is "no", I really think you need to set the minimum > > > block size to 4k, which will completely avoid the situation. > > > > I would love to get rid of the bounce buffer handling code by ensuring > > that the > > upper level code would never present scatter gather lists that would > > trigger bounce buffer > > handling code. How would I accomplish that. > > OK, so as I read the code, what it can't handle is fragments except at > the ends. As long as everything always transfers in multiples of 4k, > the problem will never occur. This means that all devices you present > need to tell the OS they have 4k sectors. I *think* you can do this by > setting blk_limits_io_min() in the driver ... but you'll have to test > this. The minimum sector size gets set also in sd.c when we probe the > disk. I think that won't override blk_limits_io_min(), but please test > (we don't have any SCSI drivers which have ever used this setting). > > The way to test, is to read back the /sys/block/<dev>/queue/ settings > when presenting a 512 byte sector disk. (And the way to activate your > bounce code would be to create a filesystem with a < PAGE_SIZE block > size). Given that the bounce buffer handling code would typically never be triggered, I not sure how useful it will be to try to get rid of the bounce buffer code using a feature that is not well tested. In any event, I will try your suggestion though. Regards, K. Y ��.n��������+%������w��{.n�����{������ܨ}���Ơz�j:+v�����w����ޙ��&�)ߡ�a����z�ޗ���ݢj��w�f
