| Message ID | 20170426144645.12476-5-farman@linux.vnet.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 04/26/2017 09:46 AM, Eric Farman wrote: > A virtio-scsi request that goes through the host sd driver and > exceeds the maximum transfer size is automatically broken up > for us. But the equivalent request going to the sg driver > presumes that any length requirements have already been honored. > Let's use the max_sectors field from the device and break up > all virtio-scsi requests (both sd and sg) to avoid problem from > the host drivers. > > Signed-off-by: Eric Farman <farman@linux.vnet.ibm.com> > --- > pc-bios/s390-ccw/s390-ccw.h | 4 ++++ > pc-bios/s390-ccw/virtio-scsi.c | 19 ++++++++++++++----- > 2 files changed, 18 insertions(+), 5 deletions(-) > > diff --git a/pc-bios/s390-ccw/s390-ccw.h b/pc-bios/s390-ccw/s390-ccw.h > index ded67bc..e1f3751 100644 > --- a/pc-bios/s390-ccw/s390-ccw.h > +++ b/pc-bios/s390-ccw/s390-ccw.h > @@ -42,6 +42,10 @@ typedef unsigned long long __u64; > #ifndef NULL > #define NULL 0 > #endif > +#ifndef MIN > +#define MIN(a, b) (((a) < (b)) ? (a) : (b)); > +#endif osdep.h defines MIN() for ALL files, so this hunk is spurious.
On 04/26/2017 11:17 AM, Eric Blake wrote: > On 04/26/2017 09:46 AM, Eric Farman wrote: >> A virtio-scsi request that goes through the host sd driver and >> exceeds the maximum transfer size is automatically broken up >> for us. But the equivalent request going to the sg driver >> presumes that any length requirements have already been honored. >> Let's use the max_sectors field from the device and break up >> all virtio-scsi requests (both sd and sg) to avoid problem from >> the host drivers. >> >> Signed-off-by: Eric Farman <farman@linux.vnet.ibm.com> >> --- >> pc-bios/s390-ccw/s390-ccw.h | 4 ++++ >> pc-bios/s390-ccw/virtio-scsi.c | 19 ++++++++++++++----- >> 2 files changed, 18 insertions(+), 5 deletions(-) >> >> diff --git a/pc-bios/s390-ccw/s390-ccw.h b/pc-bios/s390-ccw/s390-ccw.h >> index ded67bc..e1f3751 100644 >> --- a/pc-bios/s390-ccw/s390-ccw.h >> +++ b/pc-bios/s390-ccw/s390-ccw.h >> @@ -42,6 +42,10 @@ typedef unsigned long long __u64; >> #ifndef NULL >> #define NULL 0 >> #endif >> +#ifndef MIN >> +#define MIN(a, b) (((a) < (b)) ? (a) : (b)); >> +#endif > > osdep.h defines MIN() for ALL files, so this hunk is spurious. Ah, so it does. Okay, removed. Thanks! I guess it would make sense for the hunk that uses it to use MIN_NON_ZERO instead, which is also defined there. >
On Wed, 26 Apr 2017 10:17:36 -0500 Eric Blake <eblake@redhat.com> wrote: > On 04/26/2017 09:46 AM, Eric Farman wrote: > > A virtio-scsi request that goes through the host sd driver and > > exceeds the maximum transfer size is automatically broken up > > for us. But the equivalent request going to the sg driver > > presumes that any length requirements have already been honored. > > Let's use the max_sectors field from the device and break up > > all virtio-scsi requests (both sd and sg) to avoid problem from > > the host drivers. > > > > Signed-off-by: Eric Farman <farman@linux.vnet.ibm.com> > > --- > > pc-bios/s390-ccw/s390-ccw.h | 4 ++++ > > pc-bios/s390-ccw/virtio-scsi.c | 19 ++++++++++++++----- > > 2 files changed, 18 insertions(+), 5 deletions(-) > > > > diff --git a/pc-bios/s390-ccw/s390-ccw.h b/pc-bios/s390-ccw/s390-ccw.h > > index ded67bc..e1f3751 100644 > > --- a/pc-bios/s390-ccw/s390-ccw.h > > +++ b/pc-bios/s390-ccw/s390-ccw.h > > @@ -42,6 +42,10 @@ typedef unsigned long long __u64; > > #ifndef NULL > > #define NULL 0 > > #endif > > +#ifndef MIN > > +#define MIN(a, b) (((a) < (b)) ? (a) : (b)); > > +#endif > > osdep.h defines MIN() for ALL files, so this hunk is spurious. > This is a separate binary, which does not include the normal qemu headers, though.
On 04/26/2017 11:24 AM, Eric Farman wrote: > > > On 04/26/2017 11:17 AM, Eric Blake wrote: >> On 04/26/2017 09:46 AM, Eric Farman wrote: >>> A virtio-scsi request that goes through the host sd driver and >>> exceeds the maximum transfer size is automatically broken up >>> for us. But the equivalent request going to the sg driver >>> presumes that any length requirements have already been honored. >>> Let's use the max_sectors field from the device and break up >>> all virtio-scsi requests (both sd and sg) to avoid problem from >>> the host drivers. >>> >>> Signed-off-by: Eric Farman <farman@linux.vnet.ibm.com> >>> --- >>> pc-bios/s390-ccw/s390-ccw.h | 4 ++++ >>> pc-bios/s390-ccw/virtio-scsi.c | 19 ++++++++++++++----- >>> 2 files changed, 18 insertions(+), 5 deletions(-) >>> >>> diff --git a/pc-bios/s390-ccw/s390-ccw.h b/pc-bios/s390-ccw/s390-ccw.h >>> index ded67bc..e1f3751 100644 >>> --- a/pc-bios/s390-ccw/s390-ccw.h >>> +++ b/pc-bios/s390-ccw/s390-ccw.h >>> @@ -42,6 +42,10 @@ typedef unsigned long long __u64; >>> #ifndef NULL >>> #define NULL 0 >>> #endif >>> +#ifndef MIN >>> +#define MIN(a, b) (((a) < (b)) ? (a) : (b)); >>> +#endif >> >> osdep.h defines MIN() for ALL files, so this hunk is spurious. > > Ah, so it does. Okay, removed. Thanks! Er, not. Changing this header file didn't force virtio-scsi.c to recompile, so it bombs out as Cornelia points out. > > I guess it would make sense for the hunk that uses it to use > MIN_NON_ZERO instead, which is also defined there. > >> > >
diff --git a/pc-bios/s390-ccw/s390-ccw.h b/pc-bios/s390-ccw/s390-ccw.h index ded67bc..e1f3751 100644 --- a/pc-bios/s390-ccw/s390-ccw.h +++ b/pc-bios/s390-ccw/s390-ccw.h @@ -42,6 +42,10 @@ typedef unsigned long long __u64; #ifndef NULL #define NULL 0 #endif +#ifndef MIN +#define MIN(a, b) (((a) < (b)) ? (a) : (b)); +#endif + #include "cio.h" #include "iplb.h" diff --git a/pc-bios/s390-ccw/virtio-scsi.c b/pc-bios/s390-ccw/virtio-scsi.c index 6d070e2..90a8cc8 100644 --- a/pc-bios/s390-ccw/virtio-scsi.c +++ b/pc-bios/s390-ccw/virtio-scsi.c @@ -254,12 +254,21 @@ static void virtio_scsi_locate_device(VDev *vdev) int virtio_scsi_read_many(VDev *vdev, ulong sector, void *load_addr, int sec_num) { + int sector_count; int f = vdev->blk_factor; - unsigned int data_size = sec_num * virtio_get_block_size() * f; - - if (!scsi_read_10(vdev, sector * f, sec_num * f, load_addr, data_size)) { - virtio_scsi_verify_response(&resp, "virtio-scsi:read_many"); - } + unsigned int data_size; + + do { + sector_count = MIN(sec_num, vdev->config.scsi.max_sectors); + data_size = sector_count * virtio_get_block_size() * f; + if (!scsi_read_10(vdev, sector * f, sector_count * f, load_addr, + data_size)) { + virtio_scsi_verify_response(&resp, "virtio-scsi:read_many"); + } + load_addr += data_size; + sector += sector_count; + sec_num -= sector_count; + } while (sec_num > 0); return 0; }
A virtio-scsi request that goes through the host sd driver and exceeds the maximum transfer size is automatically broken up for us. But the equivalent request going to the sg driver presumes that any length requirements have already been honored. Let's use the max_sectors field from the device and break up all virtio-scsi requests (both sd and sg) to avoid problem from the host drivers. Signed-off-by: Eric Farman <farman@linux.vnet.ibm.com> --- pc-bios/s390-ccw/s390-ccw.h | 4 ++++ pc-bios/s390-ccw/virtio-scsi.c | 19 ++++++++++++++----- 2 files changed, 18 insertions(+), 5 deletions(-)