Re: [PATCH v2 00/36] Fixes, cleanups and modernization for NCR5380 drivers

2014-11-05 Thread Geert Uytterhoeven
On Wed, Nov 5, 2014 at 8:56 AM, David Gálvez  wrote:
> Do you know about the Falcon's disturbance in the SDMA clock signal
> hardware problem?
> Most Falcons, specially those used in music studios, have a hardware
> patch to fix this, it's normally called SCSI patch.
>
> Some more info:
>
> http://didierm.pagesperso-orange.fr/doc/eng/c_0a.htm

So this adds additional buffering to the clock.

Note that input pins 3 and 5 of the 74HC04 are left floating!
I recommend tying them to either GND (pin 7) or VCC (pin 14), to avoid
them picking up high-frequency signals and consuming power.

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
--
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: [V2 PATCH 2/2] scsi: TUR path is down after adapter gets reset with multipath

2014-11-05 Thread Hannes Reinecke
On 11/04/2014 04:01 PM, Christoph Hellwig wrote:
> On Tue, Nov 04, 2014 at 08:21:13AM +0100, Hannes Reinecke wrote:
>> Why did you use a wrapper for an already existing function?
>> Please fold the logic into alua_check_sense().
> 
> That's what the first version did.  See the response from Rob to it
> on why it's done this way.

Hmm. I see. But I still thing it should be wrapped into
alua_check_sense().
The main objection from Rob was that a sense code of 02/04/02 should
cause a START STOP UNIT to be sent, but the latter is might be
invalid for certain ALUA states.
Thing is, we're only sending START STOP UNIT as part of the main
SCSI EH routine, so that would work in either way.

And looking a scsi_dh_alua we're only checking a return value of
ADD_TO_MLQUEUE; any other value is treated as an I/O error.
So it doesn't really matter if alua_check_sense would return
FAILED here; it still would be folded into the final return value of
SCSI_DH_IO.

So I don't think we need to have a wrapper around alua_check_sense()
but can rather merge them both.

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 v2 12/12] IB/srp: Add multichannel support

2014-11-05 Thread Sagi Grimberg

On 11/5/2014 6:57 AM, Elliott, Robert (Server Storage) wrote:




-Original Message-
From: Sagi Grimberg [mailto:sa...@dev.mellanox.co.il]
Sent: Tuesday, November 04, 2014 6:15 AM
To: Bart Van Assche; Elliott, Robert (Server Storage); Christoph Hellwig
Cc: Jens Axboe; Sagi Grimberg; Sebastian Parschauer; Ming Lei; linux-
s...@vger.kernel.org; linux-rdma
Subject: Re: [PATCH v2 12/12] IB/srp: Add multichannel support


...

I think that Rob and I are not talking about the same issue. In
case only a single core is servicing interrupts it is indeed expected
that it will spend 100% in hard-irq, that's acceptable since it is
pounded with completions all the time.

However, I'm referring to a condition where SRP will spend infinite
time servicing a single interrupt (while loop on ib_poll_cq that never
drains) which will lead to a hard lockup.

This *can* happen, and I do believe that with an optimized IO path
it is even more likely to.


If the IB completions/interrupts are only for IOs submitted on this
CPU, then the CQ will eventually drain, because this CPU is not
submitting anything new while stuck in the loop.


They're not (or not necessarily). I'm talking about the case where the
IO completions are submitted from another CPU. This creates a cycle
where the submitter is generating completions on CPU X and the completer
is evacuating room for more submissions on CPU Y. This process can
never end while the completer is in hard-irq context.



This can become bursty, though - submit a lot of IOs, then be busy
completing all of them and not submitting more, resulting in the
queue depth bouncing from 0 to high to 0 to high.  I've seen
that with both hpsa and mpt3sas drivers.  The fio options
iodepth_batch, iodepth_batch_complete, and iodepth_low
can amplify and reduce that effect (using libaio).



blk-iopoll (or some other form of budgeting completions) should take
care of that.

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


[PATCHv2 0/3] Initial ZAC support

2014-11-05 Thread Hannes Reinecke
Hi all,

this patchset updates the ATA stack to correctly detect
host-managed ZAC devices.

Hannes Reinecke (3):
  libsas: use ata_dev_classify()
  libata: Implement ATA_DEV_ZAC
  libata-scsi: Update SATL for ZAC drives

 drivers/ata/libata-core.c   | 20 +++
 drivers/ata/libata-eh.c |  7 ++--
 drivers/ata/libata-scsi.c   | 30 +++--
 drivers/ata/libata-transport.c  |  1 +
 drivers/scsi/aic94xx/aic94xx_task.c | 10 +++---
 drivers/scsi/isci/request.c |  4 +--
 drivers/scsi/libsas/sas_ata.c   | 66 +++--
 drivers/scsi/mvsas/mv_sas.c |  4 +--
 drivers/scsi/pm8001/pm8001_hwi.c|  2 +-
 drivers/scsi/pm8001/pm80xx_hwi.c|  2 +-
 include/linux/libata.h  |  6 ++--
 include/scsi/libsas.h   | 11 ++-
 12 files changed, 76 insertions(+), 87 deletions(-)

-- 
1.8.5.2

--
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 2/3] libata: Implement ATA_DEV_ZAC

2014-11-05 Thread Hannes Reinecke
Add new ATA device type for ZAC devices.

Acked-by: Christoph Hellwig 
Acked-by: Tejun Heo 
Signed-off-by: Hannes Reinecke 
---
 drivers/ata/libata-core.c  | 20 ++--
 drivers/ata/libata-eh.c|  7 +--
 drivers/ata/libata-scsi.c  |  5 +++--
 drivers/ata/libata-transport.c |  1 +
 include/linux/libata.h |  6 --
 5 files changed, 27 insertions(+), 12 deletions(-)

diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index c5ba15a..5c84fb5 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -1043,8 +1043,8 @@ const char *sata_spd_string(unsigned int spd)
  * None.
  *
  * RETURNS:
- * Device type, %ATA_DEV_ATA, %ATA_DEV_ATAPI, %ATA_DEV_PMP or
- * %ATA_DEV_UNKNOWN the event of failure.
+ * Device type, %ATA_DEV_ATA, %ATA_DEV_ATAPI, %ATA_DEV_PMP,
+ * %ATA_DEV_ZAC, or %ATA_DEV_UNKNOWN the event of failure.
  */
 unsigned int ata_dev_classify(const struct ata_taskfile *tf)
 {
@@ -1089,6 +1089,11 @@ unsigned int ata_dev_classify(const struct ata_taskfile 
*tf)
return ATA_DEV_SEMB;
}
 
+   if ((tf->lbam == 0xcd) && (tf->lbah == 0xab)) {
+   DPRINTK("found ZAC device by sig\n");
+   return ATA_DEV_ZAC;
+   }
+
DPRINTK("unknown device\n");
return ATA_DEV_UNKNOWN;
 }
@@ -1329,7 +1334,7 @@ static int ata_hpa_resize(struct ata_device *dev)
int rc;
 
/* do we need to do it? */
-   if (dev->class != ATA_DEV_ATA ||
+   if ((dev->class != ATA_DEV_ATA && dev->class != ATA_DEV_ZAC) ||
!ata_id_has_lba(dev->id) || !ata_id_hpa_enabled(dev->id) ||
(dev->horkage & ATA_HORKAGE_BROKEN_HPA))
return 0;
@@ -1889,6 +1894,7 @@ retry:
case ATA_DEV_SEMB:
class = ATA_DEV_ATA;/* some hard drives report SEMB sig */
case ATA_DEV_ATA:
+   case ATA_DEV_ZAC:
tf.command = ATA_CMD_ID_ATA;
break;
case ATA_DEV_ATAPI:
@@ -1980,7 +1986,7 @@ retry:
rc = -EINVAL;
reason = "device reports invalid type";
 
-   if (class == ATA_DEV_ATA) {
+   if (class == ATA_DEV_ATA || class == ATA_DEV_ZAC) {
if (!ata_id_is_ata(id) && !ata_id_is_cfa(id))
goto err_out;
if (ap->host->flags & ATA_HOST_IGNORE_ATA &&
@@ -2015,7 +2021,8 @@ retry:
goto retry;
}
 
-   if ((flags & ATA_READID_POSTRESET) && class == ATA_DEV_ATA) {
+   if ((flags & ATA_READID_POSTRESET) &&
+   (class == ATA_DEV_ATA || class == ATA_DEV_ZAC)) {
/*
 * The exact sequence expected by certain pre-ATA4 drives is:
 * SRST RESET
@@ -2280,7 +2287,7 @@ int ata_dev_configure(struct ata_device *dev)
sizeof(modelbuf));
 
/* ATA-specific feature tests */
-   if (dev->class == ATA_DEV_ATA) {
+   if (dev->class == ATA_DEV_ATA || dev->class == ATA_DEV_ZAC) {
if (ata_id_is_cfa(id)) {
/* CPRM may make this media unusable */
if (id[ATA_ID_CFA_KEY_MGMT] & 1)
@@ -4033,6 +4040,7 @@ int ata_dev_revalidate(struct ata_device *dev, unsigned 
int new_class,
if (ata_class_enabled(new_class) &&
new_class != ATA_DEV_ATA &&
new_class != ATA_DEV_ATAPI &&
+   new_class != ATA_DEV_ZAC &&
new_class != ATA_DEV_SEMB) {
ata_dev_info(dev, "class mismatch %u != %u\n",
 dev->class, new_class);
diff --git a/drivers/ata/libata-eh.c b/drivers/ata/libata-eh.c
index dad83df..3dbec89 100644
--- a/drivers/ata/libata-eh.c
+++ b/drivers/ata/libata-eh.c
@@ -1809,6 +1809,7 @@ static unsigned int ata_eh_analyze_tf(struct 
ata_queued_cmd *qc,
 
switch (qc->dev->class) {
case ATA_DEV_ATA:
+   case ATA_DEV_ZAC:
if (err & ATA_ICRC)
qc->err_mask |= AC_ERR_ATA_BUS;
if (err & (ATA_UNC | ATA_AMNF))
@@ -3792,7 +3793,8 @@ int ata_eh_recover(struct ata_port *ap, ata_prereset_fn_t 
prereset,
struct ata_eh_context *ehc = &link->eh_context;
unsigned long tmp;
 
-   if (dev->class != ATA_DEV_ATA)
+   if (dev->class != ATA_DEV_ATA &&
+   dev->class != ATA_DEV_ZAC)
continue;
if (!(ehc->i.dev_action[dev->devno] &
  ATA_EH_PARK))
@@ -3873,7 +3875,8 @@ int ata_eh_recover(struct ata_port *ap, ata_prereset_fn_t 
prereset,
 
/* retry flush if necessary */
ata_for_each_dev(dev, link, ALL) {
-   if (dev->class != ATA_DEV_ATA)
+   if (dev->class != ATA_DEV_ATA &&
+   dev->

[PATCH 1/3] libsas: use ata_dev_classify()

2014-11-05 Thread Hannes Reinecke
Use the ata device class from libata in libsas instead of checking
the supported command set and switch to using ata_dev_classify()
instead of our own method.

Cc: Tejun Heo 
Cc: Dan Williams 
Acked-by: Christoph Hellwig 
Signed-off-by: Hannes Reinecke 
---
 drivers/scsi/aic94xx/aic94xx_task.c | 10 +++---
 drivers/scsi/isci/request.c |  4 +--
 drivers/scsi/libsas/sas_ata.c   | 66 +++--
 drivers/scsi/mvsas/mv_sas.c |  4 +--
 drivers/scsi/pm8001/pm8001_hwi.c|  2 +-
 drivers/scsi/pm8001/pm80xx_hwi.c|  2 +-
 include/scsi/libsas.h   | 11 ++-
 7 files changed, 25 insertions(+), 74 deletions(-)

diff --git a/drivers/scsi/aic94xx/aic94xx_task.c 
b/drivers/scsi/aic94xx/aic94xx_task.c
index 59b86e2..7abbe42 100644
--- a/drivers/scsi/aic94xx/aic94xx_task.c
+++ b/drivers/scsi/aic94xx/aic94xx_task.c
@@ -373,10 +373,10 @@ static int asd_build_ata_ascb(struct asd_ascb *ascb, 
struct sas_task *task,
 
if (unlikely(task->ata_task.device_control_reg_update))
scb->header.opcode = CONTROL_ATA_DEV;
-   else if (dev->sata_dev.command_set == ATA_COMMAND_SET)
-   scb->header.opcode = INITIATE_ATA_TASK;
-   else
+   else if (dev->sata_dev.class == ATA_DEV_ATAPI)
scb->header.opcode = INITIATE_ATAPI_TASK;
+   else
+   scb->header.opcode = INITIATE_ATA_TASK;
 
scb->ata_task.proto_conn_rate = (1 << 5); /* STP */
if (dev->port->oob_mode == SAS_OOB_MODE)
@@ -387,7 +387,7 @@ static int asd_build_ata_ascb(struct asd_ascb *ascb, struct 
sas_task *task,
if (likely(!task->ata_task.device_control_reg_update))
scb->ata_task.fis.flags |= 0x80; /* C=1: update ATA cmd reg */
scb->ata_task.fis.flags &= 0xF0; /* PM_PORT field shall be 0 */
-   if (dev->sata_dev.command_set == ATAPI_COMMAND_SET)
+   if (dev->sata_dev.class == ATA_DEV_ATAPI)
memcpy(scb->ata_task.atapi_packet, task->ata_task.atapi_packet,
   16);
scb->ata_task.sister_scb = cpu_to_le16(0x);
@@ -399,7 +399,7 @@ static int asd_build_ata_ascb(struct asd_ascb *ascb, struct 
sas_task *task,
if (task->ata_task.dma_xfer)
flags |= DATA_XFER_MODE_DMA;
if (task->ata_task.use_ncq &&
-   dev->sata_dev.command_set != ATAPI_COMMAND_SET)
+   dev->sata_dev.class != ATA_DEV_ATAPI)
flags |= ATA_Q_TYPE_NCQ;
flags |= data_dir_flags[task->data_dir];
scb->ata_task.ata_flags = flags;
diff --git a/drivers/scsi/isci/request.c b/drivers/scsi/isci/request.c
index 56e3809..cfd0084 100644
--- a/drivers/scsi/isci/request.c
+++ b/drivers/scsi/isci/request.c
@@ -694,7 +694,7 @@ sci_io_request_construct_sata(struct isci_request *ireq,
}
 
/* ATAPI */
-   if (dev->sata_dev.command_set == ATAPI_COMMAND_SET &&
+   if (dev->sata_dev.class == ATA_DEV_ATAPI &&
task->ata_task.fis.command == ATA_CMD_PACKET) {
sci_atapi_construct(ireq);
return SCI_SUCCESS;
@@ -2980,7 +2980,7 @@ static void sci_request_started_state_enter(struct 
sci_base_state_machine *sm)
state = SCI_REQ_SMP_WAIT_RESP;
} else if (task && sas_protocol_ata(task->task_proto) &&
   !task->ata_task.use_ncq) {
-   if (dev->sata_dev.command_set == ATAPI_COMMAND_SET &&
+   if (dev->sata_dev.class == ATA_DEV_ATAPI &&
task->ata_task.fis.command == ATA_CMD_PACKET) {
state = SCI_REQ_ATAPI_WAIT_H2D;
} else if (task->data_dir == DMA_NONE) {
diff --git a/drivers/scsi/libsas/sas_ata.c b/drivers/scsi/libsas/sas_ata.c
index 766098a..9f68db7 100644
--- a/drivers/scsi/libsas/sas_ata.c
+++ b/drivers/scsi/libsas/sas_ata.c
@@ -138,7 +138,7 @@ static void sas_ata_task_done(struct sas_task *task)
 
if (stat->stat == SAS_PROTO_RESPONSE || stat->stat == SAM_STAT_GOOD ||
((stat->stat == SAM_STAT_CHECK_CONDITION &&
- dev->sata_dev.command_set == ATAPI_COMMAND_SET))) {
+ dev->sata_dev.class == ATA_DEV_ATAPI))) {
memcpy(dev->sata_dev.fis, resp->ending_fis, ATA_RESP_FIS_SIZE);
 
if (!link->sactive) {
@@ -278,7 +278,7 @@ static struct sas_internal *dev_to_sas_internal(struct 
domain_device *dev)
return to_sas_internal(dev->port->ha->core.shost->transportt);
 }
 
-static void sas_get_ata_command_set(struct domain_device *dev);
+static int sas_get_ata_command_set(struct domain_device *dev);
 
 int sas_get_ata_info(struct domain_device *dev, struct ex_phy *phy)
 {
@@ -303,8 +303,7 @@ int sas_get_ata_info(struct domain_device *dev, struct 
ex_phy *phy)
}
memcpy(dev->frame_rcvd, &dev->sata_dev.rps_resp.rps.fis,
   sizeof(struct dev_to_host_fis));
-   /* TODO swit

[PATCH 3/3] libata-scsi: Update SATL for ZAC drives

2014-11-05 Thread Hannes Reinecke
ZAC (zoned-access command) drives translate into ZBC (Zoned block
command) device type for SCSI. So implement the correct mappings
into libata-scsi and update the SCSI command set versions.

Acked-by: Christoph Hellwig 
Acked-by: Tejun Heo 
Signed-off-by: Hannes Reinecke 
---
 drivers/ata/libata-scsi.c | 25 -
 1 file changed, 24 insertions(+), 1 deletion(-)

diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index bea6e7f..1db6eab 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -1969,6 +1969,7 @@ static void ata_scsi_rbuf_fill(struct ata_scsi_args *args,
 static unsigned int ata_scsiop_inq_std(struct ata_scsi_args *args, u8 *rbuf)
 {
const u8 versions[] = {
+   0x00,
0x60,   /* SAM-3 (no version claimed) */
 
0x03,
@@ -1977,6 +1978,20 @@ static unsigned int ata_scsiop_inq_std(struct 
ata_scsi_args *args, u8 *rbuf)
0x02,
0x60/* SPC-3 (no version claimed) */
};
+   const u8 versions_zbc[] = {
+   0x00,
+   0xA0,   /* SAM-5 (no version claimed) */
+
+   0x04,
+   0xC0,   /* SBC-3 (no version claimed) */
+
+   0x04,
+   0x60,   /* SPC-4 (no version claimed) */
+
+   0x60,
+   0x20,   /* ZBC (no version claimed) */
+   };
+
u8 hdr[] = {
TYPE_DISK,
0,
@@ -1991,6 +2006,11 @@ static unsigned int ata_scsiop_inq_std(struct 
ata_scsi_args *args, u8 *rbuf)
if (ata_id_removeable(args->id))
hdr[1] |= (1 << 7);
 
+   if (args->dev->class == ATA_DEV_ZAC) {
+   hdr[0] = TYPE_ZBC;
+   hdr[2] = 0x6; /* ZBC is defined in SPC-4 */
+   }
+
memcpy(rbuf, hdr, sizeof(hdr));
memcpy(&rbuf[8], "ATA ", 8);
ata_id_string(args->id, &rbuf[16], ATA_ID_PROD, 16);
@@ -2003,7 +2023,10 @@ static unsigned int ata_scsiop_inq_std(struct 
ata_scsi_args *args, u8 *rbuf)
if (rbuf[32] == 0 || rbuf[32] == ' ')
memcpy(&rbuf[32], "n/a ", 4);
 
-   memcpy(rbuf + 59, versions, sizeof(versions));
+   if (args->dev->class == ATA_DEV_ZAC)
+   memcpy(rbuf + 58, versions_zbc, sizeof(versions_zbc));
+   else
+   memcpy(rbuf + 58, versions, sizeof(versions));
 
return 0;
 }
-- 
1.8.5.2

--
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 v3 01/11] blk-mq: Add blk_mq_unique_tag()

2014-11-05 Thread Bart Van Assche

On 11/04/14 15:14, Christoph Hellwig wrote:

I've applied patches 1-3 to the core-for-3.19 branch.  Various srp
patches, including the first one fail to apply for me.  Can you
regenerate them against the drivers-for-3.19 branch?  Thanks!


Hello Christoph,

That's strange. I have compared the patches that are already in your 
tree with the patches I had posted myself with a diff tool. These 
patches look identical to what I had posted except for one CC tag that 
has been left out. If I try to apply the three patches that have not yet 
been included in your tree (9/11..11/11) on top the drivers-for-3.19 
branch then these patches apply fine. Anyway, I have rebased my tree on 
top of your drivers-for-3.19 branch, added a few other patches 
(including one block layer patch that has not yet been posted) and 
retested the SRP initiator driver against the traditional SCSI core and 
also against the scsi-mq core. The result can be found here: 
https://github.com/bvanassche/linux/commits/srp-multiple-hwq-v4. Can you 
please retry to apply patches 9/11..11/11 apply on top of the 
drivers-for-3.19 branch ?


Thanks,

Bart.
--
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/6] scsi: refactor scsi_reset_provider handling

2014-11-05 Thread Hannes Reinecke
On 10/30/2014 10:27 AM, Christoph Hellwig wrote:
> Pull the common code from the two callers into the function,
> and renamed it to scsi_ioctl_reset.
> 
> Signed-off-by: Christoph Hellwig 
Reviewed-by: Hannes Reinecke 

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 2/6] scsi: split scsi_nonblockable_ioctl

2014-11-05 Thread Hannes Reinecke
On 10/30/2014 10:27 AM, Christoph Hellwig wrote:
> The calling conventions for this function where bad as it could return
> -ENODEV both for a device not currently online and a not recognized ioctl.
> 
> Add a new scsi_ioctl_block_when_processing_errors function that wraps
> scsi_block_when_processing_errors with the a special case for the
> SG_SCSI_RESET ioctl command, and handle the SG_SCSI_RESET case itself
> in scsi_ioctl.  All callers of scsi_ioctl now must call the above helper
> to check for the EH state, so that the ioctl handler itself doesn't
> have to.
> 
> Reported-by: Robert Elliott 
> Signed-off-by: Christoph Hellwig 
Reviewed-by: Hannes Reinecke 

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 3/6] sd: fix up ->compat_ioctl

2014-11-05 Thread Hannes Reinecke
On 10/30/2014 10:27 AM, Christoph Hellwig wrote:
> No need to verify the passthrough ioctls, the real handler will
> take care of that.  Also make sure not to block for resets on
> O_NONBLOCK fds.
> 
> Signed-off-by: Christoph Hellwig 
Reviewed-by: Hannes Reinecke 

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 4/6] st: call scsi_set_medium_removal directly

2014-11-05 Thread Hannes Reinecke
On 10/30/2014 10:27 AM, Christoph Hellwig wrote:
> Signed-off-by: Christoph Hellwig 
> ---
>  drivers/scsi/st.c | 13 ++---
>  1 file changed, 6 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/scsi/st.c b/drivers/scsi/st.c
> index 7d2e036..e46e02b2 100644
> --- a/drivers/scsi/st.c
> +++ b/drivers/scsi/st.c
> @@ -861,17 +861,16 @@ static int set_mode_densblk(struct scsi_tape * STp, 
> struct st_modedef * STm)
>  /* Lock or unlock the drive door. Don't use when st_request allocated. */
>  static int do_door_lock(struct scsi_tape * STp, int do_lock)
>  {
> - int retval, cmd;
> + int retval;
>  
> - cmd = do_lock ? SCSI_IOCTL_DOORLOCK : SCSI_IOCTL_DOORUNLOCK;
>   DEBC_printk(STp, "%socking drive door.\n", do_lock ? "L" : "Unl");
> - retval = scsi_ioctl(STp->device, cmd, NULL);
> - if (!retval) {
> +
> + retval = scsi_set_medium_removal(STp->device,
> + do_lock ? SCSI_REMOVAL_PREVENT : SCSI_REMOVAL_ALLOW);
> + if (!retval)
>   STp->door_locked = do_lock ? ST_LOCKED_EXPLICIT : ST_UNLOCKED;
> - }
> - else {
> + else
>   STp->door_locked = ST_LOCK_FAILS;
> - }
>   return retval;
>  }
>  
> 
Reviewed-by: Hannes Reinecke 

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 5/6] osst: call scsi_set_medium_removal directly

2014-11-05 Thread Hannes Reinecke
On 10/30/2014 10:27 AM, Christoph Hellwig wrote:
> Signed-off-by: Christoph Hellwig 
> ---
>  drivers/scsi/osst.c | 13 ++---
>  1 file changed, 6 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/scsi/osst.c b/drivers/scsi/osst.c
> index b6d63d6..8c38464 100644
> --- a/drivers/scsi/osst.c
> +++ b/drivers/scsi/osst.c
> @@ -3327,19 +3327,18 @@ static int osst_write_frame(struct osst_tape * STp, 
> struct osst_request ** aSRpn
>  /* Lock or unlock the drive door. Don't use when struct osst_request 
> allocated. */
>  static int do_door_lock(struct osst_tape * STp, int do_lock)
>  {
> - int retval, cmd;
> + int retval;
>  
> - cmd = do_lock ? SCSI_IOCTL_DOORLOCK : SCSI_IOCTL_DOORUNLOCK;
>  #if DEBUG
>   printk(OSST_DEB_MSG "%s:D: %socking drive door.\n", tape_name(STp), 
> do_lock ? "L" : "Unl");
>  #endif
> - retval = scsi_ioctl(STp->device, cmd, NULL);
> - if (!retval) {
> +
> + retval = scsi_set_medium_removal(STp->device,
> + do_lock ? SCSI_REMOVAL_PREVENT : SCSI_REMOVAL_ALLOW);
> + if (!retval)
>   STp->door_locked = do_lock ? ST_LOCKED_EXPLICIT : ST_UNLOCKED;
> - }
> - else {
> + else
>   STp->door_locked = ST_LOCK_FAILS;
> - }
>   return retval;
>  }
>  
> 
Reviewed-by: Hannes Reinecke 

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 6/6] scsi: return EAGAIN when resetting a device under EH

2014-11-05 Thread Hannes Reinecke
On 10/30/2014 10:27 AM, Christoph Hellwig wrote:
> Signed-off-by: Christoph Hellwig 
> ---
>  drivers/scsi/scsi_ioctl.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/scsi/scsi_ioctl.c b/drivers/scsi/scsi_ioctl.c
> index 712f159..c4f7b56 100644
> --- a/drivers/scsi/scsi_ioctl.c
> +++ b/drivers/scsi/scsi_ioctl.c
> @@ -278,7 +278,7 @@ int scsi_ioctl_block_when_processing_errors(struct 
> scsi_device *sdev, int cmd,
>  {
>   if (cmd == SG_SCSI_RESET && ndelay) {
>   if (scsi_host_in_recovery(sdev->host))
> - return -ENODEV;
> + return -EAGAIN;
>   } else {
>   if (!scsi_block_when_processing_errors(sdev))
>   return -ENODEV;
> 
Reviewed-by: Hannes Reinecke 

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/6] scsi: refactor scsi_reset_provider handling

2014-11-05 Thread Martin K. Petersen
> "Christoph" == Christoph Hellwig  writes:

Christoph> Pull the common code from the two callers into the function,
Christoph> and renamed it to scsi_ioctl_reset.

s/renamed/rename/

Reviewed-by: Martin K. Petersen 

-- 
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 2/6] scsi: split scsi_nonblockable_ioctl

2014-11-05 Thread Martin K. Petersen
> "Christoph" == Christoph Hellwig  writes:

Christoph> The calling conventions for this function where bad as it
Christoph> could return -ENODEV both for a device not currently online
Christoph> and a not recognized ioctl.

s/where/were/

Christoph> Add a new scsi_ioctl_block_when_processing_errors function
Christoph> that wraps scsi_block_when_processing_errors with the a
Christoph> special case for the SG_SCSI_RESET ioctl command, and handle
Christoph> the SG_SCSI_RESET case itself in scsi_ioctl.  All callers of
Christoph> scsi_ioctl now must call the above helper to check for the EH
Christoph> state, so that the ioctl handler itself doesn't have to.

Nice cleanup!

Reviewed-by: Martin K. Petersen 

-- 
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 3/6] sd: fix up ->compat_ioctl

2014-11-05 Thread Martin K. Petersen
> "Christoph" == Christoph Hellwig  writes:

Christoph> No need to verify the passthrough ioctls, the real handler
Christoph> will take care of that.  Also make sure not to block for
Christoph> resets on O_NONBLOCK fds.

Reviewed-by: Martin K. Petersen 

-- 
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 4/6] st: call scsi_set_medium_removal directly

2014-11-05 Thread Martin K. Petersen
> "Christoph" == Christoph Hellwig  writes:

Reviewed-by: Martin K. Petersen 

-- 
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 5/6] osst: call scsi_set_medium_removal directly

2014-11-05 Thread Martin K. Petersen
> "Christoph" == Christoph Hellwig  writes:

Reviewed-by: Martin K. Petersen 

-- 
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 6/6] scsi: return EAGAIN when resetting a device under EH

2014-11-05 Thread Martin K. Petersen
> "Christoph" == Christoph Hellwig  writes:

Christoph> - return -ENODEV;
Christoph> + return -EAGAIN;

Reviewed-by: Martin K. Petersen 

-- 
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: [PATCHv2 0/3] Initial ZAC support

2014-11-05 Thread Christoph Hellwig
On Wed, Nov 05, 2014 at 01:08:19PM +0100, Hannes Reinecke wrote:
> Hi all,
> 
> this patchset updates the ATA stack to correctly detect
> host-managed ZAC devices.

And more importantly exports /dev/sg nodes from them so they can be
actually be accessed.

I'm fine with taking the whole series through the libata tree, so far
there are no conflicting libsas changes on the radar.
--
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: [PATCHv2 0/3] Initial ZAC support

2014-11-05 Thread Hannes Reinecke
On 11/05/2014 04:03 PM, Christoph Hellwig wrote:
> On Wed, Nov 05, 2014 at 01:08:19PM +0100, Hannes Reinecke wrote:
>> Hi all,
>>
>> this patchset updates the ATA stack to correctly detect
>> host-managed ZAC devices.
> 
> And more importantly exports /dev/sg nodes from them so they can be
> actually be accessed.
> 
> I'm fine with taking the whole series through the libata tree, so far
> there are no conflicting libsas changes on the radar.
> 
Tejun?
I've made the requested changes of using taskfiles in libsas,
so everything should be fine now ...

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: Large disk drives

2014-11-05 Thread Dale R. Worley
Replying to two messages at once:

> Date: Tue, 4 Nov 2014 11:14:39 -0500 (EST)
> From: Alan Stern 
> cc: "Dale R. Worley" , ,
> 
> 
> On Tue, 4 Nov 2014, James Bottomley wrote:
> 
> > On Mon, 2014-11-03 at 16:06 -0500, Dale R. Worley wrote:
> > > Was there any resolution as to how large disk drives would be handled
> > > if their interface did not support the "capacity" request that would
> > > tell how large they were?
> > 
> > Realistically no ... unless someone comes up with a reliable heuristic
> > to give us the size.

I can understand why the linux-scsi project would not want to take up
what is really a hack to work around a hardware deficiency.

> I posted a patch to allow the user to override the reported capacity:
> 
>   http://marc.info/?l=linux-scsi&m=140993840113445&w=2
> 
> Nobody responded to it.
> 
> > > Unfortunately, such devices work OK with Windows (since Windows trusts
> > > what the partition table says), you can't just say to the salesperson
> > > "It has to work on drives over 3 TB."
> > 
> > This is a stopgap: your 3TB drive can be guessed as the 16 bit capacity
> > plus 2TB, but the same won't happen for a 5TB device.  Believing the
> > partition table gives us a chicken and egg problem because something
> > still has to get the partition table on to the device.
> > 
> > I don't think "don't buy something that doesn't work" is a hugely
> > unreasonable response to this.
> 
> The problem is knowing beforehand whether it will work.  Once you buy 
> the device and can test it, returning it is annoying and time-consuming 
> at best.

Or as I would phrase it, How do you turn "Don't buy something that
doesn't work!" into an algorithm?  That is:  I'm standing in
MicroCenter, holding a box in my hand that contains a USB-to-SCSI
adapter.  How do I determine whether or not it supports large disks?
The problem being that it *works with Windows*, so I can't just ask
the friendly salesperson, Does this work with 3 terabyte disks?

In my case, the Diablotek device works, while the Kingwin EZ-Connect
does not, despite being labeled in essentially the same way.  (Neither
says that Linux is supported.)

I admit that my problem may be my deficiency in dealing with the
retail situation, as I'm a much better software engineer than shopper.

Dale
--
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: Large disk drives

2014-11-05 Thread James Bottomley
On Tue, 2014-11-04 at 11:14 -0500, Alan Stern wrote:
> On Tue, 4 Nov 2014, James Bottomley wrote:
> 
> > On Mon, 2014-11-03 at 16:06 -0500, Dale R. Worley wrote:
> > > Was there any resolution as to how large disk drives would be handled
> > > if their interface did not support the "capacity" request that would
> > > tell how large they were?
> > 
> > Realistically no ... unless someone comes up with a reliable heuristic
> > to give us the size.
> 
> I posted a patch to allow the user to override the reported capacity:
> 
>   http://marc.info/?l=linux-scsi&m=140993840113445&w=2
> 
> Nobody responded to it.

Sorry, meant to.  In principle I'm OK with this as the lever for the
hack (largely because it means we don't need to do anything) but will
the distributions support it?

> > > Unfortunately, such devices work OK with Windows (since Windows trusts
> > > what the partition table says), you can't just say to the salesperson
> > > "It has to work on drives over 3 TB."
> > 
> > This is a stopgap: your 3TB drive can be guessed as the 16 bit capacity
> > plus 2TB, but the same won't happen for a 5TB device.  Believing the
> > partition table gives us a chicken and egg problem because something
> > still has to get the partition table on to the device.
> > 
> > I don't think "don't buy something that doesn't work" is a hugely
> > unreasonable response to this.
> 
> The problem is knowing beforehand whether it will work.  Once you buy 
> the device and can test it, returning it is annoying and time-consuming 
> at best.

OK, but I still don't understand how windows gets the partition table on
there in the first place ... that must surely be some sort of guess the
disk size hack.

James


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


--
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: [PATCHv2 0/3] Initial ZAC support

2014-11-05 Thread Tejun Heo
On Wed, Nov 05, 2014 at 01:08:19PM +0100, Hannes Reinecke wrote:
> Hi all,
> 
> this patchset updates the ATA stack to correctly detect
> host-managed ZAC devices.
> 
> Hannes Reinecke (3):
>   libsas: use ata_dev_classify()
>   libata: Implement ATA_DEV_ZAC
>   libata-scsi: Update SATL for ZAC drives

Applied 1-3 to libata/for-3.19.

Thanks.

-- 
tejun
--
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: Large disk drives

2014-11-05 Thread Alan Stern
On Wed, 5 Nov 2014, James Bottomley wrote:

> On Tue, 2014-11-04 at 11:14 -0500, Alan Stern wrote:
> > On Tue, 4 Nov 2014, James Bottomley wrote:
> > 
> > > On Mon, 2014-11-03 at 16:06 -0500, Dale R. Worley wrote:
> > > > Was there any resolution as to how large disk drives would be handled
> > > > if their interface did not support the "capacity" request that would
> > > > tell how large they were?
> > > 
> > > Realistically no ... unless someone comes up with a reliable heuristic
> > > to give us the size.
> > 
> > I posted a patch to allow the user to override the reported capacity:
> > 
> > http://marc.info/?l=linux-scsi&m=140993840113445&w=2
> > 
> > Nobody responded to it.
> 
> Sorry, meant to.  In principle I'm OK with this as the lever for the
> hack (largely because it means we don't need to do anything) but will
> the distributions support it?

While I can't speak for the distributions, it's reasonable to assume 
that if something becomes part of the upstream kernel then they will 
include it.

> OK, but I still don't understand how windows gets the partition table on
> there in the first place ... that must surely be some sort of guess the
> disk size hack.

It's simpler than that: The drive is attached directly to the computer
(i.e., via SATA rather than USB) when the partition table is created.  
With no USB-SATA bridge chip to mess things up, there's no problem
determining the correct capacity.

Alan Stern

--
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: Large disk drives

2014-11-05 Thread Boaz Harrosh
On 11/05/2014 06:34 PM, Alan Stern wrote:
<>
> 
> It's simpler than that: The drive is attached directly to the computer
> (i.e., via SATA rather than USB) when the partition table is created.  
> With no USB-SATA bridge chip to mess things up, there's no problem
> determining the correct capacity.
> 

Right!

I think it should be very simple to, just as we send a
READ_CAPACITY16 / READ_CAPACITY32 / READ_CAPACITY64 or is
it a GET_CODE_PAGE ? whatever we send today we can also
have READ_CAPACITY_PART() which will send a READ of
sector 0 the raw PC_COMMAND way and run it through some
partition analyzer. We only need to support gpt or msdos
partitions I think in this case.

Then if READ_CAPACITY16 returns all (s) and
READ_CAPACITY32 is not supported and/or blacklisted,
then yes attempt a READ_CAPACITY_PART() which should
be sam-2 compatible.

It should not take longer than a weekend afternoon over
cup of Machha. If any one sends a bad device my way, I'll
do it. But anyone should be able to code something so simple.

> Alan Stern
> 

Cheers
Boaz

--
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 v3 01/11] blk-mq: Add blk_mq_unique_tag()

2014-11-05 Thread Christoph Hellwig
On Wed, Nov 05, 2014 at 01:37:14PM +0100, Bart Van Assche wrote:
> That's strange. I have compared the patches that are already in your tree
> with the patches I had posted myself with a diff tool. These patches look
> identical to what I had posted except for one CC tag that has been left out.
> If I try to apply the three patches that have not yet been included in your
> tree (9/11..11/11) on top the drivers-for-3.19 branch then these patches
> apply fine. Anyway, I have rebased my tree on top of your drivers-for-3.19
> branch, added a few other patches (including one block layer patch that has
> not yet been posted) and retested the SRP initiator driver against the
> traditional SCSI core and also against the scsi-mq core. The result can be
> found here: https://github.com/bvanassche/linux/commits/srp-multiple-hwq-v4.
> Can you please retry to apply patches 9/11..11/11 apply on top of the
> drivers-for-3.19 branch ?

I've pulled in the three remaining patches from the series from that
tree.  If you want me to pull in the remaining trivial srp patch as well
please give me a Reviewed-by: and I'll also pull it in.
--
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: Large disk drives

2014-11-05 Thread Theodore Ts'o
On Wed, Nov 05, 2014 at 05:07:48PM +0100, James Bottomley wrote:
> 
> OK, but I still don't understand how windows gets the partition table on
> there in the first place ... that must surely be some sort of guess the
> disk size hack.

99% of the time the partition table was provided by the drive
manufacturer; I would assume they have a way of doing that as a part
of their manufacturing processes that don't require an OS individually
running the equivalent of fdisk or gdisk on said drive.

   - Ted
--
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: tag handling refactor

2014-11-05 Thread Christoph Hellwig
I've pushed an updated branch with the review comments addressed to

git://git.infradead.org/users/hch/scsi.git tcq-rework

Thansk for all the reviews, and looking forward to the missing ones!

--
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: Large disk drives

2014-11-05 Thread Christoph Hellwig
On Wed, Nov 05, 2014 at 11:34:11AM -0500, Alan Stern wrote:
> > Sorry, meant to.  In principle I'm OK with this as the lever for the
> > hack (largely because it means we don't need to do anything) but will
> > the distributions support it?
> 
> While I can't speak for the distributions, it's reasonable to assume 
> that if something becomes part of the upstream kernel then they will 
> include it.

Btw, we do have precedence for looking at partition tables from SCSI
code with scsi_partsize(), so the layering violation of looking at
EFI labels for disks sizes wouldn't be something entirely new even
if we did it in kernel space.
--
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: [PATCHv2 net-next 0/3] RDMA/cxgb4,cxgb4vf,cxgb4i,csiostor: Cleanup macros

2014-11-05 Thread David Miller
From: Hariprasad Shenai 
Date: Tue,  4 Nov 2014 08:20:54 +0530

> It's not really the "hardware" which generates these hardware constant 
> symbolic
> macros/register defines of course, it's scripts developed by the hardware 
> team.
> Various patches have ended up changing the style of the symbolic 
> macros/register
> defines and some of them used the macros/register defines that matches the
> output of the script from the hardware team.

We've told you that we don't care what format your internal whatever uses
for these macros.

We have standards, tastes, and desires and reasons for naming macros
in a certain way in upstream kernel code.

I consider it flat out unacceptable to use macros with one letter
prefixes like "S_".  You simply should not do this.

Why?

Because you are polluting the global name space, that's why.

You should only define macros with very-likely-to-be-unique prefixes
otherwise you and some arch/* header file are going to define the same
thing.

This is not just theory, it has happened in the past.

I'm not applying this series, fixup your macros properly.  And to
repeat, complaints about how your internal tools generate things
will fall on deaf ears.

Thanks.

--
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] scsi: missing braces in scsi_extd_sense_format()

2014-11-05 Thread Dan Carpenter
There were missing curly braces so we always return the first
additional2[] string.

Fixes: 7046d2fa6dbd ('scsi: use sdev as argument for sense code printing')
Signed-off-by: Dan Carpenter 

diff --git a/drivers/scsi/constants.c b/drivers/scsi/constants.c
index a1a7fca..0cf43f6 100644
--- a/drivers/scsi/constants.c
+++ b/drivers/scsi/constants.c
@@ -1282,9 +1282,10 @@ scsi_extd_sense_format(unsigned char asc, unsigned char 
ascq, const char **fmt)
for (i = 0; additional2[i].fmt; i++) {
if (additional2[i].code1 == asc &&
ascq >= additional2[i].code2_min &&
-   ascq <= additional2[i].code2_max)
+   ascq <= additional2[i].code2_max) {
*fmt = additional2[i].fmt;
return additional2[i].str;
+   }
}
 #endif
return NULL;
--
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/2] bnx2fc: check IS_ERR() instead of NULL

2014-11-05 Thread Chad Dupuis

Looks ok.
Acked-by: Chad Dupuis 

On Tue, 4 Nov 2014, Dan Carpenter wrote:


The bnx2fc_if_create() function returns NULL on failure, it never
returns an error pointer.

Signed-off-by: Dan Carpenter 

diff --git a/drivers/scsi/bnx2fc/bnx2fc_fcoe.c 
b/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
index 79e5c94..dfed4be 100644
--- a/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
+++ b/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
@@ -1081,7 +1081,7 @@ static int bnx2fc_vport_create(struct fc_vport *vport, 
bool disabled)
mutex_unlock(&bnx2fc_dev_lock);
rtnl_unlock();

-   if (IS_ERR(vn_port)) {
+   if (!vn_port) {
printk(KERN_ERR PFX "bnx2fc_vport_create (%s) failed\n",
netdev->name);
return -EIO;


--
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 2/2] bnx2fc: fix an error code in _bnx2fc_create()

2014-11-05 Thread Chad Dupuis

Looks ok.
Acked-by: Chad Dupuis 

On Tue, 4 Nov 2014, Dan Carpenter wrote:


We should be returning an error code here instead of success.  Either
-ENODEV or -ENOMEM would work.  There is also a failure message in
printk().

Signed-off-by: Dan Carpenter 

diff --git a/drivers/scsi/bnx2fc/bnx2fc_fcoe.c 
b/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
index dfed4be..251b114 100644
--- a/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
+++ b/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
@@ -2195,6 +2195,7 @@ static int _bnx2fc_create(struct net_device *netdev,
interface = bnx2fc_interface_create(hba, netdev, fip_mode);
if (!interface) {
printk(KERN_ERR PFX "bnx2fc_interface_create failed\n");
+   rc = -ENOMEM;
goto ifput_err;
}



--
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 v5 1/7] vfs: Prepare for adding a new preadv/pwritev with user flags.

2014-11-05 Thread Milosz Tanski
Plumbing the flags argument through the vfs code so they can be passed down to
__generic_file_(read/write)_iter function that do the acctual work.

Signed-off-by: Milosz Tanski 
Reviewed-by: Christoph Hellwig 
Reviewed-by: Jeff Moyer 
---
 drivers/target/target_core_file.c |  6 +++---
 fs/nfsd/vfs.c |  4 ++--
 fs/read_write.c   | 27 +++
 fs/splice.c   |  2 +-
 include/linux/aio.h   |  2 ++
 include/linux/fs.h|  4 ++--
 6 files changed, 25 insertions(+), 20 deletions(-)

diff --git a/drivers/target/target_core_file.c 
b/drivers/target/target_core_file.c
index 7d6cdda..58d9a6d 100644
--- a/drivers/target/target_core_file.c
+++ b/drivers/target/target_core_file.c
@@ -350,9 +350,9 @@ static int fd_do_rw(struct se_cmd *cmd, struct scatterlist 
*sgl,
set_fs(get_ds());
 
if (is_write)
-   ret = vfs_writev(fd, &iov[0], sgl_nents, &pos);
+   ret = vfs_writev(fd, &iov[0], sgl_nents, &pos, 0);
else
-   ret = vfs_readv(fd, &iov[0], sgl_nents, &pos);
+   ret = vfs_readv(fd, &iov[0], sgl_nents, &pos, 0);
 
set_fs(old_fs);
 
@@ -528,7 +528,7 @@ fd_execute_write_same(struct se_cmd *cmd)
 
old_fs = get_fs();
set_fs(get_ds());
-   rc = vfs_writev(f, &iov[0], iov_num, &pos);
+   rc = vfs_writev(f, &iov[0], iov_num, &pos, 0);
set_fs(old_fs);
 
vfree(iov);
diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
index 989129e..ef01c78 100644
--- a/fs/nfsd/vfs.c
+++ b/fs/nfsd/vfs.c
@@ -872,7 +872,7 @@ __be32 nfsd_readv(struct file *file, loff_t offset, struct 
kvec *vec, int vlen,
 
oldfs = get_fs();
set_fs(KERNEL_DS);
-   host_err = vfs_readv(file, (struct iovec __user *)vec, vlen, &offset);
+   host_err = vfs_readv(file, (struct iovec __user *)vec, vlen, &offset, 
0);
set_fs(oldfs);
return nfsd_finish_read(file, count, host_err);
 }
@@ -960,7 +960,7 @@ nfsd_vfs_write(struct svc_rqst *rqstp, struct svc_fh *fhp, 
struct file *file,
 
/* Write the data. */
oldfs = get_fs(); set_fs(KERNEL_DS);
-   host_err = vfs_writev(file, (struct iovec __user *)vec, vlen, &pos);
+   host_err = vfs_writev(file, (struct iovec __user *)vec, vlen, &pos, 0);
set_fs(oldfs);
if (host_err < 0)
goto out_nfserr;
diff --git a/fs/read_write.c b/fs/read_write.c
index 7d9318c..94b2d34 100644
--- a/fs/read_write.c
+++ b/fs/read_write.c
@@ -653,7 +653,8 @@ unsigned long iov_shorten(struct iovec *iov, unsigned long 
nr_segs, size_t to)
 EXPORT_SYMBOL(iov_shorten);
 
 static ssize_t do_iter_readv_writev(struct file *filp, int rw, const struct 
iovec *iov,
-   unsigned long nr_segs, size_t len, loff_t *ppos, iter_fn_t fn)
+   unsigned long nr_segs, size_t len, loff_t *ppos, iter_fn_t fn,
+   int flags)
 {
struct kiocb kiocb;
struct iov_iter iter;
@@ -662,6 +663,7 @@ static ssize_t do_iter_readv_writev(struct file *filp, int 
rw, const struct iove
init_sync_kiocb(&kiocb, filp);
kiocb.ki_pos = *ppos;
kiocb.ki_nbytes = len;
+   kiocb.ki_rwflags = flags;
 
iov_iter_init(&iter, rw, iov, nr_segs, len);
ret = fn(&kiocb, &iter);
@@ -800,7 +802,8 @@ out:
 
 static ssize_t do_readv_writev(int type, struct file *file,
   const struct iovec __user * uvector,
-  unsigned long nr_segs, loff_t *pos)
+  unsigned long nr_segs, loff_t *pos,
+  int flags)
 {
size_t tot_len;
struct iovec iovstack[UIO_FASTIOV];
@@ -834,7 +837,7 @@ static ssize_t do_readv_writev(int type, struct file *file,
 
if (iter_fn)
ret = do_iter_readv_writev(file, type, iov, nr_segs, tot_len,
-   pos, iter_fn);
+   pos, iter_fn, flags);
else if (fnv)
ret = do_sync_readv_writev(file, iov, nr_segs, tot_len,
pos, fnv);
@@ -857,27 +860,27 @@ out:
 }
 
 ssize_t vfs_readv(struct file *file, const struct iovec __user *vec,
- unsigned long vlen, loff_t *pos)
+ unsigned long vlen, loff_t *pos, int flags)
 {
if (!(file->f_mode & FMODE_READ))
return -EBADF;
if (!(file->f_mode & FMODE_CAN_READ))
return -EINVAL;
 
-   return do_readv_writev(READ, file, vec, vlen, pos);
+   return do_readv_writev(READ, file, vec, vlen, pos, flags);
 }
 
 EXPORT_SYMBOL(vfs_readv);
 
 ssize_t vfs_writev(struct file *file, const struct iovec __user *vec,
-  unsigned long vlen, loff_t *pos)
+  unsigned long vlen, loff_t *pos, int flags)
 {
if (!(file->f_mode & FMODE_WRITE))
return -EBADF;
if

Re: Large disk drives

2014-11-05 Thread Dale R. Worley
> From: Alan Stern 

> I posted a patch to allow the user to override the reported capacity:
> 
>   http://marc.info/?l=linux-scsi&m=140993840113445&w=2

I see the patch, and I feel confident I could install it if I needed
to.  What command do I execute to "write to the capacity_override
attribute", before I do "blockdev --rereadpt /dev/sdX"?

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


[3.13.y.z extended stable] Patch "scsi: Fix error handling in SCSI_IOCTL_SEND_COMMAND" has been added to staging queue

2014-11-05 Thread Kamal Mostafa
This is a note to let you know that I have just added a patch titled

scsi: Fix error handling in SCSI_IOCTL_SEND_COMMAND

to the linux-3.13.y-queue branch of the 3.13.y.z extended stable tree 
which can be found at:

 
http://kernel.ubuntu.com/git?p=ubuntu/linux.git;a=shortlog;h=refs/heads/linux-3.13.y-queue

This patch is scheduled to be released in version 3.13.11.11.

If you, or anyone else, feels it should not be added to this tree, please 
reply to this email.

For more information about the 3.13.y.z tree, see
https://wiki.ubuntu.com/Kernel/Dev/ExtendedStable

Thanks.
-Kamal

--

>From 3090e28b02d0f4c80f3db2547f0e239fc2ef3a8a Mon Sep 17 00:00:00 2001
From: Jan Kara 
Date: Wed, 22 Oct 2014 20:13:39 -0600
Subject: scsi: Fix error handling in SCSI_IOCTL_SEND_COMMAND

commit 84ce0f0e94ac97217398b3b69c21c7a62ebeed05 upstream.

When sg_scsi_ioctl() fails to prepare request to submit in
blk_rq_map_kern() we jump to a label where we just end up copying
(luckily zeroed-out) kernel buffer to userspace instead of reporting
error. Fix the problem by jumping to the right label.

CC: Jens Axboe 
CC: linux-scsi@vger.kernel.org
Coverity-id: 1226871
Signed-off-by: Jan Kara 

Fixed up the, now unused, out label.

Signed-off-by: Jens Axboe 
Signed-off-by: Kamal Mostafa 
---
 block/scsi_ioctl.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/block/scsi_ioctl.c b/block/scsi_ioctl.c
index 625e3e4..91cceb2 100644
--- a/block/scsi_ioctl.c
+++ b/block/scsi_ioctl.c
@@ -487,7 +487,7 @@ int sg_scsi_ioctl(struct request_queue *q, struct gendisk 
*disk, fmode_t mode,

if (bytes && blk_rq_map_kern(q, rq, buffer, bytes, __GFP_WAIT)) {
err = DRIVER_ERROR << 24;
-   goto out;
+   goto error;
}

memset(sense, 0, sizeof(sense));
@@ -497,7 +497,6 @@ int sg_scsi_ioctl(struct request_queue *q, struct gendisk 
*disk, fmode_t mode,

blk_execute_rq(q, disk, rq, 0);

-out:
err = rq->errors & 0xff;/* only 8 bit SCSI status */
if (err) {
if (rq->sense_len && rq->sense) {
--
1.9.1

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


Re: [PATCH v2 00/36] Fixes, cleanups and modernization for NCR5380 drivers

2014-11-05 Thread Michael Schmitz
David, Geert,

my Falcon has some variant of this clock patch installed - it may not
be precisely the one described but reasonably close. It also has one
of the old 030 accelerator tricks (clock doubling of the 030 if the
CPU does not do bus cycles - named Skunk) fitted; the clock patch was
installed at the same time.

The Falcon ran stable with just SCSI disks in use (so no locking
races) until a few years ago when I started to use it for kernel
hacking. There were the occasional SCSI lockups which I attributed to
the SCSI clock problem, for want of a better explanation. IIRC when
the clock signal to the 5380 becomes unstable, the chip locks up and
needs to be reset. With the old 2.4 kernels, recovery from reset
worked OK and I've not seen disk corruption or hard read errors.

2.6 kernels changed a lot of things around SCSI midlevel and error
handling. I could never make error recovery from these SCSI lockups
work in 2.6 or 3.x until Arnd's locking fixes (and my patch to defer
command queueing if the lock had been taken by IDE). The lockups did
not stop happening until Finn's patches - and I probably need to
emphasize again that they are gone entirely now. It does seem 2.4
suffered from similar race problems, the driver just managed to
recover from those.

Leaves the current instability - I did some work on the CT60
accelerator (reflashed the firmware so I can use the CTPCI board).
This might have caused the system to become more unstable. Needs more
investigation.

Cheers,

  Michael


On Wed, Nov 5, 2014 at 9:10 PM, Geert Uytterhoeven  wrote:
> On Wed, Nov 5, 2014 at 8:56 AM, David Gálvez  wrote:
>> Do you know about the Falcon's disturbance in the SDMA clock signal
>> hardware problem?
>> Most Falcons, specially those used in music studios, have a hardware
>> patch to fix this, it's normally called SCSI patch.
>>
>> Some more info:
>>
>> http://didierm.pagesperso-orange.fr/doc/eng/c_0a.htm
>
> So this adds additional buffering to the clock.
>
> Note that input pins 3 and 5 of the 74HC04 are left floating!
> I recommend tying them to either GND (pin 7) or VCC (pin 14), to avoid
> them picking up high-frequency signals and consuming power.
>
> Gr{oetje,eeting}s,
>
> Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- 
> ge...@linux-m68k.org
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like 
> that.
> -- Linus Torvalds
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 00/36] Fixes, cleanups and modernization for NCR5380 drivers

2014-11-05 Thread Finn Thain

On Thu, 6 Nov 2014, Michael Schmitz wrote:

> Leaves the current instability - I did some work on the CT60 accelerator 
> (reflashed the firmware so I can use the CTPCI board). This might have 
> caused the system to become more unstable. Needs more investigation.

I gather from the emails we've exchanged that the "current" instability 
(data corruption) dates back to March or thereabouts, so it is unrelated 
to this patch series.

The lockups have been resolved, which leaves only the ST DMA FIFO issue, 
which seems to be an old problem. From the comments in atari_scsi, it 
doesn't look like this was ever resolved.

/* If the DMA was active, but now bit 1 is not clear, it is some
 * other 5380 interrupt that finishes the DMA transfer. We have to
 * calculate the number of residual bytes and give a warning if
 * bytes are stuck in the ST-DMA fifo (there's no way to reach them!)
 */
if (atari_dma_active && (dma_stat & 0x02)) {
unsigned long transferred;

transferred = SCSI_DMA_GETADR() - atari_dma_startaddr;
/* The ST-DMA address is incremented in 2-byte steps, but the
 * data are written only in 16-byte chunks. If the number of
 * transferred bytes is not divisible by 16, the remainder is
 * lost somewhere in outer space.
 */
if (transferred & 15)
printk(KERN_ERR "SCSI DMA error: %ld bytes lost in "
   "ST-DMA fifo\n", transferred & 15);

Presuming that this is an old issue (apparently timing sensitive), we can 
conclude that no regressions were observed in your tests. I'm content with 
this presumption. I gather that you are too, or you would not have sent a 
"tested-by" tag. Which seems to put the ball in Geert's court?

-- 
--
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] aic94xx: Fixup compilation warning

2014-11-05 Thread Christoph Hellwig
On Tue, Nov 04, 2014 at 08:10:59AM +0100, Hannes Reinecke wrote:
> gcc complained about an uninitialized warning.

Looks fine to me, although it very much is gcc overreacting.

--
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] scsi: missing braces in scsi_extd_sense_format()

2014-11-05 Thread Christoph Hellwig
On Wed, Nov 05, 2014 at 11:38:37PM +0300, Dan Carpenter wrote:
> There were missing curly braces so we always return the first
> additional2[] string.
> 
> Fixes: 7046d2fa6dbd ('scsi: use sdev as argument for sense code printing')
> Signed-off-by: Dan Carpenter 

Thanks! 
--
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 02/10] scsi: log request tag for scmd_printk()

2014-11-05 Thread Christoph Hellwig
On Tue, Nov 04, 2014 at 09:06:41AM +0100, Hannes Reinecke wrote:
> The request tag provides a concise identification of a SCSI
> command, so we should be printing that out for scmd_printk().
> 
> Suggested-by: Christoph Hellwig 
> Signed-off-by: Hannes Reinecke 

Reviewed-by: Christoph Hellwig 

--
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 03/10] scsi: use external buffer for command logging

2014-11-05 Thread Christoph Hellwig
Looks good,

Reviewed-by: Christoph Hellwig 
--
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] scsi: missing braces in scsi_extd_sense_format()

2014-11-05 Thread Hannes Reinecke
On 11/05/2014 09:38 PM, Dan Carpenter wrote:
> There were missing curly braces so we always return the first
> additional2[] string.
> 
> Fixes: 7046d2fa6dbd ('scsi: use sdev as argument for sense code printing')
> Signed-off-by: Dan Carpenter 
> 
> diff --git a/drivers/scsi/constants.c b/drivers/scsi/constants.c
> index a1a7fca..0cf43f6 100644
> --- a/drivers/scsi/constants.c
> +++ b/drivers/scsi/constants.c
> @@ -1282,9 +1282,10 @@ scsi_extd_sense_format(unsigned char asc, unsigned 
> char ascq, const char **fmt)
>   for (i = 0; additional2[i].fmt; i++) {
>   if (additional2[i].code1 == asc &&
>   ascq >= additional2[i].code2_min &&
> - ascq <= additional2[i].code2_max)
> + ascq <= additional2[i].code2_max) {
>   *fmt = additional2[i].fmt;
>   return additional2[i].str;
> + }
>   }
>  #endif
>   return NULL;
> 
D'oh.

Reviewed-by: Hannes Reinecke 

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 04/10] libata: use __scsi_format_command()

2014-11-05 Thread Christoph Hellwig
On Tue, Nov 04, 2014 at 09:06:43AM +0100, Hannes Reinecke wrote:
> libata already uses an internal buffer, so we should be using
> __scsi_format_command() here.

Looks good,

Reviewed-by: Christoph Hellwig 
--
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 07/10] scsi: Rename SERVICE_ACTION_IN to SERVICE_ACTION_IN_16

2014-11-05 Thread Christoph Hellwig
On Tue, Nov 04, 2014 at 09:06:46AM +0100, Hannes Reinecke wrote:
> SPC-3 defines SERVICE ACTION IN(12) and SERVICE ACTION IN(16).
> So rename SERVICE_ACTION_IN to SERVICE_ACTION_IN_16 to be
> consistent with SPC and to allow for better distinction.

Looks fine, I'd really like to take this at the beginning of the series.

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


Re: [PATCH 08/10] scsi: Add SPC-3 command definitions

2014-11-05 Thread Christoph Hellwig
On Tue, Nov 04, 2014 at 09:06:47AM +0100, Hannes Reinecke wrote:
> SPC-3 defines SERVICE ACTION IN(12), SERVICE_ACTION OUT(12),
> SERVICE ACTION OUT(16), and SERVICE ACTION BIDIRECTIONAL.
> And READ MEDIA SERIAL NUMBER has long since been deprecated.
> So update callers to refer to the new cdb name.

Looks good, and again I'd like to take this before all the major
logging rework.

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


Re: [PATCH 09/10] scsi: Conditionally compile in constants.c

2014-11-05 Thread Christoph Hellwig
> +static inline const char *scsi_sense_key_string(unsigned char key)
> +{ return NULL; }
> +static inline const char *scsi_extd_sense_format(unsigned char asc,
> +   unsigned char ascq,
> +   const char **fmt)
> +{ *fmt = NULL; return NULL; }
> +static inline const char *scsi_mlreturn_string(int result)
> +{ return NULL; }
> +static inline const char *scsi_hostbyte_string(int result)
> +{ return NULL; }
> +static inline const char *scsi_driverbyte_string(int result)
> +{ return NULL; }

Please apply the normal code formatting rules to these.

Otherwise looks fine,,

Reviewed-by: Christoph Hellwig 
--
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 10/10] scsi: Move remaining printk() statement to scmd_printk()

2014-11-05 Thread Christoph Hellwig
On Tue, Nov 04, 2014 at 09:06:49AM +0100, Hannes Reinecke wrote:
> One statement was missing from the conversion to dev_printk().
> 
> Signed-off-by: Hannes Reinecke 
> ---
>  drivers/scsi/scsi.c | 10 ++
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c
> index 92d5912..9880b59 100644
> --- a/drivers/scsi/scsi.c
> +++ b/drivers/scsi/scsi.c
> @@ -541,10 +541,12 @@ void scsi_log_send(struct scsi_cmnd *cmd)
>   "Send: scmd 0x%p\n", cmd);
>   scsi_print_command(cmd);
>   if (level > 3) {
> - printk(KERN_INFO "buffer = 0x%p, bufflen = %d,"
> -" queuecommand 0x%p\n",
> - scsi_sglist(cmd), scsi_bufflen(cmd),
> - cmd->device->host->hostt->queuecommand);
> + struct Scsi_Host *shost = cmd->device->host;
> + scmd_printk(KERN_INFO, cmd,
> + "buffer = 0x%p, bufflen = %d,"
> + " queuecommand 0x%p\n",
> + scsi_sglist(cmd), scsi_bufflen(cmd),
> + shost->hostt->queuecommand);

This printk doesn't look very useful, I'd rather kill it.
--
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 05/10] scsi: use per-cpu buffer for formatting sense

2014-11-05 Thread Christoph Hellwig
> +#define scmd_format_header(b, l, d, t) \
> + sdev_format_header(b, l, (d) ? (d)->disk_name : NULL, t)

I'd rather have a

static inline const char *scmd_name(struct scsi_cmnd *scmd)
{
return scmd->request->rq_disk ?
scmd->request->rq_disk->disk_name : NULL;
}

helper and use that directlu, which also gets rid of all the
local gendisk variables.

> + if (scsi_sense_is_deferred(sshdr))
> + off += scnprintf(buffer + off, buf_len - off, "[deferred] ");
> + else
> + off += scnprintf(buffer + off, buf_len - off, "[current] ");

off += scnprintf(buffer + off, buf_len - off,
scsi_sense_is_deferred(sshdr) ? "[deferred] " : "[current] ");

> +{
> + struct scsi_sense_hdr sshdr;
> +
> + if (!scsi_normalize_sense(sense_buffer, sense_len, &sshdr)) {
> + int i;
> +
> + for (i = 0; i < sense_len; i += 16) {
> + char *logbuf;
> + int len = min(sense_len - i, 16);
> + size_t off, logbuf_len;
> +
> + logbuf = scsi_log_reserve_buffer(&logbuf_len);
> + if (!logbuf)
> + break;
> + off = sdev_format_header(logbuf, logbuf_len,
> +  name, tag);
> + hex_dump_to_buffer(&sense_buffer[i], len, 16, 1,
> +logbuf + off, logbuf_len - off,
> +false);
> + dev_printk(KERN_INFO, &sdev->sdev_gendev, logbuf);
> + scsi_log_release_buffer(logbuf);
> + }
> + return;

Keeping the code inside the if in a separate helper would probably be a
lot more readable.

--
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 10/10] scsi: Move remaining printk() statement to scmd_printk()

2014-11-05 Thread Hannes Reinecke
On 11/06/2014 08:20 AM, Christoph Hellwig wrote:
> On Tue, Nov 04, 2014 at 09:06:49AM +0100, Hannes Reinecke wrote:
>> One statement was missing from the conversion to dev_printk().
>>
>> Signed-off-by: Hannes Reinecke 
>> ---
>>  drivers/scsi/scsi.c | 10 ++
>>  1 file changed, 6 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c
>> index 92d5912..9880b59 100644
>> --- a/drivers/scsi/scsi.c
>> +++ b/drivers/scsi/scsi.c
>> @@ -541,10 +541,12 @@ void scsi_log_send(struct scsi_cmnd *cmd)
>>  "Send: scmd 0x%p\n", cmd);
>>  scsi_print_command(cmd);
>>  if (level > 3) {
>> -printk(KERN_INFO "buffer = 0x%p, bufflen = %d,"
>> -   " queuecommand 0x%p\n",
>> -scsi_sglist(cmd), scsi_bufflen(cmd),
>> -cmd->device->host->hostt->queuecommand);
>> +struct Scsi_Host *shost = cmd->device->host;
>> +scmd_printk(KERN_INFO, cmd,
>> +"buffer = 0x%p, bufflen = %d,"
>> +" queuecommand 0x%p\n",
>> +scsi_sglist(cmd), scsi_bufflen(cmd),
>> +shost->hostt->queuecommand);
> 
> This printk doesn't look very useful, I'd rather kill it.
> 
Be my guest.

I'm planning on removing all the references to command pointers
anyway, but for that I'll need a consistent scsi command
enumeration. Your tcq rework should help for that.

But that will be done in a later patch series.

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 05/10] scsi: use per-cpu buffer for formatting sense

2014-11-05 Thread Hannes Reinecke
On 11/06/2014 08:33 AM, Christoph Hellwig wrote:
>> +#define scmd_format_header(b, l, d, t) \
>> +sdev_format_header(b, l, (d) ? (d)->disk_name : NULL, t)
> 
> I'd rather have a
> 
> static inline const char *scmd_name(struct scsi_cmnd *scmd)
> {
>   return scmd->request->rq_disk ?
>   scmd->request->rq_disk->disk_name : NULL;
> }
> 
> helper and use that directlu, which also gets rid of all the
> local gendisk variables.
> 
Ok.

>> +if (scsi_sense_is_deferred(sshdr))
>> +off += scnprintf(buffer + off, buf_len - off, "[deferred] ");
>> +else
>> +off += scnprintf(buffer + off, buf_len - off, "[current] ");
> 
>   off += scnprintf(buffer + off, buf_len - off,
>   scsi_sense_is_deferred(sshdr) ? "[deferred] " : "[current] ");
> 
>> +{
>> +struct scsi_sense_hdr sshdr;
>> +
>> +if (!scsi_normalize_sense(sense_buffer, sense_len, &sshdr)) {
>> +int i;
>> +
>> +for (i = 0; i < sense_len; i += 16) {
>> +char *logbuf;
>> +int len = min(sense_len - i, 16);
>> +size_t off, logbuf_len;
>> +
>> +logbuf = scsi_log_reserve_buffer(&logbuf_len);
>> +if (!logbuf)
>> +break;
>> +off = sdev_format_header(logbuf, logbuf_len,
>> + name, tag);
>> +hex_dump_to_buffer(&sense_buffer[i], len, 16, 1,
>> +   logbuf + off, logbuf_len - off,
>> +   false);
>> +dev_printk(KERN_INFO, &sdev->sdev_gendev, logbuf);
>> +scsi_log_release_buffer(logbuf);
>> +}
>> +return;
> 
> Keeping the code inside the if in a separate helper would probably be a
> lot more readable.
> 
Ok. Will be doing so.

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


[PATCH 5/6] scsi: move scsi_dispatch_cmd to scsi_lib.c

2014-11-05 Thread Christoph Hellwig
scsi_lib.c is where the rest of the I/O submission path lives, so move
scsi_dispatch_cmd there and mark it static.

Signed-off-by: Christoph Hellwig 
Reviewed-by: Bart Van Assche 
---
 drivers/scsi/scsi.c  | 81 
 drivers/scsi/scsi_lib.c  | 81 
 drivers/scsi/scsi_priv.h |  1 -
 3 files changed, 81 insertions(+), 82 deletions(-)

diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c
index bc52bbd..5fcfa02 100644
--- a/drivers/scsi/scsi.c
+++ b/drivers/scsi/scsi.c
@@ -603,87 +603,6 @@ void scsi_cmd_get_serial(struct Scsi_Host *host, struct 
scsi_cmnd *cmd)
 EXPORT_SYMBOL(scsi_cmd_get_serial);
 
 /**
- * scsi_dispatch_command - Dispatch a command to the low-level driver.
- * @cmd: command block we are dispatching.
- *
- * Return: nonzero return request was rejected and device's queue needs to be
- * plugged.
- */
-int scsi_dispatch_cmd(struct scsi_cmnd *cmd)
-{
-   struct Scsi_Host *host = cmd->device->host;
-   int rtn = 0;
-
-   atomic_inc(&cmd->device->iorequest_cnt);
-
-   /* check if the device is still usable */
-   if (unlikely(cmd->device->sdev_state == SDEV_DEL)) {
-   /* in SDEV_DEL we error all commands. DID_NO_CONNECT
-* returns an immediate error upwards, and signals
-* that the device is no longer present */
-   cmd->result = DID_NO_CONNECT << 16;
-   goto done;
-   }
-
-   /* Check to see if the scsi lld made this device blocked. */
-   if (unlikely(scsi_device_blocked(cmd->device))) {
-   /*
-* in blocked state, the command is just put back on
-* the device queue.  The suspend state has already
-* blocked the queue so future requests should not
-* occur until the device transitions out of the
-* suspend state.
-*/
-   SCSI_LOG_MLQUEUE(3, scmd_printk(KERN_INFO, cmd,
-   "queuecommand : device blocked\n"));
-   return SCSI_MLQUEUE_DEVICE_BUSY;
-   }
-
-   /* Store the LUN value in cmnd, if needed. */
-   if (cmd->device->lun_in_cdb)
-   cmd->cmnd[1] = (cmd->cmnd[1] & 0x1f) |
-  (cmd->device->lun << 5 & 0xe0);
-
-   scsi_log_send(cmd);
-
-   /*
-* Before we queue this command, check if the command
-* length exceeds what the host adapter can handle.
-*/
-   if (cmd->cmd_len > cmd->device->host->max_cmd_len) {
-   SCSI_LOG_MLQUEUE(3, scmd_printk(KERN_INFO, cmd,
-  "queuecommand : command too long. "
-  "cdb_size=%d host->max_cmd_len=%d\n",
-  cmd->cmd_len, cmd->device->host->max_cmd_len));
-   cmd->result = (DID_ABORT << 16);
-   goto done;
-   }
-
-   if (unlikely(host->shost_state == SHOST_DEL)) {
-   cmd->result = (DID_NO_CONNECT << 16);
-   goto done;
-
-   }
-
-   trace_scsi_dispatch_cmd_start(cmd);
-   rtn = host->hostt->queuecommand(host, cmd);
-   if (rtn) {
-   trace_scsi_dispatch_cmd_error(cmd, rtn);
-   if (rtn != SCSI_MLQUEUE_DEVICE_BUSY &&
-   rtn != SCSI_MLQUEUE_TARGET_BUSY)
-   rtn = SCSI_MLQUEUE_HOST_BUSY;
-
-   SCSI_LOG_MLQUEUE(3, scmd_printk(KERN_INFO, cmd,
-   "queuecommand : request rejected\n"));
-   }
-
-   return rtn;
- done:
-   cmd->scsi_done(cmd);
-   return 0;
-}
-
-/**
  * scsi_finish_command - cleanup and pass command back to upper layer
  * @cmd: the command
  *
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index b2de77b..6400c6b 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -1602,6 +1602,87 @@ static void scsi_softirq_done(struct request *rq)
 }
 
 /**
+ * scsi_dispatch_command - Dispatch a command to the low-level driver.
+ * @cmd: command block we are dispatching.
+ *
+ * Return: nonzero return request was rejected and device's queue needs to be
+ * plugged.
+ */
+static int scsi_dispatch_cmd(struct scsi_cmnd *cmd)
+{
+   struct Scsi_Host *host = cmd->device->host;
+   int rtn = 0;
+
+   atomic_inc(&cmd->device->iorequest_cnt);
+
+   /* check if the device is still usable */
+   if (unlikely(cmd->device->sdev_state == SDEV_DEL)) {
+   /* in SDEV_DEL we error all commands. DID_NO_CONNECT
+* returns an immediate error upwards, and signals
+* that the device is no longer present */
+   cmd->result = DID_NO_CONNECT << 16;
+   goto done;
+   }
+
+   /* Check to see if the scsi lld made this device blocked. */
+   if (unlikely(scsi_device_blocked(cmd->device))) {
+   /*
+* in blocked state, the command i

[PATCH 3/6] scsi: clean up S/G table freeing

2014-11-05 Thread Christoph Hellwig
Now that we are using the split completion model for the legacy request
path as well we can use scsi_mq_free_sgtables unconditionally.

Rename it to scsi_free_sgtables, use it for the legacy path and remove
scsi_release_(bidi_)buffers.

Signed-off-by: Christoph Hellwig 
---
 drivers/scsi/scsi_lib.c | 77 +
 1 file changed, 20 insertions(+), 57 deletions(-)

diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 1a3546e8..a737032 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -622,7 +622,7 @@ static void scsi_uninit_cmd(struct scsi_cmnd *cmd)
}
 }
 
-static void scsi_mq_free_sgtables(struct scsi_cmnd *cmd)
+static void scsi_free_sgtables(struct scsi_cmnd *cmd)
 {
if (cmd->sdb.table.nents)
scsi_free_sgtable(&cmd->sdb, true);
@@ -638,7 +638,6 @@ static void scsi_mq_uninit_cmd(struct scsi_cmnd *cmd)
struct Scsi_Host *shost = sdev->host;
unsigned long flags;
 
-   scsi_mq_free_sgtables(cmd);
scsi_uninit_cmd(cmd);
 
if (shost->use_cmd_list) {
@@ -649,42 +648,6 @@ static void scsi_mq_uninit_cmd(struct scsi_cmnd *cmd)
}
 }
 
-/*
- * Function:scsi_release_buffers()
- *
- * Purpose: Free resources allocate for a scsi_command.
- *
- * Arguments:   cmd- command that we are bailing.
- *
- * Lock status: Assumed that no lock is held upon entry.
- *
- * Returns: Nothing
- *
- * Notes:   In the event that an upper level driver rejects a
- * command, we must release resources allocated during
- * the __init_io() function.  Primarily this would involve
- * the scatter-gather table.
- */
-static void scsi_release_buffers(struct scsi_cmnd *cmd)
-{
-   if (cmd->sdb.table.nents)
-   scsi_free_sgtable(&cmd->sdb, false);
-
-   memset(&cmd->sdb, 0, sizeof(cmd->sdb));
-
-   if (scsi_prot_sg_count(cmd))
-   scsi_free_sgtable(cmd->prot_sdb, false);
-}
-
-static void scsi_release_bidi_buffers(struct scsi_cmnd *cmd)
-{
-   struct scsi_data_buffer *bidi_sdb = cmd->request->next_rq->special;
-
-   scsi_free_sgtable(bidi_sdb, false);
-   kmem_cache_free(scsi_sdb_cache, bidi_sdb);
-   cmd->request->next_rq->special = NULL;
-}
-
 static bool scsi_end_request(struct request *req, int error,
unsigned int bytes, unsigned int bidi_bytes)
 {
@@ -703,16 +666,14 @@ static bool scsi_end_request(struct request *req, int 
error,
if (blk_queue_add_random(q))
add_disk_randomness(req->rq_disk);
 
+   /*
+* In the MQ case the command gets freed by __blk_mq_end_io, so we have
+* to do all cleanup that depends on the command before that.
+*/
+   scsi_free_sgtables(cmd);
+
if (req->mq_ctx) {
-   /*
-* In the MQ case the command gets freed by 
__blk_mq_end_request,
-* so we have to do all cleanup that depends on it earlier.
-*
-* We also can't kick the queues from irq context, so we
-* will have to defer it to a workqueue.
-*/
scsi_mq_uninit_cmd(cmd);
-
__blk_mq_end_request(req, error);
 
if (scsi_target(sdev)->single_lun ||
@@ -723,15 +684,15 @@ static bool scsi_end_request(struct request *req, int 
error,
} else {
unsigned long flags;
 
-   if (bidi_bytes)
-   scsi_release_bidi_buffers(cmd);
+   if (bidi_bytes) {
+   kmem_cache_free(scsi_sdb_cache, req->next_rq->special);
+   req->next_rq->special = NULL;
+   }
 
spin_lock_irqsave(q->queue_lock, flags);
blk_finish_request(req, error);
spin_unlock_irqrestore(q->queue_lock, flags);
 
-   scsi_release_buffers(cmd);
-
scsi_put_command(cmd);
scsi_run_queue(q);
}
@@ -1057,12 +1018,12 @@ void scsi_io_completion(struct scsi_cmnd *cmd, unsigned 
int good_bytes)
/* Unprep the request and put it back at the head of the queue.
 * A new command will be prepared and issued.
 */
+   scsi_free_sgtables(cmd);
if (q->mq_ops) {
cmd->request->cmd_flags &= ~REQ_DONTPREP;
scsi_mq_uninit_cmd(cmd);
scsi_mq_requeue_cmd(cmd);
} else {
-   scsi_release_buffers(cmd);
scsi_requeue_command(q, cmd);
}
break;
@@ -1165,10 +1126,8 @@ int scsi_init_io(struct scsi_cmnd *cmd, gfp_t gfp_mask)
 
return BLKPREP_OK;
 err_exit:
-   if (is_mq) {
-   scsi_mq_free_sgtables(cmd);
-   } else {
-   scsi_release_buffers(cmd);
+   scsi_free_sgtables(cmd);
+   if 

[PATCH 4/6] scsi: stop passing a gfp_mask argument down the command setup path

2014-11-05 Thread Christoph Hellwig
There is no reason for ULDs to pass in a flag on how to allocate the S/G
lists.  While we don't need GFP_ATOMIC for the blk-mq case because we
don't hold locks, that decision can be made way down the chain without
having to pass a pointless gfp_mask argument.

Signed-off-by: Christoph Hellwig 
Reviewed-by: Bart Van Assche 
---
 drivers/scsi/scsi_lib.c  | 20 +---
 drivers/scsi/sd.c|  6 +++---
 drivers/scsi/sr.c|  2 +-
 include/scsi/scsi_cmnd.h |  2 +-
 4 files changed, 14 insertions(+), 16 deletions(-)

diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index a737032..b2de77b 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -588,10 +588,10 @@ static void scsi_free_sgtable(struct scsi_data_buffer 
*sdb, bool mq)
__sg_free_table(&sdb->table, SCSI_MAX_SG_SEGMENTS, mq, scsi_sg_free);
 }
 
-static int scsi_alloc_sgtable(struct scsi_data_buffer *sdb, int nents,
- gfp_t gfp_mask, bool mq)
+static int scsi_alloc_sgtable(struct scsi_data_buffer *sdb, int nents, bool mq)
 {
struct scatterlist *first_chunk = NULL;
+   gfp_t gfp_mask = mq ? GFP_NOIO : GFP_ATOMIC;
int ret;
 
BUG_ON(!nents);
@@ -1038,8 +1038,7 @@ void scsi_io_completion(struct scsi_cmnd *cmd, unsigned 
int good_bytes)
}
 }
 
-static int scsi_init_sgtable(struct request *req, struct scsi_data_buffer *sdb,
-gfp_t gfp_mask)
+static int scsi_init_sgtable(struct request *req, struct scsi_data_buffer *sdb)
 {
int count;
 
@@ -1047,7 +1046,7 @@ static int scsi_init_sgtable(struct request *req, struct 
scsi_data_buffer *sdb,
 * If sg table allocation fails, requeue request later.
 */
if (unlikely(scsi_alloc_sgtable(sdb, req->nr_phys_segments,
-   gfp_mask, req->mq_ctx != NULL)))
+   req->mq_ctx != NULL)))
return BLKPREP_DEFER;
 
/* 
@@ -1072,7 +1071,7 @@ static int scsi_init_sgtable(struct request *req, struct 
scsi_data_buffer *sdb,
  * BLKPREP_DEFER if the failure is retryable
  * BLKPREP_KILL if the failure is fatal
  */
-int scsi_init_io(struct scsi_cmnd *cmd, gfp_t gfp_mask)
+int scsi_init_io(struct scsi_cmnd *cmd)
 {
struct scsi_device *sdev = cmd->device;
struct request *rq = cmd->request;
@@ -1081,7 +1080,7 @@ int scsi_init_io(struct scsi_cmnd *cmd, gfp_t gfp_mask)
 
BUG_ON(!rq->nr_phys_segments);
 
-   error = scsi_init_sgtable(rq, &cmd->sdb, gfp_mask);
+   error = scsi_init_sgtable(rq, &cmd->sdb);
if (error)
goto err_exit;
 
@@ -1097,8 +1096,7 @@ int scsi_init_io(struct scsi_cmnd *cmd, gfp_t gfp_mask)
rq->next_rq->special = bidi_sdb;
}
 
-   error = scsi_init_sgtable(rq->next_rq, rq->next_rq->special,
- GFP_ATOMIC);
+   error = scsi_init_sgtable(rq->next_rq, rq->next_rq->special);
if (error)
goto err_exit;
}
@@ -1110,7 +1108,7 @@ int scsi_init_io(struct scsi_cmnd *cmd, gfp_t gfp_mask)
BUG_ON(prot_sdb == NULL);
ivecs = blk_rq_count_integrity_sg(rq->q, rq->bio);
 
-   if (scsi_alloc_sgtable(prot_sdb, ivecs, gfp_mask, is_mq)) {
+   if (scsi_alloc_sgtable(prot_sdb, ivecs, is_mq)) {
error = BLKPREP_DEFER;
goto err_exit;
}
@@ -1177,7 +1175,7 @@ static int scsi_setup_blk_pc_cmnd(struct scsi_device 
*sdev, struct request *req)
 * submit a request without an attached bio.
 */
if (req->bio) {
-   int ret = scsi_init_io(cmd, GFP_ATOMIC);
+   int ret = scsi_init_io(cmd);
if (unlikely(ret))
return ret;
} else {
diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index b041eca..eca990c 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -784,7 +784,7 @@ static int sd_setup_discard_cmnd(struct scsi_cmnd *cmd)
 * amount of blocks described by the request.
 */
blk_add_request_payload(rq, page, len);
-   ret = scsi_init_io(cmd, GFP_ATOMIC);
+   ret = scsi_init_io(cmd);
rq->__data_len = nr_bytes;
 
 out:
@@ -878,7 +878,7 @@ static int sd_setup_write_same_cmnd(struct scsi_cmnd *cmd)
 * knows how much to actually write.
 */
rq->__data_len = sdp->sector_size;
-   ret = scsi_init_io(cmd, GFP_ATOMIC);
+   ret = scsi_init_io(cmd);
rq->__data_len = nr_bytes;
return ret;
 }
@@ -912,7 +912,7 @@ static int sd_setup_read_write_cmnd(struct scsi_cmnd *SCpnt)
int ret;
unsigned char protect;
 
-   ret = scsi_init_io(SCpnt, GFP_ATOMIC);
+   ret = scsi_init_io(SCpnt);
if (ret != BLKPREP_OK)
goto out;
SCpnt = rq->sp

[PATCH 2/6] scsi: remove scsi_next_command

2014-11-05 Thread Christoph Hellwig
There's only one caller left, so inline it and reduce the blk-mq vs !blk-mq
diff a litte bit.

Signed-off-by: Christoph Hellwig 
Reviewed-by: Bart Van Assche 
---
 drivers/scsi/scsi_lib.c  | 18 --
 drivers/scsi/scsi_priv.h |  1 -
 2 files changed, 4 insertions(+), 15 deletions(-)

diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 38f8c85..1a3546e8 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -543,17 +543,6 @@ static void scsi_requeue_command(struct request_queue *q, 
struct scsi_cmnd *cmd)
put_device(&sdev->sdev_gendev);
 }
 
-void scsi_next_command(struct scsi_cmnd *cmd)
-{
-   struct scsi_device *sdev = cmd->device;
-   struct request_queue *q = sdev->request_queue;
-
-   scsi_put_command(cmd);
-   scsi_run_queue(q);
-
-   put_device(&sdev->sdev_gendev);
-}
-
 void scsi_run_host_queues(struct Scsi_Host *shost)
 {
struct scsi_device *sdev;
@@ -731,8 +720,6 @@ static bool scsi_end_request(struct request *req, int error,
kblockd_schedule_work(&sdev->requeue_work);
else
blk_mq_start_stopped_hw_queues(q, true);
-
-   put_device(&sdev->sdev_gendev);
} else {
unsigned long flags;
 
@@ -744,9 +731,12 @@ static bool scsi_end_request(struct request *req, int 
error,
spin_unlock_irqrestore(q->queue_lock, flags);
 
scsi_release_buffers(cmd);
-   scsi_next_command(cmd);
+
+   scsi_put_command(cmd);
+   scsi_run_queue(q);
}
 
+   put_device(&sdev->sdev_gendev);
return false;
 }
 
diff --git a/drivers/scsi/scsi_priv.h b/drivers/scsi/scsi_priv.h
index 12b8e1b..2a382c1 100644
--- a/drivers/scsi/scsi_priv.h
+++ b/drivers/scsi/scsi_priv.h
@@ -84,7 +84,6 @@ int scsi_noretry_cmd(struct scsi_cmnd *scmd);
 extern int scsi_maybe_unblock_host(struct scsi_device *sdev);
 extern void scsi_device_unbusy(struct scsi_device *sdev);
 extern void scsi_queue_insert(struct scsi_cmnd *cmd, int reason);
-extern void scsi_next_command(struct scsi_cmnd *cmd);
 extern void scsi_io_completion(struct scsi_cmnd *, unsigned int);
 extern void scsi_run_host_queues(struct Scsi_Host *shost);
 extern struct request_queue *scsi_alloc_queue(struct scsi_device *sdev);
-- 
1.9.1

--
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/6] scsi: don't use scsi_next_command in scsi_reset_provider

2014-11-05 Thread Christoph Hellwig
scsi_reset_provider already manually runs all queues for the given host,
so it doesn't need the scsi_run_queues call from it, and it doesn't need
a reference on the device because it's synchronous.

So let's just call scsi_put_command directly and avoid the device reference
dance to simplify the code.

Signed-off-by: Christoph Hellwig 
Reviewed-by: Bart Van Assche 
---
 drivers/scsi/scsi_error.c | 11 +++
 1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c
index ba19687..a1ffdb0 100644
--- a/drivers/scsi/scsi_error.c
+++ b/drivers/scsi/scsi_error.c
@@ -2335,14 +2335,9 @@ scsi_ioctl_reset(struct scsi_device *dev, int __user 
*arg)
return -EIO;
 
error = -EIO;
-   if (!get_device(&dev->sdev_gendev))
-   goto out_put_autopm_host;
-
scmd = scsi_get_command(dev, GFP_KERNEL);
-   if (!scmd) {
-   put_device(&dev->sdev_gendev);
+   if (!scmd)
goto out_put_autopm_host;
-   }
 
blk_rq_init(NULL, &req);
scmd->request = &req;
@@ -2404,10 +2399,10 @@ scsi_ioctl_reset(struct scsi_device *dev, int __user 
*arg)
 "waking up host to restart after TMF\n"));
 
wake_up(&shost->host_wait);
-
scsi_run_host_queues(shost);
 
-   scsi_next_command(scmd);
+   scsi_put_command(scmd);
+
 out_put_autopm_host:
scsi_autopm_put_host(shost);
return error;
-- 
1.9.1

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


I/O path cleanup V2

2014-11-05 Thread Christoph Hellwig
This series cleans up a couple of lose ends I noticed during the scsi-mq
work, but which weren't important enough to address during the last cycle.

These are also available at:

   git://git.infradead.org/users/hch/scsi.git scsi-io-path-cleanups

Changes since V1:
 - rebased to core-for-3.19
 - addressed review comments from Bart Van Assche 
 - dropped the last two patches for now.  There is too much churn in
   this area at the moment.

--
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 6/6] scsi: move more requeue handling into scsi_requeue_command

2014-11-05 Thread Christoph Hellwig
Move a bit code out of scsi_io_completion and into scsi_requeue_command
in preparation for further refactoring.

Signed-off-by: Christoph Hellwig 
Reviewed-by: Bart Van Assche 
---
 drivers/scsi/scsi_lib.c | 86 ++---
 1 file changed, 38 insertions(+), 48 deletions(-)

diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 6400c6b..e8457fc 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -507,42 +507,6 @@ void scsi_requeue_run_queue(struct work_struct *work)
scsi_run_queue(q);
 }
 
-/*
- * Function:   scsi_requeue_command()
- *
- * Purpose:Handle post-processing of completed commands.
- *
- * Arguments:  q   - queue to operate on
- * cmd - command that may need to be requeued.
- *
- * Returns:Nothing
- *
- * Notes:  After command completion, there may be blocks left
- * over which weren't finished by the previous command
- * this can be for a number of reasons - the main one is
- * I/O errors in the middle of the request, in which case
- * we need to request the blocks that come after the bad
- * sector.
- * Notes:  Upon return, cmd is a stale pointer.
- */
-static void scsi_requeue_command(struct request_queue *q, struct scsi_cmnd 
*cmd)
-{
-   struct scsi_device *sdev = cmd->device;
-   struct request *req = cmd->request;
-   unsigned long flags;
-
-   spin_lock_irqsave(q->queue_lock, flags);
-   blk_unprep_request(req);
-   req->special = NULL;
-   scsi_put_command(cmd);
-   blk_requeue_request(q, req);
-   spin_unlock_irqrestore(q->queue_lock, flags);
-
-   scsi_run_queue(q);
-
-   put_device(&sdev->sdev_gendev);
-}
-
 void scsi_run_host_queues(struct Scsi_Host *shost)
 {
struct scsi_device *sdev;
@@ -648,6 +612,43 @@ static void scsi_mq_uninit_cmd(struct scsi_cmnd *cmd)
}
 }
 
+/**
+ * scsi_requeue_command - requeue a command from the I/O completion path.
+ * @cmd: command that needs to be requeued
+ *
+ * After command completion, there may be blocks left over which weren't
+ * finished by the previous command this can be for a number of reasons -
+ * the main one is I/O errors in the middle of the request, in which case
+ * we need to request the blocks that come after the bad sector.
+ */
+static void scsi_requeue_command(struct scsi_cmnd *cmd)
+{
+   struct request *req = cmd->request;
+   struct request_queue *q = req->q;
+
+   scsi_free_sgtables(cmd);
+
+   if (q->mq_ops) {
+   cmd->request->cmd_flags &= ~REQ_DONTPREP;
+   scsi_mq_uninit_cmd(cmd);
+   scsi_mq_requeue_cmd(cmd);
+   } else {
+   struct scsi_device *sdev = cmd->device;
+   unsigned long flags;
+
+   spin_lock_irqsave(q->queue_lock, flags);
+   blk_unprep_request(req);
+   req->special = NULL;
+   scsi_put_command(cmd);
+   blk_requeue_request(q, req);
+   spin_unlock_irqrestore(q->queue_lock, flags);
+
+   scsi_run_queue(q);
+
+   put_device(&sdev->sdev_gendev);
+   }
+}
+
 static bool scsi_end_request(struct request *req, int error,
unsigned int bytes, unsigned int bidi_bytes)
 {
@@ -779,7 +780,6 @@ static int __scsi_error_from_host_byte(struct scsi_cmnd 
*cmd, int result)
 void scsi_io_completion(struct scsi_cmnd *cmd, unsigned int good_bytes)
 {
int result = cmd->result;
-   struct request_queue *q = cmd->device->request_queue;
struct request *req = cmd->request;
int error = 0;
struct scsi_sense_hdr sshdr;
@@ -1015,17 +1015,7 @@ void scsi_io_completion(struct scsi_cmnd *cmd, unsigned 
int good_bytes)
/*FALLTHRU*/
case ACTION_REPREP:
requeue:
-   /* Unprep the request and put it back at the head of the queue.
-* A new command will be prepared and issued.
-*/
-   scsi_free_sgtables(cmd);
-   if (q->mq_ops) {
-   cmd->request->cmd_flags &= ~REQ_DONTPREP;
-   scsi_mq_uninit_cmd(cmd);
-   scsi_mq_requeue_cmd(cmd);
-   } else {
-   scsi_requeue_command(q, cmd);
-   }
+   scsi_requeue_command(cmd);
break;
case ACTION_RETRY:
/* Retry the same command immediately */
-- 
1.9.1

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