| Message ID | 1454542114-1797-6-git-send-email-tyreld@linux.vnet.ibm.com (mailing list archive) |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
On Wed, Feb 03, 2016 at 05:28:33PM -0600, Tyrel Datwyler wrote: > A VIOSRP_HOST_CONFIG_TYPE management datagram (MAD) has existed in > the code for some time. From what information I've gathered from > Brian King this was likely implemented on the host side in a SLES 9 > based VIOS, which is no longer supported anywhere. Further, it is > not defined in PAPR or supported by any AIX based VIOS. > > Treating as bit rot and removing the sysfs interface and associated > host config code accordingly. Doesn't removing a sysfs interface potentially break userspace code?
On 02/04/2016 12:03 AM, Johannes Thumshirn wrote: > On Wed, Feb 03, 2016 at 05:28:33PM -0600, Tyrel Datwyler wrote: >> A VIOSRP_HOST_CONFIG_TYPE management datagram (MAD) has existed in >> the code for some time. From what information I've gathered from >> Brian King this was likely implemented on the host side in a SLES 9 >> based VIOS, which is no longer supported anywhere. Further, it is >> not defined in PAPR or supported by any AIX based VIOS. >> >> Treating as bit rot and removing the sysfs interface and associated >> host config code accordingly. > > Doesn't removing a sysfs interface potentially break userspace code? > In the general case yes, but I feel in this case no. First, Reading from this config attribute of a vscsi host adapter always returns nothing. Second, any userspace code using this attribute better be checking for the existence of config. Just a quick look for /sys/class/scsi_host/host*/config under other host adapters on my system I find that attribute doesn't exist for any of them. If there is truly enough concern that somebody may actually be accessing this useless attribute from userspace then we can still strip out the unsupported code, but leave the attribute and return nothing directly from the show function. -Tyrel -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Feb 04, 2016 at 09:48:23AM -0800, Tyrel Datwyler wrote: > On 02/04/2016 12:03 AM, Johannes Thumshirn wrote: > > On Wed, Feb 03, 2016 at 05:28:33PM -0600, Tyrel Datwyler wrote: > >> A VIOSRP_HOST_CONFIG_TYPE management datagram (MAD) has existed in > >> the code for some time. From what information I've gathered from > >> Brian King this was likely implemented on the host side in a SLES 9 > >> based VIOS, which is no longer supported anywhere. Further, it is > >> not defined in PAPR or supported by any AIX based VIOS. > >> > >> Treating as bit rot and removing the sysfs interface and associated > >> host config code accordingly. > > > > Doesn't removing a sysfs interface potentially break userspace code? > > > > In the general case yes, but I feel in this case no. First, Reading from > this config attribute of a vscsi host adapter always returns nothing. > Second, any userspace code using this attribute better be checking for > the existence of config. Just a quick look for > /sys/class/scsi_host/host*/config under other host adapters on my system > I find that attribute doesn't exist for any of them. > > If there is truly enough concern that somebody may actually be accessing > this useless attribute from userspace then we can still strip out the > unsupported code, but leave the attribute and return nothing directly > from the show function. Which is what I kinda prefer. Slowly deprecate and phase out, but don't break userspace. You never know who is writing some obscure piece of code relaying on some sysfs attribute. Thanks, Johannes
diff --git a/drivers/scsi/ibmvscsi/ibmvscsi.c b/drivers/scsi/ibmvscsi/ibmvscsi.c index 47cfe33..a2b8db1 100644 --- a/drivers/scsi/ibmvscsi/ibmvscsi.c +++ b/drivers/scsi/ibmvscsi/ibmvscsi.c @@ -1853,62 +1853,6 @@ static void ibmvscsi_handle_crq(struct viosrp_crq *crq, } /** - * ibmvscsi_get_host_config: Send the command to the server to get host - * configuration data. The data is opaque to us. - */ -static int ibmvscsi_do_host_config(struct ibmvscsi_host_data *hostdata, - unsigned char *buffer, int length) -{ - struct viosrp_host_config *host_config; - struct srp_event_struct *evt_struct; - unsigned long flags; - dma_addr_t addr; - int rc; - - evt_struct = get_event_struct(&hostdata->pool); - if (!evt_struct) { - dev_err(hostdata->dev, "couldn't allocate event for HOST_CONFIG!\n"); - return -1; - } - - init_event_struct(evt_struct, - sync_completion, - VIOSRP_MAD_FORMAT, - info_timeout); - - host_config = &evt_struct->iu.mad.host_config; - - /* The transport length field is only 16-bit */ - length = min(0xffff, length); - - /* Set up a lun reset SRP command */ - memset(host_config, 0x00, sizeof(*host_config)); - host_config->common.type = cpu_to_be32(VIOSRP_HOST_CONFIG_TYPE); - host_config->common.length = cpu_to_be16(length); - addr = dma_map_single(hostdata->dev, buffer, length, DMA_BIDIRECTIONAL); - - if (dma_mapping_error(hostdata->dev, addr)) { - if (!firmware_has_feature(FW_FEATURE_CMO)) - dev_err(hostdata->dev, - "dma_mapping error getting host config\n"); - free_event_struct(&hostdata->pool, evt_struct); - return -1; - } - - host_config->buffer = cpu_to_be64(addr); - - init_completion(&evt_struct->comp); - spin_lock_irqsave(hostdata->host->host_lock, flags); - rc = ibmvscsi_send_srp_event(evt_struct, hostdata, info_timeout * 2); - spin_unlock_irqrestore(hostdata->host->host_lock, flags); - if (rc == 0) - wait_for_completion(&evt_struct->comp); - dma_unmap_single(hostdata->dev, addr, length, DMA_BIDIRECTIONAL); - - return rc; -} - -/** * ibmvscsi_slave_configure: Set the "allow_restart" flag for each disk. * @sdev: struct scsi_device device to configure * @@ -2090,27 +2034,6 @@ static struct device_attribute ibmvscsi_host_os_type = { .show = show_host_os_type, }; -static ssize_t show_host_config(struct device *dev, - struct device_attribute *attr, char *buf) -{ - struct Scsi_Host *shost = class_to_shost(dev); - struct ibmvscsi_host_data *hostdata = shost_priv(shost); - - /* returns null-terminated host config data */ - if (ibmvscsi_do_host_config(hostdata, buf, PAGE_SIZE) == 0) - return strlen(buf); - else - return 0; -} - -static struct device_attribute ibmvscsi_host_config = { - .attr = { - .name = "config", - .mode = S_IRUGO, - }, - .show = show_host_config, -}; - static struct device_attribute *ibmvscsi_attrs[] = { &ibmvscsi_host_vhost_loc, &ibmvscsi_host_vhost_name, @@ -2119,7 +2042,6 @@ static struct device_attribute *ibmvscsi_attrs[] = { &ibmvscsi_host_partition_number, &ibmvscsi_host_mad_version, &ibmvscsi_host_os_type, - &ibmvscsi_host_config, NULL }; diff --git a/drivers/scsi/ibmvscsi/viosrp.h b/drivers/scsi/ibmvscsi/viosrp.h index da9bb29..04bae63 100644 --- a/drivers/scsi/ibmvscsi/viosrp.h +++ b/drivers/scsi/ibmvscsi/viosrp.h @@ -96,7 +96,6 @@ enum viosrp_mad_types { VIOSRP_EMPTY_IU_TYPE = 0x01, VIOSRP_ERROR_LOG_TYPE = 0x02, VIOSRP_ADAPTER_INFO_TYPE = 0x03, - VIOSRP_HOST_CONFIG_TYPE = 0x04, VIOSRP_CAPABILITIES_TYPE = 0x05, VIOSRP_ENABLE_FAST_FAIL = 0x08, }; @@ -162,11 +161,6 @@ struct viosrp_adapter_info { __be64 buffer; }; -struct viosrp_host_config { - struct mad_common common; - __be64 buffer; -}; - struct viosrp_fast_fail { struct mad_common common; }; @@ -204,7 +198,6 @@ union mad_iu { struct viosrp_empty_iu empty_iu; struct viosrp_error_log error_log; struct viosrp_adapter_info adapter_info; - struct viosrp_host_config host_config; struct viosrp_fast_fail fast_fail; struct viosrp_capabilities capabilities; };
A VIOSRP_HOST_CONFIG_TYPE management datagram (MAD) has existed in the code for some time. From what information I've gathered from Brian King this was likely implemented on the host side in a SLES 9 based VIOS, which is no longer supported anywhere. Further, it is not defined in PAPR or supported by any AIX based VIOS. Treating as bit rot and removing the sysfs interface and associated host config code accordingly. Signed-off-by: Tyrel Datwyler <tyreld@linux.vnet.ibm.com> --- drivers/scsi/ibmvscsi/ibmvscsi.c | 78 ---------------------------------------- drivers/scsi/ibmvscsi/viosrp.h | 7 ---- 2 files changed, 85 deletions(-)