Re: [PATCH 1/4] scsi: remove -change_queue_type method

2014-12-04 Thread Christoph Hellwig
On Wed, Dec 03, 2014 at 02:32:04PM +0100, Hannes Reinecke wrote:
 Why not setting the attribute to S_IRUGO always and drop the 'store'
 method altogether?

For now I'm printing a warning if someone writes to it.  I the long
run we can simply mark it r/o.

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


Re: [PATCH 1/4] scsi: remove -change_queue_type method

2014-12-03 Thread Hannes Reinecke
On 11/24/2014 03:36 PM, Christoph Hellwig wrote:
 Since we got rid of ordered tag support in 2010 the prime use case of
 switching on and off ordered tags has been obsolete.  The other function
 of enabling/dsiabling tagging entirely has only been correctly implemented
 by the 53c700 driver and isn't generally useful.
 
 Signed-off-by: Christoph Hellwig h...@lst.de
 ---
  drivers/infiniband/ulp/srp/ib_srp.c  |  1 -
  drivers/scsi/53c700.c| 35 ---
  drivers/scsi/aic94xx/aic94xx_init.c  |  1 -
  drivers/scsi/bnx2fc/bnx2fc_fcoe.c|  1 -
  drivers/scsi/esas2r/esas2r_main.c|  1 -
  drivers/scsi/fcoe/fcoe.c |  1 -
  drivers/scsi/fnic/fnic_main.c|  1 -
  drivers/scsi/ibmvscsi/ibmvfc.c   |  1 -
  drivers/scsi/ipr.c   | 25 -
  drivers/scsi/isci/init.c |  1 -
  drivers/scsi/libsas/sas_scsi_host.c  |  8 
  drivers/scsi/lpfc/lpfc_scsi.c|  2 --
  drivers/scsi/mpt2sas/mpt2sas_scsih.c |  1 -
  drivers/scsi/mpt3sas/mpt3sas_scsih.c |  1 -
  drivers/scsi/mvsas/mv_init.c |  1 -
  drivers/scsi/pm8001/pm8001_init.c|  1 -
  drivers/scsi/pmcraid.c   |  1 -
  drivers/scsi/qla2xxx/qla_os.c|  1 -
  drivers/scsi/scsi.c  | 16 
  drivers/scsi/scsi_debug.c| 27 ---
  drivers/scsi/scsi_sysfs.c| 30 --
  drivers/target/loopback/tcm_loop.c   |  1 -
  include/scsi/libsas.h|  1 -
  include/scsi/scsi_host.h | 13 -
  include/scsi/scsi_tcq.h  |  3 ---
  25 files changed, 4 insertions(+), 171 deletions(-)
 

[ .. ]

 diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
 index 1cb64a8..1ac38e7 100644
 --- a/drivers/scsi/scsi_sysfs.c
 +++ b/drivers/scsi/scsi_sysfs.c
 @@ -738,30 +738,12 @@ store_queue_type_field(struct device *dev, struct 
 device_attribute *attr,
  const char *buf, size_t count)
  {
   struct scsi_device *sdev = to_scsi_device(dev);
 - struct scsi_host_template *sht = sdev-host-hostt;
 - int tag_type = 0, retval;
 - int prev_tag_type = scsi_get_tag_type(sdev);
 -
 - if (!sdev-tagged_supported || !sht-change_queue_type)
 - return -EINVAL;
  
 - /*
 -  * We're never issueing order tags these days, but allow the value
 -  * for backwards compatibility.
 -  */
 - if (strncmp(buf, ordered, 7) == 0 ||
 - strncmp(buf, simple, 6) == 0)
 - tag_type = MSG_SIMPLE_TAG;
 - else if (strncmp(buf, none, 4) != 0)
 + if (!sdev-tagged_supported)
   return -EINVAL;
 -
 - if (tag_type == prev_tag_type)
 - return count;
 -
 - retval = sht-change_queue_type(sdev, tag_type);
 - if (retval  0)
 - return retval;
 -
 + 
 + sdev_printk(KERN_INFO, sdev,
 + ignoring write to deprecated queue_type attribute);
   return count;
  }
  
 @@ -938,10 +920,6 @@ static umode_t scsi_sdev_attr_is_visible(struct kobject 
 *kobj,
   !sdev-host-hostt-change_queue_depth)
   return 0;
  
 - if (attr == dev_attr_queue_type.attr 
 - !sdev-host-hostt-change_queue_type)
 - return S_IRUGO;
 -
   return attr-mode;
  }
  
Why not setting the attribute to S_IRUGO always and drop the 'store'
method altogether?

Otherwise:

Reviewed-by: Hannes Reinecke h...@suse.de

Cheers,

Hannes
-- 
Dr. Hannes Reinecke   zSeries  Storage
h...@suse.de  +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 21284 (AG Nürnberg)
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/4] scsi: remove -change_queue_type method

2014-12-03 Thread Martin K. Petersen
 Christoph == Christoph Hellwig h...@lst.de writes:

Christoph Since we got rid of ordered tag support in 2010 the prime use
Christoph case of switching on and off ordered tags has been obsolete.
Christoph The other function of enabling/dsiabling tagging entirely has
Christoph only been correctly implemented by the 53c700 driver and
Christoph isn't generally useful.

s/dsiabling/disabling/

Reviewed-by: Martin K. Petersen martin.peter...@oracle.com

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


Re: [PATCH 1/4] scsi: remove -change_queue_type method

2014-11-28 Thread Bart Van Assche

On 11/24/14 15:36, Christoph Hellwig wrote:

Since we got rid of ordered tag support in 2010 the prime use case of
switching on and off ordered tags has been obsolete.  The other function
of enabling/dsiabling tagging entirely has only been correctly implemented
by the 53c700 driver and isn't generally useful.


Reviewed-by: Bart Van Assche bvanass...@acm.org

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


[PATCH 1/4] scsi: remove -change_queue_type method

2014-11-24 Thread Christoph Hellwig
Since we got rid of ordered tag support in 2010 the prime use case of
switching on and off ordered tags has been obsolete.  The other function
of enabling/dsiabling tagging entirely has only been correctly implemented
by the 53c700 driver and isn't generally useful.

Signed-off-by: Christoph Hellwig h...@lst.de
---
 drivers/infiniband/ulp/srp/ib_srp.c  |  1 -
 drivers/scsi/53c700.c| 35 ---
 drivers/scsi/aic94xx/aic94xx_init.c  |  1 -
 drivers/scsi/bnx2fc/bnx2fc_fcoe.c|  1 -
 drivers/scsi/esas2r/esas2r_main.c|  1 -
 drivers/scsi/fcoe/fcoe.c |  1 -
 drivers/scsi/fnic/fnic_main.c|  1 -
 drivers/scsi/ibmvscsi/ibmvfc.c   |  1 -
 drivers/scsi/ipr.c   | 25 -
 drivers/scsi/isci/init.c |  1 -
 drivers/scsi/libsas/sas_scsi_host.c  |  8 
 drivers/scsi/lpfc/lpfc_scsi.c|  2 --
 drivers/scsi/mpt2sas/mpt2sas_scsih.c |  1 -
 drivers/scsi/mpt3sas/mpt3sas_scsih.c |  1 -
 drivers/scsi/mvsas/mv_init.c |  1 -
 drivers/scsi/pm8001/pm8001_init.c|  1 -
 drivers/scsi/pmcraid.c   |  1 -
 drivers/scsi/qla2xxx/qla_os.c|  1 -
 drivers/scsi/scsi.c  | 16 
 drivers/scsi/scsi_debug.c| 27 ---
 drivers/scsi/scsi_sysfs.c| 30 --
 drivers/target/loopback/tcm_loop.c   |  1 -
 include/scsi/libsas.h|  1 -
 include/scsi/scsi_host.h | 13 -
 include/scsi/scsi_tcq.h  |  3 ---
 25 files changed, 4 insertions(+), 171 deletions(-)

diff --git a/drivers/infiniband/ulp/srp/ib_srp.c 
b/drivers/infiniband/ulp/srp/ib_srp.c
index 5461924..a164036 100644
--- a/drivers/infiniband/ulp/srp/ib_srp.c
+++ b/drivers/infiniband/ulp/srp/ib_srp.c
@@ -2740,7 +2740,6 @@ static struct scsi_host_template srp_template = {
.info   = srp_target_info,
.queuecommand   = srp_queuecommand,
.change_queue_depth = srp_change_queue_depth,
-   .change_queue_type  = scsi_change_queue_type,
.eh_abort_handler   = srp_abort,
.eh_device_reset_handler= srp_reset_device,
.eh_host_reset_handler  = srp_reset_host,
diff --git a/drivers/scsi/53c700.c b/drivers/scsi/53c700.c
index aa915da..e722911 100644
--- a/drivers/scsi/53c700.c
+++ b/drivers/scsi/53c700.c
@@ -176,7 +176,6 @@ STATIC int NCR_700_slave_alloc(struct scsi_device *SDpnt);
 STATIC int NCR_700_slave_configure(struct scsi_device *SDpnt);
 STATIC void NCR_700_slave_destroy(struct scsi_device *SDpnt);
 static int NCR_700_change_queue_depth(struct scsi_device *SDpnt, int depth);
-static int NCR_700_change_queue_type(struct scsi_device *SDpnt, int depth);
 
 STATIC struct device_attribute *NCR_700_dev_attrs[];
 
@@ -326,7 +325,6 @@ NCR_700_detect(struct scsi_host_template *tpnt,
tpnt-slave_destroy = NCR_700_slave_destroy;
tpnt-slave_alloc = NCR_700_slave_alloc;
tpnt-change_queue_depth = NCR_700_change_queue_depth;
-   tpnt-change_queue_type = NCR_700_change_queue_type;
tpnt-use_blk_tags = 1;
 
if(tpnt-name == NULL)
@@ -2082,39 +2080,6 @@ NCR_700_change_queue_depth(struct scsi_device *SDp, int 
depth)
return scsi_change_queue_depth(SDp, depth);
 }
 
-static int NCR_700_change_queue_type(struct scsi_device *SDp, int tag_type)
-{
-   int change_tag = ((tag_type ==0   scsi_get_tag_type(SDp) != 0)
- || (tag_type != 0  scsi_get_tag_type(SDp) == 0));
-   struct NCR_700_Host_Parameters *hostdata = 
-   (struct NCR_700_Host_Parameters *)SDp-host-hostdata[0];
-
-   /* We have a global (per target) flag to track whether TCQ is
-* enabled, so we'll be turning it off for the entire target here.
-* our tag algorithm will fail if we mix tagged and untagged commands,
-* so quiesce the device before doing this */
-   if (change_tag)
-   scsi_target_quiesce(SDp-sdev_target);
-
-   scsi_set_tag_type(SDp, tag_type);
-   if (!tag_type) {
-   /* shift back to the default unqueued number of commands
-* (the user can still raise this) */
-   scsi_change_queue_depth(SDp, SDp-host-cmd_per_lun);
-   hostdata-tag_negotiated = ~(1  sdev_id(SDp));
-   } else {
-   /* Here, we cleared the negotiation flag above, so this
-* will force the driver to renegotiate */
-   scsi_change_queue_depth(SDp, SDp-queue_depth);
-   if (change_tag)
-   NCR_700_set_tag_neg_state(SDp, 
NCR_700_START_TAG_NEGOTIATION);
-   }
-   if (change_tag)
-   scsi_target_resume(SDp-sdev_target);
-
-   return tag_type;
-}
-
 static ssize_t
 NCR_700_show_active_tags(struct device *dev, struct device_attribute *attr, 
char *buf)
 {