RE: [PATCH 1/1] Staging: hv: storvsc: Move the storage driver outof staging

[KY Srinivasan]

> -----Original Message-----
> From: James Bottomley [mailto:James.Bottomley@xxxxxxxxxxxxxxxxxxxxx]
> Sent: Monday, October 17, 2011 11:27 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 Mon, 2011-10-17 at 15:15 +0000, KY Srinivasan wrote:
> 
> > 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.
> 
> Either way is fine by me.  The best route for this is to get the staging
> update into Linus head, then work on the actual SCSI problems.  I'd like
> other SCSI people besides me to review it.

Christoph had reviewed this code sometime back and I think I have addressed the
issues he had raised. These patches have already been applied by Greg to his
staging tree.

> 
> > > 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).
> 
> >  In any event, I will try your suggestion though.
> 
> Just to be clear: Setting the minimum I/O size is completely well
> tested: it's how we handle 4k sector drives.  What hasn't been tested is
> the ability of the *driver* to enforce the minimum using
> blk_limits_io_min().  As long at that doesn't get overridden by sd when
> it sees a 512b drive, the rest of the block paths (those which actually
> enforce the minimum) have all been well tested.
>
 
James, I used the blk_limits_io_min() to 4k and yet I see I/O requests coming in
to the driver that need to be handled with the bounce buffers. So I am not quite
sure how to avoid having the bounce buffer handling code. This is with a file system
with 2k block size.

Other than this, I have addressed all of your other comments. I am going to send these
patches out to the staging tree. Do you want me to send out a patch moving the driver
out of staging for your review as well. Let me know.

Regards,

K. Y 
   

¢éì®&Þ~º&¶¬–+-±éÝ¥Šw®žË±Êâmébžìdz¹Þ)í…æèw*jg¬±¨¶‰šŽŠÝj/êäz¹ÞŠà2ŠÞ¨è­Ú&¢)ß«a¶Úþø®G«éh®æj:+v‰¨Šwè†Ù>Wš±êÞiÛaxPjØm¶ŸÿÃ-»+ƒùdš_