Re: [PATCH] scsi disk: Use its own buffer for the vpd request

[Martin K. Petersen]

> "Bernd" == Bernd Schubert  writes:

Bernd,

Bernd> Once I noticed that scsi_get_vpd_page() works fine from other
Bernd> function calls and that it is not 0x89, but already 0x0 that
Bernd> fails fixing it became easy.

Bernd> Nix, any chance you could verify it also works for you?

Do we get an appropriate error back when we try to issue WRITE SAME
10/16? If so, I'm OK with this fix.

And thanks for looking into this!

-- 
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: sparc ESP SCSI error handling BUG+hang

[David Miller]

From: Meelis Roos 
Date: Tue, 30 Jul 2013 12:58:44 +0300 (EEST)

>> > Therefore I think the fix is going to involve adding a member to
>> > "struct esp_cmd_entry" called "->orig_tag[]" so that we can see what
>> > the original tag[] values were at esp_alloc_lun_tag() time.
>> 
>> Please try this patch:
> 
> It works on 3 consecutive boots, thank you!
> 
> Tested-by: Meelis Roos 

Thanks for testing, I'll push this to Linus via the sparc tree.
--
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] Hard disk S3 resume time optimization

[Brandt, Todd E]

This patch is a potential way to reduce the S3 resume time for SATA drives. 
Essentially this patch removes the hard disk resume time from the total system 
resume time, with the disks still taking as long to come back online but in the 
background.

The major bottleneck is in the ata port resume which sends out a wakeup command 
and then waits up to several seconds for the port to resume. This patch changes 
the ata_port_resume_common function to be non blocking. The other bottleneck is 
in the scsi disk resume code, which issues a a command to startup the disk with 
blk_execute_rq, which then waits for the command to finish (which also depends 
on the ata port being fully resumed). The patch switches the sd_resume function 
to use blk_execute_rq_nowait instead (the underlying code is identical to 
sd_resume, but is changed to non-blocking).

Signed-off-by: Todd Brandt 
---
 drivers/ata/libata-core.c |  4 +++-
 drivers/scsi/sd.c | 57 
-
 2 files changed, 59 insertions(+), 2 deletions(-)

diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index c24354d..3585092 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -118,6 +118,7 @@ struct ata_force_ent {
 
 static struct ata_force_ent *ata_force_tbl;
 static int ata_force_tbl_size;
+int ata_resume_status;
 
 static char ata_force_param_buf[PAGE_SIZE] __initdata;
 /* param_buf is thrown away after initialization, disallow read */
@@ -5398,7 +5399,8 @@ static int ata_port_resume_common(struct device *dev, 
pm_message_t mesg)
 {
struct ata_port *ap = to_ata_port(dev);
 
-   return __ata_port_resume_common(ap, mesg, NULL);
+   ata_resume_status = 0;
+   return __ata_port_resume_common(ap, mesg, &ata_resume_status);
 }
 
 static int ata_port_resume(struct device *dev)
diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index 86fcf2c..ee4d7e2 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -107,6 +107,7 @@ static int  sd_remove(struct device *);
 static void sd_shutdown(struct device *);
 static int sd_suspend(struct device *);
 static int sd_resume(struct device *);
+static int sd_resume_async(struct device *);
 static void sd_rescan(struct device *);
 static int sd_done(struct scsi_cmnd *);
 static int sd_eh_action(struct scsi_cmnd *, unsigned char *, int, int);
@@ -484,7 +485,7 @@ static struct class sd_disk_class = {
 
 static const struct dev_pm_ops sd_pm_ops = {
.suspend= sd_suspend,
-   .resume = sd_resume,
+   .resume = sd_resume_async,
.poweroff   = sd_suspend,
.restore= sd_resume,
.runtime_suspend= sd_suspend,
@@ -3137,6 +3138,60 @@ done:
return ret;
 }
 
+static void sd_resume_async_end(struct request *rq, int error)
+{
+   struct scsi_disk *sdkp = rq->end_io_data;
+
+   sd_printk(KERN_NOTICE, sdkp, "Starting disk complete (async)\n");
+   rq->end_io_data = NULL;
+   __blk_put_request(rq->q, rq);
+}
+
+static int sd_resume_async(struct device *dev)
+{
+   unsigned char cmd[6] = { START_STOP };  /* START_VALID */
+   struct scsi_disk *sdkp = scsi_disk_get_from_dev(dev);
+   struct request *req;
+   int ret = 0;
+
+   if (!sdkp->device->manage_start_stop)
+   goto done;
+
+   sd_printk(KERN_NOTICE, sdkp, "Starting disk (async)\n");
+
+   cmd[4] |= 1;/* START */
+
+   if (sdkp->device->start_stop_pwr_cond)
+   cmd[4] |= 1 << 4;   /* Active or Standby */
+
+   if (!scsi_device_online(sdkp->device)) {
+   ret = -ENODEV;
+   goto done;
+   }
+
+   req = blk_get_request(sdkp->device->request_queue, 0, __GFP_WAIT);
+   if (!req) {
+   ret = DRIVER_ERROR << 24;
+   goto done;
+   }
+
+   req->cmd_len = COMMAND_SIZE(cmd[0]);
+   memcpy(req->cmd, cmd, req->cmd_len);
+   req->sense = NULL;
+   req->sense_len = 0;
+   req->retries = SD_MAX_RETRIES;
+   req->timeout = SD_TIMEOUT;
+   req->cmd_type = REQ_TYPE_BLOCK_PC;
+   req->cmd_flags |= REQ_PM | REQ_QUIET | REQ_PREEMPT;
+
+   req->end_io_data = sdkp;
+   blk_execute_rq_nowait(req->q, NULL, req, 1, sd_resume_async_end);
+
+done:
+   scsi_disk_put(sdkp);
+   return ret;
+}
+
 /**
  * init_sd - entry point for this driver (both when built in or when
  * a module).



Todd Brandt
Linux Kernel Developer OTC, Hillsboro OR
--
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 RFC 49/51] ARM: 7796/1: scsi: Use dma_max_pfn(dev) helper for bounce_limit calculations

[Santosh Shilimkar]

DMA bounce limit is the maximum direct DMA'able memory beyond which
bounce buffers has to be used to perform dma operations. SCSI driver
relies on dma_mask but its calculation is based on max_*pfn which
don't have uniform meaning across architectures. So make use of
dma_max_pfn() which is expected to return the DMAable maximum pfn
value across architectures.

Signed-off-by: Santosh Shilimkar 
Signed-off-by: Russell King 
---
 drivers/scsi/scsi_lib.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 124392f..1be55e3 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -1684,7 +1684,7 @@ u64 scsi_calculate_bounce_limit(struct Scsi_Host *shost)
 
host_dev = scsi_get_device(shost);
if (host_dev && host_dev->dma_mask)
-   bounce_limit = *host_dev->dma_mask;
+   bounce_limit = dma_max_pfn(host_dev) << PAGE_SHIFT;
 
return bounce_limit;
 }
-- 
1.7.4.4

--
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 RFC 00/51] Preview of DMA mask changes

[Russell King - ARM Linux]

So, this patch set is a preview of the DMA mask changes which I currently
have in my tree.

This started out as a request to look at the DMA mask situation, and how
to solve the issues which we have on ARM.  One of those issues is how to
deal with the DT side of things, which I haven't yet addressed.

However, I started off reviewing how the dma_mask and coherent_dma_mask
was being used, and what I found was rather messy, and in some cases
rather buggy.  I tried to get some of the bug fixes in before the last
merge window, but it seems that the maintainers preferred to have the
full solution rather than a simple -rc suitable bug fix.

So, this is an attempt to clean things up.

The first point here is that drivers performing DMA should be calling
dma_set_mask()/dma_set_coherent_mask() in their probe function to verify
that DMA can be performed.  Lots of ARM drivers omit this step; please
refer to the DMA API documentation on this subject.

What this means is that the DMA mask provided by bus code is a default
value - nothing more.  It doesn't have to accurately reflect what the
device is actually capable of.  Apart from the storage for dev->dma_mask
being initialised for any device which is DMA capable, there is no other
initialisation which is strictly necessary at device creation time.

Now, these cleanups address two major areas:
1. The setting of DMA masks, particularly when both the coherent and
   streaming DMA masks are set together.

2. The initialisation of DMA masks by drivers - this seems to be becoming
   a popular habbit, one which may not be entirely the right solution.
   Rather than having this scattered throughout the tree, I've pulled
   that into a central location (and called it coercing the DMA mask -
   because it really is about forcing the DMA mask to be that value.)

3. Finally, addressing the long held misbelief that DMA masks somehow
   correspond with physical addresses.  We already have established
   long ago that dma_addr_t values returned from the DMA API are the
   values which you program into the DMA controller, and so are the
   bus addresses.  It is _only_ sane that DMA masks are also bus
   related too, and not related to physical address spaces.

(3) is a very important point for LPAE systems, which may still have
less than 4GB of memory, but this memory is all located above the 4GB
physical boundary.  This means with the current model, any device
using a 32-bit DMA mask fails - even though the DMA controller is
still only a 32-bit DMA controller but the 32-bit bus addresses map
to system memory.  To put it another way, the bus addresses have a
4GB physical offset on them.

This work is ongoing, and I'm still fixing the odd bug which the nightly
autobuilder randconfigs find (when it doesn't hit some other problem.)

I'm not using get_maintainer.pl for this series yet, because the list
of recipients is gigantic and would not pass through vger's filters,
let alone other mailing lists.  So this initial posting will only be
sent to those listed explicitly in Cc:'s in the patches, linux-kernel
and linux-arm-kernel... and it will take about an hour to send all
these patches out.

Patches based on -rc2.

 Documentation/DMA-API-HOWTO.txt   |   37 +--
 Documentation/DMA-API.txt |8 +++
 arch/arm/include/asm/dma-mapping.h|8 +++
 arch/arm/mm/dma-mapping.c |   49 ++--
 arch/arm/mm/init.c|   12 +++---
 arch/arm/mm/mm.h  |2 +
 arch/powerpc/kernel/vio.c |3 +-
 block/blk-settings.c  |8 ++--
 drivers/amba/bus.c|6 +--
 drivers/ata/pata_ixp4xx_cf.c  |5 ++-
 drivers/ata/pata_octeon_cf.c  |5 +-
 drivers/block/nvme-core.c |7 +--
 drivers/crypto/ixp4xx_crypto.c|   48 ++--
 drivers/dma/amba-pl08x.c  |5 ++
 drivers/dma/dw/platform.c |8 +--
 drivers/dma/edma.c|6 +--
 drivers/dma/pl330.c   |4 ++
 drivers/firmware/dcdbas.c |   23 +-
 drivers/firmware/google/gsmi.c|   13 +++--
 drivers/gpu/drm/exynos/exynos_drm_drv.c   |7 +++-
 drivers/gpu/drm/omapdrm/omap_dmm_tiler.c  |5 +-
 drivers/media/platform/omap3isp/isp.c |6 +-
 drivers/media/platform/omap3isp/isp.h |3 -
 drivers/mmc/card/queue.c  |3 +-
 drivers/mmc/host/sdhci-acpi.c |5 +-
 drivers/net/ethernet/broadcom/b44.c   |3 +-
 drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c  |8 +---
 drivers/net/ethernet/brocade/bna/bnad.c   |   13 ++
 drivers/net/ether

[PATCH v4 08/10] scsi: Generate uevents on certain unit attention codes

[Ewan D. Milne]

From: "Ewan D. Milne" 

Generate a uevent when the following Unit Attention ASC/ASCQ
codes are received:

2A/01  MODE PARAMETERS CHANGED
2A/09  CAPACITY DATA HAS CHANGED
38/07  THIN PROVISIONING SOFT THRESHOLD REACHED
3F/03  INQUIRY DATA HAS CHANGED
3F/0E  REPORTED LUNS DATA HAS CHANGED

Log kernel messages when the following Unit Attention ASC/ASCQ
codes are received that are not as specific as those above:

2A/xx  PARAMETERS CHANGED
3F/xx  TARGET OPERATING CONDITIONS HAVE CHANGED

Added logic to set expecting_cc_ua for other LUNs on SPC-3 devices
after REPORTED LUNS DATA HAS CHANGED is received, so that duplicate
uevents are not generated, and clear expecting_cc_ua when a
REPORT LUNS command completes, in accordance with the SPC-3
specification regarding reporting of the 3F 0E ASC/ASCQ UA.

Signed-off-by: Ewan D. Milne 
---
 drivers/scsi/scsi_error.c  | 118 +
 drivers/scsi/scsi_lib.c|  42 ++--
 drivers/scsi/scsi_sysfs.c  |  10 
 include/scsi/scsi_device.h |  11 -
 4 files changed, 156 insertions(+), 25 deletions(-)

diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c
index 96707a6..227041a 100644
--- a/drivers/scsi/scsi_error.c
+++ b/drivers/scsi/scsi_error.c
@@ -222,6 +222,86 @@ static inline void scsi_eh_prt_fail_stats(struct Scsi_Host 
*shost,
 }
 #endif
 
+ /**
+ * scsi_report_lun_change - Set flag on all *other* devices on the same target
+ *  to indicate that a UNIT ATTENTION is expected.
+ *  Only do this for SPC-3 devices, however.
+ * @sdev:  Device reporting the UNIT ATTENTION
+ */
+static void scsi_report_lun_change(struct scsi_device *sdev)
+{
+   struct Scsi_Host *shost = sdev->host;
+   struct scsi_device *tmp_sdev;
+
+   if (sdev->scsi_level == SCSI_SPC_3)
+   shost_for_each_device(tmp_sdev, shost) {
+   if (tmp_sdev->channel == sdev->channel &&
+   tmp_sdev->id == sdev->id &&
+   tmp_sdev != sdev)
+   tmp_sdev->expecting_cc_ua = 1;
+   }
+}
+
+/**
+ * scsi_report_sense - Examine scsi sense information and log messages for
+ *certain conditions, also issue uevents for some of them.
+ * @sshdr: sshdr to be examined
+ */
+static void scsi_report_sense(struct scsi_device *sdev,
+ struct scsi_sense_hdr *sshdr)
+{
+   enum scsi_device_event evt_type = SDEV_EVT_MAXBITS; /* i.e. none */
+   unsigned long flags;
+
+   if (sshdr->sense_key == UNIT_ATTENTION) {
+   if (sshdr->asc == 0x3f && sshdr->ascq == 0x03) {
+   evt_type = SDEV_EVT_INQUIRY_CHANGE_REPORTED;
+   sdev_printk(KERN_WARNING, sdev,
+   "Inquiry data has changed");
+   } else if (sshdr->asc == 0x3f && sshdr->ascq == 0x0e) {
+   evt_type = SDEV_EVT_LUN_CHANGE_REPORTED;
+   scsi_report_lun_change(sdev);
+   sdev_printk(KERN_WARNING, sdev,
+   "Warning! Received an indication that the "
+   "LUN assignments on this target have "
+   "changed. The Linux SCSI layer does not "
+   "automatically remap LUN assignments.\n");
+   } else if (sshdr->asc == 0x3f)
+   sdev_printk(KERN_WARNING, sdev,
+   "Warning! Received an indication that the "
+   "operating parameters on this target have "
+   "changed. The Linux SCSI layer does not "
+   "automatically adjust these parameters.\n");
+
+   if (sshdr->asc == 0x38 && sshdr->ascq == 0x07) {
+   evt_type = SDEV_EVT_SOFT_THRESHOLD_REACHED_REPORTED;
+   sdev_printk(KERN_WARNING, sdev,
+   "Warning! Received an indication that the "
+   "LUN reached a thin provisioning soft "
+   "threshold.\n");
+   }
+
+   if (sshdr->asc == 0x2a && sshdr->ascq == 0x01) {
+   evt_type = SDEV_EVT_MODE_PARAMETER_CHANGE_REPORTED;
+   sdev_printk(KERN_WARNING, sdev,
+   "Mode parameters changed");
+   } else if (sshdr->asc == 0x2a && sshdr->ascq == 0x09) {
+   evt_type = SDEV_EVT_CAPACITY_CHANGE_REPORTED;
+   sdev_printk(KERN_WARNING, sdev,
+   "Capacity data has changed");
+   } else if (sshdr->asc == 0x2a)
+   sdev_printk(KERN_WARNING, sdev,
+ 

[PATCH v4 05/10] scsi: Rename scsi_evt_thread() to scsi_evt_work()

[Ewan D. Milne]

From: "Ewan D. Milne" 

The scsi_evt_thread() function is not actually a thread, it is
a work function.  So it should be named scsi_evt_work() instead.

Signed-off-by: Ewan D. Milne 
---
 drivers/scsi/scsi_lib.c  | 4 ++--
 drivers/scsi/scsi_priv.h | 1 +
 drivers/scsi/scsi_scan.c | 3 +--
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 6585049..97699a5 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -2198,13 +2198,13 @@ static void scsi_evt_emit(struct scsi_device *sdev, 
struct scsi_event *evt)
 }
 
 /**
- * sdev_evt_thread - send a uevent for each scsi event
+ * scsi_evt_work - send a uevent for each scsi event
  * @work: work struct for scsi_device
  *
  * Dispatch queued events to their associated scsi_device kobjects
  * as uevents.
  */
-void scsi_evt_thread(struct work_struct *work)
+void scsi_evt_work(struct work_struct *work)
 {
struct scsi_device *sdev;
LIST_HEAD(event_list);
diff --git a/drivers/scsi/scsi_priv.h b/drivers/scsi/scsi_priv.h
index 07ce3f5..ed80f21 100644
--- a/drivers/scsi/scsi_priv.h
+++ b/drivers/scsi/scsi_priv.h
@@ -90,6 +90,7 @@ extern void scsi_exit_queue(void);
 struct request_queue;
 struct request;
 extern struct kmem_cache *scsi_sdb_cache;
+extern void scsi_evt_work(struct work_struct *work);
 
 /* scsi_proc.c */
 #ifdef CONFIG_SCSI_PROC_FS
diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
index 2e5fe58..0adfecb 100644
--- a/drivers/scsi/scsi_scan.c
+++ b/drivers/scsi/scsi_scan.c
@@ -244,7 +244,6 @@ static struct scsi_device *scsi_alloc_sdev(struct 
scsi_target *starget,
struct scsi_device *sdev;
int display_failure_msg = 1, ret;
struct Scsi_Host *shost = dev_to_shost(starget->dev.parent);
-   extern void scsi_evt_thread(struct work_struct *work);
extern void scsi_requeue_run_queue(struct work_struct *work);
 
sdev = kzalloc(sizeof(*sdev) + shost->transportt->device_size,
@@ -267,7 +266,7 @@ static struct scsi_device *scsi_alloc_sdev(struct 
scsi_target *starget,
INIT_LIST_HEAD(&sdev->starved_entry);
INIT_LIST_HEAD(&sdev->event_list);
spin_lock_init(&sdev->list_lock);
-   INIT_WORK(&sdev->event_work, scsi_evt_thread);
+   INIT_WORK(&sdev->event_work, scsi_evt_work);
INIT_WORK(&sdev->requeue_work, scsi_requeue_run_queue);
 
sdev->sdev_gendev.parent = get_device(&starget->dev);
-- 
1.7.11.7

--
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 v4 07/10] scsi: Clear expecting_cc_ua on successful commands

[Ewan D. Milne]

From: "Ewan D. Milne" 

If a device does not report a unit attention as expected,
clear the expecting_cc_ua flag so that we will not suppress
a future unit attention condition that is *not* expected.
INQUIRY and REPORT LUNS commands should not do this, however,
because they do not report pending unit attention conditions.

Signed-off-by: Ewan D. Milne 
---
 drivers/scsi/scsi_error.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c
index d0f71e5..96707a6 100644
--- a/drivers/scsi/scsi_error.c
+++ b/drivers/scsi/scsi_error.c
@@ -1540,6 +1540,13 @@ int scsi_decide_disposition(struct scsi_cmnd *scmd)
 */
return ADD_TO_MLQUEUE;
case GOOD:
+   /*
+* Reset expecting_cc_ua for all commands except INQUIRY and
+* REPORT LUNS, because if we didn't get a UA on this command
+* as expected, then we don't want to suppress a subsequent one.
+*/
+   if (scmd->cmnd[0] != INQUIRY && scmd->cmnd[0] != REPORT_LUNS)
+   scmd->device->expecting_cc_ua = 0;
scsi_handle_queue_ramp_up(scmd->device);
case COMMAND_TERMINATED:
return SUCCESS;
-- 
1.7.11.7

--
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 v4 10/10] scsi: Added scsi_target rescan capability to sysfs

[Ewan D. Milne]

From: "Ewan D. Milne" 

Add a "rescan" attribute to sysfs for scsi_target objects,
to permit them to be scanned for LUN changes (e.g. from udev).

Signed-off-by: Ewan D. Milne 
---
 drivers/scsi/scsi_priv.h  |  4 +++-
 drivers/scsi/scsi_scan.c  | 30 +++
 drivers/scsi/scsi_sysfs.c | 61 +++
 3 files changed, 67 insertions(+), 28 deletions(-)

diff --git a/drivers/scsi/scsi_priv.h b/drivers/scsi/scsi_priv.h
index ed80f21..7c33799 100644
--- a/drivers/scsi/scsi_priv.h
+++ b/drivers/scsi/scsi_priv.h
@@ -131,7 +131,9 @@ extern int scsi_sysfs_add_host(struct Scsi_Host *);
 extern int scsi_sysfs_register(void);
 extern void scsi_sysfs_unregister(void);
 extern void scsi_sysfs_device_initialize(struct scsi_device *);
-extern int scsi_sysfs_target_initialize(struct scsi_device *);
+extern void scsi_sysfs_target_initialize(struct scsi_target *,
+struct Scsi_Host *,
+struct device *parent);
 extern struct scsi_transport_template blank_transport_template;
 extern void __scsi_remove_device(struct scsi_device *);
 
diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
index 0adfecb..243c8b4 100644
--- a/drivers/scsi/scsi_scan.c
+++ b/drivers/scsi/scsi_scan.c
@@ -343,26 +343,6 @@ static void scsi_target_destroy(struct scsi_target 
*starget)
put_device(dev);
 }
 
-static void scsi_target_dev_release(struct device *dev)
-{
-   struct device *parent = dev->parent;
-   struct scsi_target *starget = to_scsi_target(dev);
-
-   kfree(starget);
-   put_device(parent);
-}
-
-static struct device_type scsi_target_type = {
-   .name = "scsi_target",
-   .release =  scsi_target_dev_release,
-};
-
-int scsi_is_target_device(const struct device *dev)
-{
-   return dev->type == &scsi_target_type;
-}
-EXPORT_SYMBOL(scsi_is_target_device);
-
 static struct scsi_target *__scsi_find_target(struct device *parent,
  int channel, uint id)
 {
@@ -413,15 +393,11 @@ static struct scsi_target *scsi_alloc_target(struct 
device *parent,
printk(KERN_ERR "%s: allocation failure\n", __func__);
return NULL;
}
-   dev = &starget->dev;
-   device_initialize(dev);
-   starget->reap_ref = 1;
-   dev->parent = get_device(parent);
-   dev_set_name(dev, "target%d:%d:%d", shost->host_no, channel, id);
-   dev->bus = &scsi_bus_type;
-   dev->type = &scsi_target_type;
starget->id = id;
starget->channel = channel;
+   scsi_sysfs_target_initialize(starget, shost, parent);
+   dev = &starget->dev;
+   starget->reap_ref = 1;
starget->can_queue = 0;
INIT_LIST_HEAD(&starget->siblings);
INIT_LIST_HEAD(&starget->devices);
diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
index d5d86b2..212b43a 100644
--- a/drivers/scsi/scsi_sysfs.c
+++ b/drivers/scsi/scsi_sysfs.c
@@ -1129,6 +1129,67 @@ int scsi_is_sdev_device(const struct device *dev)
 }
 EXPORT_SYMBOL(scsi_is_sdev_device);
 
+static ssize_t
+starget_store_rescan_field(struct device *dev, struct device_attribute *attr,
+  const char *buf, size_t count)
+{
+   struct scsi_target *starget = to_scsi_target(dev);
+
+   scsi_scan_target(starget->dev.parent, starget->channel, starget->id,
+SCAN_WILD_CARD, 1);
+   return count;
+}
+/* DEVICE_ATTR(rescan) clashes with dev_attr_rescan for sdev */
+struct device_attribute dev_attr_trescan =
+   __ATTR(rescan, S_IWUSR, NULL, starget_store_rescan_field);
+
+static struct attribute *scsi_target_attrs[] = {
+   &dev_attr_trescan.attr,
+   NULL
+};
+
+static struct attribute_group scsi_target_attr_group = {
+   .attrs =scsi_target_attrs,
+};
+
+static const struct attribute_group *scsi_target_attr_groups[] = {
+   &scsi_target_attr_group,
+   NULL
+};
+
+static void scsi_target_dev_release(struct device *dev)
+{
+   struct device *parent = dev->parent;
+   struct scsi_target *starget = to_scsi_target(dev);
+
+   kfree(starget);
+   put_device(parent);
+}
+
+static struct device_type scsi_target_type = {
+   .name = "scsi_target",
+   .release =  scsi_target_dev_release,
+   .groups =   scsi_target_attr_groups,
+};
+
+int scsi_is_target_device(const struct device *dev)
+{
+   return dev->type == &scsi_target_type;
+}
+EXPORT_SYMBOL(scsi_is_target_device);
+
+void scsi_sysfs_target_initialize(struct scsi_target *starget,
+ struct Scsi_Host *shost,
+ struct device *parent)
+{
+   device_initialize(&starget->dev);
+   starget->dev.parent = get_device(parent);
+   starget->dev.bus = &scsi_bus_type;
+   starget->dev.type = &scsi_target_type;
+   dev_set_name(&starget->dev, "target%d:%d:%d", shos

[PATCH v4 06/10] scsi: Move schedule_work() call to be outside lock

[Ewan D. Milne]

From: "Ewan D. Milne" 

The call to schedule_work() in sdev_evt_send() should
not be made while the sdev->list_lock is held.

Signed-off-by: Ewan D. Milne 
---
 drivers/scsi/scsi_lib.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 97699a5..f871ecf 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -2255,8 +2255,8 @@ void sdev_evt_send(struct scsi_device *sdev, struct 
scsi_event *evt)
 
spin_lock_irqsave(&sdev->list_lock, flags);
list_add_tail(&evt->node, &sdev->event_list);
-   schedule_work(&sdev->event_work);
spin_unlock_irqrestore(&sdev->list_lock, flags);
+   schedule_work(&sdev->event_work);
 }
 EXPORT_SYMBOL_GPL(sdev_evt_send);
 
-- 
1.7.11.7

--
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 v4 09/10] scsi_debug: Add optional unit attention reporting

[Ewan D. Milne]

From: "Ewan D. Milne" 

Added "report_ua" module parameter to control reporting
of unit attention conditions when the number of LUNs is
changed, or the virtual_gb size of the device is changed.

Also added capability to generate unit attention conditions:

   38 07 - THIN PROVISIONING SOFT THRESHOLD REACHED
   2A 01 - MODE PARAMETERS CHANGED

Signed-off-by: Ewan D. Milne 
---
 drivers/scsi/scsi_debug.c | 131 ++
 1 file changed, 131 insertions(+)

diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index 182d5a5..738b197 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -111,6 +111,7 @@ static const char * scsi_debug_version_date = "20100324";
 #define DEF_PTYPE   0
 #define DEF_SCSI_LEVEL   5/* INQUIRY, byte2 [5->SPC-3] */
 #define DEF_SECTOR_SIZE 512
+#define DEF_REPORT_UA 0
 #define DEF_UNMAP_ALIGNMENT 0
 #define DEF_UNMAP_GRANULARITY 1
 #define DEF_UNMAP_MAX_BLOCKS 0x
@@ -182,6 +183,7 @@ static int scsi_debug_physblk_exp = DEF_PHYSBLK_EXP;
 static int scsi_debug_ptype = DEF_PTYPE; /* SCSI peripheral type (0==disk) */
 static int scsi_debug_scsi_level = DEF_SCSI_LEVEL;
 static int scsi_debug_sector_size = DEF_SECTOR_SIZE;
+static int scsi_debug_report_ua = DEF_REPORT_UA;
 static int scsi_debug_virtual_gb = DEF_VIRTUAL_GB;
 static int scsi_debug_vpd_use_hostno = DEF_VPD_USE_HOSTNO;
 static unsigned int scsi_debug_lbpu = DEF_LBPU;
@@ -230,6 +232,8 @@ struct sdebug_dev_info {
char reset;
char stopped;
char used;
+   char luns_changed;
+   char sense_pending;
 };
 
 struct sdebug_host_info {
@@ -268,6 +272,7 @@ static int num_host_resets = 0;
 static int dix_writes;
 static int dix_reads;
 static int dif_errors;
+static int luns_changed;
 
 static DEFINE_SPINLOCK(queued_arr_lock);
 static DEFINE_RWLOCK(atomic_rw);
@@ -2756,6 +2761,7 @@ module_param_named(physblk_exp, scsi_debug_physblk_exp, 
int, S_IRUGO);
 module_param_named(ptype, scsi_debug_ptype, int, S_IRUGO | S_IWUSR);
 module_param_named(scsi_level, scsi_debug_scsi_level, int, S_IRUGO);
 module_param_named(sector_size, scsi_debug_sector_size, int, S_IRUGO);
+module_param_named(report_ua, scsi_debug_report_ua, int, S_IRUGO | S_IWUSR);
 module_param_named(unmap_alignment, scsi_debug_unmap_alignment, int, S_IRUGO);
 module_param_named(unmap_granularity, scsi_debug_unmap_granularity, int, 
S_IRUGO);
 module_param_named(unmap_max_blocks, scsi_debug_unmap_max_blocks, int, 
S_IRUGO);
@@ -2798,6 +2804,7 @@ MODULE_PARM_DESC(physblk_exp, "physical block exponent 
(def=0)");
 MODULE_PARM_DESC(ptype, "SCSI peripheral type(def=0[disk])");
 MODULE_PARM_DESC(scsi_level, "SCSI level to simulate(def=5[SPC-3])");
 MODULE_PARM_DESC(sector_size, "logical block size in bytes (def=512)");
+MODULE_PARM_DESC(report_ua, "report unit attentions when reconfigured 
(def=0)");
 MODULE_PARM_DESC(unmap_alignment, "lowest aligned thin provisioning lba 
(def=0)");
 MODULE_PARM_DESC(unmap_granularity, "thin provisioning granularity in blocks 
(def=1)");
 MODULE_PARM_DESC(unmap_max_blocks, "max # of blocks can be unmapped in one cmd 
(def=0x)");
@@ -3050,10 +3057,37 @@ static ssize_t sdebug_max_luns_store(struct 
device_driver * ddp,
 const char * buf, size_t count)
 {
 int n;
+   struct sdebug_host_info *sdbg_host;
+   struct sdebug_dev_info *devip;
 
if ((count > 0) && (1 == sscanf(buf, "%d", &n)) && (n >= 0)) {
scsi_debug_max_luns = n;
sdebug_max_tgts_luns();
+
+   if (!scsi_debug_report_ua)
+   return count;
+
+   /*
+* SPC-3 behavior is to report a UNIT ATTENTION with ASC/ASCQ
+* REPORTED LUNS DATA HAS CHANGED on every LUN on the target,
+* until a REPORT LUNS command is received.  SPC-4 behavior is
+* to report it only once.  NOTE:  scsi_debug_scsi_level does
+* not use the same values as struct scsi_device->scsi_level.
+*/
+   if (scsi_debug_scsi_level == 5) {   /* SPC-3 */
+   spin_lock(&sdebug_host_list_lock);
+   list_for_each_entry(sdbg_host, &sdebug_host_list,
+   host_list) {
+   list_for_each_entry(devip,
+   &sdbg_host->dev_info_list,
+   dev_list) {
+   devip->luns_changed = 1;
+   }
+   }
+   spin_unlock(&sdebug_host_list_lock);
+   } else if (scsi_debug_scsi_level > 5)
+   luns_changed = 1;
+
return count;
}
return -EINVAL;
@@ -3100,12 +3134,27 @@ static ssize_t sdebug_virtual_gb_store(struct 
device_driver * ddp,
 

[PATCH v4 01/10] scsi: Fix incorrect function name in comment

[Ewan D. Milne]

From: "Ewan D. Milne" 

The function name is "scsi_evt_emit", not "sdev_evt_emit".

Signed-off-by: Ewan D. Milne 
---
 drivers/scsi/scsi_lib.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 6dfb978..f6499db 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -2171,7 +2171,7 @@ scsi_device_set_state(struct scsi_device *sdev, enum 
scsi_device_state state)
 EXPORT_SYMBOL(scsi_device_set_state);
 
 /**
- * sdev_evt_emit - emit a single SCSI device uevent
+ * scsi_evt_emit - emit a single SCSI device uevent
  * @sdev: associated SCSI device
  * @evt: event to emit
  *
-- 
1.7.11.7

--
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 v4 04/10] scsi: Change to use list_for_each_entry_safe

[Ewan D. Milne]

From: "Ewan D. Milne" 

scsi_device_dev_release_usercontext() should be using
"list_for_each_entry_safe" instead of "list_for_each_safe".

Signed-off-by: Ewan D. Milne 
---
 drivers/scsi/scsi_sysfs.c | 7 ++-
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
index 7394a77..34f7580 100644
--- a/drivers/scsi/scsi_sysfs.c
+++ b/drivers/scsi/scsi_sysfs.c
@@ -335,7 +335,7 @@ static void scsi_device_dev_release_usercontext(struct 
work_struct *work)
struct scsi_device *sdev;
struct device *parent;
struct scsi_target *starget;
-   struct list_head *this, *tmp;
+   struct scsi_event *evt, *next;
unsigned long flags;
 
sdev = container_of(work, struct scsi_device, ew.work);
@@ -352,10 +352,7 @@ static void scsi_device_dev_release_usercontext(struct 
work_struct *work)
 
cancel_work_sync(&sdev->event_work);
 
-   list_for_each_safe(this, tmp, &sdev->event_list) {
-   struct scsi_event *evt;
-
-   evt = list_entry(this, struct scsi_event, node);
+   list_for_each_entry_safe(evt, next, &sdev->event_list, node) {
list_del(&evt->node);
kfree(evt);
}
-- 
1.7.11.7

--
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 v4 03/10] scsi: Add missing newline to scsi_sysfs.c

[Ewan D. Milne]

From: "Ewan D. Milne" 

show_iostat_counterbits() is obviously missing a newline in
the function declaration.

Signed-off-by: Ewan D. Milne 
---
 drivers/scsi/scsi_sysfs.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
index 04c2a27..7394a77 100644
--- a/drivers/scsi/scsi_sysfs.c
+++ b/drivers/scsi/scsi_sysfs.c
@@ -647,7 +647,8 @@ show_queue_type_field(struct device *dev, struct 
device_attribute *attr,
 static DEVICE_ATTR(queue_type, S_IRUGO, show_queue_type_field, NULL);
 
 static ssize_t
-show_iostat_counterbits(struct device *dev, struct device_attribute *attr, 
char *buf)
+show_iostat_counterbits(struct device *dev, struct device_attribute *attr,
+   char *buf)
 {
return snprintf(buf, 20, "%d\n", (int)sizeof(atomic_t) * 8);
 }
-- 
1.7.11.7

--
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 v4 02/10] scsi: Correct size of envp[]

[Ewan D. Milne]

From: "Ewan D. Milne" 

The envp[] array in scsi_evt_emit() only needs to have 2 entries.

Signed-off-by: Ewan D. Milne 
---
 drivers/scsi/scsi_lib.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index f6499db..6585049 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -2180,7 +2180,7 @@ EXPORT_SYMBOL(scsi_device_set_state);
 static void scsi_evt_emit(struct scsi_device *sdev, struct scsi_event *evt)
 {
int idx = 0;
-   char *envp[3];
+   char *envp[2];
 
switch (evt->evt_type) {
case SDEV_EVT_MEDIA_CHANGE:
-- 
1.7.11.7

--
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 v4 00/10] Enhanced Unit Attention handling

[Ewan D. Milne]

From: "Ewan D. Milne" 

This patch set adds changes to the SCSI mid-layer, sysfs and scsi_debug
to provide enhanced support for Unit Attention conditions.

There was some discussion about this a couple of years ago on the linux-scsi
mailing list:  http://marc.info/?l=linux-scsi&m=129702506514742&w=2
Although one approach is to send all SCSI sense data to a userspace daemon
for processing, this patch set does not take that approach due to the
difficulty in reliably delivering all of the data.  An interesting UA
condition might not be delivered due to a flood of media errors, for example.

The mechanism used is to flag when certain UA ASC/ASCQ codes are received
that report asynchronous changes to the storage device configuration.
An appropriate uevent is then generated for the scsi_device or scsi_target
object.  The expecting_cc_ua flag is used for REPORTED LUNS DATA HAS CHANGED
unit attention conditions to avoid generating duplicate events on multiple
LUNs.

Changes made since earlier v3 version:
   - Put fixes to existing code in separate individual patches
   - Removed UA queue overflow condition reporting
   - Removed delayed_work aggregation mechanism for events
   - Eliminated separate scsi_device and scsi_target mechanisms
   - Changed to use a single environment variable for uevents
   - Clear expecting_cc_ua on successful commands
   - Added use of expecting_cc_ua for REPORTED LUNS DATA HAS CHANGED

Changes made since earlier v2 version:

   - Remove patch 1/8 "Generate uevent on sd capacity change"
   - Remove patch 8/8 "Streamline detection of FM/EOM/ILI status"
   - Changed scsi_debug to not generate UA on INQUIRY or REPORT_LUNS
   - Changed scsi_debug to only report UA queue overflow condition
 if dsense=1, as descriptor format sense data is needed

Changes made since earlier RFC version:

   - Remove patch 1/9 "Detect overflow of sense data buffer"
 Some scsi_debug changes in this patch were moved to patch 7/8
   - Corrected Kconfig help text
   - Change name of "sdev_evt_thread" to "sdev_evt_work"
   - Change name of "starget_evt_thread" to "starget_evt_work"
   - Pull code out of scsi_check_sense() that handles UAs into
 an exported function so that drivers can report conditions
 received asynchronously

Thanks to everyone for the comments on this patch series.

Ewan D. Milne (10):
  scsi: Fix incorrect function name in comment
  scsi: Correct size of envp[]
  scsi: Add missing newline to scsi_sysfs.c
  scsi: Change to use list_for_each_entry_safe
  scsi: Rename scsi_evt_thread() to scsi_evt_work()
  scsi: Move schedule_work() call to be outside lock
  scsi: Clear expecting_cc_ua on successful commands
  scsi: Generate uevents on certain unit attention codes
  scsi_debug: Add optional unit attention reporting
  scsi: Added scsi_target rescan capability to sysfs

 drivers/scsi/scsi_debug.c  | 131 +
 drivers/scsi/scsi_error.c  | 125 ++
 drivers/scsi/scsi_lib.c|  52 +++---
 drivers/scsi/scsi_priv.h   |   5 +-
 drivers/scsi/scsi_scan.c   |  33 ++--
 drivers/scsi/scsi_sysfs.c  |  81 +---
 include/scsi/scsi_device.h |  11 +++-
 7 files changed, 372 insertions(+), 66 deletions(-)

-- 
1.7.11.7

--
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: [SCSI REGRESSION] 3.10.2 or 3.10.3: arcmsr failure at bootup / early userspace transition

[Bernd Schubert]

On 08/01/2013 06:04 PM, Nix wrote:

On 1 Aug 2013, Bernd Schubert verbalised:


On 07/30/2013 11:20 PM, Nix wrote:

On 30 Jul 2013, Bernd Schubert told this:


On 07/30/2013 02:56 AM, Nix wrote:

On 30 Jul 2013, Douglas Gilbert outgrape:


Please supply the information that Martin Petersen asked
for.


Did it in private IRC (the advantage of working for the same division of
the same company!)

I didn't realise the original fix was actually implemented to allow
Bernd, with a different Areca controller, to boot... obviously, in that
situation, reversion is wrong, since that would just replace one won't-
boot situation with another.


Unless there is very simple fix the commit should reverted, imho. It
would better then to remove write-same support from the md-layer.


I'm not using md on that machine, just LVM. Our suspicion is that ext4
is doing a WRITE SAME for some reason.


I didn't check yet for other cases, mkfs.ext4 does WRITE SAME and with
lazy init it also will happen after mounting the file system, while
lazy init is running (inode zeroing).


Well, it'll happen the first few times you mount the fs. If your fs is
years old (as mine are) the inode tables will probably have been
initialized by now!



I'm frequently doing tests with millions of files and reformating is 
ways faster than deleting the all these files.

--
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/3] hpsa: remove unneeded loop

[scameron]

On Thu, Aug 01, 2013 at 05:39:36PM +0200, Tomas Henzl wrote:
> On 08/01/2013 05:19 PM, scame...@beardog.cce.hp.com wrote:

[...]

> >> Btw. on line 1284 - isn't it similar to patch 2/3 ?

^^^ Oh, missed this the first time around, the sop driver uses the 
make_request_fn()
interface, and it's not a stacked driver either, so there is no limit to the 
number
of bios the block layer can stuff in -- make_request_fn must succeed.
If we get full we just chain them together using pointers already in the struct
bio for that purpose, so storing them in the driver requires no memory 
allocation
on the driver's part.  So while it's somewhat similar, we already have to handle
the case of the block layer stuffing infinite bios into the driver, so getting
full is not terribly out of the ordinary in that driver.

That being said, I'm poking around other bits of code lying around here
looking for similar problems, so thanks again for that one.

> > find_first_zero_bit is not atomic, but the test_and_set_bit, which is what
> > counts, is atomic.   That find_first_zero_bit is not atomic confused me 
> > about
> > this code for a long time, and is why the spin lock was there in the first
> > place.  But if there's a race on the find_first_zero_bit and it returns the
> > same bit to multiple concurrent threads, only one thread will win the
> > test_and_set_bit, and the other threads will go back around the loop to try
> > again, and get a different bit.
> 
> Yes.
> But, let's expect just one zero bit at the end of the list. The 
> find_first_zero_bit(ffzb)
> starts now,  thread+1 zeroes a new bit at the beginning, ffzb continues,
> thread+2 takes the zero bit at the end. The result it that ffzb hasn't found 
> a zero bit
> even though that at every moment that bit was there.Ffter that the function 
> returns -EBUSY.
> rc = (u16) find_first_zero_bit(qinfo->request_bits, qinfo->qdepth);
> if (rc >= qinfo->qdepth-1)
>   return (u16) -EBUSY;
> Still, I think that this is almost impossible, and if it should happen
> a requeue is not so bad.

Oh, wow.  Didn't think of that.  Hmm, technically no guarantee that
any given thread would ever get a bit, if all the other threads keep
snatching them away just ahead of an unlucky thread.

Could we, instead of giving up, go back around and try again on the theory
that some bits should be free in there someplace and the thread shouldn't
be infinitely unlucky?

[...]

-- steve
--
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: [SCSI REGRESSION] 3.10.2 or 3.10.3: arcmsr failure at bootup / early userspace transition

[Nix]

On 1 Aug 2013, Bernd Schubert verbalised:

> On 07/30/2013 11:20 PM, Nix wrote:
>> On 30 Jul 2013, Bernd Schubert told this:
>>
>>> On 07/30/2013 02:56 AM, Nix wrote:
 On 30 Jul 2013, Douglas Gilbert outgrape:

> Please supply the information that Martin Petersen asked
> for.

 Did it in private IRC (the advantage of working for the same division of
 the same company!)

 I didn't realise the original fix was actually implemented to allow
 Bernd, with a different Areca controller, to boot... obviously, in that
 situation, reversion is wrong, since that would just replace one won't-
 boot situation with another.
>>>
>>> Unless there is very simple fix the commit should reverted, imho. It
>>> would better then to remove write-same support from the md-layer.
>>
>> I'm not using md on that machine, just LVM. Our suspicion is that ext4
>> is doing a WRITE SAME for some reason.
>
> I didn't check yet for other cases, mkfs.ext4 does WRITE SAME and with
> lazy init it also will happen after mounting the file system, while
> lazy init is running (inode zeroing).

Well, it'll happen the first few times you mount the fs. If your fs is
years old (as mine are) the inode tables will probably have been
initialized by now!

-- 
NULL && (void)
--
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: Ejected Nook (usb mass storage) prevents suspend

[Alan Stern]

On Thu, 1 Aug 2013, Oliver Neukum wrote:

> On Wed, 2013-07-31 at 11:13 -0400, Alan Stern wrote:
> 
> > More importantly, if we already know that the medium is not present or
> > has been changed since it was last used, then there's no reason to call
> > sd_sync_cache() at all.
> 
> Like this?

Yes, I like this a lot better, except I would put the test for 
!sdkp->media_present in sd_suspend_common() -- no need to print the 
"Synchronizing SCSI Cache" message if you're not going to go through 
with it.

What do the SCSI people think?

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: [PATCH 1/3] hpsa: remove unneeded loop

[Tomas Henzl]

On 08/01/2013 05:19 PM, scame...@beardog.cce.hp.com wrote:
> On Thu, Aug 01, 2013 at 04:59:45PM +0200, Tomas Henzl wrote:
>> On 08/01/2013 04:21 PM, scame...@beardog.cce.hp.com wrote:
>>> On Thu, Aug 01, 2013 at 04:05:20PM +0200, Tomas Henzl wrote:
 On 08/01/2013 03:39 PM, scame...@beardog.cce.hp.com wrote:
> On Thu, Aug 01, 2013 at 03:11:22PM +0200, Tomas Henzl wrote:
>> From: Tomas Henzl 
>>
>> The cmd_pool_bits is protected everywhere with a spinlock, 
>> we don't need the test_and_set_bit, set_bit is enough and the loop
>> can be removed too.
>>
>> Signed-off-by: Tomas Henzl 
>> ---
>>  drivers/scsi/hpsa.c | 15 ++-
>>  1 file changed, 6 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c
>> index 796482b..d7df01e 100644
>> --- a/drivers/scsi/hpsa.c
>> +++ b/drivers/scsi/hpsa.c
>> @@ -2662,15 +2662,12 @@ static struct CommandList *cmd_alloc(struct 
>> ctlr_info *h)
>>  unsigned long flags;
>>  
>>  spin_lock_irqsave(&h->lock, flags);
>> -do {
>> -i = find_first_zero_bit(h->cmd_pool_bits, h->nr_cmds);
>> -if (i == h->nr_cmds) {
>> -spin_unlock_irqrestore(&h->lock, flags);
>> -return NULL;
>> -}
>> -} while (test_and_set_bit
>> - (i & (BITS_PER_LONG - 1),
>> -  h->cmd_pool_bits + (i / BITS_PER_LONG)) != 0);
>> +i = find_first_zero_bit(h->cmd_pool_bits, h->nr_cmds);
>> +if (i == h->nr_cmds) {
>> +spin_unlock_irqrestore(&h->lock, flags);
>> +return NULL;
>> +}
>> +set_bit(i & (BITS_PER_LONG - 1), h->cmd_pool_bits + (i / 
>> BITS_PER_LONG));
>>  h->nr_allocs++;
>>  spin_unlock_irqrestore(&h->lock, flags);
>>  
>> -- 
>> 1.8.3.1
>>
> Would it be better instead to just not use the spinlock for protecting
> cmd_pool_bits?  I have thought about doing this for awhile, but haven't
> gotten around to it.
>
> I think the while loop is safe without the spin lock.  And then it is
> not needed in cmd_free either.
 I was evaluating the same idea for a while too, a loop and inside just the 
 test_and_set_bit,
 maybe even a stored value to start with a likely empty bit from last time 
 to tune it a bit.
 But I know almost nothing about the use pattern, so I decided for the 
 least invasive change
 to the existing code, to not make it worse.
>>> Only reason I haven't done it is I'm loathe to make such a change to the 
>>> main i/o
>>> path without testing it like crazy before unleashing it, and it's never 
>>> been a 
>>> convenient time to slide such a change in around here and get proper testing
>>> done (and there are other rather large changes brewing).
>>>
>>> However, we have been using a similar scheme with the SCSI over PCIe driver,
>>> here: 
>>> https://github.com/HPSmartStorage/scsi-over-pcie/blob/master/block/sop.c
>>> in alloc_request() around line 1476 without problems, and nvme-core.c 
>>> contains
>>> similar code in alloc_cmdid(), so I am confident it's sound in principle.
>>> I would want to beat on it though, in case it ends up exposing a firmware 
>>> bug
>>> or something (not that I think it will, but you never know.)
>> I think the code is sound, maybe it could hypothetically return -EBUSY, 
>> because
>> the find_first_zero_bit is not atomic, but this possibility is so low that 
>> it doesn't matter.
>> Btw. on line 1284 - isn't it similar to patch 2/3 ?
> find_first_zero_bit is not atomic, but the test_and_set_bit, which is what
> counts, is atomic.   That find_first_zero_bit is not atomic confused me about
> this code for a long time, and is why the spin lock was there in the first
> place.  But if there's a race on the find_first_zero_bit and it returns the
> same bit to multiple concurrent threads, only one thread will win the
> test_and_set_bit, and the other threads will go back around the loop to try
> again, and get a different bit.

Yes.
But, let's expect just one zero bit at the end of the list. The 
find_first_zero_bit(ffzb)
starts now,  thread+1 zeroes a new bit at the beginning, ffzb continues,
thread+2 takes the zero bit at the end. The result it that ffzb hasn't found a 
zero bit
even though that at every moment that bit was there.Ffter that the function 
returns -EBUSY.
rc = (u16) find_first_zero_bit(qinfo->request_bits, qinfo->qdepth);
if (rc >= qinfo->qdepth-1)
return (u16) -EBUSY;
Still, I think that this is almost impossible, and if it should happen
a requeue is not so bad.

>
> I don't think a thread can get stuck in there never winning until all the bits
> are used up because there should always be enough bits for all the commands we
> would ever try to send con

Re: [PATCH 1/3] hpsa: remove unneeded loop

[scameron]

On Thu, Aug 01, 2013 at 04:59:45PM +0200, Tomas Henzl wrote:
> On 08/01/2013 04:21 PM, scame...@beardog.cce.hp.com wrote:
> > On Thu, Aug 01, 2013 at 04:05:20PM +0200, Tomas Henzl wrote:
> >> On 08/01/2013 03:39 PM, scame...@beardog.cce.hp.com wrote:
> >>> On Thu, Aug 01, 2013 at 03:11:22PM +0200, Tomas Henzl wrote:
>  From: Tomas Henzl 
> 
>  The cmd_pool_bits is protected everywhere with a spinlock, 
>  we don't need the test_and_set_bit, set_bit is enough and the loop
>  can be removed too.
> 
>  Signed-off-by: Tomas Henzl 
>  ---
>   drivers/scsi/hpsa.c | 15 ++-
>   1 file changed, 6 insertions(+), 9 deletions(-)
> 
>  diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c
>  index 796482b..d7df01e 100644
>  --- a/drivers/scsi/hpsa.c
>  +++ b/drivers/scsi/hpsa.c
>  @@ -2662,15 +2662,12 @@ static struct CommandList *cmd_alloc(struct 
>  ctlr_info *h)
>   unsigned long flags;
>   
>   spin_lock_irqsave(&h->lock, flags);
>  -do {
>  -i = find_first_zero_bit(h->cmd_pool_bits, h->nr_cmds);
>  -if (i == h->nr_cmds) {
>  -spin_unlock_irqrestore(&h->lock, flags);
>  -return NULL;
>  -}
>  -} while (test_and_set_bit
>  - (i & (BITS_PER_LONG - 1),
>  -  h->cmd_pool_bits + (i / BITS_PER_LONG)) != 0);
>  +i = find_first_zero_bit(h->cmd_pool_bits, h->nr_cmds);
>  +if (i == h->nr_cmds) {
>  +spin_unlock_irqrestore(&h->lock, flags);
>  +return NULL;
>  +}
>  +set_bit(i & (BITS_PER_LONG - 1), h->cmd_pool_bits + (i / 
>  BITS_PER_LONG));
>   h->nr_allocs++;
>   spin_unlock_irqrestore(&h->lock, flags);
>   
>  -- 
>  1.8.3.1
> 
> >>> Would it be better instead to just not use the spinlock for protecting
> >>> cmd_pool_bits?  I have thought about doing this for awhile, but haven't
> >>> gotten around to it.
> >>>
> >>> I think the while loop is safe without the spin lock.  And then it is
> >>> not needed in cmd_free either.
> >> I was evaluating the same idea for a while too, a loop and inside just the 
> >> test_and_set_bit,
> >> maybe even a stored value to start with a likely empty bit from last time 
> >> to tune it a bit.
> >> But I know almost nothing about the use pattern, so I decided for the 
> >> least invasive change
> >> to the existing code, to not make it worse.
> > Only reason I haven't done it is I'm loathe to make such a change to the 
> > main i/o
> > path without testing it like crazy before unleashing it, and it's never 
> > been a 
> > convenient time to slide such a change in around here and get proper testing
> > done (and there are other rather large changes brewing).
> >
> > However, we have been using a similar scheme with the SCSI over PCIe driver,
> > here: 
> > https://github.com/HPSmartStorage/scsi-over-pcie/blob/master/block/sop.c
> > in alloc_request() around line 1476 without problems, and nvme-core.c 
> > contains
> > similar code in alloc_cmdid(), so I am confident it's sound in principle.
> > I would want to beat on it though, in case it ends up exposing a firmware 
> > bug
> > or something (not that I think it will, but you never know.)
> 
> I think the code is sound, maybe it could hypothetically return -EBUSY, 
> because
> the find_first_zero_bit is not atomic, but this possibility is so low that it 
> doesn't matter.
> Btw. on line 1284 - isn't it similar to patch 2/3 ?

find_first_zero_bit is not atomic, but the test_and_set_bit, which is what
counts, is atomic.   That find_first_zero_bit is not atomic confused me about
this code for a long time, and is why the spin lock was there in the first
place.  But if there's a race on the find_first_zero_bit and it returns the
same bit to multiple concurrent threads, only one thread will win the
test_and_set_bit, and the other threads will go back around the loop to try
again, and get a different bit.

I don't think a thread can get stuck in there never winning until all the bits
are used up because there should always be enough bits for all the commands we
would ever try to send concurrently, so every thread that gets in there should
eventually get a bit.

Or, am I missing some subtlety?

> 
> Back to this patch - we can take it as it is, because of the spinlock it 
> should be safe,
> or omit it, you can then post a spinlock-less patch. I'll let the decision on 
> you.

I think I like the spin-lock-less variant better.  But I want to test it out
here for awhile first.

-- steve

> 
> tomash
> 
> 
> >
> > -- steve
> >
> >
> >  
> >>
> >>> -- steve
> >>>
> >>> --
> >>> 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  h

Re: [PATCH 1/3] hpsa: remove unneeded loop

[Tomas Henzl]

On 08/01/2013 04:21 PM, scame...@beardog.cce.hp.com wrote:
> On Thu, Aug 01, 2013 at 04:05:20PM +0200, Tomas Henzl wrote:
>> On 08/01/2013 03:39 PM, scame...@beardog.cce.hp.com wrote:
>>> On Thu, Aug 01, 2013 at 03:11:22PM +0200, Tomas Henzl wrote:
 From: Tomas Henzl 

 The cmd_pool_bits is protected everywhere with a spinlock, 
 we don't need the test_and_set_bit, set_bit is enough and the loop
 can be removed too.

 Signed-off-by: Tomas Henzl 
 ---
  drivers/scsi/hpsa.c | 15 ++-
  1 file changed, 6 insertions(+), 9 deletions(-)

 diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c
 index 796482b..d7df01e 100644
 --- a/drivers/scsi/hpsa.c
 +++ b/drivers/scsi/hpsa.c
 @@ -2662,15 +2662,12 @@ static struct CommandList *cmd_alloc(struct 
 ctlr_info *h)
unsigned long flags;
  
spin_lock_irqsave(&h->lock, flags);
 -  do {
 -  i = find_first_zero_bit(h->cmd_pool_bits, h->nr_cmds);
 -  if (i == h->nr_cmds) {
 -  spin_unlock_irqrestore(&h->lock, flags);
 -  return NULL;
 -  }
 -  } while (test_and_set_bit
 -   (i & (BITS_PER_LONG - 1),
 -h->cmd_pool_bits + (i / BITS_PER_LONG)) != 0);
 +  i = find_first_zero_bit(h->cmd_pool_bits, h->nr_cmds);
 +  if (i == h->nr_cmds) {
 +  spin_unlock_irqrestore(&h->lock, flags);
 +  return NULL;
 +  }
 +  set_bit(i & (BITS_PER_LONG - 1), h->cmd_pool_bits + (i / 
 BITS_PER_LONG));
h->nr_allocs++;
spin_unlock_irqrestore(&h->lock, flags);
  
 -- 
 1.8.3.1

>>> Would it be better instead to just not use the spinlock for protecting
>>> cmd_pool_bits?  I have thought about doing this for awhile, but haven't
>>> gotten around to it.
>>>
>>> I think the while loop is safe without the spin lock.  And then it is
>>> not needed in cmd_free either.
>> I was evaluating the same idea for a while too, a loop and inside just the 
>> test_and_set_bit,
>> maybe even a stored value to start with a likely empty bit from last time to 
>> tune it a bit.
>> But I know almost nothing about the use pattern, so I decided for the least 
>> invasive change
>> to the existing code, to not make it worse.
> Only reason I haven't done it is I'm loathe to make such a change to the main 
> i/o
> path without testing it like crazy before unleashing it, and it's never been 
> a 
> convenient time to slide such a change in around here and get proper testing
> done (and there are other rather large changes brewing).
>
> However, we have been using a similar scheme with the SCSI over PCIe driver,
> here: https://github.com/HPSmartStorage/scsi-over-pcie/blob/master/block/sop.c
> in alloc_request() around line 1476 without problems, and nvme-core.c contains
> similar code in alloc_cmdid(), so I am confident it's sound in principle.
> I would want to beat on it though, in case it ends up exposing a firmware bug
> or something (not that I think it will, but you never know.)

I think the code is sound, maybe it could hypothetically return -EBUSY, because
the find_first_zero_bit is not atomic, but this possibility is so low that it 
doesn't matter.
Btw. on line 1284 - isn't it similar to patch 2/3 ?

Back to this patch - we can take it as it is, because of the spinlock it should 
be safe,
or omit it, you can then post a spinlock-less patch. I'll let the decision on 
you.

tomash


>
> -- steve
>
>
>  
>>
>>> -- steve
>>>
>>> --
>>> 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: [SCSI REGRESSION] 3.10.2 or 3.10.3: arcmsr failure at bootup / early userspace transition

[Bernd Schubert]

On 07/30/2013 11:20 PM, Nix wrote:

On 30 Jul 2013, Bernd Schubert told this:


On 07/30/2013 02:56 AM, Nix wrote:

On 30 Jul 2013, Douglas Gilbert outgrape:


Please supply the information that Martin Petersen asked
for.


Did it in private IRC (the advantage of working for the same division of
the same company!)

I didn't realise the original fix was actually implemented to allow
Bernd, with a different Areca controller, to boot... obviously, in that
situation, reversion is wrong, since that would just replace one won't-
boot situation with another.


Unless there is very simple fix the commit should reverted, imho. It
would better then to remove write-same support from the md-layer.


I'm not using md on that machine, just LVM. Our suspicion is that ext4
is doing a WRITE SAME for some reason.



I didn't check yet for other cases, mkfs.ext4 does WRITE SAME and with 
lazy init it also will happen after mounting the file system, while lazy 
init is running (inode zeroing).



Cheers,
Bernd
--
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 disk: Use its own buffer for the vpd request

[Bernd Schubert]

Whoops, the title is wrong, it should have been:

[PATCH] scsi disk: Limit get_vpd_page buf size

On 08/01/2013 04:34 PM, Bernd Schubert wrote:

Once I noticed that scsi_get_vpd_page() works fine from other function
calls and that it is not 0x89, but already 0x0 that fails fixing it became
easy.

Nix, any chance you could verify it also works for you?


From: Bernd Schubert 

Somehow older areca firmware versions have issues with
scsi_get_vpd_page() and a large buffer.
Even scsi_get_vpd_page(, page=0,)  failed in sd_read_write_same(),
while a similar request from sd_read_block_limits() worked fine.
Limiting the buf-size to 64-bytes fixes the issue with F/W V1.46.

Fixes a regression with areca controllers and older firmware versions
introduced by commit: 66c28f97120e8a621afd5aa7a31c4b85c547d33d

Reported-by: Nix 
Signed-off-by: Bernd Schubert 
CC: sta...@vger.kernel.org
---
  drivers/scsi/sd.c |5 -
  1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index 80f39b8..02e50ae 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -2651,13 +2651,16 @@ static void sd_read_write_same(struct scsi_disk *sdkp, 
unsigned char *buffer)
struct scsi_device *sdev = sdkp->device;

if (scsi_report_opcode(sdev, buffer, SD_BUF_SIZE, INQUIRY) < 0) {
+   /* too large values might cause issues with arcmsr */
+   int vpd_buf_len = 64;
+
sdev->no_report_opcodes = 1;

/* Disable WRITE SAME if REPORT SUPPORTED OPERATION
 * CODES is unsupported and the device has an ATA
 * Information VPD page (SAT).
 */
-   if (!scsi_get_vpd_page(sdev, 0x89, buffer, SD_BUF_SIZE))
+   if (!scsi_get_vpd_page(sdev, 0x89, buffer, vpd_buf_len))
sdev->no_write_same = 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] scsi disk: Use its own buffer for the vpd request

[Bernd Schubert]

Once I noticed that scsi_get_vpd_page() works fine from other function
calls and that it is not 0x89, but already 0x0 that fails fixing it became
easy.

Nix, any chance you could verify it also works for you?


From: Bernd Schubert 

Somehow older areca firmware versions have issues with
scsi_get_vpd_page() and a large buffer.
Even scsi_get_vpd_page(, page=0,)  failed in sd_read_write_same(),
while a similar request from sd_read_block_limits() worked fine.
Limiting the buf-size to 64-bytes fixes the issue with F/W V1.46.

Fixes a regression with areca controllers and older firmware versions
introduced by commit: 66c28f97120e8a621afd5aa7a31c4b85c547d33d

Reported-by: Nix 
Signed-off-by: Bernd Schubert 
CC: sta...@vger.kernel.org
---
 drivers/scsi/sd.c |5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index 80f39b8..02e50ae 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -2651,13 +2651,16 @@ static void sd_read_write_same(struct scsi_disk *sdkp, 
unsigned char *buffer)
struct scsi_device *sdev = sdkp->device;
 
if (scsi_report_opcode(sdev, buffer, SD_BUF_SIZE, INQUIRY) < 0) {
+   /* too large values might cause issues with arcmsr */
+   int vpd_buf_len = 64;
+
sdev->no_report_opcodes = 1;
 
/* Disable WRITE SAME if REPORT SUPPORTED OPERATION
 * CODES is unsupported and the device has an ATA
 * Information VPD page (SAT).
 */
-   if (!scsi_get_vpd_page(sdev, 0x89, buffer, SD_BUF_SIZE))
+   if (!scsi_get_vpd_page(sdev, 0x89, buffer, vpd_buf_len))
sdev->no_write_same = 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

[Bug 60644] MPT2SAS drops all HDDs when under high I/O

[bugzilla-daemon]

https://bugzilla.kernel.org/show_bug.cgi?id=60644

--- Comment #16 from livea...@live.com ---
Hello .

The thing is that I'm using SATA drives and not SAS drives . The motherboard
exposes the LSI controller as 8 SATA ports .

This wasn't an issue under Windows 2012 , so I think that hardware issues are
pretty much not the cause in here .

Sorry if I'm demanding too much , but can you try to create a BTRFS RAID1 ,
fill it with data and then run :

btrfs scrub start /MOUNTPOINT

It always produces the issue in less than 2 minutes .

Thank you .

-- 
You are receiving this mail because:
You are watching the assignee of the bug.
--
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

[Bug 60644] MPT2SAS drops all HDDs when under high I/O

[bugzilla-daemon]

https://bugzilla.kernel.org/show_bug.cgi?id=60644

--- Comment #15 from Sreekanth Reddy  ---
(In reply to liveaxle from comment #14)
> Hi .

> Any updates regarding this bug ?

I tried to reproduce this issue locally, but for me this issue is not
reproduced.

Here are the steps which I followed to reproduce the issue

1. I have created RAID0 vloume on two 500 GB SAS drives.
2. Created the EXT4 file system.
3. Mounted this FS to /mnt
4. And run the IO's using cmd 'dd if=/dev/zero of=/mnt/dd.img bs=1G count=300'

Result:
IO's run successfully with out any issue.

Please let me know whether I have missed any steps while reproducing this
issue.

-- 
You are receiving this mail because:
You are watching the assignee of the bug.
--
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/3] hpsa: remove unneeded loop

[scameron]

On Thu, Aug 01, 2013 at 04:05:20PM +0200, Tomas Henzl wrote:
> On 08/01/2013 03:39 PM, scame...@beardog.cce.hp.com wrote:
> > On Thu, Aug 01, 2013 at 03:11:22PM +0200, Tomas Henzl wrote:
> >> From: Tomas Henzl 
> >>
> >> The cmd_pool_bits is protected everywhere with a spinlock, 
> >> we don't need the test_and_set_bit, set_bit is enough and the loop
> >> can be removed too.
> >>
> >> Signed-off-by: Tomas Henzl 
> >> ---
> >>  drivers/scsi/hpsa.c | 15 ++-
> >>  1 file changed, 6 insertions(+), 9 deletions(-)
> >>
> >> diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c
> >> index 796482b..d7df01e 100644
> >> --- a/drivers/scsi/hpsa.c
> >> +++ b/drivers/scsi/hpsa.c
> >> @@ -2662,15 +2662,12 @@ static struct CommandList *cmd_alloc(struct 
> >> ctlr_info *h)
> >>unsigned long flags;
> >>  
> >>spin_lock_irqsave(&h->lock, flags);
> >> -  do {
> >> -  i = find_first_zero_bit(h->cmd_pool_bits, h->nr_cmds);
> >> -  if (i == h->nr_cmds) {
> >> -  spin_unlock_irqrestore(&h->lock, flags);
> >> -  return NULL;
> >> -  }
> >> -  } while (test_and_set_bit
> >> -   (i & (BITS_PER_LONG - 1),
> >> -h->cmd_pool_bits + (i / BITS_PER_LONG)) != 0);
> >> +  i = find_first_zero_bit(h->cmd_pool_bits, h->nr_cmds);
> >> +  if (i == h->nr_cmds) {
> >> +  spin_unlock_irqrestore(&h->lock, flags);
> >> +  return NULL;
> >> +  }
> >> +  set_bit(i & (BITS_PER_LONG - 1), h->cmd_pool_bits + (i / 
> >> BITS_PER_LONG));
> >>h->nr_allocs++;
> >>spin_unlock_irqrestore(&h->lock, flags);
> >>  
> >> -- 
> >> 1.8.3.1
> >>
> > Would it be better instead to just not use the spinlock for protecting
> > cmd_pool_bits?  I have thought about doing this for awhile, but haven't
> > gotten around to it.
> >
> > I think the while loop is safe without the spin lock.  And then it is
> > not needed in cmd_free either.
> 
> I was evaluating the same idea for a while too, a loop and inside just the 
> test_and_set_bit,
> maybe even a stored value to start with a likely empty bit from last time to 
> tune it a bit.
> But I know almost nothing about the use pattern, so I decided for the least 
> invasive change
> to the existing code, to not make it worse.

Only reason I haven't done it is I'm loathe to make such a change to the main 
i/o
path without testing it like crazy before unleashing it, and it's never been a 
convenient time to slide such a change in around here and get proper testing
done (and there are other rather large changes brewing).

However, we have been using a similar scheme with the SCSI over PCIe driver,
here: https://github.com/HPSmartStorage/scsi-over-pcie/blob/master/block/sop.c
in alloc_request() around line 1476 without problems, and nvme-core.c contains
similar code in alloc_cmdid(), so I am confident it's sound in principle.
I would want to beat on it though, in case it ends up exposing a firmware bug
or something (not that I think it will, but you never know.)

-- steve


 
> 
> 
> >
> > -- steve
> >
> > --
> > 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: Ejected Nook (usb mass storage) prevents suspend

[Oliver Neukum]

On Wed, 2013-07-31 at 11:13 -0400, Alan Stern wrote:

> More importantly, if we already know that the medium is not present or
> has been changed since it was last used, then there's no reason to call
> sd_sync_cache() at all.

Like this?

Regards
Oliver



>From 8c90d860652aa99e6e60d9b32bc3aa8d4db9efa5 Mon Sep 17 00:00:00 2001
From: Oliver Neukum 
Date: Thu, 1 Aug 2013 10:08:20 +0200
Subject: [PATCH] sd: error handling during flushing caches

It makes no sense to flush the cache of a device without medium.
Errors during suspend must be handled according to their causes.
Errors due to missing media or unplugged devices must be ignored.
Errors due to devices being offlined must also be ignored.
The error returns must be modified so that the generic layer
understands them.

Signed-off-by: Oliver Neukum 
---
 drivers/scsi/scsi_pm.c |  3 ++-
 drivers/scsi/sd.c  | 69 ++
 2 files changed, 60 insertions(+), 12 deletions(-)

diff --git a/drivers/scsi/scsi_pm.c b/drivers/scsi/scsi_pm.c
index 4c5aabe..af4c050 100644
--- a/drivers/scsi/scsi_pm.c
+++ b/drivers/scsi/scsi_pm.c
@@ -54,7 +54,8 @@ scsi_bus_suspend_common(struct device *dev, int (*cb)(struct device *))
 		/*
 		 * All the high-level SCSI drivers that implement runtime
 		 * PM treat runtime suspend, system suspend, and system
-		 * hibernate identically.
+		 * hibernate nearly identically. In all cases the requirements
+		 * for runtime suspension are stricter.
 		 */
 		if (pm_runtime_suspended(dev))
 			return 0;
diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index 86fcf2c..3c7f918 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -105,7 +105,8 @@ static void sd_unlock_native_capacity(struct gendisk *disk);
 static int  sd_probe(struct device *);
 static int  sd_remove(struct device *);
 static void sd_shutdown(struct device *);
-static int sd_suspend(struct device *);
+static int sd_suspend_system(struct device *);
+static int sd_suspend_runtime(struct device *);
 static int sd_resume(struct device *);
 static void sd_rescan(struct device *);
 static int sd_done(struct scsi_cmnd *);
@@ -483,11 +484,11 @@ static struct class sd_disk_class = {
 };
 
 static const struct dev_pm_ops sd_pm_ops = {
-	.suspend		= sd_suspend,
+	.suspend		= sd_suspend_system,
 	.resume			= sd_resume,
-	.poweroff		= sd_suspend,
+	.poweroff		= sd_suspend_system,
 	.restore		= sd_resume,
-	.runtime_suspend	= sd_suspend,
+	.runtime_suspend	= sd_suspend_runtime,
 	.runtime_resume		= sd_resume,
 };
 
@@ -1437,6 +1438,8 @@ static int sd_sync_cache(struct scsi_disk *sdkp)
 	if (!scsi_device_online(sdp))
 		return -ENODEV;
 
+	if (!sdkp->media_present)
+		return 0;
 
 	for (retries = 3; retries > 0; --retries) {
 		unsigned char cmd[10] = { 0 };
@@ -1455,12 +1458,31 @@ static int sd_sync_cache(struct scsi_disk *sdkp)
 
 	if (res) {
 		sd_print_result(sdkp, res);
-		if (driver_byte(res) & DRIVER_SENSE)
+
+		if (driver_byte(res) & DRIVER_SENSE) 
 			sd_print_sense_hdr(sdkp, &sshdr);
+		/* we need to evaluate the error return  */
+		if ((scsi_sense_valid(&sshdr) &&
+			/* 0x3a is medium not present */
+			sshdr.asc == 0x3a))
+/* this is no error here */
+return 0;
+
+		switch (host_byte(res)) {
+		/* ignore errors due to racing a disconnection */
+		case DID_BAD_TARGET:
+		case DID_NO_CONNECT:
+			return 0;
+		/* signal the upper layer it might try again */
+		case DID_BUS_BUSY:
+		case DID_IMM_RETRY:
+		case DID_REQUEUE:
+		case DID_SOFT_ERROR:
+			return -EBUSY;
+		default:
+			return -EIO;
+		}
 	}
-
-	if (res)
-		return -EIO;
 	return 0;
 }
 
@@ -3062,9 +3084,17 @@ static int sd_start_stop_device(struct scsi_disk *sdkp, int start)
 		sd_print_result(sdkp, res);
 		if (driver_byte(res) & DRIVER_SENSE)
 			sd_print_sense_hdr(sdkp, &sshdr);
+		if ((scsi_sense_valid(&sshdr) &&
+			/* 0x3a is medium not present */
+			sshdr.asc == 0x3a))
+			res = 0;
 	}
 
-	return res;
+	/* SCSI error codes must not go to the generic layer */
+	if (res)
+		return -EIO;
+
+	return 0;
 }
 
 /*
@@ -3096,7 +3126,7 @@ exit:
 	scsi_disk_put(sdkp);
 }
 
-static int sd_suspend(struct device *dev)
+static int sd_suspend_common(struct device *dev, bool ignore_stop_errors)
 {
 	struct scsi_disk *sdkp = scsi_disk_get_from_dev(dev);
 	int ret = 0;
@@ -3107,13 +3137,20 @@ static int sd_suspend(struct device *dev)
 	if (sdkp->WCE) {
 		sd_printk(KERN_NOTICE, sdkp, "Synchronizing SCSI cache\n");
 		ret = sd_sync_cache(sdkp);
-		if (ret)
+		if (ret) {
+			/* ignore OFFLINE device */
+			if (ret == -ENODEV)
+ret = 0;
 			goto done;
+		}
 	}
 
 	if (sdkp->device->manage_start_stop) {
 		sd_printk(KERN_NOTICE, sdkp, "Stopping disk\n");
+		/* an error is not worth aborting a system sleep */
 		ret = sd_start_stop_device(sdkp, 0);
+		if (ignore_stop_errors)
+			ret = 0;
 	}
 
 done:
@@ -3121,6 +3158,16 @@ done:
 	return ret;
 }
 
+static int sd_suspend_system(struct device *dev)
+{
+	return sd_suspend_common(dev, true);
+}
+
+stati

Re: [PATCH 1/3] hpsa: remove unneeded loop

[Tomas Henzl]

On 08/01/2013 03:39 PM, scame...@beardog.cce.hp.com wrote:
> On Thu, Aug 01, 2013 at 03:11:22PM +0200, Tomas Henzl wrote:
>> From: Tomas Henzl 
>>
>> The cmd_pool_bits is protected everywhere with a spinlock, 
>> we don't need the test_and_set_bit, set_bit is enough and the loop
>> can be removed too.
>>
>> Signed-off-by: Tomas Henzl 
>> ---
>>  drivers/scsi/hpsa.c | 15 ++-
>>  1 file changed, 6 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c
>> index 796482b..d7df01e 100644
>> --- a/drivers/scsi/hpsa.c
>> +++ b/drivers/scsi/hpsa.c
>> @@ -2662,15 +2662,12 @@ static struct CommandList *cmd_alloc(struct 
>> ctlr_info *h)
>>  unsigned long flags;
>>  
>>  spin_lock_irqsave(&h->lock, flags);
>> -do {
>> -i = find_first_zero_bit(h->cmd_pool_bits, h->nr_cmds);
>> -if (i == h->nr_cmds) {
>> -spin_unlock_irqrestore(&h->lock, flags);
>> -return NULL;
>> -}
>> -} while (test_and_set_bit
>> - (i & (BITS_PER_LONG - 1),
>> -  h->cmd_pool_bits + (i / BITS_PER_LONG)) != 0);
>> +i = find_first_zero_bit(h->cmd_pool_bits, h->nr_cmds);
>> +if (i == h->nr_cmds) {
>> +spin_unlock_irqrestore(&h->lock, flags);
>> +return NULL;
>> +}
>> +set_bit(i & (BITS_PER_LONG - 1), h->cmd_pool_bits + (i / 
>> BITS_PER_LONG));
>>  h->nr_allocs++;
>>  spin_unlock_irqrestore(&h->lock, flags);
>>  
>> -- 
>> 1.8.3.1
>>
> Would it be better instead to just not use the spinlock for protecting
> cmd_pool_bits?  I have thought about doing this for awhile, but haven't
> gotten around to it.
>
> I think the while loop is safe without the spin lock.  And then it is
> not needed in cmd_free either.

I was evaluating the same idea for a while too, a loop and inside just the 
test_and_set_bit,
maybe even a stored value to start with a likely empty bit from last time to 
tune it a bit.
But I know almost nothing about the use pattern, so I decided for the least 
invasive change
to the existing code, to not make it worse.


>
> -- steve
>
> --
> 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: [PATCH 2/3] hpsa: fix a race in cmd_free/scsi_done

[scameron]

On Thu, Aug 01, 2013 at 03:14:00PM +0200, Tomas Henzl wrote:
> From: Tomas Henzl 
> 
> When the driver calls scsi_done and after that frees it's internal
> preallocated memory it can happen that a new job is enqueud before
> the memory is freed. The allocation fails and the message
> "cmd_alloc returned NULL" is shown.
> Patch below fixes it by moving cmd->scsi_done after cmd_free.
> 
> Signed-off-by: Tomas Henzl 
> ---
>  drivers/scsi/hpsa.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c
> index d7df01e..48fa81e 100644
> --- a/drivers/scsi/hpsa.c
> +++ b/drivers/scsi/hpsa.c
> @@ -1180,8 +1180,8 @@ static void complete_scsi_command(struct CommandList 
> *cp)
>   scsi_set_resid(cmd, ei->ResidualCnt);
>  
>   if (ei->CommandStatus == 0) {
> - cmd->scsi_done(cmd);
>   cmd_free(h, cp);
> + cmd->scsi_done(cmd);
>   return;
>   }
>  
> @@ -1353,8 +1353,8 @@ static void complete_scsi_command(struct CommandList 
> *cp)
>   dev_warn(&h->pdev->dev, "cp %p returned unknown status %x\n",
>   cp, ei->CommandStatus);
>   }
> - cmd->scsi_done(cmd);
>   cmd_free(h, cp);
> + cmd->scsi_done(cmd);
>  }
>  
>  static void hpsa_pci_unmap(struct pci_dev *pdev,
> -- 
> 1.8.3.1

Oh, nice catch!!!  Those mysterious and seemingly never reproducible
legends of the "cmd_alloc returned NULL" message that would show up
*extremely* infrequently are finally explained!

Ack.

-- steve
--
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/3] hpsa: remove unneeded variable

[scameron]

On Thu, Aug 01, 2013 at 03:14:52PM +0200, Tomas Henzl wrote:
> From: Tomas Henzl 
> 
> Remove unneeded variables.
> 
> Signed-off-by: Tomas Henzl 
> 
> ---
>  drivers/scsi/hpsa.c | 2 --
>  drivers/scsi/hpsa.h | 2 --
>  2 files changed, 4 deletions(-)
> 
> diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c
> index 48fa81e..e0f6b00 100644
> --- a/drivers/scsi/hpsa.c
> +++ b/drivers/scsi/hpsa.c
> @@ -2668,7 +2668,6 @@ static struct CommandList *cmd_alloc(struct ctlr_info 
> *h)
>   return NULL;
>   }
>   set_bit(i & (BITS_PER_LONG - 1), h->cmd_pool_bits + (i / 
> BITS_PER_LONG));
> - h->nr_allocs++;
>   spin_unlock_irqrestore(&h->lock, flags);
>  
>   c = h->cmd_pool + i;
> @@ -2740,7 +2739,6 @@ static void cmd_free(struct ctlr_info *h, struct 
> CommandList *c)
>   spin_lock_irqsave(&h->lock, flags);
>   clear_bit(i & (BITS_PER_LONG - 1),
> h->cmd_pool_bits + (i / BITS_PER_LONG));
> - h->nr_frees++;
>   spin_unlock_irqrestore(&h->lock, flags);
>  }
>  
> diff --git a/drivers/scsi/hpsa.h b/drivers/scsi/hpsa.h
> index 9816479..bc85e72 100644
> --- a/drivers/scsi/hpsa.h
> +++ b/drivers/scsi/hpsa.h
> @@ -98,8 +98,6 @@ struct ctlr_info {
>   struct ErrorInfo*errinfo_pool;
>   dma_addr_t  errinfo_pool_dhandle;
>   unsigned long   *cmd_pool_bits;
> - int nr_allocs;
> - int nr_frees;
>   int scan_finished;
>   spinlock_t  scan_lock;
>   wait_queue_head_t   scan_wait_queue;
> -- 
> 1.8.3.1

Ack.

-- steve

--
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/3] hpsa: remove unneeded loop

[scameron]

On Thu, Aug 01, 2013 at 03:11:22PM +0200, Tomas Henzl wrote:
> From: Tomas Henzl 
> 
> The cmd_pool_bits is protected everywhere with a spinlock, 
> we don't need the test_and_set_bit, set_bit is enough and the loop
> can be removed too.
> 
> Signed-off-by: Tomas Henzl 
> ---
>  drivers/scsi/hpsa.c | 15 ++-
>  1 file changed, 6 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c
> index 796482b..d7df01e 100644
> --- a/drivers/scsi/hpsa.c
> +++ b/drivers/scsi/hpsa.c
> @@ -2662,15 +2662,12 @@ static struct CommandList *cmd_alloc(struct ctlr_info 
> *h)
>   unsigned long flags;
>  
>   spin_lock_irqsave(&h->lock, flags);
> - do {
> - i = find_first_zero_bit(h->cmd_pool_bits, h->nr_cmds);
> - if (i == h->nr_cmds) {
> - spin_unlock_irqrestore(&h->lock, flags);
> - return NULL;
> - }
> - } while (test_and_set_bit
> -  (i & (BITS_PER_LONG - 1),
> -   h->cmd_pool_bits + (i / BITS_PER_LONG)) != 0);
> + i = find_first_zero_bit(h->cmd_pool_bits, h->nr_cmds);
> + if (i == h->nr_cmds) {
> + spin_unlock_irqrestore(&h->lock, flags);
> + return NULL;
> + }
> + set_bit(i & (BITS_PER_LONG - 1), h->cmd_pool_bits + (i / 
> BITS_PER_LONG));
>   h->nr_allocs++;
>   spin_unlock_irqrestore(&h->lock, flags);
>  
> -- 
> 1.8.3.1
> 

Would it be better instead to just not use the spinlock for protecting
cmd_pool_bits?  I have thought about doing this for awhile, but haven't
gotten around to it.

I think the while loop is safe without the spin lock.  And then it is
not needed in cmd_free either.

-- steve

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

[Bug 60644] MPT2SAS drops all HDDs when under high I/O

[bugzilla-daemon]

https://bugzilla.kernel.org/show_bug.cgi?id=60644

--- Comment #14 from livea...@live.com ---
Hi .

Any updates regarding this bug ?

-- 
You are receiving this mail because:
You are watching the assignee of the bug.
--
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 3/3] hpsa: remove unneeded variable

[Tomas Henzl]

From: Tomas Henzl 

Remove unneeded variables.

Signed-off-by: Tomas Henzl 

---
 drivers/scsi/hpsa.c | 2 --
 drivers/scsi/hpsa.h | 2 --
 2 files changed, 4 deletions(-)

diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c
index 48fa81e..e0f6b00 100644
--- a/drivers/scsi/hpsa.c
+++ b/drivers/scsi/hpsa.c
@@ -2668,7 +2668,6 @@ static struct CommandList *cmd_alloc(struct ctlr_info *h)
return NULL;
}
set_bit(i & (BITS_PER_LONG - 1), h->cmd_pool_bits + (i / 
BITS_PER_LONG));
-   h->nr_allocs++;
spin_unlock_irqrestore(&h->lock, flags);
 
c = h->cmd_pool + i;
@@ -2740,7 +2739,6 @@ static void cmd_free(struct ctlr_info *h, struct 
CommandList *c)
spin_lock_irqsave(&h->lock, flags);
clear_bit(i & (BITS_PER_LONG - 1),
  h->cmd_pool_bits + (i / BITS_PER_LONG));
-   h->nr_frees++;
spin_unlock_irqrestore(&h->lock, flags);
 }
 
diff --git a/drivers/scsi/hpsa.h b/drivers/scsi/hpsa.h
index 9816479..bc85e72 100644
--- a/drivers/scsi/hpsa.h
+++ b/drivers/scsi/hpsa.h
@@ -98,8 +98,6 @@ struct ctlr_info {
struct ErrorInfo*errinfo_pool;
dma_addr_t  errinfo_pool_dhandle;
unsigned long   *cmd_pool_bits;
-   int nr_allocs;
-   int nr_frees;
int scan_finished;
spinlock_t  scan_lock;
wait_queue_head_t   scan_wait_queue;
-- 
1.8.3.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 2/3] hpsa: fix a race in cmd_free/scsi_done

[Tomas Henzl]

From: Tomas Henzl 

When the driver calls scsi_done and after that frees it's internal
preallocated memory it can happen that a new job is enqueud before
the memory is freed. The allocation fails and the message
"cmd_alloc returned NULL" is shown.
Patch below fixes it by moving cmd->scsi_done after cmd_free.

Signed-off-by: Tomas Henzl 
---
 drivers/scsi/hpsa.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c
index d7df01e..48fa81e 100644
--- a/drivers/scsi/hpsa.c
+++ b/drivers/scsi/hpsa.c
@@ -1180,8 +1180,8 @@ static void complete_scsi_command(struct CommandList *cp)
scsi_set_resid(cmd, ei->ResidualCnt);
 
if (ei->CommandStatus == 0) {
-   cmd->scsi_done(cmd);
cmd_free(h, cp);
+   cmd->scsi_done(cmd);
return;
}
 
@@ -1353,8 +1353,8 @@ static void complete_scsi_command(struct CommandList *cp)
dev_warn(&h->pdev->dev, "cp %p returned unknown status %x\n",
cp, ei->CommandStatus);
}
-   cmd->scsi_done(cmd);
cmd_free(h, cp);
+   cmd->scsi_done(cmd);
 }
 
 static void hpsa_pci_unmap(struct pci_dev *pdev,
-- 
1.8.3.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/3] hpsa: remove unneeded loop

[Tomas Henzl]

From: Tomas Henzl 

The cmd_pool_bits is protected everywhere with a spinlock, 
we don't need the test_and_set_bit, set_bit is enough and the loop
can be removed too.

Signed-off-by: Tomas Henzl 
---
 drivers/scsi/hpsa.c | 15 ++-
 1 file changed, 6 insertions(+), 9 deletions(-)

diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c
index 796482b..d7df01e 100644
--- a/drivers/scsi/hpsa.c
+++ b/drivers/scsi/hpsa.c
@@ -2662,15 +2662,12 @@ static struct CommandList *cmd_alloc(struct ctlr_info 
*h)
unsigned long flags;
 
spin_lock_irqsave(&h->lock, flags);
-   do {
-   i = find_first_zero_bit(h->cmd_pool_bits, h->nr_cmds);
-   if (i == h->nr_cmds) {
-   spin_unlock_irqrestore(&h->lock, flags);
-   return NULL;
-   }
-   } while (test_and_set_bit
-(i & (BITS_PER_LONG - 1),
- h->cmd_pool_bits + (i / BITS_PER_LONG)) != 0);
+   i = find_first_zero_bit(h->cmd_pool_bits, h->nr_cmds);
+   if (i == h->nr_cmds) {
+   spin_unlock_irqrestore(&h->lock, flags);
+   return NULL;
+   }
+   set_bit(i & (BITS_PER_LONG - 1), h->cmd_pool_bits + (i / 
BITS_PER_LONG));
h->nr_allocs++;
spin_unlock_irqrestore(&h->lock, flags);
 
-- 
1.8.3.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/2] mpt2sas: Remove phys on topology change.

[Jan Vesely]

From: Jan Vesely 

CC: Nagalakshmi Nandigama 
CC: Sreekanth Reddy 
CC: Tomas Henzl 
Signed-off-by: Jan Vesely 
---
 drivers/scsi/mpt2sas/mpt2sas_transport.c |5 -
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/drivers/scsi/mpt2sas/mpt2sas_transport.c 
b/drivers/scsi/mpt2sas/mpt2sas_transport.c
index 193e7ae..e610a97 100644
--- a/drivers/scsi/mpt2sas/mpt2sas_transport.c
+++ b/drivers/scsi/mpt2sas/mpt2sas_transport.c
@@ -1006,9 +1006,12 @@ mpt2sas_transport_update_links(struct MPT2SAS_ADAPTER 
*ioc,
&mpt2sas_phy->remote_identify);
_transport_add_phy_to_an_existing_port(ioc, sas_node,
mpt2sas_phy, mpt2sas_phy->remote_identify.sas_address);
-   } else
+   } else {
memset(&mpt2sas_phy->remote_identify, 0 , sizeof(struct
sas_identify));
+   _transport_del_phy_from_an_existing_port(ioc, sas_node,
+   mpt2sas_phy);
+   }
 
if (mpt2sas_phy->phy)
mpt2sas_phy->phy->negotiated_linkrate =
-- 
1.7.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 2/2] mpt3sas: Remove phys on topology change

[Jan Vesely]

From: Jan Vesely 

CC: Nagalakshmi Nandigama 
CC: Sreekanth Reddy 
CC: Tomas Henzl 
Signed-off-by: Jan Vesely 
---
 drivers/scsi/mpt3sas/mpt3sas_transport.c |5 -
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/drivers/scsi/mpt3sas/mpt3sas_transport.c 
b/drivers/scsi/mpt3sas/mpt3sas_transport.c
index 87ca2b7..45bc98e 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_transport.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_transport.c
@@ -1003,9 +1003,12 @@ mpt3sas_transport_update_links(struct MPT3SAS_ADAPTER 
*ioc,
&mpt3sas_phy->remote_identify);
_transport_add_phy_to_an_existing_port(ioc, sas_node,
mpt3sas_phy, mpt3sas_phy->remote_identify.sas_address);
-   } else
+   } else {
memset(&mpt3sas_phy->remote_identify, 0 , sizeof(struct
sas_identify));
+   _transport_del_phy_from_an_existing_port(ioc, sas_node,
+   mpt3sas_phy);
+   }
 
if (mpt3sas_phy->phy)
mpt3sas_phy->phy->negotiated_linkrate =
-- 
1.7.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 0/2] mpt{2,3}sas remove disconnected phys on topology change

[Jan Vesely]

From: Jan Vesely 

These two patches add phy removal on link loss. This change keeps sysfs
up-to-date with actually connected phys. Without these patches,
 disconnected phys remain listed under their former ports.

tested on both mpt2sas and mpt3sas hw.

CC: Nagalakshmi Nandigama 
CC: Sreekanth Reddy 
CC: Tomas Henzl 
Signed-off-by: Jan Vesely 

Jan Vesely (2):
  mpt2sas: Remove phys on topology change.
  mpt3sas: Remove phys on topology change

 drivers/scsi/mpt2sas/mpt2sas_transport.c |5 -
 drivers/scsi/mpt3sas/mpt3sas_transport.c |5 -
 2 files changed, 8 insertions(+), 2 deletions(-)

--
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 RESEND v2] block: modify __bio_add_page check to accept pages that don't start a new segment

[Jan Vesely]

On Mon 25 Mar 2013 20:40:09 CET, Jens Axboe wrote:
> On Mon, Mar 25 2013, Jan Vesely wrote:
>>  51506edc5741209311913
>>
>> On Mon 25 Mar 2013 15:24:57 CET, Jens Axboe wrote:
>>> On Mon, Mar 25 2013, Jan Vesely wrote:
 v2: changed a comment

 The original behavior was to refuse all pages after the maximum number of
 segments has been reached. However, some drivers (like st) craft their 
 buffers
 to potentially require exactly max segments and multiple pages in the last
 segment. This patch modifies the check to allow pages that can be merged 
 into
 the last segment.

 Fixes EBUSY failures when using large tape block size in high
 memory fragmentation condition.
 This regression was introduced by commit
  46081b166415acb66d4b3150ecefcd9460bb48a1
  st: Increase success probability in driver buffer allocation

 Signed-off-by: Jan Vesely 

 CC: Alexander Viro 
 CC: FUJITA Tomonori 
 CC: Kai Makisara 
 CC: James Bottomley 
 CC: Jens Axboe 
 CC: sta...@vger.kernel.org
 ---
  fs/bio.c | 27 +--
  1 file changed, 17 insertions(+), 10 deletions(-)

 diff --git a/fs/bio.c b/fs/bio.c
 index bb5768f..bc6af71 100644
 --- a/fs/bio.c
 +++ b/fs/bio.c
 @@ -500,7 +500,6 @@ static int __bio_add_page(struct request_queue *q, 
 struct bio *bio, struct page
  *page, unsigned int len, unsigned int offset,
  unsigned short max_sectors)
  {
 -  int retried_segments = 0;
struct bio_vec *bvec;

/*
 @@ -551,18 +550,13 @@ static int __bio_add_page(struct request_queue *q, 
 struct bio *bio, struct page
return 0;

/*
 -   * we might lose a segment or two here, but rather that than
 -   * make this too complex.
 +   * The first part of the segment count check,
 +   * reduce segment count if possible
 */

 -  while (bio->bi_phys_segments >= queue_max_segments(q)) {
 -
 -  if (retried_segments)
 -  return 0;
 -
 -  retried_segments = 1;
 +  if (bio->bi_phys_segments >= queue_max_segments(q))
blk_recount_segments(q, bio);
 -  }
 +

/*
 * setup the new entry, we might clear it again later if we
 @@ -572,6 +566,19 @@ static int __bio_add_page(struct request_queue *q, 
 struct bio *bio, struct page
bvec->bv_page = page;
bvec->bv_len = len;
bvec->bv_offset = offset;
 +  
 +  /*
 +   * the other part of the segment count check, allow mergeable pages
 +   */
 +  if ((bio->bi_phys_segments > queue_max_segments(q)) ||
 +  ( (bio->bi_phys_segments == queue_max_segments(q)) &&
 +  !BIOVEC_PHYS_MERGEABLE(bvec - 1, bvec))) {
 +  bvec->bv_page = NULL;
 +  bvec->bv_len = 0;
 +  bvec->bv_offset = 0;
 +  return 0;
 +  }
 +
>>>
>>> This is a bit messy, I think. bi_phys_segments should never be allowed
>>> to go beyond queue_ma_segments(), so the > test does not look right.
>>> Maybe it's an artifact of when we fall through with this patch, we bump
>>> bi_phys_segments even if the segments are physicall contig and
>>> mergeable.
>>
>> yeah. it is messy, I tried to go for the least invasive changes.
>>
>> I took the '>' test from the original while loop '>='. The original
>> behavior guaranteed bio->bi_phys_segments <= max_segments, if the bio
>> satisfied this condition to begin with.  I did not find any guarantees
>> that the 'bio' parameter of this function has to satisfy this
>> condition in general.
>>
>> My understanding is that if a caller of this function (or one of the
>> two that call this one) provides an invalid (segment-count-wise) bio,
>> it will fail (return 0 added length), and let the caller handle the
>> situation.  I admit, I did not check all the call paths that use these
>> functions.
>
> Yes, that is how it works. So that should be fine.
>
>>> What happens when the segment is physically mergeable, but the resulting
>>> merged segment is too large (bigger than q->limits.max_segment_size)?
>>>
>>
>> ah, yes. I guess I need a check that follows __blk_recalc_rq_segments
>> more closely.  We know that at this point all pages are merged into
>> segments, so a helper function that would be used by both
>> __blk_recalc_rq_segments and this check is possible.
>>
>>
>> I still assume that a temporary increase of bi_phys_segments above
>> max_segments is ok.  If we want to avoid this situation we would need
>> to merge tail pages right away. That's imo uglier.
>
> Yes, it's OK if we just ensure that we clear the valid segment flag. At
> least that would be the best sort of solution, to ensure that it's
> recalculated properly when someone checks it.
>

Hi Jens,

v3 has been around for few 

Re: [PATCH] virtio-scsi: Fix virtqueue affinity setup

[Paolo Bonzini]

> vscsi->num_queues counts the number of request virtqueue which does not
> include the control and event virtqueue. It is wrong to subtract
> VIRTIO_SCSI_VQ_BASE from vscsi->num_queues.

Reviewed-by: Paolo Bonzini 

> This patch fixes the following panic.
> 
> (qemu) device_del scsi0
> 
>  BUG: unable to handle kernel NULL pointer dereference at 0020
>  IP: [] __virtscsi_set_affinity+0x6f/0x120
>  PGD 0
>  Oops:  [#1] SMP
>  Modules linked in:
>  CPU: 0 PID: 659 Comm: kworker/0:1 Not tainted 3.11.0-rc2+ #1172
>  Hardware name: Bochs Bochs, BIOS Bochs 01/01/2011
>  Workqueue: kacpi_hotplug _handle_hotplug_event_func
>  task: 88007bee1cc0 ti: 88007bfe4000 task.ti: 88007bfe4000
>  RIP: 0010:[]  []
>  __virtscsi_set_affinity+0x6f/0x120
>  RSP: 0018:88007bfe5a38  EFLAGS: 00010202
>  RAX: 0010 RBX: 880077fd0d28 RCX: 0050
>  RDX:  RSI: 0246 RDI: 
>  RBP: 88007bfe5a58 R08: 880077f6ff00 R09: 0001
>  R10: 8143e673 R11: 0001 R12: 0001
>  R13: 880077fd0800 R14:  R15: 88007bf489b0
>  FS:  () GS:88007ea0() knlGS:
>  CS:  0010 DS:  ES:  CR0: 8005003b
>  CR2: 0020 CR3: 79f8b000 CR4: 06f0
>  Stack:
>   880077fd0d28  880077fd0800 0008
>   88007bfe5a78 8179b37d 88007bccc800 88007bccc800
>   88007bfe5a98 8179b3b6 88007bccc800 880077fd0d28
>  Call Trace:
>   [] virtscsi_set_affinity+0x2d/0x40
>   [] virtscsi_remove_vqs+0x26/0x50
>   [] virtscsi_remove+0x82/0xa0
>   [] virtio_dev_remove+0x22/0x70
>   [] __device_release_driver+0x69/0xd0
>   [] device_release_driver+0x2d/0x40
>   [] bus_remove_device+0x116/0x150
>   [] device_del+0x126/0x1e0
>   [] device_unregister+0x16/0x30
>   [] unregister_virtio_device+0x19/0x30
>   [] virtio_pci_remove+0x36/0x80
>   [] pci_device_remove+0x37/0x70
>   [] __device_release_driver+0x69/0xd0
>   [] device_release_driver+0x2d/0x40
>   [] bus_remove_device+0x116/0x150
>   [] device_del+0x126/0x1e0
>   [] pci_stop_bus_device+0x9c/0xb0
>   [] pci_stop_and_remove_bus_device+0x16/0x30
>   [] acpiphp_disable_slot+0x8e/0x150
>   [] hotplug_event_func+0xba/0x1a0
>   [] ? acpi_os_release_object+0xe/0x12
>   [] _handle_hotplug_event_func+0x31/0x70
>   [] process_one_work+0x183/0x500
>   [] worker_thread+0x122/0x400
>   [] ? manage_workers+0x2d0/0x2d0
>   [] kthread+0xce/0xe0
>   [] ? kthread_freezable_should_stop+0x70/0x70
>   [] ret_from_fork+0x7c/0xb0
>   [] ? kthread_freezable_should_stop+0x70/0x70
>  Code: 01 00 00 00 74 59 45 31 e4 83 bb c8 01 00 00 02 74 46 66 2e 0f 1f 84
>  00 00 00 00 00 49 63 c4 48 c1 e0 04 48 8b bc 0
> 3 10 02 00 00 <48> 8b 47 20 48 8b 80 d0 01 00 00 48 8b 40 50 48 85 c0 74 07
> be
>  RIP  [] __virtscsi_set_affinity+0x6f/0x120
>   RSP 
>  CR2: 0020
>  ---[ end trace 99679331a3775f48 ]---
> 
> CC: sta...@vger.kernel.org
> Signed-off-by: Asias He 
> ---
>  drivers/scsi/virtio_scsi.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/scsi/virtio_scsi.c b/drivers/scsi/virtio_scsi.c
> index 2168258..74b88ef 100644
> --- a/drivers/scsi/virtio_scsi.c
> +++ b/drivers/scsi/virtio_scsi.c
> @@ -751,7 +751,7 @@ static void __virtscsi_set_affinity(struct virtio_scsi
> *vscsi, bool affinity)
>  
>   vscsi->affinity_hint_set = true;
>   } else {
> - for (i = 0; i < vscsi->num_queues - VIRTIO_SCSI_VQ_BASE; i++)
> + for (i = 0; i < vscsi->num_queues; i++)
>   virtqueue_set_affinity(vscsi->req_vqs[i].vq, -1);
>  
>   vscsi->affinity_hint_set = false;
> --
> 1.8.3.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