[RFC v5 01/26] scsi/atari_scsi: Dont select CONFIG_NVRAM
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
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
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
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
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
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()
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
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
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
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
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
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
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
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
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