Re: [PATCH 37/45] drivers: use req op accessor

2016-06-06 Thread Hannes Reinecke
On 06/05/2016 09:32 PM, mchri...@redhat.com wrote:
> From: Mike Christie <mchri...@redhat.com>
> 
> The req operation REQ_OP is separated from the rq_flag_bits
> definition. This converts the block layer drivers to
> use req_op to get the op from the request struct.
> 
> Signed-off-by: Mike Christie <mchri...@redhat.com>
> ---
>  drivers/block/loop.c  |  6 +++---
>  drivers/block/mtip32xx/mtip32xx.c |  2 +-
>  drivers/block/nbd.c   |  2 +-
>  drivers/block/rbd.c   |  4 ++--
>  drivers/block/xen-blkfront.c  |  8 +---
>  drivers/ide/ide-floppy.c  |  2 +-
>  drivers/md/dm.c   |  2 +-
>  drivers/mmc/card/block.c  |  7 +++
>  drivers/mmc/card/queue.c  |  6 ++
>  drivers/mmc/card/queue.h  |  5 -
>  drivers/mtd/mtd_blkdevs.c |  2 +-
>  drivers/nvme/host/core.c  |  2 +-
>  drivers/nvme/host/nvme.h  |  4 ++--
>  drivers/scsi/sd.c | 25 -----
>  14 files changed, 43 insertions(+), 34 deletions(-)
> 
Reviewed-by: Hannes Reinecke <h...@suse.com>

Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)


Re: [PATCH 37/45] drivers: use req op accessor

2016-06-06 Thread Hannes Reinecke
On 06/05/2016 09:32 PM, mchri...@redhat.com wrote:
> From: Mike Christie 
> 
> The req operation REQ_OP is separated from the rq_flag_bits
> definition. This converts the block layer drivers to
> use req_op to get the op from the request struct.
> 
> Signed-off-by: Mike Christie 
> ---
>  drivers/block/loop.c  |  6 +++---
>  drivers/block/mtip32xx/mtip32xx.c |  2 +-
>  drivers/block/nbd.c   |  2 +-
>  drivers/block/rbd.c   |  4 ++--
>  drivers/block/xen-blkfront.c  |  8 +---
>  drivers/ide/ide-floppy.c  |  2 +-
>  drivers/md/dm.c   |  2 +-
>  drivers/mmc/card/block.c  |  7 +++
>  drivers/mmc/card/queue.c  |  6 ++
>  drivers/mmc/card/queue.h  |  5 -
>  drivers/mtd/mtd_blkdevs.c |  2 +-
>  drivers/nvme/host/core.c  |  2 +-
>  drivers/nvme/host/nvme.h  |  4 ++--
>  drivers/scsi/sd.c | 25 -
>  14 files changed, 43 insertions(+), 34 deletions(-)
> 
Reviewed-by: Hannes Reinecke 

Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)


Re: [PATCH 28/45] target: use bio op accessors

2016-06-06 Thread Hannes Reinecke
On 06/05/2016 09:32 PM, mchri...@redhat.com wrote:
> From: Mike Christie <mchri...@redhat.com>
> 
> Separate the op from the rq_flag_bits and have the target layer
> set/get the bio using bio_set_op_attrs/bio_op.
> 
> Signed-off-by: Mike Christie <mchri...@redhat.com>
> ---
>  drivers/target/target_core_iblock.c | 29 ++---
>  drivers/target/target_core_pscsi.c  |  2 +-
>  2 files changed, 15 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/target/target_core_iblock.c 
> b/drivers/target/target_core_iblock.c
> index c25109c..22af12f 100644
> --- a/drivers/target/target_core_iblock.c
> +++ b/drivers/target/target_core_iblock.c
[ .. ]
> @@ -689,18 +690,15 @@ iblock_execute_rw(struct se_cmd *cmd, struct 
> scatterlist *sgl, u32 sgl_nents,
>* Force writethrough using WRITE_FUA if a volatile write cache
>* is not enabled, or if initiator set the Force Unit Access 
> bit.
>*/
> + op = REQ_OP_WRITE;
>   if (test_bit(QUEUE_FLAG_FUA, >queue_flags)) {
>   if (cmd->se_cmd_flags & SCF_FUA)
> - rw = WRITE_FUA;
> + op_flags = WRITE_FUA;
>   else if (!test_bit(QUEUE_FLAG_WC, >queue_flags))
> - rw = WRITE_FUA;
> - else
> - rw = WRITE;
> - } else {
> - rw = WRITE;
> + op_flags = WRITE_FUA;
>   }
Wrong intendation.

>   } else {
> - rw = READ;
> + op = REQ_OP_READ;
>   }
>  
>   ibr = kzalloc(sizeof(struct iblock_req), GFP_KERNEL);
> @@ -714,7 +712,7 @@ iblock_execute_rw(struct se_cmd *cmd, struct scatterlist 
> *sgl, u32 sgl_nents,
>   return 0;
>   }
>  
> - bio = iblock_get_bio(cmd, block_lba, sgl_nents, rw);
> + bio = iblock_get_bio(cmd, block_lba, sgl_nents, op, op_flags);
>   if (!bio)
>   goto fail_free_ibr;
>  
> @@ -738,7 +736,8 @@ iblock_execute_rw(struct se_cmd *cmd, struct scatterlist 
> *sgl, u32 sgl_nents,
>   bio_cnt = 0;
>   }
>  
> - bio = iblock_get_bio(cmd, block_lba, sg_num, rw);
> + bio = iblock_get_bio(cmd, block_lba, sg_num, op,
> +  op_flags);
>   if (!bio)
>   goto fail_put_bios;
>  
> diff --git a/drivers/target/target_core_pscsi.c 
> b/drivers/target/target_core_pscsi.c
> index de18790..81564c8 100644
> --- a/drivers/target/target_core_pscsi.c
> +++ b/drivers/target/target_core_pscsi.c
> @@ -922,7 +922,7 @@ pscsi_map_sg(struct se_cmd *cmd, struct scatterlist *sgl, 
> u32 sgl_nents,
>   goto fail;
>  
>   if (rw)
> - bio->bi_rw |= REQ_WRITE;
> + bio_set_op_attrs(bio, REQ_OP_WRITE, 0);
>  
>   pr_debug("PSCSI: Allocated bio: %p,"
>   " dir: %s nr_vecs: %d\n", bio,
> 
Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)


Re: [PATCH 28/45] target: use bio op accessors

2016-06-06 Thread Hannes Reinecke
On 06/05/2016 09:32 PM, mchri...@redhat.com wrote:
> From: Mike Christie 
> 
> Separate the op from the rq_flag_bits and have the target layer
> set/get the bio using bio_set_op_attrs/bio_op.
> 
> Signed-off-by: Mike Christie 
> ---
>  drivers/target/target_core_iblock.c | 29 ++---
>  drivers/target/target_core_pscsi.c  |  2 +-
>  2 files changed, 15 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/target/target_core_iblock.c 
> b/drivers/target/target_core_iblock.c
> index c25109c..22af12f 100644
> --- a/drivers/target/target_core_iblock.c
> +++ b/drivers/target/target_core_iblock.c
[ .. ]
> @@ -689,18 +690,15 @@ iblock_execute_rw(struct se_cmd *cmd, struct 
> scatterlist *sgl, u32 sgl_nents,
>* Force writethrough using WRITE_FUA if a volatile write cache
>* is not enabled, or if initiator set the Force Unit Access 
> bit.
>*/
> + op = REQ_OP_WRITE;
>   if (test_bit(QUEUE_FLAG_FUA, >queue_flags)) {
>   if (cmd->se_cmd_flags & SCF_FUA)
> - rw = WRITE_FUA;
> + op_flags = WRITE_FUA;
>   else if (!test_bit(QUEUE_FLAG_WC, >queue_flags))
> - rw = WRITE_FUA;
> - else
> - rw = WRITE;
> - } else {
> - rw = WRITE;
> + op_flags = WRITE_FUA;
>   }
Wrong intendation.

>   } else {
> - rw = READ;
> + op = REQ_OP_READ;
>   }
>  
>   ibr = kzalloc(sizeof(struct iblock_req), GFP_KERNEL);
> @@ -714,7 +712,7 @@ iblock_execute_rw(struct se_cmd *cmd, struct scatterlist 
> *sgl, u32 sgl_nents,
>   return 0;
>   }
>  
> - bio = iblock_get_bio(cmd, block_lba, sgl_nents, rw);
> + bio = iblock_get_bio(cmd, block_lba, sgl_nents, op, op_flags);
>   if (!bio)
>   goto fail_free_ibr;
>  
> @@ -738,7 +736,8 @@ iblock_execute_rw(struct se_cmd *cmd, struct scatterlist 
> *sgl, u32 sgl_nents,
>   bio_cnt = 0;
>   }
>  
> - bio = iblock_get_bio(cmd, block_lba, sg_num, rw);
> + bio = iblock_get_bio(cmd, block_lba, sg_num, op,
> +  op_flags);
>   if (!bio)
>   goto fail_put_bios;
>  
> diff --git a/drivers/target/target_core_pscsi.c 
> b/drivers/target/target_core_pscsi.c
> index de18790..81564c8 100644
> --- a/drivers/target/target_core_pscsi.c
> +++ b/drivers/target/target_core_pscsi.c
> @@ -922,7 +922,7 @@ pscsi_map_sg(struct se_cmd *cmd, struct scatterlist *sgl, 
> u32 sgl_nents,
>   goto fail;
>  
>   if (rw)
> - bio->bi_rw |= REQ_WRITE;
> +     bio_set_op_attrs(bio, REQ_OP_WRITE, 0);
>  
>   pr_debug("PSCSI: Allocated bio: %p,"
>   " dir: %s nr_vecs: %d\n", bio,
> 
Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)


Re: [PATCH 25/45] bcache: use bio op accessors

2016-06-06 Thread Hannes Reinecke
On 06/05/2016 09:32 PM, mchri...@redhat.com wrote:
> From: Mike Christie <mchri...@redhat.com>
> 
> Separate the op from the rq_flag_bits and have bcache
> set/get the bio using bio_set_op_attrs/bio_op.
> 
> Signed-off-by: Mike Christie <mchri...@redhat.com>
> ---
>  drivers/md/bcache/btree.c |  4 ++--
>  drivers/md/bcache/debug.c |  4 ++--
>  drivers/md/bcache/journal.c   |  7 ---
>  drivers/md/bcache/movinggc.c  |  2 +-
>  drivers/md/bcache/request.c   | 14 +++---
>  drivers/md/bcache/super.c | 24 +---
>  drivers/md/bcache/writeback.c |  4 ++--
>  7 files changed, 31 insertions(+), 28 deletions(-)
> 
Reviewed-by: Hannes Reinecke <h...@suse.com>

Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)


Re: [PATCH 25/45] bcache: use bio op accessors

2016-06-06 Thread Hannes Reinecke
On 06/05/2016 09:32 PM, mchri...@redhat.com wrote:
> From: Mike Christie 
> 
> Separate the op from the rq_flag_bits and have bcache
> set/get the bio using bio_set_op_attrs/bio_op.
> 
> Signed-off-by: Mike Christie 
> ---
>  drivers/md/bcache/btree.c |  4 ++--
>  drivers/md/bcache/debug.c |  4 ++--
>  drivers/md/bcache/journal.c   |  7 ---
>  drivers/md/bcache/movinggc.c  |  2 +-
>  drivers/md/bcache/request.c   | 14 +++---
>  drivers/md/bcache/super.c | 24 +---
>  drivers/md/bcache/writeback.c |  4 ++--
>  7 files changed, 31 insertions(+), 28 deletions(-)
> 
Reviewed-by: Hannes Reinecke 

Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)


Re: [PATCH 24/45] dm: use bio op accessors

2016-06-06 Thread Hannes Reinecke
On 06/05/2016 09:32 PM, mchri...@redhat.com wrote:
> From: Mike Christie <mchri...@redhat.com>
> 
> Separate the op from the rq_flag_bits and have dm
> set/get the bio using bio_set_op_attrs/bio_op.
> 
> Signed-off-by: Mike Christie <mchri...@redhat.com>
> 
Reviewed-by: Hannes Reinecke <h...@suse.com>

Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)


Re: [PATCH 24/45] dm: use bio op accessors

2016-06-06 Thread Hannes Reinecke
On 06/05/2016 09:32 PM, mchri...@redhat.com wrote:
> From: Mike Christie 
> 
> Separate the op from the rq_flag_bits and have dm
> set/get the bio using bio_set_op_attrs/bio_op.
> 
> Signed-off-by: Mike Christie 
> 
Reviewed-by: Hannes Reinecke 

Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)


Re: [PATCH 09/45] block discard: use bio set op accessor

2016-06-06 Thread Hannes Reinecke
On 06/05/2016 09:31 PM, mchri...@redhat.com wrote:
> From: Mike Christie <mchri...@redhat.com>
> 
> This converts the block issue discard helper and users to use
> the bio_set_op_attrs accessor and only pass in the operation flags
> like REQ_SEQURE.
> 
> Signed-off-by: Mike Christie <mchri...@redhat.com>
> ---
>  block/blk-lib.c| 13 +++--
>  drivers/md/dm-thin.c   |  2 +-
>  include/linux/blkdev.h |  3 ++-
>  3 files changed, 10 insertions(+), 8 deletions(-)
> 
Reviewed-by: Hannes Reinecke <h...@suse.com>

Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)


Re: [PATCH 09/45] block discard: use bio set op accessor

2016-06-06 Thread Hannes Reinecke
On 06/05/2016 09:31 PM, mchri...@redhat.com wrote:
> From: Mike Christie 
> 
> This converts the block issue discard helper and users to use
> the bio_set_op_attrs accessor and only pass in the operation flags
> like REQ_SEQURE.
> 
> Signed-off-by: Mike Christie 
> ---
>  block/blk-lib.c| 13 +++--
>  drivers/md/dm-thin.c   |  2 +-
>  include/linux/blkdev.h |  3 ++-
>  3 files changed, 10 insertions(+), 8 deletions(-)
> 
Reviewed-by: Hannes Reinecke 

Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)


Re: [PATCH 08/45] block, fs, mm, drivers: use bio set/get op accessors

2016-06-06 Thread Hannes Reinecke
On 06/05/2016 09:31 PM, mchri...@redhat.com wrote:
> From: Mike Christie <mchri...@redhat.com>
> 
> This patch converts the simple bi_rw use cases in the block,
> drivers, mm and fs code to set/get the bio operation using
> bio_set_op_attrs/bio_op
> 
> These should be simple one or two liner cases, so I just did them
> in one patch. The next patches handle the more complicated
> cases in a module per patch.
> 
> Signed-off-by: Mike Christie <mchri...@redhat.com>
> ---
> 
> v5:
> 1. Add missed crypto call.
> 2. Change nfs bi_rw check to bi_op.
> 
>  block/bio.c | 13 ++---
>  block/blk-core.c|  6 +++---
>  block/blk-flush.c   |  2 +-
>  block/blk-lib.c |  4 ++--
>  block/blk-map.c |  2 +-
>  block/blk-merge.c   | 12 ++--
>  drivers/block/brd.c |  2 +-
>  drivers/block/floppy.c  |  2 +-
>  drivers/block/pktcdvd.c |  4 ++--
>  drivers/block/rsxx/dma.c|  2 +-
>  drivers/block/zram/zram_drv.c   |  2 +-
>  drivers/lightnvm/rrpc.c |  6 +++---
>  drivers/scsi/osd/osd_initiator.c|  8 
>  drivers/staging/lustre/lustre/llite/lloop.c |  6 +++---
>  fs/crypto/crypto.c  |  2 +-
>  fs/exofs/ore.c  |  2 +-
>  fs/ext4/page-io.c   |  6 +++---
>  fs/ext4/readpage.c  |  2 +-
>  fs/jfs/jfs_logmgr.c |  4 ++--
>  fs/jfs/jfs_metapage.c   |  4 ++--
>  fs/logfs/dev_bdev.c | 12 ++--
>  fs/nfs/blocklayout/blocklayout.c|  4 ++--
>  include/linux/bio.h | 15 ++-
>  mm/page_io.c|  4 ++--
>  24 files changed, 65 insertions(+), 61 deletions(-)
> 
[ .. ]
> diff --git a/include/linux/bio.h b/include/linux/bio.h
> index 09c5308..4568647 100644
> --- a/include/linux/bio.h
> +++ b/include/linux/bio.h
> @@ -109,18 +109,23 @@ static inline bool bio_has_data(struct bio *bio)
>  {
>   if (bio &&
>   bio->bi_iter.bi_size &&
> - !(bio->bi_rw & REQ_DISCARD))
> + bio_op(bio) != REQ_OP_DISCARD)
>   return true;
>  
>   return false;
>  }
>  
> +static inline bool bio_no_advance_iter(struct bio *bio)
> +{
> + return bio_op(bio) == REQ_OP_DISCARD || bio_op(bio) == 
> REQ_OP_WRITE_SAME;
> +}
> +
>  static inline bool bio_is_rw(struct bio *bio)
>  {
>   if (!bio_has_data(bio))
>   return false;
>  
> - if (bio->bi_rw & BIO_NO_ADVANCE_ITER_MASK)
> + if (bio_no_advance_iter(bio))
>   return false;
>  
>   return true;
> @@ -228,7 +233,7 @@ static inline void bio_advance_iter(struct bio *bio, 
> struct bvec_iter *iter,
>  {
>   iter->bi_sector += bytes >> 9;
>  
> - if (bio->bi_rw & BIO_NO_ADVANCE_ITER_MASK)
> + if (bio_no_advance_iter(bio))
>   iter->bi_size -= bytes;
>   else
>   bvec_iter_advance(bio->bi_io_vec, iter, bytes);
Hmm. Can't you drop 'BIO_NO_ADVANCE_ITER_MASK' after this patch?

> @@ -256,10 +261,10 @@ static inline unsigned bio_segments(struct bio *bio)
>* differently:
>*/
>  
> - if (bio->bi_rw & REQ_DISCARD)
> + if (bio_op(bio) == REQ_OP_DISCARD)
>   return 1;
>  
> - if (bio->bi_rw & REQ_WRITE_SAME)
> + if (bio_op(bio) == REQ_OP_WRITE_SAME)
>   return 1;
>  
>   bio_for_each_segment(bv, bio, iter)
> diff --git a/mm/page_io.c b/mm/page_io.c
> index 5a5fd66..dcc5d37 100644
> --- a/mm/page_io.c
> +++ b/mm/page_io.c
> @@ -317,7 +317,7 @@ int __swap_writepage(struct page *page, struct 
> writeback_control *wbc,
>   ret = -ENOMEM;
>   goto out;
>   }
> - bio->bi_rw = WRITE;
> + bio_set_op_attrs(bio, REQ_OP_WRITE, 0);
>   if (wbc->sync_mode == WB_SYNC_ALL)
>   bio->bi_rw |= REQ_SYNC;
>   count_vm_event(PSWPOUT);
> @@ -370,7 +370,7 @@ int swap_readpage(struct page *page)
>   ret = -ENOMEM;
>   goto out;
>   }
> - bio->bi_rw = READ;
> + bio_set_op_attrs(bio, REQ_OP_READ, 0);
>   count_vm_event(PSWPIN);
>   submit_bio(bio);
>  out:
> 
Reviewed-by: Hannes Reinecke <h...@suse.com>

Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)


Re: [PATCH 08/45] block, fs, mm, drivers: use bio set/get op accessors

2016-06-06 Thread Hannes Reinecke
On 06/05/2016 09:31 PM, mchri...@redhat.com wrote:
> From: Mike Christie 
> 
> This patch converts the simple bi_rw use cases in the block,
> drivers, mm and fs code to set/get the bio operation using
> bio_set_op_attrs/bio_op
> 
> These should be simple one or two liner cases, so I just did them
> in one patch. The next patches handle the more complicated
> cases in a module per patch.
> 
> Signed-off-by: Mike Christie 
> ---
> 
> v5:
> 1. Add missed crypto call.
> 2. Change nfs bi_rw check to bi_op.
> 
>  block/bio.c | 13 ++---
>  block/blk-core.c|  6 +++---
>  block/blk-flush.c   |  2 +-
>  block/blk-lib.c |  4 ++--
>  block/blk-map.c |  2 +-
>  block/blk-merge.c   | 12 ++--
>  drivers/block/brd.c |  2 +-
>  drivers/block/floppy.c  |  2 +-
>  drivers/block/pktcdvd.c |  4 ++--
>  drivers/block/rsxx/dma.c|  2 +-
>  drivers/block/zram/zram_drv.c   |  2 +-
>  drivers/lightnvm/rrpc.c |  6 +++---
>  drivers/scsi/osd/osd_initiator.c|  8 
>  drivers/staging/lustre/lustre/llite/lloop.c |  6 +++---
>  fs/crypto/crypto.c  |  2 +-
>  fs/exofs/ore.c  |  2 +-
>  fs/ext4/page-io.c   |  6 +++---
>  fs/ext4/readpage.c  |  2 +-
>  fs/jfs/jfs_logmgr.c |  4 ++--
>  fs/jfs/jfs_metapage.c   |  4 ++--
>  fs/logfs/dev_bdev.c | 12 ++--
>  fs/nfs/blocklayout/blocklayout.c|  4 ++--
>  include/linux/bio.h | 15 ++-
>  mm/page_io.c|  4 ++--
>  24 files changed, 65 insertions(+), 61 deletions(-)
> 
[ .. ]
> diff --git a/include/linux/bio.h b/include/linux/bio.h
> index 09c5308..4568647 100644
> --- a/include/linux/bio.h
> +++ b/include/linux/bio.h
> @@ -109,18 +109,23 @@ static inline bool bio_has_data(struct bio *bio)
>  {
>   if (bio &&
>   bio->bi_iter.bi_size &&
> - !(bio->bi_rw & REQ_DISCARD))
> + bio_op(bio) != REQ_OP_DISCARD)
>   return true;
>  
>   return false;
>  }
>  
> +static inline bool bio_no_advance_iter(struct bio *bio)
> +{
> + return bio_op(bio) == REQ_OP_DISCARD || bio_op(bio) == 
> REQ_OP_WRITE_SAME;
> +}
> +
>  static inline bool bio_is_rw(struct bio *bio)
>  {
>   if (!bio_has_data(bio))
>   return false;
>  
> - if (bio->bi_rw & BIO_NO_ADVANCE_ITER_MASK)
> + if (bio_no_advance_iter(bio))
>   return false;
>  
>   return true;
> @@ -228,7 +233,7 @@ static inline void bio_advance_iter(struct bio *bio, 
> struct bvec_iter *iter,
>  {
>   iter->bi_sector += bytes >> 9;
>  
> - if (bio->bi_rw & BIO_NO_ADVANCE_ITER_MASK)
> + if (bio_no_advance_iter(bio))
>   iter->bi_size -= bytes;
>   else
>   bvec_iter_advance(bio->bi_io_vec, iter, bytes);
Hmm. Can't you drop 'BIO_NO_ADVANCE_ITER_MASK' after this patch?

> @@ -256,10 +261,10 @@ static inline unsigned bio_segments(struct bio *bio)
>* differently:
>*/
>  
> - if (bio->bi_rw & REQ_DISCARD)
> + if (bio_op(bio) == REQ_OP_DISCARD)
>   return 1;
>  
> - if (bio->bi_rw & REQ_WRITE_SAME)
> + if (bio_op(bio) == REQ_OP_WRITE_SAME)
>   return 1;
>  
>   bio_for_each_segment(bv, bio, iter)
> diff --git a/mm/page_io.c b/mm/page_io.c
> index 5a5fd66..dcc5d37 100644
> --- a/mm/page_io.c
> +++ b/mm/page_io.c
> @@ -317,7 +317,7 @@ int __swap_writepage(struct page *page, struct 
> writeback_control *wbc,
>   ret = -ENOMEM;
>   goto out;
>   }
> - bio->bi_rw = WRITE;
> + bio_set_op_attrs(bio, REQ_OP_WRITE, 0);
>       if (wbc->sync_mode == WB_SYNC_ALL)
>   bio->bi_rw |= REQ_SYNC;
>   count_vm_event(PSWPOUT);
> @@ -370,7 +370,7 @@ int swap_readpage(struct page *page)
>   ret = -ENOMEM;
>   goto out;
>   }
> - bio->bi_rw = READ;
> + bio_set_op_attrs(bio, REQ_OP_READ, 0);
>   count_vm_event(PSWPIN);
>   submit_bio(bio);
>  out:
> 
Reviewed-by: Hannes Reinecke 

Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)


Re: [PATCH 07/45] bcache: use op_is_write instead of checking for REQ_WRITE

2016-06-06 Thread Hannes Reinecke
On 06/05/2016 09:31 PM, mchri...@redhat.com wrote:
> From: Mike Christie <mchri...@redhat.com>
> 
> We currently set REQ_WRITE/WRITE for all non READ IOs
> like discard, flush, writesame, etc. In the next patches where we
> no longer set up the op as a bitmap, we will not be able to
> detect a operation direction like writesame by testing if REQ_WRITE is
> set.
> 
> This has bcache use the op_is_write helper which will do the right
> thing.
> 
> Signed-off-by: Mike Christie <mchri...@redhat.com>
> ---
>  drivers/md/bcache/io.c  | 2 +-
>  drivers/md/bcache/request.c | 6 +++---
>  2 files changed, 4 insertions(+), 4 deletions(-)
> 
(Could probably folded together with the two previous patches)

Reviewed-by: Hannes Reinecke <h...@suse.com>

Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)


Re: [PATCH 07/45] bcache: use op_is_write instead of checking for REQ_WRITE

2016-06-06 Thread Hannes Reinecke
On 06/05/2016 09:31 PM, mchri...@redhat.com wrote:
> From: Mike Christie 
> 
> We currently set REQ_WRITE/WRITE for all non READ IOs
> like discard, flush, writesame, etc. In the next patches where we
> no longer set up the op as a bitmap, we will not be able to
> detect a operation direction like writesame by testing if REQ_WRITE is
> set.
> 
> This has bcache use the op_is_write helper which will do the right
> thing.
> 
> Signed-off-by: Mike Christie 
> ---
>  drivers/md/bcache/io.c  | 2 +-
>  drivers/md/bcache/request.c | 6 +++---
>  2 files changed, 4 insertions(+), 4 deletions(-)
> 
(Could probably folded together with the two previous patches)

Reviewed-by: Hannes Reinecke 

Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)


Re: [PATCH 05/45] block, drivers, cgroup: use op_is_write helper instead of checking for REQ_WRITE

2016-06-06 Thread Hannes Reinecke
On 06/05/2016 09:31 PM, mchri...@redhat.com wrote:
> From: Mike Christie <mchri...@redhat.com>
> 
> We currently set REQ_WRITE/WRITE for all non READ IOs
> like discard, flush, writesame, etc. In the next patches where we
> no longer set up the op as a bitmap, we will not be able to
> detect a operation direction like writesame by testing if REQ_WRITE is
> set.
> 
> This patch converts the drivers and cgroup to use the
> op_is_write helper. This should just cover the simple
> cases. I did dm, md and bcache in their own patches
> because they were more involved.
> 
> Signed-off-by: Mike Christie <mchri...@redhat.com>
> ---
Reviewed-by: Hannes Reinecke <h...@suse.com>

Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)


Re: [PATCH 02/45] block: add REQ_OP definitions and helpers

2016-06-06 Thread Hannes Reinecke
On 06/05/2016 09:31 PM, mchri...@redhat.com wrote:
> From: Mike Christie <mchri...@redhat.com>
> 
> The following patches separate the operation (WRITE, READ, DISCARD,
> etc) from the rq_flag_bits flags. This patch adds definitions for
> request/bio operations (REQ_OPs) and adds request/bio accessors to
> get/set the op.
> 
> In this patch the REQ_OPs match the REQ rq_flag_bits ones
> for compat reasons while all the code is converted to use the
> op accessors in the set. In the last patches the op will become a
> number and the accessors and helpers in this patch will be dropped
> or updated.
> 
> Signed-off-by: Mike Christie <mchri...@redhat.com>
> ---
Reviewed-by: Hannes Reinecke <h...@suse.com>

Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)


Re: [PATCH 05/45] block, drivers, cgroup: use op_is_write helper instead of checking for REQ_WRITE

2016-06-06 Thread Hannes Reinecke
On 06/05/2016 09:31 PM, mchri...@redhat.com wrote:
> From: Mike Christie 
> 
> We currently set REQ_WRITE/WRITE for all non READ IOs
> like discard, flush, writesame, etc. In the next patches where we
> no longer set up the op as a bitmap, we will not be able to
> detect a operation direction like writesame by testing if REQ_WRITE is
> set.
> 
> This patch converts the drivers and cgroup to use the
> op_is_write helper. This should just cover the simple
> cases. I did dm, md and bcache in their own patches
> because they were more involved.
> 
> Signed-off-by: Mike Christie 
> ---
Reviewed-by: Hannes Reinecke 

Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)


Re: [PATCH 02/45] block: add REQ_OP definitions and helpers

2016-06-06 Thread Hannes Reinecke
On 06/05/2016 09:31 PM, mchri...@redhat.com wrote:
> From: Mike Christie 
> 
> The following patches separate the operation (WRITE, READ, DISCARD,
> etc) from the rq_flag_bits flags. This patch adds definitions for
> request/bio operations (REQ_OPs) and adds request/bio accessors to
> get/set the op.
> 
> In this patch the REQ_OPs match the REQ rq_flag_bits ones
> for compat reasons while all the code is converted to use the
> op accessors in the set. In the last patches the op will become a
> number and the accessors and helpers in this patch will be dropped
> or updated.
> 
> Signed-off-by: Mike Christie 
> ---
Reviewed-by: Hannes Reinecke 

Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)


Re: [PATCH 06/45] dm: use op_is_write instead of checking for REQ_WRITE

2016-06-06 Thread Hannes Reinecke
On 06/05/2016 09:31 PM, mchri...@redhat.com wrote:
> From: Mike Christie <mchri...@redhat.com>
> 
> We currently set REQ_WRITE/WRITE for all non READ IOs
> like discard, flush, writesame, etc. In the next patches where we
> no longer set up the op as a bitmap, we will not be able to
> detect a operation direction like writesame by testing if REQ_WRITE is
> set.
> 
> This has dm use the op_is_write helper which will do the right
> thing.
> 
> Signed-off-by: Mike Christie <mchri...@redhat.com>
> ---

(Could probably be folded into the previous patch)

Reviewed-by: Hannes Reinecke <h...@suse.com>

Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)


Re: [PATCH 06/45] dm: use op_is_write instead of checking for REQ_WRITE

2016-06-06 Thread Hannes Reinecke
On 06/05/2016 09:31 PM, mchri...@redhat.com wrote:
> From: Mike Christie 
> 
> We currently set REQ_WRITE/WRITE for all non READ IOs
> like discard, flush, writesame, etc. In the next patches where we
> no longer set up the op as a bitmap, we will not be able to
> detect a operation direction like writesame by testing if REQ_WRITE is
> set.
> 
> This has dm use the op_is_write helper which will do the right
> thing.
> 
> Signed-off-by: Mike Christie 
> ---

(Could probably be folded into the previous patch)

Reviewed-by: Hannes Reinecke 

Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)


Re: [PATCH 01/45] block/fs/drivers: remove rw argument from submit_bio

2016-06-06 Thread Hannes Reinecke
On 06/05/2016 09:31 PM, mchri...@redhat.com wrote:
> From: Mike Christie <mchri...@redhat.com>
> 
> This has callers of submit_bio/submit_bio_wait set the bio->bi_rw
> instead of passing it in. This makes that use the same as
> generic_make_request and how we set the other bio fields.
> 
> Signed-off-by: Mike Christie <mchri...@redhat.com>
> ---
Reviewed-by: Hannes Reinecke <h...@suse.com>

Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)


Re: [PATCH 01/45] block/fs/drivers: remove rw argument from submit_bio

2016-06-06 Thread Hannes Reinecke
On 06/05/2016 09:31 PM, mchri...@redhat.com wrote:
> From: Mike Christie 
> 
> This has callers of submit_bio/submit_bio_wait set the bio->bi_rw
> instead of passing it in. This makes that use the same as
> generic_make_request and how we set the other bio fields.
> 
> Signed-off-by: Mike Christie 
> ---
Reviewed-by: Hannes Reinecke 

Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)


Re: [PATCH v6 0/8] block: prepare for multipage bvecs

2016-06-01 Thread Hannes Reinecke
On 06/01/2016 03:43 PM, Christoph Hellwig wrote:
> These patches look good on their own.  They might be an easier sell
> just as bio cleanups :)

Fully agree. I've seen (some) improvements with those patches, so
I'd prefer to have them.

You can add:

Tested-by: Hannes Reinecke <h...@suse.com>

Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)


Re: [PATCH v6 0/8] block: prepare for multipage bvecs

2016-06-01 Thread Hannes Reinecke
On 06/01/2016 03:43 PM, Christoph Hellwig wrote:
> These patches look good on their own.  They might be an easier sell
> just as bio cleanups :)

Fully agree. I've seen (some) improvements with those patches, so
I'd prefer to have them.

You can add:

Tested-by: Hannes Reinecke 

Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)


Re: [PATCH v2 2/3] /dev/dax, core: file operations and dax-mmap

2016-05-18 Thread Hannes Reinecke
On 05/18/2016 11:10 AM, Paul Mackerras wrote:
> On Wed, May 18, 2016 at 10:07:19AM +0200, Hannes Reinecke wrote:
>> On 05/18/2016 12:19 AM, Dan Williams wrote:
>>> On Tue, May 17, 2016 at 3:57 AM, Johannes Thumshirn <jthumsh...@suse.de> 
>>> wrote:
>>>> On Sat, May 14, 2016 at 11:26:29PM -0700, Dan Williams wrote:
>>>>> The "Device DAX" core enables dax mappings of performance / feature
>>>>> differentiated memory.  An open mapping or file handle keeps the backing
>>>>> struct device live, but new mappings are only possible while the device
>>>>> is enabled.   Faults are handled under rcu_read_lock to synchronize
>>>>> with the enabled state of the device.
>>>>>
>>>>> Similar to the filesystem-dax case the backing memory may optionally
>>>>> have struct page entries.  However, unlike fs-dax there is no support
>>>>> for private mappings, or mappings that are not backed by media (see
>>>>> use of zero-page in fs-dax).
>>>>>
>>>>> Mappings are always guaranteed to match the alignment of the dax_region.
>>>>> If the dax_region is configured to have a 2MB alignment, all mappings
>>>>> are guaranteed to be backed by a pmd entry.  Contrast this determinism
>>>>> with the fs-dax case where pmd mappings are opportunistic.  If userspace
>>>>> attempts to force a misaligned mapping, the driver will fail the mmap
>>>>> attempt.  See dax_dev_check_vma() for other scenarios that are rejected,
>>>>> like MAP_PRIVATE mappings.
>>>>>
>>>>> Cc: Jeff Moyer <jmo...@redhat.com>
>>>>> Cc: Christoph Hellwig <h...@lst.de>
>>>>> Cc: Andrew Morton <a...@linux-foundation.org>
>>>>> Cc: Dave Hansen <dave.han...@linux.intel.com>
>>>>> Cc: Ross Zwisler <ross.zwis...@linux.intel.com>
>>>>> Signed-off-by: Dan Williams <dan.j.willi...@intel.com>
>>>>> ---
>>>>>  drivers/dax/Kconfig |1
>>>>>  drivers/dax/dax.c   |  316 
>>>>> +++
>>>>>  mm/huge_memory.c|1
>>>>>  mm/hugetlb.c|1
>>>>>  4 files changed, 319 insertions(+)
>>>>>
>>>>> diff --git a/drivers/dax/Kconfig b/drivers/dax/Kconfig
>>>>> index 86ffbaa891ad..cedab7572de3 100644
>>>>> --- a/drivers/dax/Kconfig
>>>>> +++ b/drivers/dax/Kconfig
>>>>> @@ -1,6 +1,7 @@
>>>>>  menuconfig DEV_DAX
>>>>>   tristate "DAX: direct access to differentiated memory"
>>>>>   default m if NVDIMM_DAX
>>>>> + depends on TRANSPARENT_HUGEPAGE
>>>>>   help
>>>>> Support raw access to differentiated (persistence, bandwidth,
>>>>> latency...) memory via an mmap(2) capable character
>>>>> diff --git a/drivers/dax/dax.c b/drivers/dax/dax.c
>>>>> index 8207fb33a992..b2fe8a0ce866 100644
>>>>> --- a/drivers/dax/dax.c
>>>>> +++ b/drivers/dax/dax.c
>>>>> @@ -49,6 +49,7 @@ struct dax_region {
>>>>>   * @region - parent region
>>>>>   * @dev - device backing the character device
>>>>>   * @kref - enable this data to be tracked in filp->private_data
>>>>> + * @alive - !alive + rcu grace period == no new mappings can be 
>>>>> established
>>>>>   * @id - child id in the region
>>>>>   * @num_resources - number of physical address extents in this device
>>>>>   * @res - array of physical address ranges
>>>>> @@ -57,6 +58,7 @@ struct dax_dev {
>>>>>   struct dax_region *region;
>>>>>   struct device *dev;
>>>>>   struct kref kref;
>>>>> + bool alive;
>>>>>   int id;
>>>>>   int num_resources;
>>>>>   struct resource res[0];
>>>>> @@ -150,6 +152,10 @@ static void destroy_dax_dev(void *_dev)
>>>>>
>>>>>   dev_dbg(dev, "%s\n", __func__);
>>>>>
>>>>> + /* disable and flush fault handlers, TODO unmap inodes */
>>>>> + dax_dev->alive = false;
>>>>> + synchronize_rcu();
>>>>> +
>>>>
>>>> IIRC RCU is only protecting a pointer, not the content of the 

Re: [PATCH v2 2/3] /dev/dax, core: file operations and dax-mmap

2016-05-18 Thread Hannes Reinecke
On 05/18/2016 11:10 AM, Paul Mackerras wrote:
> On Wed, May 18, 2016 at 10:07:19AM +0200, Hannes Reinecke wrote:
>> On 05/18/2016 12:19 AM, Dan Williams wrote:
>>> On Tue, May 17, 2016 at 3:57 AM, Johannes Thumshirn  
>>> wrote:
>>>> On Sat, May 14, 2016 at 11:26:29PM -0700, Dan Williams wrote:
>>>>> The "Device DAX" core enables dax mappings of performance / feature
>>>>> differentiated memory.  An open mapping or file handle keeps the backing
>>>>> struct device live, but new mappings are only possible while the device
>>>>> is enabled.   Faults are handled under rcu_read_lock to synchronize
>>>>> with the enabled state of the device.
>>>>>
>>>>> Similar to the filesystem-dax case the backing memory may optionally
>>>>> have struct page entries.  However, unlike fs-dax there is no support
>>>>> for private mappings, or mappings that are not backed by media (see
>>>>> use of zero-page in fs-dax).
>>>>>
>>>>> Mappings are always guaranteed to match the alignment of the dax_region.
>>>>> If the dax_region is configured to have a 2MB alignment, all mappings
>>>>> are guaranteed to be backed by a pmd entry.  Contrast this determinism
>>>>> with the fs-dax case where pmd mappings are opportunistic.  If userspace
>>>>> attempts to force a misaligned mapping, the driver will fail the mmap
>>>>> attempt.  See dax_dev_check_vma() for other scenarios that are rejected,
>>>>> like MAP_PRIVATE mappings.
>>>>>
>>>>> Cc: Jeff Moyer 
>>>>> Cc: Christoph Hellwig 
>>>>> Cc: Andrew Morton 
>>>>> Cc: Dave Hansen 
>>>>> Cc: Ross Zwisler 
>>>>> Signed-off-by: Dan Williams 
>>>>> ---
>>>>>  drivers/dax/Kconfig |1
>>>>>  drivers/dax/dax.c   |  316 
>>>>> +++
>>>>>  mm/huge_memory.c|1
>>>>>  mm/hugetlb.c|1
>>>>>  4 files changed, 319 insertions(+)
>>>>>
>>>>> diff --git a/drivers/dax/Kconfig b/drivers/dax/Kconfig
>>>>> index 86ffbaa891ad..cedab7572de3 100644
>>>>> --- a/drivers/dax/Kconfig
>>>>> +++ b/drivers/dax/Kconfig
>>>>> @@ -1,6 +1,7 @@
>>>>>  menuconfig DEV_DAX
>>>>>   tristate "DAX: direct access to differentiated memory"
>>>>>   default m if NVDIMM_DAX
>>>>> + depends on TRANSPARENT_HUGEPAGE
>>>>>   help
>>>>> Support raw access to differentiated (persistence, bandwidth,
>>>>> latency...) memory via an mmap(2) capable character
>>>>> diff --git a/drivers/dax/dax.c b/drivers/dax/dax.c
>>>>> index 8207fb33a992..b2fe8a0ce866 100644
>>>>> --- a/drivers/dax/dax.c
>>>>> +++ b/drivers/dax/dax.c
>>>>> @@ -49,6 +49,7 @@ struct dax_region {
>>>>>   * @region - parent region
>>>>>   * @dev - device backing the character device
>>>>>   * @kref - enable this data to be tracked in filp->private_data
>>>>> + * @alive - !alive + rcu grace period == no new mappings can be 
>>>>> established
>>>>>   * @id - child id in the region
>>>>>   * @num_resources - number of physical address extents in this device
>>>>>   * @res - array of physical address ranges
>>>>> @@ -57,6 +58,7 @@ struct dax_dev {
>>>>>   struct dax_region *region;
>>>>>   struct device *dev;
>>>>>   struct kref kref;
>>>>> + bool alive;
>>>>>   int id;
>>>>>   int num_resources;
>>>>>   struct resource res[0];
>>>>> @@ -150,6 +152,10 @@ static void destroy_dax_dev(void *_dev)
>>>>>
>>>>>   dev_dbg(dev, "%s\n", __func__);
>>>>>
>>>>> + /* disable and flush fault handlers, TODO unmap inodes */
>>>>> + dax_dev->alive = false;
>>>>> + synchronize_rcu();
>>>>> +
>>>>
>>>> IIRC RCU is only protecting a pointer, not the content of the pointer, so 
>>>> this
>>>> looks wrong to me.
>>>
>>> The driver is using RCU to guarantee that all currently running fault
>>> handlers have

Re: [PATCH v2 2/3] /dev/dax, core: file operations and dax-mmap

2016-05-18 Thread Hannes Reinecke
On 05/18/2016 12:19 AM, Dan Williams wrote:
> On Tue, May 17, 2016 at 3:57 AM, Johannes Thumshirn <jthumsh...@suse.de> 
> wrote:
>> On Sat, May 14, 2016 at 11:26:29PM -0700, Dan Williams wrote:
>>> The "Device DAX" core enables dax mappings of performance / feature
>>> differentiated memory.  An open mapping or file handle keeps the backing
>>> struct device live, but new mappings are only possible while the device
>>> is enabled.   Faults are handled under rcu_read_lock to synchronize
>>> with the enabled state of the device.
>>>
>>> Similar to the filesystem-dax case the backing memory may optionally
>>> have struct page entries.  However, unlike fs-dax there is no support
>>> for private mappings, or mappings that are not backed by media (see
>>> use of zero-page in fs-dax).
>>>
>>> Mappings are always guaranteed to match the alignment of the dax_region.
>>> If the dax_region is configured to have a 2MB alignment, all mappings
>>> are guaranteed to be backed by a pmd entry.  Contrast this determinism
>>> with the fs-dax case where pmd mappings are opportunistic.  If userspace
>>> attempts to force a misaligned mapping, the driver will fail the mmap
>>> attempt.  See dax_dev_check_vma() for other scenarios that are rejected,
>>> like MAP_PRIVATE mappings.
>>>
>>> Cc: Jeff Moyer <jmo...@redhat.com>
>>> Cc: Christoph Hellwig <h...@lst.de>
>>> Cc: Andrew Morton <a...@linux-foundation.org>
>>> Cc: Dave Hansen <dave.han...@linux.intel.com>
>>> Cc: Ross Zwisler <ross.zwis...@linux.intel.com>
>>> Signed-off-by: Dan Williams <dan.j.willi...@intel.com>
>>> ---
>>>  drivers/dax/Kconfig |1
>>>  drivers/dax/dax.c   |  316 
>>> +++
>>>  mm/huge_memory.c|1
>>>  mm/hugetlb.c|1
>>>  4 files changed, 319 insertions(+)
>>>
>>> diff --git a/drivers/dax/Kconfig b/drivers/dax/Kconfig
>>> index 86ffbaa891ad..cedab7572de3 100644
>>> --- a/drivers/dax/Kconfig
>>> +++ b/drivers/dax/Kconfig
>>> @@ -1,6 +1,7 @@
>>>  menuconfig DEV_DAX
>>>   tristate "DAX: direct access to differentiated memory"
>>>   default m if NVDIMM_DAX
>>> + depends on TRANSPARENT_HUGEPAGE
>>>   help
>>> Support raw access to differentiated (persistence, bandwidth,
>>> latency...) memory via an mmap(2) capable character
>>> diff --git a/drivers/dax/dax.c b/drivers/dax/dax.c
>>> index 8207fb33a992..b2fe8a0ce866 100644
>>> --- a/drivers/dax/dax.c
>>> +++ b/drivers/dax/dax.c
>>> @@ -49,6 +49,7 @@ struct dax_region {
>>>   * @region - parent region
>>>   * @dev - device backing the character device
>>>   * @kref - enable this data to be tracked in filp->private_data
>>> + * @alive - !alive + rcu grace period == no new mappings can be established
>>>   * @id - child id in the region
>>>   * @num_resources - number of physical address extents in this device
>>>   * @res - array of physical address ranges
>>> @@ -57,6 +58,7 @@ struct dax_dev {
>>>   struct dax_region *region;
>>>   struct device *dev;
>>>   struct kref kref;
>>> + bool alive;
>>>   int id;
>>>   int num_resources;
>>>   struct resource res[0];
>>> @@ -150,6 +152,10 @@ static void destroy_dax_dev(void *_dev)
>>>
>>>   dev_dbg(dev, "%s\n", __func__);
>>>
>>> + /* disable and flush fault handlers, TODO unmap inodes */
>>> + dax_dev->alive = false;
>>> + synchronize_rcu();
>>> +
>>
>> IIRC RCU is only protecting a pointer, not the content of the pointer, so 
>> this
>> looks wrong to me.
> 
> The driver is using RCU to guarantee that all currently running fault
> handlers have either completed or will see the new state of ->alive
> when they start.  Reference counts are protecting the actual dax_dev
> object.
> 
Hmm.
This is the same 'creative' RCU usage Mike Snitzer has been trying
when trying to improve device-mapper performance.

>From my understanding RCU is protecting the _pointer_, not the
values of the structure pointed to.
IOW we are guaranteed to have a valid pointer at any time.
But at the same time _no_ guarantee is made about the _contents_ of
the structure.
It might well be that using 'synchronize_rcu' giving you similar
results (as synchronize_rcu() is essentially waiting a SMP grace
period, after which all CPUs should be seeing the update).
However, I haven't been able to find that this is a guaranteed
behaviour.
So from my understanding you have to use locking primitives
protecting the contents of the structure or exchange the _entire_
structure if you want to rely on RCU here.

Can we get some clarification here?
Paul?

Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)


Re: [PATCH v2 2/3] /dev/dax, core: file operations and dax-mmap

2016-05-18 Thread Hannes Reinecke
On 05/18/2016 12:19 AM, Dan Williams wrote:
> On Tue, May 17, 2016 at 3:57 AM, Johannes Thumshirn  
> wrote:
>> On Sat, May 14, 2016 at 11:26:29PM -0700, Dan Williams wrote:
>>> The "Device DAX" core enables dax mappings of performance / feature
>>> differentiated memory.  An open mapping or file handle keeps the backing
>>> struct device live, but new mappings are only possible while the device
>>> is enabled.   Faults are handled under rcu_read_lock to synchronize
>>> with the enabled state of the device.
>>>
>>> Similar to the filesystem-dax case the backing memory may optionally
>>> have struct page entries.  However, unlike fs-dax there is no support
>>> for private mappings, or mappings that are not backed by media (see
>>> use of zero-page in fs-dax).
>>>
>>> Mappings are always guaranteed to match the alignment of the dax_region.
>>> If the dax_region is configured to have a 2MB alignment, all mappings
>>> are guaranteed to be backed by a pmd entry.  Contrast this determinism
>>> with the fs-dax case where pmd mappings are opportunistic.  If userspace
>>> attempts to force a misaligned mapping, the driver will fail the mmap
>>> attempt.  See dax_dev_check_vma() for other scenarios that are rejected,
>>> like MAP_PRIVATE mappings.
>>>
>>> Cc: Jeff Moyer 
>>> Cc: Christoph Hellwig 
>>> Cc: Andrew Morton 
>>> Cc: Dave Hansen 
>>> Cc: Ross Zwisler 
>>> Signed-off-by: Dan Williams 
>>> ---
>>>  drivers/dax/Kconfig |1
>>>  drivers/dax/dax.c   |  316 
>>> +++
>>>  mm/huge_memory.c|1
>>>  mm/hugetlb.c|1
>>>  4 files changed, 319 insertions(+)
>>>
>>> diff --git a/drivers/dax/Kconfig b/drivers/dax/Kconfig
>>> index 86ffbaa891ad..cedab7572de3 100644
>>> --- a/drivers/dax/Kconfig
>>> +++ b/drivers/dax/Kconfig
>>> @@ -1,6 +1,7 @@
>>>  menuconfig DEV_DAX
>>>   tristate "DAX: direct access to differentiated memory"
>>>   default m if NVDIMM_DAX
>>> + depends on TRANSPARENT_HUGEPAGE
>>>   help
>>> Support raw access to differentiated (persistence, bandwidth,
>>> latency...) memory via an mmap(2) capable character
>>> diff --git a/drivers/dax/dax.c b/drivers/dax/dax.c
>>> index 8207fb33a992..b2fe8a0ce866 100644
>>> --- a/drivers/dax/dax.c
>>> +++ b/drivers/dax/dax.c
>>> @@ -49,6 +49,7 @@ struct dax_region {
>>>   * @region - parent region
>>>   * @dev - device backing the character device
>>>   * @kref - enable this data to be tracked in filp->private_data
>>> + * @alive - !alive + rcu grace period == no new mappings can be established
>>>   * @id - child id in the region
>>>   * @num_resources - number of physical address extents in this device
>>>   * @res - array of physical address ranges
>>> @@ -57,6 +58,7 @@ struct dax_dev {
>>>   struct dax_region *region;
>>>   struct device *dev;
>>>   struct kref kref;
>>> + bool alive;
>>>   int id;
>>>   int num_resources;
>>>   struct resource res[0];
>>> @@ -150,6 +152,10 @@ static void destroy_dax_dev(void *_dev)
>>>
>>>   dev_dbg(dev, "%s\n", __func__);
>>>
>>> + /* disable and flush fault handlers, TODO unmap inodes */
>>> + dax_dev->alive = false;
>>> + synchronize_rcu();
>>> +
>>
>> IIRC RCU is only protecting a pointer, not the content of the pointer, so 
>> this
>> looks wrong to me.
> 
> The driver is using RCU to guarantee that all currently running fault
> handlers have either completed or will see the new state of ->alive
> when they start.  Reference counts are protecting the actual dax_dev
> object.
> 
Hmm.
This is the same 'creative' RCU usage Mike Snitzer has been trying
when trying to improve device-mapper performance.

>From my understanding RCU is protecting the _pointer_, not the
values of the structure pointed to.
IOW we are guaranteed to have a valid pointer at any time.
But at the same time _no_ guarantee is made about the _contents_ of
the structure.
It might well be that using 'synchronize_rcu' giving you similar
results (as synchronize_rcu() is essentially waiting a SMP grace
period, after which all CPUs should be seeing the update).
However, I haven't been able to find that this is a guaranteed
behaviour.
So from my understanding you have to use locking primitives
protecting the contents of the structure or exchange the _entire_
structure if you want to rely on RCU here.

Can we get some clarification here?
Paul?

Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)


Re: [PATCH] NVMe: Only release requested regions

2016-05-10 Thread Hannes Reinecke
On 05/10/2016 03:14 PM, Johannes Thumshirn wrote:
> The NVMe driver only requests the PCIe device's memory regions but releases
> all possible regions (including eventual I/O regions). This leads to a stale
> warning entry in dmesg about freeing non existent resources.
> 
> Signed-off-by: Johannes Thumshirn <jthumsh...@suse.de>
> ---
>  drivers/nvme/host/pci.c | 9 +++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
Reviewed-by: Hannes Reinecke <h...@suse.com>

Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)


Re: [PATCH] NVMe: Only release requested regions

2016-05-10 Thread Hannes Reinecke
On 05/10/2016 03:14 PM, Johannes Thumshirn wrote:
> The NVMe driver only requests the PCIe device's memory regions but releases
> all possible regions (including eventual I/O regions). This leads to a stale
> warning entry in dmesg about freeing non existent resources.
> 
> Signed-off-by: Johannes Thumshirn 
> ---
>  drivers/nvme/host/pci.c | 9 +++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
Reviewed-by: Hannes Reinecke 

Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)


Re: Regression in v4.6-rc due to SCSI multipath change

2016-05-04 Thread Hannes Reinecke
On 05/04/2016 08:51 AM, Paul Mackerras wrote:
> Current upstream kernels fail to boot on my POWER8 server with
> multipath SCSI disks and IPR host bus adapters.  What happens is that
> the system finds each disk twice (as normal) and then prints messages
> like this:
> 
> [2.827761] sd 1:2:4:0: alua: supports implicit TPGS
> [2.827875] sd 1:2:4:0: alua: No device descriptors found
> [2.827923] sd 1:2:4:0: alua: Attach failed (-22)
> [2.827979] device-mapper: table: 253:0: multipath: error attaching 
> hardware handler
> [2.828048] device-mapper: ioctl: error adding target to table
> 
> Eventually dracut times out (this is with Fedora 23) enters emergency
> mode.
> 
> I bisected the problem down to commit 0047220c6c36 ("scsi_dh_alua: use
> unique device id", 2016-02-19).  It seems that this commit adds the
> restriction that we can only do multipath with disks that have stuff
> in their VPD page 83 that scsi_vpd_lun_id() can parse.  The disks on
> my server apparently don't.
> 
> I instrumented scsi_vpd_lun_id() to find out what was going on.  The
> disks on this machine have a vendor-specific designator and a T10
> vendor ID based designator, but no designators of types 2, 3 or 8.
> An example from one disk is:
> 
> 02 01 00 20 49 42 4d 20 20 20 20 20 49 50 52 2d 30 20 20 20 35 45 43
> 34 41 42 30 30 30 30 30 30 30 30 32 30
> 
> 02 00 00 14 30 20 20 20 20 20 20 20 20 20 20 20 20 20 20 20 20 20 20 20
> 
> I have a patch that extends scsi_vpd_lun_id() to be able to use the
> T10 vendor ID based designator, which fixes the problem on my system.
> I'll post the patch shortly.
> 
Please do. I'm about to draft a patch myself, but if you already
have one ...

> However, was it really intentional that multipath now can't be used
> with disks like these, when it worked just fine previously?
> 
Well. The thing is, ALUA can't really work if no VPD descriptors are
found, and so the check itself is correct.

Howver, we really need to parse all possible VPD descriptors, for
sure, so that is indeed a bug.
I'm preparing a patch for decoding all possible VPD descriptors, too.

Let's see who's first :-)

Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)


Re: Regression in v4.6-rc due to SCSI multipath change

2016-05-04 Thread Hannes Reinecke
On 05/04/2016 08:51 AM, Paul Mackerras wrote:
> Current upstream kernels fail to boot on my POWER8 server with
> multipath SCSI disks and IPR host bus adapters.  What happens is that
> the system finds each disk twice (as normal) and then prints messages
> like this:
> 
> [2.827761] sd 1:2:4:0: alua: supports implicit TPGS
> [2.827875] sd 1:2:4:0: alua: No device descriptors found
> [2.827923] sd 1:2:4:0: alua: Attach failed (-22)
> [2.827979] device-mapper: table: 253:0: multipath: error attaching 
> hardware handler
> [2.828048] device-mapper: ioctl: error adding target to table
> 
> Eventually dracut times out (this is with Fedora 23) enters emergency
> mode.
> 
> I bisected the problem down to commit 0047220c6c36 ("scsi_dh_alua: use
> unique device id", 2016-02-19).  It seems that this commit adds the
> restriction that we can only do multipath with disks that have stuff
> in their VPD page 83 that scsi_vpd_lun_id() can parse.  The disks on
> my server apparently don't.
> 
> I instrumented scsi_vpd_lun_id() to find out what was going on.  The
> disks on this machine have a vendor-specific designator and a T10
> vendor ID based designator, but no designators of types 2, 3 or 8.
> An example from one disk is:
> 
> 02 01 00 20 49 42 4d 20 20 20 20 20 49 50 52 2d 30 20 20 20 35 45 43
> 34 41 42 30 30 30 30 30 30 30 30 32 30
> 
> 02 00 00 14 30 20 20 20 20 20 20 20 20 20 20 20 20 20 20 20 20 20 20 20
> 
> I have a patch that extends scsi_vpd_lun_id() to be able to use the
> T10 vendor ID based designator, which fixes the problem on my system.
> I'll post the patch shortly.
> 
Please do. I'm about to draft a patch myself, but if you already
have one ...

> However, was it really intentional that multipath now can't be used
> with disks like these, when it worked just fine previously?
> 
Well. The thing is, ALUA can't really work if no VPD descriptors are
found, and so the check itself is correct.

Howver, we really need to parse all possible VPD descriptors, for
sure, so that is indeed a bug.
I'm preparing a patch for decoding all possible VPD descriptors, too.

Let's see who's first :-)

Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)


Re: [PATCH] block: initialize hd_stuct's reference before assigning it

2016-05-03 Thread Hannes Reinecke
On 05/03/2016 03:38 PM, Johannes Thumshirn wrote:
> Inititialize the hd_struct's perpcu reference before assigning the hd_struct
> to the partition table list.
> 
> This fixes a race which could be triggered using a simple partition
> creation/deletion loop with virtio-blk on aarch64.
> 
> Signed-off-by: Johannes Thumshirn <jthumsh...@suse.de>
> ---
>  block/partition-generic.c | 7 +--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/block/partition-generic.c b/block/partition-generic.c
> index 5d87019..aa5b83a 100644
> --- a/block/partition-generic.c
> +++ b/block/partition-generic.c
> @@ -361,6 +361,10 @@ struct hd_struct *add_partition(struct gendisk *disk, 
> int partno,
>   goto out_del;
>   }
>  
> + err = -ENOMEM;
> + if (hd_ref_init(p))
> + goto out_free_info;
> +
>   /* everything is up and running, commence */
>   rcu_assign_pointer(ptbl->part[partno], p);
>  
> @@ -368,8 +372,7 @@ struct hd_struct *add_partition(struct gendisk *disk, int 
> partno,
>   if (!dev_get_uevent_suppress(ddev))
>   kobject_uevent(>kobj, KOBJ_ADD);
>  
> - if (!hd_ref_init(p))
> - return p;
> + return p;
>  
>  out_free_info:
>   free_part_info(p);
> 
Suggested-by: Hannes Reinecke <h...@suse.com>
Reviewed-by: Hannes Reinecke <h...@suse.com>

:-)

Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)


Re: [PATCH] block: initialize hd_stuct's reference before assigning it

2016-05-03 Thread Hannes Reinecke
On 05/03/2016 03:38 PM, Johannes Thumshirn wrote:
> Inititialize the hd_struct's perpcu reference before assigning the hd_struct
> to the partition table list.
> 
> This fixes a race which could be triggered using a simple partition
> creation/deletion loop with virtio-blk on aarch64.
> 
> Signed-off-by: Johannes Thumshirn 
> ---
>  block/partition-generic.c | 7 +--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/block/partition-generic.c b/block/partition-generic.c
> index 5d87019..aa5b83a 100644
> --- a/block/partition-generic.c
> +++ b/block/partition-generic.c
> @@ -361,6 +361,10 @@ struct hd_struct *add_partition(struct gendisk *disk, 
> int partno,
>   goto out_del;
>   }
>  
> + err = -ENOMEM;
> + if (hd_ref_init(p))
> + goto out_free_info;
> +
>   /* everything is up and running, commence */
>   rcu_assign_pointer(ptbl->part[partno], p);
>  
> @@ -368,8 +372,7 @@ struct hd_struct *add_partition(struct gendisk *disk, int 
> partno,
>   if (!dev_get_uevent_suppress(ddev))
>   kobject_uevent(>kobj, KOBJ_ADD);
>  
> - if (!hd_ref_init(p))
> -     return p;
> + return p;
>  
>  out_free_info:
>   free_part_info(p);
> 
Suggested-by: Hannes Reinecke 
Reviewed-by: Hannes Reinecke 

:-)

Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)


Re: [PATCH 1/1] [RFC] workqueue: fix ghost PENDING flag while doing MQ IO

2016-04-26 Thread Hannes Reinecke
On 04/26/2016 07:45 PM, Tejun Heo wrote:
> Hello, Peter.
> 
> On Tue, Apr 26, 2016 at 10:27:59AM -0700, Peter Hurley wrote:
>>> It's unlikely to make any measureable difference.  Is xchg() actually
>>> cheaper than store + rmb?
>>
>> store + mfence (full barrier), yes. Roughly 2x faster.
>>
>> https://lkml.org/lkml/2015/11/2/607
> 
> Ah, didn't know that.  Thanks for the pointer.
> 
>>> I'm not necessarily against making all clearings of
>>> PENDING to be followed by a rmb or use xhcg.  Reasons 2-4 are pretty
>>> weak tho.
>>
>> I agree 2 and 3 are not the best reasons.
>> Actually, it looks that I'm in the minority anyway, and that style-wise,
>> naked barrier is preferred.
> 
> As long as what's happening is clearly documented, I think either is
> fine.  I'm gonna go with Roman's mb patch for -stable fix but think
> it'd be nice to have a separate patch to consolidate the paths which
> clear PENDING and make them use xchg.  If you can spin up a patch for
> that, I'd be happy to apply it to wq/for-3.7.
>      ^^^
Ah. Time warp.
I knew it would happen eventually :-)

Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)


Re: [PATCH 1/1] [RFC] workqueue: fix ghost PENDING flag while doing MQ IO

2016-04-26 Thread Hannes Reinecke
On 04/26/2016 07:45 PM, Tejun Heo wrote:
> Hello, Peter.
> 
> On Tue, Apr 26, 2016 at 10:27:59AM -0700, Peter Hurley wrote:
>>> It's unlikely to make any measureable difference.  Is xchg() actually
>>> cheaper than store + rmb?
>>
>> store + mfence (full barrier), yes. Roughly 2x faster.
>>
>> https://lkml.org/lkml/2015/11/2/607
> 
> Ah, didn't know that.  Thanks for the pointer.
> 
>>> I'm not necessarily against making all clearings of
>>> PENDING to be followed by a rmb or use xhcg.  Reasons 2-4 are pretty
>>> weak tho.
>>
>> I agree 2 and 3 are not the best reasons.
>> Actually, it looks that I'm in the minority anyway, and that style-wise,
>> naked barrier is preferred.
> 
> As long as what's happening is clearly documented, I think either is
> fine.  I'm gonna go with Roman's mb patch for -stable fix but think
> it'd be nice to have a separate patch to consolidate the paths which
> clear PENDING and make them use xchg.  If you can spin up a patch for
> that, I'd be happy to apply it to wq/for-3.7.
>      ^^^
Ah. Time warp.
I knew it would happen eventually :-)

Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)


Re: [PATCH 1/3] hisi_sas: add device and slot alloc hw methods

2016-04-15 Thread Hannes Reinecke
On 04/15/2016 03:36 PM, John Garry wrote:
> Add methods to use HW specific versions of
> functions to allocate slot and device.
> HW specific methods are permitted to workaround
> device id vs IPTT collision issue in v2 hw.
> 
> Signed-off-by: John Garry <john.ga...@huawei.com>
> ---
>  drivers/scsi/hisi_sas/hisi_sas.h  |  3 +++
>  drivers/scsi/hisi_sas/hisi_sas_main.c | 11 +--
>  2 files changed, 12 insertions(+), 2 deletions(-)
> 
Reviewed-by: Hannes Reinecke <h...@suse.com>

Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)


Re: [PATCH 1/3] hisi_sas: add device and slot alloc hw methods

2016-04-15 Thread Hannes Reinecke
On 04/15/2016 03:36 PM, John Garry wrote:
> Add methods to use HW specific versions of
> functions to allocate slot and device.
> HW specific methods are permitted to workaround
> device id vs IPTT collision issue in v2 hw.
> 
> Signed-off-by: John Garry 
> ---
>  drivers/scsi/hisi_sas/hisi_sas.h  |  3 +++
>  drivers/scsi/hisi_sas/hisi_sas_main.c | 11 +--
>  2 files changed, 12 insertions(+), 2 deletions(-)
> 
Reviewed-by: Hannes Reinecke 

Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)


Re: [PATCH 2/3] hisi_sas: add slot_index_alloc_quirk_v2_hw()

2016-04-15 Thread Hannes Reinecke
On 04/15/2016 03:36 PM, John Garry wrote:
> Add v2 hw custom function slot_index_alloc_quirk_v2_hw().
> SAS devices should have IPTT bit0 equal to 1.
> 
> Signed-off-by: John Garry <john.ga...@huawei.com>
> ---
>  drivers/scsi/hisi_sas/hisi_sas_v2_hw.c | 28 
>  1 file changed, 28 insertions(+)
> 
Reviewed-by: Hannes Reinecke <h...@suse.com>

Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)


Re: [PATCH 3/3] hisi_sas: add alloc_dev_quirk_v2_hw()

2016-04-15 Thread Hannes Reinecke
On 04/15/2016 03:36 PM, John Garry wrote:
> Add custom version of function to allocate
> device, alloc_dev_quirk_v2_hw().
> For sata devices the device id bit0 should be
> 0.
> 
> Signed-off-by: John Garry <john.ga...@huawei.com>
> ---
>  drivers/scsi/hisi_sas/hisi_sas_v2_hw.c | 30 ++
>  1 file changed, 30 insertions(+)
> 
Reviewed-by: Hannes Reinecke <h...@suse.com>

Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)


Re: [PATCH 2/3] hisi_sas: add slot_index_alloc_quirk_v2_hw()

2016-04-15 Thread Hannes Reinecke
On 04/15/2016 03:36 PM, John Garry wrote:
> Add v2 hw custom function slot_index_alloc_quirk_v2_hw().
> SAS devices should have IPTT bit0 equal to 1.
> 
> Signed-off-by: John Garry 
> ---
>  drivers/scsi/hisi_sas/hisi_sas_v2_hw.c | 28 
>  1 file changed, 28 insertions(+)
> 
Reviewed-by: Hannes Reinecke 

Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)


Re: [PATCH 3/3] hisi_sas: add alloc_dev_quirk_v2_hw()

2016-04-15 Thread Hannes Reinecke
On 04/15/2016 03:36 PM, John Garry wrote:
> Add custom version of function to allocate
> device, alloc_dev_quirk_v2_hw().
> For sata devices the device id bit0 should be
> 0.
> 
> Signed-off-by: John Garry 
> ---
>  drivers/scsi/hisi_sas/hisi_sas_v2_hw.c | 30 ++
>  1 file changed, 30 insertions(+)
> 
Reviewed-by: Hannes Reinecke 

Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)


Re: [patch v2] scsi_dh_alua: uninitialized variable in alua_rtpg()

2016-04-15 Thread Hannes Reinecke
On 04/14/2016 08:20 PM, Dan Carpenter wrote:
> It's possible to use "err" without initializing it.  If it happens to be
> a 2 which is SCSI_DH_RETRY then that could cause a bug.  Bart Van Assche
> pointed out that we should probably re-initialize it for every iteration
> through the retry loop.
> 
> Signed-off-by: Dan Carpenter <dan.carpen...@oracle.com>
> ---
> v2: The first version just initialized it at the start of the function.
> 
> diff --git a/drivers/scsi/device_handler/scsi_dh_alua.c 
> b/drivers/scsi/device_handler/scsi_dh_alua.c
> index 8eaed05..a655cf2 100644
> --- a/drivers/scsi/device_handler/scsi_dh_alua.c
> +++ b/drivers/scsi/device_handler/scsi_dh_alua.c
> @@ -532,6 +532,7 @@ static int alua_rtpg(struct scsi_device *sdev, struct 
> alua_port_group *pg)
>   return SCSI_DH_DEV_TEMP_BUSY;
>  
>   retry:
> + err = 0;
>   retval = submit_rtpg(sdev, buff, bufflen, _hdr, pg->flags);
>  
>   if (retval) {
> 
Reviewed-by: Hannes Reinecke <h...@suse.com>

Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)


Re: [patch v2] scsi_dh_alua: uninitialized variable in alua_rtpg()

2016-04-15 Thread Hannes Reinecke
On 04/14/2016 08:20 PM, Dan Carpenter wrote:
> It's possible to use "err" without initializing it.  If it happens to be
> a 2 which is SCSI_DH_RETRY then that could cause a bug.  Bart Van Assche
> pointed out that we should probably re-initialize it for every iteration
> through the retry loop.
> 
> Signed-off-by: Dan Carpenter 
> ---
> v2: The first version just initialized it at the start of the function.
> 
> diff --git a/drivers/scsi/device_handler/scsi_dh_alua.c 
> b/drivers/scsi/device_handler/scsi_dh_alua.c
> index 8eaed05..a655cf2 100644
> --- a/drivers/scsi/device_handler/scsi_dh_alua.c
> +++ b/drivers/scsi/device_handler/scsi_dh_alua.c
> @@ -532,6 +532,7 @@ static int alua_rtpg(struct scsi_device *sdev, struct 
> alua_port_group *pg)
>   return SCSI_DH_DEV_TEMP_BUSY;
>  
>   retry:
> + err = 0;
>   retval = submit_rtpg(sdev, buff, bufflen, _hdr, pg->flags);
>  
>       if (retval) {
> 
Reviewed-by: Hannes Reinecke 

Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)


Re: [PATCH 2/2] target: use new "dbroot" target attribute

2016-04-14 Thread Hannes Reinecke
On 04/13/2016 10:25 PM, Lee Duncan wrote:
> This commit updates the target core ALUA and PR
> modules to use the new "dbroot" attribute instead
> of assuming the target database is in "/var/target".
> 
> Signed-off-by: Lee Duncan <ldun...@suse.com>
> ---
>  drivers/target/target_core_alua.c | 6 +++---
>  drivers/target/target_core_pr.c   | 2 +-
>  2 files changed, 4 insertions(+), 4 deletions(-)
> 
Reviewed-by: Hannes Reinecke <h...@suse.com>

Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)


Re: [PATCH 2/2] target: use new "dbroot" target attribute

2016-04-14 Thread Hannes Reinecke
On 04/13/2016 10:25 PM, Lee Duncan wrote:
> This commit updates the target core ALUA and PR
> modules to use the new "dbroot" attribute instead
> of assuming the target database is in "/var/target".
> 
> Signed-off-by: Lee Duncan 
> ---
>  drivers/target/target_core_alua.c | 6 +++---
>  drivers/target/target_core_pr.c   | 2 +-
>  2 files changed, 4 insertions(+), 4 deletions(-)
> 
Reviewed-by: Hannes Reinecke 

Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)


Re: [PATCHv2 1/2] target: make target db location configurable

2016-04-14 Thread Hannes Reinecke
On 04/13/2016 10:25 PM, Lee Duncan wrote:
> This commit adds the read-write attribute "dbroot",
> in the top-level CONFIGFS (core) target directory,
> normally /sys/kernel/config/target. This attribute
> defaults to "/var/target" but can be changed by
> writing a new pathname string to it. Changing this
> attribute is only allowed when no fabric drivers
> are loaded and the supplied value specifies an
> existing directory.
> 
> Target modules that care about the target database
> root directory will be modified to use this
> attribute in a future commit.
> 
> Signed-off-by: Lee Duncan <ldun...@suse.com>
> ---
>  drivers/target/target_core_configfs.c | 51 
> +++
>  drivers/target/target_core_internal.h |  6 +
>  2 files changed, 57 insertions(+)
> 
> diff --git a/drivers/target/target_core_configfs.c 
> b/drivers/target/target_core_configfs.c
> index 713c63d9681b..bfedbd92b77f 100644
> --- a/drivers/target/target_core_configfs.c
> +++ b/drivers/target/target_core_configfs.c
> @@ -99,6 +99,56 @@ static ssize_t target_core_item_version_show(struct 
> config_item *item,
>  
>  CONFIGFS_ATTR_RO(target_core_item_, version);
>  
> +char db_root[DB_ROOT_LEN] = DB_ROOT_DEFAULT;
> +static char db_root_stage[DB_ROOT_LEN];
> +
> +static ssize_t target_core_item_dbroot_show(struct config_item *item,
> + char *page)
> +{
> + return sprintf(page, "%s\n", db_root);
> +}
> +
> +static ssize_t target_core_item_dbroot_store(struct config_item *item,
> + const char *page, size_t count)
> +{
> + ssize_t read_bytes;
> + struct file *fp;
> +
> + if (!list_empty(_tf_list)) {
> + pr_err("db_root: cannot be changed: target drivers registered");
> + return -EINVAL;
> + }
Locking?

Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)


Re: [PATCHv2 1/2] target: make target db location configurable

2016-04-14 Thread Hannes Reinecke
On 04/13/2016 10:25 PM, Lee Duncan wrote:
> This commit adds the read-write attribute "dbroot",
> in the top-level CONFIGFS (core) target directory,
> normally /sys/kernel/config/target. This attribute
> defaults to "/var/target" but can be changed by
> writing a new pathname string to it. Changing this
> attribute is only allowed when no fabric drivers
> are loaded and the supplied value specifies an
> existing directory.
> 
> Target modules that care about the target database
> root directory will be modified to use this
> attribute in a future commit.
> 
> Signed-off-by: Lee Duncan 
> ---
>  drivers/target/target_core_configfs.c | 51 
> +++
>  drivers/target/target_core_internal.h |  6 +
>  2 files changed, 57 insertions(+)
> 
> diff --git a/drivers/target/target_core_configfs.c 
> b/drivers/target/target_core_configfs.c
> index 713c63d9681b..bfedbd92b77f 100644
> --- a/drivers/target/target_core_configfs.c
> +++ b/drivers/target/target_core_configfs.c
> @@ -99,6 +99,56 @@ static ssize_t target_core_item_version_show(struct 
> config_item *item,
>  
>  CONFIGFS_ATTR_RO(target_core_item_, version);
>  
> +char db_root[DB_ROOT_LEN] = DB_ROOT_DEFAULT;
> +static char db_root_stage[DB_ROOT_LEN];
> +
> +static ssize_t target_core_item_dbroot_show(struct config_item *item,
> + char *page)
> +{
> + return sprintf(page, "%s\n", db_root);
> +}
> +
> +static ssize_t target_core_item_dbroot_store(struct config_item *item,
> + const char *page, size_t count)
> +{
> + ssize_t read_bytes;
> + struct file *fp;
> +
> + if (!list_empty(_tf_list)) {
> + pr_err("db_root: cannot be changed: target drivers registered");
> + return -EINVAL;
> + }
Locking?

Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)


Re: [PATCH 00/42] v5: separate operations from flags in the bio/request structs

2016-04-14 Thread Hannes Reinecke
On 04/13/2016 09:35 PM, mchri...@redhat.com wrote:
> The following patches begin to cleanup the request->cmd_flags and
> bio->bi_rw mess. We currently use cmd_flags to specify the operation,
> attributes and state of the request. For bi_rw we use it for similar
> info and also the priority but then also have another bi_flags field
> for state. At some point, we abused them so much we just made cmd_flags
> 64 bits, so we could add more.
> 
> The following patches seperate the operation (read, write discard,
> flush, etc) from cmd_flags/bi_rw.
> 
> This patchset was made against linux-next from today April 13
> (git tag next-20160413).
> 
> I put a git tree here:
> https://github.com/mikechristie/linux-kernel.git
> The patches are in the op branch.
> 
A round of applause for you.

For the entire series:

Reviewed-by: Hannes Reinecke <h...@suse.com>

Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)


Re: [PATCH 00/42] v5: separate operations from flags in the bio/request structs

2016-04-14 Thread Hannes Reinecke
On 04/13/2016 09:35 PM, mchri...@redhat.com wrote:
> The following patches begin to cleanup the request->cmd_flags and
> bio->bi_rw mess. We currently use cmd_flags to specify the operation,
> attributes and state of the request. For bi_rw we use it for similar
> info and also the priority but then also have another bi_flags field
> for state. At some point, we abused them so much we just made cmd_flags
> 64 bits, so we could add more.
> 
> The following patches seperate the operation (read, write discard,
> flush, etc) from cmd_flags/bi_rw.
> 
> This patchset was made against linux-next from today April 13
> (git tag next-20160413).
> 
> I put a git tree here:
> https://github.com/mikechristie/linux-kernel.git
> The patches are in the op branch.
> 
A round of applause for you.

For the entire series:

Reviewed-by: Hannes Reinecke 

Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)


Re: [PATCH v3 2/2] Revert "scsi: fix soft lockup in scsi_remove_target() on module removal"

2016-03-31 Thread Hannes Reinecke
On 03/31/2016 02:53 PM, Johannes Thumshirn wrote:
> This reverts commit 90a88d6ef88edcfc4f644dddc7eef4ea41bccf8b.
> 
> Signed-off-by: Johannes Thumshirn <jthumsh...@suse.de>
> ---
>  drivers/scsi/scsi_sysfs.c | 6 ++
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
Reviewed-by: Hannes Reinecke <h...@suse.com>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke   zSeries & Storage
h...@suse.de  +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)


Re: [PATCH v3 2/2] Revert "scsi: fix soft lockup in scsi_remove_target() on module removal"

2016-03-31 Thread Hannes Reinecke
On 03/31/2016 02:53 PM, Johannes Thumshirn wrote:
> This reverts commit 90a88d6ef88edcfc4f644dddc7eef4ea41bccf8b.
> 
> Signed-off-by: Johannes Thumshirn 
> ---
>  drivers/scsi/scsi_sysfs.c | 6 ++
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
Reviewed-by: Hannes Reinecke 

Cheers,

Hannes
-- 
Dr. Hannes Reinecke   zSeries & Storage
h...@suse.de  +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)


Re: [PATCH v3 1/2] scsi: Add intermediate STARGET_REMOVE state to scsi_target_state

2016-03-31 Thread Hannes Reinecke
On 03/31/2016 02:53 PM, Johannes Thumshirn wrote:
> Add intermediate STARGET_REMOVE state to scsi_target_state to avoid running
> into the BUG_ON() in scsi_target_reap().
> 
> This intermediate state is only valid in the path from scsi_remove_target() to
> scsi_target_destroy() indicating this target is going to be removed.
> 
> Signed-off-by: Johannes Thumshirn <jthumsh...@suse.de>
> Fixes: 40998193560dab6c3ce8d25f4fa58a23e252ef38
> ---
>  drivers/scsi/scsi_scan.c   | 2 ++
>  drivers/scsi/scsi_sysfs.c  | 2 ++
>  include/scsi/scsi_device.h | 1 +
>  3 files changed, 5 insertions(+)
> 
> diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
> index 6a82066..63b8bca 100644
> --- a/drivers/scsi/scsi_scan.c
> +++ b/drivers/scsi/scsi_scan.c
> @@ -315,6 +315,8 @@ static void scsi_target_destroy(struct scsi_target 
> *starget)
>   struct Scsi_Host *shost = dev_to_shost(dev->parent);
>   unsigned long flags;
>  
> + BUG_ON(starget->state != STARGET_REMOVE &&
> +starget->state != STARGET_CREATED);
>   starget->state = STARGET_DEL;
>   transport_destroy_device(dev);
>   spin_lock_irqsave(shost->host_lock, flags);
> diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
> index 00bc721..0df82e8 100644
> --- a/drivers/scsi/scsi_sysfs.c
> +++ b/drivers/scsi/scsi_sysfs.c
> @@ -1279,11 +1279,13 @@ restart:
>   spin_lock_irqsave(shost->host_lock, flags);
>   list_for_each_entry(starget, >__targets, siblings) {
>   if (starget->state == STARGET_DEL ||
> + starget->state == STARGET_REMOVE ||
>   starget == last_target)
>   continue;
>   if (starget->dev.parent == dev || >dev == dev) {
>   kref_get(>reap_ref);
>   last_target = starget;
> + starget->state = STARGET_REMOVE;
>   spin_unlock_irqrestore(shost->host_lock, flags);
>   __scsi_remove_target(starget);
>   scsi_target_reap(starget);
> diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h
> index f63a167..2bffaa6 100644
> --- a/include/scsi/scsi_device.h
> +++ b/include/scsi/scsi_device.h
> @@ -240,6 +240,7 @@ scmd_printk(const char *, const struct scsi_cmnd *, const 
> char *, ...);
>  enum scsi_target_state {
>   STARGET_CREATED = 1,
>   STARGET_RUNNING,
> + STARGET_REMOVE,
>   STARGET_DEL,
>  };
>  
> 
Reviewed-by: Hannes Reinecke <h...@suse.com>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke   zSeries & Storage
h...@suse.de  +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)


Re: [PATCH v3 1/2] scsi: Add intermediate STARGET_REMOVE state to scsi_target_state

2016-03-31 Thread Hannes Reinecke
On 03/31/2016 02:53 PM, Johannes Thumshirn wrote:
> Add intermediate STARGET_REMOVE state to scsi_target_state to avoid running
> into the BUG_ON() in scsi_target_reap().
> 
> This intermediate state is only valid in the path from scsi_remove_target() to
> scsi_target_destroy() indicating this target is going to be removed.
> 
> Signed-off-by: Johannes Thumshirn 
> Fixes: 40998193560dab6c3ce8d25f4fa58a23e252ef38
> ---
>  drivers/scsi/scsi_scan.c   | 2 ++
>  drivers/scsi/scsi_sysfs.c  | 2 ++
>  include/scsi/scsi_device.h | 1 +
>  3 files changed, 5 insertions(+)
> 
> diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
> index 6a82066..63b8bca 100644
> --- a/drivers/scsi/scsi_scan.c
> +++ b/drivers/scsi/scsi_scan.c
> @@ -315,6 +315,8 @@ static void scsi_target_destroy(struct scsi_target 
> *starget)
>   struct Scsi_Host *shost = dev_to_shost(dev->parent);
>   unsigned long flags;
>  
> + BUG_ON(starget->state != STARGET_REMOVE &&
> +starget->state != STARGET_CREATED);
>   starget->state = STARGET_DEL;
>   transport_destroy_device(dev);
>   spin_lock_irqsave(shost->host_lock, flags);
> diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
> index 00bc721..0df82e8 100644
> --- a/drivers/scsi/scsi_sysfs.c
> +++ b/drivers/scsi/scsi_sysfs.c
> @@ -1279,11 +1279,13 @@ restart:
>   spin_lock_irqsave(shost->host_lock, flags);
>   list_for_each_entry(starget, >__targets, siblings) {
>   if (starget->state == STARGET_DEL ||
> + starget->state == STARGET_REMOVE ||
>   starget == last_target)
>   continue;
>   if (starget->dev.parent == dev || >dev == dev) {
>   kref_get(>reap_ref);
>   last_target = starget;
> + starget->state = STARGET_REMOVE;
>   spin_unlock_irqrestore(shost->host_lock, flags);
>   __scsi_remove_target(starget);
>   scsi_target_reap(starget);
> diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h
> index f63a167..2bffaa6 100644
> --- a/include/scsi/scsi_device.h
> +++ b/include/scsi/scsi_device.h
> @@ -240,6 +240,7 @@ scmd_printk(const char *, const struct scsi_cmnd *, const 
> char *, ...);
>  enum scsi_target_state {
>   STARGET_CREATED = 1,
>   STARGET_RUNNING,
> + STARGET_REMOVE,
>   STARGET_DEL,
>  };
>  
> 
Reviewed-by: Hannes Reinecke 

Cheers,

Hannes
-- 
Dr. Hannes Reinecke   zSeries & Storage
h...@suse.de  +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)


Re: [PATCH v3 23/23] ncr5380: Call complete_cmd() for disconnected commands on bus reset

2016-03-23 Thread Hannes Reinecke
On 03/21/2016 03:32 AM, Finn Thain wrote:
> I'm told that some targets are liable to disconnect a REQUEST SENSE
> command. Theoretically this would cause a command undergoing autosense to
> be moved onto the disconnected list. The bus reset handler must call
> complete_cmd() for these commands, otherwise the hostdata->sensing pointer
> will not get cleared. That would cause autosense processing to stall and
> a timeout or an incorrect scsi_eh_restore_cmnd() would eventually follow.
> 
> Signed-off-by: Finn Thain <fth...@telegraphics.com.au>
> 
> ---
>  drivers/scsi/NCR5380.c |2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> Index: linux/drivers/scsi/NCR5380.c
> ===
> --- linux.orig/drivers/scsi/NCR5380.c 2016-03-21 13:31:40.0 +1100
> +++ linux/drivers/scsi/NCR5380.c  2016-03-21 13:31:47.0 +1100
> @@ -2437,7 +2437,7 @@ static int NCR5380_bus_reset(struct scsi
>   struct scsi_cmnd *cmd = NCR5380_to_scmd(ncmd);
>  
>   set_host_byte(cmd, DID_RESET);
> - cmd->scsi_done(cmd);
> + complete_cmd(instance, cmd);
>   }
>   INIT_LIST_HEAD(>disconnected);
>  
> 
> 
Reviewed-by: Hannes Reinecke <h...@suse.com>

Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)


Re: [PATCH v3 23/23] ncr5380: Call complete_cmd() for disconnected commands on bus reset

2016-03-23 Thread Hannes Reinecke
On 03/21/2016 03:32 AM, Finn Thain wrote:
> I'm told that some targets are liable to disconnect a REQUEST SENSE
> command. Theoretically this would cause a command undergoing autosense to
> be moved onto the disconnected list. The bus reset handler must call
> complete_cmd() for these commands, otherwise the hostdata->sensing pointer
> will not get cleared. That would cause autosense processing to stall and
> a timeout or an incorrect scsi_eh_restore_cmnd() would eventually follow.
> 
> Signed-off-by: Finn Thain 
> 
> ---
>  drivers/scsi/NCR5380.c |2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> Index: linux/drivers/scsi/NCR5380.c
> ===
> --- linux.orig/drivers/scsi/NCR5380.c 2016-03-21 13:31:40.0 +1100
> +++ linux/drivers/scsi/NCR5380.c  2016-03-21 13:31:47.0 +1100
> @@ -2437,7 +2437,7 @@ static int NCR5380_bus_reset(struct scsi
>   struct scsi_cmnd *cmd = NCR5380_to_scmd(ncmd);
>  
>   set_host_byte(cmd, DID_RESET);
> - cmd->scsi_done(cmd);
> + complete_cmd(instance, cmd);
>   }
>   INIT_LIST_HEAD(>disconnected);
>  
> 
> 
Reviewed-by: Hannes Reinecke 

Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)


Re: [PATCH v3 20/23] atari_scsi: Set a reasonable default for cmd_per_lun

2016-03-23 Thread Hannes Reinecke
On 03/21/2016 03:32 AM, Finn Thain wrote:
> This setting does not need to be conditional on Atari ST or TT.
> 
> Signed-off-by: Finn Thain <fth...@telegraphics.com.au>
> Tested-by: Michael Schmitz <schmitz...@gmail.com>
> 
> ---
> 
> Changed since v1:
> - Set the default cmd_per_lun to 4 based on test results.
> 
> Changed since v2:
> - Revert the default cmd_per_lun to 2, like in the v1 patch, because
> a uniform default across all ten 5380 wrapper drivers is worth more
> than a tiny improvement in one particular microbenchmark on one system.
> Michael tells me that 2 is also the best setting for his Atari Falcon.
> 
> ---
>  drivers/scsi/atari_scsi.c |3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
Reviewed-by: Hannes Reinecke <h...@suse.com>

Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)


Re: [PATCH v3 20/23] atari_scsi: Set a reasonable default for cmd_per_lun

2016-03-23 Thread Hannes Reinecke
On 03/21/2016 03:32 AM, Finn Thain wrote:
> This setting does not need to be conditional on Atari ST or TT.
> 
> Signed-off-by: Finn Thain 
> Tested-by: Michael Schmitz 
> 
> ---
> 
> Changed since v1:
> - Set the default cmd_per_lun to 4 based on test results.
> 
> Changed since v2:
> - Revert the default cmd_per_lun to 2, like in the v1 patch, because
> a uniform default across all ten 5380 wrapper drivers is worth more
> than a tiny improvement in one particular microbenchmark on one system.
> Michael tells me that 2 is also the best setting for his Atari Falcon.
> 
> ---
>  drivers/scsi/atari_scsi.c |3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
Reviewed-by: Hannes Reinecke 

Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)


Re: [PATCH v3 14/23] ncr5380: Reduce max_lun limit

2016-03-23 Thread Hannes Reinecke
On 03/21/2016 03:32 AM, Finn Thain wrote:
> The driver has a limit of eight LUs because of the byte-sized bitfield
> that is used for busy flags. That means the maximum LUN is 7. The default
> is 8.
> 
> Signed-off-by: Finn Thain <fth...@telegraphics.com.au>
> Tested-by: Michael Schmitz <schmitz...@gmail.com>
> 
> ---
> 
> Changed since v1:
> - Reduce shost->max_lun limit instead of adding 'MAX_LUN' limit.
> 
> ---
>  drivers/scsi/NCR5380.c |    2 ++
>  1 file changed, 2 insertions(+)
> 
Reviewed-by: Hannes Reinecke <h...@suse.com>

Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)


Re: [PATCH v3 14/23] ncr5380: Reduce max_lun limit

2016-03-23 Thread Hannes Reinecke
On 03/21/2016 03:32 AM, Finn Thain wrote:
> The driver has a limit of eight LUs because of the byte-sized bitfield
> that is used for busy flags. That means the maximum LUN is 7. The default
> is 8.
> 
> Signed-off-by: Finn Thain 
> Tested-by: Michael Schmitz 
> 
> ---
> 
> Changed since v1:
> - Reduce shost->max_lun limit instead of adding 'MAX_LUN' limit.
> 
> ---
>  drivers/scsi/NCR5380.c |2 ++
>  1 file changed, 2 insertions(+)
> 
Reviewed-by: Hannes Reinecke 

Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)


Re: [PATCH v3 04/23] atari_NCR5380: Remove DMA_MIN_SIZE macro

2016-03-23 Thread Hannes Reinecke
On 03/21/2016 03:31 AM, Finn Thain wrote:
> Only the atari_scsi and sun3_scsi drivers define DMA_MIN_SIZE.
> Both drivers also define NCR5380_dma_xfer_len, which means
> DMA_MIN_SIZE can be removed from the core driver.
> 
> This removes another discrepancy between the two core drivers.
> 
> Signed-off-by: Finn Thain <fth...@telegraphics.com.au>
> Tested-by: Michael Schmitz <schmitz...@gmail.com>
> 
> ---
> 
> Changes since v1:
> - Retain MIN_DMA_SIZE macro in wrapper drivers.
> 
Reviewed-by: Hannes Reinecke <h...@suse.com>

Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)


Re: [PATCH v3 04/23] atari_NCR5380: Remove DMA_MIN_SIZE macro

2016-03-23 Thread Hannes Reinecke
On 03/21/2016 03:31 AM, Finn Thain wrote:
> Only the atari_scsi and sun3_scsi drivers define DMA_MIN_SIZE.
> Both drivers also define NCR5380_dma_xfer_len, which means
> DMA_MIN_SIZE can be removed from the core driver.
> 
> This removes another discrepancy between the two core drivers.
> 
> Signed-off-by: Finn Thain 
> Tested-by: Michael Schmitz 
> 
> ---
> 
> Changes since v1:
> - Retain MIN_DMA_SIZE macro in wrapper drivers.
> 
Reviewed-by: Hannes Reinecke 

Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)


Re: [PATCH] scsi: fc: use get/put_unaligned64 for wwn access

2016-03-19 Thread Hannes Reinecke
On 03/16/2016 05:39 PM, Arnd Bergmann wrote:
> A bug in the gcc-6.0 prerelease version caused at least one
> driver (lpfc) to have excessive stack usage when dealing with
> wwn data, on the ARM architecture.
> 
> lpfc_scsi.c: In function 'lpfc_find_next_oas_lun':
> lpfc_scsi.c:117:1: warning: the frame size of 1152 bytes is larger than 1024 
> bytes [-Wframe-larger-than=]
> 
> I have reported this as a gcc regression in
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70232
> 
> However, using a better implementation of wwn_to_u64() not only
> helps with the particular gcc problem but also leads to better
> object code for any version or architecture.
> 
> The kernel already provides get_unaligned_be64() and
> put_unaligned_be64() helper functions that provide an
> optimized implementation with the desired semantics.
> 
> The lpfc_find_next_oas_lun() function in the example that
> grew from 1146 bytes to 5144 bytes when moving from gcc-5.3
> to gcc-6.0 is now 804 bytes, as the optimized
> get_unaligned_be64() load can be done in three instructions.
> The stack usage is now down to 28 bytes from 128 bytes with
> gcc-5.3 before.
> 
> Signed-off-by: Arnd Bergmann <a...@arndb.de>
> ---
>  include/scsi/scsi_transport_fc.h | 15 +++----
>  1 file changed, 3 insertions(+), 12 deletions(-)
> 
Reviewed-by: Hannes Reinecke <h...@suse.com>

Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)


Re: [PATCH] scsi: fc: use get/put_unaligned64 for wwn access

2016-03-19 Thread Hannes Reinecke
On 03/16/2016 05:39 PM, Arnd Bergmann wrote:
> A bug in the gcc-6.0 prerelease version caused at least one
> driver (lpfc) to have excessive stack usage when dealing with
> wwn data, on the ARM architecture.
> 
> lpfc_scsi.c: In function 'lpfc_find_next_oas_lun':
> lpfc_scsi.c:117:1: warning: the frame size of 1152 bytes is larger than 1024 
> bytes [-Wframe-larger-than=]
> 
> I have reported this as a gcc regression in
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70232
> 
> However, using a better implementation of wwn_to_u64() not only
> helps with the particular gcc problem but also leads to better
> object code for any version or architecture.
> 
> The kernel already provides get_unaligned_be64() and
> put_unaligned_be64() helper functions that provide an
> optimized implementation with the desired semantics.
> 
> The lpfc_find_next_oas_lun() function in the example that
> grew from 1146 bytes to 5144 bytes when moving from gcc-5.3
> to gcc-6.0 is now 804 bytes, as the optimized
> get_unaligned_be64() load can be done in three instructions.
> The stack usage is now down to 28 bytes from 128 bytes with
> gcc-5.3 before.
> 
> Signed-off-by: Arnd Bergmann 
> ---
>  include/scsi/scsi_transport_fc.h | 15 +++----
>  1 file changed, 3 insertions(+), 12 deletions(-)
> 
Reviewed-by: Hannes Reinecke 

Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)


Re: [PATCH 20/22] atari_scsi: Set a reasonable default for cmd_per_lun

2016-03-15 Thread Hannes Reinecke
On 03/15/2016 04:27 AM, Finn Thain wrote:
> 
> On Mon, 14 Mar 2016, Hannes Reinecke wrote:
> 
>> On 03/14/2016 05:27 AM, Finn Thain wrote:
>>> This setting does not need to be conditional on Atari ST or TT.
>>>
>>> Without TCQ support, cmd_per_lun == 2 is probably reasonable...
>>>
>>> Signed-off-by: Finn Thain <fth...@telegraphics.com.au>
>>>
>>> ---
>>>  drivers/scsi/atari_scsi.c |3 +--
>>>  1 file changed, 1 insertion(+), 2 deletions(-)
>>>
>>> Index: linux/drivers/scsi/atari_scsi.c
>>> ===
>>> --- linux.orig/drivers/scsi/atari_scsi.c2016-03-14 15:26:45.0 
>>> +1100
>>> +++ linux/drivers/scsi/atari_scsi.c 2016-03-14 15:26:55.0 +1100
>>> @@ -750,6 +750,7 @@ static struct scsi_host_template atari_s
>>> .eh_abort_handler   = atari_scsi_abort,
>>> .eh_bus_reset_handler   = atari_scsi_bus_reset,
>>> .this_id= 7,
>>> +   .cmd_per_lun= 2,
>>> .use_clustering = DISABLE_CLUSTERING,
>>> .cmd_size   = NCR5380_CMD_SIZE,
>>>  };
>> _2_ ? Are you being overly cheeky here?
>> I sincerely doubt the driver is capable of submitting two
>> simultaneous commands ...
> 
> Right. The LLD has LU busy flags to prevent a LU from being issued more 
> than one command.
> 
>> Care to explain?
> 
> It seemed harmless and it is consistent with the all of the other 5380 
> drivers.
> 
> I don't know why it was done that way. Perhaps it was done to create a 
> pipeline. That is, to keep a small number of commands in the LLD issue 
> queue so that the NCR5380_main() work item does not have to terminate and 
> then get requeued needlessly.
> 
Like I suspected.
While I'm aware of the reasoning, I sincerely doubt whether it makes
any difference in real life.
After all, a 'BUSY' return value still relies on someone kicking the
queue so that the next command can be submitted. So it's not much
different from using a queuedepth of '1' and use the 'official' way.

Have you done any benchmarking here?
Would be very interesting to check if it makes a difference in real
life ...

Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)


Re: [PATCH 20/22] atari_scsi: Set a reasonable default for cmd_per_lun

2016-03-15 Thread Hannes Reinecke
On 03/15/2016 04:27 AM, Finn Thain wrote:
> 
> On Mon, 14 Mar 2016, Hannes Reinecke wrote:
> 
>> On 03/14/2016 05:27 AM, Finn Thain wrote:
>>> This setting does not need to be conditional on Atari ST or TT.
>>>
>>> Without TCQ support, cmd_per_lun == 2 is probably reasonable...
>>>
>>> Signed-off-by: Finn Thain 
>>>
>>> ---
>>>  drivers/scsi/atari_scsi.c |3 +--
>>>  1 file changed, 1 insertion(+), 2 deletions(-)
>>>
>>> Index: linux/drivers/scsi/atari_scsi.c
>>> ===
>>> --- linux.orig/drivers/scsi/atari_scsi.c2016-03-14 15:26:45.0 
>>> +1100
>>> +++ linux/drivers/scsi/atari_scsi.c 2016-03-14 15:26:55.0 +1100
>>> @@ -750,6 +750,7 @@ static struct scsi_host_template atari_s
>>> .eh_abort_handler   = atari_scsi_abort,
>>> .eh_bus_reset_handler   = atari_scsi_bus_reset,
>>> .this_id= 7,
>>> +   .cmd_per_lun= 2,
>>> .use_clustering = DISABLE_CLUSTERING,
>>> .cmd_size   = NCR5380_CMD_SIZE,
>>>  };
>> _2_ ? Are you being overly cheeky here?
>> I sincerely doubt the driver is capable of submitting two
>> simultaneous commands ...
> 
> Right. The LLD has LU busy flags to prevent a LU from being issued more 
> than one command.
> 
>> Care to explain?
> 
> It seemed harmless and it is consistent with the all of the other 5380 
> drivers.
> 
> I don't know why it was done that way. Perhaps it was done to create a 
> pipeline. That is, to keep a small number of commands in the LLD issue 
> queue so that the NCR5380_main() work item does not have to terminate and 
> then get requeued needlessly.
> 
Like I suspected.
While I'm aware of the reasoning, I sincerely doubt whether it makes
any difference in real life.
After all, a 'BUSY' return value still relies on someone kicking the
queue so that the next command can be submitted. So it's not much
different from using a queuedepth of '1' and use the 'official' way.

Have you done any benchmarking here?
Would be very interesting to check if it makes a difference in real
life ...

Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)


Re: [PATCH 09/22] ncr5380: Adopt uniform DMA setup convention

2016-03-15 Thread Hannes Reinecke
On 03/15/2016 04:19 AM, Finn Thain wrote:
> 
> On Mon, 14 Mar 2016, Hannes Reinecke wrote:
> 
>>> @@ -1555,8 +1555,7 @@ static int NCR5380_transfer_dma(struct S
>>> NCR5380_read(RESET_PARITY_INTERRUPT_REG);
>>> *data = d + c;
>>> *count = 0;
>>> -   *phase = NCR5380_read(STATUS_REG) & PHASE_MASK;
>>> -   return foo;
>>> +   return result;
>>>  }
>>>  
>>>  /*
>>
>> Don't you miss a phase update here?
> 
> I guess I missed explaining the change in the commit log.
> 
> The *phase assignment is redundant because after NCR5380_transfer_dma() 
> returns control to NCR5380_information_transfer(), the latter routine then 
> also returns, and so *phase is dead.
> 
Right. Please add this to the commit message.

Otherwise:

Reviewed-by: Hannes Reinecke <h...@suse.com>

Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)


Re: [PATCH 09/22] ncr5380: Adopt uniform DMA setup convention

2016-03-15 Thread Hannes Reinecke
On 03/15/2016 04:19 AM, Finn Thain wrote:
> 
> On Mon, 14 Mar 2016, Hannes Reinecke wrote:
> 
>>> @@ -1555,8 +1555,7 @@ static int NCR5380_transfer_dma(struct S
>>> NCR5380_read(RESET_PARITY_INTERRUPT_REG);
>>> *data = d + c;
>>> *count = 0;
>>> -   *phase = NCR5380_read(STATUS_REG) & PHASE_MASK;
>>> -   return foo;
>>> +   return result;
>>>  }
>>>  
>>>  /*
>>
>> Don't you miss a phase update here?
> 
> I guess I missed explaining the change in the commit log.
> 
> The *phase assignment is redundant because after NCR5380_transfer_dma() 
> returns control to NCR5380_information_transfer(), the latter routine then 
> also returns, and so *phase is dead.
> 
Right. Please add this to the commit message.

Otherwise:

Reviewed-by: Hannes Reinecke 

Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)


Re: [PATCH 2/3] lpfc: fix misleading indentation

2016-03-14 Thread Hannes Reinecke
On 03/14/2016 04:25 PM, Arnd Bergmann wrote:
> On Monday 14 March 2016 16:19:58 Hannes Reinecke wrote:
>>>   vports = lpfc_create_vport_work_array(phba);
>>> - if (vports != NULL)
>>> + if (vports != NULL) {
>>>   for (i = 0; i <= phba->max_vports && vports[i] != NULL; i++) {
>>>   struct Scsi_Host *shost;
>>>   shost = lpfc_shost_from_vport(vports[i]);
>>> @@ -2877,7 +2877,8 @@ lpfc_online(struct lpfc_hba *phba)
>>>   }
>>>   spin_unlock_irq(shost->host_lock);
>>>   }
>>> - lpfc_destroy_vport_work_array(phba, vports);
>>> + }
>>> + lpfc_destroy_vport_work_array(phba, vports);
>>>  
>>>   lpfc_unblock_mgmt_io(phba);
>>>   return 0;
>>>
>> Nope.
>>
>> vports is only valid from within the indentation block, so it should
>> be moved into it.
>>
>>
> 
> Well, every other user of the function also looks like
> 
>   vports = lpfc_create_vport_work_array(phba);
>   if (vports != NULL) {
>   do_something(vports);
>   }
>       lpfc_destroy_vport_work_array(phba, vports);
> 
> and lpfc_destroy_vport_work_array() does nothing if its argument is NULL.
> 
> I still think my patch is the correct fix for the warning.
> 
Okay, good point.

Reviewed-by: Hannes Reinecke <h...@suse.com>

Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)


Re: [PATCH 2/3] lpfc: fix misleading indentation

2016-03-14 Thread Hannes Reinecke
On 03/14/2016 04:25 PM, Arnd Bergmann wrote:
> On Monday 14 March 2016 16:19:58 Hannes Reinecke wrote:
>>>   vports = lpfc_create_vport_work_array(phba);
>>> - if (vports != NULL)
>>> + if (vports != NULL) {
>>>   for (i = 0; i <= phba->max_vports && vports[i] != NULL; i++) {
>>>   struct Scsi_Host *shost;
>>>   shost = lpfc_shost_from_vport(vports[i]);
>>> @@ -2877,7 +2877,8 @@ lpfc_online(struct lpfc_hba *phba)
>>>   }
>>>   spin_unlock_irq(shost->host_lock);
>>>   }
>>> - lpfc_destroy_vport_work_array(phba, vports);
>>> + }
>>> + lpfc_destroy_vport_work_array(phba, vports);
>>>  
>>>   lpfc_unblock_mgmt_io(phba);
>>>   return 0;
>>>
>> Nope.
>>
>> vports is only valid from within the indentation block, so it should
>> be moved into it.
>>
>>
> 
> Well, every other user of the function also looks like
> 
>   vports = lpfc_create_vport_work_array(phba);
>   if (vports != NULL) {
>   do_something(vports);
>   }
>       lpfc_destroy_vport_work_array(phba, vports);
> 
> and lpfc_destroy_vport_work_array() does nothing if its argument is NULL.
> 
> I still think my patch is the correct fix for the warning.
> 
Okay, good point.

Reviewed-by: Hannes Reinecke 

Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)


Re: [PATCH 3/3] megaraid_sas: add missing curly braces in ioctl handler

2016-03-14 Thread Hannes Reinecke
On 03/14/2016 03:29 PM, Arnd Bergmann wrote:
> gcc-6 found a dubious indentation in the megasas_mgmt_fw_ioctl
> function:
> 
> drivers/scsi/megaraid/megaraid_sas_base.c: In function 
> 'megasas_mgmt_fw_ioctl':
> drivers/scsi/megaraid/megaraid_sas_base.c:6658:4: warning: statement is 
> indented as if it were guarded by... [-Wmisleading-indentation]
> kbuff_arr[i] = NULL;
> ^
> drivers/scsi/megaraid/megaraid_sas_base.c:6653:3: note: ...this 'if' clause, 
> but it is not
>if (kbuff_arr[i])
>^~
> 
> The code is actually correct, as there is no downside in clearing a NULL
> pointer again.
> 
> This clarifies the code and avoids the warning by adding extra curly
> braces.
> 
> Signed-off-by: Arnd Bergmann <a...@arndb.de>
> Fixes: 90dc9d98f01b ("megaraid_sas : MFI MPT linked list corruption fix")
> ---
>  drivers/scsi/megaraid/megaraid_sas_base.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/scsi/megaraid/megaraid_sas_base.c 
> b/drivers/scsi/megaraid/megaraid_sas_base.c
> index 5c08568ccfbf..2627200d4f82 100644
> --- a/drivers/scsi/megaraid/megaraid_sas_base.c
> +++ b/drivers/scsi/megaraid/megaraid_sas_base.c
> @@ -6650,12 +6650,13 @@ out:
>   }
>  
>   for (i = 0; i < ioc->sge_count; i++) {
> - if (kbuff_arr[i])
> + if (kbuff_arr[i]) {
>   dma_free_coherent(>pdev->dev,
> le32_to_cpu(kern_sge32[i].length),
> kbuff_arr[i],
> le32_to_cpu(kern_sge32[i].phys_addr));
>   kbuff_arr[i] = NULL;
> +     }
>   }
>  
>   megasas_return_cmd(instance, cmd);
> 
Reviewed-by: Hannes Reinecke <h...@suse.com>

Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)


Re: [PATCH 3/3] megaraid_sas: add missing curly braces in ioctl handler

2016-03-14 Thread Hannes Reinecke
On 03/14/2016 03:29 PM, Arnd Bergmann wrote:
> gcc-6 found a dubious indentation in the megasas_mgmt_fw_ioctl
> function:
> 
> drivers/scsi/megaraid/megaraid_sas_base.c: In function 
> 'megasas_mgmt_fw_ioctl':
> drivers/scsi/megaraid/megaraid_sas_base.c:6658:4: warning: statement is 
> indented as if it were guarded by... [-Wmisleading-indentation]
> kbuff_arr[i] = NULL;
> ^
> drivers/scsi/megaraid/megaraid_sas_base.c:6653:3: note: ...this 'if' clause, 
> but it is not
>if (kbuff_arr[i])
>^~
> 
> The code is actually correct, as there is no downside in clearing a NULL
> pointer again.
> 
> This clarifies the code and avoids the warning by adding extra curly
> braces.
> 
> Signed-off-by: Arnd Bergmann 
> Fixes: 90dc9d98f01b ("megaraid_sas : MFI MPT linked list corruption fix")
> ---
>  drivers/scsi/megaraid/megaraid_sas_base.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/scsi/megaraid/megaraid_sas_base.c 
> b/drivers/scsi/megaraid/megaraid_sas_base.c
> index 5c08568ccfbf..2627200d4f82 100644
> --- a/drivers/scsi/megaraid/megaraid_sas_base.c
> +++ b/drivers/scsi/megaraid/megaraid_sas_base.c
> @@ -6650,12 +6650,13 @@ out:
>   }
>  
>   for (i = 0; i < ioc->sge_count; i++) {
> - if (kbuff_arr[i])
> + if (kbuff_arr[i]) {
>   dma_free_coherent(>pdev->dev,
> le32_to_cpu(kern_sge32[i].length),
> kbuff_arr[i],
> le32_to_cpu(kern_sge32[i].phys_addr));
>   kbuff_arr[i] = NULL;
> + }
>   }
>  
>   megasas_return_cmd(instance, cmd);
> 
Reviewed-by: Hannes Reinecke 

Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)


Re: [PATCH 2/3] lpfc: fix misleading indentation

2016-03-14 Thread Hannes Reinecke
On 03/14/2016 03:29 PM, Arnd Bergmann wrote:
> gcc-6 complains about the indentation of the lpfc_destroy_vport_work_array()
> call in lpfc_online(), which clearly doesn't look right:
> 
> drivers/scsi/lpfc/lpfc_init.c: In function 'lpfc_online':
> drivers/scsi/lpfc/lpfc_init.c:2880:3: warning: statement is indented as if it 
> were guarded by... [-Wmisleading-indentation]
>lpfc_destroy_vport_work_array(phba, vports);
>^
> drivers/scsi/lpfc/lpfc_init.c:2863:2: note: ...this 'if' clause, but it is not
>   if (vports != NULL)
>   ^~
> 
> Looking at the patch that introduced this code, it's clear that the
> behavior is correct and the indentation is wrong.
> 
> This fixes the indentation and adds curly braces around the previous
> if() block for clarity, as that is most likely what caused the code
> to be misindented in the first place.
> 
> Signed-off-by: Arnd Bergmann <a...@arndb.de>
> Fixes: 549e55cd2a1b ("[SCSI] lpfc 8.2.2 : Fix locking around HBA's port_list")
> ---
>  drivers/scsi/lpfc/lpfc_init.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/scsi/lpfc/lpfc_init.c b/drivers/scsi/lpfc/lpfc_init.c
> index a544366a367e..f57d02c3b6cf 100644
> --- a/drivers/scsi/lpfc/lpfc_init.c
> +++ b/drivers/scsi/lpfc/lpfc_init.c
> @@ -2860,7 +2860,7 @@ lpfc_online(struct lpfc_hba *phba)
>   }
>  
>   vports = lpfc_create_vport_work_array(phba);
> - if (vports != NULL)
> + if (vports != NULL) {
>   for (i = 0; i <= phba->max_vports && vports[i] != NULL; i++) {
>   struct Scsi_Host *shost;
>   shost = lpfc_shost_from_vport(vports[i]);
> @@ -2877,7 +2877,8 @@ lpfc_online(struct lpfc_hba *phba)
>   }
>   spin_unlock_irq(shost->host_lock);
>   }
> - lpfc_destroy_vport_work_array(phba, vports);
> + }
> + lpfc_destroy_vport_work_array(phba, vports);
>  
>   lpfc_unblock_mgmt_io(phba);
>   return 0;
> 
Nope.

vports is only valid from within the indentation block, so it should
be moved into it.

Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)


Re: [PATCH 2/3] lpfc: fix misleading indentation

2016-03-14 Thread Hannes Reinecke
On 03/14/2016 03:29 PM, Arnd Bergmann wrote:
> gcc-6 complains about the indentation of the lpfc_destroy_vport_work_array()
> call in lpfc_online(), which clearly doesn't look right:
> 
> drivers/scsi/lpfc/lpfc_init.c: In function 'lpfc_online':
> drivers/scsi/lpfc/lpfc_init.c:2880:3: warning: statement is indented as if it 
> were guarded by... [-Wmisleading-indentation]
>lpfc_destroy_vport_work_array(phba, vports);
>^
> drivers/scsi/lpfc/lpfc_init.c:2863:2: note: ...this 'if' clause, but it is not
>   if (vports != NULL)
>   ^~
> 
> Looking at the patch that introduced this code, it's clear that the
> behavior is correct and the indentation is wrong.
> 
> This fixes the indentation and adds curly braces around the previous
> if() block for clarity, as that is most likely what caused the code
> to be misindented in the first place.
> 
> Signed-off-by: Arnd Bergmann 
> Fixes: 549e55cd2a1b ("[SCSI] lpfc 8.2.2 : Fix locking around HBA's port_list")
> ---
>  drivers/scsi/lpfc/lpfc_init.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/scsi/lpfc/lpfc_init.c b/drivers/scsi/lpfc/lpfc_init.c
> index a544366a367e..f57d02c3b6cf 100644
> --- a/drivers/scsi/lpfc/lpfc_init.c
> +++ b/drivers/scsi/lpfc/lpfc_init.c
> @@ -2860,7 +2860,7 @@ lpfc_online(struct lpfc_hba *phba)
>   }
>  
>   vports = lpfc_create_vport_work_array(phba);
> - if (vports != NULL)
> + if (vports != NULL) {
>   for (i = 0; i <= phba->max_vports && vports[i] != NULL; i++) {
>   struct Scsi_Host *shost;
>   shost = lpfc_shost_from_vport(vports[i]);
> @@ -2877,7 +2877,8 @@ lpfc_online(struct lpfc_hba *phba)
>   }
>   spin_unlock_irq(shost->host_lock);
>   }
> - lpfc_destroy_vport_work_array(phba, vports);
> + }
> + lpfc_destroy_vport_work_array(phba, vports);
>  
>   lpfc_unblock_mgmt_io(phba);
>   return 0;
> 
Nope.

vports is only valid from within the indentation block, so it should
be moved into it.

Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)


Re: [PATCH 22/22] mac_scsi: Fix pseudo DMA implementation

2016-03-14 Thread Hannes Reinecke
On 03/14/2016 05:27 AM, Finn Thain wrote:
> Fix various issues: Comments about bus errors are incorrect. The
> PDMA asm must return the size of the memory access that faulted so the
> transfer count can be adjusted accordingly. A phase change may cause a
> bus error but should not be treated as failure. A bus error does not
> always imply a phase change and generally the transfer may continue.
> Scatter/gather can't be used with PDMA due to overruns (which is a pity
> because peak throughput seems to double with SG_ALL).
> 
> Signed-off-by: Finn Thain <fth...@telegraphics.com.au>
> 
> ---
>  drivers/scsi/NCR5380.h  |2 
>  drivers/scsi/mac_scsi.c |  210 
> ++--
>  2 files changed, 118 insertions(+), 94 deletions(-)
> 
Reviewed-by: Hannes Reinecke <h...@suse.com>

Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)


Re: [PATCH 22/22] mac_scsi: Fix pseudo DMA implementation

2016-03-14 Thread Hannes Reinecke
On 03/14/2016 05:27 AM, Finn Thain wrote:
> Fix various issues: Comments about bus errors are incorrect. The
> PDMA asm must return the size of the memory access that faulted so the
> transfer count can be adjusted accordingly. A phase change may cause a
> bus error but should not be treated as failure. A bus error does not
> always imply a phase change and generally the transfer may continue.
> Scatter/gather can't be used with PDMA due to overruns (which is a pity
> because peak throughput seems to double with SG_ALL).
> 
> Signed-off-by: Finn Thain 
> 
> ---
>  drivers/scsi/NCR5380.h  |2 
>  drivers/scsi/mac_scsi.c |  210 
> ++--
>  2 files changed, 118 insertions(+), 94 deletions(-)
> 
Reviewed-by: Hannes Reinecke 

Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)


Re: [PATCH 21/22] atari_scsi: Allow can_queue to be increased for Falcon

2016-03-14 Thread Hannes Reinecke
On 03/14/2016 05:27 AM, Finn Thain wrote:
> The benefit of limiting can_queue to 1 is that atari_scsi shares the
> ST DMA chip more fairly with other drivers (e.g. falcon-ide).
> 
> Unfortunately, this can limit SCSI bus utilization. On systems without
> IDE, atari_scsi should issue SCSI commands whenever it can arbitrate for
> the bus. Make that possible by making can_queue configurable.
> 
> Signed-off-by: Finn Thain <fth...@telegraphics.com.au>
> 
> ---
>  drivers/scsi/atari_scsi.c |   83 
> --
>  1 file changed, 22 insertions(+), 61 deletions(-)
> 
Hmm. We actually have a similar reasoning in the aic7xxx driver,
where the driver core wants to setup pointers to the _next_ command.
So maybe it makes sense to add a pointer to the 'next' command
somewhere in the NCR core structure, to be submitted whenever bus
arbitration tells you so?

But in the end it really looks like a real issue somewhere in the
code, be it the driver or the block/scsi layer.

Nevertheless, setting it to '1' is certainly the correct way here.

Reviewed-by: Hannes Reinecke <h...@suse.com>

Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)


Re: [PATCH 21/22] atari_scsi: Allow can_queue to be increased for Falcon

2016-03-14 Thread Hannes Reinecke
On 03/14/2016 05:27 AM, Finn Thain wrote:
> The benefit of limiting can_queue to 1 is that atari_scsi shares the
> ST DMA chip more fairly with other drivers (e.g. falcon-ide).
> 
> Unfortunately, this can limit SCSI bus utilization. On systems without
> IDE, atari_scsi should issue SCSI commands whenever it can arbitrate for
> the bus. Make that possible by making can_queue configurable.
> 
> Signed-off-by: Finn Thain 
> 
> ---
>  drivers/scsi/atari_scsi.c |   83 
> --
>  1 file changed, 22 insertions(+), 61 deletions(-)
> 
Hmm. We actually have a similar reasoning in the aic7xxx driver,
where the driver core wants to setup pointers to the _next_ command.
So maybe it makes sense to add a pointer to the 'next' command
somewhere in the NCR core structure, to be submitted whenever bus
arbitration tells you so?

But in the end it really looks like a real issue somewhere in the
code, be it the driver or the block/scsi layer.

Nevertheless, setting it to '1' is certainly the correct way here.

Reviewed-by: Hannes Reinecke 

Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)


Re: [PATCH 20/22] atari_scsi: Set a reasonable default for cmd_per_lun

2016-03-14 Thread Hannes Reinecke
On 03/14/2016 05:27 AM, Finn Thain wrote:
> This setting does not need to be conditional on Atari ST or TT.
> 
> Without TCQ support, cmd_per_lun == 2 is probably reasonable...
> 
> Signed-off-by: Finn Thain <fth...@telegraphics.com.au>
> 
> ---
>  drivers/scsi/atari_scsi.c |3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> Index: linux/drivers/scsi/atari_scsi.c
> ===
> --- linux.orig/drivers/scsi/atari_scsi.c  2016-03-14 15:26:45.0 
> +1100
> +++ linux/drivers/scsi/atari_scsi.c   2016-03-14 15:26:55.0 +1100
> @@ -750,6 +750,7 @@ static struct scsi_host_template atari_s
>   .eh_abort_handler   = atari_scsi_abort,
>   .eh_bus_reset_handler   = atari_scsi_bus_reset,
>   .this_id= 7,
> + .cmd_per_lun= 2,
>   .use_clustering = DISABLE_CLUSTERING,
>   .cmd_size   = NCR5380_CMD_SIZE,
>  };
_2_ ? Are you being overly cheeky here?
I sincerely doubt the driver is capable of submitting two
simultaneous commands ...
Care to explain?

Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)


Re: [PATCH 20/22] atari_scsi: Set a reasonable default for cmd_per_lun

2016-03-14 Thread Hannes Reinecke
On 03/14/2016 05:27 AM, Finn Thain wrote:
> This setting does not need to be conditional on Atari ST or TT.
> 
> Without TCQ support, cmd_per_lun == 2 is probably reasonable...
> 
> Signed-off-by: Finn Thain 
> 
> ---
>  drivers/scsi/atari_scsi.c |3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> Index: linux/drivers/scsi/atari_scsi.c
> ===
> --- linux.orig/drivers/scsi/atari_scsi.c  2016-03-14 15:26:45.0 
> +1100
> +++ linux/drivers/scsi/atari_scsi.c   2016-03-14 15:26:55.0 +1100
> @@ -750,6 +750,7 @@ static struct scsi_host_template atari_s
>   .eh_abort_handler   = atari_scsi_abort,
>   .eh_bus_reset_handler   = atari_scsi_bus_reset,
>   .this_id= 7,
> + .cmd_per_lun= 2,
>   .use_clustering = DISABLE_CLUSTERING,
>   .cmd_size   = NCR5380_CMD_SIZE,
>  };
_2_ ? Are you being overly cheeky here?
I sincerely doubt the driver is capable of submitting two
simultaneous commands ...
Care to explain?

Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)


Re: [PATCH 19/22] ncr5380: Update usage documentation

2016-03-14 Thread Hannes Reinecke
On 03/14/2016 05:27 AM, Finn Thain wrote:
> Update kernel parameter documentation for atari_scsi, mac_scsi and
> g_NCR5380 drivers. Remove duplication.
> 
> Signed-off-by: Finn Thain <fth...@telegraphics.com.au>
> 
> ---
>  Documentation/scsi/g_NCR5380.txt   |   17 ++-
>  Documentation/scsi/scsi-parameters.txt |   11 +++---
>  drivers/scsi/g_NCR5380.c   |   36 
> -
>  3 files changed, 16 insertions(+), 48 deletions(-)
> 
Reviewed-by: Hannes Reinecke <h...@suse.com>

Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)


Re: [PATCH 19/22] ncr5380: Update usage documentation

2016-03-14 Thread Hannes Reinecke
On 03/14/2016 05:27 AM, Finn Thain wrote:
> Update kernel parameter documentation for atari_scsi, mac_scsi and
> g_NCR5380 drivers. Remove duplication.
> 
> Signed-off-by: Finn Thain 
> 
> ---
>  Documentation/scsi/g_NCR5380.txt   |   17 ++-
>  Documentation/scsi/scsi-parameters.txt |   11 +++---
>  drivers/scsi/g_NCR5380.c   |   36 
> -
>  3 files changed, 16 insertions(+), 48 deletions(-)
> 
Reviewed-by: Hannes Reinecke 

Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)


Re: [PATCH 18/22] ncr5380: Remove DONT_USE_INTR and AUTOPROBE_IRQ macros

2016-03-14 Thread Hannes Reinecke
On 03/14/2016 05:27 AM, Finn Thain wrote:
> Signed-off-by: Finn Thain <fth...@telegraphics.com.au>
> 
> ---
>  drivers/scsi/NCR5380.c   |   12 +---
>  drivers/scsi/NCR5380.h   |4 
>  drivers/scsi/arm/oak.c   |2 --
>  drivers/scsi/dmx3191d.c  |2 --
>  drivers/scsi/dtc.c   |   12 +++-
>  drivers/scsi/g_NCR5380.c |2 --
>  drivers/scsi/pas16.c |1 -
>  drivers/scsi/t128.c  |1 -
>  8 files changed, 4 insertions(+), 32 deletions(-)
> 
Reviewed-by: Hannes Reinecke <h...@suse.com>

Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)


Re: [PATCH 17/22] ncr5380: Remove remaining register storage qualifiers

2016-03-14 Thread Hannes Reinecke
On 03/14/2016 05:27 AM, Finn Thain wrote:
> Signed-off-by: Finn Thain <fth...@telegraphics.com.au>
> 
> ---
>  drivers/scsi/NCR5380.c |6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> Index: linux/drivers/scsi/NCR5380.c
> ===
> --- linux.orig/drivers/scsi/NCR5380.c 2016-03-14 15:26:50.0 +1100
> +++ linux/drivers/scsi/NCR5380.c  2016-03-14 15:26:51.0 +1100
> @@ -1559,9 +1559,9 @@ static int NCR5380_transfer_dma(struct S
>   unsigned char **data)
>  {
>   struct NCR5380_hostdata *hostdata = shost_priv(instance);
> - register int c = *count;
> - register unsigned char p = *phase;
> - register unsigned char *d = *data;
> + int c = *count;
> + unsigned char p = *phase;
> + unsigned char *d = *data;
>   unsigned char tmp;
>   int result = 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
> 
Reviewed-by: Hannes Reinecke <h...@suse.com>

Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)


Re: [PATCH 18/22] ncr5380: Remove DONT_USE_INTR and AUTOPROBE_IRQ macros

2016-03-14 Thread Hannes Reinecke
On 03/14/2016 05:27 AM, Finn Thain wrote:
> Signed-off-by: Finn Thain 
> 
> ---
>  drivers/scsi/NCR5380.c   |   12 +---
>  drivers/scsi/NCR5380.h   |4 
>  drivers/scsi/arm/oak.c   |2 --
>  drivers/scsi/dmx3191d.c  |2 --
>  drivers/scsi/dtc.c   |   12 +++-
>  drivers/scsi/g_NCR5380.c |2 --
>  drivers/scsi/pas16.c |1 -
>  drivers/scsi/t128.c  |1 -
>  8 files changed, 4 insertions(+), 32 deletions(-)
> 
Reviewed-by: Hannes Reinecke 

Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)


Re: [PATCH 17/22] ncr5380: Remove remaining register storage qualifiers

2016-03-14 Thread Hannes Reinecke
On 03/14/2016 05:27 AM, Finn Thain wrote:
> Signed-off-by: Finn Thain 
> 
> ---
>  drivers/scsi/NCR5380.c |6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> Index: linux/drivers/scsi/NCR5380.c
> ===
> --- linux.orig/drivers/scsi/NCR5380.c 2016-03-14 15:26:50.0 +1100
> +++ linux/drivers/scsi/NCR5380.c  2016-03-14 15:26:51.0 +1100
> @@ -1559,9 +1559,9 @@ static int NCR5380_transfer_dma(struct S
>   unsigned char **data)
>  {
>   struct NCR5380_hostdata *hostdata = shost_priv(instance);
> - register int c = *count;
> - register unsigned char p = *phase;
> - register unsigned char *d = *data;
> + int c = *count;
> + unsigned char p = *phase;
> + unsigned char *d = *data;
>   unsigned char tmp;
>   int result = 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
> 
Reviewed-by: Hannes Reinecke 

Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)


Re: [PATCH 16/22] ncr5380: Fix register decoding for debugging

2016-03-14 Thread Hannes Reinecke
On 03/14/2016 05:27 AM, Finn Thain wrote:
> Decode all bits in the chip registers. They are all useful at times.
> Fix printk severity so that this output can be suppressed along with
> the other debugging output.
> 
> Signed-off-by: Finn Thain <fth...@telegraphics.com.au>
> 
> ---
>  drivers/scsi/NCR5380.c |   42 +-
>  1 file changed, 25 insertions(+), 17 deletions(-)
> 
> Index: linux/drivers/scsi/NCR5380.c
> ===
> --- linux.orig/drivers/scsi/NCR5380.c 2016-03-14 15:26:48.0 +1100
> +++ linux/drivers/scsi/NCR5380.c  2016-03-14 15:26:50.0 +1100
> @@ -256,12 +256,20 @@ static struct {
>   {0, NULL}
>  },
>  basrs[] = {
> + {BASR_END_DMA_TRANSFER, "END OF DMA"},
> + {BASR_DRQ, "DRQ"},
> + {BASR_PARITY_ERROR, "PARITY ERROR"},
> + {BASR_IRQ, "IRQ"},
> + {BASR_PHASE_MATCH, "PHASE MATCH"},
> + {BASR_BUSY_ERROR, "BUSY ERROR"},
>   {BASR_ATN, "ATN"},
>   {BASR_ACK, "ACK"},
>   {0, NULL}
>  },
>  icrs[] = {
>   {ICR_ASSERT_RST, "ASSERT RST"},
> + {ICR_ARBITRATION_PROGRESS, "ARB. IN PROGRESS"},
> + {ICR_ARBITRATION_LOST, "LOST ARB."},
>   {ICR_ASSERT_ACK, "ASSERT ACK"},
>   {ICR_ASSERT_BSY, "ASSERT BSY"},
>   {ICR_ASSERT_SEL, "ASSERT SEL"},
> @@ -270,14 +278,14 @@ icrs[] = {
>   {0, NULL}
>  },
>  mrs[] = {
> - {MR_BLOCK_DMA_MODE, "MODE BLOCK DMA"},
> - {MR_TARGET, "MODE TARGET"},
> - {MR_ENABLE_PAR_CHECK, "MODE PARITY CHECK"},
> - {MR_ENABLE_PAR_INTR, "MODE PARITY INTR"},
> - {MR_ENABLE_EOP_INTR, "MODE EOP INTR"},
> - {MR_MONITOR_BSY, "MODE MONITOR BSY"},
> - {MR_DMA_MODE, "MODE DMA"},
> - {MR_ARBITRATE, "MODE ARBITRATION"},
> + {MR_BLOCK_DMA_MODE, "BLOCK DMA MODE"},
> + {MR_TARGET, "TARGET"},
> + {MR_ENABLE_PAR_CHECK, "PARITY CHECK"},
> + {MR_ENABLE_PAR_INTR, "PARITY INTR"},
> + {MR_ENABLE_EOP_INTR, "EOP INTR"},
> + {MR_MONITOR_BSY, "MONITOR BSY"},
> + {MR_DMA_MODE, "DMA MODE"},
> + {MR_ARBITRATE, "ARBITRATE"},
>   {0, NULL}
>  };
>  
> @@ -298,23 +306,23 @@ static void NCR5380_print(struct Scsi_Ho
>   icr = NCR5380_read(INITIATOR_COMMAND_REG);
>   basr = NCR5380_read(BUS_AND_STATUS_REG);
>  
> - printk("STATUS_REG: %02x ", status);
> + printk(KERN_DEBUG "SR =   0x%02x : ", status);
>   for (i = 0; signals[i].mask; ++i)
>   if (status & signals[i].mask)
> - printk(",%s", signals[i].name);
> - printk("\nBASR: %02x ", basr);
> + printk(KERN_CONT "%s, ", signals[i].name);
> + printk(KERN_CONT "\nBASR = 0x%02x : ", basr);
>   for (i = 0; basrs[i].mask; ++i)
>   if (basr & basrs[i].mask)
> - printk(",%s", basrs[i].name);
> - printk("\nICR: %02x ", icr);
> + printk(KERN_CONT "%s, ", basrs[i].name);
> + printk(KERN_CONT "\nICR =  0x%02x : ", icr);
>   for (i = 0; icrs[i].mask; ++i)
>   if (icr & icrs[i].mask)
> - printk(",%s", icrs[i].name);
> - printk("\nMODE: %02x ", mr);
> + printk(KERN_CONT "%s, ", icrs[i].name);
> + printk(KERN_CONT "\nMR =   0x%02x : ", mr);
>   for (i = 0; mrs[i].mask; ++i)
>   if (mr & mrs[i].mask)
> - printk(",%s", mrs[i].name);
> - printk("\n");
> + printk(KERN_CONT "%s, ", mrs[i].name);
> + printk(KERN_CONT "\n");
>  }
>  
>  static struct {
> 
> 
Well ... using individual printk() like here might end up in each
call to be broken up into individual lines.
You might want to consider using a line buffer here or, better
still, move to seq_file or debugfs output.

But as the original code did that, too:

Reviewed-by: Hannes Reinecke <h...@suse.com>

Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)


Re: [PATCH 16/22] ncr5380: Fix register decoding for debugging

2016-03-14 Thread Hannes Reinecke
On 03/14/2016 05:27 AM, Finn Thain wrote:
> Decode all bits in the chip registers. They are all useful at times.
> Fix printk severity so that this output can be suppressed along with
> the other debugging output.
> 
> Signed-off-by: Finn Thain 
> 
> ---
>  drivers/scsi/NCR5380.c |   42 +-
>  1 file changed, 25 insertions(+), 17 deletions(-)
> 
> Index: linux/drivers/scsi/NCR5380.c
> ===
> --- linux.orig/drivers/scsi/NCR5380.c 2016-03-14 15:26:48.0 +1100
> +++ linux/drivers/scsi/NCR5380.c  2016-03-14 15:26:50.0 +1100
> @@ -256,12 +256,20 @@ static struct {
>   {0, NULL}
>  },
>  basrs[] = {
> + {BASR_END_DMA_TRANSFER, "END OF DMA"},
> + {BASR_DRQ, "DRQ"},
> + {BASR_PARITY_ERROR, "PARITY ERROR"},
> + {BASR_IRQ, "IRQ"},
> + {BASR_PHASE_MATCH, "PHASE MATCH"},
> + {BASR_BUSY_ERROR, "BUSY ERROR"},
>   {BASR_ATN, "ATN"},
>   {BASR_ACK, "ACK"},
>   {0, NULL}
>  },
>  icrs[] = {
>   {ICR_ASSERT_RST, "ASSERT RST"},
> + {ICR_ARBITRATION_PROGRESS, "ARB. IN PROGRESS"},
> + {ICR_ARBITRATION_LOST, "LOST ARB."},
>   {ICR_ASSERT_ACK, "ASSERT ACK"},
>   {ICR_ASSERT_BSY, "ASSERT BSY"},
>   {ICR_ASSERT_SEL, "ASSERT SEL"},
> @@ -270,14 +278,14 @@ icrs[] = {
>   {0, NULL}
>  },
>  mrs[] = {
> - {MR_BLOCK_DMA_MODE, "MODE BLOCK DMA"},
> - {MR_TARGET, "MODE TARGET"},
> - {MR_ENABLE_PAR_CHECK, "MODE PARITY CHECK"},
> - {MR_ENABLE_PAR_INTR, "MODE PARITY INTR"},
> - {MR_ENABLE_EOP_INTR, "MODE EOP INTR"},
> - {MR_MONITOR_BSY, "MODE MONITOR BSY"},
> - {MR_DMA_MODE, "MODE DMA"},
> - {MR_ARBITRATE, "MODE ARBITRATION"},
> + {MR_BLOCK_DMA_MODE, "BLOCK DMA MODE"},
> + {MR_TARGET, "TARGET"},
> + {MR_ENABLE_PAR_CHECK, "PARITY CHECK"},
> + {MR_ENABLE_PAR_INTR, "PARITY INTR"},
> + {MR_ENABLE_EOP_INTR, "EOP INTR"},
> + {MR_MONITOR_BSY, "MONITOR BSY"},
> + {MR_DMA_MODE, "DMA MODE"},
> + {MR_ARBITRATE, "ARBITRATE"},
>   {0, NULL}
>  };
>  
> @@ -298,23 +306,23 @@ static void NCR5380_print(struct Scsi_Ho
>   icr = NCR5380_read(INITIATOR_COMMAND_REG);
>   basr = NCR5380_read(BUS_AND_STATUS_REG);
>  
> - printk("STATUS_REG: %02x ", status);
> + printk(KERN_DEBUG "SR =   0x%02x : ", status);
>   for (i = 0; signals[i].mask; ++i)
>   if (status & signals[i].mask)
> - printk(",%s", signals[i].name);
> - printk("\nBASR: %02x ", basr);
> + printk(KERN_CONT "%s, ", signals[i].name);
> + printk(KERN_CONT "\nBASR = 0x%02x : ", basr);
>   for (i = 0; basrs[i].mask; ++i)
>   if (basr & basrs[i].mask)
> - printk(",%s", basrs[i].name);
> - printk("\nICR: %02x ", icr);
> + printk(KERN_CONT "%s, ", basrs[i].name);
> + printk(KERN_CONT "\nICR =  0x%02x : ", icr);
>   for (i = 0; icrs[i].mask; ++i)
>   if (icr & icrs[i].mask)
> - printk(",%s", icrs[i].name);
> - printk("\nMODE: %02x ", mr);
> +     printk(KERN_CONT "%s, ", icrs[i].name);
> + printk(KERN_CONT "\nMR =   0x%02x : ", mr);
>   for (i = 0; mrs[i].mask; ++i)
>   if (mr & mrs[i].mask)
> - printk(",%s", mrs[i].name);
> - printk("\n");
> + printk(KERN_CONT "%s, ", mrs[i].name);
> + printk(KERN_CONT "\n");
>  }
>  
>  static struct {
> 
> 
Well ... using individual printk() like here might end up in each
call to be broken up into individual lines.
You might want to consider using a line buffer here or, better
still, move to seq_file or debugfs output.

But as the original code did that, too:

Reviewed-by: Hannes Reinecke 

Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)


Re: [PATCH 15/22] dmx3191d: Drop max_sectors limit

2016-03-14 Thread Hannes Reinecke
On 03/14/2016 05:27 AM, Finn Thain wrote:
> The dmx3191d driver is not capable of DMA or PDMA so all transfers
> use PIO. Now that large slow PIO transfers periodically stop and call
> cond_resched(), the max_sectors limit can go away.
> 
> Signed-off-by: Finn Thain <fth...@telegraphics.com.au>
> 
> ---
>  drivers/scsi/dmx3191d.c |1 -
>  1 file changed, 1 deletion(-)
> 
Reviewed-by: Hannes Reinecke <h...@suse.com>

Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)


Re: [PATCH 15/22] dmx3191d: Drop max_sectors limit

2016-03-14 Thread Hannes Reinecke
On 03/14/2016 05:27 AM, Finn Thain wrote:
> The dmx3191d driver is not capable of DMA or PDMA so all transfers
> use PIO. Now that large slow PIO transfers periodically stop and call
> cond_resched(), the max_sectors limit can go away.
> 
> Signed-off-by: Finn Thain 
> 
> ---
>  drivers/scsi/dmx3191d.c |1 -
>  1 file changed, 1 deletion(-)
> 
Reviewed-by: Hannes Reinecke 

Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)


Re: [PATCH 14/22] ncr5380: Add MAX_LUN limit

2016-03-14 Thread Hannes Reinecke
On 03/14/2016 05:27 AM, Finn Thain wrote:
> The driver has a limit of eight LUs because of the byte-sized bitfield
> that is used for busy flags. Reject commands with LUN > 7.
> 
> Signed-off-by: Finn Thain <fth...@telegraphics.com.au>
> 
> ---
>  drivers/scsi/NCR5380.c |6 ++
>  drivers/scsi/NCR5380.h |2 ++
>  2 files changed, 8 insertions(+)
> 
> Index: linux/drivers/scsi/NCR5380.c
> ===
> --- linux.orig/drivers/scsi/NCR5380.c 2016-03-14 15:26:45.0 +1100
> +++ linux/drivers/scsi/NCR5380.c  2016-03-14 15:26:48.0 +1100
> @@ -661,6 +661,12 @@ static int NCR5380_queue_command(struct
>   }
>  #endif /* (NDEBUG & NDEBUG_NO_WRITE) */
>  
> + if (cmd->device->lun > MAX_LUN) {
> + cmd->result = DID_NO_CONNECT << 16;
> + cmd->scsi_done(cmd);
> + return 0;
> + }
> +
>   cmd->result = 0;
>  
>   if (!NCR5380_acquire_dma_irq(instance))
> Index: linux/drivers/scsi/NCR5380.h
> ===
> --- linux.orig/drivers/scsi/NCR5380.h 2016-03-14 15:26:45.0 +1100
> +++ linux/drivers/scsi/NCR5380.h  2016-03-14 15:26:48.0 +1100
> @@ -244,6 +244,8 @@ struct NCR5380_hostdata {
>  
>  #ifdef __KERNEL__
>  
> +#define MAX_LUN  7
> +
>  struct NCR5380_cmd {
>   struct list_head list;
>  };
> 
> 
Why not simply use shost->max_lun ?

Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)


Re: [PATCH 13/22] ncr5380: Remove disused atari_NCR5380.c core driver

2016-03-14 Thread Hannes Reinecke
On 03/14/2016 05:27 AM, Finn Thain wrote:
> Now that atari_scsi and sun3_scsi have been converted to use the NCR5380.c
> core driver, remove atari_NCR5380.c. Also remove the last vestiges of its
> Tagged Command Queueing implementation from the wrapper drivers.
> 
> The TCQ support in atari_NCR5380.c is abandoned by this patch. It is not
> merged into the remaining core driver because,
> 
> 1) atari_scsi defines SUPPORT_TAGS but leaves FLAG_TAGGED_QUEUING disabled
> by default, which indicates that it is mostly undesirable.
> 
> 2) I'm told that it doesn't work correctly when enabled.
> 
> 3) The algorithm does not make use of block layer tags which it will have
> to do because scmd->tag is deprecated.
> 
> 4) sun3_scsi doesn't define SUPPORT_TAGS at all, yet the the SUPPORT_TAGS
> macro interacts with the CONFIG_SUN3 macro in 'interesting' ways.
> 
> 5) Compile-time configuration with macros like SUPPORT_TAGS caused the
> configuration space to explode, leading to untestable and unmaintainable
> code that is too hard to reason about.
> 
> The merge_contiguous_buffers() code is also abandoned. This was unused
> by sun3_scsi. Only atari_scsi used it and then only on TT, because only TT
> supports scatter/gather. I suspect that the TT would work fine with
> ENABLE_CLUSTERING instead. If someone can benchmark the difference then
> perhaps the merge_contiguous_buffers() code can be be justified. Until
> then we are better off without the extra complexity.
> 
> Signed-off-by: Finn Thain <fth...@telegraphics.com.au>
> 
> ---
>  drivers/scsi/NCR5380.c   |   22 
>  drivers/scsi/NCR5380.h   |   19 
>  drivers/scsi/atari_NCR5380.c | 2632 
> ---
>  drivers/scsi/atari_scsi.c|   11 
>  drivers/scsi/mac_scsi.c  |    8 
>  drivers/scsi/sun3_scsi.c |   11 
>  6 files changed, 4 insertions(+), 2699 deletions(-)
> 
Reviewed-by: Hannes Reinecke <h...@suse.com>

Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)


Re: [PATCH 14/22] ncr5380: Add MAX_LUN limit

2016-03-14 Thread Hannes Reinecke
On 03/14/2016 05:27 AM, Finn Thain wrote:
> The driver has a limit of eight LUs because of the byte-sized bitfield
> that is used for busy flags. Reject commands with LUN > 7.
> 
> Signed-off-by: Finn Thain 
> 
> ---
>  drivers/scsi/NCR5380.c |6 ++
>  drivers/scsi/NCR5380.h |2 ++
>  2 files changed, 8 insertions(+)
> 
> Index: linux/drivers/scsi/NCR5380.c
> ===
> --- linux.orig/drivers/scsi/NCR5380.c 2016-03-14 15:26:45.0 +1100
> +++ linux/drivers/scsi/NCR5380.c  2016-03-14 15:26:48.0 +1100
> @@ -661,6 +661,12 @@ static int NCR5380_queue_command(struct
>   }
>  #endif /* (NDEBUG & NDEBUG_NO_WRITE) */
>  
> + if (cmd->device->lun > MAX_LUN) {
> + cmd->result = DID_NO_CONNECT << 16;
> + cmd->scsi_done(cmd);
> + return 0;
> + }
> +
>   cmd->result = 0;
>  
>   if (!NCR5380_acquire_dma_irq(instance))
> Index: linux/drivers/scsi/NCR5380.h
> ===
> --- linux.orig/drivers/scsi/NCR5380.h 2016-03-14 15:26:45.0 +1100
> +++ linux/drivers/scsi/NCR5380.h  2016-03-14 15:26:48.0 +1100
> @@ -244,6 +244,8 @@ struct NCR5380_hostdata {
>  
>  #ifdef __KERNEL__
>  
> +#define MAX_LUN  7
> +
>  struct NCR5380_cmd {
>   struct list_head list;
>  };
> 
> 
Why not simply use shost->max_lun ?

Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)


Re: [PATCH 13/22] ncr5380: Remove disused atari_NCR5380.c core driver

2016-03-14 Thread Hannes Reinecke
On 03/14/2016 05:27 AM, Finn Thain wrote:
> Now that atari_scsi and sun3_scsi have been converted to use the NCR5380.c
> core driver, remove atari_NCR5380.c. Also remove the last vestiges of its
> Tagged Command Queueing implementation from the wrapper drivers.
> 
> The TCQ support in atari_NCR5380.c is abandoned by this patch. It is not
> merged into the remaining core driver because,
> 
> 1) atari_scsi defines SUPPORT_TAGS but leaves FLAG_TAGGED_QUEUING disabled
> by default, which indicates that it is mostly undesirable.
> 
> 2) I'm told that it doesn't work correctly when enabled.
> 
> 3) The algorithm does not make use of block layer tags which it will have
> to do because scmd->tag is deprecated.
> 
> 4) sun3_scsi doesn't define SUPPORT_TAGS at all, yet the the SUPPORT_TAGS
> macro interacts with the CONFIG_SUN3 macro in 'interesting' ways.
> 
> 5) Compile-time configuration with macros like SUPPORT_TAGS caused the
> configuration space to explode, leading to untestable and unmaintainable
> code that is too hard to reason about.
> 
> The merge_contiguous_buffers() code is also abandoned. This was unused
> by sun3_scsi. Only atari_scsi used it and then only on TT, because only TT
> supports scatter/gather. I suspect that the TT would work fine with
> ENABLE_CLUSTERING instead. If someone can benchmark the difference then
> perhaps the merge_contiguous_buffers() code can be be justified. Until
> then we are better off without the extra complexity.
> 
> Signed-off-by: Finn Thain 
> 
> ---
>  drivers/scsi/NCR5380.c   |   22 
>  drivers/scsi/NCR5380.h   |   19 
>  drivers/scsi/atari_NCR5380.c | 2632 
> ---
>  drivers/scsi/atari_scsi.c|   11 
>  drivers/scsi/mac_scsi.c  |    8 
>  drivers/scsi/sun3_scsi.c |   11 
>  6 files changed, 4 insertions(+), 2699 deletions(-)
> 
Reviewed-by: Hannes Reinecke 

Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)


Re: [PATCH 12/22] sun3_scsi: Adopt NCR5380.c core driver

2016-03-14 Thread Hannes Reinecke
On 03/14/2016 05:27 AM, Finn Thain wrote:
> Add support for the custom Sun 3 DMA logic to the NCR5380.c core driver.
> This code is copied from atari_NCR5380.c.
> 
> Signed-off-by: Finn Thain <fth...@telegraphics.com.au>
> 
> ---
> 
> The Sun 3 DMA code is still configured by macros. I have simplified things
> slightly but I have avoided more ambitious refactoring. It's not clear to
> me what that should look like and I can't test sun3_scsi anyway. At least
> this permits the removal of atari_NCR5380.c.
> 
> ---
>  drivers/scsi/NCR5380.c   |  131 
> +++
>  drivers/scsi/sun3_scsi.c |8 +-
>  2 files changed, 124 insertions(+), 15 deletions(-)
> 
Reviewed-by: Hannes Reinecke <h...@suse.com>

Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)


Re: [PATCH 12/22] sun3_scsi: Adopt NCR5380.c core driver

2016-03-14 Thread Hannes Reinecke
On 03/14/2016 05:27 AM, Finn Thain wrote:
> Add support for the custom Sun 3 DMA logic to the NCR5380.c core driver.
> This code is copied from atari_NCR5380.c.
> 
> Signed-off-by: Finn Thain 
> 
> ---
> 
> The Sun 3 DMA code is still configured by macros. I have simplified things
> slightly but I have avoided more ambitious refactoring. It's not clear to
> me what that should look like and I can't test sun3_scsi anyway. At least
> this permits the removal of atari_NCR5380.c.
> 
> ---
>  drivers/scsi/NCR5380.c   |  131 
> +++
>  drivers/scsi/sun3_scsi.c |8 +-
>  2 files changed, 124 insertions(+), 15 deletions(-)
> 
Reviewed-by: Hannes Reinecke 

Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)


Re: [PATCH 11/22] atari_scsi: Adopt NCR5380.c core driver

2016-03-14 Thread Hannes Reinecke
On 03/14/2016 05:27 AM, Finn Thain wrote:
> Add support for the Atari ST DMA chip to the NCR5380.c core driver.
> This code is copied from atari_NCR5380.c.
> 
> Signed-off-by: Finn Thain <fth...@telegraphics.com.au>
> 
> ---
>  drivers/scsi/NCR5380.c|   32 
>  drivers/scsi/atari_scsi.c |6 +++---
>  2 files changed, 35 insertions(+), 3 deletions(-)
> 
Reviewd-by: Hannes Reinecke <h...@suse.com>

Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)


Re: [PATCH 11/22] atari_scsi: Adopt NCR5380.c core driver

2016-03-14 Thread Hannes Reinecke
On 03/14/2016 05:27 AM, Finn Thain wrote:
> Add support for the Atari ST DMA chip to the NCR5380.c core driver.
> This code is copied from atari_NCR5380.c.
> 
> Signed-off-by: Finn Thain 
> 
> ---
>  drivers/scsi/NCR5380.c|   32 
>  drivers/scsi/atari_scsi.c |6 +++---
>  2 files changed, 35 insertions(+), 3 deletions(-)
> 
Reviewd-by: Hannes Reinecke 

Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)


Re: [PATCH 10/22] ncr5380: Merge DMA implementation from atari_NCR5380 core driver

2016-03-14 Thread Hannes Reinecke
On 03/14/2016 05:27 AM, Finn Thain wrote:
> Adopt the DMA implementation from atari_NCR5380.c. This means that
> atari_scsi and sun3_scsi can make use of the NCR5380.c core driver
> and the atari_NCR5380.c driver fork can be made redundant.
> 
> Signed-off-by: Finn Thain <fth...@telegraphics.com.au>
> 
> ---
>  drivers/scsi/NCR5380.c  |  170 
> +++-
>  drivers/scsi/arm/cumana_1.c |3 
>  drivers/scsi/arm/oak.c  |3 
>  drivers/scsi/dmx3191d.c |1 
>  drivers/scsi/dtc.c  |2 
>  drivers/scsi/dtc.h  |1 
>  drivers/scsi/g_NCR5380.c|2 
>  drivers/scsi/g_NCR5380.h|1 
>  drivers/scsi/mac_scsi.c |3 
>  drivers/scsi/pas16.c|2 
>  drivers/scsi/pas16.h|1 
>  drivers/scsi/t128.c |2 
>  drivers/scsi/t128.h |1 
>  13 files changed, 152 insertions(+), 40 deletions(-)
> 
Reviewed-by: Hannes Reinecke <h...@suse.com>

Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)


Re: [PATCH 10/22] ncr5380: Merge DMA implementation from atari_NCR5380 core driver

2016-03-14 Thread Hannes Reinecke
On 03/14/2016 05:27 AM, Finn Thain wrote:
> Adopt the DMA implementation from atari_NCR5380.c. This means that
> atari_scsi and sun3_scsi can make use of the NCR5380.c core driver
> and the atari_NCR5380.c driver fork can be made redundant.
> 
> Signed-off-by: Finn Thain 
> 
> ---
>  drivers/scsi/NCR5380.c  |  170 
> +++-
>  drivers/scsi/arm/cumana_1.c |3 
>  drivers/scsi/arm/oak.c  |3 
>  drivers/scsi/dmx3191d.c |1 
>  drivers/scsi/dtc.c  |2 
>  drivers/scsi/dtc.h  |1 
>  drivers/scsi/g_NCR5380.c|2 
>  drivers/scsi/g_NCR5380.h|1 
>  drivers/scsi/mac_scsi.c |3 
>  drivers/scsi/pas16.c|2 
>  drivers/scsi/pas16.h|1 
>  drivers/scsi/t128.c |2 
>  drivers/scsi/t128.h |1 
>  13 files changed, 152 insertions(+), 40 deletions(-)
> 
Reviewed-by: Hannes Reinecke 

Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)


<    3   4   5   6   7   8   9   10   11   12   >