RE: [PATCH 6/7] scsi: storvsc: Don't assume that the scatterlist is not chained

2015-03-24 Thread KY Srinivasan


 -Original Message-
 From: Olaf Hering [mailto:o...@aepfle.de]
 Sent: Tuesday, March 24, 2015 1:56 AM
 To: KY Srinivasan
 Cc: gre...@linuxfoundation.org; linux-ker...@vger.kernel.org;
 de...@linuxdriverproject.org; jbottom...@parallels.com;
 h...@infradead.org; linux-scsi@vger.kernel.org; a...@canonical.com;
 vkuzn...@redhat.com; jasow...@redhat.com
 Subject: Re: [PATCH 6/7] scsi: storvsc: Don't assume that the scatterlist is 
 not
 chained
 
 On Mon, Mar 23, K. Y. Srinivasan wrote:
 
  @@ -653,32 +640,39 @@ static unsigned int
 copy_from_bounce_buffer(struct scatterlist *orig_sgl,
  unsigned long bounce_addr = 0;
  unsigned long dest_addr = 0;
  unsigned long flags;
  +   struct scatterlist *cur_dest_sgl;
  +   struct scatterlist *cur_src_sgl;
 
  local_irq_save(flags);
  -
  +   cur_dest_sgl = orig_sgl;
  +   cur_src_sgl = bounce_sgl;
  for (i = 0; i  orig_sgl_count; i++) {
  -   dest_addr = sg_kmap_atomic(orig_sgl,i) + orig_sgl[i].offset;
  +   dest_addr = (unsigned long)
  +   kmap_atomic(sg_page(cur_dest_sgl)) +
  +   cur_dest_sgl-offset;
  dest = dest_addr;
  destlen = orig_sgl[i].length;
  +   destlen = cur_dest_sgl-length;
 
 Double assignment.

Thanks Olaf; I will fix this and resubmit.

K. Y

 
 Olaf


Re: [PATCH 6/7] scsi: storvsc: Don't assume that the scatterlist is not chained

2015-03-24 Thread Olaf Hering
On Mon, Mar 23, K. Y. Srinivasan wrote:

 @@ -653,32 +640,39 @@ static unsigned int copy_from_bounce_buffer(struct 
 scatterlist *orig_sgl,
   unsigned long bounce_addr = 0;
   unsigned long dest_addr = 0;
   unsigned long flags;
 + struct scatterlist *cur_dest_sgl;
 + struct scatterlist *cur_src_sgl;
  
   local_irq_save(flags);
 -
 + cur_dest_sgl = orig_sgl;
 + cur_src_sgl = bounce_sgl;
   for (i = 0; i  orig_sgl_count; i++) {
 - dest_addr = sg_kmap_atomic(orig_sgl,i) + orig_sgl[i].offset;
 + dest_addr = (unsigned long)
 + kmap_atomic(sg_page(cur_dest_sgl)) +
 + cur_dest_sgl-offset;
   dest = dest_addr;
   destlen = orig_sgl[i].length;
 + destlen = cur_dest_sgl-length;

Double assignment.

Olaf
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [PATCH 6/7] scsi: storvsc: Don't assume that the scatterlist is not chained

2015-03-23 Thread Long Li


 -Original Message-
 From: devel [mailto:driverdev-devel-boun...@linuxdriverproject.org] On
 Behalf Of K. Y. Srinivasan
 Sent: Monday, March 23, 2015 2:07 PM
 To: gre...@linuxfoundation.org; linux-ker...@vger.kernel.org;
 de...@linuxdriverproject.org; oher...@suse.com; jbottom...@parallels.com;
 h...@infradead.org; linux-scsi@vger.kernel.org; a...@canonical.com;
 vkuzn...@redhat.com; jasow...@redhat.com
 Subject: [PATCH 6/7] scsi: storvsc: Don't assume that the scatterlist is not
 chained
 
 The current code assumes that the scatterlists presented are not chained.
 Fix the code to not make this assumption.
 
 Signed-off-by: K. Y. Srinivasan k...@microsoft.com
Reviewed-by: Long Li lon...@microsoft.com

 ---
  drivers/scsi/storvsc_drv.c |   98 +--
  1 files changed, 57 insertions(+), 41 deletions(-)
 
 diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c index
 88f5d79..a599677 100644
 --- a/drivers/scsi/storvsc_drv.c
 +++ b/drivers/scsi/storvsc_drv.c
 @@ -626,19 +626,6 @@ cleanup:
   return NULL;
  }
 
 -/* Disgusting wrapper functions */
 -static inline unsigned long sg_kmap_atomic(struct scatterlist *sgl, int idx) 
 -{
 - void *addr = kmap_atomic(sg_page(sgl + idx));
 - return (unsigned long)addr;
 -}
 -
 -static inline void sg_kunmap_atomic(unsigned long addr) -{
 - kunmap_atomic((void *)addr);
 -}
 -
 -
  /* Assume the original sgl has enough room */  static unsigned int
 copy_from_bounce_buffer(struct scatterlist *orig_sgl,
   struct scatterlist *bounce_sgl, @@ -
 653,32 +640,39 @@ static unsigned int copy_from_bounce_buffer(struct
 scatterlist *orig_sgl,
   unsigned long bounce_addr = 0;
   unsigned long dest_addr = 0;
   unsigned long flags;
 + struct scatterlist *cur_dest_sgl;
 + struct scatterlist *cur_src_sgl;
 
   local_irq_save(flags);
 -
 + cur_dest_sgl = orig_sgl;
 + cur_src_sgl = bounce_sgl;
   for (i = 0; i  orig_sgl_count; i++) {
 - dest_addr = sg_kmap_atomic(orig_sgl,i) + orig_sgl[i].offset;
 + dest_addr = (unsigned long)
 + kmap_atomic(sg_page(cur_dest_sgl)) +
 + cur_dest_sgl-offset;
   dest = dest_addr;
   destlen = orig_sgl[i].length;
 + destlen = cur_dest_sgl-length;
 
   if (bounce_addr == 0)
 - bounce_addr = sg_kmap_atomic(bounce_sgl,j);
 + bounce_addr = (unsigned long)kmap_atomic(
 + sg_page(cur_src_sgl));
 
   while (destlen) {
 - src = bounce_addr + bounce_sgl[j].offset;
 - srclen = bounce_sgl[j].length - bounce_sgl[j].offset;
 + src = bounce_addr + cur_src_sgl-offset;
 + srclen = cur_src_sgl-length - cur_src_sgl-offset;
 
   copylen = min(srclen, destlen);
   memcpy((void *)dest, (void *)src, copylen);
 
   total_copied += copylen;
 - bounce_sgl[j].offset += copylen;
 + cur_src_sgl-offset += copylen;
   destlen -= copylen;
   dest += copylen;
 
 - if (bounce_sgl[j].offset == bounce_sgl[j].length) {
 + if (cur_src_sgl-offset == cur_src_sgl-length) {
   /* full */
 - sg_kunmap_atomic(bounce_addr);
 + kunmap_atomic((void *)bounce_addr);
   j++;
 
   /*
 @@ -692,21 +686,27 @@ static unsigned int copy_from_bounce_buffer(struct
 scatterlist *orig_sgl,
   /*
* We are done; cleanup and return.
*/
 - sg_kunmap_atomic(dest_addr -
 orig_sgl[i].offset);
 + kunmap_atomic((void *)(dest_addr -
 + cur_dest_sgl-offset));
   local_irq_restore(flags);
   return total_copied;
   }
 
   /* if we need to use another bounce buffer */
 - if (destlen || i != orig_sgl_count - 1)
 - bounce_addr =
 sg_kmap_atomic(bounce_sgl,j);
 + if (destlen || i != orig_sgl_count - 1) {
 + cur_src_sgl = sg_next(cur_src_sgl);
 + bounce_addr = (unsigned long)
 + kmap_atomic(
 + sg_page(cur_src_sgl));
 + }