[RFC v5 01/26] scsi/atari_scsi: Dont select CONFIG_NVRAM

2015-07-25 Thread Finn Thain
On powerpc, setting CONFIG_NVRAM=n builds a kernel with no NVRAM support.
Setting CONFIG_NVRAM=m enables the /dev/nvram misc device module without
enabling NVRAM support in drivers. Setting CONFIG_NVRAM=y enables the
misc device (built-in) and also enables NVRAM support in drivers.

m68k shares the valkyriefb driver with powerpc, and since that driver uses
NVRAM, it is affected by CONFIG_ATARI_SCSI, because of the use of
select NVRAM.

Adopt the powerpc convention on m68k to avoid surprises.

Signed-off-by: Finn Thain fth...@telegraphics.com.au
Tested-by: Christian T. Steigies c...@debian.org

---

This patch temporarily disables CONFIG_NVRAM on Atari, to prevent build
failures when bisecting the rest of this patch series. It gets enabled
again with the introduction of CONFIG_HAVE_ARCH_NVRAM_OPS, once the
nvram_* global functions have been moved to an ops struct.

The removal of select NVRAM may mean that some kernel configs (such
as Debian/m68k) may need tweaking.

---
 drivers/char/Kconfig  |5 +
 drivers/scsi/Kconfig  |6 +++---
 drivers/scsi/atari_scsi.c |8 
 3 files changed, 8 insertions(+), 11 deletions(-)

Index: linux/drivers/char/Kconfig
===
--- linux.orig/drivers/char/Kconfig 2015-07-25 17:42:34.0 +1000
+++ linux/drivers/char/Kconfig  2015-07-25 17:45:24.0 +1000
@@ -247,7 +247,7 @@ source drivers/char/hw_random/Kconfig
 
 config NVRAM
tristate /dev/nvram support
-   depends on ATARI || X86 || (ARM  RTC_DRV_CMOS) || GENERIC_NVRAM
+   depends on X86 || (ARM  RTC_DRV_CMOS) || GENERIC_NVRAM
---help---
  If you say Y here and create a character special file /dev/nvram
  with major number 10 and minor number 144 using mknod (man mknod),
@@ -265,9 +265,6 @@ config NVRAM
  should NEVER idly tamper with it. See Ralf Brown's interrupt list
  for a guide to the use of CMOS bytes by your BIOS.
 
- On Atari machines, /dev/nvram is always configured and does not need
- to be selected.
-
  To compile this driver as a module, choose M here: the
  module will be called nvram.
 
Index: linux/drivers/scsi/Kconfig
===
--- linux.orig/drivers/scsi/Kconfig 2015-07-25 17:42:34.0 +1000
+++ linux/drivers/scsi/Kconfig  2015-07-25 17:45:24.0 +1000
@@ -1609,14 +1609,14 @@ config ATARI_SCSI
tristate Atari native SCSI support
depends on ATARI  SCSI
select SCSI_SPI_ATTRS
-   select NVRAM
---help---
  If you have an Atari with built-in NCR5380 SCSI controller (TT,
  Falcon, ...) say Y to get it supported. Of course also, if you have
  a compatible SCSI controller (e.g. for Medusa).
 
- To compile this driver as a module, choose M here: the
- module will be called atari_scsi.
+ To compile this driver as a module, choose M here: the module will
+ be called atari_scsi. If you also enable NVRAM support, the SCSI
+ host's ID is taken from the setting in TT RTC NVRAM.
 
  This driver supports both styles of NCR integration into the
  system: the TT style (separate DMA), and the Falcon style (via
Index: linux/drivers/scsi/atari_scsi.c
===
--- linux.orig/drivers/scsi/atari_scsi.c2015-07-25 17:42:34.0 
+1000
+++ linux/drivers/scsi/atari_scsi.c 2015-07-25 17:45:24.0 +1000
@@ -875,9 +875,10 @@ static int __init atari_scsi_probe(struc
if (ATARIHW_PRESENT(TT_SCSI)  setup_sg_tablesize = 0)
atari_scsi_template.sg_tablesize = setup_sg_tablesize;
 
-   if (setup_hostid = 0) {
+   if (setup_hostid = 0)
atari_scsi_template.this_id = setup_hostid  7;
-   } else {
+#ifdef CONFIG_NVRAM
+   else
/* Test if a host id is set in the NVRam */
if (ATARIHW_PRESENT(TT_CLK)  nvram_check_checksum()) {
unsigned char b = nvram_read_byte(14);
@@ -888,8 +889,7 @@ static int __init atari_scsi_probe(struc
if (b  0x80)
atari_scsi_template.this_id = b  7;
}
-   }
-
+#endif
 
 #ifdef REAL_DMA
/* If running on a Falcon and if there's TT-Ram (i.e., more than one


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


Re: [PATCH] target: fix crash in cmd tracing when cmd didn't match a LUN

2015-07-25 Thread Nicholas A. Bellinger
On Sat, 2015-07-25 at 08:48 +0200, Christoph Hellwig wrote:
 On Fri, Jul 24, 2015 at 01:32:14PM -0700, Nicholas A. Bellinger wrote:
  We've already been through this discussion a couple of years back when
  target_submit_cmd() first came into existence.
  
  The reason iscsi/iser-target continues to be a special case is due to
  immediate data vs. non immediate data and their respective command
  sequence number ordering requirements.
 
 I don't see how immediate data plays into this, the write_pending
 callbacks can simply skip the data transfer path, similar to what
 Bart's port of the latest SRP target to lio does as well.

iscsit_execute_cmd() is using iscsit_transport-iscsit_get_dataout() for
any remaining solicited data-out (R2T/RDMA_READ) payload when immediate
write data is smaller than total EDTL.

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


Re: [PATCH 14/20] scsi_dh_alua: parse target device id

2015-07-25 Thread Hannes Reinecke
On 07/24/2015 05:28 PM, Martin K. Petersen wrote:
 Christoph == Christoph Hellwig h...@lst.de writes:
 
 Christoph Can you move this to scsi_mod.ko?  I'll need the same code
 Christoph for the NFS SCSI layout driver soon.
 
 Same here. Working on copy offload again.
 
Hehe. Thought that was needed after all.
(I dimly remember having some parsing code in my initial VPD page
patchset :-)

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)
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 16/20] scsi_dh_alua: Use workqueue for RTPG

2015-07-25 Thread Hannes Reinecke
On 07/24/2015 05:21 PM, Christoph Hellwig wrote:
 +charwork_q_name[264];
 
 create_workqueue and friends now accept printf-like format
 string, so there is no need for this temporary buffer.
 
 +int error;
 +struct completion   init_complete;
 
 Please rename error to init_error and only assign to it before calling
 complete().
 
 Also I'm not sure what the real point of init_complete is, shouldn't
 we just have a mutex held in alua_initialize and alua_activate to
 synchronize the two against each other?
 
Hmm. I guess I could; I'll check here.

 +rcu_read_lock();
 +pg = rcu_dereference(h-pg);
 +if (pg) {
 +kref_get(pg-kref);
 +rcu_read_unlock();
 +alua_rtpg_queue(pg, sdev, NULL);
 +kref_put(pg-kref, release_port_group);
 +} else
 +rcu_read_unlock();
 +}
 
 How about:
 
   rcu_read_lock();
   pg = rcu_dereference(h-pg);
   if (!pg) {
   rcu_read_unlock();
   return;
   }
 
   kref_get(pg-kref);
   rcu_read_unlock();
   alua_rtpg_queue(pg, sdev, NULL);
   kref_put(pg-kref, release_port_group);
 

Ok.

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)
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 04/20] scsi_dh_alua: Improve error handling

2015-07-25 Thread Hannes Reinecke
On 07/24/2015 04:48 PM, Christoph Hellwig wrote:
 This seems to be a bit of a catchall.  Can you split the logging changes
 from actual error code logic changes and describe the latter in more detail?
 
Ok.

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)
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 08/20] scsi_dh_alua: Make stpg synchronous

2015-07-25 Thread Hannes Reinecke
On 07/24/2015 04:51 PM, Christoph Hellwig wrote:
 -memset(h-buff, 0, stpg_len);
 -h-buff[4] = TPGS_STATE_OPTIMIZED  0x0f;
 -h-buff[6] = (h-group_id  8)  0xff;
 -h-buff[7] = h-group_id  0xff;
 +memset(stpg_data, 0, stpg_len);
 +stpg_data[4] = TPGS_STATE_OPTIMIZED  0x0f;
 +put_unaligned_be16(group_id, stpg_data[6]);
 
 Unrelated get/put_unaligned changes again.
 
 
 -if (!scsi_normalize_sense(h-sense, SCSI_SENSE_BUFFERSIZE,
 +if (!(driver_byte(retval)  DRIVER_SENSE) ||
 +!scsi_normalize_sense(h-sense, SCSI_SENSE_BUFFERSIZE,
 
 Where does this come from?
 
Hmm. Probably slipped in during patch rework. I'll be removing it.

 +(!h-pref) 
 
 
 no need for braces here.
 
OK.

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)
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 09/20] scsi_dh_alua: switch to scsi_execute()

2015-07-25 Thread Hannes Reinecke
On 07/24/2015 04:53 PM, Christoph Hellwig wrote:
 Seems like this should use scsi_execute_req_flags instead so that it doesn't
 have to deal with the raw sense buffer.
 
Ok, I'll have a look here.

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)
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 14/20] scsi_dh_alua: parse target device id

2015-07-25 Thread Hannes Reinecke
On 07/24/2015 05:07 PM, Christoph Hellwig wrote:
 On Wed, Jul 08, 2015 at 11:06:12AM +0200, Hannes Reinecke wrote:
 Parse VPD descriptor to figure out the device identification.
 As devices might implement several descriptors the order
 of preference is:
 - NAA IEE Registered Extended
 - EUI-64 based 16-byte
 - EUI-64 based 12-byte
 - NAA IEEE Registered
 - NAA IEEE Extended
 A SCSI name string descriptor is preferred to all of them
 if the identification is longer than 16 bytes.
 
 Can you move this to scsi_mod.ko?  I'll need the same code for the NFS
 SCSI layout driver soon.
 
Ok, will be doing so.

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)
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 11/20] scsi_dh_alua: Use separate alua_port_group structure

2015-07-25 Thread Hannes Reinecke
On 07/24/2015 04:58 PM, Christoph Hellwig wrote:
 On Wed, Jul 08, 2015 at 11:06:09AM +0200, Hannes Reinecke wrote:
 +pg = kzalloc(sizeof(struct alua_port_group), GFP_KERNEL);
 +if (!pg) {
 +sdev_printk(KERN_WARNING, sdev,
 +%s: kzalloc port group failed\n,
 +ALUA_DH_NAME);
 +/* Temporary failure, bypass */
 +return SCSI_DH_DEV_TEMP_BUSY;
 +}
 +pg-group_id = group_id;
 +pg-buff = pg-inq;
 +pg-bufflen = ALUA_INQUIRY_SIZE;
 +pg-tpgs = h-tpgs;
 +pg-state = TPGS_STATE_OPTIMIZED;
 +kref_init(pg-kref);
 +spin_lock(port_group_lock);
 +list_add(pg-node, port_group_list);
 +h-pg = pg;
 +spin_unlock(port_group_lock);
 
 Is there any high level protection against someone racing to allocate
 this structure, e.g. from a sysfs-initiated scan?
 
Not in this patch, as this is called during device scan only. It is
assumed that higher levels will protect against simultaneous scans.

Real protection against concurrent updates is done in patch
'scsi_dh_alua: parse device id', as with that we can easily hit existing
port groups.

 -len = (h-buff[0]  24) + (h-buff[1]  16) +
 -(h-buff[2]  8) + h-buff[3] + 4;
 +len = get_unaligned_be32(pg-buff[0]) + 4;
 
 Andother spurious get/set_unaligned conversion.  I'd really recommend doing
 all of them before the atual series.
 
Okay, will be doing so.

 +rcu_read_lock();
 +pg = rcu_dereference(h-pg);
 +if (!pg) {
 +rcu_read_unlock();
 +return -ENXIO;
 +}
 +rcu_read_unlock();
 +
  if (optimize)
 -h-flags |= ALUA_OPTIMIZE_STPG;
 +pg-flags |= ALUA_OPTIMIZE_STPG;
  else
 -h-flags = ~ALUA_OPTIMIZE_STPG;
 +pg-flags |= ~ALUA_OPTIMIZE_STPG;
 
 You'll need to move the rcu_read_unlock here to be safe.
 
Ok.

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)
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] qla2xxx: Return the fabric command state for non-task management requests

2015-07-25 Thread Christoph Hellwig
Which debug printk do you care about?  I'd much prefer having a trace
point inside the driver which could even pretty print it instead of the
hack where a driver defined binary value is printed by the core.
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] target: add support for START_STOP_UNIT SCSI opcode

2015-07-25 Thread Christoph Hellwig
On Fri, Jul 24, 2015 at 03:06:12AM +, Elliott, Robert (Server Storage) 
wrote:
 Note that the officially published versions of the ISO and ANSI 
 standards don't carry that revision number r36; they just have
 the standard name and year.  SBC-3 revision 36 became 
 ANSI INCITS 514-2014 Information technology - SCSI Block 
 Commands - 3 (SBC-3).
 
 T10 isn't really obligated to keep making particular working 
 drafts available, although the ones that have been assigned
 version descriptors (in SPC-n) are more likely to stick 
 around.  For SBC-3, only revisions 35 and 36 earned those.

Unfortunately the final T10 standards are only available for a high
monetary cost, so they aren't useful for Free Software development.

We work arounds this by referencing specific drafts that are available.
If T10 would regress even more by not providing them we'd have to find
other work arounds.
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] target: fix crash in cmd tracing when cmd didn't match a LUN

2015-07-25 Thread Christoph Hellwig
On Fri, Jul 24, 2015 at 01:32:14PM -0700, Nicholas A. Bellinger wrote:
 We've already been through this discussion a couple of years back when
 target_submit_cmd() first came into existence.
 
 The reason iscsi/iser-target continues to be a special case is due to
 immediate data vs. non immediate data and their respective command
 sequence number ordering requirements.

I don't see how immediate data plays into this, the write_pending
callbacks can simply skip the data transfer path, similar to what
Bart's port of the latest SRP target to lio does as well.
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 4/5] scsi_dh_alua: add 'state' callback function

2015-07-25 Thread Hannes Reinecke
On 07/24/2015 04:42 PM, Christoph Hellwig wrote:
 As per the comment on patch 3 I'd rather expose the ALUA state in the core
 SCSI code.  But having this alua_state attribute in core SCSI code sounds
 fine to me.
 
Sure, we can have an attribute 'alua_state' in the core code; after all,
ALUA is pretty much standard by now.

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)
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/5] scsi: rescan VPD attributes

2015-07-25 Thread Hannes Reinecke
On 07/24/2015 04:43 PM, Christoph Hellwig wrote:
 On Fri, Jul 24, 2015 at 04:40:42PM +0200, Hannes Reinecke wrote:
 Yeah, possibly. After all, the variable isn't expected to change
 under rcu_read_lock().
 
 Actually it can and will change, that's the point.  But if you use
 a local variable you keep a single version of it, which won't be
 freed until after rcu_read_unlock.
 
Yeah, I figured this as well. Will be updating the patch.

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)
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 3/3] cxlflash: Virtual LUN support

2015-07-25 Thread Matthew R. Ochs
Hi Brian,

Thanks for reviewing. Comments inline below.


-matt

On Jul 24, 2015, at 3:15 PM, Brian King wrote:

 On 07/16/2015 06:26 PM, Matthew R. Ochs wrote:
 +
 +/**
 + * ba_clone() - frees a block from the block allocator
 + * @ba_lun: Block allocator from which to allocate a block.
 + * @to_free:Block to free.
 + *
 + * Return: 0 on success, -1 on failure
 + */
 +static int ba_clone(struct ba_lun *ba_lun, u64 to_clone)
 +{
 +struct ba_lun_info *lun_info =
 +(struct ba_lun_info *)ba_lun-ba_lun_handle;
 +
 +if (validate_alloc(lun_info, to_clone)) {
 +pr_err(%s: AUN %llX is not allocated on lun_id %llX\n,
 +   __func__, to_clone, ba_lun-lun_id);
 
 Suggest using dev_err here instead to scope the error to the adapter.

Sure, we plan on transitioning to dev_* prints where it make sense. This routine
in particular will likely not be transitioned, but we can add a dev print in the
outer caller to track the device.

 
 +return -1;
 
 Might be nice to return a better error to the user in both of the error cases
 in this code.

We can look at doing this although in this particular case I'm not sure it would
help things much from a user perspective. These failures are more indicative
of an internal driver bug and not one a user would be all that interested in.

 +
 +rc = ba_init(blka-ba_lun);
 +if (rc) {
 +pr_err(%s: cannot init block_alloc, rc=%d\n, __func__, rc);
 +goto init_ba_exit;
 
 The goto here is unnecessary

We'll remove it.

 +
 +result = scsi_execute(sdev, scsi_cmd, DMA_TO_DEVICE, cmd_buf,
 +  CMD_BUFSIZE, sense_buf,
 +  (MC_DISCOVERY_TIMEOUT*HZ), 5, 0, NULL);
 
 5 seconds seems a little on the short side for this command. Perhaps using
 sdev-request_queue-rq_timeout would be better?

Sure, we'll look into using the rq_timeout value.

 
 +
 +if (result) {
 +pr_err(%s: command failed for offset %lld
 +   result=0x%x\n, __func__, offset, result);
 +rc = -EIO;
 +goto out;
 +}
 +}
 +
 +out:
 
 You need to free cmd_buf and sense_buf here.

Good catch. We just fixed this yesterday.

 +nsectors = (resize-req_size * CXLFLASH_BLOCK_SIZE) / gli-blk_len;
 +new_size = (nsectors + MC_CHUNK_SIZE - 1) / MC_CHUNK_SIZE;
 
 Use DIV_ROUND_UP instead.

Will do.

 +lli-lun_index = cfg-promote_lun_index;
 +writeq_be(lli-lun_id[0],
 +  afu-afu_map-global.fc_port[0]
 +  [cfg-promote_lun_index]);
 
 Would improve readability IMHO if you either put the second parm all on one
 line or used a local variable to make it better fit on one line, rather than
 line breaking where you did.

I agree. We've been doing this to other places in the driver as well for
the same reason. Will look at fixing this one too as part of v3.

 
 +writeq_be(lli-lun_id[1],
 +  afu-afu_map-global.fc_port[1]
 +  [cfg-promote_lun_index]);
 +cfg-promote_lun_index++;
 +pr_debug(%s: Virtual LUN on slot %d  id0=%llx, id1=%llx\n,
 + __func__, lli-lun_index, lli-lun_id[0],
 + lli-lun_id[1]);
 
 Suggest changing the pr_debug calls to dev_dbg calls where possible to scope
 the messages to an adapter.

Same response as earlier. We'll look at refactoring all of the pr_*'s to 
dev_*'s where
possible and makes sense.

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