Re: uas: kernel BUG at block/blk-tag.c:89 is back again :|

2014-07-03 Thread Christoph Hellwig
Hi Hans,

please test the path below:

---
From: Christoph Hellwig h...@lst.de
Subject: block: don't assume last put of shared tags is for the host

There is no inherent reason why the last put of a tag structure must be
the one for the Scsi_Host, as device model objects can be held for
arbitrary periods.  Merge blk_free_tags and __blk_free_tags into a single
funtion that just release a references and get rid of the BUG() when the
host reference wasn't the last.

Signed-off-by: Christoph Hellwig h...@lst.de

diff --git a/block/blk-tag.c b/block/blk-tag.c
index 3f33d86..a185b86 100644
--- a/block/blk-tag.c
+++ b/block/blk-tag.c
@@ -27,18 +27,15 @@ struct request *blk_queue_find_tag(struct request_queue *q, 
int tag)
 EXPORT_SYMBOL(blk_queue_find_tag);
 
 /**
- * __blk_free_tags - release a given set of tag maintenance info
+ * blk_free_tags - release a given set of tag maintenance info
  * @bqt:   the tag map to free
  *
- * Tries to free the specified @bqt.  Returns true if it was
- * actually freed and false if there are still references using it
+ * Drop the reference count on @bqt and frees it when the last reference
+ * is dropped.
  */
-static int __blk_free_tags(struct blk_queue_tag *bqt)
+void blk_free_tags(struct blk_queue_tag *bqt)
 {
-   int retval;
-
-   retval = atomic_dec_and_test(bqt-refcnt);
-   if (retval) {
+   if (atomic_dec_and_test(bqt-refcnt)) {
BUG_ON(find_first_bit(bqt-tag_map, bqt-max_depth) 
bqt-max_depth);
 
@@ -50,9 +47,8 @@ static int __blk_free_tags(struct blk_queue_tag *bqt)
 
kfree(bqt);
}
-
-   return retval;
 }
+EXPORT_SYMBOL(blk_free_tags);
 
 /**
  * __blk_queue_free_tags - release tag maintenance info
@@ -69,28 +65,13 @@ void __blk_queue_free_tags(struct request_queue *q)
if (!bqt)
return;
 
-   __blk_free_tags(bqt);
+   blk_free_tags(bqt);
 
q-queue_tags = NULL;
queue_flag_clear_unlocked(QUEUE_FLAG_QUEUED, q);
 }
 
 /**
- * blk_free_tags - release a given set of tag maintenance info
- * @bqt:   the tag map to free
- *
- * For externally managed @bqt frees the map.  Callers of this
- * function must guarantee to have released all the queues that
- * might have been using this tag map.
- */
-void blk_free_tags(struct blk_queue_tag *bqt)
-{
-   if (unlikely(!__blk_free_tags(bqt)))
-   BUG();
-}
-EXPORT_SYMBOL(blk_free_tags);
-
-/**
  * blk_queue_free_tags - release tag maintenance info
  * @q:  the request queue for the device
  *
--
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 3.15 099/139] libiscsi, iser: Adjust data_length to include protection information

2014-07-03 Thread Christoph Hellwig
On Wed, Jul 02, 2014 at 04:48:37PM -0600, Alex Williamson wrote:
 Thanks for the pointer.  It's curious why there was no objection to this
 going into stable if it's been known to be broken for a week.

I've never seen the stable patch on linux-scsi.  IMHO it should never
have been proposed for -stable with all the issues it caused.

--
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 3.15 099/139] libiscsi, iser: Adjust data_length to include protection information

2014-07-03 Thread Christoph Hellwig
On Wed, Jul 02, 2014 at 03:52:35PM -0700, James Bottomley wrote:
 In which case it's not marked for stable backport ... I assume it should
 be?

Only if the patch that regression went to stable, which it shouldn't,

--
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: Crash on WRITE SAME

2014-07-03 Thread Sagi Grimberg

On 7/2/2014 9:36 PM, Christoph Hellwig wrote:

On Wed, Jul 02, 2014 at 02:29:05PM -0400, Mikulas Patocka wrote:

And what about protection information for commands that transfer partial
sectors? (for example, UMAP transfers 24 bytes). Should
scsi_transfer_length return 24 or 32 in this case?

As far as I understand so far PI is only defined for READ/WRITE
commands.  But I'll defer to Martin who is a much better source for
information on this topic.


hch is correct, PI supported operations are (stated in SBC)

a) COMPARE AND WRITE;
b) ORWRITE (16);
c) ORWRITE (32);
d) READ (10);
e) READ (12);
f) READ (16);
g) READ (32);
h) VERIFY (10);
i) VERIFY (12);
j) VERIFY (16);
k) VERIFY (32);
l) WRITE (10);
m) WRITE (12);
n) WRITE (16);
o) WRITE (32);
p) WRITE AND VERIFY (10);
q) WRITE AND VERIFY (12);
r) WRITE AND VERIFY (16);
s) WRITE AND VERIFY (32);
t) WRITE SAME (10);
u) WRITE SAME (16);
v) WRITE SAME (32);
y) XDWRITEREAD (10);
z) XDWRITEREAD (32);
aa) XPWRITE (10); and
ab) XPWRITE (32).

Sagi.
--
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 3/4] megaraid_sas: Fix LD/VF affiliation parsing

2014-07-03 Thread Christoph Hellwig
On Mon, Jun 30, 2014 at 05:19:57PM -0700, Adam Radford wrote:
 James/linux-scsi,
 
 The following patch for megaraid_sas fixes the LD/VF affiliation policy 
 parsing code to account for LD targetId's and Hidden LD's (not yet affiliated 
 with any Virtual Functions).

Hi Adam,

this doesn't address Martins comment about removing the initial != 0
case.  And while we're at it there basically is no significant shared
code between the PlasmaFW111 and !PlasmaFW111, so split these into
two different functions.  With those two levels of indentation removed
the code might actually become readable, too.

Also please remove the James/linux-scsi, address in the individual
patches, that's something that should just go into the cover letter,
and make sure to include all Reviewed-by: and Acked-by: tags when
resending already reviewd patches.

Thanks,
Christoph
--
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] [SCSI] qla2xxx: Remove duplicate __iomem annotation

2014-07-03 Thread Christoph Hellwig
Looks good,

Reviewed-by: Christoph Hellwig h...@lst.de
--
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 V2] scsi_lib: removes ambiguous Unhandled error code messages.

2014-07-03 Thread Christoph Hellwig
On Wed, Jun 25, 2014 at 08:38:47PM +, Elliott, Robert (Server Storage) 
wrote:
 Since the ACTION_FAIL case always prints the sense key
 and additional sense code:

 perhaps the description string should be removed altogether?

 For the Unhandled error code (for which you are proposing
 removing the string) and the timeout case, the scsi_print_result 
 call already prints hostbyte and driverbyte, which explain what 
 happened in more detail:

I'm tempted to agree and just remove the description.  Do you want to
send a patch for this?
--
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 1/4] pm8001: clean bitmap management functions

2014-07-03 Thread Christoph Hellwig
Suresh,

do you maintain pm8001 for pmcs now?  There have been lots of pm8001
patches on the list, many of them with reviews from various people
including Jack.  I'd be really greatful if someone could collect them
and resend them as a single coherent series.

Also it the MAINTAINERS information for pm8001 isn't uptodate anymore
please update it.

Thanks,
Christoph
--
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] MAINTAINERS: Remove two ancient EATA sections

2014-07-03 Thread Christoph Hellwig
On Sat, Jun 28, 2014 at 09:54:02AM -0700, Joe Perches wrote:
 These haven't had a single ack by the listed maintainer
 in all git history and the email addresses don't work.
 
 And entry for Michael Neuffer is already in CREDITS.

Looks fine to me..

--
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] [SCSI] hpsa: fix non-x86 builds

2014-07-03 Thread Christoph Hellwig
On Thu, Jun 26, 2014 at 09:34:45AM -0500, scame...@beardog.cce.hp.com wrote:
 Thanks.

Do you plan to include this with the next hpsa update, or should I take
this as an ACK and apply it?

--
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 01/14] block: Get rid of bdev_integrity_enabled()

2014-07-03 Thread Sagi Grimberg

On 6/11/2014 7:31 PM, Christoph Hellwig wrote:

On Wed, May 28, 2014 at 11:28:35PM -0400, Martin K. Petersen wrote:

bdev_integrity_enabled() is only used by bio_integrity_enabled().
Combine these two functions.

Signed-off-by: Martin K. Petersen martin.peter...@oracle.com

Looks good,

Reviewed-by: Christoph Hellwig h...@lst.de


Reviewed-by: Sagi Grimberg sa...@mellanox.com

--
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 02/14] block: Replace bi_integrity with bi_special

2014-07-03 Thread Sagi Grimberg

On 6/12/2014 3:18 AM, Martin K. Petersen wrote:

Christoph == Christoph Hellwig h...@infradead.org writes:

Christoph Instead of having a union of pointer just make it a void
Christoph pointer. I also think special is a terribly generic name, but
Christoph I don't really have a better idea at hand.

I needed something that could encompass additional information to be
passed for integrity, copy offload and discard requests.

Another option is that we forgo the union name:

 union {
#if defined(CONFIG_BLK_DEV_INTEGRITY)
 struct bio_integrity_payload *bi_integrity;
#endif
 struct bio_copy *bi_copy;
 };

That's the way Jens has done it in struct request. I think I like that
better and it doesn't send the same up-for-grabs signal that a void
pointer might.

Jens: Any preference?



A nameless union makes more sense to me here.

Sagi.
--
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 05/14] block: Deprecate the use of the term sector in the context of block integrity

2014-07-03 Thread Sagi Grimberg

On 5/29/2014 6:28 AM, Martin K. Petersen wrote:

The protection interval is not necessarily tied to the logical block
size of a block device. Stop using the terms sector and sectors.

Signed-off-by: Martin K. Petersen martin.peter...@oracle.com
---
  block/bio-integrity.c  | 46 +-
  block/blk-integrity.c  | 10 +-
  drivers/scsi/sd_dif.c  | 46 +++---
  include/linux/blkdev.h |  6 +++---
  4 files changed, 52 insertions(+), 56 deletions(-)

diff --git a/block/bio-integrity.c b/block/bio-integrity.c
index e06b3c807eef..c52a8fd98706 100644
--- a/block/bio-integrity.c
+++ b/block/bio-integrity.c
@@ -191,29 +191,25 @@ bool bio_integrity_enabled(struct bio *bio)
  EXPORT_SYMBOL(bio_integrity_enabled);
  
  /**

- * bio_integrity_hw_sectors - Convert 512b sectors to hardware ditto
+ * bio_integrity_intervals - Return number of integrity intervals for a bio
   * @bi:   blk_integrity profile for device
- * @sectors:   Number of 512 sectors to convert
+ * @sectors:   Size of the bio in 512-byte sectors
   *
   * Description: The block layer calculates everything in 512 byte
- * sectors but integrity metadata is done in terms of the hardware
- * sector size of the storage device.  Convert the block layer sectors
- * to physical sectors.
+ * sectors but integrity metadata is done in terms of the data integrity
+ * interval size of the storage device.  Convert the block layer sectors
+ * to the appropriate number of integrity intervals.
   */
-static inline unsigned int bio_integrity_hw_sectors(struct blk_integrity *bi,
-   unsigned int sectors)
+static inline unsigned int bio_integrity_intervals(struct blk_integrity *bi,
+  unsigned int sectors)
  {
-   /* At this point there are only 512b or 4096b DIF/EPP devices */
-   if (bi-sector_size == 4096)
-   return sectors = 3;
-
-   return sectors;
+   return sectors  (ilog2(bi-interval) - 9);
  }


Now that protection information interval does not necessarily match the 
sector_size, should this routine
protect against bogus bi-interval (e.g. fail if bi-interval  
sector_size for example)? Not sure
if this check is really needed here, but it might be useful to have 
(although protection interval

is still effectively sector_size).

  
  static inline unsigned int bio_integrity_bytes(struct blk_integrity *bi,

   unsigned int sectors)
  {
-   return bio_integrity_hw_sectors(bi, sectors) * bi-tuple_size;
+   return bio_integrity_intervals(bi, sectors) * bi-tuple_size;
  }
  
  /**

@@ -227,25 +223,25 @@ static int bio_integrity_generate_verify(struct bio *bio, 
int operate)
struct blk_integrity_exchg bix;
struct bio_vec *bv;
struct bio_integrity_payload *bip = bio_integrity(bio);
-   sector_t sector;
-   unsigned int sectors, ret = 0, i;
+   sector_t seed;
+   unsigned int intervals, ret = 0, i;
void *prot_buf = page_address(bip-bip_vec-bv_page) +
bip-bip_vec-bv_offset;
  
  	if (operate)

-   sector = bio-bi_iter.bi_sector;
+   seed = bio-bi_iter.bi_sector;
else
-   sector = bip-bip_iter.bi_sector;
+   seed = bip-bip_iter.bi_sector;
  
  	bix.disk_name = bio-bi_bdev-bd_disk-disk_name;

-   bix.sector_size = bi-sector_size;
+   bix.interval = bi-interval;
  
  	bio_for_each_segment_all(bv, bio, i) {

void *kaddr = kmap_atomic(bv-bv_page);
bix.data_buf = kaddr + bv-bv_offset;
bix.data_size = bv-bv_len;
bix.prot_buf = prot_buf;
-   bix.sector = sector;
+   bix.seed = seed;
  
  		if (operate)

bi-generate_fn(bix);
@@ -257,9 +253,9 @@ static int bio_integrity_generate_verify(struct bio *bio, 
int operate)
}
}
  
-		sectors = bv-bv_len / bi-sector_size;

-   sector += sectors;
-   prot_buf += sectors * bi-tuple_size;
+   intervals = bv-bv_len / bi-interval;
+   seed += intervals;
+   prot_buf += intervals * bi-tuple_size;
  
  		kunmap_atomic(kaddr);

}
@@ -300,17 +296,17 @@ int bio_integrity_prep(struct bio *bio)
unsigned long start, end;
unsigned int len, nr_pages;
unsigned int bytes, offset, i;
-   unsigned int sectors;
+   unsigned int intervals;
  
  	bi = bdev_get_integrity(bio-bi_bdev);

q = bdev_get_queue(bio-bi_bdev);
BUG_ON(bi == NULL);
BUG_ON(bio_integrity(bio));
  
-	sectors = bio_integrity_hw_sectors(bi, bio_sectors(bio));

+   intervals = bio_integrity_intervals(bi, bio_sectors(bio));
  
  	/* Allocate kernel buffer for protection data */

-   len = sectors * bi-tuple_size;
+   len 

Re: [PATCH 06/14] block: Clean up the code used to generate and verify integrity metadata

2014-07-03 Thread Sagi Grimberg

On 5/29/2014 6:28 AM, Martin K. Petersen wrote:

Instead of the operate parameter we pass in a seed value and a pointer
to a function that can be used to process the integrity metadata. The
generation function is changed to have a return value to fit into this
scheme.

Signed-off-by: Martin K. Petersen martin.peter...@oracle.com
---
  block/bio-integrity.c  |  82 ++
  drivers/scsi/sd_dif.c  | 106 ++---
  include/linux/bio.h|  12 ++
  include/linux/blkdev.h |   9 ++---
  4 files changed, 94 insertions(+), 115 deletions(-)

diff --git a/block/bio-integrity.c b/block/bio-integrity.c
index c52a8fd98706..e711b9c71767 100644
--- a/block/bio-integrity.c
+++ b/block/bio-integrity.c
@@ -213,49 +213,37 @@ static inline unsigned int bio_integrity_bytes(struct 
blk_integrity *bi,
  }
  
  /**

- * bio_integrity_generate_verify - Generate/verify integrity metadata for a bio
+ * bio_integrity_process - Process integrity metadata for a bio
   * @bio:  bio to generate/verify integrity metadata for
- * @operate:   operate number, 1 for generate, 0 for verify
+ * @proc_fn:   Pointer to the relevant processing function
   */
-static int bio_integrity_generate_verify(struct bio *bio, int operate)
+static int bio_integrity_process(struct bio *bio,
+integrity_processing_fn *proc_fn)
  {
struct blk_integrity *bi = bdev_get_integrity(bio-bi_bdev);
-   struct blk_integrity_exchg bix;
+   struct blk_integrity_iter iter;
struct bio_vec *bv;
struct bio_integrity_payload *bip = bio_integrity(bio);
-   sector_t seed;
-   unsigned int intervals, ret = 0, i;
+   unsigned int i, ret = 0;
void *prot_buf = page_address(bip-bip_vec-bv_page) +
bip-bip_vec-bv_offset;
  
-	if (operate)

-   seed = bio-bi_iter.bi_sector;
-   else
-   seed = bip-bip_iter.bi_sector;
-
-   bix.disk_name = bio-bi_bdev-bd_disk-disk_name;
-   bix.interval = bi-interval;
+   iter.disk_name = bio-bi_bdev-bd_disk-disk_name;
+   iter.interval = bi-interval;
+   iter.seed = bip_get_seed(bip);
+   iter.prot_buf = prot_buf;
  
  	bio_for_each_segment_all(bv, bio, i) {

void *kaddr = kmap_atomic(bv-bv_page);
-   bix.data_buf = kaddr + bv-bv_offset;
-   bix.data_size = bv-bv_len;
-   bix.prot_buf = prot_buf;
-   bix.seed = seed;
-
-   if (operate)
-   bi-generate_fn(bix);
-   else {
-   ret = bi-verify_fn(bix);
-   if (ret) {
-   kunmap_atomic(kaddr);
-   return ret;
-   }
-   }
  
-		intervals = bv-bv_len / bi-interval;

-   seed += intervals;
-   prot_buf += intervals * bi-tuple_size;
+   iter.data_buf = kaddr + bv-bv_offset;
+   iter.data_size = bv-bv_len;
+
+   ret = proc_fn(iter);
+   if (ret) {
+   kunmap_atomic(kaddr);
+   return ret;
+   }
  
  		kunmap_atomic(kaddr);

}
@@ -263,20 +251,6 @@ static int bio_integrity_generate_verify(struct bio *bio, 
int operate)
  }
  
  /**

- * bio_integrity_generate - Generate integrity metadata for a bio
- * @bio:   bio to generate integrity metadata for
- *
- * Description: Generates integrity metadata for a bio by calling the
- * block device's generation callback function.  The bio must have a
- * bip attached with enough room to accommodate the generated
- * integrity metadata.
- */
-static void bio_integrity_generate(struct bio *bio)
-{
-   bio_integrity_generate_verify(bio, 1);
-}
-
-/**
   * bio_integrity_prep - Prepare bio for integrity I/O
   * @bio:  bio to prepare
   *
@@ -327,7 +301,7 @@ int bio_integrity_prep(struct bio *bio)
  
  	bip-bip_owns_buf = 1;

bip-bip_iter.bi_size = len;
-   bip-bip_iter.bi_sector = bio-bi_iter.bi_sector;
+   bip_set_seed(bip, bio-bi_iter.bi_sector);
  
  	/* Map it */

offset = offset_in_page(buf);
@@ -363,26 +337,13 @@ int bio_integrity_prep(struct bio *bio)
  
  	/* Auto-generate integrity metadata if this is a write */

if (bio_data_dir(bio) == WRITE)
-   bio_integrity_generate(bio);
+   bio_integrity_process(bio, bi-generate_fn);
  
  	return 0;

  }
  EXPORT_SYMBOL(bio_integrity_prep);
  
  /**

- * bio_integrity_verify - Verify integrity metadata for a bio
- * @bio:   bio to verify
- *
- * Description: This function is called to verify the integrity of a
- * bio. The data in the bio io_vec is compared to the integrity
- * metadata returned by the HBA.
- */
-static int bio_integrity_verify(struct bio *bio)
-{
-   return bio_integrity_generate_verify(bio, 0);
-}
-
-/**
   * bio_integrity_verify_fn - 

Re: [PATCH 07/14] block: Add prefix to block integrity profile flags

2014-07-03 Thread Sagi Grimberg

On 5/29/2014 6:28 AM, Martin K. Petersen wrote:

Add a BLK_ prefix to the integrity profile flags. Also rename the flags
to be more consistent with the generate/verify terminology in the rest
of the integrity code.

Signed-off-by: Martin K. Petersen martin.peter...@oracle.com
---
  block/bio-integrity.c  |  4 ++--
  block/blk-integrity.c  | 43 ++-
  include/linux/blkdev.h |  6 --
  3 files changed, 28 insertions(+), 25 deletions(-)

diff --git a/block/bio-integrity.c b/block/bio-integrity.c
index e711b9c71767..c91181e3d18d 100644
--- a/block/bio-integrity.c
+++ b/block/bio-integrity.c
@@ -179,11 +179,11 @@ bool bio_integrity_enabled(struct bio *bio)
return false;
  
  	if (bio_data_dir(bio) == READ  bi-verify_fn != NULL 

-   (bi-flags  INTEGRITY_FLAG_READ))
+   (bi-flags  BLK_INTEGRITY_VERIFY))
return true;
  
  	if (bio_data_dir(bio) == WRITE  bi-generate_fn != NULL 

-   (bi-flags  INTEGRITY_FLAG_WRITE))
+   (bi-flags  BLK_INTEGRITY_GENERATE))
return true;
  
  	return false;

diff --git a/block/blk-integrity.c b/block/blk-integrity.c
index 3760d0aeed92..95f451a3c581 100644
--- a/block/blk-integrity.c
+++ b/block/blk-integrity.c
@@ -269,42 +269,42 @@ static ssize_t integrity_tag_size_show(struct 
blk_integrity *bi, char *page)
return sprintf(page, 0\n);
  }
  
-static ssize_t integrity_read_store(struct blk_integrity *bi,

-   const char *page, size_t count)
+static ssize_t integrity_verify_store(struct blk_integrity *bi,
+ const char *page, size_t count)
  {
char *p = (char *) page;
unsigned long val = simple_strtoul(p, p, 10);
  
  	if (val)

-   bi-flags |= INTEGRITY_FLAG_READ;
+   bi-flags |= BLK_INTEGRITY_VERIFY;
else
-   bi-flags = ~INTEGRITY_FLAG_READ;
+   bi-flags = ~BLK_INTEGRITY_VERIFY;
  
  	return count;

  }
  
-static ssize_t integrity_read_show(struct blk_integrity *bi, char *page)

+static ssize_t integrity_verify_show(struct blk_integrity *bi, char *page)
  {
-   return sprintf(page, %d\n, (bi-flags  INTEGRITY_FLAG_READ) != 0);
+   return sprintf(page, %d\n, (bi-flags  BLK_INTEGRITY_VERIFY) != 0);
  }
  
-static ssize_t integrity_write_store(struct blk_integrity *bi,

-const char *page, size_t count)
+static ssize_t integrity_generate_store(struct blk_integrity *bi,
+   const char *page, size_t count)
  {
char *p = (char *) page;
unsigned long val = simple_strtoul(p, p, 10);
  
  	if (val)

-   bi-flags |= INTEGRITY_FLAG_WRITE;
+   bi-flags |= BLK_INTEGRITY_GENERATE;
else
-   bi-flags = ~INTEGRITY_FLAG_WRITE;
+   bi-flags = ~BLK_INTEGRITY_GENERATE;
  
  	return count;

  }
  
-static ssize_t integrity_write_show(struct blk_integrity *bi, char *page)

+static ssize_t integrity_generate_show(struct blk_integrity *bi, char *page)
  {
-   return sprintf(page, %d\n, (bi-flags  INTEGRITY_FLAG_WRITE) != 0);
+   return sprintf(page, %d\n, (bi-flags  BLK_INTEGRITY_GENERATE) != 0);
  }
  
  static struct integrity_sysfs_entry integrity_format_entry = {

@@ -317,23 +317,23 @@ static struct integrity_sysfs_entry 
integrity_tag_size_entry = {
.show = integrity_tag_size_show,
  };
  
-static struct integrity_sysfs_entry integrity_read_entry = {

+static struct integrity_sysfs_entry integrity_verify_entry = {
.attr = { .name = read_verify, .mode = S_IRUGO | S_IWUSR },
-   .show = integrity_read_show,
-   .store = integrity_read_store,
+   .show = integrity_verify_show,
+   .store = integrity_verify_store,
  };
  
-static struct integrity_sysfs_entry integrity_write_entry = {

+static struct integrity_sysfs_entry integrity_generate_entry = {
.attr = { .name = write_generate, .mode = S_IRUGO | S_IWUSR },
-   .show = integrity_write_show,
-   .store = integrity_write_store,
+   .show = integrity_generate_show,
+   .store = integrity_generate_store,
  };
  
  static struct attribute *integrity_attrs[] = {

integrity_format_entry.attr,
integrity_tag_size_entry.attr,
-   integrity_read_entry.attr,
-   integrity_write_entry.attr,
+   integrity_verify_entry.attr,
+   integrity_generate_entry.attr,
NULL,
  };
  
@@ -406,7 +406,7 @@ int blk_integrity_register(struct gendisk *disk, struct blk_integrity *template)
  
  		kobject_uevent(bi-kobj, KOBJ_ADD);
  
-		bi-flags |= INTEGRITY_FLAG_READ | INTEGRITY_FLAG_WRITE;

+   bi-flags |= BLK_INTEGRITY_VERIFY | BLK_INTEGRITY_GENERATE;
bi-interval = queue_logical_block_size(disk-queue);
disk-integrity = bi;
} else
@@ -419,6 +419,7 @@ int blk_integrity_register(struct gendisk *disk, struct 

Re: [PATCH 08/14] block: Add a disk flag to block integrity profile

2014-07-03 Thread Sagi Grimberg

On 6/25/2014 2:49 PM, Martin K. Petersen wrote:

Christoph == Christoph Hellwig h...@infradead.org writes:

Christoph On Wed, Jun 11, 2014 at 09:30:34PM -0400, Martin K. Petersen wrote:

/sys/block/foo/integrity/disk_is_formatted_with_pi
/sys/block/foo/integrity/disk_is_integrity_capable
/sys/block/foo/integrity/disk_supports_storing_pi

Or would you prefer something other than disk? target?
storage_device?

Christoph I'd defintively prefer the target_ prefix and one of the
Christoph descriptive suffixes.

OK, will tweak.



But this entry doesn't refer to a target, it refers to a backend device.
IMO disk_ or device_ or storage_device prefixes would be better...

Sagi.
--
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 09/14] block: Relocate integrity flags

2014-07-03 Thread Sagi Grimberg

On 5/29/2014 6:28 AM, Martin K. Petersen wrote:

Move flags affecting the integrity code out of the bio bi_flags and into
the block integrity payload.

Signed-off-by: Martin K. Petersen martin.peter...@oracle.com
---
  block/bio-integrity.c |  4 ++--
  drivers/scsi/sd_dif.c |  4 ++--
  include/linux/bio.h   | 27 ++-
  include/linux/blk_types.h |  6 ++
  4 files changed, 32 insertions(+), 9 deletions(-)

diff --git a/block/bio-integrity.c b/block/bio-integrity.c
index c91181e3d18d..877bce028766 100644
--- a/block/bio-integrity.c
+++ b/block/bio-integrity.c
@@ -98,7 +98,7 @@ void bio_integrity_free(struct bio *bio)
struct bio_integrity_payload *bip = bio_integrity(bio);
struct bio_set *bs = bio-bi_pool;
  
-	if (bip-bip_owns_buf)

+   if (bip_get_flag(bip, BIP_BLOCK_INTEGRITY))
kfree(page_address(bip-bip_vec-bv_page) +
  bip-bip_vec-bv_offset);
  
@@ -299,7 +299,7 @@ int bio_integrity_prep(struct bio *bio)

return -EIO;
}
  
-	bip-bip_owns_buf = 1;

+   bip_set_flag(bip, BIP_BLOCK_INTEGRITY);
bip-bip_iter.bi_size = len;
bip_set_seed(bip, bio-bi_iter.bi_sector);
  
diff --git a/drivers/scsi/sd_dif.c b/drivers/scsi/sd_dif.c

index 1d401f864fbe..95d5cb806f58 100644
--- a/drivers/scsi/sd_dif.c
+++ b/drivers/scsi/sd_dif.c
@@ -326,7 +326,7 @@ void sd_dif_prepare(struct request *rq, sector_t hw_sector,
unsigned int j;
  
  		/* Already remapped? */

-   if (bio_flagged(bio, BIO_MAPPED_INTEGRITY))
+   if (bip_get_flag(bip, BIP_MAPPED_INTEGRITY))
break;
  
  		virt = bip_get_seed(bip)  0x;

@@ -347,7 +347,7 @@ void sd_dif_prepare(struct request *rq, sector_t hw_sector,
kunmap_atomic(sdt);
}
  
-		bio-bi_flags |= (1  BIO_MAPPED_INTEGRITY);

+   bip_set_flag(bip, BIP_MAPPED_INTEGRITY);
}
  }
  
diff --git a/include/linux/bio.h b/include/linux/bio.h

index 295545de8790..adc806325c36 100644
--- a/include/linux/bio.h
+++ b/include/linux/bio.h
@@ -296,7 +296,7 @@ struct bio_integrity_payload {
  
  	unsigned short		bip_slab;	/* slab the bip came from */

unsigned short  bip_vcnt;   /* # of integrity bio_vecs */
-   unsignedbip_owns_buf:1; /* should free bip_buf */
+   unsigned short  bip_flags;  /* control flags */
  
  	struct work_struct	bip_work;	/* I/O completion */
  
@@ -304,6 +304,31 @@ struct bio_integrity_payload {

struct bio_vec  bip_inline_vecs[0];/* embedded bvec array */
  };
  
+enum bip_flags {

+   BIP_BLOCK_INTEGRITY = 0,/* block layer owns integrity data, not fs */
+   BIP_MAPPED_INTEGRITY,   /* integrity metadata has been remapped */
+   BIP_CTRL_NOCHECK,   /* disable controller integrity checking */
+   BIP_DISK_NOCHECK,   /* disable disk integrity checking */
+};
+
+static inline bool bip_get_flag(struct bio_integrity_payload *bip,
+   enum bip_flags flag)
+{
+   if (bip  bip-bip_flags  (1  flag))
+   return true;
+
+   return false;
+}
+
+static inline void bip_set_flag(struct bio_integrity_payload *bip,
+   enum bip_flags flag)
+{
+   if (!bip)
+   return;
+
+   bip-bip_flags |= (1  flag);
+}
+
  static inline sector_t bip_get_seed(struct bio_integrity_payload *bip)
  {
return bip-bip_iter.bi_sector;
diff --git a/include/linux/blk_types.h b/include/linux/blk_types.h
index 9cce1fcd6793..b2e389a16534 100644
--- a/include/linux/blk_types.h
+++ b/include/linux/blk_types.h
@@ -120,10 +120,8 @@ struct bio {
  #define BIO_USER_MAPPED 6 /* contains user pages */
  #define BIO_EOPNOTSUPP7   /* not supported */
  #define BIO_NULL_MAPPED 8 /* contains invalid user pages */
-#define BIO_FS_INTEGRITY 9 /* fs owns integrity data, not block layer */
-#define BIO_QUIET  10  /* Make BIO Quiet */
-#define BIO_MAPPED_INTEGRITY 11/* integrity metadata has been remapped */
-#define BIO_SNAP_STABLE12  /* bio data must be snapshotted during 
write */
+#define BIO_QUIET  9   /* Make BIO Quiet */
+#define BIO_SNAP_STABLE10  /* bio data must be snapshotted during 
write */
  
  /*

   * Flags starting here get preserved by bio_reset() - this includes


Christoph had comments on this, but I'm fine with this either way.

Other than that:

Reviewed-by: Sagi Grimbergsa...@mellanox.com


--
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 11/14] block: Don't merge requests if integrity flags differ

2014-07-03 Thread Sagi Grimberg

On 5/29/2014 6:28 AM, Martin K. Petersen wrote:

We'd occasionally merge requests with conflicting integrity flags.
Introduce a merge helper which checks that the requests have compatible
integrity payloads.

Signed-off-by: Martin K. Petersen martin.peter...@oracle.com
---
  block/blk-integrity.c  | 36 ++--
  block/blk-merge.c  |  6 +++---
  include/linux/blkdev.h | 20 ++--
  3 files changed, 39 insertions(+), 23 deletions(-)

diff --git a/block/blk-integrity.c b/block/blk-integrity.c
index 8cf87655152b..da608e73bdb1 100644
--- a/block/blk-integrity.c
+++ b/block/blk-integrity.c
@@ -186,37 +186,53 @@ int blk_integrity_compare(struct gendisk *gd1, struct 
gendisk *gd2)
  }
  EXPORT_SYMBOL(blk_integrity_compare);
  
-int blk_integrity_merge_rq(struct request_queue *q, struct request *req,

-  struct request *next)
+bool blk_integrity_merge_rq(struct request_queue *q, struct request *req,
+   struct request *next)
  {
-   if (blk_integrity_rq(req) != blk_integrity_rq(next))
-   return -1;
+   if (blk_integrity_rq(req) == 0  blk_integrity_rq(next) == 0)
+   return true;
+
+   if (blk_integrity_rq(req) == 0 || blk_integrity_rq(next) == 0)
+   return false;
+
+   if (bio_integrity(req-bio)-bip_flags !=
+   bio_integrity(next-bio)-bip_flags)
+   return false;
  
  	if (req-nr_integrity_segments + next-nr_integrity_segments 

q-limits.max_integrity_segments)
-   return -1;
+   return false;
  
-	return 0;

+   return true;
  }
  EXPORT_SYMBOL(blk_integrity_merge_rq);
  
-int blk_integrity_merge_bio(struct request_queue *q, struct request *req,

-   struct bio *bio)
+bool blk_integrity_merge_bio(struct request_queue *q, struct request *req,
+struct bio *bio)
  {
int nr_integrity_segs;
struct bio *next = bio-bi_next;
  
+	if (blk_integrity_rq(req) == 0  bio_integrity(bio) == NULL)

+   return true;
+
+   if (blk_integrity_rq(req) == 0 || bio_integrity(bio) == NULL)
+   return false;
+
+   if (bio_integrity(req-bio)-bip_flags != bio_integrity(bio)-bip_flags)
+   return false;
+
bio-bi_next = NULL;
nr_integrity_segs = blk_rq_count_integrity_sg(q, bio);
bio-bi_next = next;
  
  	if (req-nr_integrity_segments + nr_integrity_segs 

q-limits.max_integrity_segments)
-   return -1;
+   return false;
  
  	req-nr_integrity_segments += nr_integrity_segs;
  
-	return 0;

+   return true;
  }
  EXPORT_SYMBOL(blk_integrity_merge_bio);
  
diff --git a/block/blk-merge.c b/block/blk-merge.c

index 6c583f9c5b65..21e38f407785 100644
--- a/block/blk-merge.c
+++ b/block/blk-merge.c
@@ -294,7 +294,7 @@ static inline int ll_new_hw_segment(struct request_queue *q,
if (req-nr_phys_segments + nr_phys_segs  queue_max_segments(q))
goto no_merge;
  
-	if (bio_integrity(bio)  blk_integrity_merge_bio(q, req, bio))

+   if (blk_integrity_merge_bio(q, req, bio) == false)
goto no_merge;
  
  	/*

@@ -391,7 +391,7 @@ static int ll_merge_requests_fn(struct request_queue *q, 
struct request *req,
if (total_phys_segments  queue_max_segments(q))
return 0;
  
-	if (blk_integrity_rq(req)  blk_integrity_merge_rq(q, req, next))

+   if (blk_integrity_merge_rq(q, req, next) == false)
return 0;
  
  	/* Merge is OK... */

@@ -569,7 +569,7 @@ bool blk_rq_merge_ok(struct request *rq, struct bio *bio)
return false;
  
  	/* only merge integrity protected bio into ditto rq */

-   if (bio_integrity(bio) != blk_integrity_rq(rq))
+   if (blk_integrity_merge_bio(rq-q, rq, bio) == false)
return false;
  
  	/* must be using the same buffer */

diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 9bf6f761f1ac..45cd70cda4e8 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -1467,10 +1467,10 @@ extern int blk_integrity_compare(struct gendisk *, 
struct gendisk *);
  extern int blk_rq_map_integrity_sg(struct request_queue *, struct bio *,
   struct scatterlist *);
  extern int blk_rq_count_integrity_sg(struct request_queue *, struct bio *);
-extern int blk_integrity_merge_rq(struct request_queue *, struct request *,
- struct request *);
-extern int blk_integrity_merge_bio(struct request_queue *, struct request *,
-  struct bio *);
+extern bool blk_integrity_merge_rq(struct request_queue *, struct request *,
+  struct request *);
+extern bool blk_integrity_merge_bio(struct request_queue *, struct request *,
+   struct bio *);
  
  static inline

  struct blk_integrity 

Re: [PATCH 04/14] block: Remove bip_buf

2014-07-03 Thread Sagi Grimberg

On 6/11/2014 7:35 PM, Christoph Hellwig wrote:

On Wed, May 28, 2014 at 11:28:38PM -0400, Martin K. Petersen wrote:

bip_buf is not really needed so we can remove it.

Signed-off-by: Martin K. Petersen martin.peter...@oracle.com

Looks good,

Reviewed-by: Christoph Hellwig h...@lst.de


Reviewed-by: Sagi Grimberg sa...@mellanox.com

--
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 05/14] block: Deprecate the use of the term sector in the context of block integrity

2014-07-03 Thread Sagi Grimberg

On 7/3/2014 12:35 PM, Sagi Grimberg wrote:

On 5/29/2014 6:28 AM, Martin K. Petersen wrote:

The protection interval is not necessarily tied to the logical block
size of a block device. Stop using the terms sector and sectors.

Signed-off-by: Martin K. Petersen martin.peter...@oracle.com
---
  block/bio-integrity.c  | 46 
+-

  block/blk-integrity.c  | 10 +-
  drivers/scsi/sd_dif.c  | 46 
+++---

  include/linux/blkdev.h |  6 +++---
  4 files changed, 52 insertions(+), 56 deletions(-)

diff --git a/block/bio-integrity.c b/block/bio-integrity.c
index e06b3c807eef..c52a8fd98706 100644
--- a/block/bio-integrity.c
+++ b/block/bio-integrity.c
@@ -191,29 +191,25 @@ bool bio_integrity_enabled(struct bio *bio)
  EXPORT_SYMBOL(bio_integrity_enabled);
/**
- * bio_integrity_hw_sectors - Convert 512b sectors to hardware ditto
+ * bio_integrity_intervals - Return number of integrity intervals 
for a bio

   * @bi:blk_integrity profile for device
- * @sectors:Number of 512 sectors to convert
+ * @sectors:Size of the bio in 512-byte sectors
   *
   * Description: The block layer calculates everything in 512 byte
- * sectors but integrity metadata is done in terms of the hardware
- * sector size of the storage device.  Convert the block layer sectors
- * to physical sectors.
+ * sectors but integrity metadata is done in terms of the data 
integrity
+ * interval size of the storage device.  Convert the block layer 
sectors

+ * to the appropriate number of integrity intervals.
   */
-static inline unsigned int bio_integrity_hw_sectors(struct 
blk_integrity *bi,

-unsigned int sectors)
+static inline unsigned int bio_integrity_intervals(struct 
blk_integrity *bi,

+   unsigned int sectors)
  {
-/* At this point there are only 512b or 4096b DIF/EPP devices */
-if (bi-sector_size == 4096)
-return sectors = 3;
-
-return sectors;
+return sectors  (ilog2(bi-interval) - 9);
  }


Now that protection information interval does not necessarily match 
the sector_size, should this routine
protect against bogus bi-interval (e.g. fail if bi-interval  
sector_size for example)? Not sure
if this check is really needed here, but it might be useful to have 
(although protection interval

is still effectively sector_size).



For v1 scsi_transfer_length should also be modified to use protection 
interval.


Sagi.
--
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


[PATCH 2/3] bnx2fc: Rebranding bnx2fc driver

2014-07-03 Thread vikas.chaudhary
From: Saurav Kashyap saurav.kash...@qlogic.com

QLogic has acquired the NetXtremeII products and drivers from Broadcom.
This patch re-brands bnx2fc driver as a QLogic driver

Signed-off-by: Saurav Kashyap saurav.kash...@qlogic.com
Signed-off-by: Vikas Chaudhary vikas.chaudh...@qlogic.com
Acked-by: Eddie Wai eddie@broadcom.com
---
 drivers/scsi/bnx2fc/57xx_hsi_bnx2fc.h  | 13 +
 drivers/scsi/bnx2fc/Kconfig|  4 ++--
 drivers/scsi/bnx2fc/bnx2fc.h   |  7 ---
 drivers/scsi/bnx2fc/bnx2fc_constants.h | 13 +
 drivers/scsi/bnx2fc/bnx2fc_debug.c | 13 +
 drivers/scsi/bnx2fc/bnx2fc_debug.h | 13 +
 drivers/scsi/bnx2fc/bnx2fc_els.c   |  3 ++-
 drivers/scsi/bnx2fc/bnx2fc_fcoe.c  | 11 ++-
 drivers/scsi/bnx2fc/bnx2fc_hwi.c   |  3 ++-
 drivers/scsi/bnx2fc/bnx2fc_io.c|  3 ++-
 drivers/scsi/bnx2fc/bnx2fc_tgt.c   |  3 ++-
 11 files changed, 72 insertions(+), 14 deletions(-)

diff --git a/drivers/scsi/bnx2fc/57xx_hsi_bnx2fc.h 
b/drivers/scsi/bnx2fc/57xx_hsi_bnx2fc.h
index e1f1e34..fe2106c 100644
--- a/drivers/scsi/bnx2fc/57xx_hsi_bnx2fc.h
+++ b/drivers/scsi/bnx2fc/57xx_hsi_bnx2fc.h
@@ -1,3 +1,16 @@
+/* 57xx_hsi_bnx2fc.h: QLogic NetXtreme II Linux FCoE offload driver.
+ * Handles operations such as session offload/upload etc, and manages
+ * session resources such as connection id and qp resources.
+ *
+ * Copyright (c) 2008 - 2013 Broadcom Corporation
+ * Copyright (c) 2014, QLogic Corporation
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation.
+ *
+ */
+
 #ifndef __57XX_FCOE_HSI_LINUX_LE__
 #define __57XX_FCOE_HSI_LINUX_LE__
 
diff --git a/drivers/scsi/bnx2fc/Kconfig b/drivers/scsi/bnx2fc/Kconfig
index cfcad8b..f245d54 100644
--- a/drivers/scsi/bnx2fc/Kconfig
+++ b/drivers/scsi/bnx2fc/Kconfig
@@ -1,5 +1,5 @@
 config SCSI_BNX2X_FCOE
-   tristate Broadcom NetXtreme II FCoE support
+   tristate QLogic NetXtreme II FCoE support
depends on PCI
select NETDEVICES
select ETHERNET
@@ -8,5 +8,5 @@ config SCSI_BNX2X_FCOE
select LIBFCOE
select CNIC
---help---
-   This driver supports FCoE offload for the Broadcom NetXtreme II
+   This driver supports FCoE offload for the QLogic NetXtreme II
devices.
diff --git a/drivers/scsi/bnx2fc/bnx2fc.h b/drivers/scsi/bnx2fc/bnx2fc.h
index 6a97665..1346e05 100644
--- a/drivers/scsi/bnx2fc/bnx2fc.h
+++ b/drivers/scsi/bnx2fc/bnx2fc.h
@@ -1,8 +1,7 @@
-#ifndef _BNX2FC_H_
-#define _BNX2FC_H_
-/* bnx2fc.h: Broadcom NetXtreme II Linux FCoE offload driver.
+/* bnx2fc.h: QLogic NetXtreme II Linux FCoE offload driver.
  *
  * Copyright (c) 2008 - 2013 Broadcom Corporation
+ * Copyright (c) 2014, QLogic Corporation
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by
@@ -11,6 +10,8 @@
  * Written by: Bhanu Prakash Gollapudi (bprak...@broadcom.com)
  */
 
+#ifndef _BNX2FC_H_
+#define _BNX2FC_H_
 #define pr_fmt(fmt) KBUILD_MODNAME :  fmt
 
 #include linux/module.h
diff --git a/drivers/scsi/bnx2fc/bnx2fc_constants.h 
b/drivers/scsi/bnx2fc/bnx2fc_constants.h
index dad9924..e147cc7 100644
--- a/drivers/scsi/bnx2fc/bnx2fc_constants.h
+++ b/drivers/scsi/bnx2fc/bnx2fc_constants.h
@@ -1,3 +1,16 @@
+/* bnx2fc_constants.h: QLogic NetXtreme II Linux FCoE offload driver.
+ * Handles operations such as session offload/upload etc, and manages
+ * session resources such as connection id and qp resources.
+ *
+ * Copyright (c) 2008 - 2013 Broadcom Corporation
+ * Copyright (c) 2014, QLogic Corporation
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation.
+ *
+ */
+
 #ifndef __BNX2FC_CONSTANTS_H_
 #define __BNX2FC_CONSTANTS_H_
 
diff --git a/drivers/scsi/bnx2fc/bnx2fc_debug.c 
b/drivers/scsi/bnx2fc/bnx2fc_debug.c
index 0cbee1b..d055df0 100644
--- a/drivers/scsi/bnx2fc/bnx2fc_debug.c
+++ b/drivers/scsi/bnx2fc/bnx2fc_debug.c
@@ -1,3 +1,16 @@
+/* bnx2fc_debug.c: QLogic NetXtreme II Linux FCoE offload driver.
+ * Handles operations such as session offload/upload etc, and manages
+ * session resources such as connection id and qp resources.
+ *
+ * Copyright (c) 2008 - 2013 Broadcom Corporation
+ * Copyright (c) 2014, QLogic Corporation
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation.
+ *
+ */
+
 #include bnx2fc.h
 
 void BNX2FC_IO_DBG(const struct bnx2fc_cmd *io_req, const char *fmt, ...)
diff --git a/drivers/scsi/bnx2fc/bnx2fc_debug.h 
b/drivers/scsi/bnx2fc/bnx2fc_debug.h
index 4808ff9..2b90067 100644
--- a/drivers/scsi/bnx2fc/bnx2fc_debug.h
+++ 

[PATCH 0/3] bnx2i/bnx2fc: Rebranding changes for bnx2i and bnx2fc driver

2014-07-03 Thread vikas.chaudhary
From: Vikas Chaudhary vikas.chaudh...@qlogic.com

The following set of patches are for rebranding Broadcom bnx2i and bnx2fc 
driver as QLogic drivers.
Please apply the following patches to the scsi tree at your earliest 
convenience.

Saurav Kashyap (1):
  bnx2fc: Rebranding bnx2fc driver

Vikas Chaudhary (2):
  bnx2i: Rebranding bnx2i driver
  MAINTAINERS: Update maintainer list for bnx2i and bnx2fc
--
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


[PATCH 1/3] bnx2i: Rebranding bnx2i driver

2014-07-03 Thread vikas.chaudhary
From: Vikas Chaudhary vikas.chaudh...@qlogic.com

QLogic has acquired the NetXtremeII products and drivers from Broadcom.
This patch re-brands bnx2i driver as a QLogic driver

Signed-off-by: Vikas Chaudhary vikas.chaudh...@qlogic.com
Acked-by: Eddie Wai eddie@broadcom.com
---
 drivers/scsi/bnx2i/57xx_iscsi_constants.h |  6 --
 drivers/scsi/bnx2i/57xx_iscsi_hsi.h   |  6 --
 drivers/scsi/bnx2i/Kconfig|  4 ++--
 drivers/scsi/bnx2i/bnx2i.h|  6 --
 drivers/scsi/bnx2i/bnx2i_hwi.c|  6 --
 drivers/scsi/bnx2i/bnx2i_init.c   | 10 ++
 drivers/scsi/bnx2i/bnx2i_iscsi.c  |  8 +---
 drivers/scsi/bnx2i/bnx2i_sysfs.c  |  6 --
 8 files changed, 33 insertions(+), 19 deletions(-)

diff --git a/drivers/scsi/bnx2i/57xx_iscsi_constants.h 
b/drivers/scsi/bnx2i/57xx_iscsi_constants.h
index 3d33767..9175341 100644
--- a/drivers/scsi/bnx2i/57xx_iscsi_constants.h
+++ b/drivers/scsi/bnx2i/57xx_iscsi_constants.h
@@ -1,13 +1,15 @@
-/* 57xx_iscsi_constants.h: Broadcom NetXtreme II iSCSI HSI
+/* 57xx_iscsi_constants.h: QLogic NetXtreme II iSCSI HSI
  *
  * Copyright (c) 2006 - 2013 Broadcom Corporation
+ * Copyright (c) 2014, QLogic Corporation
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by
  * the Free Software Foundation.
  *
  * Written by: Anil Veerabhadrappa (ani...@broadcom.com)
- * Maintained by: Eddie Wai (eddie@broadcom.com)
+ * Previously Maintained by: Eddie Wai (eddie@broadcom.com)
+ * Maintained by: qlogic-storage-upstr...@qlogic.com
  */
 #ifndef __57XX_ISCSI_CONSTANTS_H_
 #define __57XX_ISCSI_CONSTANTS_H_
diff --git a/drivers/scsi/bnx2i/57xx_iscsi_hsi.h 
b/drivers/scsi/bnx2i/57xx_iscsi_hsi.h
index 7052a83..19b3a97 100644
--- a/drivers/scsi/bnx2i/57xx_iscsi_hsi.h
+++ b/drivers/scsi/bnx2i/57xx_iscsi_hsi.h
@@ -1,13 +1,15 @@
-/* 57xx_iscsi_hsi.h: Broadcom NetXtreme II iSCSI HSI.
+/* 57xx_iscsi_hsi.h: QLogic NetXtreme II iSCSI HSI.
  *
  * Copyright (c) 2006 - 2013 Broadcom Corporation
+ * Copyright (c) 2014, QLogic Corporation
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by
  * the Free Software Foundation.
  *
  * Written by: Anil Veerabhadrappa (ani...@broadcom.com)
- * Maintained by: Eddie Wai (eddie@broadcom.com)
+ * Previously Maintained by: Eddie Wai (eddie@broadcom.com)
+ * Maintained by: qlogic-storage-upstr...@qlogic.com
  */
 #ifndef __57XX_ISCSI_HSI_LINUX_LE__
 #define __57XX_ISCSI_HSI_LINUX_LE__
diff --git a/drivers/scsi/bnx2i/Kconfig b/drivers/scsi/bnx2i/Kconfig
index 01cff18..44ce54e 100644
--- a/drivers/scsi/bnx2i/Kconfig
+++ b/drivers/scsi/bnx2i/Kconfig
@@ -1,5 +1,5 @@
 config SCSI_BNX2_ISCSI
-   tristate Broadcom NetXtreme II iSCSI support
+   tristate QLogic NetXtreme II iSCSI support
depends on NET
depends on PCI
select SCSI_ISCSI_ATTRS
@@ -8,5 +8,5 @@ config SCSI_BNX2_ISCSI
select NET_VENDOR_BROADCOM
select CNIC
---help---
-   This driver supports iSCSI offload for the Broadcom NetXtreme II
+   This driver supports iSCSI offload for the QLogic NetXtreme II
devices.
diff --git a/drivers/scsi/bnx2i/bnx2i.h b/drivers/scsi/bnx2i/bnx2i.h
index c73bbcb..ed7f322 100644
--- a/drivers/scsi/bnx2i/bnx2i.h
+++ b/drivers/scsi/bnx2i/bnx2i.h
@@ -1,15 +1,17 @@
-/* bnx2i.h: Broadcom NetXtreme II iSCSI driver.
+/* bnx2i.h: QLogic NetXtreme II iSCSI driver.
  *
  * Copyright (c) 2006 - 2013 Broadcom Corporation
  * Copyright (c) 2007, 2008 Red Hat, Inc.  All rights reserved.
  * Copyright (c) 2007, 2008 Mike Christie
+ * Copyright (c) 2014, QLogic Corporation
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by
  * the Free Software Foundation.
  *
  * Written by: Anil Veerabhadrappa (ani...@broadcom.com)
- * Maintained by: Eddie Wai (eddie@broadcom.com)
+ * Previously Maintained by: Eddie Wai (eddie@broadcom.com)
+ * Maintained by: qlogic-storage-upstr...@qlogic.com
  */
 
 #ifndef _BNX2I_H_
diff --git a/drivers/scsi/bnx2i/bnx2i_hwi.c b/drivers/scsi/bnx2i/bnx2i_hwi.c
index b5ffd28..cfb8e90 100644
--- a/drivers/scsi/bnx2i/bnx2i_hwi.c
+++ b/drivers/scsi/bnx2i/bnx2i_hwi.c
@@ -1,15 +1,17 @@
-/* bnx2i_hwi.c: Broadcom NetXtreme II iSCSI driver.
+/* bnx2i_hwi.c: QLogic NetXtreme II iSCSI driver.
  *
  * Copyright (c) 2006 - 2013 Broadcom Corporation
  * Copyright (c) 2007, 2008 Red Hat, Inc.  All rights reserved.
  * Copyright (c) 2007, 2008 Mike Christie
+ * Copyright (c) 2014, QLogic Corporation
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by
  * the Free Software Foundation.
  *
  * Written by: Anil Veerabhadrappa 

[PATCH 3/3] MAINTAINERS: Update maintainer list for bnx2i and bnx2fc

2014-07-03 Thread vikas.chaudhary
From: Vikas Chaudhary vikas.chaudh...@qlogic.com

Signed-off-by: Vikas Chaudhary vikas.chaudh...@qlogic.com
Acked-by: Eddie Wai eddie@broadcom.com
---
 MAINTAINERS | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index 2482eee..dec99fb 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1943,13 +1943,13 @@ S:  Supported
 F: drivers/net/wireless/brcm80211/
 
 BROADCOM BNX2FC 10 GIGABIT FCOE DRIVER
-M: Eddie Wai eddie@broadcom.com
+M: qlogic-storage-upstr...@qlogic.com
 L: linux-scsi@vger.kernel.org
 S: Supported
 F: drivers/scsi/bnx2fc/
 
 BROADCOM BNX2I 1/10 GIGABIT iSCSI DRIVER
-M: Eddie Wai eddie@broadcom.com
+M: qlogic-storage-upstr...@qlogic.com
 L: linux-scsi@vger.kernel.org
 S: Supported
 F: drivers/scsi/bnx2i/
-- 
1.8.2.GIT

--
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] [SCSI] hpsa: fix non-x86 builds

2014-07-03 Thread scameron
On Thu, Jul 03, 2014 at 01:43:48AM -0700, Christoph Hellwig wrote:
 On Thu, Jun 26, 2014 at 09:34:45AM -0500, scame...@beardog.cce.hp.com wrote:
  Thanks.
 
 Do you plan to include this with the next hpsa update, or should I take
 this as an ACK and apply it?

Take it as an ACK.  I should have been clearer.

-- steve

--
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 3.15 099/139] libiscsi, iser: Adjust data_length to include protection information

2014-07-03 Thread James Bottomley
On Wed, 2014-07-02 at 23:05 -0700, Christoph Hellwig wrote:
 On Wed, Jul 02, 2014 at 03:52:35PM -0700, James Bottomley wrote:
  In which case it's not marked for stable backport ... I assume it should
  be?
 
 Only if the patch that regression went to stable, which it shouldn't,

Well it did:

https://git.kernel.org/cgit/linux/kernel/git/stable/linux-stable.git/commit/drivers/scsi?h=linux-3.15.yid=04423ddea30a7fb7232636eda8aed55ea5b972fe

The stable process is a bit broken from my point of view since it
doesn't copy linux-scsi on SCSI commits ... but this time even I didn't
get a cc, so really no-one who knew there was a problem had a chance to
comment.

The choice is either request revert from stable or add fix.

James


--
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 v5 1/2] libahci: Implement the function ahci_restart_engine to restart the port dma engine.

2014-07-03 Thread Tejun Heo
Hello,

On Wed, Jul 02, 2014 at 03:45:07PM +0530, Suman Tripathi wrote:
 +int ahci_restart_engine(struct ata_port *ap)
 +{
 + struct ahci_host_priv *hpriv = ap-host-private_data;
 +
 + ahci_stop_engine(ap);
 + ahci_start_fis_rx(ap);
 + hpriv-start_engine(ap);
 +
 + return 0;
 +}
 +EXPORT_SYMBOL_GPL(ahci_restart_engine);

I don't get this function.  No other ahci needs it and it's not even
contained libahci proper.  It calls back to the calling low level
driver.  Why not just expose ahci_start_fis_rx() and implement the
above in xgene?

Thanks.

-- 
tejun
--
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 0/5] SRP initiator patches for kernel 3.17

2014-07-03 Thread Bart Van Assche
(CC-ing linux-scsi)

On 07/03/14 15:44, Bart Van Assche wrote:
 This patch series consists of the following five patches, of which three
 are bug fixes and two are performance optimizations:
 
 0001-scsi_transport_srp-Fix-fast_io_fail_tmo-dev_loss_tmo.patch
 0002-IB-srp-Fix-deadlock-between-host-removal-and-multipa.patch
 0003-IB-srp-Fix-residual-handling.patch
 0004-IB-srp-Use-P_Key-cache-for-P_Key-lookups.patch
 0005-IB-srp-Optimize-completion-queue-polling.patch
 
 --
 To unsubscribe from this list: send the line unsubscribe linux-rdma in
 the body of a message to majord...@vger.kernel.org
 More majordomo info at  http://vger.kernel.org/majordomo-info.html
 .
 

--
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 0/5] SRP initiator patches for kernel 3.17

2014-07-03 Thread Bart Van Assche
(CC'ing linux-scsi)

On 07/03/14 15:45, Bart Van Assche wrote: If scsi_remove_host() is called 
while an rport is in the blocked
 state then scsi_remove_host() will only finish if the rport is
 unblocked from inside a timer function. Make sure that an rport
 only enters the blocked state if a timer will be started that
 will unblock it. This avoids that unloading the ib_srp kernel
 module after having disconnected the initiator from the target
 system results in a deadlock if both the fast_io_fail_tmo and
 dev_loss_tmo parameters have been set to off.
 
 Signed-off-by: Bart Van Assche bvanass...@acm.org
 Cc: James Bottomley jbottom...@parallels.com
 Cc: Sagi Grimberg sa...@mellanox.com
 Cc: Sebastian Parschauer sebastian.rie...@profitbricks.com
 Cc: David Dillow d...@thedillows.org
 Cc: sta...@vger.kernel.org
 ---
  drivers/scsi/scsi_transport_srp.c | 3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)
 
 diff --git a/drivers/scsi/scsi_transport_srp.c 
 b/drivers/scsi/scsi_transport_srp.c
 index 13e8983..a0c5bfd 100644
 --- a/drivers/scsi/scsi_transport_srp.c
 +++ b/drivers/scsi/scsi_transport_srp.c
 @@ -473,7 +473,8 @@ static void __srp_start_tl_fail_timers(struct srp_rport 
 *rport)
   if (delay  0)
   queue_delayed_work(system_long_wq, rport-reconnect_work,
  1UL * delay * HZ);
 - if (srp_rport_set_state(rport, SRP_RPORT_BLOCKED) == 0) {
 + if ((fast_io_fail_tmo = 0 || dev_loss_tmo = 0) 
 + srp_rport_set_state(rport, SRP_RPORT_BLOCKED) == 0) {
   pr_debug(%s new state: %d\n, dev_name(shost-shost_gendev),
rport-state);
   scsi_target_block(shost-shost_gendev);
 

--
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 0/5] SRP initiator patches for kernel 3.17

2014-07-03 Thread Sagi Grimberg

On 7/3/2014 5:17 PM, Bart Van Assche wrote:

This patch series consists of the following five patches, of which three
are bug fixes and two are performance optimizations:

0001-scsi_transport_srp-Fix-fast_io_fail_tmo-dev_loss_tmo.patch
0002-IB-srp-Fix-deadlock-between-host-removal-and-multipa.patch
0003-IB-srp-Fix-residual-handling.patch
0004-IB-srp-Use-P_Key-cache-for-P_Key-lookups.patch
0005-IB-srp-Optimize-completion-queue-polling.patch

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



Hey Bart,

The series looks good,

You can add to the series:
Reviewed-by: Sagi Grimberg sa...@mellanox.com

Specifically to patches #1, #4, #5 you can add:
Tested-by: Sagi Grimberg sa...@mellanox.com

Sagi.
--
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


[PATCH] scsi: break from queue depth adjusting loops when device found

2014-07-03 Thread Stephen M. Cameron
From: Stephen M. Cameron scame...@beardog.cce.hp.com

Don't loop through all the devices even after
finding the one we're looking for

Signed-off-by: Stephen M. Cameron scame...@beardog.cce.hp.com
Reviewed-by: Robert Elliott elli...@hp.com
---
 drivers/scsi/scsi_error.c |2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c
index cbe38e5..db8a488 100644
--- a/drivers/scsi/scsi_error.c
+++ b/drivers/scsi/scsi_error.c
@@ -635,6 +635,7 @@ static void scsi_handle_queue_ramp_up(struct scsi_device 
*sdev)
sht-change_queue_depth(tmp_sdev, tmp_sdev-queue_depth + 1,
SCSI_QDEPTH_RAMP_UP);
sdev-last_queue_ramp_up = jiffies;
+   break;
}
 }
 
@@ -657,6 +658,7 @@ static void scsi_handle_queue_full(struct scsi_device *sdev)
 */
sht-change_queue_depth(tmp_sdev, tmp_sdev-queue_depth - 1,
SCSI_QDEPTH_QFULL);
+   break;
}
 }
 

--
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


[PATCH 0/5] hpsa a few small updates for early July 2014

2014-07-03 Thread Stephen M. Cameron
Nothing very big here, just a few small updates for now.

I still have a giant ball of patches waiting in the wings, but it
is unfortunately not quite ready yet.

---

Robert Elliott (1):
  hpsa: do not unconditionally copy sense data

Stephen M. Cameron (4):
  hpsa: remove online devices from offline device list
  hpsa: fix bad -ENOMEM return value in hpsa_big_passthru_ioctl
  hpsa: make hpsa_init_one return -ENOMEM if allocation of 
h-lockup_detected fails
  hpsa: fix 6-byte READ/WRITE with 0 length data xfer


 drivers/scsi/hpsa.c |   31 +++
 1 files changed, 19 insertions(+), 12 deletions(-)

-- 
-- steve

--
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


[PATCH 1/5] hpsa: remove online devices from offline device list

2014-07-03 Thread Stephen M. Cameron
From: Stephen M. Cameron scame...@beardog.cce.hp.com

When devices come on line, they should be removed from the list of
offline devices that are monitored.

Signed-off-by: Stephen M. Cameron scame...@beardog.cce.hp.com
Reviewed-by: Scott Teel scott.t...@hp.com
Reviewed-by: Joe Handzik joseph.t.hand...@hp.com
---
 drivers/scsi/hpsa.c |6 +-
 1 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c
index 31184b3..8cd1a9b 100644
--- a/drivers/scsi/hpsa.c
+++ b/drivers/scsi/hpsa.c
@@ -6913,8 +6913,12 @@ static int hpsa_offline_devices_ready(struct ctlr_info 
*h)
d = list_entry(this, struct offline_device_entry,
offline_list);
spin_unlock_irqrestore(h-offline_device_lock, flags);
-   if (!hpsa_volume_offline(h, d-scsi3addr))
+   if (!hpsa_volume_offline(h, d-scsi3addr)) {
+   spin_lock_irqsave(h-offline_device_lock, flags);
+   list_del(d-offline_list);
+   spin_unlock_irqrestore(h-offline_device_lock, flags);
return 1;
+   }
spin_lock_irqsave(h-offline_device_lock, flags);
}
spin_unlock_irqrestore(h-offline_device_lock, flags);

--
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


[PATCH 2/5] hpsa: fix bad -ENOMEM return value in hpsa_big_passthru_ioctl

2014-07-03 Thread Stephen M. Cameron
From: Stephen M. Cameron scame...@beardog.cce.hp.com

When copy_from_user fails, return -EFAULT, not -ENOMEM

Signed-off-by: Stephen M. Cameron scame...@beardog.cce.hp.com
Reported-by: Robert Elliott elli...@hp.com
Reviewed-by: Joe Handzik joseph.t.hand...@hp.com
Reviewed-by: Scott Teel scott.t...@hp.com
Cc: sta...@vger.kernel.org
---
 drivers/scsi/hpsa.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c
index 8cd1a9b..08b34e9 100644
--- a/drivers/scsi/hpsa.c
+++ b/drivers/scsi/hpsa.c
@@ -5092,7 +5092,7 @@ static int hpsa_big_passthru_ioctl(struct ctlr_info *h, 
void __user *argp)
}
if (ioc-Request.Type.Direction  XFER_WRITE) {
if (copy_from_user(buff[sg_used], data_ptr, sz)) {
-   status = -ENOMEM;
+   status = -EFAULT;
goto cleanup1;
}
} else

--
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


[PATCH 4/5] hpsa: fix 6-byte READ/WRITE with 0 length data xfer

2014-07-03 Thread Stephen M. Cameron
From: Stephen M. Cameron scame...@beardog.cce.hp.com

a 6-byte READ/WRITE CDB with a 0 block data transfer really
means a 256 block data transfer.  The RAID mapping code failed
to handle this case.  For 10/12/16 byte READ/WRITEs, 0 just means
no data should be transferred, and should not trigger BUG_ON.

Signed-off-by: Stephen M. Cameron scame...@beardog.cce.hp.com
Reported-by: Robert Elliott elli...@hp.com
Reviewed-by: Robert Elliott elli...@hp.com
---
 drivers/scsi/hpsa.c |3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c
index 794d726..a97a7ff 100644
--- a/drivers/scsi/hpsa.c
+++ b/drivers/scsi/hpsa.c
@@ -3686,6 +3686,8 @@ static int hpsa_scsi_ioaccel_raid_map(struct ctlr_info *h,
(((u64) cmd-cmnd[2])  8) |
cmd-cmnd[3];
block_cnt = cmd-cmnd[4];
+   if (block_cnt == 0)
+   block_cnt = 256;
break;
case WRITE_10:
is_write = 1;
@@ -3734,7 +3736,6 @@ static int hpsa_scsi_ioaccel_raid_map(struct ctlr_info *h,
default:
return IO_ACCEL_INELIGIBLE; /* process via normal I/O path */
}
-   BUG_ON(block_cnt == 0);
last_block = first_block + block_cnt - 1;
 
/* check for write to non-RAID-0 */

--
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


[PATCH 5/5] hpsa: do not unconditionally copy sense data

2014-07-03 Thread Stephen M. Cameron
From: Robert Elliott elli...@hp.com

Signed-off-by: Robert Elliott elli...@hp.com
Signed-off-by: Stephen M. Cameron scame...@beardog.cce.hp.com
Reviewed-by: Stephen M. Cameron scame...@beardog.cce.hp.com
---
 drivers/scsi/hpsa.c |   16 
 1 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c
index a97a7ff..f937ea9 100644
--- a/drivers/scsi/hpsa.c
+++ b/drivers/scsi/hpsa.c
@@ -1708,7 +1708,14 @@ static void complete_scsi_command(struct CommandList *cp)
 
cmd-result |= ei-ScsiStatus;
 
-   /* copy the sense data whether we need to or not. */
+   scsi_set_resid(cmd, ei-ResidualCnt);
+   if (ei-CommandStatus == 0) {
+   cmd_free(h, cp);
+   cmd-scsi_done(cmd);
+   return;
+   }
+
+   /* copy the sense data */
if (SCSI_SENSE_BUFFERSIZE  sizeof(ei-SenseInfo))
sense_data_size = SCSI_SENSE_BUFFERSIZE;
else
@@ -1717,13 +1724,6 @@ static void complete_scsi_command(struct CommandList *cp)
sense_data_size = ei-SenseLen;
 
memcpy(cmd-sense_buffer, ei-SenseInfo, sense_data_size);
-   scsi_set_resid(cmd, ei-ResidualCnt);
-
-   if (ei-CommandStatus == 0) {
-   cmd_free(h, cp);
-   cmd-scsi_done(cmd);
-   return;
-   }
 
/* For I/O accelerator commands, copy over some fields to the normal
 * CISS header used below for error handling.

--
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


[PATCH 3/5] hpsa: make hpsa_init_one return -ENOMEM if allocation of h-lockup_detected fails

2014-07-03 Thread Stephen M. Cameron
From: Stephen M. Cameron scame...@beardog.cce.hp.com

Signed-off-by: Stephen M. Cameron scame...@beardog.cce.hp.com
Reviewed-by: Joe Handzik joseph.t.hand...@hp.com
---
 drivers/scsi/hpsa.c |4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c
index 08b34e9..794d726 100644
--- a/drivers/scsi/hpsa.c
+++ b/drivers/scsi/hpsa.c
@@ -6999,8 +6999,10 @@ reinit_after_soft_reset:
 
/* Allocate and clear per-cpu variable lockup_detected */
h-lockup_detected = alloc_percpu(u32);
-   if (!h-lockup_detected)
+   if (!h-lockup_detected) {
+   rc = -ENOMEM;
goto clean1;
+   }
set_lockup_detected_for_all_cpus(h, 0);
 
rc = hpsa_pci_init(h);

--
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] scsi: break from queue depth adjusting loops when device found

2014-07-03 Thread Douglas Gilbert

On 14-07-03 11:05 AM, Stephen M. Cameron wrote:

From: Stephen M. Cameron scame...@beardog.cce.hp.com

Don't loop through all the devices even after
finding the one we're looking for

Signed-off-by: Stephen M. Cameron scame...@beardog.cce.hp.com
Reviewed-by: Robert Elliott elli...@hp.com


With my scsi_debug testing on Christoph's core-for-3.17
tree I can simulate infrequent TSF conditions using the
every_nth mechanism. When one is triggered, all scsi
devices (that scsi_debug is monitoring) get their
queue_depth adjusted.

So:
Acked-by: Douglas Gilbert dgilb...@interlog.com



---
  drivers/scsi/scsi_error.c |2 ++
  1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c
index cbe38e5..db8a488 100644
--- a/drivers/scsi/scsi_error.c
+++ b/drivers/scsi/scsi_error.c
@@ -635,6 +635,7 @@ static void scsi_handle_queue_ramp_up(struct scsi_device 
*sdev)
sht-change_queue_depth(tmp_sdev, tmp_sdev-queue_depth + 1,
SCSI_QDEPTH_RAMP_UP);
sdev-last_queue_ramp_up = jiffies;
+   break;
}
  }

@@ -657,6 +658,7 @@ static void scsi_handle_queue_full(struct scsi_device *sdev)
 */
sht-change_queue_depth(tmp_sdev, tmp_sdev-queue_depth - 1,
SCSI_QDEPTH_QFULL);
+   break;
}
  }


--
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



--
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: [RFC][PATCH 1/5 v2] tracing: Add trace_seq_buffer_ptr() helper function

2014-07-03 Thread Steven Rostedt
On Thu, 26 Jun 2014 20:14:15 -0700
James Bottomley james.bottom...@hansenpartnership.com wrote:

 On Thu, 2014-06-26 at 21:06 -0400, Steven Rostedt wrote:
  As this patch is in my 3.17 queue and it touches the kvm and scsi
  tracepoint code, I figured I should at least do the courtesy of
  notifying the maintainers of those subsystems.
  
  Do you have any issues with this going through my tree? If not, please
  give me an Acked-by.
 
 Missing cc to linux-scsi added.
 
 Hannes has a set of patches to move logging to trace points.  I don't
 think there's a clash but I'd like him to comment before you do this.

I'm about to push this out. I already tested it fully at my end. Is
there any objection to this change?

I'll post the patches but I'll wait till I get a reply before I push it
to my for-next branch. I try not to rebase that branch. If there's an
issue, I'll have to revert the scsi changes.

-- Steve

 
 James
 
 
  Thanks!
  
  -- Steve
  
  On Thu, 26 Jun 2014 17:49:02 -0400
  Steven Rostedt rost...@goodmis.org wrote:
  
   From: Steven Rostedt (Red Hat) rost...@goodmis.org
   
   There's several locations in the kernel that open code the calculation
   of the next location in the trace_seq buffer. This is usually done with
   
 p-buffer + p-len
   
   Instead of having this open coded, supply a helper function in the
   header to do it for them. This function is called trace_seq_buffer_ptr().
   
   Signed-off-by: Steven Rostedt rost...@goodmis.org
   ---
arch/x86/kvm/mmutrace.h |  2 +-
drivers/scsi/scsi_trace.c   | 16 
include/linux/trace_seq.h   | 15 +++
kernel/trace/trace_output.c | 14 +++---
4 files changed, 31 insertions(+), 16 deletions(-)
   
   diff --git a/arch/x86/kvm/mmutrace.h b/arch/x86/kvm/mmutrace.h
   index 9d2e0ffcb190..2e5652b62fd6 100644
   --- a/arch/x86/kvm/mmutrace.h
   +++ b/arch/x86/kvm/mmutrace.h
   @@ -22,7 +22,7 @@
 __entry-unsync = sp-unsync;

#define KVM_MMU_PAGE_PRINTK() ({ \
   - const char *ret = p-buffer + p-len;   \
   + const char *ret = trace_seq_buffer_ptr(p);  \
 static const char *access_str[] = { \
 ---, --x, w--, w-x, -u-, -ux, wu-, wux  \
 };  \
   diff --git a/drivers/scsi/scsi_trace.c b/drivers/scsi/scsi_trace.c
   index 2bea4f0b684a..503594e5f76d 100644
   --- a/drivers/scsi/scsi_trace.c
   +++ b/drivers/scsi/scsi_trace.c
   @@ -28,7 +28,7 @@ scsi_trace_misc(struct trace_seq *, unsigned char *, 
   int);
static const char *
scsi_trace_rw6(struct trace_seq *p, unsigned char *cdb, int len)
{
   - const char *ret = p-buffer + p-len;
   + const char *ret = trace_seq_buffer_ptr(p);
 sector_t lba = 0, txlen = 0;

 lba |= ((cdb[1]  0x1F)  16);
   @@ -46,7 +46,7 @@ scsi_trace_rw6(struct trace_seq *p, unsigned char *cdb, 
   int len)
static const char *
scsi_trace_rw10(struct trace_seq *p, unsigned char *cdb, int len)
{
   - const char *ret = p-buffer + p-len;
   + const char *ret = trace_seq_buffer_ptr(p);
 sector_t lba = 0, txlen = 0;

 lba |= (cdb[2]  24);
   @@ -71,7 +71,7 @@ scsi_trace_rw10(struct trace_seq *p, unsigned char 
   *cdb, int len)
static const char *
scsi_trace_rw12(struct trace_seq *p, unsigned char *cdb, int len)
{
   - const char *ret = p-buffer + p-len;
   + const char *ret = trace_seq_buffer_ptr(p);
 sector_t lba = 0, txlen = 0;

 lba |= (cdb[2]  24);
   @@ -94,7 +94,7 @@ scsi_trace_rw12(struct trace_seq *p, unsigned char 
   *cdb, int len)
static const char *
scsi_trace_rw16(struct trace_seq *p, unsigned char *cdb, int len)
{
   - const char *ret = p-buffer + p-len;
   + const char *ret = trace_seq_buffer_ptr(p);
 sector_t lba = 0, txlen = 0;

 lba |= ((u64)cdb[2]  56);
   @@ -125,7 +125,7 @@ scsi_trace_rw16(struct trace_seq *p, unsigned char 
   *cdb, int len)
static const char *
scsi_trace_rw32(struct trace_seq *p, unsigned char *cdb, int len)
{
   - const char *ret = p-buffer + p-len, *cmd;
   + const char *ret = trace_seq_buffer_ptr(p), *cmd;
 sector_t lba = 0, txlen = 0;
 u32 ei_lbrt = 0;

   @@ -180,7 +180,7 @@ out:
static const char *
scsi_trace_unmap(struct trace_seq *p, unsigned char *cdb, int len)
{
   - const char *ret = p-buffer + p-len;
   + const char *ret = trace_seq_buffer_ptr(p);
 unsigned int regions = cdb[7]  8 | cdb[8];

 trace_seq_printf(p, regions=%u, (regions - 8) / 16);
   @@ -192,7 +192,7 @@ scsi_trace_unmap(struct trace_seq *p, unsigned char 
   *cdb, int len)
static const char *
scsi_trace_service_action_in(struct trace_seq *p, unsigned char *cdb, 
   int len)
{
   - const char *ret = p-buffer + p-len, *cmd;
   + const char *ret = trace_seq_buffer_ptr(p), *cmd;
 sector_t lba = 0;
 u32 alloc_len = 

Re: [PATCH 3.15 099/139] libiscsi, iser: Adjust data_length to include protection information

2014-07-03 Thread Greg Kroah-Hartman
On Thu, Jul 03, 2014 at 06:54:05AM -0700, James Bottomley wrote:
 On Wed, 2014-07-02 at 23:05 -0700, Christoph Hellwig wrote:
  On Wed, Jul 02, 2014 at 03:52:35PM -0700, James Bottomley wrote:
   In which case it's not marked for stable backport ... I assume it should
   be?
  
  Only if the patch that regression went to stable, which it shouldn't,
 
 Well it did:
 
 https://git.kernel.org/cgit/linux/kernel/git/stable/linux-stable.git/commit/drivers/scsi?h=linux-3.15.yid=04423ddea30a7fb7232636eda8aed55ea5b972fe
 
 The stable process is a bit broken from my point of view since it
 doesn't copy linux-scsi on SCSI commits

I copy everyone on the patch.  You were not on that patch as it doesn't
look like it went through your tree at all.

 ... but this time even I didn't get a cc, so really no-one who knew
 there was a problem had a chance to comment.

The target people (Nicholas) asked for the patch to be applied, they
knew what was going on here.

 The choice is either request revert from stable or add fix.

Agreed, just let me know.

greg k-h
--
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 3.15 099/139] libiscsi, iser: Adjust data_length to include protection information

2014-07-03 Thread Christoph Hellwig
On Thu, Jul 03, 2014 at 06:54:05AM -0700, James Bottomley wrote:
 Well it did:
 
 https://git.kernel.org/cgit/linux/kernel/git/stable/linux-stable.git/commit/drivers/scsi?h=linux-3.15.yid=04423ddea30a7fb7232636eda8aed55ea5b972fe
 
 The stable process is a bit broken from my point of view since it
 doesn't copy linux-scsi on SCSI commits ... but this time even I didn't
 get a cc, so really no-one who knew there was a problem had a chance to
 comment.
 
 The choice is either request revert from stable or add fix.

Unfortunately the fix hasn't even made it to mainline yet, despite lots
of people tripping over it.  If you're too busy I'm happy to send it to
Linus, it's been sitting in my tree for over a week now.

So if we can get it in ASAP we can also send it to stable, otherwise
a revert is the only option.

--
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 3.15 099/139] libiscsi, iser: Adjust data_length to include protection information

2014-07-03 Thread Christoph Hellwig
On Thu, Jul 03, 2014 at 09:27:48AM -0700, Greg Kroah-Hartman wrote:
 I copy everyone on the patch.  You were not on that patch as it doesn't
 look like it went through your tree at all.

It went through the target tree despite exclusive touching scsi
initiator side code.  I'm still not sure how this happened, but we
should take care to avoid this in the future.  In addition to this
regression due to a complete lack of testing it also caused various
merge issues.

--
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: SCSI eats error from flush failure during hot plug

2014-07-03 Thread Steven Haber
Both patches work fine -- zero uncommitted writes over several hundred
power cycles.

On Tue, Jul 1, 2014 at 1:15 AM, Christoph Hellwig h...@infradead.org wrote:
 On Mon, Jun 30, 2014 at 11:45:31AM -0700, Steven Haber wrote:
 Our qual rig is having some woes. I'll let you know how the test works
 out after I finish resuscitating it.

 Might be worth to test that patch James send in reply then.  They should
 have an identical effect and James seems to prefer the version testing
 for the length.

--
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] scsi: break from queue depth adjusting loops when device found

2014-07-03 Thread Christoph Hellwig
On Thu, Jul 03, 2014 at 10:05:57AM -0500, Stephen M. Cameron wrote:
 From: Stephen M. Cameron scame...@beardog.cce.hp.com
 
 Don't loop through all the devices even after
 finding the one we're looking for

The comments in the code seem to indicate that we want to modify
the queue depth for all LUNs on a given target.

Ccing Mike and Vasu as they wrote this code.

--
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: SCSI eats error from flush failure during hot plug

2014-07-03 Thread Christoph Hellwig
On Thu, Jul 03, 2014 at 09:57:32AM -0700, Steven Haber wrote:
 Both patches work fine -- zero uncommitted writes over several hundred
 power cycles.

Thanks for the testing.

Below is James' patch with a trivial comment fix and a proper changelog.
James, can I get your signoff for this one?  I'll throw it into the
core-for-3.16 tree then.

---
From 92510736caa3b14a964af981ba288950042e79e3 Mon Sep 17 00:00:00 2001
From: James Bottomley james.bottom...@hansenpartnership.com
Date: Thu, 3 Jul 2014 19:17:34 +0200
Subject: scsi: handle flush errors properly

Flush commands don't transfer data and thus need to be special cased
in the I/O completion handler so that we can propagate errors to
the block layer and filesystem.

Reported-by: Steven Haber ste...@qumulo.com
Tested-by: Steven Haber ste...@qumulo.com
---
 drivers/scsi/scsi_lib.c |8 
 1 file changed, 8 insertions(+)

diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index f7e3163..143d0e2 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -733,6 +733,14 @@ void scsi_io_completion(struct scsi_cmnd *cmd, unsigned 
int good_bytes)
scsi_next_command(cmd);
return;
}
+   } else if (blk_rq_bytes(req) == 0  result  !sense_deferred) {
+   /*
+* Certain non BLOCK_PC requests are commands that don't
+* actually transfer anything (FLUSH), so cannot use
+* good_bytes == 0 as the signal for an error.  This sets the
+* error explicitly for the problem case.
+*/
+   error = __scsi_error_from_host_byte(cmd, result);
}
 
/* no bidi support for !REQ_TYPE_BLOCK_PC yet */
-- 
1.7.10.4

--
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] scsi: break from queue depth adjusting loops when device found

2014-07-03 Thread Mike Christie
On 07/03/2014 12:11 PM, Christoph Hellwig wrote:
 On Thu, Jul 03, 2014 at 10:05:57AM -0500, Stephen M. Cameron wrote:
 From: Stephen M. Cameron scame...@beardog.cce.hp.com

 Don't loop through all the devices even after
 finding the one we're looking for
 
 The comments in the code seem to indicate that we want to modify
 the queue depth for all LUNs on a given target.
 
 Ccing Mike and Vasu as they wrote this code.
 

Ccing James Smart for more details.

We copied this behavior from lpfc's SAM_STAT_TASK_SET_FULL handling
(look at around the 2.6.32 kernel for reference).
--
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 0/5] hpsa a few small updates for early July 2014

2014-07-03 Thread Christoph Hellwig
Thanks Stephen,

applied to the for 3.17 queue.

--
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 0/3] bnx2i/bnx2fc: Rebranding changes for bnx2i and bnx2fc driver

2014-07-03 Thread Christoph Hellwig
Thanks Vikas,

I've applied this to my drivers queue for 3.17, which I will push out
publically as soon the current set of 3.16 updates has hit Linus' tree.

--
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] drivers: message: fusion: Simplify rounding

2014-07-03 Thread Joe Lawrence
ALIGN is certainly more readable to me.

Reviewed-by: Joe Lawrence joe.lawre...@stratus.com

-- Joe

On Tue, 1 Jul 2014, Rasmus Villemoes wrote:

 Rounding up to a multiple of 4 should be done using the ALIGN
 macro. As a bonus, this also makes the generated code smaller.
 
 In GetIocFacts(), sz is assigned to a few lines below without being
 read in the meantime, so it is ok that it doesn't end up with the same
 value as facts-FWImageSize.
 
 Signed-off-by: Rasmus Villemoes li...@rasmusvillemoes.dk
 ---
  drivers/message/fusion/mptbase.c | 7 +--
  drivers/message/fusion/mptctl.c  | 7 +--
  2 files changed, 2 insertions(+), 12 deletions(-)
 
 diff --git a/drivers/message/fusion/mptbase.c 
 b/drivers/message/fusion/mptbase.c
 index ebc0af7..10b16a1 100644
 --- a/drivers/message/fusion/mptbase.c
 +++ b/drivers/message/fusion/mptbase.c
 @@ -3175,12 +3175,7 @@ GetIocFacts(MPT_ADAPTER *ioc, int sleepFlag, int 
 reason)
   facts-FWImageSize = le32_to_cpu(facts-FWImageSize);
   }
  
 - sz = facts-FWImageSize;
 - if ( sz  0x01 )
 - sz += 1;
 - if ( sz  0x02 )
 - sz += 2;
 - facts-FWImageSize = sz;
 + facts-FWImageSize = ALIGN(facts-FWImageSize, 4);
  
   if (!facts-RequestFrameSize) {
   /*  Something is wrong!  */
 diff --git a/drivers/message/fusion/mptctl.c b/drivers/message/fusion/mptctl.c
 index 8a050e8..1004392 100644
 --- a/drivers/message/fusion/mptctl.c
 +++ b/drivers/message/fusion/mptctl.c
 @@ -1749,12 +1749,7 @@ mptctl_replace_fw (unsigned long arg)
  
   /* Allocate memory for the new FW image
*/
 - newFwSize = karg.newImageSize;
 -
 - if (newFwSize  0x01)
 - newFwSize += 1;
 - if (newFwSize  0x02)
 - newFwSize += 2;
 + newFwSize = ALIGN(karg.newImageSize, 4);
  
   mpt_alloc_fw_memory(ioc, newFwSize);
   if (ioc-cached_fw == NULL)
 -- 
 1.9.2
 
 --
 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
 
--
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 3.15 099/139] libiscsi, iser: Adjust data_length to include protection information

2014-07-03 Thread James Bottomley
On Thu, 2014-07-03 at 09:33 -0700, Christoph Hellwig wrote:
 On Thu, Jul 03, 2014 at 09:27:48AM -0700, Greg Kroah-Hartman wrote:
  I copy everyone on the patch.  You were not on that patch as it doesn't
  look like it went through your tree at all.
 
 It went through the target tree despite exclusive touching scsi
 initiator side code.  I'm still not sure how this happened, but we
 should take care to avoid this in the future.  In addition to this
 regression due to a complete lack of testing it also caused various
 merge issues.

OK, Nic cc'd

Please explain how this commit:

commit d77e65350f2d82dfa0557707d505711f5a43c8fd
Author: Sagi Grimberg sa...@mellanox.com
Date:   Wed Jun 11 12:09:58 2014 +0300

libiscsi, iser: Adjust data_length to include protection information

In case protection information exists over the wire
iscsi header data length is required to include it.
Use protection information aware scsi helpers to set
the correct transfer length.

In order to avoid breakage, remove iser transfer length
checks for each task as they are not always true and
somewhat redundant anyway.

Signed-off-by: Sagi Grimberg sa...@mellanox.com
Reviewed-by: Mike Christie micha...@cs.wisc.edu
Acked-by: Mike Christie micha...@cs.wisc.edu
Cc: sta...@vger.kernel.org # 3.15+
Signed-off-by: Nicholas Bellinger n...@linux-iscsi.org

 drivers/infiniband/ulp/iser/iser_initiator.c | 34 
 drivers/scsi/libiscsi.c  | 18 +++

Came to go through the target tree even though it's initiator only and
how come it was tagged for stable?

James


--
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] scsi: mpt2sas: mpt2sas_base.c: Fix for possible null pointer dereference

2014-07-03 Thread Joe Lawrence
On Wed, 28 May 2014, Christoph Hellwig wrote:

  -   ioc_status = le16_to_cpu(mpi_reply-IOCStatus)  MPI2_IOCSTATUS_MASK;
  +   if (mpi_reply) {
  +   ioc_status = le16_to_cpu(mpi_reply-IOCStatus)  
  MPI2_IOCSTATUS_MASK;
  +   }
   
  if (ioc_status != MPI2_IOCSTATUS_SUCCESS)
  ioc-port_enable_failed = 1;
 
 ioc_status isn't initialized without the reply and used here as well
 as later in the function.  I think we'll need input from LSI or others
 with the spec on what to do when we didn't get a reply.

Any update on this? 

The mpt3 version checks for !mpi_reply and returns 1.  Which leads to 
another question -- should mpt{2,3}sas_port_enable_done ever return 0 (as their 
respective comments describe)?

Regards,

-- Joe
--
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] scsi: mpt2sas: mpt2sas_base.c: Fix for possible null pointer dereference

2014-07-03 Thread Rickard Strandqvist
2014-07-03 19:45 GMT+02:00 Joe Lawrence joe.lawre...@stratus.com:
 On Wed, 28 May 2014, Christoph Hellwig wrote:

  -   ioc_status = le16_to_cpu(mpi_reply-IOCStatus)  MPI2_IOCSTATUS_MASK;
  +   if (mpi_reply) {
  +   ioc_status = le16_to_cpu(mpi_reply-IOCStatus)  
  MPI2_IOCSTATUS_MASK;
  +   }
 
  if (ioc_status != MPI2_IOCSTATUS_SUCCESS)
  ioc-port_enable_failed = 1;

 ioc_status isn't initialized without the reply and used here as well
 as later in the function.  I think we'll need input from LSI or others
 with the spec on what to do when we didn't get a reply.

 Any update on this?

 The mpt3 version checks for !mpi_reply and returns 1.  Which leads to
 another question -- should mpt{2,3}sas_port_enable_done ever return 0 (as 
 their
 respective comments describe)?

 Regards,

 -- Joe


Hi

Good questions Joe!
And good someone else brought this up, because I guess it's not meant to me.

And it looks however now that I've done quite a few more patches that
there should not have been any {}
And then, it is perhaps good to sett a start value for ioc_status. My
suggestion is:

u16 ioc_status = MPI2_IOCSTATUS_OP_STATE_NOT_SUPPORTED;


Kind regards
Rickard Strandqvist
--
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 3/4] megaraid_sas: Fix LD/VF affiliation parsing

2014-07-03 Thread adam radford
On Thu, Jul 3, 2014 at 1:18 AM, Christoph Hellwig h...@infradead.org wrote:
 On Mon, Jun 30, 2014 at 05:19:57PM -0700, Adam Radford wrote:
 James/linux-scsi,

 The following patch for megaraid_sas fixes the LD/VF affiliation policy 
 parsing code to account for LD targetId's and Hidden LD's (not yet 
 affiliated with any Virtual Functions).

 Hi Adam,

 this doesn't address Martins comment about removing the initial != 0
 case.  And while we're at it there basically is no significant shared
 code between the PlasmaFW111 and !PlasmaFW111, so split these into
 two different functions.  With those two levels of indentation removed
 the code might actually become readable, too.

 Also please remove the James/linux-scsi, address in the individual
 patches, that's something that should just go into the cover letter,
 and make sure to include all Reviewed-by: and Acked-by: tags when
 resending already reviewd patches.

 Thanks,
 Christoph

Christoph,

Thanks for your and Martin's feedback.

Actually there is a missing call to megasas_get_ld_vf_affiliation()
with initial = 1 that should be at the bottom of megasas_probe_one().
This got dropped inadvertently during a manual merge.  I will add that
and split the megasas_get_ld_vf_affiliation() function into 2
functions (as you suggested) and resubmit the patch series while
including all currently received Reviewed-by and Acked-by messages
for the other patches and dropping the James/linux-scsi text.

-Adam
--
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