Re: [PATCH] scsi: Fix a harmless double shift bug
On 12/7/18 8:20 PM, Martin K. Petersen wrote: > > Jens, > > This went in through your tree. Can you please pick this fix up? Yep, applied, thanks Dan. -- Jens Axboe
Re: [PATCH 00/15] lpfc updates for 12.0.0.9
James, > Update lpfc to revision 12.0.0.9 > > This patch contains lpfc bug fixes Applied to 4.21/scsi-queue, thanks! -- Martin K. Petersen Oracle Linux Engineering
Re: [PATCH] scsi: bnx2fc: Fix NULL dereference in error handling
Dan, > If "interface" is NULL then we can't release it and trying to will > only lead to an Oops. Applied to 4.20/scsi-fixes, thanks! -- Martin K. Petersen Oracle Linux Engineering
Re: [PATCH] Revert "scsi: qla2xxx: Fix NVMe Target discovery"
Himanshu, > This reverts commit db186382af21e926e90df19499475f2552192b77. > > This commit introduced regression with FCP discovery so revert it back > to fix discovery for FCP luns Applied to 4.20/scsi-fixes. -- Martin K. Petersen Oracle Linux Engineering
Re: [PATCH] scsi: Fix a harmless double shift bug
Jens, This went in through your tree. Can you please pick this fix up? > On Thu, Nov 29, 2018 at 01:37:10PM +0300, Dan Carpenter wrote: >> Smatch generates a warning: >> >> drivers/scsi/scsi_lib.c:1656 scsi_mq_done() warn: test_bit() takes a bit >> number >> >> The problem is that SCMD_STATE_COMPLETE is supposed to be bit number 0 >> and not a mask like "(1 << 0)". It is used like this: >> >> if (test_and_set_bit(SCMD_STATE_COMPLETE, >state)) >> >> The test_and_set_bit() has a shift built in so it's a double left shift >> and uses bit number 1 instead of number 0. This bug is harmless because >> it's done consistently and it doesn't clash with any other flags. >> >> Fixes: f1342709d18a ("scsi: Do not rely on blk-mq for double completions") >> Signed-off-by: Dan Carpenter > > Nice catch, thanks for the fix. > > Reviewed-by: Keith Busch Acked-by: Martin K. Petersen -- Martin K. Petersen Oracle Linux Engineering
Re: [PATCH 00/20] smartpqi updates
Hi Brace, In this patch series, do you have fixed and verified the timeout issue? https://patchwork.kernel.org/patch/10637797/ Thanks, - Alex Don Brace 于2018年12月8日周六 上午6:28写道: > > These patches are based on Linus's tree > > The changes are: > > - smartpqi-add-support-for-PQI-Config-Table-handshake > . add support for get/set controller features. > - smartpqi-add-retries-for-device-resets > . re-attempt device reset. > - smartpqi-add-no_write_same-for-logical-volumes > . turn off WRITE SAME for logical volumes. > - smartpqi-correct-host-serial-num-for-ssa > . update host serial number > - smartpqi-turn-off-lun-data-caching-for-ptraid > . get fresh lun list from RBODS. > - smartpqi-refactor-sending-controller-raid-requests > . condense commonly used code. > - smartpqi-add-sysfs-attributes > . add in driver attributes. > - smartpqi-add-h3c-ssid > . add support for more controllers. > - smartpqi-fix-disk-name-mount-point > . correct sysfs attribute for unique ids. > - smartpqi-wake-up-drives-after-os-resumes-from-suspend > . have OS start up disks after resume. > - smartpqi-enhance-numa-node-detection > . set pci device to correct NUMA node. > - smartpqi-add-support-for-huawei-controllers > . add support for more controllers. > - smartpqi-check-for-null-device-pointers > . wait for all outstanding I/O to complete before removing a volume > - smartpqi-allow-for-larger-raid-maps > . correct rare case for very large volume configurations. > - smartpqi-do-not-offline-disks-for-transient-did-no-connect-conditions > . remove call to scsi_device_set_state > - smartpqi-correct-volume-status > . correct rare case for volume deletion during a scan operation. > - smartpqi-correct-lun-reset-issues > . clear scsi cmd result after a reset. > - smartpqi-add-module-param-to-disable-irq-affinity > . allow some customers to change IRQ affinity. > - smartpqi-add-smp_utils-support > . add support for smp_utils. > - smartpqi-bump-driver-version > > --- > > Ajish Koshy (2): > smartpqi: add support for huawei controllers > smartpqi: allow for larger raid maps > > Dave Carroll (7): > smartpqi: add no_write_same for logical volumes > smartpqi: turn off lun data caching for ptraid > smartpqi: refactor sending controller raid requests > smartpqi: add sysfs attributes > smartpqi: wake up drives after os resumes from suspend > smartpqi: do not offline disks for transient did no connect conditions > smartpqi: correct volume status > > Don Brace (3): > smartpqi: add module param to disable irq affinity > smartpqi: add smp_utils support > smartpqi: bump driver version > > Kevin Barnett (2): > smartpqi: add support for PQI Config Table handshake > smartpqi: correct lun reset issues > > Mahesh Rajashekhara (3): > Add retries for device reset > smartpqi: correct host serial num for ssa > smartpqi: check for null device pointers > > Murthy Bhat (2): > smartpqi: add h3c ssid > smartpqi: fix disk name mount point > > Sagar Biradar (1): > smartpqi: enhance numa node detection > > > drivers/scsi/smartpqi/smartpqi.h | 144 +++ > drivers/scsi/smartpqi/smartpqi_init.c | 992 > > drivers/scsi/smartpqi/smartpqi_sas_transport.c | 164 > 3 files changed, 1131 insertions(+), 169 deletions(-) > > -- > Signature -- Thanks and Best Regards, Feng Li(Alex)
Re: [PATCH 0/2] zfcp: small bugfix on top of previous v4.21 patches
Steffen, > One new recovery fix, which is not urgent, for an old bug. It's > sufficient to apply it on top of the previously sent 23 zfcp updates > for the v4.21 merge window Applied to 4.21/scsi-queue, thanks! -- Martin K. Petersen Oracle Linux Engineering
Re: [PATCH v7 0/5] target: user configurable T10 Vendor ID
David, > This patch-set allows for the modification of the T10 Vendor > Identification string returned in the SCSI INQUIRY response, via the > target/core/$backstore/$name/wwn/vendor_id ConfigFS path. Applied to 4.21/scsi-queue, thank you! -- Martin K. Petersen Oracle Linux Engineering
Re: [PATCH v2] qla2xxx: Split the __qla2x00_abort_all_cmds() function
Bart, > Nesting in __qla2x00_abort_all_cmds() is way too deep. Reduce the > nesting level by introducing a helper function. This patch does not > change any functionality. Applied to 4.21/scsi-queue. Thank you. -- Martin K. Petersen Oracle Linux Engineering
Re: [PATCH] scsi: csiostor: remove flush_scheduled_work()
Varun, > flush_scheduled_work() is not required as csio_hw_exit_workers() calls > cancel_work_sync() for hw->evtq_work. Applied to 4.21/scsi-queue, thanks! -- Martin K. Petersen Oracle Linux Engineering
Re: [PATCH v2 01/23] zfcp: make DIX experimental, disabled, and independent of DIF
Steffen, > Introduce separate zfcp module parameters to individually select > support for: DIF which should work (zfcp.dif, which used to be > DIF+DIX, disabled) or DIX+DIF which can cause trouble (zfcp.dix, new, > disabled). Applied to 4.21/scsi-queue. -- Martin K. Petersen Oracle Linux Engineering
Re: [PATCH] scsi: ufs: Remove redundant sense size definition
Avri, > By spec, the ufs sense data is 18 bytes long. Applied to 4.21/scsi-queue, thanks! -- Martin K. Petersen Oracle Linux Engineering
[PATCH 20/20] smartpqi: bump driver version
Reviewed-by: Gerry Morong Reviewed-by: Dave Carroll Signed-off-by: Don Brace --- drivers/scsi/smartpqi/smartpqi_init.c |6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/scsi/smartpqi/smartpqi_init.c b/drivers/scsi/smartpqi/smartpqi_init.c index 8d94028d0a9d..1b0fa974b4a7 100644 --- a/drivers/scsi/smartpqi/smartpqi_init.c +++ b/drivers/scsi/smartpqi/smartpqi_init.c @@ -40,11 +40,11 @@ #define BUILD_TIMESTAMP #endif -#define DRIVER_VERSION "1.1.4-130" +#define DRIVER_VERSION "1.2.4-065" #define DRIVER_MAJOR 1 -#define DRIVER_MINOR 1 +#define DRIVER_MINOR 2 #define DRIVER_RELEASE 4 -#define DRIVER_REVISION130 +#define DRIVER_REVISION65 #define DRIVER_NAME"Microsemi PQI Driver (v" \ DRIVER_VERSION BUILD_TIMESTAMP ")"
[PATCH 19/20] smartpqi: add smp_utils support
Reviewed-by: Scott Benesh Reviewed-by: Mahesh Rajashekhara Reviewed-by: Scott Teel Reviewed-by: Dave Carroll Reviewed-by: Kevin Barnett Signed-off-by: Don Brace --- drivers/scsi/smartpqi/smartpqi.h | 89 + drivers/scsi/smartpqi/smartpqi_init.c | 131 --- drivers/scsi/smartpqi/smartpqi_sas_transport.c | 164 +++- 3 files changed, 324 insertions(+), 60 deletions(-) diff --git a/drivers/scsi/smartpqi/smartpqi.h b/drivers/scsi/smartpqi/smartpqi.h index 4f52b5be3693..ba499a636f43 100644 --- a/drivers/scsi/smartpqi/smartpqi.h +++ b/drivers/scsi/smartpqi/smartpqi.h @@ -21,6 +21,9 @@ #if !defined(_SMARTPQI_H) #define _SMARTPQI_H +#include +#include + #pragma pack(1) #define PQI_DEVICE_SIGNATURE "PQI DREG" @@ -855,6 +858,7 @@ struct pqi_scsi_dev { u8 unique_id[16]; u8 is_physical_device : 1; u8 is_external_raid_device : 1; + u8 is_expander_smp_device : 1; u8 target_lun_valid : 1; u8 device_gone : 1; u8 new_device : 1; @@ -964,6 +968,7 @@ struct pqi_sas_node { struct pqi_sas_port { struct list_head port_list_entry; u64 sas_address; + struct pqi_scsi_dev *device; struct sas_port *port; int next_phy_index; struct list_head phy_list_head; @@ -1129,11 +1134,14 @@ enum pqi_ctrl_mode { #define BMIC_WRITE 0x27 #define BMIC_SENSE_CONTROLLER_PARAMETERS 0x64 #define BMIC_SENSE_SUBSYSTEM_INFORMATION 0x66 +#define BMIC_CSMI_PASSTHRU 0x68 #define BMIC_WRITE_HOST_WELLNESS 0xa5 #define BMIC_FLUSH_CACHE 0xc2 #define BMIC_SET_DIAG_OPTIONS 0xf4 #define BMIC_SENSE_DIAG_OPTIONS0xf5 +#define CSMI_CC_SAS_SMP_PASSTHRU 0X17 + #define SA_FLUSH_CACHE 0x1 #define MASKED_DEVICE(lunid) ((lunid)[3] & 0xc0) @@ -1160,6 +1168,10 @@ struct bmic_identify_controller { u8 reserved3[32]; }; +#define SA_EXPANDER_SMP_DEVICE 0x05 +/*SCSI Invalid Device Type for SAS devices*/ +#define PQI_SAS_SCSI_INVALID_DEVTYPE 0xff + struct bmic_identify_physical_device { u8 scsi_bus; /* SCSI Bus number on controller */ u8 scsi_id;/* SCSI ID on this bus */ @@ -1240,6 +1252,50 @@ struct bmic_identify_physical_device { u8 padding_to_multiple_of_512[9]; }; +struct bmic_smp_request { + u8 frame_type; + u8 function; + u8 allocated_response_length; + u8 request_length; + u8 additional_request_bytes[1016]; +}; + +struct bmic_smp_response { + u8 frame_type; + u8 function; + u8 function_result; + u8 response_length; + u8 additional_response_bytes[1016]; +}; + +struct bmic_csmi_ioctl_header { + __le32 header_length; + u8 signature[8]; + __le32 timeout; + __le32 control_code; + __le32 return_code; + __le32 length; +}; + +struct bmic_csmi_smp_passthru { + u8 phy_identifier; + u8 port_identifier; + u8 connection_rate; + u8 reserved; + __be64 destination_sas_address; + __le32 request_length; + struct bmic_smp_request request; + u8 connection_status; + u8 reserved1[3]; + __le32 response_length; + struct bmic_smp_response response; +}; + +struct bmic_csmi_smp_passthru_buffer { + struct bmic_csmi_ioctl_header ioctl_header; + struct bmic_csmi_smp_passthru parameters; +}; + struct bmic_flush_cache { u8 disable_flag; u8 system_power_action; @@ -1263,6 +1319,36 @@ struct bmic_diag_options { #pragma pack() +static inline struct pqi_ctrl_info *shost_to_hba(struct Scsi_Host *shost) +{ + void *hostdata = shost_priv(shost); + + return *((struct pqi_ctrl_info **)hostdata); +} + +static inline bool pqi_ctrl_offline(struct pqi_ctrl_info *ctrl_info) +{ + return !ctrl_info->controller_online; +} + +static inline void pqi_ctrl_busy(struct pqi_ctrl_info *ctrl_info) +{ + atomic_inc(_info->num_busy_threads); +} + +static inline void pqi_ctrl_unbusy(struct pqi_ctrl_info *ctrl_info) +{ + atomic_dec(_info->num_busy_threads); +} + +static inline bool pqi_ctrl_blocked(struct pqi_ctrl_info *ctrl_info) +{ + return ctrl_info->block_requests; +} + +void pqi_sas_smp_handler(struct bsg_job *job, struct Scsi_Host *shost, + struct sas_rphy *rphy); + int pqi_add_sas_host(struct Scsi_Host *shost, struct pqi_ctrl_info *ctrl_info); void pqi_delete_sas_host(struct pqi_ctrl_info *ctrl_info); int pqi_add_sas_device(struct pqi_sas_node *pqi_sas_node, @@ -1271,6 +1357,9 @@ void pqi_remove_sas_device(struct pqi_scsi_dev *device); struct pqi_scsi_dev
[PATCH 17/20] smartpqi: correct lun reset issues
From: Kevin Barnett Problem: The Linux kernel takes a logical volume offline after a LUN reset. This is generally accompanied by this message in the dmesg output: Device offlined - not ready after error recovery Root Cause: The root cause is a "quirk" in the timeout handling in the Linux SCSI layer. The Linux kernel places a 30-second timeout on most media access commands (reads and writes) that it send to device drivers. When a media access command times out, the Linux kernel goes into error recovery mode for the LUN that was the target of the command that timed out. Every command that timed out is kept on a list inside of the Linux kernel to be retried later. The kernel attempts to recover the command(s) that timed out by issuing a LUN reset followed by a TEST UNIT READY. If the LUN reset and TEST UNIT READY commands are successful, the kernel retries the command(s) that timed out. Each SCSI command issued by the kernel has a result field associated with it. This field indicates the final result of the command (success or error). When a command times out, the kernel places a value in this result field indicating that the command timed out. The "quirk" is that after the LUN reset and TEST UNIT READY commands are completed, the kernel checks each command on the timed-out command list before retrying it. If the result field is still "timed out", the kernel treats that command as not having been successfully recovered for a retry. If the number of commands that are in this state are greater than two, the kernel takes the LUN offline. Fix: When our RAIDStack receives a LUN reset, it simply waits until all outstanding commands complete. Generally, all of these outstanding commands complete successfully. Therefore, the fix in the smartpqi driver is to always set the command result field to indicate success when a request completes successfully. This normally isn’t necessary because the result field is always initialized to success when the command is submitted to the driver. So when the command completes successfully, the result field is left untouched. But in this case, the kernel changes the result field behind the driver’s back and then expects the field to be changed by the driver as the commands that timed-out complete. Reviewed-by: Dave Carroll Reviewed-by: Scott Teel Signed-off-by: Kevin Barnett Signed-off-by: Don Brace --- drivers/scsi/smartpqi/smartpqi_init.c |3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/scsi/smartpqi/smartpqi_init.c b/drivers/scsi/smartpqi/smartpqi_init.c index bee14fc8a35e..2f2a07a38dad 100644 --- a/drivers/scsi/smartpqi/smartpqi_init.c +++ b/drivers/scsi/smartpqi/smartpqi_init.c @@ -2841,6 +2841,9 @@ static unsigned int pqi_process_io_intr(struct pqi_ctrl_info *ctrl_info, switch (response->header.iu_type) { case PQI_RESPONSE_IU_RAID_PATH_IO_SUCCESS: case PQI_RESPONSE_IU_AIO_PATH_IO_SUCCESS: + if (io_request->scmd) + io_request->scmd->result = 0; + /* fall through */ case PQI_RESPONSE_IU_GENERAL_MANAGEMENT: break; case PQI_RESPONSE_IU_VENDOR_GENERAL:
[PATCH 18/20] smartpqi: add module param to disable irq affinity
The PCI_IRQ_AFFINITY flag prevents customers from changing the smp_affinity and smp_affinity_list entries. Add a module parameter to allow this flag to be turned off. Reviewed-by: Scott Teel Reviewed-by: Murthy Bhat Reviewed-by: Mahesh Rajashekhara Reviewed-by: Dave Carroll Reviewed-by: Kevin Barnett Signed-off-by: Don Brace --- drivers/scsi/smartpqi/smartpqi_init.c | 17 +++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/drivers/scsi/smartpqi/smartpqi_init.c b/drivers/scsi/smartpqi/smartpqi_init.c index 2f2a07a38dad..f9812281988c 100644 --- a/drivers/scsi/smartpqi/smartpqi_init.c +++ b/drivers/scsi/smartpqi/smartpqi_init.c @@ -144,6 +144,11 @@ MODULE_PARM_DESC(lockup_action, "Action to take when controller locked up.\n" "\t\tSupported: none, reboot, panic\n" "\t\tDefault: none"); +static bool pqi_disable_irq_affinity; +module_param(pqi_disable_irq_affinity, bool, 0644); +MODULE_PARM_DESC(pqi_disable_irq_affinity, + "\t\tTurn off managed irq affinity. Allows smp_affinity to be changed."); + static char *raid_levels[] = { "RAID-0", "RAID-4", @@ -3276,10 +3281,14 @@ static void pqi_free_irqs(struct pqi_ctrl_info *ctrl_info) static int pqi_enable_msix_interrupts(struct pqi_ctrl_info *ctrl_info) { int num_vectors_enabled; + unsigned int flags = PCI_IRQ_MSIX; + + if (!pqi_disable_irq_affinity) + flags |= PCI_IRQ_AFFINITY; num_vectors_enabled = pci_alloc_irq_vectors(ctrl_info->pci_dev, PQI_MIN_MSIX_VECTORS, ctrl_info->num_queue_groups, - PCI_IRQ_MSIX | PCI_IRQ_AFFINITY); + flags); if (num_vectors_enabled < 0) { dev_err(_info->pci_dev->dev, "MSI-X init failed with error %d\n", @@ -5507,7 +5516,11 @@ static int pqi_map_queues(struct Scsi_Host *shost) { struct pqi_ctrl_info *ctrl_info = shost_to_hba(shost); - return blk_mq_pci_map_queues(>tag_set, ctrl_info->pci_dev, 0); + if (!pqi_disable_irq_affinity) + return blk_mq_pci_map_queues(>tag_set, + ctrl_info->pci_dev, 0); + else + return 0; } static int pqi_getpciinfo_ioctl(struct pqi_ctrl_info *ctrl_info,
[PATCH 15/20] smartpqi: do not offline disks for transient did no connect conditions
From: Dave Carroll Reviewed-by: Murthy Bhat Reviewed-by: Mahesh Rajashekhara Reviewed-by: Scott Teel Reviewed-by: Kevin Barnett Signed-off-by: Dave Carroll Signed-off-by: Don Brace --- drivers/scsi/smartpqi/smartpqi_init.c |4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/scsi/smartpqi/smartpqi_init.c b/drivers/scsi/smartpqi/smartpqi_init.c index 45c797bc1483..f146ebf3921f 100644 --- a/drivers/scsi/smartpqi/smartpqi_init.c +++ b/drivers/scsi/smartpqi/smartpqi_init.c @@ -1686,6 +1686,7 @@ static void pqi_scsi_update_device(struct pqi_scsi_dev *existing_device, new_device->raid_bypass_configured; existing_device->raid_bypass_enabled = new_device->raid_bypass_enabled; + existing_device->device_offline = false; /* To prevent this from being freed later. */ new_device->raid_map = NULL; @@ -2589,10 +2590,9 @@ static inline void pqi_take_device_offline(struct scsi_device *sdev, char *path) return; device->device_offline = true; - scsi_device_set_state(sdev, SDEV_OFFLINE); ctrl_info = shost_to_hba(sdev->host); pqi_schedule_rescan_worker(ctrl_info); - dev_err(_info->pci_dev->dev, "offlined %s scsi %d:%d:%d:%d\n", + dev_err(_info->pci_dev->dev, "re-scanning %s scsi %d:%d:%d:%d\n", path, ctrl_info->scsi_host->host_no, device->bus, device->target, device->lun); }
[PATCH 16/20] smartpqi: correct volume status
From: Dave Carroll - fix race condition when a unit is deleted after an RLL, and before we have gotten the LV_STATUS page of the unit. - In this case we will get a standard inquiry, rather than the desired page. This will result in a unit presented which no longer exists. - If we ask for LV_STATUS, insure we get LV_STATUS Reviewed-by: Murthy Bhat Reviewed-by: Mahesh Rajashekhara Reviewed-by: Scott Teel Reviewed-by: Kevin Barnett Signed-off-by: Dave Carroll Signed-off-by: Don Brace --- drivers/scsi/smartpqi/smartpqi_init.c |3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/scsi/smartpqi/smartpqi_init.c b/drivers/scsi/smartpqi/smartpqi_init.c index f146ebf3921f..bee14fc8a35e 100644 --- a/drivers/scsi/smartpqi/smartpqi_init.c +++ b/drivers/scsi/smartpqi/smartpqi_init.c @@ -1270,6 +1270,9 @@ static void pqi_get_volume_status(struct pqi_ctrl_info *ctrl_info, if (rc) goto out; + if (vpd->page_code != CISS_VPD_LV_STATUS) + goto out; + page_length = offsetof(struct ciss_vpd_logical_volume_status, volume_status) + vpd->page_length; if (page_length < sizeof(*vpd))
[PATCH 09/20] smartpqi: fix disk name mount point
From: Murthy Bhat - fix a formatting issue. Reviewed-by: Mahesh Rajashekhara Reviewed-by: Scott Teel Reviewed-by: Dave Carroll Reviewed-by: Kevin Barnett Signed-off-by: Murthy Bhat Signed-off-by: Don Brace --- drivers/scsi/smartpqi/smartpqi_init.c |7 ++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/drivers/scsi/smartpqi/smartpqi_init.c b/drivers/scsi/smartpqi/smartpqi_init.c index 7ece16495a09..6716d30dc5ef 100644 --- a/drivers/scsi/smartpqi/smartpqi_init.c +++ b/drivers/scsi/smartpqi/smartpqi_init.c @@ -5833,7 +5833,12 @@ static ssize_t pqi_unique_id_show(struct device *dev, spin_unlock_irqrestore(_info->scsi_device_list_lock, flags); - return snprintf(buffer, PAGE_SIZE, "%16phN", uid); + return snprintf(buffer, PAGE_SIZE, + "%02X%02X%02X%02X%02X%02X%02X%02X%02X%02X%02X%02X%02X%02X%02X%02X\n", + uid[0], uid[1], uid[2], uid[3], + uid[4], uid[5], uid[6], uid[7], + uid[8], uid[9], uid[10], uid[11], + uid[12], uid[13], uid[14], uid[15]); } static ssize_t pqi_lunid_show(struct device *dev,
[PATCH 13/20] smartpqi: check for null device pointers
From: Mahesh Rajashekhara - wait on all outstanding I/O to complete before the device is removed. - check for null device pointers in IO entry/completion functions. Reviewed-by: Scott Teel Reviewed-by: Murthy Bhat Reviewed-by: Dave Carroll Reviewed-by: Kevin Barnett Signed-off-by: Mahesh Rajashekhara Signed-off-by: Don Brace --- drivers/scsi/smartpqi/smartpqi.h |2 + drivers/scsi/smartpqi/smartpqi_init.c | 65 +++-- 2 files changed, 64 insertions(+), 3 deletions(-) diff --git a/drivers/scsi/smartpqi/smartpqi.h b/drivers/scsi/smartpqi/smartpqi.h index a39c324dedab..4f52b5be3693 100644 --- a/drivers/scsi/smartpqi/smartpqi.h +++ b/drivers/scsi/smartpqi/smartpqi.h @@ -862,6 +862,7 @@ struct pqi_scsi_dev { u8 volume_offline : 1; boolaio_enabled;/* only valid for physical disks */ boolin_reset; + boolin_remove; booldevice_offline; u8 vendor[8]; /* bytes 8-15 of inquiry data */ u8 model[16]; /* bytes 16-31 of inquiry data */ @@ -1063,6 +1064,7 @@ struct pqi_ctrl_info { struct mutexlun_reset_mutex; boolcontroller_online; boolblock_requests; + boolin_shutdown; u8 inbound_spanning_supported : 1; u8 outbound_spanning_supported : 1; u8 pqi_mode_enabled : 1; diff --git a/drivers/scsi/smartpqi/smartpqi_init.c b/drivers/scsi/smartpqi/smartpqi_init.c index c5640b379505..5f7ae5946ddc 100644 --- a/drivers/scsi/smartpqi/smartpqi_init.c +++ b/drivers/scsi/smartpqi/smartpqi_init.c @@ -74,6 +74,8 @@ static int pqi_aio_submit_io(struct pqi_ctrl_info *ctrl_info, struct scsi_cmnd *scmd, u32 aio_handle, u8 *cdb, unsigned int cdb_length, struct pqi_queue_group *queue_group, struct pqi_encryption_info *encryption_info, bool raid_bypass); +static int pqi_device_wait_for_pending_io(struct pqi_ctrl_info *ctrl_info, + struct pqi_scsi_dev *device, unsigned long timeout_secs); /* for flags argument to pqi_submit_raid_request_synchronous() */ #define PQI_SYNC_FLAGS_INTERRUPTABLE 0x1 @@ -317,6 +319,17 @@ static inline bool pqi_device_in_reset(struct pqi_scsi_dev *device) return device->in_reset; } +static inline void pqi_device_remove_start(struct pqi_scsi_dev *device) +{ + device->in_remove = true; +} + +static inline bool pqi_device_in_remove(struct pqi_ctrl_info *ctrl_info, + struct pqi_scsi_dev *device) +{ + return device->in_remove & !ctrl_info->in_shutdown; +} + static inline void pqi_schedule_rescan_worker_with_delay( struct pqi_ctrl_info *ctrl_info, unsigned long delay) { @@ -1487,9 +1500,24 @@ static int pqi_add_device(struct pqi_ctrl_info *ctrl_info, return rc; } +#define PQI_PENDING_IO_TIMEOUT_SECS20 + static inline void pqi_remove_device(struct pqi_ctrl_info *ctrl_info, struct pqi_scsi_dev *device) { + int rc; + + pqi_device_remove_start(device); + + rc = pqi_device_wait_for_pending_io(ctrl_info, device, + PQI_PENDING_IO_TIMEOUT_SECS); + if (rc) + dev_err(_info->pci_dev->dev, + "scsi %d:%d:%d:%d removing device with %d outstanding commands\n", + ctrl_info->scsi_host->host_no, device->bus, + device->target, device->lun, + atomic_read(>scsi_cmds_outstanding)); + if (pqi_is_logical_device(device)) scsi_remove_device(device->sdev); else @@ -5042,7 +5070,17 @@ void pqi_prep_for_scsi_done(struct scsi_cmnd *scmd) { struct pqi_scsi_dev *device; + if (!scmd->device) { + set_host_byte(scmd, DID_NO_CONNECT); + return; + } + device = scmd->device->hostdata; + if (!device) { + set_host_byte(scmd, DID_NO_CONNECT); + return; + } + atomic_dec(>scsi_cmds_outstanding); } @@ -5059,9 +5097,16 @@ static int pqi_scsi_queue_command(struct Scsi_Host *shost, device = scmd->device->hostdata; ctrl_info = shost_to_hba(shost); + if (!device) { + set_host_byte(scmd, DID_NO_CONNECT); + pqi_scsi_done(scmd); + return 0; + } + atomic_inc(>scsi_cmds_outstanding); - if (pqi_ctrl_offline(ctrl_info)) { + if (pqi_ctrl_offline(ctrl_info) || pqi_device_in_remove(ctrl_info, + device)) { set_host_byte(scmd, DID_NO_CONNECT); pqi_scsi_done(scmd); return 0; @@ -5214,12 +5259,23 @@ static void pqi_fail_io_queued_for_device(struct pqi_ctrl_info *ctrl_info, } static int pqi_device_wait_for_pending_io(struct pqi_ctrl_info *ctrl_info, - struct pqi_scsi_dev
[PATCH 11/20] smartpqi: enhance numa node detection
From: Sagar Biradar - set pci_dev->dev to 0 only if the node is NO_NUMA_NODE. If not, do not reset the value but retain it. Reviewed-by: Murthy Bhat Reviewed-by: Mahesh Rajashekhara Reviewed-by: Dave Carroll Reviewed-by: Scott Teel Reviewed-by: Kevin Barnett Signed-off-by: Sagar Biradar Signed-off-by: Don Brace --- drivers/scsi/smartpqi/smartpqi_init.c | 10 +++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/drivers/scsi/smartpqi/smartpqi_init.c b/drivers/scsi/smartpqi/smartpqi_init.c index acde0abbbf43..a98b3de40c74 100644 --- a/drivers/scsi/smartpqi/smartpqi_init.c +++ b/drivers/scsi/smartpqi/smartpqi_init.c @@ -7131,7 +7131,7 @@ static int pqi_pci_probe(struct pci_dev *pci_dev, const struct pci_device_id *id) { int rc; - int node; + int node, cp_node; struct pqi_ctrl_info *ctrl_info; pqi_print_ctrl_info(pci_dev, id); @@ -7149,8 +7149,12 @@ static int pqi_pci_probe(struct pci_dev *pci_dev, "controller device ID matched using wildcards\n"); node = dev_to_node(_dev->dev); - if (node == NUMA_NO_NODE) - set_dev_node(_dev->dev, 0); + if (node == NUMA_NO_NODE) { + cp_node = cpu_to_node(0); + if (cp_node == NUMA_NO_NODE) + cp_node = 0; + set_dev_node(_dev->dev, cp_node); + } ctrl_info = pqi_alloc_ctrl_info(node); if (!ctrl_info) {
[PATCH 14/20] smartpqi: allow for larger raid maps
From: Ajish Koshy Reviewed-by: Murthy Bhat Reviewed-by: Mahesh Rajashekhara Reviewed-by: Dave Carroll Reviewed-by: Scott Teel Reviewed-by: Kevin Barnett Signed-off-by: Ajish Koshy Signed-off-by: Don Brace --- drivers/scsi/smartpqi/smartpqi_init.c | 59 - 1 file changed, 28 insertions(+), 31 deletions(-) diff --git a/drivers/scsi/smartpqi/smartpqi_init.c b/drivers/scsi/smartpqi/smartpqi_init.c index 5f7ae5946ddc..45c797bc1483 100644 --- a/drivers/scsi/smartpqi/smartpqi_init.c +++ b/drivers/scsi/smartpqi/smartpqi_init.c @@ -1115,8 +1115,6 @@ static int pqi_validate_raid_map(struct pqi_ctrl_info *ctrl_info, char *err_msg; u32 raid_map_size; u32 r5or6_blocks_per_row; - unsigned int num_phys_disks; - unsigned int num_raid_map_entries; raid_map_size = get_unaligned_le32(_map->structure_size); @@ -1125,22 +1123,6 @@ static int pqi_validate_raid_map(struct pqi_ctrl_info *ctrl_info, goto bad_raid_map; } - if (raid_map_size > sizeof(*raid_map)) { - err_msg = "RAID map too large"; - goto bad_raid_map; - } - - num_phys_disks = get_unaligned_le16(_map->layout_map_count) * - (get_unaligned_le16(_map->data_disks_per_row) + - get_unaligned_le16(_map->metadata_disks_per_row)); - num_raid_map_entries = num_phys_disks * - get_unaligned_le16(_map->row_cnt); - - if (num_raid_map_entries > RAID_MAP_MAX_ENTRIES) { - err_msg = "invalid number of map entries in RAID map"; - goto bad_raid_map; - } - if (device->raid_level == SA_RAID_1) { if (get_unaligned_le16(_map->layout_map_count) != 2) { err_msg = "invalid RAID-1 map"; @@ -1179,27 +1161,45 @@ static int pqi_get_raid_map(struct pqi_ctrl_info *ctrl_info, struct pqi_scsi_dev *device) { int rc; - enum dma_data_direction dir; - struct pqi_raid_path_request request; + u32 raid_map_size; struct raid_map *raid_map; raid_map = kmalloc(sizeof(*raid_map), GFP_KERNEL); if (!raid_map) return -ENOMEM; - rc = pqi_build_raid_path_request(ctrl_info, , - CISS_GET_RAID_MAP, device->scsi3addr, raid_map, - sizeof(*raid_map), 0, ); + rc = pqi_send_scsi_raid_request(ctrl_info, CISS_GET_RAID_MAP, + device->scsi3addr, raid_map, sizeof(*raid_map), + 0, NULL, NO_TIMEOUT); + if (rc) goto error; - rc = pqi_submit_raid_request_synchronous(ctrl_info, , 0, - NULL, NO_TIMEOUT); + raid_map_size = get_unaligned_le32(_map->structure_size); - pqi_pci_unmap(ctrl_info->pci_dev, request.sg_descriptors, 1, dir); + if (raid_map_size > sizeof(*raid_map)) { - if (rc) - goto error; + kfree(raid_map); + + raid_map = kmalloc(raid_map_size, GFP_KERNEL); + if (!raid_map) + return -ENOMEM; + + rc = pqi_send_scsi_raid_request(ctrl_info, CISS_GET_RAID_MAP, + device->scsi3addr, raid_map, raid_map_size, + 0, NULL, NO_TIMEOUT); + if (rc) + goto error; + + if (get_unaligned_le32(_map->structure_size) + != raid_map_size) { + dev_warn(_info->pci_dev->dev, + "Requested %d bytes, received %d bytes", + raid_map_size, + get_unaligned_le32(_map->structure_size)); + goto error; + } + } rc = pqi_validate_raid_map(ctrl_info, device, raid_map); if (rc) @@ -2459,9 +2459,6 @@ static int pqi_raid_bypass_submit_scsi_cmd(struct pqi_ctrl_info *ctrl_info, (map_row * total_disks_per_row) + first_column; } - if (unlikely(map_index >= RAID_MAP_MAX_ENTRIES)) - return PQI_RAID_BYPASS_INELIGIBLE; - aio_handle = raid_map->disk_data[map_index].aio_handle; disk_block = get_unaligned_le64(_map->disk_starting_blk) + first_row * strip_size +
[PATCH 12/20] smartpqi: add support for huawei controllers
From: Ajish Koshy Reviewed-by: Murthy Bhat Reviewed-by: Mahesh Rajashekhara Reviewed-by: Dave Carroll Reviewed-by: Scott Teel Reviewed-by: Kevin Barnett Signed-off-by: Ajish Koshy Signed-off-by: Don Brace --- drivers/scsi/smartpqi/smartpqi_init.c | 24 1 file changed, 24 insertions(+) diff --git a/drivers/scsi/smartpqi/smartpqi_init.c b/drivers/scsi/smartpqi/smartpqi_init.c index a98b3de40c74..c5640b379505 100644 --- a/drivers/scsi/smartpqi/smartpqi_init.c +++ b/drivers/scsi/smartpqi/smartpqi_init.c @@ -7382,6 +7382,30 @@ static const struct pci_device_id pqi_pci_id_table[] = { PCI_DEVICE_SUB(PCI_VENDOR_ID_ADAPTEC2, 0x028f, 0x1bd4, 0x004c) }, + { + PCI_DEVICE_SUB(PCI_VENDOR_ID_ADAPTEC2, 0x028f, + 0x19e5, 0xd227) + }, + { + PCI_DEVICE_SUB(PCI_VENDOR_ID_ADAPTEC2, 0x028f, + 0x19e5, 0xd228) + }, + { + PCI_DEVICE_SUB(PCI_VENDOR_ID_ADAPTEC2, 0x028f, + 0x19e5, 0xd229) + }, + { + PCI_DEVICE_SUB(PCI_VENDOR_ID_ADAPTEC2, 0x028f, + 0x19e5, 0xd22a) + }, + { + PCI_DEVICE_SUB(PCI_VENDOR_ID_ADAPTEC2, 0x028f, + 0x19e5, 0xd22b) + }, + { + PCI_DEVICE_SUB(PCI_VENDOR_ID_ADAPTEC2, 0x028f, + 0x19e5, 0xd22c) + }, { PCI_DEVICE_SUB(PCI_VENDOR_ID_ADAPTEC2, 0x028f, PCI_VENDOR_ID_ADAPTEC2, 0x0110)
[PATCH 10/20] smartpqi: wake up drives after os resumes from suspend
From: Dave Carroll - set allow_restart option during scsi_device init. This allows the kernel to send a START/STOP Unit command to the drive if it encounters a 4/2 check condition in sense data. Reviewed-by: Murthy Bhat Reviewed-by: Mahesh Rajashekhara Reviewed-by: Kevin Barnett Signed-off-by: Dave Carroll Signed-off-by: Don Brace --- drivers/scsi/smartpqi/smartpqi_init.c |2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/scsi/smartpqi/smartpqi_init.c b/drivers/scsi/smartpqi/smartpqi_init.c index 6716d30dc5ef..acde0abbbf43 100644 --- a/drivers/scsi/smartpqi/smartpqi_init.c +++ b/drivers/scsi/smartpqi/smartpqi_init.c @@ -5434,6 +5434,8 @@ static int pqi_slave_alloc(struct scsi_device *sdev) } if (pqi_is_logical_device(device)) pqi_disable_write_same(sdev); + else + sdev->allow_restart = 1; } spin_unlock_irqrestore(_info->scsi_device_list_lock, flags);
[PATCH 02/20] Add retries for device reset
From: Mahesh Rajashekhara Reviewed-by: Ajish Koshy Reviewed-by: Murthy Bhat Reviewed-by: Justin Lindley Reviewed-by: Scott Benesh Reviewed-by: Dave Carroll Reviewed-by: Scott Teel Reviewed-by: Kevin Barnett Signed-off-by: Mahesh Rajashekhara Signed-off-by: Don Brace --- drivers/scsi/smartpqi/smartpqi.h |1 + drivers/scsi/smartpqi/smartpqi_init.c | 14 +- 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/drivers/scsi/smartpqi/smartpqi.h b/drivers/scsi/smartpqi/smartpqi.h index bbf056ddd026..646982e45904 100644 --- a/drivers/scsi/smartpqi/smartpqi.h +++ b/drivers/scsi/smartpqi/smartpqi.h @@ -587,6 +587,7 @@ typedef u32 pqi_index_t; #define SOP_TASK_ATTRIBUTE_ACA 4 #define SOP_TMF_COMPLETE 0x0 +#define SOP_TMF_REJECTED 0x4 #define SOP_TMF_FUNCTION_SUCCEEDED 0x8 /* additional CDB bytes usage field codes */ diff --git a/drivers/scsi/smartpqi/smartpqi_init.c b/drivers/scsi/smartpqi/smartpqi_init.c index 72e635cc594c..db95b1cb6889 100644 --- a/drivers/scsi/smartpqi/smartpqi_init.c +++ b/drivers/scsi/smartpqi/smartpqi_init.c @@ -2665,6 +2665,9 @@ static int pqi_interpret_task_management_response( case SOP_TMF_FUNCTION_SUCCEEDED: rc = 0; break; + case SOP_TMF_REJECTED: + rc = -EAGAIN; + break; default: rc = -EIO; break; @@ -5218,14 +5221,23 @@ static int pqi_lun_reset(struct pqi_ctrl_info *ctrl_info, return rc; } +#define PQI_LUN_RESET_RETRIES 3 +#define PQI_LUN_RESET_RETRY_INTERVAL_MSECS 1 /* Performs a reset at the LUN level. */ static int pqi_device_reset(struct pqi_ctrl_info *ctrl_info, struct pqi_scsi_dev *device) { int rc; + unsigned int retries; - rc = pqi_lun_reset(ctrl_info, device); + for (retries = 0;;) { + rc = pqi_lun_reset(ctrl_info, device); + if (rc != -EAGAIN || + ++retries > PQI_LUN_RESET_RETRIES) + break; + msleep(PQI_LUN_RESET_RETRY_INTERVAL_MSECS); + } if (rc == 0) rc = pqi_device_wait_for_pending_io(ctrl_info, device);
[PATCH 07/20] smartpqi: add sysfs attributes
From: Dave Carroll - add sysfs device attributes, unique_id, lunid and path_info. Reviewed-by: Scott Teel Reviewed-by: Kevin Barnett Signed-off-by: Dave Carroll Signed-off-by: Don Brace --- drivers/scsi/smartpqi/smartpqi.h |3 drivers/scsi/smartpqi/smartpqi_init.c | 232 + 2 files changed, 235 insertions(+) diff --git a/drivers/scsi/smartpqi/smartpqi.h b/drivers/scsi/smartpqi/smartpqi.h index fcc4b937de71..a39c324dedab 100644 --- a/drivers/scsi/smartpqi/smartpqi.h +++ b/drivers/scsi/smartpqi/smartpqi.h @@ -852,6 +852,7 @@ struct pqi_scsi_dev { u8 scsi3addr[8]; __be64 wwid; u8 volume_id[16]; + u8 unique_id[16]; u8 is_physical_device : 1; u8 is_external_raid_device : 1; u8 target_lun_valid : 1; @@ -898,6 +899,8 @@ struct pqi_scsi_dev { #define CISS_VPD_LV_DEVICE_GEOMETRY0xc1/* vendor-specific page */ #define CISS_VPD_LV_BYPASS_STATUS 0xc2/* vendor-specific page */ #define CISS_VPD_LV_STATUS 0xc3/* vendor-specific page */ +#define SCSI_VPD_HEADER_SZ 4 +#define SCSI_VPD_DEVICE_ID_IDX 8 /* Index of page id in page */ #define VPD_PAGE (1 << 8) diff --git a/drivers/scsi/smartpqi/smartpqi_init.c b/drivers/scsi/smartpqi/smartpqi_init.c index 004a7b9aac26..20c8c85660e7 100644 --- a/drivers/scsi/smartpqi/smartpqi_init.c +++ b/drivers/scsi/smartpqi/smartpqi_init.c @@ -573,6 +573,79 @@ static inline int pqi_scsi_inquiry(struct pqi_ctrl_info *ctrl_info, buffer, buffer_length, vpd_page, NULL, NO_TIMEOUT); } +static bool pqi_vpd_page_supported(struct pqi_ctrl_info *ctrl_info, + u8 *scsi3addr, u16 vpd_page) +{ + int rc; + int i; + int pages; + unsigned char *buf, bufsize; + + buf = kzalloc(256, GFP_KERNEL); + if (!buf) + return false; + + /* Get the size of the page list first */ + rc = pqi_scsi_inquiry(ctrl_info, scsi3addr, + VPD_PAGE | SCSI_VPD_SUPPORTED_PAGES, + buf, SCSI_VPD_HEADER_SZ); + if (rc != 0) + goto exit_unsupported; + + pages = buf[3]; + if ((pages + SCSI_VPD_HEADER_SZ) <= 255) + bufsize = pages + SCSI_VPD_HEADER_SZ; + else + bufsize = 255; + + /* Get the whole VPD page list */ + rc = pqi_scsi_inquiry(ctrl_info, scsi3addr, + VPD_PAGE | SCSI_VPD_SUPPORTED_PAGES, + buf, bufsize); + if (rc != 0) + goto exit_unsupported; + + pages = buf[3]; + for (i = 1; i <= pages; i++) + if (buf[3 + i] == vpd_page) + goto exit_supported; + +exit_unsupported: + kfree(buf); + return false; + +exit_supported: + kfree(buf); + return true; +} + +static int pqi_get_device_id(struct pqi_ctrl_info *ctrl_info, + u8 *scsi3addr, u8 *device_id, int buflen) +{ + int rc; + unsigned char *buf; + + if (!pqi_vpd_page_supported(ctrl_info, scsi3addr, SCSI_VPD_DEVICE_ID)) + return 1; /* function not supported */ + + buf = kzalloc(64, GFP_KERNEL); + if (!buf) + return -ENOMEM; + + rc = pqi_scsi_inquiry(ctrl_info, scsi3addr, + VPD_PAGE | SCSI_VPD_DEVICE_ID, + buf, 64); + if (rc == 0) { + if (buflen > 16) + buflen = 16; + memcpy(device_id, [SCSI_VPD_DEVICE_ID_IDX], buflen); + } + + kfree(buf); + + return rc; +} + static int pqi_identify_physical_device(struct pqi_ctrl_info *ctrl_info, struct pqi_scsi_dev *device, struct bmic_identify_physical_device *buffer, @@ -1244,6 +1317,14 @@ static int pqi_get_device_info(struct pqi_ctrl_info *ctrl_info, } } + if (pqi_get_device_id(ctrl_info, device->scsi3addr, + device->unique_id, sizeof(device->unique_id)) < 0) + dev_warn(_info->pci_dev->dev, + "Can't get device id for scsi %d:%d:%d:%d\n", + ctrl_info->scsi_host->host_no, + device->bus, device->target, + device->lun); + out: kfree(buffer); @@ -1775,6 +1856,12 @@ static inline bool pqi_skip_device(u8 *scsi3addr) return false; } +static inline bool pqi_expose_device(struct pqi_scsi_dev *device) +{ + return !device->is_physical_device || + !pqi_skip_device(device->scsi3addr); +} + static int pqi_update_scsi_devices(struct pqi_ctrl_info *ctrl_info) { int i; @@ -5722,6 +5809,145 @@ static struct device_attribute *pqi_shost_attrs[] = { NULL }; +static ssize_t pqi_unique_id_show(struct device *dev, + struct device_attribute *attr, char *buffer) +{ +
[PATCH 08/20] smartpqi: add h3c ssid
From: Murthy Bhat Reviewed-by: Scott Benesh Reviewed-by: Mahesh Rajashekhara Reviewed-by: Dave Carroll Reviewed-by: Scott Teel Reviewed-by: Kevin Barnett Signed-off-by: Murthy Bhat Signed-off-by: Don Brace --- drivers/scsi/smartpqi/smartpqi_init.c |8 1 file changed, 8 insertions(+) diff --git a/drivers/scsi/smartpqi/smartpqi_init.c b/drivers/scsi/smartpqi/smartpqi_init.c index 20c8c85660e7..7ece16495a09 100644 --- a/drivers/scsi/smartpqi/smartpqi_init.c +++ b/drivers/scsi/smartpqi/smartpqi_init.c @@ -7327,6 +7327,14 @@ static const struct pci_device_id pqi_pci_id_table[] = { PCI_DEVICE_SUB(PCI_VENDOR_ID_ADAPTEC2, 0x028f, 0x193d, 0x8461) }, + { + PCI_DEVICE_SUB(PCI_VENDOR_ID_ADAPTEC2, 0x028f, + 0x193d, 0xc460) + }, + { + PCI_DEVICE_SUB(PCI_VENDOR_ID_ADAPTEC2, 0x028f, + 0x193d, 0xc461) + }, { PCI_DEVICE_SUB(PCI_VENDOR_ID_ADAPTEC2, 0x028f, 0x193d, 0xf460)
[PATCH 04/20] smartpqi: correct host serial num for ssa
From: Mahesh Rajashekhara Reviewed-by: Scott Benesh Reviewed-by: Ajish Koshy Reviewed-by: Murthy Bhat Reviewed-by: Mahesh Rajashekhara Reviewed-by: Dave Carroll Reviewed-by: Scott Teel Reviewed-by: Kevin Barnett Signed-off-by: Mahesh Rajashekhara Signed-off-by: Don Brace --- drivers/scsi/smartpqi/smartpqi_init.c |3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/scsi/smartpqi/smartpqi_init.c b/drivers/scsi/smartpqi/smartpqi_init.c index 8e68dba1c789..667a27ae3c07 100644 --- a/drivers/scsi/smartpqi/smartpqi_init.c +++ b/drivers/scsi/smartpqi/smartpqi_init.c @@ -645,6 +645,7 @@ struct bmic_host_wellness_driver_version { u8 driver_version_tag[2]; __le16 driver_version_length; chardriver_version[32]; + u8 dont_write_tag[2]; u8 end_tag[2]; }; @@ -674,6 +675,8 @@ static int pqi_write_driver_version_to_host_wellness( strncpy(buffer->driver_version, "Linux " DRIVER_VERSION, sizeof(buffer->driver_version) - 1); buffer->driver_version[sizeof(buffer->driver_version) - 1] = '\0'; + buffer->dont_write_tag[0] = 'D'; + buffer->dont_write_tag[1] = 'W'; buffer->end_tag[0] = 'Z'; buffer->end_tag[1] = 'Z';
[PATCH 01/20] smartpqi: add support for PQI Config Table handshake
From: Kevin Barnett Add support for new IUs and parsing of the Firmware Features section of the PQI Config Table to implement the "handshake" between the driver and firmware to communicate firmware features supported and enabled by the driver. Reviewed-by: Ajish Koshy Reviewed-by: Mahesh Rajashekhara Reviewed-by: Murthy Bhat Reviewed-by: Justin Lindley Reviewed-by: Scott Teel Signed-off-by: Kevin Barnett Signed-off-by: Don Brace --- drivers/scsi/smartpqi/smartpqi.h | 43 ++ drivers/scsi/smartpqi/smartpqi_init.c | 254 - 2 files changed, 293 insertions(+), 4 deletions(-) diff --git a/drivers/scsi/smartpqi/smartpqi.h b/drivers/scsi/smartpqi/smartpqi.h index e97bf2670315..bbf056ddd026 100644 --- a/drivers/scsi/smartpqi/smartpqi.h +++ b/drivers/scsi/smartpqi/smartpqi.h @@ -389,6 +389,35 @@ struct pqi_task_management_response { u8 response_code; }; +struct pqi_vendor_general_request { + struct pqi_iu_header header; + __le16 request_id; + __le16 function_code; + union { + struct { + __le16 first_section; + __le16 last_section; + u8 reserved[48]; + } config_table_update; + + struct { + __le64 buffer_address; + __le32 buffer_length; + u8 reserved[40]; + } ofa_memory_allocation; + } data; +}; + +struct pqi_vendor_general_response { + struct pqi_iu_header header; + __le16 request_id; + __le16 function_code; + __le16 status; + u8 reserved[2]; +}; + +#define PQI_VENDOR_GENERAL_CONFIG_TABLE_UPDATE 0 + struct pqi_aio_error_info { u8 status; u8 service_response; @@ -419,6 +448,7 @@ struct pqi_raid_error_info { #define PQI_REQUEST_IU_GENERAL_ADMIN 0x60 #define PQI_REQUEST_IU_REPORT_VENDOR_EVENT_CONFIG 0x72 #define PQI_REQUEST_IU_SET_VENDOR_EVENT_CONFIG 0x73 +#define PQI_REQUEST_IU_VENDOR_GENERAL 0x75 #define PQI_REQUEST_IU_ACKNOWLEDGE_VENDOR_EVENT0xf6 #define PQI_RESPONSE_IU_GENERAL_MANAGEMENT 0x81 @@ -430,6 +460,7 @@ struct pqi_raid_error_info { #define PQI_RESPONSE_IU_AIO_PATH_IO_ERROR 0xf3 #define PQI_RESPONSE_IU_AIO_PATH_DISABLED 0xf4 #define PQI_RESPONSE_IU_VENDOR_EVENT 0xf5 +#define PQI_RESPONSE_IU_VENDOR_GENERAL 0xf7 #define PQI_GENERAL_ADMIN_FUNCTION_REPORT_DEVICE_CAPABILITY0x0 #define PQI_GENERAL_ADMIN_FUNCTION_CREATE_IQ 0x10 @@ -644,6 +675,7 @@ struct pqi_encryption_info { #define PQI_CONFIG_TABLE_MAX_LENGTH((u16)~0) /* configuration table section IDs */ +#define PQI_CONFIG_TABLE_ALL_SECTIONS (-1) #define PQI_CONFIG_TABLE_SECTION_GENERAL_INFO 0 #define PQI_CONFIG_TABLE_SECTION_FIRMWARE_FEATURES 1 #define PQI_CONFIG_TABLE_SECTION_FIRMWARE_ERRATA 2 @@ -680,6 +712,17 @@ struct pqi_config_table_general_info { /* command */ }; +struct pqi_config_table_firmware_features { + struct pqi_config_table_section_header header; + __le16 num_elements; + u8 features_supported[]; +/* u8 features_requested_by_host[]; */ +/* u8 features_enabled[]; */ +}; + +#define PQI_FIRMWARE_FEATURE_OFA 0 +#define PQI_FIRMWARE_FEATURE_SMP 1 + struct pqi_config_table_debug { struct pqi_config_table_section_header header; __le32 scratchpad; diff --git a/drivers/scsi/smartpqi/smartpqi_init.c b/drivers/scsi/smartpqi/smartpqi_init.c index a25a07a0b7f0..72e635cc594c 100644 --- a/drivers/scsi/smartpqi/smartpqi_init.c +++ b/drivers/scsi/smartpqi/smartpqi_init.c @@ -2706,6 +2706,12 @@ static unsigned int pqi_process_io_intr(struct pqi_ctrl_info *ctrl_info, case PQI_RESPONSE_IU_AIO_PATH_IO_SUCCESS: case PQI_RESPONSE_IU_GENERAL_MANAGEMENT: break; + case PQI_RESPONSE_IU_VENDOR_GENERAL: + io_request->status = + get_unaligned_le16( + &((struct pqi_vendor_general_response *) + response)->status); + break; case PQI_RESPONSE_IU_TASK_MANAGEMENT: io_request->status = pqi_interpret_task_management_response( @@ -5947,6 +5953,233 @@ static int pqi_get_ctrl_firmware_version(struct pqi_ctrl_info *ctrl_info) return rc; } +struct pqi_config_table_section_info { + struct pqi_ctrl_info *ctrl_info; + void*section; + u32 section_offset; + void __iomem*section_iomem_addr; +}; + +static inline bool pqi_is_firmware_feature_supported( +
[PATCH 00/20] smartpqi updates
These patches are based on Linus's tree The changes are: - smartpqi-add-support-for-PQI-Config-Table-handshake . add support for get/set controller features. - smartpqi-add-retries-for-device-resets . re-attempt device reset. - smartpqi-add-no_write_same-for-logical-volumes . turn off WRITE SAME for logical volumes. - smartpqi-correct-host-serial-num-for-ssa . update host serial number - smartpqi-turn-off-lun-data-caching-for-ptraid . get fresh lun list from RBODS. - smartpqi-refactor-sending-controller-raid-requests . condense commonly used code. - smartpqi-add-sysfs-attributes . add in driver attributes. - smartpqi-add-h3c-ssid . add support for more controllers. - smartpqi-fix-disk-name-mount-point . correct sysfs attribute for unique ids. - smartpqi-wake-up-drives-after-os-resumes-from-suspend . have OS start up disks after resume. - smartpqi-enhance-numa-node-detection . set pci device to correct NUMA node. - smartpqi-add-support-for-huawei-controllers . add support for more controllers. - smartpqi-check-for-null-device-pointers . wait for all outstanding I/O to complete before removing a volume - smartpqi-allow-for-larger-raid-maps . correct rare case for very large volume configurations. - smartpqi-do-not-offline-disks-for-transient-did-no-connect-conditions . remove call to scsi_device_set_state - smartpqi-correct-volume-status . correct rare case for volume deletion during a scan operation. - smartpqi-correct-lun-reset-issues . clear scsi cmd result after a reset. - smartpqi-add-module-param-to-disable-irq-affinity . allow some customers to change IRQ affinity. - smartpqi-add-smp_utils-support . add support for smp_utils. - smartpqi-bump-driver-version --- Ajish Koshy (2): smartpqi: add support for huawei controllers smartpqi: allow for larger raid maps Dave Carroll (7): smartpqi: add no_write_same for logical volumes smartpqi: turn off lun data caching for ptraid smartpqi: refactor sending controller raid requests smartpqi: add sysfs attributes smartpqi: wake up drives after os resumes from suspend smartpqi: do not offline disks for transient did no connect conditions smartpqi: correct volume status Don Brace (3): smartpqi: add module param to disable irq affinity smartpqi: add smp_utils support smartpqi: bump driver version Kevin Barnett (2): smartpqi: add support for PQI Config Table handshake smartpqi: correct lun reset issues Mahesh Rajashekhara (3): Add retries for device reset smartpqi: correct host serial num for ssa smartpqi: check for null device pointers Murthy Bhat (2): smartpqi: add h3c ssid smartpqi: fix disk name mount point Sagar Biradar (1): smartpqi: enhance numa node detection drivers/scsi/smartpqi/smartpqi.h | 144 +++ drivers/scsi/smartpqi/smartpqi_init.c | 992 drivers/scsi/smartpqi/smartpqi_sas_transport.c | 164 3 files changed, 1131 insertions(+), 169 deletions(-) -- Signature
[PATCH 05/20] smartpqi: turn off lun data caching for ptraid
From: Dave Carroll - allow update the luns for PTRAID devices. Reviewed-by: Ajish Koshy Reviewed-by: Murthy Bhat Reviewed-by: Mahesh Rajashekhara Reviewed-by: Justin Lindley Reviewed-by: Scott Teel Reviewed-by: Kevin Barnett Signed-off-by: Dave Carroll Signed-off-by: Don Brace --- drivers/scsi/smartpqi/smartpqi.h |6 +++ drivers/scsi/smartpqi/smartpqi_init.c | 79 ++--- 2 files changed, 79 insertions(+), 6 deletions(-) diff --git a/drivers/scsi/smartpqi/smartpqi.h b/drivers/scsi/smartpqi/smartpqi.h index 646982e45904..fcc4b937de71 100644 --- a/drivers/scsi/smartpqi/smartpqi.h +++ b/drivers/scsi/smartpqi/smartpqi.h @@ -1126,6 +1126,8 @@ enum pqi_ctrl_mode { #define BMIC_SENSE_SUBSYSTEM_INFORMATION 0x66 #define BMIC_WRITE_HOST_WELLNESS 0xa5 #define BMIC_FLUSH_CACHE 0xc2 +#define BMIC_SET_DIAG_OPTIONS 0xf4 +#define BMIC_SENSE_DIAG_OPTIONS0xf5 #define SA_FLUSH_CACHE 0x1 @@ -1250,6 +1252,10 @@ enum bmic_flush_cache_shutdown_event { RESTART = 4 }; +struct bmic_diag_options { + __le32 options; +}; + #pragma pack() int pqi_add_sas_host(struct Scsi_Host *shost, struct pqi_ctrl_info *ctrl_info); diff --git a/drivers/scsi/smartpqi/smartpqi_init.c b/drivers/scsi/smartpqi/smartpqi_init.c index 667a27ae3c07..9d9e90e13f5c 100644 --- a/drivers/scsi/smartpqi/smartpqi_init.c +++ b/drivers/scsi/smartpqi/smartpqi_init.c @@ -395,6 +395,7 @@ static int pqi_build_raid_path_request(struct pqi_ctrl_info *ctrl_info, u16 vpd_page, enum dma_data_direction *dir) { u8 *cdb; + size_t cdb_length = buffer_length; memset(request, 0, sizeof(*request)); @@ -417,7 +418,7 @@ static int pqi_build_raid_path_request(struct pqi_ctrl_info *ctrl_info, cdb[1] = 0x1; cdb[2] = (u8)vpd_page; } - cdb[4] = (u8)buffer_length; + cdb[4] = (u8)cdb_length; break; case CISS_REPORT_LOG: case CISS_REPORT_PHYS: @@ -427,32 +428,36 @@ static int pqi_build_raid_path_request(struct pqi_ctrl_info *ctrl_info, cdb[1] = CISS_REPORT_PHYS_EXTENDED; else cdb[1] = CISS_REPORT_LOG_EXTENDED; - put_unaligned_be32(buffer_length, [6]); + put_unaligned_be32(cdb_length, [6]); break; case CISS_GET_RAID_MAP: request->data_direction = SOP_READ_FLAG; cdb[0] = CISS_READ; cdb[1] = CISS_GET_RAID_MAP; - put_unaligned_be32(buffer_length, [6]); + put_unaligned_be32(cdb_length, [6]); break; case SA_FLUSH_CACHE: request->data_direction = SOP_WRITE_FLAG; cdb[0] = BMIC_WRITE; cdb[6] = BMIC_FLUSH_CACHE; - put_unaligned_be16(buffer_length, [7]); + put_unaligned_be16(cdb_length, [7]); break; + case BMIC_SENSE_DIAG_OPTIONS: + cdb_length = 0; case BMIC_IDENTIFY_CONTROLLER: case BMIC_IDENTIFY_PHYSICAL_DEVICE: request->data_direction = SOP_READ_FLAG; cdb[0] = BMIC_READ; cdb[6] = cmd; - put_unaligned_be16(buffer_length, [7]); + put_unaligned_be16(cdb_length, [7]); break; + case BMIC_SET_DIAG_OPTIONS: + cdb_length = 0; case BMIC_WRITE_HOST_WELLNESS: request->data_direction = SOP_WRITE_FLAG; cdb[0] = BMIC_WRITE; cdb[6] = cmd; - put_unaligned_be16(buffer_length, [7]); + put_unaligned_be16(cdb_length, [7]); break; default: dev_err(_info->pci_dev->dev, "unknown command 0x%c\n", @@ -618,6 +623,54 @@ static int pqi_flush_cache(struct pqi_ctrl_info *ctrl_info, return rc; } + +#define PQI_FETCH_PTRAID_DATA (1UL<<31) + +static int pqi_set_diag_rescan(struct pqi_ctrl_info *ctrl_info) +{ + int rc; + struct pqi_raid_path_request request; + struct bmic_diag_options *diag; + enum dma_data_direction pci_direction; + + diag = kzalloc(sizeof(*diag), GFP_KERNEL); + if (!diag) + return -ENOMEM; + + rc = pqi_build_raid_path_request(ctrl_info, , + BMIC_SENSE_DIAG_OPTIONS, RAID_CTLR_LUNID, diag, + sizeof(*diag), 0, _direction); + if (rc) + goto out; + + rc = pqi_submit_raid_request_synchronous(ctrl_info, , + 0, NULL, NO_TIMEOUT); + + pqi_pci_unmap(ctrl_info->pci_dev, request.sg_descriptors, 1, + pci_direction); + + if (rc) + goto out; + + diag->options |= cpu_to_le32(PQI_FETCH_PTRAID_DATA); + + rc =
[PATCH 03/20] smartpqi: add no_write_same for logical volumes
From: Dave Carroll During slave_alloc, for logical volumes include no_write_same into the scsi_device structure. This will insure that WRITE_SAME will not be used for LD's. Reviewed-by: Ajish Koshy Reviewed-by: Murthy Bhat Reviewed-by: Mahesh Rajashekhara Reviewed-by: Justin Lindley Reviewed-by: Scott Benesh Reviewed-by: Scott Teel Reviewed-by: Kevin Barnett Signed-off-by: Dave Carroll Signed-off-by: Don Brace --- drivers/scsi/smartpqi/smartpqi_init.c |7 +++ 1 file changed, 7 insertions(+) diff --git a/drivers/scsi/smartpqi/smartpqi_init.c b/drivers/scsi/smartpqi/smartpqi_init.c index db95b1cb6889..8e68dba1c789 100644 --- a/drivers/scsi/smartpqi/smartpqi_init.c +++ b/drivers/scsi/smartpqi/smartpqi_init.c @@ -176,6 +176,11 @@ static inline void pqi_scsi_done(struct scsi_cmnd *scmd) scmd->scsi_done(scmd); } +static inline void pqi_disable_write_same(struct scsi_device *sdev) +{ + sdev->no_write_same = 1; +} + static inline bool pqi_scsi3addr_equal(u8 *scsi3addr1, u8 *scsi3addr2) { return memcmp(scsi3addr1, scsi3addr2, 8) == 0; @@ -5326,6 +5331,8 @@ static int pqi_slave_alloc(struct scsi_device *sdev) scsi_change_queue_depth(sdev, device->advertised_queue_depth); } + if (pqi_is_logical_device(device)) + pqi_disable_write_same(sdev); } spin_unlock_irqrestore(_info->scsi_device_list_lock, flags);
[PATCH 06/20] smartpqi: refactor sending controller raid requests
From: Dave Carroll cleanup the common code which creates a raid path request for the controller LUNID and sends it synchronously, into a common routine; Reviewed-by: Murthy Bhat Reviewed-by: Mahesh Rajashekhara Reviewed-by: Scott Teel Reviewed-by: Kevin Barnett Signed-off-by: Dave Carroll Signed-off-by: Don Brace --- drivers/scsi/smartpqi/smartpqi_init.c | 134 +++-- 1 file changed, 46 insertions(+), 88 deletions(-) diff --git a/drivers/scsi/smartpqi/smartpqi_init.c b/drivers/scsi/smartpqi/smartpqi_init.c index 9d9e90e13f5c..004a7b9aac26 100644 --- a/drivers/scsi/smartpqi/smartpqi_init.c +++ b/drivers/scsi/smartpqi/smartpqi_init.c @@ -519,44 +519,58 @@ static void pqi_free_io_request(struct pqi_io_request *io_request) atomic_dec(_request->refcount); } -static int pqi_identify_controller(struct pqi_ctrl_info *ctrl_info, - struct bmic_identify_controller *buffer) +static int pqi_send_scsi_raid_request(struct pqi_ctrl_info *ctrl_info, u8 cmd, + u8 *scsi3addr, void *buffer, size_t buffer_length, u16 vpd_page, + struct pqi_raid_error_info *error_info, + unsigned long timeout_msecs) { int rc; enum dma_data_direction dir; struct pqi_raid_path_request request; rc = pqi_build_raid_path_request(ctrl_info, , - BMIC_IDENTIFY_CONTROLLER, RAID_CTLR_LUNID, buffer, - sizeof(*buffer), 0, ); + cmd, scsi3addr, buffer, + buffer_length, vpd_page, ); if (rc) return rc; - rc = pqi_submit_raid_request_synchronous(ctrl_info, , 0, - NULL, NO_TIMEOUT); + rc = pqi_submit_raid_request_synchronous(ctrl_info, , +0, error_info, timeout_msecs); pqi_pci_unmap(ctrl_info->pci_dev, request.sg_descriptors, 1, dir); return rc; } -static int pqi_scsi_inquiry(struct pqi_ctrl_info *ctrl_info, - u8 *scsi3addr, u16 vpd_page, void *buffer, size_t buffer_length) +/* Helper functions for pqi_send_scsi_raid_request */ + +static inline int pqi_send_ctrl_raid_request(struct pqi_ctrl_info *ctrl_info, + u8 cmd, void *buffer, size_t buffer_length) { - int rc; - enum dma_data_direction dir; - struct pqi_raid_path_request request; + return pqi_send_scsi_raid_request(ctrl_info, cmd, RAID_CTLR_LUNID, + buffer, buffer_length, 0, NULL, NO_TIMEOUT); +} - rc = pqi_build_raid_path_request(ctrl_info, , - INQUIRY, scsi3addr, buffer, buffer_length, vpd_page, - ); - if (rc) - return rc; +static inline int pqi_send_ctrl_raid_with_error(struct pqi_ctrl_info *ctrl_info, + u8 cmd, void *buffer, size_t buffer_length, + struct pqi_raid_error_info *error_info) +{ + return pqi_send_scsi_raid_request(ctrl_info, cmd, RAID_CTLR_LUNID, + buffer, buffer_length, 0, error_info, NO_TIMEOUT); +} - rc = pqi_submit_raid_request_synchronous(ctrl_info, , 0, - NULL, NO_TIMEOUT); - pqi_pci_unmap(ctrl_info->pci_dev, request.sg_descriptors, 1, dir); - return rc; +static inline int pqi_identify_controller(struct pqi_ctrl_info *ctrl_info, + struct bmic_identify_controller *buffer) +{ + return pqi_send_ctrl_raid_request(ctrl_info, BMIC_IDENTIFY_CONTROLLER, + buffer, sizeof(*buffer)); +} + +static inline int pqi_scsi_inquiry(struct pqi_ctrl_info *ctrl_info, + u8 *scsi3addr, u16 vpd_page, void *buffer, size_t buffer_length) +{ + return pqi_send_scsi_raid_request(ctrl_info, INQUIRY, scsi3addr, + buffer, buffer_length, vpd_page, NULL, NO_TIMEOUT); } static int pqi_identify_physical_device(struct pqi_ctrl_info *ctrl_info, @@ -590,9 +604,7 @@ static int pqi_flush_cache(struct pqi_ctrl_info *ctrl_info, enum bmic_flush_cache_shutdown_event shutdown_event) { int rc; - struct pqi_raid_path_request request; struct bmic_flush_cache *flush_cache; - enum dma_data_direction dir; /* * Don't bother trying to flush the cache if the controller is @@ -607,17 +619,9 @@ static int pqi_flush_cache(struct pqi_ctrl_info *ctrl_info, flush_cache->shutdown_event = shutdown_event; - rc = pqi_build_raid_path_request(ctrl_info, , - SA_FLUSH_CACHE, RAID_CTLR_LUNID, flush_cache, - sizeof(*flush_cache), 0, ); - if (rc) - goto out; + rc = pqi_send_ctrl_raid_request(ctrl_info, SA_FLUSH_CACHE, flush_cache, + sizeof(*flush_cache)); - rc = pqi_submit_raid_request_synchronous(ctrl_info, , - 0, NULL, NO_TIMEOUT); - - pqi_pci_unmap(ctrl_info->pci_dev, request.sg_descriptors, 1, dir); -out: kfree(flush_cache); return rc; @@ -629,66 +633,32 @@ static int pqi_flush_cache(struct pqi_ctrl_info
Re: BUG in copy_page_to_iter() when iscsi sets ENABLE_CLUSTERING
Note that independent of what we do in the Linux iSCSI initiator this is a network DOS, so we'll have to fix it. On Wed, Dec 05, 2018 at 12:09:40PM -0800, Lee Duncan wrote: > I recently found what I believe is a bug, and I'd appreciate feedback > on if that is correct, and if so how to proceed. > > BACKGROUND > > Recently Christoph Hellwig sent an email to driver maintainers for > drivers that set ".use_clustering" to DISABLE_CLUSTERING in their SCSI > Host templates, asking if the setting could be changed to > ENABLE_CLUSTERING. > > As part of answering that question, I set ENABLE_CLUSTERING in > drivers/scsi/iscsi_tcp.c and tested both the iscsi initiator and > target. > > As a reminder, setting ENABLE_CLUSTERING means that adjacent bio-s can > be merged. This can make IO faster, but it means that drivers must be > able to deal with IOs that cross page boundaries, since bio merges can > create such IOs. > > RESULTS > > The iscsi initiator code can handle ENABLE_CLUSTERING just fine, but > the iscsi target code fails. It seems to assume that IOs do *NOT* > cross a page boundary. > > The problem is in iscsi lib/iov_iter.c, in the functions > copy_page_to_iter() and page_copy_sane() (see below for how to > reproduce): > > >> static inline bool page_copy_sane(struct page *page, size_t offset, size_t > >> n) > >> { > >> struct page *head = compound_head(page); > >> size_t v = n + offset + page_address(page) - page_address(head); > >> > >> if (likely(n <= v && v <= (PAGE_SIZE << compound_order(head > >> return true; > >> WARN_ON(1); > >> return false; > >> } > >> > >> size_t copy_page_to_iter(struct page *page, size_t offset, size_t bytes, > >> struct iov_iter *i) > >> { > >> if (unlikely(!page_copy_sane(page, offset, bytes))) > >> return 0; > >> if (i->type & (ITER_BVEC|ITER_KVEC)) { > >> void *kaddr = kmap_atomic(page); > >> size_t wanted = copy_to_iter(kaddr + offset, bytes, i); > >> kunmap_atomic(kaddr); > >> return wanted; > >> } else if (unlikely(iov_iter_is_discard(i))) > >> return bytes; > >> else if (likely(!iov_iter_is_pipe(i))) > >> return copy_page_to_iter_iovec(page, offset, bytes, i); > >> else > >> return copy_page_to_iter_pipe(page, offset, bytes, i); > >> } > > Causing the following WARN_ON stack trace (repeatedly): > > >> ... > >> [ 78.644559] WARNING: CPU: 0 PID: 2192 at lib/iov_iter.c:830 > >> copy_page_to_iter+0x1a6/0x2e0 > >> [ 78.644561] Modules linked in: iscsi_tcp(E) libiscsi_tcp(E) libiscsi(E) > >> scsi_transport_iscsi(E) rfcomm(E) iscsi_target_mod(E) target_core_pscsi(E) > >> target_core_file(E) target_core_iblock(E) target_core_user(E) uio(E) > >> target_core_mod(E) configfs(E) af_packet(E) iscsi_ibft(E) > >> iscsi_boot_sysfs(E) vmw_vsock_vmci_transport(E) vsock(E) bnep(E) fuse(E) > >> crct10dif_pclmul(E) crc32_pclmul(E) ghash_clmulni_intel(E) xfs(E) > >> aesni_intel(E) snd_ens1371(E) aes_x86_64(E) snd_ac97_codec(E) > >> crypto_simd(E) cryptd(E) ac97_bus(E) glue_helper(E) snd_rawmidi(E) > >> vmw_balloon(E) snd_seq_device(E) snd_pcm(E) pcspkr(E) snd_timer(E) snd(E) > >> uvcvideo(E) btusb(E) videobuf2_vmalloc(E) btrtl(E) videobuf2_memops(E) > >> videobuf2_v4l2(E) btbcm(E) btintel(E) videodev(E) bluetooth(E) > >> videobuf2_common(E) vmw_vmci(E) ecdh_generic(E) rfkill(E) soundcore(E) > >> mptctl(E) gameport(E) joydev(E) i2c_piix4(E) e1000(E) ac(E) button(E) > >> btrfs(E) libcrc32c(E) xor(E) raid6_pq(E) hid_generic(E) usbhid(E) > >> sr_mod(E) cdrom(E) ata_generic(E) > >> [ 78.644583] crc32c_intel(E) serio_raw(E) mptspi(E) > >> scsi_transport_spi(E) mptscsih(E) ata_piix(E) uhci_hcd(E) ehci_pci(E) > >> ehci_hcd(E) vmwgfx(E) drm_kms_helper(E) syscopyarea(E) sysfillrect(E) > >> sysimgblt(E) fb_sys_fops(E) usbcore(E) ttm(E) mptbase(E) drm(E) sg(E) > >> dm_multipath(E) dm_mod(E) scsi_dh_rdac(E) scsi_dh_emc(E) scsi_dh_alua(E) > >> [ 78.644593] CPU: 0 PID: 2192 Comm: iscsi_trx Tainted: GE > >> 4.20.0-rc4-1-default+ #1 openSUSE Tumbleweed (unreleased) > >> [ 78.644593] Hardware name: VMware, Inc. VMware Virtual Platform/440BX > >> Desktop Reference Platform, BIOS 6.00 05/19/2017 > >> [ 78.644595] RIP: 0010:copy_page_to_iter+0x1a6/0x2e0 > >> [ 78.644596] Code: 0c 24 48 98 49 89 ce 48 89 c2 49 29 c6 48 29 ca 4d 01 > >> f4 48 01 d5 48 85 c0 0f 85 71 ff ff ff 48 85 ed 0f 84 68 ff ff ff eb b2 > >> <0f> 0b 45 31 ed eb 8d bf 01 00 00 00 e8 d9 1e c5 ff 65 4c 8b 34 25 > >> [ 78.644596] RSP: 0018:a04b41eefbe0 EFLAGS: 00010206 > >> [ 78.644597] RAX: f7acc40de180 RBX: a04b41eefd80 RCX: > >> 0028a014 > >> [ 78.644598] RDX: 2000 RSI: 1000 RDI: > >> f7acc000 > >> [ 78.644598] RBP: f7acc40de180 R08: 8f1703786000 R09: > >>
Incorrect warning in fc_rport_destroy
Hi Hannes, Commit bbc0f8bd88ab ("scsi: libfc: Add WARN_ON() when deleting rports") added a warning whose intent was to check whether the rport was still linked into the peer list. It doesn't work as intended and I consistently see messages like the following: [ 66.157471] host1: rport fffcd1: work delete [ 66.157518] WARNING: CPU: 1 PID: 151 at drivers/scsi/libfc/fc_rport.c:187 fc_rport_destroy+0x29/0x30 [libfc] [ 66.157519] Modules linked in: nfsv3 nfs_acl nfs lockd grace fscache bnx2fc cnic uio fcoe libfcoe libfc scsi_transport_fc ebtable_broute ebtables bridge 8021q garp mrp stp llc ipt_REJECT nf_reject_ipv4 xt_tcpudp xt_multiport xt_conntrack nf_conntrack nf_defrag_ipv6 nf_defrag_ipv4 iptable_filter dm_multipath sunrpc sb_edac intel_powerclamp crct10dif_pclmul crc32_pclmul ghash_clmulni_intel pcbc aesni_intel aes_x86_64 crypto_simd dm_mod cryptd lpc_ich glue_helper i2c_i801 psmouse hpilo ipmi_si sg intel_rapl_perf ipmi_devintf ipmi_msghandler hed acpi_power_meter xen_wdt ip_tables x_tables sd_mod uhci_hcd serio_raw xhci_pci ehci_pci ehci_hcd xhci_hcd hpsa bnx2x scsi_transport_sas mdio libcrc32c scsi_dh_rdac scsi_dh_hp_sw scsi_dh_emc scsi_dh_alua scsi_mod ipv6 crc_ccitt [ 66.157599] CPU: 1 PID: 151 Comm: kworker/u32:1 Not tainted 4.19.0+0 #1 [ 66.157601] Hardware name: HP ProLiant BL460c Gen9, BIOS I36 09/12/2016 [ 66.157608] Workqueue: fc_rport_eq fc_rport_work [libfc] [ 66.157616] RIP: e030:fc_rport_destroy+0x29/0x30 [libfc] [ 66.157619] Code: 00 0f 1f 44 00 00 48 8b 87 c0 00 00 00 48 8d 97 c0 00 00 00 48 39 c2 75 11 48 81 c7 f8 00 00 00 be 08 01 00 00 e9 27 2b bb c0 <0f> 0b eb eb 0f 1f 00 0f 1f 44 00 00 48 8b 3d 0c 4f 01 00 e9 9f 77 [ 66.157621] RSP: e02b:c900405cfe08 EFLAGS: 00010287 [ 66.157624] RAX: 880190ca96d0 RBX: 88016b734800 RCX: [ 66.157625] RDX: 88016b7348d0 RSI: 0001 RDI: 88016b734810 [ 66.157627] RBP: 88016b734848 R08: R09: 095b [ 66.157629] R10: 0004 R11: R12: 88016b7fb7e8 [ 66.157630] R13: 8801904de000 R14: 88016b7348e0 R15: 88016b734810 [ 66.157654] FS: () GS:88019364() knlGS: [ 66.157656] CS: e033 DS: ES: CR0: 80050033 [ 66.157658] CR2: 7f2aac406010 CR3: 00018b0d6000 CR4: 00042660 [ 66.157669] Call Trace: [ 66.157681] fc_rport_work+0x1e3/0x740 [libfc] [ 66.157693] process_one_work+0x165/0x370 [ 66.157698] worker_thread+0x49/0x3e0 [ 66.157704] kthread+0xf8/0x130 [ 66.157708] ? rescuer_thread+0x310/0x310 [ 66.157712] ? kthread_bind+0x10/0x10 [ 66.157719] ret_from_fork+0x35/0x40 [ 66.157724] ---[ end trace 78306fa56e2fdf61 ]--- This happens erroneously for two reasons: 1) If the rport is never linked into the peer list it will not be considered empty since the list_head is never initialized. 2) If the rport is deleted from the peer list using list_del_rcu(), then the list_head is in an undefined state and it is not considered empty. I'm not sure how to check that the rport has been removed from the peer list without iterating through the list. Can this check either be removed or fixed? Thanks, -- Ross Lagerwall
Urgently need money? We can help you!
Urgently need money? We can help you! Are you by the current situation in trouble or threatens you in trouble? In this way, we give you the ability to take a new development. As a rich person I feel obliged to assist people who are struggling to give them a chance. Everyone deserved a second chance and since the Government fails, it will have to come from others. No amount is too crazy for us and the maturity we determine by mutual agreement. No surprises, no extra costs, but just the agreed amounts and nothing else. Don't wait any longer and comment on this post. Please specify the amount you want to borrow and we will contact you with all the possibilities. contact us today at stewarrt.l...@gmail.com
[PATCH 4/4] mpt3sas: Update driver version to 27.101.00.00.
Update driver version from 27.100.00.00 to 27.101.00.00. Signed-off-by: Suganath Prabu --- drivers/scsi/mpt3sas/mpt3sas_base.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.h b/drivers/scsi/mpt3sas/mpt3sas_base.h index 3a294b9..8003519 100644 --- a/drivers/scsi/mpt3sas/mpt3sas_base.h +++ b/drivers/scsi/mpt3sas/mpt3sas_base.h @@ -75,9 +75,9 @@ #define MPT3SAS_DRIVER_NAME"mpt3sas" #define MPT3SAS_AUTHOR "Avago Technologies " #define MPT3SAS_DESCRIPTION"LSI MPT Fusion SAS 3.0 Device Driver" -#define MPT3SAS_DRIVER_VERSION "27.100.00.00" +#define MPT3SAS_DRIVER_VERSION "27.101.00.00" #define MPT3SAS_MAJOR_VERSION 27 -#define MPT3SAS_MINOR_VERSION 100 +#define MPT3SAS_MINOR_VERSION 101 #define MPT3SAS_BUILD_VERSION 0 #define MPT3SAS_RELEASE_VERSION00 -- 1.8.3.1
[PATCH 3/4] mpt3sas: Replace readl with ioc->base_readl.
Use ioc->base_readl to restrict the readl retries to only Aero controllers. Signed-off-by: Suganath Prabu --- drivers/scsi/mpt3sas/mpt3sas_base.c | 39 +++-- 1 file changed, 20 insertions(+), 19 deletions(-) diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.c b/drivers/scsi/mpt3sas/mpt3sas_base.c index d371c8e..8a0851e 100644 --- a/drivers/scsi/mpt3sas/mpt3sas_base.c +++ b/drivers/scsi/mpt3sas/mpt3sas_base.c @@ -742,7 +742,7 @@ mpt3sas_halt_firmware(struct MPT3SAS_ADAPTER *ioc) dump_stack(); - doorbell = readl(>chip->Doorbell); + doorbell = ioc->base_readl(>chip->Doorbell); if ((doorbell & MPI2_IOC_STATE_MASK) == MPI2_IOC_STATE_FAULT) mpt3sas_base_fault_info(ioc , doorbell); else { @@ -1351,10 +1351,10 @@ _base_mask_interrupts(struct MPT3SAS_ADAPTER *ioc) u32 him_register; ioc->mask_interrupts = 1; - him_register = readl(>chip->HostInterruptMask); + him_register = ioc->base_readl(>chip->HostInterruptMask); him_register |= MPI2_HIM_DIM + MPI2_HIM_RIM + MPI2_HIM_RESET_IRQ_MASK; writel(him_register, >chip->HostInterruptMask); - readl(>chip->HostInterruptMask); + ioc->base_readl(>chip->HostInterruptMask); } /** @@ -1368,7 +1368,7 @@ _base_unmask_interrupts(struct MPT3SAS_ADAPTER *ioc) { u32 him_register; - him_register = readl(>chip->HostInterruptMask); + him_register = ioc->base_readl(>chip->HostInterruptMask); him_register &= ~MPI2_HIM_RIM; writel(him_register, >chip->HostInterruptMask); ioc->mask_interrupts = 0; @@ -4880,7 +4880,7 @@ mpt3sas_base_get_iocstate(struct MPT3SAS_ADAPTER *ioc, int cooked) { u32 s, sc; - s = readl(>chip->Doorbell); + s = ioc->base_readl(>chip->Doorbell); sc = s & MPI2_IOC_STATE_MASK; return cooked ? sc : s; } @@ -4936,7 +4936,7 @@ _base_wait_for_doorbell_int(struct MPT3SAS_ADAPTER *ioc, int timeout) count = 0; cntdn = 1000 * timeout; do { - int_status = readl(>chip->HostInterruptStatus); + int_status = ioc->base_readl(>chip->HostInterruptStatus); if (int_status & MPI2_HIS_IOC2SYS_DB_STATUS) { dhsprintk(ioc, ioc_info(ioc, "%s: successful count(%d), timeout(%d)\n", @@ -4962,7 +4962,7 @@ _base_spin_on_doorbell_int(struct MPT3SAS_ADAPTER *ioc, int timeout) count = 0; cntdn = 2000 * timeout; do { - int_status = readl(>chip->HostInterruptStatus); + int_status = ioc->base_readl(>chip->HostInterruptStatus); if (int_status & MPI2_HIS_IOC2SYS_DB_STATUS) { dhsprintk(ioc, ioc_info(ioc, "%s: successful count(%d), timeout(%d)\n", @@ -5000,14 +5000,14 @@ _base_wait_for_doorbell_ack(struct MPT3SAS_ADAPTER *ioc, int timeout) count = 0; cntdn = 1000 * timeout; do { - int_status = readl(>chip->HostInterruptStatus); + int_status = ioc->base_readl(>chip->HostInterruptStatus); if (!(int_status & MPI2_HIS_SYS2IOC_DB_STATUS)) { dhsprintk(ioc, ioc_info(ioc, "%s: successful count(%d), timeout(%d)\n", __func__, count, timeout)); return 0; } else if (int_status & MPI2_HIS_IOC2SYS_DB_STATUS) { - doorbell = readl(>chip->Doorbell); + doorbell = ioc->base_readl(>chip->Doorbell); if ((doorbell & MPI2_IOC_STATE_MASK) == MPI2_IOC_STATE_FAULT) { mpt3sas_base_fault_info(ioc , doorbell); @@ -5042,7 +5042,7 @@ _base_wait_for_doorbell_not_used(struct MPT3SAS_ADAPTER *ioc, int timeout) count = 0; cntdn = 1000 * timeout; do { - doorbell_reg = readl(>chip->Doorbell); + doorbell_reg = ioc->base_readl(>chip->Doorbell); if (!(doorbell_reg & MPI2_DOORBELL_USED)) { dhsprintk(ioc, ioc_info(ioc, "%s: successful count(%d), timeout(%d)\n", @@ -5157,13 +5157,13 @@ _base_handshake_req_reply_wait(struct MPT3SAS_ADAPTER *ioc, int request_bytes, __le32 *mfp; /* make sure doorbell is not in use */ - if ((readl(>chip->Doorbell) & MPI2_DOORBELL_USED)) { + if ((ioc->base_readl(>chip->Doorbell) & MPI2_DOORBELL_USED)) { ioc_err(ioc, "doorbell is in use (line=%d)\n", __LINE__); return -EFAULT; } /* clear pending doorbell interrupts from previous state changes */ - if (readl(>chip->HostInterruptStatus) & + if (ioc->base_readl(>chip->HostInterruptStatus) & MPI2_HIS_IOC2SYS_DB_STATUS)
[PATCH 1/4] mpt3sas: Introduce flag for aero based controllers.
Adding flag "is_aero_ioc" to differentiate aero based controllers from other gen35 controllers. Signed-off-by: Suganath Prabu --- drivers/scsi/mpt3sas/mpt3sas_base.h | 1 + drivers/scsi/mpt3sas/mpt3sas_scsih.c | 14 -- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.h b/drivers/scsi/mpt3sas/mpt3sas_base.h index 4b8b602..f200929 100644 --- a/drivers/scsi/mpt3sas/mpt3sas_base.h +++ b/drivers/scsi/mpt3sas/mpt3sas_base.h @@ -1399,6 +1399,7 @@ struct MPT3SAS_ADAPTER { void*device_remove_in_progress; u16 device_remove_in_progress_sz; u8 is_gen35_ioc; + u8 is_aero_ioc; PUT_SMID_IO_FP_HIP put_smid_scsi_io; }; diff --git a/drivers/scsi/mpt3sas/mpt3sas_scsih.c b/drivers/scsi/mpt3sas/mpt3sas_scsih.c index 5b9806d..039dee4 100644 --- a/drivers/scsi/mpt3sas/mpt3sas_scsih.c +++ b/drivers/scsi/mpt3sas/mpt3sas_scsih.c @@ -10366,10 +10366,6 @@ _scsih_probe(struct pci_dev *pdev, const struct pci_device_id *id) ioc->id = mpt3_ids++; sprintf(ioc->driver_name, "%s", MPT3SAS_DRIVER_NAME); switch (pdev->device) { - case MPI26_MFGPAGE_DEVID_CFG_SEC_3816: - case MPI26_MFGPAGE_DEVID_CFG_SEC_3916: - dev_info(>dev, - "HBA is in Configurable Secure mode\n"); case MPI26_MFGPAGE_DEVID_SAS3508: case MPI26_MFGPAGE_DEVID_SAS3508_1: case MPI26_MFGPAGE_DEVID_SAS3408: @@ -10377,12 +10373,18 @@ _scsih_probe(struct pci_dev *pdev, const struct pci_device_id *id) case MPI26_MFGPAGE_DEVID_SAS3516_1: case MPI26_MFGPAGE_DEVID_SAS3416: case MPI26_MFGPAGE_DEVID_SAS3616: + ioc->is_gen35_ioc = 1; + break; + case MPI26_MFGPAGE_DEVID_CFG_SEC_3816: + case MPI26_MFGPAGE_DEVID_CFG_SEC_3916: + dev_info(>dev, + "HBA is in Configurable Secure mode\n"); case MPI26_MFGPAGE_DEVID_HARD_SEC_3816: case MPI26_MFGPAGE_DEVID_HARD_SEC_3916: - ioc->is_gen35_ioc = 1; + ioc->is_aero_ioc = ioc->is_gen35_ioc = 1; break; default: - ioc->is_gen35_ioc = 0; + ioc->is_gen35_ioc = ioc->is_aero_ioc = 0; } if ((ioc->hba_mpi_version_belonged == MPI25_VERSION && pdev->revision >= SAS3_PCI_DEVICE_C0_REVISION) || -- 1.8.3.1
[PATCH 2/4] mpt3sas: Add separate function for aero doorbell reads.
Sometimes Aero controllers appears to be returning bad data (0) for doorbell register read and if retries are performed immediately after the bad read, they return good data. Workaround is added to retry read from doorbell registers for maximum three times if driver get the zero. Added functions base_readl_aero for Aero IOC and base_readl for gen35 and other controllers. Signed-off-by: Suganath Prabu --- drivers/scsi/mpt3sas/mpt3sas_base.c | 30 ++ drivers/scsi/mpt3sas/mpt3sas_base.h | 2 ++ 2 files changed, 32 insertions(+) diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.c b/drivers/scsi/mpt3sas/mpt3sas_base.c index 9254b52..d371c8e 100644 --- a/drivers/scsi/mpt3sas/mpt3sas_base.c +++ b/drivers/scsi/mpt3sas/mpt3sas_base.c @@ -157,6 +157,32 @@ module_param_call(mpt3sas_fwfault_debug, _scsih_set_fwfault_debug, param_get_int, _fwfault_debug, 0644); /** + * _base_readl_aero - retry readl for max three times. + * @addr - MPT Fusion system interface register address + * + * Retry the readl() for max three times if it gets zero value + * while reading the system interface register. + */ +static inline u32 +_base_readl_aero(const volatile void __iomem *addr) +{ + u32 i = 0, ret_val; + + do { + ret_val = readl(addr); + i++; + } while (ret_val == 0 && i < 3); + + return ret_val; +} + +static inline u32 +_base_readl(const volatile void __iomem *addr) +{ + return readl(addr); +} + +/** * _base_clone_reply_to_sys_mem - copies reply to reply free iomem * in BAR0 space. * @@ -6398,6 +6424,10 @@ mpt3sas_base_attach(struct MPT3SAS_ADAPTER *ioc) ioc->rdpq_array_enable_assigned = 0; ioc->dma_mask = 0; + if (ioc->is_aero_ioc) + ioc->base_readl = &_base_readl_aero; + else + ioc->base_readl = &_base_readl; r = mpt3sas_base_map_resources(ioc); if (r) goto out_free_resources; diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.h b/drivers/scsi/mpt3sas/mpt3sas_base.h index f200929..3a294b9 100644 --- a/drivers/scsi/mpt3sas/mpt3sas_base.h +++ b/drivers/scsi/mpt3sas/mpt3sas_base.h @@ -912,6 +912,7 @@ typedef void (*NVME_BUILD_PRP)(struct MPT3SAS_ADAPTER *ioc, u16 smid, typedef void (*PUT_SMID_IO_FP_HIP) (struct MPT3SAS_ADAPTER *ioc, u16 smid, u16 funcdep); typedef void (*PUT_SMID_DEFAULT) (struct MPT3SAS_ADAPTER *ioc, u16 smid); +typedef u32 (*BASE_READ_REG) (const volatile void __iomem *addr); /* IOC Facts and Port Facts converted from little endian to cpu */ union mpi3_version_union { @@ -1392,6 +1393,7 @@ struct MPT3SAS_ADAPTER { u8 hide_drives; spinlock_t diag_trigger_lock; u8 diag_trigger_active; + BASE_READ_REG base_readl; struct SL_WH_MASTER_TRIGGER_T diag_trigger_master; struct SL_WH_EVENT_TRIGGERS_T diag_trigger_event; struct SL_WH_SCSI_TRIGGERS_T diag_trigger_scsi; -- 1.8.3.1
[PATCH 0/4] mpt3sas: Fix hardware bug in aero controllers.
Problem statement: Sometimes aero controllers appears to be returning bad data (0) for doorbell register read and if retries are performed immediately after the bad read, they return good data. Fix: In below four patches added workaround to retry read operation from controller doorbell registers for maximum three times, if read returns zero. Suganath Prabu (4): mpt3sas: Introduce flag for aero based controllers. mpt3sas: Add separate function for aero doorbell reads. mpt3sas: Replace readl with ioc->base_readl. mpt3sas: Update driver version to 27.101.00.00. drivers/scsi/mpt3sas/mpt3sas_base.c | 69 ++-- drivers/scsi/mpt3sas/mpt3sas_base.h | 7 ++-- drivers/scsi/mpt3sas/mpt3sas_scsih.c | 14 3 files changed, 63 insertions(+), 27 deletions(-) -- 1.8.3.1
[PATCH] Revert "scsi: qla2xxx: Fix NVMe Target discovery"
This reverts commit db186382af21e926e90df19499475f2552192b77. This commit introduced regression with FCP discovery so revert it back to fix discovery for FCP luns Cc: Signed-off-by: Himanshu Madhani --- Hi Martin, This patch has introduced regression for LUN discovery with FC. Please apply this revert to 4.20/scsi-fixes branch at your earliest convenience. Thanks, Himanshu --- drivers/scsi/qla2xxx/qla_os.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/scsi/qla2xxx/qla_os.c b/drivers/scsi/qla2xxx/qla_os.c index b658b9a5eb1e..d0ecc729a90a 100644 --- a/drivers/scsi/qla2xxx/qla_os.c +++ b/drivers/scsi/qla2xxx/qla_os.c @@ -4886,10 +4886,10 @@ void qla24xx_create_new_sess(struct scsi_qla_host *vha, struct qla_work_evt *e) fcport->d_id = e->u.new_sess.id; fcport->flags |= FCF_FABRIC_DEVICE; fcport->fw_login_state = DSC_LS_PLOGI_PEND; - if (e->u.new_sess.fc4_type & FS_FC4TYPE_FCP) + if (e->u.new_sess.fc4_type == FS_FC4TYPE_FCP) fcport->fc4_type = FC4_TYPE_FCP_SCSI; - if (e->u.new_sess.fc4_type & FS_FC4TYPE_NVME) { + if (e->u.new_sess.fc4_type == FS_FC4TYPE_NVME) { fcport->fc4_type = FC4_TYPE_OTHER; fcport->fc4f_nvme = FC4_TYPE_NVME; } -- 2.12.0
Re: [PATCH 01/10] gdth: refactor ioc_general
On Thu, 6 Dec 2018, Christoph Hellwig wrote: > This function is a huge mess with duplicated error handling. Split out > a few useful helpers and use goto labels to untangle the error handling > and no-data ioctl handling. > > Signed-off-by: Christoph Hellwig > --- > drivers/scsi/gdth.c | 244 +++- > 1 file changed, 126 insertions(+), 118 deletions(-) > > diff --git a/drivers/scsi/gdth.c b/drivers/scsi/gdth.c > index 16709735b546..45e67d4cb3af 100644 > --- a/drivers/scsi/gdth.c > +++ b/drivers/scsi/gdth.c > @@ -4155,131 +4155,139 @@ static int ioc_resetdrv(void __user *arg, char > *cmnd) > return 0; > } > > -static int ioc_general(void __user *arg, char *cmnd) > +static void gdth_ioc_addr32(gdth_ha_str *ha, gdth_ioctl_general *gen, > + u64 paddr) > { > -gdth_ioctl_general gen; > -char *buf = NULL; > -u64 paddr; > -gdth_ha_str *ha; > -int rval; > + if (ha->cache_feat & SCATTER_GATHER) { > + gen->command.u.cache.DestAddr = 0x; > + gen->command.u.cache.sg_canz = 1; > + gen->command.u.cache.sg_lst[0].sg_ptr = (u32)paddr; > + gen->command.u.cache.sg_lst[0].sg_len = gen->data_len; > + gen->command.u.cache.sg_lst[1].sg_len = 0; > + } else { > + gen->command.u.cache.DestAddr = paddr; > + gen->command.u.cache.sg_canz = 0; > + } > +} > > -if (copy_from_user(, arg, sizeof(gdth_ioctl_general))) > -return -EFAULT; > -ha = gdth_find_ha(gen.ionode); > -if (!ha) > -return -EFAULT; > +static void gdth_ioc_addr64(gdth_ha_str *ha, gdth_ioctl_general *gen, > + u64 paddr) > +{ > + if (ha->cache_feat & SCATTER_GATHER) { > + gen->command.u.cache64.DestAddr = (u64)-1; > + gen->command.u.cache64.sg_canz = 1; > + gen->command.u.cache64.sg_lst[0].sg_ptr = paddr; > + gen->command.u.cache64.sg_lst[0].sg_len = gen->data_len; > + gen->command.u.cache64.sg_lst[1].sg_len = 0; > + } else { > + gen->command.u.cache64.DestAddr = paddr; > + gen->command.u.cache64.sg_canz = 0; > + } > +} > > -if (gen.data_len > INT_MAX) > -return -EINVAL; > -if (gen.sense_len > INT_MAX) > -return -EINVAL; > -if (gen.data_len + gen.sense_len > INT_MAX) > -return -EINVAL; > +static void gdth_ioc_cacheservice(gdth_ha_str *ha, gdth_ioctl_general *gen, > + u64 paddr) > +{ > + if (ha->cache_feat & GDT_64BIT) { > + /* copy elements from 32-bit IOCTL structure */ > + gen->command.u.cache64.BlockCnt = gen->command.u.cache.BlockCnt; > + gen->command.u.cache64.BlockNo = gen->command.u.cache.BlockNo; > + gen->command.u.cache64.DeviceNo = gen->command.u.cache.DeviceNo; > > -if (gen.data_len + gen.sense_len != 0) { > -if (!(buf = gdth_ioctl_alloc(ha, gen.data_len + gen.sense_len, > - FALSE, ))) > -return -EFAULT; > -if (copy_from_user(buf, arg + sizeof(gdth_ioctl_general), > - gen.data_len + gen.sense_len)) { > -gdth_ioctl_free(ha, gen.data_len+gen.sense_len, buf, paddr); > -return -EFAULT; > -} > + gdth_ioc_addr64(ha, gen, paddr); > + } else { > + gdth_ioc_addr32(ha, gen, paddr); > + } > +} > > -if (gen.command.OpCode == GDT_IOCTL) { > -gen.command.u.ioctl.p_param = paddr; > -} else if (gen.command.Service == CACHESERVICE) { > -if (ha->cache_feat & GDT_64BIT) { > -/* copy elements from 32-bit IOCTL structure */ > -gen.command.u.cache64.BlockCnt = > gen.command.u.cache.BlockCnt; > -gen.command.u.cache64.BlockNo = gen.command.u.cache.BlockNo; > -gen.command.u.cache64.DeviceNo = > gen.command.u.cache.DeviceNo; > -/* addresses */ > -if (ha->cache_feat & SCATTER_GATHER) { > -gen.command.u.cache64.DestAddr = (u64)-1; > -gen.command.u.cache64.sg_canz = 1; > -gen.command.u.cache64.sg_lst[0].sg_ptr = paddr; > -gen.command.u.cache64.sg_lst[0].sg_len = gen.data_len; > -gen.command.u.cache64.sg_lst[1].sg_len = 0; > -} else { > -gen.command.u.cache64.DestAddr = paddr; > -gen.command.u.cache64.sg_canz = 0; > -} > -} else { > -if (ha->cache_feat & SCATTER_GATHER) { > -gen.command.u.cache.DestAddr = 0x; > -gen.command.u.cache.sg_canz = 1; > -gen.command.u.cache.sg_lst[0].sg_ptr = (u32)paddr; > -gen.command.u.cache.sg_lst[0].sg_len = gen.data_len; > -
Re: 9305-16i fault 0x5853 (regression from 4.17.2 to 4.19.2) and EEH fence errors
Would be nice to be pointed to the correct place to report major regressions, if not here. Note that the same error occurs: * on 4.18.19 * on 4.19.8-rc1 * with the latest firmware (16.00.01) * on a number of other peoples' powerpc64/power9 hardware. Note that both 4.18.X and 4.19.Y will, in addition to the above error, also occasionally hit EEH fences (on 4.18 this appears to be the predominant error, on 4.19 this is less common). [ 1711.273181] EEH: Frozen PHB#31-PE#fd detected [ 1711.273235] EEH: PE location: N/A, PHB location: N/A [ 1711.273297] CPU: 13 PID: 10993 Comm: kworker/u128:4 Tainted: G W 4.18.0-3-powerpc64le #1 Debian 4.18.20-2 [ 1711.273307] Workqueue: poll_mpt3sas0_statu _base_fault_reset_work [mpt3sas] [ 1711.273310] Call Trace: [ 1711.273314] [c00fb5623a80] [c097690c] dump_stack+0xb0/0xf4 (unreliable) [ 1711.273318] [c00fb5623ac0] [c003cad0] eeh_dev_check_failure+0x5e0/0x600 [ 1711.273321] [c00fb5623b70] [c003cb7c] eeh_check_failure+0x8c/0xd0 [ 1711.273326] [c00fb5623bb0] [c00808b6935c] mpt3sas_base_get_iocstate+0x94/0xb0 [mpt3sas] [ 1711.273330] [c00fb5623bf0] [c00808b6da10] _base_fault_reset_work+0xd8/0x310 [mpt3sas] [ 1711.273334] [c00fb5623c80] [c0129040] process_one_work+0x260/0x530 [ 1711.273336] [c00fb5623d20] [c0129398] worker_thread+0x88/0x5d0 [ 1711.273340] [c00fb5623dc0] [c0132238] kthread+0x1a8/0x1b0 [ 1711.273344] [c00fb5623e30] [c000bdd4] ret_from_kernel_thread+0x5c/0x88 [ 1711.273347] mpt3sas_cm0: SAS host is non-operational [ 1711.273690] EEH: Detected PCI bus error on PHB#31-PE#fd [ 1711.273693] EEH: This PCI device has failed 1 times in the last hour and will be permanently disabled after 5 failures. [ 1711.273694] EEH: Notify device drivers to shutdown [ 1711.273697] EEH: Beginning: 'error_detected(IO frozen)' [ 1711.273699] EEH: PE#fd (PCI 0031:01:00.0): Invoking mpt3sas->error_detected(IO frozen) [ 1711.273701] mpt3sas_cm0: PCI error: detected callback, state(2)!! [ 1711.274838] EEH: PE#fd (PCI 0031:01:00.0): mpt3sas driver reports: 'need reset' [ 1711.274839] EEH: Finished:'error_detected(IO frozen)' with aggregate recovery state:'need reset' [ 1711.274842] EEH: Collect temporary log [ 1711.274861] EEH: of node=0031:01:00.0 [ 1711.274863] EEH: PCI device/vendor: 00c41000 [ 1711.274864] EEH: PCI cmd/status register: 00100142 [ 1711.274865] EEH: PCI-E capabilities and status follow: [ 1711.274872] EEH: PCI-E 00: 0002d010 10008025 2950 00415083 [ 1711.274878] EEH: PCI-E 10: 1083 [ 1711.274879] EEH: PCI-E 20: [ 1711.274879] EEH: PCI-E AER capability register set follows: [ 1711.274886] EEH: PCI-E AER 00: 1e020001 00462031 [ 1711.274892] EEH: PCI-E AER 10: 2000 01e0 [ 1711.274897] EEH: PCI-E AER 20: [ 1711.274899] EEH: PCI-E AER 30: 00010015 [ 1711.274900] PHB4 PHB#49 Diag-data (Version: 1) [ 1711.274901] brdgCtl:0002 [ 1711.274902] RootSts:00060040 00402000 a0830008 00100107 0800 [ 1711.274902] PhbSts: 001c 001c [ 1711.274903] Lem:00011000 1000 [ 1711.274904] PhbErr: 0880 0800 214898000240 a0084000 [ 1711.274905] RxeArbErr: 0008 0008 07170176 8a82018a [ 1711.274906] PblErr: 0002 0002 [ 1711.274907] RegbErr:00400040 0040 8804 [ 1711.274908] PE[0fd] A/B: 820080230100 8a82018a [ 1711.274909] PE[100] A/B: 80003bfe 300d2593 [ 1711.274910] PE[1fd] A/B: 8000 8000 [ 1711.274910] EEH: Reset without hotplug activity [ 1716.201556] EEH: Notify device drivers the completion of reset [ 1716.201559] EEH: Beginning: 'slot_reset' [ 1716.201568] EEH: PE#fd (PCI 0031:01:00.0): Invoking mpt3sas->slot_reset() [ 1716.201569] mpt3sas_cm0: PCI error: slot reset callback!! [ 1716.201649] mpt3sas 0031:01:00.0: Using 64-bit DMA iommu bypass [ 1716.201656] mpt3sas_cm0: 64 BIT PCI BUS DMA ADDRESSING SUPPORTED, total mem (131847972 kB) [ 1716.256003] mpt3sas_cm0: CurrentHostPageSize is 0: Setting default host page size to 4k [ 1716.256024] mpt3sas_cm0: MSI-X vectors supported: 96, no of cores: 64, max_msix_vectors: -1 [ 1716.258361] mpt3sas0-msix0: PCI-MSI-X enabled: IRQ 75 [ 1716.258363] mpt3sas0-msix1: PCI-MSI-X enabled: IRQ 76 [ 1716.258364] mpt3sas0-msix2: PCI-MSI-X enabled: IRQ 77 [ 1716.258365] mpt3sas0-msix3: PCI-MSI-X enabled: IRQ 78 [ 1716.258366] mpt3sas0-msix4: PCI-MSI-X enabled: IRQ 79 [ 1716.258367] mpt3sas0-msix5: PCI-MSI-X enabled: IRQ 89 [ 1716.258368] mpt3sas0-msix6: PCI-MSI-X enabled: IRQ 90 [ 1716.258368] mpt3sas0-msix7: PCI-MSI-X enabled: IRQ 91 [ 1716.258369]
SCSI reset loop with sym 53c895
Hey everybody, I'm the arch lead for Gentoo Linux on alpha (yes, the arch still exists). Starting with 4.20-rc*, I get SCSI reset loops shortly after boot: aboot: starting kernel boot/vmlinuz-4.20.0-rc3 with arguments ro root=/dev/sda2 console=ttyS0 loglevel=6 [0.00] Linux version 4.20.0-rc3+ (klausman@monolith) (gcc version 8.2.0 (Gentoo 8.2.0-r4 p1.5)) #49 SMP Sun Nov 25 20:23:08 CET 2018 [0.00] Booting on Tsunami variation Clipper using machine vector Clipper from SRM [0.00] Major Options: SMP EV67 LEGACY_START VERBOSE_MCHECK DISCONTIGMEM MAGIC_SYSRQ [0.00] Command line: ro root=/dev/sda2 console=ttyS0 loglevel=6 [0.00] Raw memory layout: [0.00] memcluster 0, usage 1, start0, end 256 [0.00] memcluster 1, usage 0, start 256, end 130985 [0.00] memcluster 2, usage 1, start 130985, end 131072 [0.00] memcluster 3, usage 0, start 131072, end 1048562 [0.00] memcluster 4, usage 1, start 1048562, end 1048576 [0.00] Initializing bootmem allocator on Node ID 0 [0.00] memcluster 1, usage 0, start 256, end 130985 [0.00] memcluster 3, usage 0, start 131072, end 1048562 [0.00] Detected node memory: start 256, end 1048562 [0.00] 8192K Bcache detected; load hit latency 21 cycles, load miss latency 175 cycles [0.00] Kernel command line: ro root=/dev/sda2 console=ttyS0 loglevel=6 [0.00] Sorting __ex_table... [0.00] random: get_random_u64 called from cache_random_seq_create+0x98/0x1b0 with crng_init=0 [0.00] random: get_random_u32 called from new_slab+0x210/0x700 with crng_init=0 [0.00] Turning on RTC interrupts. [0.069335] random: get_random_u32 called from bucket_table_alloc+0xbc/0x230 with crng_init=0 [0.069335] random: fast init done [0.218749] SCSI subsystem initialized [0.244140] pci 0001:01:03.0: Firmware left e100 interrupts enabled; disabling [0.247070] Initialise system trusted keyrings [0.272460] Key type asymmetric registered [0.273437] Asymmetric key parser 'x509' registered [0.441406] sym0: Symbios NVRAM, ID 7, Fast-40, LVD, parity checking [0.442382] sym0: open drain IRQ line driver, using on-chip SRAM [0.443359] sym0: using LOAD/STORE-based firmware. [0.444335] sym0: SCSI BUS has been reset. [0.445312] sym0:0: ERROR (81:0) (0-0-0) (0/7/0) @ (scripta 38:f31c0004). [0.445312] sym0: script cmd = e21c0004 [0.445312] sym0: regdump: [0.445312] ca [0.445312] 00 [0.445312] 00 [0.445312] 07 [0.445312] 47 [0.445312] 00 [0.445312] 00 [0.445312] 0f [0.445312] 00 [0.445312] 08 [0.445312] 00 [0.445312] 00 [0.445312] 80 [0.445312] 00 [0.445312] 08 [0.445312] 0a [0.445312] 07 [0.445312] ff [0.445312] ff [0.445312] ff [0.445312] 00 [0.445312] ff [0.445312] ff [0.445312] ff [0.445312] . (repeats here, forever) The machine is a Compaq ES40 (4-cpu, 8G of RAM) with two 53c895s, which have six SCA disks each. Some of these disks are quite old, over a decade in some cases, but have never had problems (and booting 4.19.5 works just fine). I haven't tested 4.20-rc5 yet, but I doubt the behaviour will be different. As for the rest, it's a pretty normal setup aside from the exotic arch, no fancy extra buses, no AGP or memory holes. I'll try to bisect this when the machine can be taken out of testing and image building, but that might not happen before Christmas or even the new year. Any ideas for something to try? Best, Tobias PS: Not subscribed to the list, so please CC me on replies, thanks!
[Bug 199435] HPSA + P420i resetting logical Direct-Access never complete
https://bugzilla.kernel.org/show_bug.cgi?id=199435 --- Comment #27 from Gaetan Trellu (gaetan.tre...@incloudus.com) --- By compiling the hpsa kernel module from SourceForge on Ubuntu 16.04 with kernel 4.4 solved the issue for us. Steps: # apt-get install dkms build-essential # tar xjvf hpsa-3.4.20-141.tar.bz2 # cd hpsa-3.4.20/drivers/ # sudo cp -a scsi /usr/src/hpsa-3.4.20.141 # dkms add -m hpsa -v 3.4.20.141 # dkms build -m hpsa -v 3.4.20.141 # dkms install -m hpsa -v 3.4.20.141 Link: https://sourceforge.net/projects/cciss/ -- You are receiving this mail because: You are the assignee for the bug.
Re: [PATCH 03/10] gdth: remove gdth_{alloc,free}_ioctl
On 06/12/2018 17:50, Johannes Thumshirn wrote: > Why not calling dma_alloc_coherent() directly instead of using the > pci_alloc_consistent() wrapper? Ah should've read the whole series -- Johannes ThumshirnSUSE Labs Filesystems jthumsh...@suse.de+49 911 74053 689 SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: Felix Imendörffer, Jane Smithard, Graham Norton HRB 21284 (AG Nürnberg) Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850
Re: [PATCH 03/10] gdth: remove gdth_{alloc,free}_ioctl
On 06/12/2018 16:57, Christoph Hellwig wrote: > Out of the three callers once insists on the scratch buffer, and the > others are fine with a new allocation. Switch those two to juse use > pci_alloc_consistent directly, and open code the scratch buffer > allocation in the remaining one. This avoids a case where we might > be doing a memory allocation under a spinlock with irqs disabled. Why not calling dma_alloc_coherent() directly instead of using the pci_alloc_consistent() wrapper? Johannes -- Johannes ThumshirnSUSE Labs Filesystems jthumsh...@suse.de+49 911 74053 689 SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: Felix Imendörffer, Jane Smithard, Graham Norton HRB 21284 (AG Nürnberg) Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850
[PATCH 2/2] zfcp: improve kdoc for return of zfcp_status_read_refill()
Complements v2.6.35 commit 64deb6efdc55 ("[SCSI] zfcp: Use status_read_buf_num provided by FCP channel") which replaced the hardcoded 16 with a variable value Also complements already existing fixups for above commit v2.6.35 commit 8d88cf3f3b9a ("[SCSI] zfcp: Update status read mempool") v3.10 commit 9edf7d75ee5f ("[SCSI] zfcp: status read buffers on first adapter open with link down") Signed-off-by: Steffen Maier Reviewed-by: Jens Remus --- drivers/s390/scsi/zfcp_aux.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/s390/scsi/zfcp_aux.c b/drivers/s390/scsi/zfcp_aux.c index 882789fff574..9cf30d124b9e 100644 --- a/drivers/s390/scsi/zfcp_aux.c +++ b/drivers/s390/scsi/zfcp_aux.c @@ -264,10 +264,10 @@ static void zfcp_free_low_mem_buffers(struct zfcp_adapter *adapter) * zfcp_status_read_refill - refill the long running status_read_requests * @adapter: ptr to struct zfcp_adapter for which the buffers should be refilled * - * Returns: 0 on success, 1 otherwise - * - * if there are 16 or more status_read requests missing an adapter_reopen - * is triggered + * Return: + * * 0 on success meaning at least one status read is pending + * * 1 if posting failed and not a single status read buffer is pending, + * also triggers adapter reopen recovery */ int zfcp_status_read_refill(struct zfcp_adapter *adapter) { -- 2.16.4
[PATCH 1/2] zfcp: fix posting too many status read buffers leading to adapter shutdown
Suppose adapter (open) recovery is between opened QDIO queues and before (the end of) initial posting of status read buffers (SRBs). This time window can be seconds long due to FSF_PROT_HOST_CONNECTION_INITIALIZING causing by design looping with exponential increase sleeps in the function performing exchange config data during recovery [zfcp_erp_adapter_strat_fsf_xconf()]. Recovery triggered by local link up. Suppose an event occurs for which the FCP channel would send an unsolicited notification to zfcp by means of a previously posted SRB. We saw it with local cable pull (link down) in multi-initiator zoning with multiple NPIV-enabled subchannels of the same shared FCP channel. As soon as zfcp_erp_adapter_strategy_open_fsf() starts posting the initial status read buffers from within the adapter's ERP thread, the channel does send an unsolicited notification. Since v2.6.27 commit d26ab06ede83 ("[SCSI] zfcp: receiving an unsolicted status can lead to I/O stall"), zfcp_fsf_status_read_handler() schedules adapter->stat_work to re-fill the just consumed SRB from a work item. Now the ERP thread and the work item post SRBs in parallel. Both contexts call the helper function zfcp_status_read_refill(). The tracking of missing (to be posted / re-filled) SRBs is not thread-safe due to separate atomic_read() and atomic_dec(), in order to depend on posting success. Hence, both contexts can see atomic_read(>stat_miss) == 1. One of the two contexts posts one too many SRB. Zfcp gets QDIO_ERROR_SLSB_STATE on the output queue (trace tag "qdireq1") leading to zfcp_erp_adapter_shutdown() in zfcp_qdio_handler_error(). An obvious and seemingly clean fix would be to schedule stat_work from the ERP thread and wait for it to finish. This would serialize all SRB re-fills. However, we already have another work item wait on the ERP thread: adapter->scan_work runs zfcp_fc_scan_ports() which calls zfcp_fc_eval_gpn_ft(). The latter calls zfcp_erp_wait() to wait for all the open port recoveries during zfcp auto port scan, but in fact it waits for any pending recovery including an adapter recovery. This approach leads to a deadlock. [see also v3.19 commit 18f87a67e6d6 ("zfcp: auto port scan resiliency"); v2.6.37 commit d3e1088d6873 ("[SCSI] zfcp: No ERP escalation on gpn_ft eval"); v2.6.28 commit fca55b6fb587 ("[SCSI] zfcp: fix deadlock between wq triggered port scan and ERP") fixing v2.6.27 commit c57a39a45a76 ("[SCSI] zfcp: wait until adapter is finished with ERP during auto-port"); v2.6.27 commit cc8c282963bd ("[SCSI] zfcp: Automatically attach remote ports")] Instead make the accounting of missing SRBs atomic for parallel execution in both the ERP thread and adapter->stat_work. Signed-off-by: Steffen Maier Fixes: d26ab06ede83 ("[SCSI] zfcp: receiving an unsolicted status can lead to I/O stall") Cc: #2.6.27+ Reviewed-by: Jens Remus --- drivers/s390/scsi/zfcp_aux.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/s390/scsi/zfcp_aux.c b/drivers/s390/scsi/zfcp_aux.c index df10f4e07a4a..882789fff574 100644 --- a/drivers/s390/scsi/zfcp_aux.c +++ b/drivers/s390/scsi/zfcp_aux.c @@ -271,16 +271,16 @@ static void zfcp_free_low_mem_buffers(struct zfcp_adapter *adapter) */ int zfcp_status_read_refill(struct zfcp_adapter *adapter) { - while (atomic_read(>stat_miss) > 0) + while (atomic_add_unless(>stat_miss, -1, 0)) if (zfcp_fsf_status_read(adapter->qdio)) { + atomic_inc(>stat_miss); /* undo add -1 */ if (atomic_read(>stat_miss) >= adapter->stat_read_buf_num) { zfcp_erp_adapter_reopen(adapter, 0, "axsref1"); return 1; } break; - } else - atomic_dec(>stat_miss); + } return 0; } -- 2.16.4
[PATCH 0/2] zfcp: small bugfix on top of previous v4.21 patches
James, Martin, One new recovery fix, which is not urgent, for an old bug. It's sufficient to apply it on top of the previously sent 23 zfcp updates for the v4.21 merge window [https://www.spinics.net/lists/linux-scsi/msg125211.html]. The 2 new patches apply to Martin's 4.21/scsi-queue and to James' misc branch. Steffen Maier (2): zfcp: fix posting too many status read buffers leading to adapter shutdown zfcp: improve kdoc for return of zfcp_status_read_refill() drivers/s390/scsi/zfcp_aux.c | 14 +++--- 1 file changed, 7 insertions(+), 7 deletions(-) -- 2.16.4
[PATCH 09/10] gdth: remove interrupt coalescing support
This code has been under a never defined ifdef since the beginning of time (or at least history), and has just bitrotted. Nuke it. Signed-off-by: Christoph Hellwig --- drivers/scsi/gdth.c | 151 1 file changed, 12 insertions(+), 139 deletions(-) diff --git a/drivers/scsi/gdth.c b/drivers/scsi/gdth.c index b8a033f18d7d..14f1d15cb6eb 100644 --- a/drivers/scsi/gdth.c +++ b/drivers/scsi/gdth.c @@ -89,10 +89,6 @@ * phase: unused */ - -/* interrupt coalescing */ -/* #define INT_COAL */ - /* statistics */ #define GDTH_STATISTICS @@ -192,9 +188,6 @@ static u8 DebugState = DEBUG_GDTH; #ifdef GDTH_STATISTICS static u32 max_rq=0, max_index=0, max_sg=0; -#ifdef INT_COAL -static u32 max_int_coal=0; -#endif static u32 act_ints=0, act_ios=0, act_stats=0, act_rq=0; static struct timer_list gdth_timer; #endif @@ -1189,9 +1182,6 @@ static int gdth_search_drives(gdth_ha_str *ha) gdth_arcdl_str *alst; gdth_alist_str *alst2; gdth_oem_str_ioctl *oemstr; -#ifdef INT_COAL -gdth_perf_modes *pmod; -#endif TRACE(("gdth_search_drives() hanum %d\n", ha->hanum)); ok = 0; @@ -1234,35 +1224,6 @@ static int gdth_search_drives(gdth_ha_str *ha) cdev_cnt = (u16)ha->info; ha->fw_vers = ha->service; -#ifdef INT_COAL -if (ha->type == GDT_PCIMPR) { -/* set perf. modes */ -pmod = (gdth_perf_modes *)ha->pscratch; -pmod->version = 1; -pmod->st_mode = 1;/* enable one status buffer */ -*((u64 *)>st_buff_addr1) = ha->coal_stat_phys; -pmod->st_buff_indx1= COALINDEX; -pmod->st_buff_addr2= 0; -pmod->st_buff_u_addr2 = 0; -pmod->st_buff_indx2= 0; -pmod->st_buff_size = sizeof(gdth_coal_status) * MAXOFFSETS; -pmod->cmd_mode = 0;// disable all cmd buffers -pmod->cmd_buff_addr1 = 0; -pmod->cmd_buff_u_addr1 = 0; -pmod->cmd_buff_indx1 = 0; -pmod->cmd_buff_addr2 = 0; -pmod->cmd_buff_u_addr2 = 0; -pmod->cmd_buff_indx2 = 0; -pmod->cmd_buff_size= 0; -pmod->reserved1= 0; -pmod->reserved2= 0; -if (gdth_internal_cmd(ha, CACHESERVICE, GDT_IOCTL, SET_PERF_MODES, - INVALID_CHANNEL,sizeof(gdth_perf_modes))) { -printk("GDT-HA %d: Interrupt coalescing activated\n", ha->hanum); -} -} -#endif - /* detect number of buses - try new IOCTL */ iocr = (gdth_raw_iochan_str *)ha->pscratch; iocr->hdr.version= 0x; @@ -2538,12 +2499,6 @@ static irqreturn_t __gdth_interrupt(gdth_ha_str *ha, u8 IStatus; u16 Service; unsigned long flags = 0; -#ifdef INT_COAL -int coalesced = FALSE; -int next = FALSE; -gdth_coal_status *pcs = NULL; -int act_int_coal = 0; -#endif TRACE(("gdth_interrupt() IRQ %d\n", ha->irq)); @@ -2570,24 +2525,6 @@ static irqreturn_t __gdth_interrupt(gdth_ha_str *ha, ++act_ints; #endif -#ifdef INT_COAL -/* See if the fw is returning coalesced status */ -if (IStatus == COALINDEX) { -/* Coalesced status. Setup the initial status - buffer pointer and flags */ -pcs = ha->coal_stat; -coalesced = TRUE; -next = TRUE; -} - -do { -if (coalesced) { -/* For coalesced requests all status - information is found in the status buffer */ -IStatus = (u8)(pcs->status & 0xff); -} -#endif - if (ha->type == GDT_PCI) { dp6_ptr = ha->brd; if (IStatus & 0x80) { /* error flag */ @@ -2620,28 +2557,15 @@ static irqreturn_t __gdth_interrupt(gdth_ha_str *ha, dp6m_ptr = ha->brd; if (IStatus & 0x80) { /* error flag */ IStatus &= ~0x80; -#ifdef INT_COAL -if (coalesced) -ha->status = pcs->ext_status & 0x; -else -#endif -ha->status = readw(_ptr->i960r.status); +ha->status = readw(_ptr->i960r.status); TRACE2(("gdth_interrupt() error %d/%d\n",IStatus,ha->status)); } else /* no error */ ha->status = S_OK; -#ifdef INT_COAL -/* get information */ -if (coalesced) { -ha->info = pcs->info0; -ha->info2 = pcs->info1; -ha->service = (pcs->ext_status >> 16) & 0x; -} else -#endif -{ -ha->info = readl(_ptr->i960r.info[0]); -ha->service = readw(_ptr->i960r.service); -ha->info2 = readl(_ptr->i960r.info[1]); -} + +ha->info = readl(_ptr->i960r.info[0]); +ha->service =
[PATCH 04/10] gdth: remove ISA and EISA support
The non-PCI code has bitrotted for quite a while and will just oops on load because it passes a NULL pointer to the PCI DMA routines. Lets kill it for good - if someone really wants to use one of these cards I'll help mentoring them to write a proper driver glue. Signed-off-by: Christoph Hellwig --- drivers/scsi/Kconfig | 2 +- drivers/scsi/gdth.c | 709 ++- drivers/scsi/gdth.h | 30 -- 3 files changed, 24 insertions(+), 717 deletions(-) diff --git a/drivers/scsi/Kconfig b/drivers/scsi/Kconfig index f07444d30b21..0cfa385625d8 100644 --- a/drivers/scsi/Kconfig +++ b/drivers/scsi/Kconfig @@ -676,7 +676,7 @@ config SCSI_DMX3191D config SCSI_GDTH tristate "Intel/ICP (former GDT SCSI Disk Array) RAID Controller support" - depends on (ISA || EISA || PCI) && SCSI && ISA_DMA_API + depends on PCI && SCSI ---help--- Formerly called GDT SCSI Disk Array Controller Support. diff --git a/drivers/scsi/gdth.c b/drivers/scsi/gdth.c index 7174e7a88da2..45ddecd0284c 100644 --- a/drivers/scsi/gdth.c +++ b/drivers/scsi/gdth.c @@ -1,6 +1,6 @@ / * Linux driver for * - * ICP vortex GmbH:GDT ISA/EISA/PCI Disk Array Controllers * + * ICP vortex GmbH:GDT PCI Disk Array Controllers * * Intel Corporation: Storage RAID Controllers * * * * gdth.c * @@ -32,15 +32,10 @@ / /* All GDT Disk Array Controllers are fully supported by this driver. - * This includes the PCI/EISA/ISA SCSI Disk Array Controllers and the + * This includes the PCI SCSI Disk Array Controllers and the * PCI Fibre Channel Disk Array Controllers. See gdth.h for a complete * list of all controller types. * - * If you have one or more GDT3000/3020 EISA controllers with - * controller BIOS disabled, you have to set the IRQ values with the - * command line option "gdth=irq1,irq2,...", where the irq1,irq2,... are - * the IRQ values for the EISA controllers. - * * After the optional list of IRQ values, other possible * command line options are: * disable:Ydisable driver @@ -61,14 +56,12 @@ * access a shared resource from several nodes, * appropriate controller firmware required * shared_access:N enable driver reserve/release protocol - * probe_eisa_isa:Y scan for EISA/ISA controllers - * probe_eisa_isa:N do not scan for EISA/ISA controllers * force_dma32:Yuse only 32 bit DMA mode * force_dma32:Nuse 64 bit DMA mode, if supported * * The default values are: "gdth=disable:N,reserve_mode:1,reverse_scan:N, * max_ids:127,rescan:N,hdr_channel:0, - * shared_access:Y,probe_eisa_isa:N,force_dma32:N". + * shared_access:Y,force_dma32:N". * Here is another example: "gdth=reserve_list:0,1,2,0,0,1,3,0,rescan:Y". * * When loading the gdth driver as a module, the same options are available. @@ -79,7 +72,7 @@ * * Default: "modprobe gdth disable=0 reserve_mode=1 reverse_scan=0 * max_ids=127 rescan=0 hdr_channel=0 shared_access=0 - * probe_eisa_isa=0 force_dma32=0" + * force_dma32=0" * The other example: "modprobe gdth reserve_list=0,1,2,0,0,1,3,0 rescan=1". */ @@ -286,12 +279,6 @@ static struct timer_list gdth_timer; #define BUS_L2P(a,b)((b)>(a)->virt_bus ? (b-1):(b)) -#ifdef CONFIG_ISA -static u8 gdth_drq_tab[4] = {5,6,7,7};/* DRQ table */ -#endif -#if defined(CONFIG_EISA) || defined(CONFIG_ISA) -static u8 gdth_irq_tab[6] = {0,10,11,12,14,0};/* IRQ table */ -#endif static u8 gdth_polling; /* polling if TRUE */ static int gdth_ctr_count = 0;/* controller count */ static LIST_HEAD(gdth_instances); /* controller list */ @@ -325,10 +312,6 @@ static u8 gdth_direction_tab[0x100] = { }; /* LILO and modprobe/insmod parameters */ -/* IRQ list for GDT3000/3020 EISA controllers */ -static int irq[MAXHA] __initdata = -{0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff, - 0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff}; /* disable driver flag */ static int disable __initdata = 0; /* reserve flag */ @@ -348,13 +331,10 @@ static int max_ids = MAXID; static int rescan = 0; /* shared access */ static int shared_access = 1; -/* enable support for EISA and ISA controllers */ -static int probe_eisa_isa = 0; /* 64 bit DMA mode, support for drives > 2 TB, if force_dma32 = 0 */ static int force_dma32 = 0; /* parameters for
[PATCH 03/10] gdth: remove gdth_{alloc,free}_ioctl
Out of the three callers once insists on the scratch buffer, and the others are fine with a new allocation. Switch those two to juse use pci_alloc_consistent directly, and open code the scratch buffer allocation in the remaining one. This avoids a case where we might be doing a memory allocation under a spinlock with irqs disabled. Signed-off-by: Christoph Hellwig --- drivers/scsi/gdth.c | 7 ++-- drivers/scsi/gdth_proc.c | 71 drivers/scsi/gdth_proc.h | 3 -- 3 files changed, 25 insertions(+), 56 deletions(-) diff --git a/drivers/scsi/gdth.c b/drivers/scsi/gdth.c index 45e67d4cb3af..7174e7a88da2 100644 --- a/drivers/scsi/gdth.c +++ b/drivers/scsi/gdth.c @@ -4232,7 +4232,7 @@ static int ioc_general(void __user *arg, char *cmnd) gdth_ioctl_general gen; gdth_ha_str *ha; char *buf = NULL; - u64 paddr; + dma_addr_t paddr; int rval; if (copy_from_user(, arg, sizeof(gdth_ioctl_general))) @@ -4251,7 +4251,8 @@ static int ioc_general(void __user *arg, char *cmnd) if (gen.data_len + gen.sense_len == 0) goto execute; - buf = gdth_ioctl_alloc(ha, gen.data_len + gen.sense_len, FALSE, ); +buf = pci_alloc_consistent(ha->pdev, gen.data_len + gen.sense_len, + ); if (!buf) return -EFAULT; @@ -4286,7 +4287,7 @@ static int ioc_general(void __user *arg, char *cmnd) rval = 0; out_free_buf: - gdth_ioctl_free(ha, gen.data_len+gen.sense_len, buf, paddr); + pci_free_consistent(ha->pdev, gen.data_len + gen.sense_len, buf, paddr); return rval; } diff --git a/drivers/scsi/gdth_proc.c b/drivers/scsi/gdth_proc.c index bd5532a80b0e..8e77f8fd8641 100644 --- a/drivers/scsi/gdth_proc.c +++ b/drivers/scsi/gdth_proc.c @@ -31,7 +31,6 @@ static int gdth_set_asc_info(struct Scsi_Host *host, char *buffer, int i, found; gdth_cmd_strgdtcmd; gdth_cpar_str *pcpar; -u64 paddr; charcmnd[MAX_COMMAND_SIZE]; memset(cmnd, 0xff, 12); @@ -113,13 +112,23 @@ static int gdth_set_asc_info(struct Scsi_Host *host, char *buffer, } if (wb_mode) { -if (!gdth_ioctl_alloc(ha, sizeof(gdth_cpar_str), TRUE, )) -return(-EBUSY); + unsigned long flags; + + BUILD_BUG_ON(sizeof(gdth_cpar_str) > GDTH_SCRATCH); + + spin_lock_irqsave(>smp_lock, flags); + if (ha->scratch_busy) { + spin_unlock_irqrestore(>smp_lock, flags); +return -EBUSY; + } + ha->scratch_busy = TRUE; + spin_unlock_irqrestore(>smp_lock, flags); + pcpar = (gdth_cpar_str *)ha->pscratch; memcpy( pcpar, >cpar, sizeof(gdth_cpar_str) ); gdtcmd.Service = CACHESERVICE; gdtcmd.OpCode = GDT_IOCTL; -gdtcmd.u.ioctl.p_param = paddr; +gdtcmd.u.ioctl.p_param = ha->scratch_phys; gdtcmd.u.ioctl.param_size = sizeof(gdth_cpar_str); gdtcmd.u.ioctl.subfunc = CACHE_CONFIG; gdtcmd.u.ioctl.channel = INVALID_CHANNEL; @@ -127,7 +136,10 @@ static int gdth_set_asc_info(struct Scsi_Host *host, char *buffer, gdth_execute(host, , cmnd, 30, NULL); -gdth_ioctl_free(ha, GDTH_SCRATCH, ha->pscratch, paddr); + spin_lock_irqsave(>smp_lock, flags); + ha->scratch_busy = FALSE; + spin_unlock_irqrestore(>smp_lock, flags); + printk("Done.\n"); return(orig_length); } @@ -143,7 +155,7 @@ int gdth_show_info(struct seq_file *m, struct Scsi_Host *host) int id, i, j, k, sec, flag; int no_mdrv = 0, drv_no, is_mirr; u32 cnt; -u64 paddr; +dma_addr_t paddr; int rc = -ENOMEM; gdth_cmd_str *gdtcmd; @@ -232,7 +244,7 @@ int gdth_show_info(struct seq_file *m, struct Scsi_Host *host) seq_puts(m, "\nPhysical Devices:"); flag = FALSE; -buf = gdth_ioctl_alloc(ha, size, FALSE, ); +buf = pci_alloc_consistent(ha->pdev, size, ); if (!buf) goto stop_output; for (i = 0; i < ha->bus_cnt; ++i) { @@ -406,7 +418,7 @@ int gdth_show_info(struct seq_file *m, struct Scsi_Host *host) seq_printf(m, " To Array Drv.:\t%s\n", hrec); } - + if (!flag) seq_puts(m, "\n --\n"); @@ -500,7 +512,7 @@ int gdth_show_info(struct seq_file *m, struct Scsi_Host *host) } } } -gdth_ioctl_free(ha, size, buf, paddr); + pci_free_consistent(ha->pdev, size, buf, paddr); for (i = 0; i < MAX_HDRIVES; ++i) { if (!(ha->hdr[i].present)) @@ -553,47 +565,6 @@ int gdth_show_info(struct seq_file *m, struct Scsi_Host *host) return rc; } -static char *gdth_ioctl_alloc(gdth_ha_str *ha, int size, int scratch, - u64 *paddr) -{ -unsigned long flags; -char *ret_val; - -if (size ==
[PATCH 02/10] gdth: reuse dma coherent allocation in gdth_show_info
gdth_show_info currently allocs and frees a dma buffer four times, which isn't very efficient. Reuse a single allocation instead. Signed-off-by: Christoph Hellwig --- drivers/scsi/gdth_proc.c | 20 +--- 1 file changed, 5 insertions(+), 15 deletions(-) diff --git a/drivers/scsi/gdth_proc.c b/drivers/scsi/gdth_proc.c index 3a9751a80225..bd5532a80b0e 100644 --- a/drivers/scsi/gdth_proc.c +++ b/drivers/scsi/gdth_proc.c @@ -226,11 +226,13 @@ int gdth_show_info(struct seq_file *m, struct Scsi_Host *host) #endif if (ha->more_proc) { +size_t size = max_t(size_t, GDTH_SCRATCH, sizeof(gdth_hget_str)); + /* more information: 2. about physical devices */ seq_puts(m, "\nPhysical Devices:"); flag = FALSE; -buf = gdth_ioctl_alloc(ha, GDTH_SCRATCH, FALSE, ); +buf = gdth_ioctl_alloc(ha, size, FALSE, ); if (!buf) goto stop_output; for (i = 0; i < ha->bus_cnt; ++i) { @@ -323,7 +325,6 @@ int gdth_show_info(struct seq_file *m, struct Scsi_Host *host) } } } -gdth_ioctl_free(ha, GDTH_SCRATCH, buf, paddr); if (!flag) seq_puts(m, "\n --\n"); @@ -332,9 +333,6 @@ int gdth_show_info(struct seq_file *m, struct Scsi_Host *host) seq_puts(m, "\nLogical Drives:"); flag = FALSE; -buf = gdth_ioctl_alloc(ha, GDTH_SCRATCH, FALSE, ); -if (!buf) -goto stop_output; for (i = 0; i < MAX_LDRIVES; ++i) { if (!ha->hdr[i].is_logdrv) continue; @@ -408,7 +406,6 @@ int gdth_show_info(struct seq_file *m, struct Scsi_Host *host) seq_printf(m, " To Array Drv.:\t%s\n", hrec); } -gdth_ioctl_free(ha, GDTH_SCRATCH, buf, paddr); if (!flag) seq_puts(m, "\n --\n"); @@ -417,9 +414,6 @@ int gdth_show_info(struct seq_file *m, struct Scsi_Host *host) seq_puts(m, "\nArray Drives:"); flag = FALSE; -buf = gdth_ioctl_alloc(ha, GDTH_SCRATCH, FALSE, ); -if (!buf) -goto stop_output; for (i = 0; i < MAX_LDRIVES; ++i) { if (!(ha->hdr[i].is_arraydrv && ha->hdr[i].is_master)) continue; @@ -468,8 +462,7 @@ int gdth_show_info(struct seq_file *m, struct Scsi_Host *host) hrec); } } -gdth_ioctl_free(ha, GDTH_SCRATCH, buf, paddr); - + if (!flag) seq_puts(m, "\n --\n"); @@ -477,9 +470,6 @@ int gdth_show_info(struct seq_file *m, struct Scsi_Host *host) seq_puts(m, "\nHost Drives:"); flag = FALSE; -buf = gdth_ioctl_alloc(ha, sizeof(gdth_hget_str), FALSE, ); -if (!buf) -goto stop_output; for (i = 0; i < MAX_LDRIVES; ++i) { if (!ha->hdr[i].is_logdrv || (ha->hdr[i].is_arraydrv && !ha->hdr[i].is_master)) @@ -510,7 +500,7 @@ int gdth_show_info(struct seq_file *m, struct Scsi_Host *host) } } } -gdth_ioctl_free(ha, sizeof(gdth_hget_str), buf, paddr); +gdth_ioctl_free(ha, size, buf, paddr); for (i = 0; i < MAX_HDRIVES; ++i) { if (!(ha->hdr[i].present)) -- 2.19.1
[PATCH 01/10] gdth: refactor ioc_general
This function is a huge mess with duplicated error handling. Split out a few useful helpers and use goto labels to untangle the error handling and no-data ioctl handling. Signed-off-by: Christoph Hellwig --- drivers/scsi/gdth.c | 244 +++- 1 file changed, 126 insertions(+), 118 deletions(-) diff --git a/drivers/scsi/gdth.c b/drivers/scsi/gdth.c index 16709735b546..45e67d4cb3af 100644 --- a/drivers/scsi/gdth.c +++ b/drivers/scsi/gdth.c @@ -4155,131 +4155,139 @@ static int ioc_resetdrv(void __user *arg, char *cmnd) return 0; } -static int ioc_general(void __user *arg, char *cmnd) +static void gdth_ioc_addr32(gdth_ha_str *ha, gdth_ioctl_general *gen, + u64 paddr) { -gdth_ioctl_general gen; -char *buf = NULL; -u64 paddr; -gdth_ha_str *ha; -int rval; + if (ha->cache_feat & SCATTER_GATHER) { + gen->command.u.cache.DestAddr = 0x; + gen->command.u.cache.sg_canz = 1; + gen->command.u.cache.sg_lst[0].sg_ptr = (u32)paddr; + gen->command.u.cache.sg_lst[0].sg_len = gen->data_len; + gen->command.u.cache.sg_lst[1].sg_len = 0; + } else { + gen->command.u.cache.DestAddr = paddr; + gen->command.u.cache.sg_canz = 0; + } +} -if (copy_from_user(, arg, sizeof(gdth_ioctl_general))) -return -EFAULT; -ha = gdth_find_ha(gen.ionode); -if (!ha) -return -EFAULT; +static void gdth_ioc_addr64(gdth_ha_str *ha, gdth_ioctl_general *gen, + u64 paddr) +{ + if (ha->cache_feat & SCATTER_GATHER) { + gen->command.u.cache64.DestAddr = (u64)-1; + gen->command.u.cache64.sg_canz = 1; + gen->command.u.cache64.sg_lst[0].sg_ptr = paddr; + gen->command.u.cache64.sg_lst[0].sg_len = gen->data_len; + gen->command.u.cache64.sg_lst[1].sg_len = 0; + } else { + gen->command.u.cache64.DestAddr = paddr; + gen->command.u.cache64.sg_canz = 0; + } +} -if (gen.data_len > INT_MAX) -return -EINVAL; -if (gen.sense_len > INT_MAX) -return -EINVAL; -if (gen.data_len + gen.sense_len > INT_MAX) -return -EINVAL; +static void gdth_ioc_cacheservice(gdth_ha_str *ha, gdth_ioctl_general *gen, + u64 paddr) +{ + if (ha->cache_feat & GDT_64BIT) { + /* copy elements from 32-bit IOCTL structure */ + gen->command.u.cache64.BlockCnt = gen->command.u.cache.BlockCnt; + gen->command.u.cache64.BlockNo = gen->command.u.cache.BlockNo; + gen->command.u.cache64.DeviceNo = gen->command.u.cache.DeviceNo; -if (gen.data_len + gen.sense_len != 0) { -if (!(buf = gdth_ioctl_alloc(ha, gen.data_len + gen.sense_len, - FALSE, ))) -return -EFAULT; -if (copy_from_user(buf, arg + sizeof(gdth_ioctl_general), - gen.data_len + gen.sense_len)) { -gdth_ioctl_free(ha, gen.data_len+gen.sense_len, buf, paddr); -return -EFAULT; -} + gdth_ioc_addr64(ha, gen, paddr); + } else { + gdth_ioc_addr32(ha, gen, paddr); + } +} -if (gen.command.OpCode == GDT_IOCTL) { -gen.command.u.ioctl.p_param = paddr; -} else if (gen.command.Service == CACHESERVICE) { -if (ha->cache_feat & GDT_64BIT) { -/* copy elements from 32-bit IOCTL structure */ -gen.command.u.cache64.BlockCnt = gen.command.u.cache.BlockCnt; -gen.command.u.cache64.BlockNo = gen.command.u.cache.BlockNo; -gen.command.u.cache64.DeviceNo = gen.command.u.cache.DeviceNo; -/* addresses */ -if (ha->cache_feat & SCATTER_GATHER) { -gen.command.u.cache64.DestAddr = (u64)-1; -gen.command.u.cache64.sg_canz = 1; -gen.command.u.cache64.sg_lst[0].sg_ptr = paddr; -gen.command.u.cache64.sg_lst[0].sg_len = gen.data_len; -gen.command.u.cache64.sg_lst[1].sg_len = 0; -} else { -gen.command.u.cache64.DestAddr = paddr; -gen.command.u.cache64.sg_canz = 0; -} -} else { -if (ha->cache_feat & SCATTER_GATHER) { -gen.command.u.cache.DestAddr = 0x; -gen.command.u.cache.sg_canz = 1; -gen.command.u.cache.sg_lst[0].sg_ptr = (u32)paddr; -gen.command.u.cache.sg_lst[0].sg_len = gen.data_len; -gen.command.u.cache.sg_lst[1].sg_len = 0; -} else { -gen.command.u.cache.DestAddr = paddr; -gen.command.u.cache.sg_canz = 0; -} -} -} else if
[PATCH 10/10] gdth: use generic DMA API
Switch from the legacy PCI DMA API to the generic DMA API. Also switch to dma_map_single from pci_map_page in one case where this makes the code simpler. Signed-off-by: Christoph Hellwig --- drivers/scsi/gdth.c | 59 +++- drivers/scsi/gdth_proc.c | 4 +-- 2 files changed, 30 insertions(+), 33 deletions(-) diff --git a/drivers/scsi/gdth.c b/drivers/scsi/gdth.c index 14f1d15cb6eb..e1071fc2249b 100644 --- a/drivers/scsi/gdth.c +++ b/drivers/scsi/gdth.c @@ -2077,9 +2077,9 @@ static int gdth_fill_cache_cmd(gdth_ha_str *ha, struct scsi_cmnd *scp, if (scsi_bufflen(scp)) { cmndinfo->dma_dir = (read_write == 1 ? -PCI_DMA_TODEVICE : PCI_DMA_FROMDEVICE); -sgcnt = pci_map_sg(ha->pdev, scsi_sglist(scp), scsi_sg_count(scp), - cmndinfo->dma_dir); +DMA_TO_DEVICE : DMA_FROM_DEVICE); +sgcnt = dma_map_sg(>pdev->dev, scsi_sglist(scp), + scsi_sg_count(scp), cmndinfo->dma_dir); if (mode64) { struct scatterlist *sl; @@ -2153,8 +2153,6 @@ static int gdth_fill_raw_cmd(gdth_ha_str *ha, struct scsi_cmnd *scp, u8 b) dma_addr_t sense_paddr; int cmd_index, sgcnt, mode64; u8 t,l; -struct page *page; -unsigned long offset; struct gdth_cmndinfo *cmndinfo; t = scp->device->id; @@ -2196,10 +2194,8 @@ static int gdth_fill_raw_cmd(gdth_ha_str *ha, struct scsi_cmnd *scp, u8 b) } } else { -page = virt_to_page(scp->sense_buffer); -offset = (unsigned long)scp->sense_buffer & ~PAGE_MASK; -sense_paddr = pci_map_page(ha->pdev,page,offset, - 16,PCI_DMA_FROMDEVICE); +sense_paddr = dma_map_single(>pdev->dev, scp->sense_buffer, 16, +DMA_FROM_DEVICE); cmndinfo->sense_paddr = sense_paddr; cmdp->OpCode = GDT_WRITE; /* always */ @@ -2240,9 +2236,9 @@ static int gdth_fill_raw_cmd(gdth_ha_str *ha, struct scsi_cmnd *scp, u8 b) } if (scsi_bufflen(scp)) { -cmndinfo->dma_dir = PCI_DMA_BIDIRECTIONAL; -sgcnt = pci_map_sg(ha->pdev, scsi_sglist(scp), scsi_sg_count(scp), - cmndinfo->dma_dir); +cmndinfo->dma_dir = DMA_BIDIRECTIONAL; +sgcnt = dma_map_sg(>pdev->dev, scsi_sglist(scp), + scsi_sg_count(scp), cmndinfo->dma_dir); if (mode64) { struct scatterlist *sl; @@ -2750,12 +2746,12 @@ static int gdth_sync_event(gdth_ha_str *ha, int service, u8 index, return 2; } if (scsi_bufflen(scp)) -pci_unmap_sg(ha->pdev, scsi_sglist(scp), scsi_sg_count(scp), +dma_unmap_sg(>pdev->dev, scsi_sglist(scp), scsi_sg_count(scp), cmndinfo->dma_dir); if (cmndinfo->sense_paddr) -pci_unmap_page(ha->pdev, cmndinfo->sense_paddr, 16, - PCI_DMA_FROMDEVICE); +dma_unmap_page(>pdev->dev, cmndinfo->sense_paddr, 16, + DMA_FROM_DEVICE); if (ha->status == S_OK) { cmndinfo->status = S_OK; @@ -3659,8 +3655,8 @@ static int ioc_general(void __user *arg, char *cmnd) if (gen.data_len + gen.sense_len == 0) goto execute; -buf = pci_alloc_consistent(ha->pdev, gen.data_len + gen.sense_len, - ); +buf = dma_alloc_coherent(>pdev->dev, gen.data_len + gen.sense_len, + , GFP_KERNEL); if (!buf) return -EFAULT; @@ -3695,7 +3691,8 @@ static int ioc_general(void __user *arg, char *cmnd) rval = 0; out_free_buf: - pci_free_consistent(ha->pdev, gen.data_len + gen.sense_len, buf, paddr); + dma_free_coherent(>pdev->dev, gen.data_len + gen.sense_len, buf, + paddr); return rval; } @@ -4140,14 +4137,14 @@ static int gdth_pci_probe_one(gdth_pci_str *pcistr, gdth_ha_str **ha_out) error = -ENOMEM; - ha->pscratch = pci_alloc_consistent(ha->pdev, GDTH_SCRATCH, - _dma_handle); + ha->pscratch = dma_alloc_coherent(>pdev->dev, GDTH_SCRATCH, + _dma_handle, GFP_KERNEL); if (!ha->pscratch) goto out_free_irq; ha->scratch_phys = scratch_dma_handle; - ha->pmsg = pci_alloc_consistent(ha->pdev, sizeof(gdth_msg_str), - _dma_handle); + ha->pmsg = dma_alloc_coherent(>pdev->dev, sizeof(gdth_msg_str), + _dma_handle, GFP_KERNEL); if (!ha->pmsg) goto out_free_pscratch; ha->msg_phys = scratch_dma_handle; @@ -4174,16 +4171,16 @@ static int
[PATCH 07/10] gdth: remove dead dma statistics code
This code can't be built into the kernel without editing the source file and is not generall useful. Signed-off-by: Christoph Hellwig --- drivers/scsi/gdth.c | 18 -- drivers/scsi/gdth_proc.c | 8 2 files changed, 26 deletions(-) diff --git a/drivers/scsi/gdth.c b/drivers/scsi/gdth.c index 63d704301875..b8a033f18d7d 100644 --- a/drivers/scsi/gdth.c +++ b/drivers/scsi/gdth.c @@ -2126,12 +2126,6 @@ static int gdth_fill_cache_cmd(gdth_ha_str *ha, struct scsi_cmnd *scp, cmdp->u.cache64.sg_canz = sgcnt; scsi_for_each_sg(scp, sl, sgcnt, i) { cmdp->u.cache64.sg_lst[i].sg_ptr = sg_dma_address(sl); -#ifdef GDTH_DMA_STATISTICS -if (cmdp->u.cache64.sg_lst[i].sg_ptr > (u64)0x) -ha->dma64_cnt++; -else -ha->dma32_cnt++; -#endif cmdp->u.cache64.sg_lst[i].sg_len = sg_dma_len(sl); } } else { @@ -2141,9 +2135,6 @@ static int gdth_fill_cache_cmd(gdth_ha_str *ha, struct scsi_cmnd *scp, cmdp->u.cache.sg_canz = sgcnt; scsi_for_each_sg(scp, sl, sgcnt, i) { cmdp->u.cache.sg_lst[i].sg_ptr = sg_dma_address(sl); -#ifdef GDTH_DMA_STATISTICS -ha->dma32_cnt++; -#endif cmdp->u.cache.sg_lst[i].sg_len = sg_dma_len(sl); } } @@ -2298,12 +2289,6 @@ static int gdth_fill_raw_cmd(gdth_ha_str *ha, struct scsi_cmnd *scp, u8 b) cmdp->u.raw64.sg_ranz = sgcnt; scsi_for_each_sg(scp, sl, sgcnt, i) { cmdp->u.raw64.sg_lst[i].sg_ptr = sg_dma_address(sl); -#ifdef GDTH_DMA_STATISTICS -if (cmdp->u.raw64.sg_lst[i].sg_ptr > (u64)0x) -ha->dma64_cnt++; -else -ha->dma32_cnt++; -#endif cmdp->u.raw64.sg_lst[i].sg_len = sg_dma_len(sl); } } else { @@ -2313,9 +2298,6 @@ static int gdth_fill_raw_cmd(gdth_ha_str *ha, struct scsi_cmnd *scp, u8 b) cmdp->u.raw.sg_ranz = sgcnt; scsi_for_each_sg(scp, sl, sgcnt, i) { cmdp->u.raw.sg_lst[i].sg_ptr = sg_dma_address(sl); -#ifdef GDTH_DMA_STATISTICS -ha->dma32_cnt++; -#endif cmdp->u.raw.sg_lst[i].sg_len = sg_dma_len(sl); } } diff --git a/drivers/scsi/gdth_proc.c b/drivers/scsi/gdth_proc.c index 8e77f8fd8641..fc36c49a5334 100644 --- a/drivers/scsi/gdth_proc.c +++ b/drivers/scsi/gdth_proc.c @@ -229,14 +229,6 @@ int gdth_show_info(struct seq_file *m, struct Scsi_Host *host) " Serial No.: \t0x%8X\tCache RAM size:\t%d KB\n", ha->binfo.ser_no, ha->binfo.memsize / 1024); -#ifdef GDTH_DMA_STATISTICS -/* controller statistics */ -seq_puts(m, "\nController Statistics:\n"); -seq_printf(m, - " 32-bit DMA buffer:\t%lu\t64-bit DMA buffer:\t%lu\n", - ha->dma32_cnt, ha->dma64_cnt); -#endif - if (ha->more_proc) { size_t size = max_t(size_t, GDTH_SCRATCH, sizeof(gdth_hget_str)); -- 2.19.1
[PATCH 08/10] gdth: remove dead code under #ifdef GDTH_IOCTL_PROC
This can't ever be compiled into the kernel, so remove it. Signed-off-by: Christoph Hellwig --- drivers/scsi/gdth_ioctl.h | 89 --- drivers/scsi/gdth_proc.c | 18 2 files changed, 107 deletions(-) diff --git a/drivers/scsi/gdth_ioctl.h b/drivers/scsi/gdth_ioctl.h index 4c91894ac244..ee4c9bf1022a 100644 --- a/drivers/scsi/gdth_ioctl.h +++ b/drivers/scsi/gdth_ioctl.h @@ -27,11 +27,7 @@ #define GDTH_MAXSG 32 /* max. s/g elements */ #define MAX_LDRIVES 255 /* max. log. drive count */ -#ifdef GDTH_IOCTL_PROC -#define MAX_HDRIVES 100 /* max. host drive count */ -#else #define MAX_HDRIVES MAX_LDRIVES /* max. host drive count */ -#endif /* scatter/gather element */ typedef struct { @@ -178,91 +174,6 @@ typedef struct { gdth_evt_data event_data; } __attribute__((packed)) gdth_evt_str; - -#ifdef GDTH_IOCTL_PROC -/* IOCTL structure (write) */ -typedef struct { -u32 magic; /* IOCTL magic */ -u16 ioctl; /* IOCTL */ -u16 ionode; /* controller number */ -u16 service;/* controller service */ -u16 timeout;/* timeout */ -union { -struct { -u8 command[512]; /* controller command */ -u8 data[1];/* add. data */ -} general; -struct { -u8 lock; /* lock/unlock */ -u8 drive_cnt; /* drive count */ -u16 drives[MAX_HDRIVES];/* drives */ -} lockdrv; -struct { -u8 lock; /* lock/unlock */ -u8 channel;/* channel */ -} lockchn; -struct { -int erase; /* erase event ? */ -int handle; -u8 evt[EVENT_SIZE];/* event structure */ -} event; -struct { -u8 bus;/* SCSI bus */ -u8 target; /* target ID */ -u8 lun;/* LUN */ -u8 cmd_len;/* command length */ -u8 cmd[12];/* SCSI command */ -} scsi; -struct { -u16 hdr_no; /* host drive number */ -u8 flag; /* old meth./add/remove */ -} rescan; -} iu; -} gdth_iowr_str; - -/* IOCTL structure (read) */ -typedef struct { -u32 size; /* buffer size */ -u32 status; /* IOCTL error code */ -union { -struct { -u8 data[1];/* data */ -} general; -struct { -u16 version;/* driver version */ -} drvers; -struct { -u8 type; /* controller type */ -u16 info; /* slot etc. */ -u16 oem_id; /* OEM ID */ -u16 bios_ver; /* not used */ -u16 access; /* not used */ -u16 ext_type; /* extended type */ -u16 device_id; /* device ID */ -u16 sub_device_id; /* sub device ID */ -} ctrtype; -struct { -u8 version;/* OS version */ -u8 subversion; /* OS subversion */ -u16 revision; /* revision */ -} osvers; -struct { -u16 count; /* controller count */ -} ctrcnt; -struct { -int handle; -u8 evt[EVENT_SIZE];/* event structure */ -} event; -struct { -u8 bus;/* SCSI bus, 0xff: invalid */ -u8 target; /* target ID */ -u8 lun;/* LUN */ -u8 cluster_type; /* cluster properties */ -} hdr_list[MAX_HDRIVES];/* index is host drive number */ -} iu; -} gdth_iord_str; -#endif - /* GDTIOCTL_GENERAL */ typedef struct { u16 ionode; /* controller number */ diff --git a/drivers/scsi/gdth_proc.c b/drivers/scsi/gdth_proc.c index fc36c49a5334..5a13ccac8dee 100644 --- a/drivers/scsi/gdth_proc.c +++ b/drivers/scsi/gdth_proc.c @@ -557,24 +557,6 @@ int gdth_show_info(struct seq_file *m, struct Scsi_Host *host) return rc; } -#ifdef GDTH_IOCTL_PROC -static int gdth_ioctl_check_bin(gdth_ha_str *ha, u16 size) -{ -unsigned long flags; -int ret_val; - -
[PATCH 05/10] gdth: remove direct serial port access
Remove never compile in support for sending debug traces straight to the serial port using direct port access. Signed-off-by: Christoph Hellwig --- drivers/scsi/gdth.c | 70 - 1 file changed, 70 deletions(-) diff --git a/drivers/scsi/gdth.c b/drivers/scsi/gdth.c index 45ddecd0284c..e8121b80233c 100644 --- a/drivers/scsi/gdth.c +++ b/drivers/scsi/gdth.c @@ -185,79 +185,9 @@ static void gdth_scsi_done(struct scsi_cmnd *scp); #ifdef DEBUG_GDTH static u8 DebugState = DEBUG_GDTH; - -#ifdef __SERIAL__ -#define MAX_SERBUF 160 -static void ser_init(void); -static void ser_puts(char *str); -static void ser_putc(char c); -static int ser_printk(const char *fmt, ...); -static char strbuf[MAX_SERBUF+1]; -#ifdef __COM2__ -#define COM_BASE 0x2f8 -#else -#define COM_BASE 0x3f8 -#endif -static void ser_init() -{ -unsigned port=COM_BASE; - -outb(0x80,port+3); -outb(0,port+1); -/* 19200 Baud, if 9600: outb(12,port) */ -outb(6, port); -outb(3,port+3); -outb(0,port+1); -/* -ser_putc('I'); -ser_putc(' '); -*/ -} - -static void ser_puts(char *str) -{ -char *ptr; - -ser_init(); -for (ptr=str;*ptr;++ptr) -ser_putc(*ptr); -} - -static void ser_putc(char c) -{ -unsigned port=COM_BASE; - -while ((inb(port+5) & 0x20)==0); -outb(c,port); -if (c==0x0a) -{ -while ((inb(port+5) & 0x20)==0); -outb(0x0d,port); -} -} - -static int ser_printk(const char *fmt, ...) -{ -va_list args; -int i; - -va_start(args,fmt); -i = vsprintf(strbuf,fmt,args); -ser_puts(strbuf); -va_end(args); -return i; -} - -#define TRACE(a){if (DebugState==1) {ser_printk a;}} -#define TRACE2(a) {if (DebugState==1 || DebugState==2) {ser_printk a;}} -#define TRACE3(a) {if (DebugState!=0) {ser_printk a;}} - -#else /* !__SERIAL__ */ #define TRACE(a){if (DebugState==1) {printk a;}} #define TRACE2(a) {if (DebugState==1 || DebugState==2) {printk a;}} #define TRACE3(a) {if (DebugState!=0) {printk a;}} -#endif - #else /* !DEBUG */ #define TRACE(a) #define TRACE2(a) -- 2.19.1
[PATCH 06/10] gdth: remove dead rtc code
This code has been under the never defined GDTH_RTC ifdef forever, nuke it. Signed-off-by: Christoph Hellwig --- drivers/scsi/gdth.c | 32 1 file changed, 32 deletions(-) diff --git a/drivers/scsi/gdth.c b/drivers/scsi/gdth.c index e8121b80233c..63d704301875 100644 --- a/drivers/scsi/gdth.c +++ b/drivers/scsi/gdth.c @@ -115,10 +115,6 @@ #include #include #include - -#ifdef GDTH_RTC -#include -#endif #include #include @@ -1197,11 +1193,6 @@ static int gdth_search_drives(gdth_ha_str *ha) gdth_perf_modes *pmod; #endif -#ifdef GDTH_RTC -u8 rtc[12]; -unsigned long flags; -#endif - TRACE(("gdth_search_drives() hanum %d\n", ha->hanum)); ok = 0; @@ -1221,29 +1212,6 @@ static int gdth_search_drives(gdth_ha_str *ha) } TRACE2(("gdth_search_drives(): SCREENSERVICE initialized\n")); -#ifdef GDTH_RTC -/* read realtime clock info, send to controller */ -/* 1. wait for the falling edge of update flag */ -spin_lock_irqsave(_lock, flags); -for (j = 0; j < 100; ++j) -if (CMOS_READ(RTC_FREQ_SELECT) & RTC_UIP) -break; -for (j = 0; j < 100; ++j) -if (!(CMOS_READ(RTC_FREQ_SELECT) & RTC_UIP)) -break; -/* 2. read info */ -do { -for (j = 0; j < 12; ++j) -rtc[j] = CMOS_READ(j); -} while (rtc[0] != CMOS_READ(0)); -spin_unlock_irqrestore(_lock, flags); -TRACE2(("gdth_search_drives(): RTC: %x/%x/%x\n",*(u32 *)[0], -*(u32 *)[4], *(u32 *)[8])); -/* 3. send to controller firmware */ -gdth_internal_cmd(ha, SCREENSERVICE, GDT_REALTIME, *(u32 *)[0], - *(u32 *)[4], *(u32 *)[8]); -#endif - /* unfreeze all IOs */ gdth_internal_cmd(ha, CACHESERVICE, GDT_UNFREEZE_IO, 0, 0, 0); -- 2.19.1
various fixups for gdth
Cleans up various oddities found during a code audit, and drops the legacy ISA support which hasn't had a chance to actually work for a long time.
Re: [PATCH v10 4/6] scsi: ufs: Add core reset support
On 23/10/2018 06:35, Can Guo wrote: > From: Dov Levenglick > > Enables core reset support. Add full initialization of the PHY and the > controller before initializing UFS PHY and during link recovery. > > Signed-off-by: Dov Levenglick > Signed-off-by: Amit Nischal > Signed-off-by: Subhash Jadavani > Signed-off-by: Can Guo FWIW, this patch does not apply cleanly on top of v4.20-rc4 because it clashes with df032bf27a414acf61c957ec2fad22a57d903b39 ("scsi: ufs: Add a bsg endpoint that supports UPIUs"). Seems git's 3-way merge is smart enough to handle the conflict. Regards.
Re: [PATCH v7 3/5] target: add device vendor_id configfs attribute
David, > Indeed, the comment should refer to page 0x83. > @Martin: all patches in this series have now been reviewed+acked. Can > you fix the above comment (s/0x80/0x83) if/when you merge, or >should I resend the series with this fixed? I'll fix it up. -- Martin K. Petersen Oracle Linux Engineering
Re: [PATCH v7 3/5] target: add device vendor_id configfs attribute
On Thu, 6 Dec 2018 15:25:42 +0300, Roman Bolshakov wrote: > > /* > > + * STANDARD and VPD page 0x80 T10 Vendor Identification > > Perhaps you meant 0x83 (Device Identification VPD page, T10 vendor ID > based designator). INQUIRY page 0x80 is Unit Serial Number. Indeed, the comment should refer to page 0x83. @Martin: all patches in this series have now been reviewed+acked. Can you fix the above comment (s/0x80/0x83) if/when you merge, or should I resend the series with this fixed? Cheers, David
Re: [PATCH v7 2/5] target: consistently null-terminate t10_wwn strings
On Wed, Dec 05, 2018 at 01:18:35PM +0100, David Disseldorp wrote: > In preparation for supporting user provided vendor strings, add an extra > byte to the vendor, model and revision arrays in struct t10_wwn. This > ensures that the full INQUIRY data can be carried in the arrays along > with a null-terminator. > > Change a number of array readers and writers so that they account for > explicit null-termination: > - The pscsi_set_inquiry_info() and emulate_model_alias_store() codepaths > don't currently explicitly null-terminate; fix this. > - Existing t10_wwn field dumps use for-loops which step over > null-terminators for right-padding. > + Use printf with width specifiers instead. > > Signed-off-by: David Disseldorp > --- > drivers/target/target_core_configfs.c | 16 +++ > drivers/target/target_core_device.c | 46 ++-- > drivers/target/target_core_pscsi.c| 50 > +++ > drivers/target/target_core_spc.c | 7 ++--- > drivers/target/target_core_stat.c | 32 +- > include/target/target_core_base.h | 14 +++--- > 6 files changed, 63 insertions(+), 102 deletions(-) > Reviewed-by: Roman Bolshakov Thank you, Roman Bolshakov
Re: [PATCH v7 3/5] target: add device vendor_id configfs attribute
On Wed, Dec 05, 2018 at 01:18:36PM +0100, David Disseldorp wrote: > The vendor_id attribute will allow for the modification of the T10 > Vendor Identification string returned in inquiry responses. Its value > can be viewed and modified via the ConfigFS path at: > target/core/$backstore/$name/wwn/vendor_id > > "LIO-ORG" remains the default value, which is set when the backstore > device is enabled. > > Signed-off-by: David Disseldorp > --- > drivers/target/target_core_configfs.c | 70 > +++ > 1 file changed, 70 insertions(+) > > diff --git a/drivers/target/target_core_configfs.c > b/drivers/target/target_core_configfs.c > index 8277bcf81d6e..f099c2ae451f 100644 > --- a/drivers/target/target_core_configfs.c > +++ b/drivers/target/target_core_configfs.c > @@ -1217,6 +1217,74 @@ static struct t10_wwn *to_t10_wwn(struct config_item > *item) > } > > /* > + * STANDARD and VPD page 0x80 T10 Vendor Identification Perhaps you meant 0x83 (Device Identification VPD page, T10 vendor ID based designator). INQUIRY page 0x80 is Unit Serial Number. Besides that, Reviewed-by: Roman Bolshakov Thank you, Roman
Re: [PATCH] scsi: t10-pi: Return correct ref tag when queue has no integrity profile
Hi, 在 2018/12/6 20:04, John Garry 写道: On 06/12/2018 04:17, Martin K. Petersen wrote: + Bart, Had you considered to use lower_32_bits() instead of "0x"? That would to avoid that reviewers have to count the 'f'-s to verify correctness of t10_pi_ref_tag(). I hadn't. I guess I tend to think of lower_32_bits() as something you do to pointers, not to block numbers. Xiang Chen tested the patch successfully. I'll let him provide tested-by tag. I have tested the patch with running fio on 3008 (on our ARM64 board with 4 DIF disks and 8 non-DIF disks), and now it works well. Tested-by: Xiang Chen Thanks, John .
Re: [PATCH v10 0/6] Support for Qualcomm UFS QMP PHY on SDM845
On 05/12/2018 08:01, Vivek Gautam wrote: > On Tue, Oct 23, 2018 at 10:07 AM Can Guo wrote: >> >> This patch series adds support for UFS QMP PHY on SDM845 and the >> compatible string for it. This patch series depends on the current >> proposed QMP V3 USB3 UNI PHY support for sdm845 driver [1], on >> the DT bindings for the QMP V3 USB3 PHYs based dirver [2], and also >> rebased on updated pipe_clk initialization sequence [3]. This series >> can only be merged once the dependent patches do. >> [1] >> http://lists-archives.com/linux-kernel/29071659-dt-bindings-phy-qcom-qmp-update-bindings-for-sdm845.html >> [2] >> http://lists-archives.com/linux-kernel/29071660-phy-qcom-qmp-add-qmp-v3-usb3-uni-phy-support-for-sdm845.html >> [3] https://patchwork.kernel.org/patch/10376551/ > > Besides my comment for PATCH 4/6, I have already reviewed the entire series, > and it looks good. > If adding new bindings for sdm845 needs a further review, can you separate out > just the phy patches from this series (patch 1, 2, 3 & 6), and re-send them. > We can ask Kishon if he can pull them in for this merge window. > Thanks. I'm confused. I was trying to 'git am' this series on top of v4.20 but it has been partly merged AFAICT. commit 0d58280cf1e61b06cb4d4aab672efccdc28794f6 Author: Can Guo AuthorDate: Thu Sep 20 21:27:54 2018 -0700 Commit: Kishon Vijay Abraham I CommitDate: Tue Sep 25 16:10:13 2018 +0530 commit 6b04526812ac41ba82317caa8df3549dda2cab97 Author: Can Guo AuthorDate: Thu Sep 20 21:27:55 2018 -0700 Commit: Kishon Vijay Abraham I CommitDate: Tue Sep 25 16:10:14 2018 +0530 commit cc31cdbef9b7166fe42e08267349cfbaa32696b6 Author: Can Guo AuthorDate: Thu Sep 20 21:27:56 2018 -0700 Commit: Kishon Vijay Abraham I CommitDate: Tue Sep 25 16:10:14 2018 +0530 Can't find [PATCH v10 4/6] scsi: ufs: Add core reset support [PATCH v10 5/6] scsi: ufs: Power on phy after it is initialized commit 99c7c7364b714e1de54a25c3642d991de1675e27 Author: Can Guo AuthorDate: Tue Sep 25 12:10:12 2018 +0530 Commit: Kishon Vijay Abraham I CommitDate: Tue Sep 25 16:10:14 2018 +0530 So basically, only patches 4 and 5 have not landed yet? (Are they perhaps in someone's -next tree?) Regards.
Re: [PATCH] scsi: t10-pi: Return correct ref tag when queue has no integrity profile
On 06/12/2018 04:17, Martin K. Petersen wrote: + Bart, Had you considered to use lower_32_bits() instead of "0x"? That would to avoid that reviewers have to count the 'f'-s to verify correctness of t10_pi_ref_tag(). I hadn't. I guess I tend to think of lower_32_bits() as something you do to pointers, not to block numbers. Xiang Chen tested the patch successfully. I'll let him provide tested-by tag. Thanks, John
Re: [PATCH RFC] sr: mark the device as changed when burning a CD
Dne 6.12.2018 v 11:34 Maurizio Lombardi napsal(a): > This is what I see when a cd burn operation completes: > This is the complete blktrace log: 11,034 0.81759 11653 D W 63488 (2a 00 00 03 3c 29 00 00 1f 00 ..) [wodim] 11,034 0.81759 11653 D W 63488 (2a 00 00 03 3c 29 00 00 1f 00 ..) [wodim] 11,034 0.81759 11653 D W 63488 (2a 00 00 03 3c 29 00 00 1f 00 ..) [wodim] 11,034 0.81759 11653 D W 63488 (2a 00 00 03 3c 29 00 00 1f 00 ..) [wodim] 11,038 0.049744147 11653 D W 63488 (2a 00 00 03 3c 48 00 00 1f 00 ..) [wodim] 11,038 0.049744147 11653 D W 63488 (2a 00 00 03 3c 48 00 00 1f 00 ..) [wodim] 11,038 0.049744147 11653 D W 63488 (2a 00 00 03 3c 48 00 00 1f 00 ..) [wodim] 11,038 0.049744147 11653 D W 63488 (2a 00 00 03 3c 48 00 00 1f 00 ..) [wodim] 11,03 12 0.102512363 11653 D W 63488 (2a 00 00 03 3c 67 00 00 1f 00 ..) [wodim] 11,03 12 0.102512363 11653 D W 63488 (2a 00 00 03 3c 67 00 00 1f 00 ..) [wodim] 11,03 12 0.102512363 11653 D W 63488 (2a 00 00 03 3c 67 00 00 1f 00 ..) [wodim] 11,03 12 0.102512363 11653 D W 63488 (2a 00 00 03 3c 67 00 00 1f 00 ..) [wodim] 11,03 16 0.152219178 11653 D W 63488 (2a 00 00 03 3c 86 00 00 1f 00 ..) [wodim] 11,03 16 0.152219178 11653 D W 63488 (2a 00 00 03 3c 86 00 00 1f 00 ..) [wodim] 11,03 16 0.152219178 11653 D W 63488 (2a 00 00 03 3c 86 00 00 1f 00 ..) [wodim] 11,03 16 0.152219178 11653 D W 63488 (2a 00 00 03 3c 86 00 00 1f 00 ..) [wodim] 11,03 20 0.205093330 11653 D W 63488 (2a 00 00 03 3c a5 00 00 1f 00 ..) [wodim] 11,03 20 0.205093330 11653 D W 63488 (2a 00 00 03 3c a5 00 00 1f 00 ..) [wodim] 11,03 20 0.205093330 11653 D W 63488 (2a 00 00 03 3c a5 00 00 1f 00 ..) [wodim] 11,03 20 0.205093330 11653 D W 63488 (2a 00 00 03 3c a5 00 00 1f 00 ..) [wodim] 11,03 24 0.254684616 11653 D W 63488 (2a 00 00 03 3c c4 00 00 1f 00 ..) [wodim] 11,03 24 0.254684616 11653 D W 63488 (2a 00 00 03 3c c4 00 00 1f 00 ..) [wodim] 11,03 24 0.254684616 11653 D W 63488 (2a 00 00 03 3c c4 00 00 1f 00 ..) [wodim] 11,03 24 0.254684616 11653 D W 63488 (2a 00 00 03 3c c4 00 00 1f 00 ..) [wodim] 11,03 28 0.307478799 11653 D W 63488 (2a 00 00 03 3c e3 00 00 1f 00 ..) [wodim] 11,03 28 0.307478799 11653 D W 63488 (2a 00 00 03 3c e3 00 00 1f 00 ..) [wodim] 11,03 28 0.307478799 11653 D W 63488 (2a 00 00 03 3c e3 00 00 1f 00 ..) [wodim] 11,03 28 0.307478799 11653 D W 63488 (2a 00 00 03 3c e3 00 00 1f 00 ..) [wodim] 11,03 32 0.357166787 11653 D W 63488 (2a 00 00 03 3d 02 00 00 1f 00 ..) [wodim] 11,03 32 0.357166787 11653 D W 63488 (2a 00 00 03 3d 02 00 00 1f 00 ..) [wodim] 11,03 32 0.357166787 11653 D W 63488 (2a 00 00 03 3d 02 00 00 1f 00 ..) [wodim] 11,03 32 0.357166787 11653 D W 63488 (2a 00 00 03 3d 02 00 00 1f 00 ..) [wodim] 11,03 36 0.409940696 11653 D W 63488 (2a 00 00 03 3d 21 00 00 1f 00 ..) [wodim] 11,03 36 0.409940696 11653 D W 63488 (2a 00 00 03 3d 21 00 00 1f 00 ..) [wodim] 11,03 36 0.409940696 11653 D W 63488 (2a 00 00 03 3d 21 00 00 1f 00 ..) [wodim] 11,03 36 0.409940696 11653 D W 63488 (2a 00 00 03 3d 21 00 00 1f 00 ..) [wodim] 11,03 40 0.459603865 11653 D W 63488 (2a 00 00 03 3d 40 00 00 1f 00 ..) [wodim] 11,03 40 0.459603865 11653 D W 63488 (2a 00 00 03 3d 40 00 00 1f 00 ..) [wodim] 11,03 40 0.459603865 11653 D W 63488 (2a 00 00 03 3d 40 00 00 1f 00 ..) [wodim] 11,03 40 0.459603865 11653 D W 63488 (2a 00 00 03 3d 40 00 00 1f 00 ..) [wodim] 11,03 44 0.512511601 11653 D W 63488 (2a 00 00 03 3d 5f 00 00 1f 00 ..) [wodim] 11,03 44 0.512511601 11653 D W 63488 (2a 00 00 03 3d 5f 00 00 1f 00 ..) [wodim] 11,03 44 0.512511601 11653 D W 63488 (2a 00 00 03 3d 5f 00 00 1f 00 ..) [wodim] 11,03 44 0.512511601 11653 D W 63488 (2a 00 00 03 3d 5f 00 00 1f 00 ..) [wodim] 11,03 48 0.562033003 11653 D W 63488 (2a 00 00 03 3d 7e 00 00 1f 00 ..) [wodim] 11,03 48 0.562033003 11653 D W 63488 (2a 00 00 03 3d 7e 00 00 1f 00 ..) [wodim] 11,03 48 0.562033003 11653 D W 63488 (2a 00 00 03 3d 7e 00 00 1f 00 ..) [wodim] 11,03 48 0.562033003 11653 D W 63488 (2a 00 00 03 3d 7e 00 00 1f 00 ..) [wodim] 11,03 52 0.614956620 11653 D W 63488 (2a 00 00 03 3d 9d 00 00 1f 00 ..)
Re: [PATCH RFC] sr: mark the device as changed when burning a CD
Hi Jens, Dne 20.6.2018 v 16:09 Jens Axboe napsal(a): > On 6/20/18 5:52 AM, Maurizio Lombardi wrote: >> Hi Jens, >> >> Dne 23.5.2018 v 16:42 Jens Axboe napsal(a): >>> On 5/23/18 3:19 AM, Maurizio Lombardi wrote: Dne 22.5.2018 v 16:47 Jens Axboe napsal(a): > It's been many years, but back in the day the program writing the cd > would eject the disc once done. This of course forces a reload of > the toc and clearing of the flag. What program is this? Seems like > it should probably eject when it's done. They are using wodim to burn the CDs on their servers. The problem is that they do not want the CD to be ejected because their drives lack a motorized tray, thus requiring manual intervention which they would like to avoid. >>> >>> I took a quick look at it, man that sr driver needs a bit of love :-) >>> >>> Anyway, I wonder if something like the below would work. Check for >>> a close track command in the sr completion handler, and flag the media >>> as changed if we see one. Totally untested... >>> >>> >>> diff --git a/drivers/scsi/sr.c b/drivers/scsi/sr.c >>> index 3f3cb72e0c0c..48f0d7a096db 100644 >>> --- a/drivers/scsi/sr.c >>> +++ b/drivers/scsi/sr.c >>> @@ -328,6 +328,9 @@ static int sr_done(struct scsi_cmnd *SCpnt) >>> scmd_printk(KERN_INFO, SCpnt, "done: %x\n", result); >>> #endif >>> >>> + if (SCpnt->cmnd[0] == GPCMD_CLOSE_TRACK) >>> + cd->device->changed = 1; >>> + >>> /* >>> * Handle MEDIUM ERRORs or VOLUME OVERFLOWs that indicate partial >>> * success. Since this is a relatively rare error condition, no >>> >> >> I just want to let you know that I tested the patch but unfortunately it >> doesn't work. >> I will try to find out what GPCMD_* commands are passed to sr_done() when >> burning a disc >> to see if there is another one which we could use. > > It's been a decade since I last messed with this or burned a cd-r, so that > would be appreciated. blktrace should be enough to tell you what commands > are being sent. > You remember this patch? The problem was that after a cd burn operation completes, you can't mount the cd unless you eject it first. I finally carried out the test you suggested by using blktrace. This is what I see when a cd burn operation completes: 11,03 32 0.357166787 11653 D W 63488 (2a 00 00 03 3d 02 00 00 1f 00 ..) [wodim] 11,03 32 0.357166787 11653 D W 63488 (2a 00 00 03 3d 02 00 00 1f 00 ..) [wodim] 11,03 32 0.357166787 11653 D W 63488 (2a 00 00 03 3d 02 00 00 1f 00 ..) [wodim] 11,03 32 0.357166787 11653 D W 63488 (2a 00 00 03 3d 02 00 00 1f 00 ..) [wodim] 11,03 80 0.922358386 11653 D W 63488 (2a 00 00 03 3e 57 00 00 1f 00 ..) [wodim] 11,03 80 0.922358386 11653 D W 63488 (2a 00 00 03 3e 57 00 00 1f 00 ..) [wodim] 11,03 80 0.922358386 11653 D W 63488 (2a 00 00 03 3e 57 00 00 1f 00 ..) [wodim] 11,03 80 0.922358386 11653 D W 63488 (2a 00 00 03 3e 57 00 00 1f 00 ..) [wodim] 11,03 112 1.332351325 11653 D W 63488 (2a 00 00 03 3f 4f 00 00 1f 00 ..) [wodim] 11,03 112 1.332351325 11653 D W 63488 (2a 00 00 03 3f 4f 00 00 1f 00 ..) [wodim] 11,03 112 1.332351325 11653 D W 63488 (2a 00 00 03 3f 4f 00 00 1f 00 ..) [wodim] 11,03 112 1.332351325 11653 D W 63488 (2a 00 00 03 3f 4f 00 00 1f 00 ..) [wodim] 11,03 152 1.791786352 11653 D W 63488 (2a 00 00 03 40 66 00 00 1f 00 ..) [wodim] 11,03 152 1.791786352 11653 D W 63488 (2a 00 00 03 40 66 00 00 1f 00 ..) [wodim] 11,03 152 1.791786352 11653 D W 63488 (2a 00 00 03 40 66 00 00 1f 00 ..) [wodim] 11,03 152 1.791786352 11653 D W 63488 (2a 00 00 03 40 66 00 00 1f 00 ..) [wodim] 11,03 196 2.357046291 11653 D W 63488 (2a 00 00 03 41 bb 00 00 1f 00 ..) [wodim] 11,03 196 2.357046291 11653 D W 63488 (2a 00 00 03 41 bb 00 00 1f 00 ..) [wodim] 11,03 196 2.357046291 11653 D W 63488 (2a 00 00 03 41 bb 00 00 1f 00 ..) [wodim] 11,03 196 2.357046291 11653 D W 63488 (2a 00 00 03 41 bb 00 00 1f 00 ..) [wodim] 11,03 304 3.600032959 11653 D N 0 (35 00 ..) [wodim] 11,03 304 3.600032959 11653 D N 0 (35 00 ..) [wodim] 11,03 304 3.600032959 11653 D N 0 (35 00 ..) [wodim] 11,03 304 3.600032959 11653 D N 0 (35 00 ..) [wodim] 11,03 33218.496219927 11653 D N 0 (35 00 ..) [wodim] 11,03 33218.496219927 11653 D N 0 (35 00 ..) [wodim] 11,03 33218.496219927 11653 D N 0 (35 00 ..) [wodim] 11,03 33218.496219927 11653 D N 0 (35 00 ..) [wodim] 11,03 35218.499114653 7433 D N 0 (00 ..) [kworker/3:2] 11,03 35218.499114653 7433 D N 0 (00 ..)
Re: [PATCH] scsi: t10-pi: Return correct ref tag when queue has no integrity profile
Bart, > Had you considered to use lower_32_bits() instead of "0x"? > That would to avoid that reviewers have to count the 'f'-s to verify > correctness of t10_pi_ref_tag(). I hadn't. I guess I tend to think of lower_32_bits() as something you do to pointers, not to block numbers. -- Martin K. Petersen Oracle Linux Engineering
BUG in copy_page_to_iter() when iscsi sets ENABLE_CLUSTERING
I recently found what I believe is a bug, and I'd appreciate feedback on if that is correct, and if so how to proceed. BACKGROUND Recently Christoph Hellwig sent an email to driver maintainers for drivers that set ".use_clustering" to DISABLE_CLUSTERING in their SCSI Host templates, asking if the setting could be changed to ENABLE_CLUSTERING. As part of answering that question, I set ENABLE_CLUSTERING in drivers/scsi/iscsi_tcp.c and tested both the iscsi initiator and target. As a reminder, setting ENABLE_CLUSTERING means that adjacent bio-s can be merged. This can make IO faster, but it means that drivers must be able to deal with IOs that cross page boundaries, since bio merges can create such IOs. RESULTS The iscsi initiator code can handle ENABLE_CLUSTERING just fine, but the iscsi target code fails. It seems to assume that IOs do *NOT* cross a page boundary. The problem is in iscsi lib/iov_iter.c, in the functions copy_page_to_iter() and page_copy_sane() (see below for how to reproduce): >> static inline bool page_copy_sane(struct page *page, size_t offset, size_t n) >> { >> struct page *head = compound_head(page); >> size_t v = n + offset + page_address(page) - page_address(head); >> >> if (likely(n <= v && v <= (PAGE_SIZE << compound_order(head >> return true; >> WARN_ON(1); >> return false; >> } >> >> size_t copy_page_to_iter(struct page *page, size_t offset, size_t bytes, >> struct iov_iter *i) >> { >> if (unlikely(!page_copy_sane(page, offset, bytes))) >> return 0; >> if (i->type & (ITER_BVEC|ITER_KVEC)) { >> void *kaddr = kmap_atomic(page); >> size_t wanted = copy_to_iter(kaddr + offset, bytes, i); >> kunmap_atomic(kaddr); >> return wanted; >> } else if (unlikely(iov_iter_is_discard(i))) >> return bytes; >> else if (likely(!iov_iter_is_pipe(i))) >> return copy_page_to_iter_iovec(page, offset, bytes, i); >> else >> return copy_page_to_iter_pipe(page, offset, bytes, i); >> } Causing the following WARN_ON stack trace (repeatedly): >> ... >> [ 78.644559] WARNING: CPU: 0 PID: 2192 at lib/iov_iter.c:830 >> copy_page_to_iter+0x1a6/0x2e0 >> [ 78.644561] Modules linked in: iscsi_tcp(E) libiscsi_tcp(E) libiscsi(E) >> scsi_transport_iscsi(E) rfcomm(E) iscsi_target_mod(E) target_core_pscsi(E) >> target_core_file(E) target_core_iblock(E) target_core_user(E) uio(E) >> target_core_mod(E) configfs(E) af_packet(E) iscsi_ibft(E) >> iscsi_boot_sysfs(E) vmw_vsock_vmci_transport(E) vsock(E) bnep(E) fuse(E) >> crct10dif_pclmul(E) crc32_pclmul(E) ghash_clmulni_intel(E) xfs(E) >> aesni_intel(E) snd_ens1371(E) aes_x86_64(E) snd_ac97_codec(E) crypto_simd(E) >> cryptd(E) ac97_bus(E) glue_helper(E) snd_rawmidi(E) vmw_balloon(E) >> snd_seq_device(E) snd_pcm(E) pcspkr(E) snd_timer(E) snd(E) uvcvideo(E) >> btusb(E) videobuf2_vmalloc(E) btrtl(E) videobuf2_memops(E) videobuf2_v4l2(E) >> btbcm(E) btintel(E) videodev(E) bluetooth(E) videobuf2_common(E) vmw_vmci(E) >> ecdh_generic(E) rfkill(E) soundcore(E) mptctl(E) gameport(E) joydev(E) >> i2c_piix4(E) e1000(E) ac(E) button(E) btrfs(E) libcrc32c(E) xor(E) >> raid6_pq(E) hid_generic(E) usbhid(E) sr_mod(E) cdrom(E) ata_generic(E) >> [ 78.644583] crc32c_intel(E) serio_raw(E) mptspi(E) scsi_transport_spi(E) >> mptscsih(E) ata_piix(E) uhci_hcd(E) ehci_pci(E) ehci_hcd(E) vmwgfx(E) >> drm_kms_helper(E) syscopyarea(E) sysfillrect(E) sysimgblt(E) fb_sys_fops(E) >> usbcore(E) ttm(E) mptbase(E) drm(E) sg(E) dm_multipath(E) dm_mod(E) >> scsi_dh_rdac(E) scsi_dh_emc(E) scsi_dh_alua(E) >> [ 78.644593] CPU: 0 PID: 2192 Comm: iscsi_trx Tainted: GE >> 4.20.0-rc4-1-default+ #1 openSUSE Tumbleweed (unreleased) >> [ 78.644593] Hardware name: VMware, Inc. VMware Virtual Platform/440BX >> Desktop Reference Platform, BIOS 6.00 05/19/2017 >> [ 78.644595] RIP: 0010:copy_page_to_iter+0x1a6/0x2e0 >> [ 78.644596] Code: 0c 24 48 98 49 89 ce 48 89 c2 49 29 c6 48 29 ca 4d 01 >> f4 48 01 d5 48 85 c0 0f 85 71 ff ff ff 48 85 ed 0f 84 68 ff ff ff eb b2 <0f> >> 0b 45 31 ed eb 8d bf 01 00 00 00 e8 d9 1e c5 ff 65 4c 8b 34 25 >> [ 78.644596] RSP: 0018:a04b41eefbe0 EFLAGS: 00010206 >> [ 78.644597] RAX: f7acc40de180 RBX: a04b41eefd80 RCX: >> 0028a014 >> [ 78.644598] RDX: 2000 RSI: 1000 RDI: >> f7acc000 >> [ 78.644598] RBP: f7acc40de180 R08: 8f1703786000 R09: >> 2000 >> [ 78.644599] R10: 02c0 R11: 8f16b3f31400 R12: >> >> [ 78.644600] R13: 2000 R14: 0008 R15: >> 3800 >> [ 78.644601] FS: () GS:8f1739c0() >> knlGS: >> [ 78.644601] CS: 0010 DS: ES: CR0: 80050033 >> [ 78.644602] CR2:
Re: DISABLE_CLUSTERING in scsi drivers
On 11/21/18 1:41 AM, Christoph Hellwig wrote: > Hi all, > > you in the To list maintain or wrote SCSI drivers that set the > DISABLE_CLUSTERING flag, which basically disable merges of any > bio segments. We already have the actual max_segment size limit > to say which length a segment should have, independent of merged > or originally created, so this limit generally should rarely if > ever be required, and mostly is an old cut an paste error. > > Can you go over your drivers and check if it could be removed? > I have tested changing this to ENABLE_CLUSTERING in iscsi_tcp.c. This code is used for both the iscsi initiator and the target. On the initiator side, there is no problem with enabling clustering of bios. But on the target side, the code seems to assume that IOs do not cross page boundaries, resulting in WARN_ON messages and failed IO when ENABLE_CLUSTERING is set: > [ 78.644595] RIP: 0010:copy_page_to_iter+0x1a6/0x2e0 > [ 78.644596] Code: 0c 24 48 98 49 89 ce 48 89 c2 49 29 c6 48 29 ca 4d 01 f4 > 48 01 d5 48 85 c0 0f 85 71 ff ff ff 48 85 ed 0f 84 68 ff ff ff eb b2 <0f> 0b > 45 31 ed eb 8d bf 01 00 00 00 e8 d9 1e c5 ff 65 4c 8b 34 25 > [ 78.644596] RSP: 0018:a04b41eefbe0 EFLAGS: 00010206 > [ 78.644597] RAX: f7acc40de180 RBX: a04b41eefd80 RCX: > 0028a014 > [ 78.644598] RDX: 2000 RSI: 1000 RDI: > f7acc000 > [ 78.644598] RBP: f7acc40de180 R08: 8f1703786000 R09: > 2000 > [ 78.644599] R10: 02c0 R11: 8f16b3f31400 R12: > > [ 78.644600] R13: 2000 R14: 0008 R15: > 3800 > [ 78.644601] FS: () GS:8f1739c0() > knlGS: > [ 78.644601] CS: 0010 DS: ES: CR0: 80050033 > [ 78.644602] CR2: 7f0f54772000 CR3: 00012e670004 CR4: > 003606f0 > [ 78.644624] DR0: DR1: DR2: > > [ 78.644625] DR3: DR6: fffe0ff0 DR7: > 0400 > [ 78.644625] Call Trace: > [ 78.644631] skb_copy_datagram_iter+0x16f/0x270 > [ 78.644633] tcp_recvmsg+0x223/0xc30 > [ 78.644637] inet_recvmsg+0x4b/0xe0 > [ 78.644644] iscsit_do_rx_data.constprop.9+0x89/0x110 [iscsi_target_mod] > [ 78.644650] rx_data+0x58/0x70 [iscsi_target_mod] > [ 78.644654] iscsit_get_rx_pdu+0xa7b/0xd20 [iscsi_target_mod] > [ 78.644657] ? preempt_count_sub+0x43/0x50 > [ 78.644659] ? _raw_spin_unlock_irq+0x22/0x40 > [ 78.644663] ? iscsi_target_tx_thread+0x1d0/0x1d0 [iscsi_target_mod] > [ 78.644667] ? iscsi_target_rx_thread+0x71/0xd0 [iscsi_target_mod] > [ 78.644670] iscsi_target_rx_thread+0x71/0xd0 [iscsi_target_mod] > [ 78.644672] kthread+0x116/0x130 > [ 78.644673] ? kthread_create_worker_on_cpu+0x40/0x40 > [ 78.644675] ret_from_fork+0x24/0x50 > [ 78.644678] ---[ end trace a0d6d5ab0e8625ee ]--- > The problem seems to be in iov_iter.c:copy_page_to_iter(), where it first calls page_copy_sane(), which makes sure the copy does not cross a page boundary: > static inline bool page_copy_sane(struct page *page, size_t offset, size_t n) > { > struct page *head = compound_head(page); > size_t v = n + offset + page_address(page) - page_address(head); > > if (likely(n <= v && v <= (PAGE_SIZE << compound_order(head > return true; > WARN_ON(1); > return false; > } I will submit a separate BUG email about this to the linux-scsi and target-devel mailing lists, since it's not clear to me how to fix this. In the mean time, the iscsi_tcp.c file still needs DISABLE_CLUSTERING. -- Lee "Deviants will be sacrificed to ensure group solidarity."
Re: [PATCH] scsi: t10-pi: Return correct ref tag when queue has no integrity profile
On 12/5/18 6:04 AM, Martin K. Petersen wrote: Since the return value of this function is 'u32', can the ' & 0x' be left out? Absolutely, and I almost zapped it. However, I decided to leave it to emphasize the point that the reference tag is truncated to a 32-bit value. To me, this is more obvious than having to backtrack and spot the u32 in the function definition. I generally appreciate some sort of commentary around a return statement if the value deviates from the ordinary. The parentheses around the shift value irk me but had to leave those in place to silence gcc. Hi Martin, Had you considered to use lower_32_bits() instead of "0x"? That would to avoid that reviewers have to count the 'f'-s to verify correctness of t10_pi_ref_tag(). Thanks, Bart.
Re: [PATCH] scsi: t10-pi: Return correct ref tag when queue has no integrity profile
Hi Bart, > Since the return value of this function is 'u32', can the ' & > 0x' be left out? Absolutely, and I almost zapped it. However, I decided to leave it to emphasize the point that the reference tag is truncated to a 32-bit value. To me, this is more obvious than having to backtrack and spot the u32 in the function definition. I generally appreciate some sort of commentary around a return statement if the value deviates from the ordinary. The parentheses around the shift value irk me but had to leave those in place to silence gcc. -- Martin K. Petersen Oracle Linux Engineering
[PATCH v7 3/5] target: add device vendor_id configfs attribute
The vendor_id attribute will allow for the modification of the T10 Vendor Identification string returned in inquiry responses. Its value can be viewed and modified via the ConfigFS path at: target/core/$backstore/$name/wwn/vendor_id "LIO-ORG" remains the default value, which is set when the backstore device is enabled. Signed-off-by: David Disseldorp --- drivers/target/target_core_configfs.c | 70 +++ 1 file changed, 70 insertions(+) diff --git a/drivers/target/target_core_configfs.c b/drivers/target/target_core_configfs.c index 8277bcf81d6e..f099c2ae451f 100644 --- a/drivers/target/target_core_configfs.c +++ b/drivers/target/target_core_configfs.c @@ -1217,6 +1217,74 @@ static struct t10_wwn *to_t10_wwn(struct config_item *item) } /* + * STANDARD and VPD page 0x80 T10 Vendor Identification + */ +static ssize_t target_wwn_vendor_id_show(struct config_item *item, + char *page) +{ + return sprintf(page, "%s\n", _t10_wwn(item)->vendor[0]); +} + +static ssize_t target_wwn_vendor_id_store(struct config_item *item, + const char *page, size_t count) +{ + struct t10_wwn *t10_wwn = to_t10_wwn(item); + struct se_device *dev = t10_wwn->t10_dev; + /* +2 to allow for a trailing (stripped) '\n' and null-terminator */ + unsigned char buf[INQUIRY_VENDOR_LEN + 2]; + char *stripped = NULL; + size_t len; + int i; + + len = strlcpy(buf, page, sizeof(buf)); + if (len < sizeof(buf)) { + /* Strip any newline added from userspace. */ + stripped = strstrip(buf); + len = strlen(stripped); + } + if (len > INQUIRY_VENDOR_LEN) { + pr_err("Emulated T10 Vendor Identification exceeds" + " INQUIRY_VENDOR_LEN: " __stringify(INQUIRY_VENDOR_LEN) + "\n"); + return -EOVERFLOW; + } + + /* +* SPC 4.3.1: +* ASCII data fields shall contain only ASCII printable characters (i.e., +* code values 20h to 7Eh) and may be terminated with one or more ASCII +* null (00h) characters. +*/ + for (i = 0; i < len; i++) { + if ((stripped[i] < 0x20) || (stripped[i] > 0x7E)) { + pr_err("Emulated T10 Vendor Identification contains" + " non-ASCII-printable characters\n"); + return -EINVAL; + } + } + + /* +* Check to see if any active exports exist. If they do exist, fail +* here as changing this information on the fly (underneath the +* initiator side OS dependent multipath code) could cause negative +* effects. +*/ + if (dev->export_count) { + pr_err("Unable to set T10 Vendor Identification while" + " active %d exports exist\n", dev->export_count); + return -EINVAL; + } + + BUILD_BUG_ON(sizeof(dev->t10_wwn.vendor) != INQUIRY_VENDOR_LEN + 1); + strlcpy(dev->t10_wwn.vendor, stripped, sizeof(dev->t10_wwn.vendor)); + + pr_debug("Target_Core_ConfigFS: Set emulated T10 Vendor Identification:" +" %s\n", dev->t10_wwn.vendor); + + return count; +} + +/* * VPD page 0x80 Unit serial */ static ssize_t target_wwn_vpd_unit_serial_show(struct config_item *item, @@ -1362,6 +1430,7 @@ DEF_DEV_WWN_ASSOC_SHOW(vpd_assoc_target_port, 0x10); /* VPD page 0x83 Association: SCSI Target Device */ DEF_DEV_WWN_ASSOC_SHOW(vpd_assoc_scsi_target_device, 0x20); +CONFIGFS_ATTR(target_wwn_, vendor_id); CONFIGFS_ATTR(target_wwn_, vpd_unit_serial); CONFIGFS_ATTR_RO(target_wwn_, vpd_protocol_identifier); CONFIGFS_ATTR_RO(target_wwn_, vpd_assoc_logical_unit); @@ -1369,6 +1438,7 @@ CONFIGFS_ATTR_RO(target_wwn_, vpd_assoc_target_port); CONFIGFS_ATTR_RO(target_wwn_, vpd_assoc_scsi_target_device); static struct configfs_attribute *target_core_dev_wwn_attrs[] = { + _wwn_attr_vendor_id, _wwn_attr_vpd_unit_serial, _wwn_attr_vpd_protocol_identifier, _wwn_attr_vpd_assoc_logical_unit, -- 2.13.7
[PATCH v7 5/5] target: perform t10_wwn ID initialisation in target_alloc_device()
Initialise the t10_wwn vendor, model and revision defaults when a device is allocated instead of when it's enabled. This ensures that custom vendor or model strings set prior to enablement are not later overwritten with default values. The TRANSPORT_FLAG_PASSTHROUGH conditional can be dropped for the following reasons: - target_core_pscsi overwrites the defaults in the pscsi_configure_device() callback. + the contents is then only used for ConfigFS via $pscsi_dev/statistics/scsi_lu/vend, etc. - target_core_user doesn't touch the defaults, nor are they used for anything outside of ConfigFS. Signed-off-by: David Disseldorp Reviewed-by: Roman Bolshakov --- drivers/target/target_core_device.c | 21 +++-- 1 file changed, 7 insertions(+), 14 deletions(-) diff --git a/drivers/target/target_core_device.c b/drivers/target/target_core_device.c index ebd787bb29a8..5ead7eae30b5 100644 --- a/drivers/target/target_core_device.c +++ b/drivers/target/target_core_device.c @@ -810,6 +810,13 @@ struct se_device *target_alloc_device(struct se_hba *hba, const char *name) mutex_init(_lun->lun_tg_pt_md_mutex); xcopy_lun->lun_tpg = _pt_tpg; + /* Preload the default INQUIRY const values */ + strlcpy(dev->t10_wwn.vendor, "LIO-ORG", sizeof(dev->t10_wwn.vendor)); + strlcpy(dev->t10_wwn.model, dev->transport->inquiry_prod, + sizeof(dev->t10_wwn.model)); + strlcpy(dev->t10_wwn.revision, dev->transport->inquiry_rev, + sizeof(dev->t10_wwn.revision)); + return dev; } @@ -984,20 +991,6 @@ int target_configure_device(struct se_device *dev) */ INIT_WORK(>qf_work_queue, target_qf_do_work); - /* -* Preload the initial INQUIRY const values if we are doing -* anything virtual (IBLOCK, FILEIO, RAMDISK), but not for TCM/pSCSI -* passthrough because this is being provided by the backend LLD. -*/ - if (!(dev->transport->transport_flags & TRANSPORT_FLAG_PASSTHROUGH)) { - strlcpy(dev->t10_wwn.vendor, "LIO-ORG", - sizeof(dev->t10_wwn.vendor)); - strlcpy(dev->t10_wwn.model, dev->transport->inquiry_prod, - sizeof(dev->t10_wwn.model)); - strlcpy(dev->t10_wwn.revision, dev->transport->inquiry_rev, - sizeof(dev->t10_wwn.revision)); - } - scsi_dump_inquiry(dev); spin_lock(>device_lock); -- 2.13.7
[PATCH v7 2/5] target: consistently null-terminate t10_wwn strings
In preparation for supporting user provided vendor strings, add an extra byte to the vendor, model and revision arrays in struct t10_wwn. This ensures that the full INQUIRY data can be carried in the arrays along with a null-terminator. Change a number of array readers and writers so that they account for explicit null-termination: - The pscsi_set_inquiry_info() and emulate_model_alias_store() codepaths don't currently explicitly null-terminate; fix this. - Existing t10_wwn field dumps use for-loops which step over null-terminators for right-padding. + Use printf with width specifiers instead. Signed-off-by: David Disseldorp --- drivers/target/target_core_configfs.c | 16 +++ drivers/target/target_core_device.c | 46 ++-- drivers/target/target_core_pscsi.c| 50 +++ drivers/target/target_core_spc.c | 7 ++--- drivers/target/target_core_stat.c | 32 +- include/target/target_core_base.h | 14 +++--- 6 files changed, 63 insertions(+), 102 deletions(-) diff --git a/drivers/target/target_core_configfs.c b/drivers/target/target_core_configfs.c index f6b1549f4142..8277bcf81d6e 100644 --- a/drivers/target/target_core_configfs.c +++ b/drivers/target/target_core_configfs.c @@ -613,12 +613,17 @@ static void dev_set_t10_wwn_model_alias(struct se_device *dev) const char *configname; configname = config_item_name(>dev_group.cg_item); - if (strlen(configname) >= 16) { + if (strlen(configname) >= INQUIRY_MODEL_LEN) { pr_warn("dev[%p]: Backstore name '%s' is too long for " - "INQUIRY_MODEL, truncating to 16 bytes\n", dev, + "INQUIRY_MODEL, truncating to 15 characters\n", dev, configname); } - snprintf(>t10_wwn.model[0], 16, "%s", configname); + /* +* XXX We can't use sizeof(dev->t10_wwn.model) (INQUIRY_MODEL_LEN + 1) +* here without potentially breaking existing setups, so continue to +* truncate one byte shorter than what can be carried in INQUIRY. +*/ + strlcpy(dev->t10_wwn.model, configname, INQUIRY_MODEL_LEN); } static ssize_t emulate_model_alias_store(struct config_item *item, @@ -640,11 +645,12 @@ static ssize_t emulate_model_alias_store(struct config_item *item, if (ret < 0) return ret; + BUILD_BUG_ON(sizeof(dev->t10_wwn.model) != INQUIRY_MODEL_LEN + 1); if (flag) { dev_set_t10_wwn_model_alias(dev); } else { - strncpy(>t10_wwn.model[0], - dev->transport->inquiry_prod, 16); + strlcpy(dev->t10_wwn.model, dev->transport->inquiry_prod, + sizeof(dev->t10_wwn.model)); } da->emulate_model_alias = flag; return count; diff --git a/drivers/target/target_core_device.c b/drivers/target/target_core_device.c index 47b5ef153135..ebd787bb29a8 100644 --- a/drivers/target/target_core_device.c +++ b/drivers/target/target_core_device.c @@ -720,36 +720,17 @@ void core_dev_free_initiator_node_lun_acl( static void scsi_dump_inquiry(struct se_device *dev) { struct t10_wwn *wwn = >t10_wwn; - char buf[17]; - int i, device_type; + int device_type = dev->transport->get_device_type(dev); + /* * Print Linux/SCSI style INQUIRY formatting to the kernel ring buffer */ - for (i = 0; i < 8; i++) - if (wwn->vendor[i] >= 0x20) - buf[i] = wwn->vendor[i]; - else - buf[i] = ' '; - buf[i] = '\0'; - pr_debug(" Vendor: %s\n", buf); - - for (i = 0; i < 16; i++) - if (wwn->model[i] >= 0x20) - buf[i] = wwn->model[i]; - else - buf[i] = ' '; - buf[i] = '\0'; - pr_debug(" Model: %s\n", buf); - - for (i = 0; i < 4; i++) - if (wwn->revision[i] >= 0x20) - buf[i] = wwn->revision[i]; - else - buf[i] = ' '; - buf[i] = '\0'; - pr_debug(" Revision: %s\n", buf); - - device_type = dev->transport->get_device_type(dev); + pr_debug(" Vendor: %-" __stringify(INQUIRY_VENDOR_LEN) "s\n", + wwn->vendor); + pr_debug(" Model: %-" __stringify(INQUIRY_MODEL_LEN) "s\n", + wwn->model); + pr_debug(" Revision: %-" __stringify(INQUIRY_REVISION_LEN) "s\n", + wwn->revision); pr_debug(" Type: %s ", scsi_device_type(device_type)); } @@ -1009,11 +990,12 @@ int target_configure_device(struct se_device *dev) * passthrough because this is being provided by the backend LLD. */ if (!(dev->transport->transport_flags & TRANSPORT_FLAG_PASSTHROUGH)) { - strncpy(>t10_wwn.vendor[0], "LIO-ORG", 8); -
[PATCH v7 1/5] target: use consistent left-aligned ASCII INQUIRY data
spc5r17.pdf specifies: 4.3.1 ASCII data field requirements ASCII data fields shall contain only ASCII printable characters (i.e., code values 20h to 7Eh) and may be terminated with one or more ASCII null (00h) characters. ASCII data fields described as being left-aligned shall have any unused bytes at the end of the field (i.e., highest offset) and the unused bytes shall be filled with ASCII space characters (20h). LIO currently space-pads the T10 VENDOR IDENTIFICATION and PRODUCT IDENTIFICATION fields in the standard INQUIRY data. However, the PRODUCT REVISION LEVEL field in the standard INQUIRY data as well as the T10 VENDOR IDENTIFICATION field in the INQUIRY Device Identification VPD Page are zero-terminated/zero-padded. Fix this inconsistency by using space-padding for all of the above fields. Signed-off-by: David Disseldorp Reviewed-by: Christoph Hellwig Reviewed-by: Bryant G. Ly Reviewed-by: Lee Duncan Reviewed-by: Hannes Reinecke Reviewed-by: Roman Bolshakov --- drivers/target/target_core_spc.c | 17 - 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/drivers/target/target_core_spc.c b/drivers/target/target_core_spc.c index f459118bc11b..c37dd36ec77d 100644 --- a/drivers/target/target_core_spc.c +++ b/drivers/target/target_core_spc.c @@ -108,12 +108,17 @@ spc_emulate_inquiry_std(struct se_cmd *cmd, unsigned char *buf) buf[7] = 0x2; /* CmdQue=1 */ - memcpy([8], "LIO-ORG ", 8); - memset([16], 0x20, 16); + /* +* ASCII data fields described as being left-aligned shall have any +* unused bytes at the end of the field (i.e., highest offset) and the +* unused bytes shall be filled with ASCII space characters (20h). +*/ + memset([8], 0x20, 8 + 16 + 4); + memcpy([8], "LIO-ORG", sizeof("LIO-ORG") - 1); memcpy([16], dev->t10_wwn.model, - min_t(size_t, strlen(dev->t10_wwn.model), 16)); + strnlen(dev->t10_wwn.model, 16)); memcpy([32], dev->t10_wwn.revision, - min_t(size_t, strlen(dev->t10_wwn.revision), 4)); + strnlen(dev->t10_wwn.revision, 4)); buf[4] = 31; /* Set additional length to 31 */ return 0; @@ -251,7 +256,9 @@ spc_emulate_evpd_83(struct se_cmd *cmd, unsigned char *buf) buf[off] = 0x2; /* ASCII */ buf[off+1] = 0x1; /* T10 Vendor ID */ buf[off+2] = 0x0; - memcpy([off+4], "LIO-ORG", 8); + /* left align Vendor ID and pad with spaces */ + memset([off+4], 0x20, 8); + memcpy([off+4], "LIO-ORG", sizeof("LIO-ORG") - 1); /* Extra Byte for NULL Terminator */ id_len++; /* Identifier Length */ -- 2.13.7
[PATCH v7 0/5] target: user configurable T10 Vendor ID
This patch-set allows for the modification of the T10 Vendor Identification string returned in the SCSI INQUIRY response, via the target/core/$backstore/$name/wwn/vendor_id ConfigFS path. Changes since v6: - PATCH 2/5 + fill pscsi inquiry data using proper sd->inquiry pointer names + dump pscsi sd->inquiry data using sprintf + tweak emulate_model_alias truncation warning + drop a few duplicate BUILD_BUG_ON()s + drop Hannes' sign-off - PATCH 3/5 + check user vendor string for non-ascii-printable chars + strip newline before INQUIRY_VENDOR_LEN length check + drop sign-offs from Bryant, Lee and Hannes Changes since v5: - remove unnecessary TRANSPORT_FLAG_PASSTHROUGH conditional from t10_wwn ID defaults initialisation. Changes since v4: - merge null-termination changes into a single patch - add patch to initialise t10_wwn ID defaults earlier - use strlcpy() instead of strncpy() in some places Changes since v3: - perform explicit null termination of t10_wwn vendor, model and revision fields. - replace field dump for-loops Changes since v2: - https://www.spinics.net/lists/target-devel/msg10720.html - Support eight byte vendor ID strings - Split out consistent INQUIRY data padding as a separate patch - Drop t10_wwn.model buffer print fix, already upstream Changes since v1: - https://www.spinics.net/lists/target-devel/msg10545.html - Rebase against nab's for-next branch, which includes Christoph's configfs API changes. David Disseldorp (5): target: use consistent left-aligned ASCII INQUIRY data target: consistently null-terminate t10_wwn strings target: add device vendor_id configfs attribute target: remove hardcoded T10 Vendor ID in INQUIRY response target: perform t10_wwn ID initialisation in target_alloc_device() drivers/target/target_core_configfs.c | 86 +-- drivers/target/target_core_device.c | 55 + drivers/target/target_core_pscsi.c| 50 +--- drivers/target/target_core_spc.c | 20 +-- drivers/target/target_core_stat.c | 32 +++--- include/target/target_core_base.h | 14 - 6 files changed, 145 insertions(+), 112 deletions(-)
Re: [PATCH] scsi: t10-pi: Return correct ref tag when queue has no integrity profile
On 12/4/18 6:31 PM, Martin K. Petersen wrote: Commit ddd0bc756983 ("block: move ref_tag calculation func to the block layer") moved ref tag calculation from SCSI to a library function. However, this change broke returning the correct ref tag for devices operating in DIF mode since these do not have an associated block integrity profile. This in turn caused read/write failures on PI-formatted disks attached to an mpt3sas controller. Fixes: ddd0bc756983 ("block: move ref_tag calculation func to the block layer") Cc: sta...@vger.kernel.org # 4.19+ Reported-by: John Garry Signed-off-by: Martin K. Petersen --- include/linux/t10-pi.h | 9 + 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/include/linux/t10-pi.h b/include/linux/t10-pi.h index b9626aa7e90c..3e2a80cc7b56 100644 --- a/include/linux/t10-pi.h +++ b/include/linux/t10-pi.h @@ -39,12 +39,13 @@ struct t10_pi_tuple { static inline u32 t10_pi_ref_tag(struct request *rq) { + unsigned int shift = ilog2(queue_logical_block_size(rq->q)); + #ifdef CONFIG_BLK_DEV_INTEGRITY - return blk_rq_pos(rq) >> - (rq->q->integrity.interval_exp - 9) & 0x; -#else - return -1U; + if (rq->q->integrity.interval_exp) + shift = rq->q->integrity.interval_exp; #endif + return blk_rq_pos(rq) >> (shift - SECTOR_SHIFT) & 0x; } Since the return value of this function is 'u32', can the ' & 0x' be left out? Thanks, Bart.
98IvH6E5wF5vcK8DK6
238.144.170.101106.119.122.156 ¿ª ¸÷ Àà ÔöÖµ Ë°Õý ¹æ Õæ ƱQQ:2211261333 ÁÖ ³Ì£º13632225663(΢ÐÅͬºÅ)
[PATCH] scsi: t10-pi: Return correct ref tag when queue has no integrity profile
Commit ddd0bc756983 ("block: move ref_tag calculation func to the block layer") moved ref tag calculation from SCSI to a library function. However, this change broke returning the correct ref tag for devices operating in DIF mode since these do not have an associated block integrity profile. This in turn caused read/write failures on PI-formatted disks attached to an mpt3sas controller. Fixes: ddd0bc756983 ("block: move ref_tag calculation func to the block layer") Cc: sta...@vger.kernel.org # 4.19+ Reported-by: John Garry Signed-off-by: Martin K. Petersen --- include/linux/t10-pi.h | 9 + 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/include/linux/t10-pi.h b/include/linux/t10-pi.h index b9626aa7e90c..3e2a80cc7b56 100644 --- a/include/linux/t10-pi.h +++ b/include/linux/t10-pi.h @@ -39,12 +39,13 @@ struct t10_pi_tuple { static inline u32 t10_pi_ref_tag(struct request *rq) { + unsigned int shift = ilog2(queue_logical_block_size(rq->q)); + #ifdef CONFIG_BLK_DEV_INTEGRITY - return blk_rq_pos(rq) >> - (rq->q->integrity.interval_exp - 9) & 0x; -#else - return -1U; + if (rq->q->integrity.interval_exp) + shift = rq->q->integrity.interval_exp; #endif + return blk_rq_pos(rq) >> (shift - SECTOR_SHIFT) & 0x; } extern const struct blk_integrity_profile t10_pi_type1_crc; -- 2.19.2
UBSAN: Undefined behaviour in drivers/scsi/megaraid/megaraid_sas_fp.c:117:32
UBSAN: Undefined behaviour in drivers/scsi/megaraid/megaraid_sas_fp.c:117:32 index 255 is out of range for type 'MR_LD_SPAN_MAP [1]' This commit 51087a8617fe (megaraid_sas : Extended VD support) defined those, struct MR_FW_RAID_MAP { u8 ldTgtIdToLd[MAX_RAIDMAP_LOGICAL_DRIVES+\ MAX_RAIDMAP_VIEWS]; struct MR_LD_SPAN_MAP ldSpanMap[1]; struct MR_FW_RAID_MAP_ALL { struct MR_FW_RAID_MAP raidMap; struct MR_LD_RAID *MR_LdRaidGet(u32 ld, struct MR_DRV_RAID_MAP_ALL *map) return >raidMap.ldSpanMap[ld].ldRaid; Then, there are several paths could trigger that undefined behavior due to out-of-bound access. mr_update_load_balance_params for (ldCount = 0; ldCount < MAX_LOGICAL_DRIVES_EXT;\ ldCount++; ld = MR_TargetIdToLdGet(ldCount, drv_map); raid = MR_LdRaidGet(ld, drv_map) megasas_build_io_fusion megasas_build_ld_nonrw_fusion ld = MR_TargetIdToLdGet(device_id, local_map_ptr); raid = MR_LdRaidGet(ld, local_map_ptr); Any clue?
Re: [PATCH v6 3/5] target: add device vendor_id configfs attribute
On Tue, 2018-12-04 at 16:26 +0300, Roman Bolshakov wrote: > wrt PATCH 5 in the series. Should we allow to set vendor_id for for > pscsi? I think we should allow that. Bart.
Re: [PATCH v6 3/5] target: add device vendor_id configfs attribute
On Tue, 4 Dec 2018 15:13:51 +0300, Roman Bolshakov wrote: > > + /* Assume ASCII encoding. Strip any newline added from userspace. */ > > + BUILD_BUG_ON(sizeof(dev->t10_wwn.vendor) != INQUIRY_VENDOR_LEN + 1); > > + strlcpy(dev->t10_wwn.vendor, strstrip(buf), > > + sizeof(dev->t10_wwn.vendor)); > > + > > Should we allow non-ASCII characters? No :) > It's okay to strip final newline > for convenience. A simple loop would ensure the rest is conformant to > SPC. EINVAL can be returned otherwise. I'll add an isascii() loop in the next round. Cheers, David
Re: [PATCH v6 3/5] target: add device vendor_id configfs attribute
On Tue, 4 Dec 2018 16:26:59 +0300, Roman Bolshakov wrote: > wrt PATCH 5 in the series. Should we allow to set vendor_id for for > pscsi? target_transport_configure sets t10_wwn fields for pscsi, but but > an attempt to set vendor_id will overwrite the value recieved from > scsi_device. I considered adding an if (PASSTHROUGH){return -EINVAL}, but we already allow the model/product string to be set for pscsi+tcmu backends via emulate_model_alias, so decided against it. > And (please correct me if I'm wrong) it's not used anywhere except > statistics config_group. That means in the actual INQUIRY commands it > will still return vendor_id of the underlying storage. If that's that's > true, this means an attempt to set vendor_id will be succesful but it > won't do what's intended for. That's correct. Cheers, David
Re: [PATCH v6 3/5] target: add device vendor_id configfs attribute
On Tue, Dec 04, 2018 at 03:13:51PM +0300, Roman Bolshakov wrote: > On Tue, Dec 04, 2018 at 11:12:36AM +0100, David Disseldorp wrote: > > The vendor_id attribute will allow for the modification of the T10 > > Vendor Identification string returned in inquiry responses. Its value > > can be viewed and modified via the ConfigFS path at: > > target/core/$backstore/$name/wwn/vendor_id > > > > "LIO-ORG" remains the default value, which is set when the backstore > > device is enabled. > > > > Signed-off-by: David Disseldorp > > Reviewed-by: Bryant G. Ly > > Reviewed-by: Lee Duncan > > Reviewed-by: Hannes Reinecke > > --- > > drivers/target/target_core_configfs.c | 48 > > +++ > > 1 file changed, 48 insertions(+) > > > > diff --git a/drivers/target/target_core_configfs.c > > b/drivers/target/target_core_configfs.c > > index 34872f24e8bf..67303c3f9cb4 100644 > > --- a/drivers/target/target_core_configfs.c > > +++ b/drivers/target/target_core_configfs.c > > + > > + /* Assume ASCII encoding. Strip any newline added from userspace. */ > > + BUILD_BUG_ON(sizeof(dev->t10_wwn.vendor) != INQUIRY_VENDOR_LEN + 1); > > + strlcpy(dev->t10_wwn.vendor, strstrip(buf), > > + sizeof(dev->t10_wwn.vendor)); > > + > > Should we allow non-ASCII characters? It's okay to strip final newline > for convenience. A simple loop would ensure the rest is conformant to > SPC. EINVAL can be returned otherwise. > > And for fuzzy purposes there could be a special backstore that does all > sorts of nasty things. > > According to SPC 4.3.1: > ASCII data fields shall contain only ASCII printable characters (i.e., > code values 20h to 7Eh) and may be terminated with one or more ASCII > null (00h) characters. > > 3.3.10 shall > keyword indicating a mandatory requirement > Note 1 to entry: Designers are required to implement all such > interoperability with other products that conform to this standard. > > Thank you, > Roman wrt PATCH 5 in the series. Should we allow to set vendor_id for for pscsi? target_transport_configure sets t10_wwn fields for pscsi, but but an attempt to set vendor_id will overwrite the value recieved from scsi_device. And (please correct me if I'm wrong) it's not used anywhere except statistics config_group. That means in the actual INQUIRY commands it will still return vendor_id of the underlying storage. If that's that's true, this means an attempt to set vendor_id will be succesful but it won't do what's intended for. Perhaps -ENOSYS or an error code of your preference could be returned if device has TRANSPORT_FLAG_PASSTHROUGH. Roman
Re: [PATCH v6 5/5] target: perform t10_wwn ID initialisation in target_alloc_device()
On Tue, Dec 04, 2018 at 11:12:38AM +0100, David Disseldorp wrote: > Initialise the t10_wwn vendor, model and revision defaults when a > device is allocated instead of when it's enabled. This ensures that > custom vendor or model strings set prior to enablement are not later > overwritten with default values. > > The TRANSPORT_FLAG_PASSTHROUGH conditional can be dropped for the > following reasons: > - target_core_pscsi overwrites the defaults in the > pscsi_configure_device() callback. > + the contents is then only used for ConfigFS via > $pscsi_dev/info, $pscsi_dev/statistics/scsi_lu/vend, etc. > - target_core_user doesn't touch the defaults, nor are they used for > anything outside of ConfigFS. > > Signed-off-by: David Disseldorp > --- > drivers/target/target_core_device.c | 27 ++- > 1 file changed, 10 insertions(+), 17 deletions(-) > Reviewed-by: Roman Bolshakov Thanks, Roman
Re: [PATCH v6 4/5] target: remove hardcoded T10 Vendor ID in INQUIRY response
On Tue, Dec 04, 2018 at 11:12:37AM +0100, David Disseldorp wrote: > Use the value stored in t10_wwn.vendor, which defaults to "LIO-ORG", but > can be reconfigured via the vendor_id ConfigFS attribute. > > Signed-off-by: David Disseldorp > Reviewed-by: Bryant G. Ly > Reviewed-by: Lee Duncan > Reviewed-by: Hannes Reinecke > --- > drivers/target/target_core_spc.c | 8 +--- > 1 file changed, 5 insertions(+), 3 deletions(-) > Reviewed-by: Roman Bolshakov Thank you, Roman
Re: [PATCH v6 3/5] target: add device vendor_id configfs attribute
On Tue, Dec 04, 2018 at 11:12:36AM +0100, David Disseldorp wrote: > The vendor_id attribute will allow for the modification of the T10 > Vendor Identification string returned in inquiry responses. Its value > can be viewed and modified via the ConfigFS path at: > target/core/$backstore/$name/wwn/vendor_id > > "LIO-ORG" remains the default value, which is set when the backstore > device is enabled. > > Signed-off-by: David Disseldorp > Reviewed-by: Bryant G. Ly > Reviewed-by: Lee Duncan > Reviewed-by: Hannes Reinecke > --- > drivers/target/target_core_configfs.c | 48 > +++ > 1 file changed, 48 insertions(+) > > diff --git a/drivers/target/target_core_configfs.c > b/drivers/target/target_core_configfs.c > index 34872f24e8bf..67303c3f9cb4 100644 > --- a/drivers/target/target_core_configfs.c > +++ b/drivers/target/target_core_configfs.c > + > + /* Assume ASCII encoding. Strip any newline added from userspace. */ > + BUILD_BUG_ON(sizeof(dev->t10_wwn.vendor) != INQUIRY_VENDOR_LEN + 1); > + strlcpy(dev->t10_wwn.vendor, strstrip(buf), > + sizeof(dev->t10_wwn.vendor)); > + Should we allow non-ASCII characters? It's okay to strip final newline for convenience. A simple loop would ensure the rest is conformant to SPC. EINVAL can be returned otherwise. And for fuzzy purposes there could be a special backstore that does all sorts of nasty things. According to SPC 4.3.1: ASCII data fields shall contain only ASCII printable characters (i.e., code values 20h to 7Eh) and may be terminated with one or more ASCII null (00h) characters. 3.3.10 shall keyword indicating a mandatory requirement Note 1 to entry: Designers are required to implement all such interoperability with other products that conform to this standard. Thank you, Roman
Re: [PATCH v5 2/5] target: consistently null-terminate t10_wwn strings
On Sat, Dec 01, 2018 at 12:34:20AM +0100, David Disseldorp wrote: > In preparation for supporting user provided vendor strings, add an extra > byte to the vendor, model and revision arrays in struct t10_wwn. This > ensures that the full INQUIRY data can be carried in the arrays along > with a null-terminator. > > Change a number of array readers and writers so that they account for > explicit null-termination: > - The pscsi_set_inquiry_info() and emulate_model_alias_store() codepaths > don't currently explicitly null-terminate; fix this. > - Existing t10_wwn field dumps use for-loops which step over > null-terminators for right-padding. > + Use printf with width specifiers instead. > > Signed-off-by: David Disseldorp > --- > drivers/target/target_core_configfs.c | 14 +++--- > drivers/target/target_core_device.c | 49 > --- > drivers/target/target_core_pscsi.c| 18 - > drivers/target/target_core_spc.c | 7 ++--- > drivers/target/target_core_stat.c | 32 +-- > include/target/target_core_base.h | 14 +++--- > 6 files changed, 61 insertions(+), 73 deletions(-) > > diff --git a/drivers/target/target_core_configfs.c > b/drivers/target/target_core_configfs.c > index f6b1549f4142..34872f24e8bf 100644 > --- a/drivers/target/target_core_configfs.c > +++ b/drivers/target/target_core_configfs.c > @@ -613,12 +613,17 @@ static void dev_set_t10_wwn_model_alias(struct > se_device *dev) > const char *configname; > > configname = config_item_name(>dev_group.cg_item); > - if (strlen(configname) >= 16) { > + if (strlen(configname) >= INQUIRY_MODEL_LEN) { > pr_warn("dev[%p]: Backstore name '%s' is too long for " > "INQUIRY_MODEL, truncating to 16 bytes\n", dev, The warning (which I understand predates your patch) is misleading, it should mention truncation to 15 instead of 16 bytes and your comment just below explains this. > configname); > } > - snprintf(>t10_wwn.model[0], 16, "%s", configname); > + /* > + * XXX We can't use sizeof(dev->t10_wwn.model) (INQUIRY_MODEL_LEN + 1) > + * here without potentially breaking existing setups, so continue to > + * truncate one byte shorter than what can be carried in INQUIRY. > + */ > + strlcpy(dev->t10_wwn.model, configname, INQUIRY_MODEL_LEN); > } > > diff --git a/drivers/target/target_core_device.c > b/drivers/target/target_core_device.c > index 47b5ef153135..5512871f50e4 100644 > --- a/drivers/target/target_core_device.c > +++ b/drivers/target/target_core_device.c > @@ -1008,12 +989,16 @@ int target_configure_device(struct se_device *dev) >* anything virtual (IBLOCK, FILEIO, RAMDISK), but not for TCM/pSCSI >* passthrough because this is being provided by the backend LLD. >*/ > + BUILD_BUG_ON(sizeof(dev->t10_wwn.vendor) != INQUIRY_VENDOR_LEN + 1); > + BUILD_BUG_ON(sizeof(dev->t10_wwn.model) != INQUIRY_MODEL_LEN + 1); > + BUILD_BUG_ON(sizeof(dev->t10_wwn.revision) != INQUIRY_REVISION_LEN + 1); I'm sorry I'm missing something. Why BUILD_BUG_ON is added in many places? > diff --git a/drivers/target/target_core_pscsi.c > b/drivers/target/target_core_pscsi.c > index 47d76c862014..1002829f2038 100644 > --- a/drivers/target/target_core_pscsi.c > +++ b/drivers/target/target_core_pscsi.c > @@ -190,9 +190,15 @@ pscsi_set_inquiry_info(struct scsi_device *sdev, struct > t10_wwn *wwn) > /* >* Use sdev->inquiry from drivers/scsi/scsi_scan.c:scsi_alloc_sdev() >*/ > - memcpy(>vendor[0], [8], sizeof(wwn->vendor)); > - memcpy(>model[0], [16], sizeof(wwn->model)); > - memcpy(>revision[0], [32], sizeof(wwn->revision)); > + BUILD_BUG_ON(sizeof(wwn->vendor) != INQUIRY_VENDOR_LEN + 1); > + snprintf(wwn->vendor, sizeof(wwn->vendor), > + "%." __stringify(INQUIRY_VENDOR_LEN) "s", [8]); > + BUILD_BUG_ON(sizeof(wwn->model) != INQUIRY_MODEL_LEN + 1); > + snprintf(wwn->model, sizeof(wwn->model), > + "%." __stringify(INQUIRY_MODEL_LEN) "s", [16]); > + BUILD_BUG_ON(sizeof(wwn->revision) != INQUIRY_REVISION_LEN + 1); > + snprintf(wwn->revision, sizeof(wwn->revision), > + "%." __stringify(INQUIRY_REVISION_LEN) "s", [32]); > } > The parts of the sdev->inquiry have been already right-padded with spaces by scsi_sanitize_inquiry_string in scsi_probe_lun. Thus, it's enough to replace sizeof with the new length definitions. Also, it's possible to use sdev->model,vendor,rev pointers like in pscsi_show_configfs_dev_params instead of explicit offsets [8], [16], [32]. > static int > @@ -826,21 +832,21 @@ static ssize_t pscsi_show_configfs_dev_params(struct > se_device *dev, char *b) > if (sd) { > bl += sprintf(b + bl, ""); > bl += sprintf(b + bl, "Vendor: "); > - for (i = 0; i < 8; i++) { > +
[PATCH v6 2/5] target: consistently null-terminate t10_wwn strings
In preparation for supporting user provided vendor strings, add an extra byte to the vendor, model and revision arrays in struct t10_wwn. This ensures that the full INQUIRY data can be carried in the arrays along with a null-terminator. Change a number of array readers and writers so that they account for explicit null-termination: - The pscsi_set_inquiry_info() and emulate_model_alias_store() codepaths don't currently explicitly null-terminate; fix this. - Existing t10_wwn field dumps use for-loops which step over null-terminators for right-padding. + Use printf with width specifiers instead. Signed-off-by: David Disseldorp Reviewed-by: Hannes Reinecke --- drivers/target/target_core_configfs.c | 14 +++--- drivers/target/target_core_device.c | 49 --- drivers/target/target_core_pscsi.c| 18 - drivers/target/target_core_spc.c | 7 ++--- drivers/target/target_core_stat.c | 32 +-- include/target/target_core_base.h | 14 +++--- 6 files changed, 61 insertions(+), 73 deletions(-) diff --git a/drivers/target/target_core_configfs.c b/drivers/target/target_core_configfs.c index f6b1549f4142..34872f24e8bf 100644 --- a/drivers/target/target_core_configfs.c +++ b/drivers/target/target_core_configfs.c @@ -613,12 +613,17 @@ static void dev_set_t10_wwn_model_alias(struct se_device *dev) const char *configname; configname = config_item_name(>dev_group.cg_item); - if (strlen(configname) >= 16) { + if (strlen(configname) >= INQUIRY_MODEL_LEN) { pr_warn("dev[%p]: Backstore name '%s' is too long for " "INQUIRY_MODEL, truncating to 16 bytes\n", dev, configname); } - snprintf(>t10_wwn.model[0], 16, "%s", configname); + /* +* XXX We can't use sizeof(dev->t10_wwn.model) (INQUIRY_MODEL_LEN + 1) +* here without potentially breaking existing setups, so continue to +* truncate one byte shorter than what can be carried in INQUIRY. +*/ + strlcpy(dev->t10_wwn.model, configname, INQUIRY_MODEL_LEN); } static ssize_t emulate_model_alias_store(struct config_item *item, @@ -640,11 +645,12 @@ static ssize_t emulate_model_alias_store(struct config_item *item, if (ret < 0) return ret; + BUILD_BUG_ON(sizeof(dev->t10_wwn.model) != INQUIRY_MODEL_LEN + 1); if (flag) { dev_set_t10_wwn_model_alias(dev); } else { - strncpy(>t10_wwn.model[0], - dev->transport->inquiry_prod, 16); + strlcpy(dev->t10_wwn.model, dev->transport->inquiry_prod, + sizeof(dev->t10_wwn.model)); } da->emulate_model_alias = flag; return count; diff --git a/drivers/target/target_core_device.c b/drivers/target/target_core_device.c index 47b5ef153135..5512871f50e4 100644 --- a/drivers/target/target_core_device.c +++ b/drivers/target/target_core_device.c @@ -720,36 +720,17 @@ void core_dev_free_initiator_node_lun_acl( static void scsi_dump_inquiry(struct se_device *dev) { struct t10_wwn *wwn = >t10_wwn; - char buf[17]; - int i, device_type; + int device_type = dev->transport->get_device_type(dev); + /* * Print Linux/SCSI style INQUIRY formatting to the kernel ring buffer */ - for (i = 0; i < 8; i++) - if (wwn->vendor[i] >= 0x20) - buf[i] = wwn->vendor[i]; - else - buf[i] = ' '; - buf[i] = '\0'; - pr_debug(" Vendor: %s\n", buf); - - for (i = 0; i < 16; i++) - if (wwn->model[i] >= 0x20) - buf[i] = wwn->model[i]; - else - buf[i] = ' '; - buf[i] = '\0'; - pr_debug(" Model: %s\n", buf); - - for (i = 0; i < 4; i++) - if (wwn->revision[i] >= 0x20) - buf[i] = wwn->revision[i]; - else - buf[i] = ' '; - buf[i] = '\0'; - pr_debug(" Revision: %s\n", buf); - - device_type = dev->transport->get_device_type(dev); + pr_debug(" Vendor: %-" __stringify(INQUIRY_VENDOR_LEN) "s\n", + wwn->vendor); + pr_debug(" Model: %-" __stringify(INQUIRY_MODEL_LEN) "s\n", + wwn->model); + pr_debug(" Revision: %-" __stringify(INQUIRY_REVISION_LEN) "s\n", + wwn->revision); pr_debug(" Type: %s ", scsi_device_type(device_type)); } @@ -1008,12 +989,16 @@ int target_configure_device(struct se_device *dev) * anything virtual (IBLOCK, FILEIO, RAMDISK), but not for TCM/pSCSI * passthrough because this is being provided by the backend LLD. */ + BUILD_BUG_ON(sizeof(dev->t10_wwn.vendor) != INQUIRY_VENDOR_LEN + 1); + BUILD_BUG_ON(sizeof(dev->t10_wwn.model) !=
[PATCH v6 3/5] target: add device vendor_id configfs attribute
The vendor_id attribute will allow for the modification of the T10 Vendor Identification string returned in inquiry responses. Its value can be viewed and modified via the ConfigFS path at: target/core/$backstore/$name/wwn/vendor_id "LIO-ORG" remains the default value, which is set when the backstore device is enabled. Signed-off-by: David Disseldorp Reviewed-by: Bryant G. Ly Reviewed-by: Lee Duncan Reviewed-by: Hannes Reinecke --- drivers/target/target_core_configfs.c | 48 +++ 1 file changed, 48 insertions(+) diff --git a/drivers/target/target_core_configfs.c b/drivers/target/target_core_configfs.c index 34872f24e8bf..67303c3f9cb4 100644 --- a/drivers/target/target_core_configfs.c +++ b/drivers/target/target_core_configfs.c @@ -1217,6 +1217,52 @@ static struct t10_wwn *to_t10_wwn(struct config_item *item) } /* + * STANDARD and VPD page 0x80 T10 Vendor Identification + */ +static ssize_t target_wwn_vendor_id_show(struct config_item *item, + char *page) +{ + return sprintf(page, "%s\n", _t10_wwn(item)->vendor[0]); +} + +static ssize_t target_wwn_vendor_id_store(struct config_item *item, + const char *page, size_t count) +{ + struct t10_wwn *t10_wwn = to_t10_wwn(item); + struct se_device *dev = t10_wwn->t10_dev; + unsigned char buf[INQUIRY_VENDOR_LEN + 1]; + + if (strlen(page) > INQUIRY_VENDOR_LEN) { + pr_err("Emulated T10 Vendor Identification exceeds" + " INQUIRY_VENDOR_LEN: " __stringify(INQUIRY_VENDOR_LEN) + "\n"); + return -EOVERFLOW; + } + strlcpy(buf, page, sizeof(buf)); + /* +* Check to see if any active exports exist. If they do exist, fail +* here as changing this information on the fly (underneath the +* initiator side OS dependent multipath code) could cause negative +* effects. +*/ + if (dev->export_count) { + pr_err("Unable to set T10 Vendor Identification while" + " active %d exports exist\n", dev->export_count); + return -EINVAL; + } + + /* Assume ASCII encoding. Strip any newline added from userspace. */ + BUILD_BUG_ON(sizeof(dev->t10_wwn.vendor) != INQUIRY_VENDOR_LEN + 1); + strlcpy(dev->t10_wwn.vendor, strstrip(buf), + sizeof(dev->t10_wwn.vendor)); + + pr_debug("Target_Core_ConfigFS: Set emulated T10 Vendor Identification:" +" %s\n", dev->t10_wwn.vendor); + + return count; +} + +/* * VPD page 0x80 Unit serial */ static ssize_t target_wwn_vpd_unit_serial_show(struct config_item *item, @@ -1362,6 +1408,7 @@ DEF_DEV_WWN_ASSOC_SHOW(vpd_assoc_target_port, 0x10); /* VPD page 0x83 Association: SCSI Target Device */ DEF_DEV_WWN_ASSOC_SHOW(vpd_assoc_scsi_target_device, 0x20); +CONFIGFS_ATTR(target_wwn_, vendor_id); CONFIGFS_ATTR(target_wwn_, vpd_unit_serial); CONFIGFS_ATTR_RO(target_wwn_, vpd_protocol_identifier); CONFIGFS_ATTR_RO(target_wwn_, vpd_assoc_logical_unit); @@ -1369,6 +1416,7 @@ CONFIGFS_ATTR_RO(target_wwn_, vpd_assoc_target_port); CONFIGFS_ATTR_RO(target_wwn_, vpd_assoc_scsi_target_device); static struct configfs_attribute *target_core_dev_wwn_attrs[] = { + _wwn_attr_vendor_id, _wwn_attr_vpd_unit_serial, _wwn_attr_vpd_protocol_identifier, _wwn_attr_vpd_assoc_logical_unit, -- 2.13.7
[PATCH v6 1/5] target: use consistent left-aligned ASCII INQUIRY data
spc5r17.pdf specifies: 4.3.1 ASCII data field requirements ASCII data fields shall contain only ASCII printable characters (i.e., code values 20h to 7Eh) and may be terminated with one or more ASCII null (00h) characters. ASCII data fields described as being left-aligned shall have any unused bytes at the end of the field (i.e., highest offset) and the unused bytes shall be filled with ASCII space characters (20h). LIO currently space-pads the T10 VENDOR IDENTIFICATION and PRODUCT IDENTIFICATION fields in the standard INQUIRY data. However, the PRODUCT REVISION LEVEL field in the standard INQUIRY data as well as the T10 VENDOR IDENTIFICATION field in the INQUIRY Device Identification VPD Page are zero-terminated/zero-padded. Fix this inconsistency by using space-padding for all of the above fields. Signed-off-by: David Disseldorp Reviewed-by: Christoph Hellwig Reviewed-by: Bryant G. Ly Reviewed-by: Lee Duncan Reviewed-by: Hannes Reinecke Reviewed-by: Roman Bolshakov --- drivers/target/target_core_spc.c | 17 - 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/drivers/target/target_core_spc.c b/drivers/target/target_core_spc.c index f459118bc11b..c37dd36ec77d 100644 --- a/drivers/target/target_core_spc.c +++ b/drivers/target/target_core_spc.c @@ -108,12 +108,17 @@ spc_emulate_inquiry_std(struct se_cmd *cmd, unsigned char *buf) buf[7] = 0x2; /* CmdQue=1 */ - memcpy([8], "LIO-ORG ", 8); - memset([16], 0x20, 16); + /* +* ASCII data fields described as being left-aligned shall have any +* unused bytes at the end of the field (i.e., highest offset) and the +* unused bytes shall be filled with ASCII space characters (20h). +*/ + memset([8], 0x20, 8 + 16 + 4); + memcpy([8], "LIO-ORG", sizeof("LIO-ORG") - 1); memcpy([16], dev->t10_wwn.model, - min_t(size_t, strlen(dev->t10_wwn.model), 16)); + strnlen(dev->t10_wwn.model, 16)); memcpy([32], dev->t10_wwn.revision, - min_t(size_t, strlen(dev->t10_wwn.revision), 4)); + strnlen(dev->t10_wwn.revision, 4)); buf[4] = 31; /* Set additional length to 31 */ return 0; @@ -251,7 +256,9 @@ spc_emulate_evpd_83(struct se_cmd *cmd, unsigned char *buf) buf[off] = 0x2; /* ASCII */ buf[off+1] = 0x1; /* T10 Vendor ID */ buf[off+2] = 0x0; - memcpy([off+4], "LIO-ORG", 8); + /* left align Vendor ID and pad with spaces */ + memset([off+4], 0x20, 8); + memcpy([off+4], "LIO-ORG", sizeof("LIO-ORG") - 1); /* Extra Byte for NULL Terminator */ id_len++; /* Identifier Length */ -- 2.13.7
[PATCH v6 4/5] target: remove hardcoded T10 Vendor ID in INQUIRY response
Use the value stored in t10_wwn.vendor, which defaults to "LIO-ORG", but can be reconfigured via the vendor_id ConfigFS attribute. Signed-off-by: David Disseldorp Reviewed-by: Bryant G. Ly Reviewed-by: Lee Duncan Reviewed-by: Hannes Reinecke --- drivers/target/target_core_spc.c | 8 +--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/drivers/target/target_core_spc.c b/drivers/target/target_core_spc.c index 8ffe712cb44d..4503f3336bc2 100644 --- a/drivers/target/target_core_spc.c +++ b/drivers/target/target_core_spc.c @@ -115,7 +115,8 @@ spc_emulate_inquiry_std(struct se_cmd *cmd, unsigned char *buf) */ memset([8], 0x20, INQUIRY_VENDOR_LEN + INQUIRY_MODEL_LEN + INQUIRY_REVISION_LEN); - memcpy([8], "LIO-ORG", sizeof("LIO-ORG") - 1); + memcpy([8], dev->t10_wwn.vendor, + strnlen(dev->t10_wwn.vendor, INQUIRY_VENDOR_LEN)); memcpy([16], dev->t10_wwn.model, strnlen(dev->t10_wwn.model, INQUIRY_MODEL_LEN)); memcpy([32], dev->t10_wwn.revision, @@ -258,8 +259,9 @@ spc_emulate_evpd_83(struct se_cmd *cmd, unsigned char *buf) buf[off+1] = 0x1; /* T10 Vendor ID */ buf[off+2] = 0x0; /* left align Vendor ID and pad with spaces */ - memset([off+4], 0x20, 8); - memcpy([off+4], "LIO-ORG", sizeof("LIO-ORG") - 1); + memset([off+4], 0x20, INQUIRY_VENDOR_LEN); + memcpy([off+4], dev->t10_wwn.vendor, + strnlen(dev->t10_wwn.vendor, INQUIRY_VENDOR_LEN)); /* Extra Byte for NULL Terminator */ id_len++; /* Identifier Length */ -- 2.13.7
[PATCH v6 5/5] target: perform t10_wwn ID initialisation in target_alloc_device()
Initialise the t10_wwn vendor, model and revision defaults when a device is allocated instead of when it's enabled. This ensures that custom vendor or model strings set prior to enablement are not later overwritten with default values. The TRANSPORT_FLAG_PASSTHROUGH conditional can be dropped for the following reasons: - target_core_pscsi overwrites the defaults in the pscsi_configure_device() callback. + the contents is then only used for ConfigFS via $pscsi_dev/info, $pscsi_dev/statistics/scsi_lu/vend, etc. - target_core_user doesn't touch the defaults, nor are they used for anything outside of ConfigFS. Signed-off-by: David Disseldorp --- drivers/target/target_core_device.c | 27 ++- 1 file changed, 10 insertions(+), 17 deletions(-) diff --git a/drivers/target/target_core_device.c b/drivers/target/target_core_device.c index 5512871f50e4..bfc1b8b49940 100644 --- a/drivers/target/target_core_device.c +++ b/drivers/target/target_core_device.c @@ -810,6 +810,16 @@ struct se_device *target_alloc_device(struct se_hba *hba, const char *name) mutex_init(_lun->lun_tg_pt_md_mutex); xcopy_lun->lun_tpg = _pt_tpg; + /* Preload the default INQUIRY const values */ + BUILD_BUG_ON(sizeof(dev->t10_wwn.vendor) != INQUIRY_VENDOR_LEN + 1); + strlcpy(dev->t10_wwn.vendor, "LIO-ORG", sizeof(dev->t10_wwn.vendor)); + BUILD_BUG_ON(sizeof(dev->t10_wwn.model) != INQUIRY_MODEL_LEN + 1); + strlcpy(dev->t10_wwn.model, dev->transport->inquiry_prod, + sizeof(dev->t10_wwn.model)); + BUILD_BUG_ON(sizeof(dev->t10_wwn.revision) != INQUIRY_REVISION_LEN + 1); + strlcpy(dev->t10_wwn.revision, dev->transport->inquiry_rev, + sizeof(dev->t10_wwn.revision)); + return dev; } @@ -984,23 +994,6 @@ int target_configure_device(struct se_device *dev) */ INIT_WORK(>qf_work_queue, target_qf_do_work); - /* -* Preload the initial INQUIRY const values if we are doing -* anything virtual (IBLOCK, FILEIO, RAMDISK), but not for TCM/pSCSI -* passthrough because this is being provided by the backend LLD. -*/ - BUILD_BUG_ON(sizeof(dev->t10_wwn.vendor) != INQUIRY_VENDOR_LEN + 1); - BUILD_BUG_ON(sizeof(dev->t10_wwn.model) != INQUIRY_MODEL_LEN + 1); - BUILD_BUG_ON(sizeof(dev->t10_wwn.revision) != INQUIRY_REVISION_LEN + 1); - if (!(dev->transport->transport_flags & TRANSPORT_FLAG_PASSTHROUGH)) { - strlcpy(dev->t10_wwn.vendor, "LIO-ORG", - sizeof(dev->t10_wwn.vendor)); - strlcpy(dev->t10_wwn.model, dev->transport->inquiry_prod, - sizeof(dev->t10_wwn.model)); - strlcpy(dev->t10_wwn.revision, dev->transport->inquiry_rev, - sizeof(dev->t10_wwn.revision)); - } - scsi_dump_inquiry(dev); spin_lock(>device_lock); -- 2.13.7
[PATCH v6 0/5] target: user configurable T10 Vendor ID
This patch-set allows for the modification of the T10 Vendor Identification string returned in the SCSI INQUIRY response, via the target/core/$backstore/$name/wwn/vendor_id ConfigFS path. Changes since v5: - remove unnecessary TRANSPORT_FLAG_PASSTHROUGH conditional from t10_wwn ID defaults initialisation. Changes since v4: - merge null-termination changes into a single patch - add patch to initialise t10_wwn ID defaults earlier - use strlcpy() instead of strncpy() in some places Changes since v3: - perform explicit null termination of t10_wwn vendor, model and revision fields. - replace field dump for-loops Changes since v2: - https://www.spinics.net/lists/target-devel/msg10720.html - Support eight byte vendor ID strings - Split out consistent INQUIRY data padding as a separate patch - Drop t10_wwn.model buffer print fix, already upstream Changes since v1: - https://www.spinics.net/lists/target-devel/msg10545.html - Rebase against nab's for-next branch, which includes Christoph's configfs API changes.