Re: [LSF/MM TOPIC] multiqueue and interrupt assignment

2016-03-02 Thread Ming Lei
On Wed, Feb 3, 2016 at 11:03 PM, Hannes Reinecke  wrote:
> On 02/03/2016 02:32 PM, Sagi Grimberg wrote:
>>
>>> Indeed, something like this.
>>> Quite some issues would be solved if we could push a hctx mapping
>>> into blk-mq, instead of having it assign its own made-up one.
>>
>> For that you can provide your own .map_queue in blk_mq_ops I think
>> (no one does that at the moment). This requires every driver to
>> implement it's own routine (probably with a similar logic) though...
>
> And at the same time direct interrupt assigment from the driver is
> frowned upon ... feels a bit stupid, having to setup a cpu-to-queue
> assigment (which typically is identical to the cpu-to-msix
> assignment), then pass this information to blk-mq, which then passed
> it to user-space, which then uses the information to setup a
> cpu-to-msix assignment.
> There is room for improvement there ...
>
> Are there any plans addressing this in blk-mq?

Last year, I sent a patchset to address the issue[1], but
it wasn't good enough for merge, and I am happy to discuss
the issue further.

[1] http://marc.info/?t=14434969112&r=1&w=2

> What does NVMe and virtio do?

virtio just takes the default irq affinity setting,  which means
the irq for one vq is only handled by the 1st CPU when it is
set to route to a group of CPU.

For NVMe, I remembered that the irq affinity setting is still
fixed after setting up the queue, and it should be better to
adjust it after hw/sw queue mapping is changed.

Thanks,
Ming

>
> Cheers,
>
> Hannes
> --
> Dr. Hannes ReineckeTeamlead Storage & Networking
> h...@suse.de   +49 911 74053 688
> SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
> GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
> HRB 21284 (AG Nürnberg)
> --
> To unsubscribe from this list: send the line "unsubscribe linux-block" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html



-- 
Ming Lei
--
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 v5 04/15] scsi: ufs: verify hba controller hce reg value

2016-03-02 Thread Hannes Reinecke
On 03/01/2016 09:32 PM, yga...@codeaurora.org wrote:
>> On 02/28/2016 09:32 PM, Yaniv Gardi wrote:
>>> Sometimes due to hw issues it takes some time to the
>>> host controller register to update. In order to verify the register
>>> has updated, a polling is done until its value is set.
>>>
>>> In addition the functions ufshcd_hba_stop() and
>>> ufshcd_wait_for_register() was updated with an additional input
>>> parameter, indicating the timeout between reads will
>>> be done by sleeping or spinning the cpu.
>>>
>>> Signed-off-by: Raviv Shvili 
>>> Signed-off-by: Yaniv Gardi 
>>>
>>> ---
>>>  drivers/scsi/ufs/ufshcd.c | 53
>>> ---
>>>  drivers/scsi/ufs/ufshcd.h | 12 +++
>>>  2 files changed, 35 insertions(+), 30 deletions(-)
>>>
>>> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
>>> index 3400ceb..80031e6 100644
>>> --- a/drivers/scsi/ufs/ufshcd.c
>>> +++ b/drivers/scsi/ufs/ufshcd.c
>>> @@ -240,11 +240,13 @@ static inline void ufshcd_disable_irq(struct
>>> ufs_hba *hba)
>>>   * @val - wait condition
>>>   * @interval_us - polling interval in microsecs
>>>   * @timeout_ms - timeout in millisecs
>>> + * @can_sleep - perform sleep or just spin
>>>   *
>>>   * Returns -ETIMEDOUT on error, zero on success
>>>   */
>>> -static int ufshcd_wait_for_register(struct ufs_hba *hba, u32 reg, u32
>>> mask,
>>> -   u32 val, unsigned long interval_us, unsigned long timeout_ms)
>>> +int ufshcd_wait_for_register(struct ufs_hba *hba, u32 reg, u32 mask,
>>> +   u32 val, unsigned long interval_us,
>>> +   unsigned long timeout_ms, bool can_sleep)
>>>  {
>>> int err = 0;
>>> unsigned long timeout = jiffies + msecs_to_jiffies(timeout_ms);
>>> @@ -253,9 +255,10 @@ static int ufshcd_wait_for_register(struct ufs_hba
>>> *hba, u32 reg, u32 mask,
>>> val = val & mask;
>>>
>>> while ((ufshcd_readl(hba, reg) & mask) != val) {
>>> -   /* wakeup within 50us of expiry */
>>> -   usleep_range(interval_us, interval_us + 50);
>>> -
>>> +   if (can_sleep)
>>> +   usleep_range(interval_us, interval_us + 50);
>>> +   else
>>> +   udelay(interval_us);
>>> if (time_after(jiffies, timeout)) {
>>> if ((ufshcd_readl(hba, reg) & mask) != val)
>>> err = -ETIMEDOUT;
>>> @@ -1459,7 +1462,7 @@ ufshcd_clear_cmd(struct ufs_hba *hba, int tag)
>>>  */
>>> err = ufshcd_wait_for_register(hba,
>>> REG_UTP_TRANSFER_REQ_DOOR_BELL,
>>> -   mask, ~mask, 1000, 1000);
>>> +   mask, ~mask, 1000, 1000, true);
>>>
>>> return err;
>>>  }
>>> @@ -2815,6 +2818,23 @@ out:
>>>  }
>>>
>>>  /**
>>> + * ufshcd_hba_stop - Send controller to reset state
>>> + * @hba: per adapter instance
>>> + * @can_sleep: perform sleep or just spin
>>> + */
>>> +static inline void ufshcd_hba_stop(struct ufs_hba *hba, bool can_sleep)
>>> +{
>>> +   int err;
>>> +
>>> +   ufshcd_writel(hba, CONTROLLER_DISABLE,  REG_CONTROLLER_ENABLE);
>>> +   err = ufshcd_wait_for_register(hba, REG_CONTROLLER_ENABLE,
>>> +   CONTROLLER_ENABLE, CONTROLLER_DISABLE,
>>> +   10, 1, can_sleep);
>>> +   if (err)
>>> +   dev_err(hba->dev, "%s: Controller disable failed\n", __func__);
>>> +}
>>> +
>> Shouldn't you return an error here?
>> If the controller disable failed you probably need a hard reset or
>> something, otherwise I would assume that every other command from that
>> point on will not work as expected.
>>
>> Cheers,
>>
>> Hannes
> 
> 
> Hello Hannes,
> The original routine signature is:
> void ufshcd_hba_stop(struct ufs_hba *hba);
> 
> as you can see, no return value, the reason is simple - there is nothing
> we can do if writing to the register fails.
> 
> all we wanted to do here, was to add a graceful time to change the
> register value. also, we decided to add error msg in case the value is not
> change within this timeout.
> We can not do anything else, not to say, return error, as there is no
> error handling in such case.
> 
> So, as far as i see it, we only improved the already exists logic, by
> adding some graceful time to the register change, and also, by adding an
> error message that was absent before.
> 
Thanks for the explanation.

Reviewed-by: Hannes Reinecke 

Cheers,

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


Re: [PATCH v5 03/15] scsi: ufs: implement scsi host timeout handler

2016-03-02 Thread Hannes Reinecke
On 03/01/2016 09:25 PM, yga...@codeaurora.org wrote:
>> On 02/28/2016 09:32 PM, Yaniv Gardi wrote:
>>> A race condition exists between request requeueing and scsi layer
>>> error handling:
>>> When UFS driver queuecommand returns a busy status for a request,
>>> it will be requeued and its tag will be freed and set to -1.
>>> At the same time it is possible that the request will timeout and
>>> scsi layer will start error handling for it. The scsi layer reuses
>>> the request and its tag to send error related commands to the device,
>>> however its tag is no longer valid.
>> Hmm. How can the host return a 'busy' status for a request?
>> From my understanding we have three possibilities:
>>
>> 1) queuecommand returns busy; however, that means that the command has
>> never been send and this issue shouldn't occur
>> 2) The command returns with BUSY status. But in this case it has already
>> been returned, so there cannot be any timeout coming in.
>> 3) The host receives a command with a tag which is already in-use.
>> However, that should have been prevented by the block-layer, which
>> really should ensure that this situation never happens.
>>
>> So either way I look at it, it really looks like a bug and adding a
>> timeout handler will just paper over it.
>> (Not that a timeout handler is a bad idea, in fact I'm convinced that
>> you need one. Just not for this purpose.)
>>
>> So can you elaborate how this 'busy' status comes about?
>> Is the command sent to the device?
>>
>> Cheers,
>>
>> Hannes
> 
> 
> Hi Hannes,
> 
> it's going to be a bit long :)
> I think you are missing the point.
> I will describe a race condition happened to us a while ago, that was
> quite difficult to understand and fix.
> So, this patch is not about the "busy" returning to the scsi dispatch
> routine. it's about the abort triggered after 30 seconds.
> 
> imagine a request being queued and sent to the scsi, and then to the ufs.
> a timer, initialized to 30 seconds start ticking.
> but the request is never sent to the ufs device, as queuecommand() returns
> with "SCSI_MLQUEUE_HOST_BUSY"
> by looking at the code, this could happen, for example:
>   err = ufshcd_hold(hba, true);
>   if (err) {
>   err = SCSI_MLQUEUE_HOST_BUSY;
>   goto out;
>   }
> 
Uuhhh.
You probably should not have pointed me to that piece of code ...
open-coding loops in ufshcd_hold() ... shudder.
(Did I ever review that one? Must've ...)
_Anyway_: sleeping in queuecommand is always a bad idea, as then
precisely those issues you've just described will happen.

Couldn't you just call
ufshcd_hold(hba, false)
instead of
ufshcd_hold(hba, true)
?
The request will be requeued more-or-less immediately, avoiding the
issue with timeout handler kicking in.
And the queue will remain blocked until the ungate work item returns, at
which point I/O submission will continue.
As the request will be requeued to the head of the queue there won't be
other I/O competing with tags, so it shouldn't have any adverse effects.

Wouldn't that work?

Cheers,

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


[PATCHv2 6/6] scsi_sysfs: call 'device_add' after attaching device handler

2016-03-02 Thread Hannes Reinecke
'device_add' will be evaluating the 'is_visible' callback
when creating the sysfs attributes. As by this time the
device handler has not been attached the 'access_state'
attribute will never be visible.

This patch moves the code around so that the device handler
is present by the time 'is_visible' is evaluated to
correctly display the 'access_state' attribute.

Reviewed-by: Christoph Hellwig 
Signed-off-by: Hannes Reinecke 
---
 drivers/scsi/scsi_sysfs.c | 15 ---
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
index c5ac171..d164419 100644
--- a/drivers/scsi/scsi_sysfs.c
+++ b/drivers/scsi/scsi_sysfs.c
@@ -1220,13 +1220,6 @@ int scsi_sysfs_add_sdev(struct scsi_device *sdev)
 
scsi_autopm_get_device(sdev);
 
-   error = device_add(&sdev->sdev_gendev);
-   if (error) {
-   sdev_printk(KERN_INFO, sdev,
-   "failed to add device: %d\n", error);
-   return error;
-   }
-
error = scsi_dh_add_device(sdev);
if (error)
/*
@@ -1235,6 +1228,14 @@ int scsi_sysfs_add_sdev(struct scsi_device *sdev)
sdev_printk(KERN_INFO, sdev,
"failed to add device handler: %d\n", error);
 
+   error = device_add(&sdev->sdev_gendev);
+   if (error) {
+   sdev_printk(KERN_INFO, sdev,
+   "failed to add device: %d\n", error);
+   scsi_dh_remove_device(sdev);
+   return error;
+   }
+
device_enable_async_suspend(&sdev->sdev_dev);
error = device_add(&sdev->sdev_dev);
if (error) {
-- 
1.8.5.6

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


[PATCHv2 3/6] scsi_dh_alua: update 'access_state' field

2016-03-02 Thread Hannes Reinecke
Track attached SCSI devices and update the 'access_state' field
whenever an ALUA state change has been detected.

Reviewed-by: Bart Van Assche 
Reviewed-by: Ewan Milne 
Reviewed-by: Christoph Hellwig 
Signed-off-by: Hannes Reinecke 
---
 drivers/scsi/device_handler/scsi_dh_alua.c | 48 --
 1 file changed, 46 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/device_handler/scsi_dh_alua.c 
b/drivers/scsi/device_handler/scsi_dh_alua.c
index 19f6539..5bcdf8d 100644
--- a/drivers/scsi/device_handler/scsi_dh_alua.c
+++ b/drivers/scsi/device_handler/scsi_dh_alua.c
@@ -24,6 +24,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -75,6 +76,7 @@ struct alua_port_group {
struct kref kref;
struct rcu_head rcu;
struct list_headnode;
+   struct list_headdh_list;
unsigned char   device_id_str[256];
int device_id_len;
int group_id;
@@ -92,6 +94,7 @@ struct alua_port_group {
 };
 
 struct alua_dh_data {
+   struct list_headnode;
struct alua_port_group  *pg;
int group_id;
spinlock_t  pg_lock;
@@ -247,6 +250,7 @@ struct alua_port_group *alua_alloc_pg(struct scsi_device 
*sdev,
INIT_DELAYED_WORK(&pg->rtpg_work, alua_rtpg_work);
INIT_LIST_HEAD(&pg->rtpg_list);
INIT_LIST_HEAD(&pg->node);
+   INIT_LIST_HEAD(&pg->dh_list);
spin_lock_init(&pg->lock);
 
spin_lock(&port_group_lock);
@@ -328,6 +332,8 @@ static int alua_check_vpd(struct scsi_device *sdev, struct 
alua_dh_data *h,
 {
int rel_port = -1, group_id;
struct alua_port_group *pg, *old_pg = NULL;
+   bool pg_updated;
+   unsigned long flags;
 
group_id = scsi_vpd_tpg_id(sdev, &rel_port);
if (group_id < 0) {
@@ -357,10 +363,22 @@ static int alua_check_vpd(struct scsi_device *sdev, 
struct alua_dh_data *h,
old_pg = h->pg;
if (old_pg != pg) {
/* port group has changed. Update to new port group */
+   if (h->pg) {
+   spin_lock_irqsave(&old_pg->lock, flags);
+   list_del_rcu(&h->node);
+   spin_unlock_irqrestore(&old_pg->lock, flags);
+   }
rcu_assign_pointer(h->pg, pg);
+   pg_updated = true;
}
+
+   spin_lock_irqsave(&pg->lock, flags);
if (sdev->synchronous_alua)
pg->flags |= ALUA_SYNC_STPG;
+   if (pg_updated)
+   list_add_rcu(&h->node, &pg->dh_list);
+   spin_unlock_irqrestore(&pg->lock, flags);
+
alua_rtpg_queue(h->pg, sdev, NULL, true);
spin_unlock(&h->pg_lock);
 
@@ -613,8 +631,18 @@ static int alua_rtpg(struct scsi_device *sdev, struct 
alua_port_group *pg)
if (spin_trylock_irqsave(&tmp_pg->lock, flags)) {
if ((tmp_pg == pg) ||
!(tmp_pg->flags & ALUA_PG_RUNNING)) {
+   struct alua_dh_data *h;
+
tmp_pg->state = desc[0] & 0x0f;
tmp_pg->pref = desc[0] >> 7;
+   rcu_read_lock();
+   list_for_each_entry_rcu(h,
+   &tmp_pg->dh_list, node) {
+   /* h->sdev should always be 
valid */
+   BUG_ON(!h->sdev);
+   h->sdev->access_state = desc[0];
+   }
+   rcu_read_unlock();
}
if (tmp_pg == pg)
valid_states = desc[1];
@@ -645,10 +673,22 @@ static int alua_rtpg(struct scsi_device *sdev, struct 
alua_port_group *pg)
pg->interval = 2;
err = SCSI_DH_RETRY;
} else {
+   struct alua_dh_data *h;
+
/* Transitioning time exceeded, set port to standby */
err = SCSI_DH_IO;
pg->state = SCSI_ACCESS_STATE_STANDBY;
pg->expiry = 0;
+   rcu_read_lock();
+   list_for_each_entry_rcu(h, &pg->dh_list, node) {
+   BUG_ON(!h->sdev);
+   h->sdev->access_state =
+   (pg->state & SCSI_ACCESS_STATE_MASK);
+   if (pg->pref)
+   h->sdev->access_state |=
+   SCSI_ACCESS_STATE_PREFERRED;
+   }
+  

[PATCHv2 0/6] SCSI 'access_state' attribute

2016-03-02 Thread Hannes Reinecke
Hi all,

here's the patchset to add an 'access_state' and 'preferred_path'
attribute. It will display the access state of a path if a
hardware handler is attached.
The access_state is given in terms of SCSI ALUA, and
the vendor-specific access state (eg for rdac or alua)
are mapped onto the ALUA values.
Additionally the 'is_visible' callback is updated to
only display the attributes if they are supported.

Changes to v1:
- Use sprintf instead of snprintf
- Call missing scsi_dh_remove() after a failed device_add()

Hannes Reinecke (6):
  scsi: Add 'access_state' and 'preferred_path' attribute
  scsi_dh_alua: use common definitions for ALUA state
  scsi_dh_alua: update 'access_state' field
  scsi_dh_rdac: update 'access_state' field
  scsi_dh_emc: update 'access_state' field
  scsi_sysfs: call 'device_add' after attaching device handler

 drivers/scsi/device_handler/scsi_dh_alua.c | 106 +++--
 drivers/scsi/device_handler/scsi_dh_emc.c  |   7 +-
 drivers/scsi/device_handler/scsi_dh_rdac.c |  38 +--
 drivers/scsi/scsi_sysfs.c  |  89 ++--
 include/scsi/scsi_device.h |   1 +
 include/scsi/scsi_proto.h  |  12 
 6 files changed, 203 insertions(+), 50 deletions(-)

-- 
1.8.5.6

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


[PATCHv2 5/6] scsi_dh_emc: update 'access_state' field

2016-03-02 Thread Hannes Reinecke
Update the 'access_state' field of the SCSI device whenever
the path state changes.

Reviewed-by: Christoph Hellwig 
Signed-off-by: Hannes Reinecke 
---
 drivers/scsi/device_handler/scsi_dh_emc.c | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/scsi/device_handler/scsi_dh_emc.c 
b/drivers/scsi/device_handler/scsi_dh_emc.c
index e6fb97c..375d818 100644
--- a/drivers/scsi/device_handler/scsi_dh_emc.c
+++ b/drivers/scsi/device_handler/scsi_dh_emc.c
@@ -199,7 +199,12 @@ static int parse_sp_info_reply(struct scsi_device *sdev,
csdev->lun_state = csdev->buffer[4];
csdev->current_sp = csdev->buffer[8];
csdev->port = csdev->buffer[7];
-
+   if (csdev->lun_state == CLARIION_LUN_OWNED)
+   sdev->access_state = SCSI_ACCESS_STATE_OPTIMAL;
+   else
+   sdev->access_state = SCSI_ACCESS_STATE_STANDBY;
+   if (csdev->default_sp == csdev->current_sp)
+   sdev->access_state |= SCSI_ACCESS_STATE_PREFERRED;
 out:
return err;
 }
-- 
1.8.5.6

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


[PATCHv2 4/6] scsi_dh_rdac: update 'access_state' field

2016-03-02 Thread Hannes Reinecke
Track attached SCSI devices and update the 'access_state'
whenever the path state of the device changes.

Reviewed-by: Christoph Hellwig 
Signed-off-by: Hannes Reinecke 
---
 drivers/scsi/device_handler/scsi_dh_rdac.c | 38 --
 1 file changed, 31 insertions(+), 7 deletions(-)

diff --git a/drivers/scsi/device_handler/scsi_dh_rdac.c 
b/drivers/scsi/device_handler/scsi_dh_rdac.c
index 93880ed..06fbd0b 100644
--- a/drivers/scsi/device_handler/scsi_dh_rdac.c
+++ b/drivers/scsi/device_handler/scsi_dh_rdac.c
@@ -165,6 +165,7 @@ struct rdac_controller {
struct work_struct  ms_work;
struct scsi_device  *ms_sdev;
struct list_headms_head;
+   struct list_headdh_list;
 };
 
 struct c2_inquiry {
@@ -181,7 +182,9 @@ struct c2_inquiry {
 };
 
 struct rdac_dh_data {
+   struct list_headnode;
struct rdac_controller  *ctlr;
+   struct scsi_device  *sdev;
 #define UNINITIALIZED_LUN  (1 << 8)
unsignedlun;
 
@@ -392,6 +395,7 @@ static struct rdac_controller *get_controller(int index, 
char *array_name,
INIT_WORK(&ctlr->ms_work, send_mode_select);
INIT_LIST_HEAD(&ctlr->ms_head);
list_add(&ctlr->node, &ctlr_list);
+   INIT_LIST_HEAD(&ctlr->dh_list);
 
return ctlr;
 }
@@ -455,7 +459,8 @@ static int get_lun_info(struct scsi_device *sdev, struct 
rdac_dh_data *h,
 
 static int check_ownership(struct scsi_device *sdev, struct rdac_dh_data *h)
 {
-   int err;
+   int err, access_state;
+   struct rdac_dh_data *tmp;
struct c9_inquiry *inqp;
 
h->state = RDAC_STATE_ACTIVE;
@@ -471,19 +476,31 @@ static int check_ownership(struct scsi_device *sdev, 
struct rdac_dh_data *h)
h->mode = RDAC_MODE; /* LUN in RDAC mode */
 
/* Update ownership */
-   if (inqp->avte_cvp & 0x1)
+   if (inqp->avte_cvp & 0x1) {
h->lun_state = RDAC_LUN_OWNED;
-   else {
+   access_state = SCSI_ACCESS_STATE_OPTIMAL;
+   } else {
h->lun_state = RDAC_LUN_UNOWNED;
-   if (h->mode == RDAC_MODE)
+   if (h->mode == RDAC_MODE) {
h->state = RDAC_STATE_PASSIVE;
+   access_state = SCSI_ACCESS_STATE_STANDBY;
+   } else
+   access_state = SCSI_ACCESS_STATE_ACTIVE;
}
 
/* Update path prio*/
-   if (inqp->path_prio & 0x1)
+   if (inqp->path_prio & 0x1) {
h->preferred = RDAC_PREFERRED;
-   else
+   access_state |= SCSI_ACCESS_STATE_PREFERRED;
+   } else
h->preferred = RDAC_NON_PREFERRED;
+   rcu_read_lock();
+   list_for_each_entry_rcu(tmp, &h->ctlr->dh_list, node) {
+   /* h->sdev should always be valid */
+   BUG_ON(!tmp->sdev);
+   tmp->sdev->access_state = access_state;
+   }
+   rcu_read_unlock();
}
 
return err;
@@ -508,6 +525,10 @@ static int initialize_controller(struct scsi_device *sdev,
h->ctlr = get_controller(index, array_name, array_id, sdev);
if (!h->ctlr)
err = SCSI_DH_RES_TEMP_UNAVAIL;
+   else {
+   list_add_rcu(&h->node, &h->ctlr->dh_list);
+   h->sdev = sdev;
+   }
spin_unlock(&list_lock);
}
return err;
@@ -829,8 +850,11 @@ static void rdac_bus_detach( struct scsi_device *sdev )
flush_workqueue(kmpath_rdacd);
 
spin_lock(&list_lock);
-   if (h->ctlr)
+   if (h->ctlr) {
+   list_del_rcu(&h->node);
+   h->sdev = NULL;
kref_put(&h->ctlr->kref, release_controller);
+   }
spin_unlock(&list_lock);
sdev->handler_data = NULL;
kfree(h);
-- 
1.8.5.6

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


[PATCHv2 2/6] scsi_dh_alua: use common definitions for ALUA state

2016-03-02 Thread Hannes Reinecke
scsi_proto.h now contains definitions for the ALUA state,
so we don't have to carry them in the device handler.

Reviewed-by: Bart van Assche 
Reviewed-by: Christoph Hellwig 
Signed-off-by: Hannes Reinecke 
---
 drivers/scsi/device_handler/scsi_dh_alua.c | 58 +-
 1 file changed, 25 insertions(+), 33 deletions(-)

diff --git a/drivers/scsi/device_handler/scsi_dh_alua.c 
b/drivers/scsi/device_handler/scsi_dh_alua.c
index 9a7657e..19f6539 100644
--- a/drivers/scsi/device_handler/scsi_dh_alua.c
+++ b/drivers/scsi/device_handler/scsi_dh_alua.c
@@ -31,14 +31,6 @@
 #define ALUA_DH_NAME "alua"
 #define ALUA_DH_VER "2.0"
 
-#define TPGS_STATE_OPTIMIZED   0x0
-#define TPGS_STATE_NONOPTIMIZED0x1
-#define TPGS_STATE_STANDBY 0x2
-#define TPGS_STATE_UNAVAILABLE 0x3
-#define TPGS_STATE_LBA_DEPENDENT   0x4
-#define TPGS_STATE_OFFLINE 0xe
-#define TPGS_STATE_TRANSITIONING   0xf
-
 #define TPGS_SUPPORT_NONE  0x00
 #define TPGS_SUPPORT_OPTIMIZED 0x01
 #define TPGS_SUPPORT_NONOPTIMIZED  0x02
@@ -180,7 +172,7 @@ static int submit_stpg(struct scsi_device *sdev, int 
group_id,
 
/* Prepare the data buffer */
memset(stpg_data, 0, stpg_len);
-   stpg_data[4] = TPGS_STATE_OPTIMIZED & 0x0f;
+   stpg_data[4] = SCSI_ACCESS_STATE_OPTIMAL;
put_unaligned_be16(group_id, &stpg_data[6]);
 
/* Prepare the command. */
@@ -248,7 +240,7 @@ struct alua_port_group *alua_alloc_pg(struct scsi_device 
*sdev,
}
pg->group_id = group_id;
pg->tpgs = tpgs;
-   pg->state = TPGS_STATE_OPTIMIZED;
+   pg->state = SCSI_ACCESS_STATE_OPTIMAL;
if (optimize_stpg)
pg->flags |= ALUA_OPTIMIZE_STPG;
kref_init(&pg->kref);
@@ -378,22 +370,22 @@ static int alua_check_vpd(struct scsi_device *sdev, 
struct alua_dh_data *h,
return SCSI_DH_OK;
 }
 
-static char print_alua_state(int state)
+static char print_alua_state(unsigned char state)
 {
switch (state) {
-   case TPGS_STATE_OPTIMIZED:
+   case SCSI_ACCESS_STATE_OPTIMAL:
return 'A';
-   case TPGS_STATE_NONOPTIMIZED:
+   case SCSI_ACCESS_STATE_ACTIVE:
return 'N';
-   case TPGS_STATE_STANDBY:
+   case SCSI_ACCESS_STATE_STANDBY:
return 'S';
-   case TPGS_STATE_UNAVAILABLE:
+   case SCSI_ACCESS_STATE_UNAVAILABLE:
return 'U';
-   case TPGS_STATE_LBA_DEPENDENT:
+   case SCSI_ACCESS_STATE_LBA:
return 'L';
-   case TPGS_STATE_OFFLINE:
+   case SCSI_ACCESS_STATE_OFFLINE:
return 'O';
-   case TPGS_STATE_TRANSITIONING:
+   case SCSI_ACCESS_STATE_TRANSITIONING:
return 'T';
default:
return 'X';
@@ -647,7 +639,7 @@ static int alua_rtpg(struct scsi_device *sdev, struct 
alua_port_group *pg)
valid_states&TPGS_SUPPORT_OPTIMIZED?'A':'a');
 
switch (pg->state) {
-   case TPGS_STATE_TRANSITIONING:
+   case SCSI_ACCESS_STATE_TRANSITIONING:
if (time_before(jiffies, pg->expiry)) {
/* State transition, retry */
pg->interval = 2;
@@ -655,11 +647,11 @@ static int alua_rtpg(struct scsi_device *sdev, struct 
alua_port_group *pg)
} else {
/* Transitioning time exceeded, set port to standby */
err = SCSI_DH_IO;
-   pg->state = TPGS_STATE_STANDBY;
+   pg->state = SCSI_ACCESS_STATE_STANDBY;
pg->expiry = 0;
}
break;
-   case TPGS_STATE_OFFLINE:
+   case SCSI_ACCESS_STATE_OFFLINE:
/* Path unusable */
err = SCSI_DH_DEV_OFFLINED;
pg->expiry = 0;
@@ -693,20 +685,20 @@ static unsigned alua_stpg(struct scsi_device *sdev, 
struct alua_port_group *pg)
return SCSI_DH_RETRY;
}
switch (pg->state) {
-   case TPGS_STATE_OPTIMIZED:
+   case SCSI_ACCESS_STATE_OPTIMAL:
return SCSI_DH_OK;
-   case TPGS_STATE_NONOPTIMIZED:
+   case SCSI_ACCESS_STATE_ACTIVE:
if ((pg->flags & ALUA_OPTIMIZE_STPG) &&
!pg->pref &&
(pg->tpgs & TPGS_MODE_IMPLICIT))
return SCSI_DH_OK;
break;
-   case TPGS_STATE_STANDBY:
-   case TPGS_STATE_UNAVAILABLE:
+   case SCSI_ACCESS_STATE_STANDBY:
+   case SCSI_ACCESS_STATE_UNAVAILABLE:
break;
-   case TPGS_STATE_OFFLINE:
+   case SCSI_ACCESS_STATE_OFFLINE:
return SCSI_DH_IO;
-   case TPGS_STATE_TRANSITIONING:
+   case SCSI_ACCESS_STATE_TRANSITIONING:
break;
default:
sdev_printk(KERN_INFO, sdev,
@@ -760,7 +752,7 @@ static void alua_rtpg_work(struct work_struct *work)

[PATCHv2 1/6] scsi: Add 'access_state' and 'preferred_path' attribute

2016-03-02 Thread Hannes Reinecke
Add an 'access_state' field to struct scsi_device
and display them in sysfs as 'access_state' and
'preferred_path' attribute.

Reviewed-by: Christoph Hellwig 
Signed-off-by: Hannes Reinecke 
---
 drivers/scsi/scsi_sysfs.c  | 74 ++
 include/scsi/scsi_device.h |  1 +
 include/scsi/scsi_proto.h  | 12 
 3 files changed, 87 insertions(+)

diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
index 7e57800..c5ac171 100644
--- a/drivers/scsi/scsi_sysfs.c
+++ b/drivers/scsi/scsi_sysfs.c
@@ -81,6 +81,33 @@ const char *scsi_host_state_name(enum scsi_host_state state)
return name;
 }
 
+static const struct {
+   unsigned char   value;
+   char*name;
+} sdev_access_states[] = {
+   { SCSI_ACCESS_STATE_OPTIMAL, "active/optimized" },
+   { SCSI_ACCESS_STATE_ACTIVE, "active/non-optimized" },
+   { SCSI_ACCESS_STATE_STANDBY, "standby" },
+   { SCSI_ACCESS_STATE_UNAVAILABLE, "unavailable" },
+   { SCSI_ACCESS_STATE_LBA, "lba-dependent" },
+   { SCSI_ACCESS_STATE_OFFLINE, "offline" },
+   { SCSI_ACCESS_STATE_TRANSITIONING, "transitioning" },
+};
+
+const char *scsi_access_state_name(unsigned char state)
+{
+   int i;
+   char *name = NULL;
+
+   for (i = 0; i < ARRAY_SIZE(sdev_access_states); i++) {
+   if (sdev_access_states[i].value == state) {
+   name = sdev_access_states[i].name;
+   break;
+   }
+   }
+   return name;
+}
+
 static int check_set(unsigned long long *val, char *src)
 {
char *last;
@@ -973,6 +1000,43 @@ sdev_store_dh_state(struct device *dev, struct 
device_attribute *attr,
 
 static DEVICE_ATTR(dh_state, S_IRUGO | S_IWUSR, sdev_show_dh_state,
   sdev_store_dh_state);
+
+static ssize_t
+sdev_show_access_state(struct device *dev,
+  struct device_attribute *attr,
+  char *buf)
+{
+   struct scsi_device *sdev = to_scsi_device(dev);
+   unsigned char access_state;
+   const char *access_state_name;
+
+   if (!sdev->handler)
+   return -EINVAL;
+
+   access_state = (sdev->access_state & SCSI_ACCESS_STATE_MASK);
+   access_state_name = scsi_access_state_name(access_state);
+
+   return sprintf(buf, "%s\n",
+  access_state_name ? access_state_name : "unknown");
+}
+static DEVICE_ATTR(access_state, S_IRUGO, sdev_show_access_state, NULL);
+
+static ssize_t
+sdev_show_preferred_path(struct device *dev,
+struct device_attribute *attr,
+char *buf)
+{
+   struct scsi_device *sdev = to_scsi_device(dev);
+
+   if (!sdev->handler)
+   return -EINVAL;
+
+   if (sdev->access_state & SCSI_ACCESS_STATE_PREFERRED)
+   return sprintf(buf, "1\n");
+   else
+   return sprintf(buf, "0\n");
+}
+static DEVICE_ATTR(preferred_path, S_IRUGO, sdev_show_preferred_path, NULL);
 #endif
 
 static ssize_t
@@ -1020,6 +1084,14 @@ static umode_t scsi_sdev_attr_is_visible(struct kobject 
*kobj,
!sdev->host->hostt->change_queue_depth)
return 0;
 
+#ifdef CONFIG_SCSI_DH
+   if (attr == &dev_attr_access_state.attr &&
+   !sdev->handler)
+   return 0;
+   if (attr == &dev_attr_preferred_path.attr &&
+   !sdev->handler)
+   return 0;
+#endif
return attr->mode;
 }
 
@@ -1063,6 +1135,8 @@ static struct attribute *scsi_sdev_attrs[] = {
&dev_attr_wwid.attr,
 #ifdef CONFIG_SCSI_DH
&dev_attr_dh_state.attr,
+   &dev_attr_access_state.attr,
+   &dev_attr_preferred_path.attr,
 #endif
&dev_attr_queue_ramp_up_period.attr,
REF_EVT(media_change),
diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h
index 4af2b24..c067019 100644
--- a/include/scsi/scsi_device.h
+++ b/include/scsi/scsi_device.h
@@ -201,6 +201,7 @@ struct scsi_device {
struct scsi_device_handler *handler;
void*handler_data;
 
+   unsigned char   access_state;
enum scsi_device_state sdev_state;
unsigned long   sdev_data[0];
 } __attribute__((aligned(sizeof(unsigned long;
diff --git a/include/scsi/scsi_proto.h b/include/scsi/scsi_proto.h
index a9fbf1b..c2ae21c 100644
--- a/include/scsi/scsi_proto.h
+++ b/include/scsi/scsi_proto.h
@@ -277,5 +277,17 @@ struct scsi_lun {
__u8 scsi_lun[8];
 };
 
+/* SPC asymmetric access states */
+#define SCSI_ACCESS_STATE_OPTIMAL 0x00
+#define SCSI_ACCESS_STATE_ACTIVE  0x01
+#define SCSI_ACCESS_STATE_STANDBY 0x02
+#define SCSI_ACCESS_STATE_UNAVAILABLE 0x03
+#define SCSI_ACCESS_STATE_LBA 0x04
+#define SCSI_ACCESS_STATE_OFFLINE 0x0e
+#define SCSI_ACCESS_STATE_TRANSITIONING 0x0f
+
+/* Values for REPORT TARGET GROUP STATES */
+#define SCSI_ACCESS_STATE_MASK0x0f
+#define SCSI_ACCESS_STATE_PREFERRED   

[PATCHv2] scsi_sysfs: add 'is_bin_visible' callback

2016-03-02 Thread Hannes Reinecke
Add 'is_bin_visible' callback to blank out unsupported vpd pages.

Reviewed-by: Shane Seymour 
Reviewed-by: Johannes Thumshirn 
Signed-off-by: Hannes Reinecke 

diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
index 00bc721..7e57800 100644
--- a/drivers/scsi/scsi_sysfs.c
+++ b/drivers/scsi/scsi_sysfs.c
@@ -1023,6 +1023,22 @@ static umode_t scsi_sdev_attr_is_visible(struct kobject 
*kobj,
return attr->mode;
 }
 
+static umode_t scsi_sdev_bin_attr_is_visible(struct kobject *kobj,
+struct bin_attribute *attr, int i)
+{
+   struct device *dev = container_of(kobj, struct device, kobj);
+   struct scsi_device *sdev = to_scsi_device(dev);
+
+
+   if (attr == &dev_attr_vpd_pg80 && !sdev->vpd_pg80)
+   return 0;
+
+   if (attr == &dev_attr_vpd_pg83 && sdev->vpd_pg83)
+   return 0;
+
+   return S_IRUGO;
+}
+
 /* Default template for device attributes.  May NOT be modified */
 static struct attribute *scsi_sdev_attrs[] = {
&dev_attr_device_blocked.attr,
@@ -1068,6 +1084,7 @@ static struct attribute_group scsi_sdev_attr_group = {
.attrs =scsi_sdev_attrs,
.bin_attrs =scsi_sdev_bin_attrs,
.is_visible =   scsi_sdev_attr_is_visible,
+   .is_bin_visible = scsi_sdev_bin_attr_is_visible,
 };
 
 static const struct attribute_group *scsi_sdev_attr_groups[] = {
--
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] sg: fix dxferp in from_to case

2016-03-02 Thread Douglas Gilbert

This patch is in response to this report:
   http://www.spinics.net/lists/linux-scsi/msg93456.html

One of the strange things that the original sg driver did was let
the user provide both a data-out buffer (it followed the
sg_header+cdb) _and_ specify a reply length greater than zero. What
happened was that the user data-out buffer was copied into some
kernel buffers and then the mid level was told a read type operation
would take place with the data from the device overwriting the same
kernel buffers. The user would then read those kernel buffers back
into the user space.

From what I can tell, the above action was broken by a change in
2008 and syzkaller found that out recently.

   ChangeLog:
  make sure that a user space pointer is passed through
  when data follows the sg_header structure and command.
  Fix the abnormal case when a non-zero reply_len is also
  given.

Signed-off-by: Douglas Gilbert 
diff --git a/drivers/scsi/sg.c b/drivers/scsi/sg.c
index 5e82067..ae7d9bd 100644
--- a/drivers/scsi/sg.c
+++ b/drivers/scsi/sg.c
@@ -652,7 +652,8 @@ sg_write(struct file *filp, const char __user *buf, size_t count, loff_t * ppos)
 	else
 		hp->dxfer_direction = (mxsize > 0) ? SG_DXFER_FROM_DEV : SG_DXFER_NONE;
 	hp->dxfer_len = mxsize;
-	if (hp->dxfer_direction == SG_DXFER_TO_DEV)
+	if ((hp->dxfer_direction == SG_DXFER_TO_DEV) ||
+	(hp->dxfer_direction == SG_DXFER_TO_FROM_DEV))
 		hp->dxferp = (char __user *)buf + cmd_size;
 	else
 		hp->dxferp = NULL;


[added to the 3.18 stable tree] qla2xxx: Use pci_enable_msix_range() instead of pci_enable_msix()

2016-03-02 Thread Sasha Levin
From: Quinn Tran 

This patch has been added to the 3.18 stable tree. If you have any
objections, please let us know.

===

[ Upstream commit 84e32a06f4f8756ce9ec3c8dc7e97896575f0771 ]

As result of deprecation of MSI-X/MSI enablement functions
pci_enable_msix() and pci_enable_msi_block() all drivers
using these two interfaces need to be updated to use the
new pci_enable_msi_range()  or pci_enable_msi_exact()
and pci_enable_msix_range() or pci_enable_msix_exact()
interfaces.

Log message code 0x00c6 preserved, although it is reported
after successful call to pci_enable_msix_range(), not before
possibly unsuccessful call to pci_enable_msix(). Consumers
of the error code should not notice the difference.

Signed-off-by: Alexander Gordeev 
Acked-by: Chad Dupuis 
Cc: qla2xxx-upstr...@qlogic.com
Cc: linux-scsi@vger.kernel.org
Cc: linux-...@vger.kernel.org
Signed-off-by: Christoph Hellwig 
Signed-off-by: Sasha Levin 
---
 drivers/scsi/qla2xxx/qla_init.c | 10 +-
 drivers/scsi/qla2xxx/qla_isr.c  |  4 ++--
 drivers/scsi/qla2xxx/qla_mid.c  |  4 ++--
 drivers/scsi/qla2xxx/qla_os.c   |  6 ++
 drivers/scsi/qla2xxx/qla_tmpl.c | 16 
 5 files changed, 31 insertions(+), 9 deletions(-)

diff --git a/drivers/scsi/qla2xxx/qla_init.c b/drivers/scsi/qla2xxx/qla_init.c
index a894b99..c919ac0 100644
--- a/drivers/scsi/qla2xxx/qla_init.c
+++ b/drivers/scsi/qla2xxx/qla_init.c
@@ -2146,7 +2146,7 @@ qla2x00_init_rings(scsi_qla_host_t *vha)
/* Clear outstanding commands array. */
for (que = 0; que < ha->max_req_queues; que++) {
req = ha->req_q_map[que];
-   if (!req)
+   if (!req || !test_bit(que, ha->req_qid_map))
continue;
req->out_ptr = (void *)(req->ring + req->length);
*req->out_ptr = 0;
@@ -2163,7 +2163,7 @@ qla2x00_init_rings(scsi_qla_host_t *vha)
 
for (que = 0; que < ha->max_rsp_queues; que++) {
rsp = ha->rsp_q_map[que];
-   if (!rsp)
+   if (!rsp || !test_bit(que, ha->rsp_qid_map))
continue;
rsp->in_ptr = (void *)(rsp->ring + rsp->length);
*rsp->in_ptr = 0;
@@ -4908,7 +4908,7 @@ qla25xx_init_queues(struct qla_hw_data *ha)
 
for (i = 1; i < ha->max_rsp_queues; i++) {
rsp = ha->rsp_q_map[i];
-   if (rsp) {
+   if (rsp && test_bit(i, ha->rsp_qid_map)) {
rsp->options &= ~BIT_0;
ret = qla25xx_init_rsp_que(base_vha, rsp);
if (ret != QLA_SUCCESS)
@@ -4923,8 +4923,8 @@ qla25xx_init_queues(struct qla_hw_data *ha)
}
for (i = 1; i < ha->max_req_queues; i++) {
req = ha->req_q_map[i];
-   if (req) {
-   /* Clear outstanding commands array. */
+   if (req && test_bit(i, ha->req_qid_map)) {
+   /* Clear outstanding commands array. */
req->options &= ~BIT_0;
ret = qla25xx_init_req_que(base_vha, req);
if (ret != QLA_SUCCESS)
diff --git a/drivers/scsi/qla2xxx/qla_isr.c b/drivers/scsi/qla2xxx/qla_isr.c
index a04a1b1..e191177 100644
--- a/drivers/scsi/qla2xxx/qla_isr.c
+++ b/drivers/scsi/qla2xxx/qla_isr.c
@@ -2981,9 +2981,9 @@ qla24xx_enable_msix(struct qla_hw_data *ha, struct 
rsp_que *rsp)
"MSI-X: Failed to enable support "
"-- %d/%d\n Retry with %d vectors.\n",
ha->msix_count, ret, ret);
+   ha->msix_count = ret;
+   ha->max_rsp_queues = ha->msix_count - 1;
}
-   ha->msix_count = ret;
-   ha->max_rsp_queues = ha->msix_count - 1;
ha->msix_entries = kzalloc(sizeof(struct qla_msix_entry) *
ha->msix_count, GFP_KERNEL);
if (!ha->msix_entries) {
diff --git a/drivers/scsi/qla2xxx/qla_mid.c b/drivers/scsi/qla2xxx/qla_mid.c
index 5c2e031..7c0b33d 100644
--- a/drivers/scsi/qla2xxx/qla_mid.c
+++ b/drivers/scsi/qla2xxx/qla_mid.c
@@ -595,7 +595,7 @@ qla25xx_delete_queues(struct scsi_qla_host *vha)
/* Delete request queues */
for (cnt = 1; cnt < ha->max_req_queues; cnt++) {
req = ha->req_q_map[cnt];
-   if (req) {
+   if (req && test_bit(cnt, ha->req_qid_map)) {
ret = qla25xx_delete_req_que(vha, req);
if (ret != QLA_SUCCESS) {
ql_log(ql_log_warn, vha, 0x00ea,
@@ -609,7 +609,7 @@ qla25xx_delete_queues(struct scsi_qla_host *vha)
/* Delete response queues */
for (cnt = 1; cnt < ha->max_rsp_queues; cnt++) {
rsp = ha->rsp_q_map[cnt];
-   if (rsp) {
+   if (rsp && test_bit(cnt, ha->rsp_qid_map)) {
ret = qla25xx_delete_rsp_que(vha, rsp);
if (ret != QLA_SUC

[added to the 4.1 stable tree] qla2xxx: Use pci_enable_msix_range() instead of pci_enable_msix()

2016-03-02 Thread Sasha Levin
From: Quinn Tran 

This patch has been added to the 4.1 stable tree. If you have any
objections, please let us know.

===

[ Upstream commit 84e32a06f4f8756ce9ec3c8dc7e97896575f0771 ]

As result of deprecation of MSI-X/MSI enablement functions
pci_enable_msix() and pci_enable_msi_block() all drivers
using these two interfaces need to be updated to use the
new pci_enable_msi_range()  or pci_enable_msi_exact()
and pci_enable_msix_range() or pci_enable_msix_exact()
interfaces.

Log message code 0x00c6 preserved, although it is reported
after successful call to pci_enable_msix_range(), not before
possibly unsuccessful call to pci_enable_msix(). Consumers
of the error code should not notice the difference.

Signed-off-by: Alexander Gordeev 
Acked-by: Chad Dupuis 
Cc: qla2xxx-upstr...@qlogic.com
Cc: linux-scsi@vger.kernel.org
Cc: linux-...@vger.kernel.org
Signed-off-by: Christoph Hellwig 
Signed-off-by: Sasha Levin 
---
 drivers/scsi/qla2xxx/qla_init.c | 10 +-
 drivers/scsi/qla2xxx/qla_isr.c  |  4 ++--
 drivers/scsi/qla2xxx/qla_mid.c  |  4 ++--
 drivers/scsi/qla2xxx/qla_os.c   |  6 ++
 drivers/scsi/qla2xxx/qla_tmpl.c | 16 
 5 files changed, 31 insertions(+), 9 deletions(-)

diff --git a/drivers/scsi/qla2xxx/qla_init.c b/drivers/scsi/qla2xxx/qla_init.c
index b323ad0..60f9651 100644
--- a/drivers/scsi/qla2xxx/qla_init.c
+++ b/drivers/scsi/qla2xxx/qla_init.c
@@ -2194,7 +2194,7 @@ qla2x00_init_rings(scsi_qla_host_t *vha)
/* Clear outstanding commands array. */
for (que = 0; que < ha->max_req_queues; que++) {
req = ha->req_q_map[que];
-   if (!req)
+   if (!req || !test_bit(que, ha->req_qid_map))
continue;
req->out_ptr = (void *)(req->ring + req->length);
*req->out_ptr = 0;
@@ -2211,7 +2211,7 @@ qla2x00_init_rings(scsi_qla_host_t *vha)
 
for (que = 0; que < ha->max_rsp_queues; que++) {
rsp = ha->rsp_q_map[que];
-   if (!rsp)
+   if (!rsp || !test_bit(que, ha->rsp_qid_map))
continue;
rsp->in_ptr = (void *)(rsp->ring + rsp->length);
*rsp->in_ptr = 0;
@@ -4957,7 +4957,7 @@ qla25xx_init_queues(struct qla_hw_data *ha)
 
for (i = 1; i < ha->max_rsp_queues; i++) {
rsp = ha->rsp_q_map[i];
-   if (rsp) {
+   if (rsp && test_bit(i, ha->rsp_qid_map)) {
rsp->options &= ~BIT_0;
ret = qla25xx_init_rsp_que(base_vha, rsp);
if (ret != QLA_SUCCESS)
@@ -4972,8 +4972,8 @@ qla25xx_init_queues(struct qla_hw_data *ha)
}
for (i = 1; i < ha->max_req_queues; i++) {
req = ha->req_q_map[i];
-   if (req) {
-   /* Clear outstanding commands array. */
+   if (req && test_bit(i, ha->req_qid_map)) {
+   /* Clear outstanding commands array. */
req->options &= ~BIT_0;
ret = qla25xx_init_req_que(base_vha, req);
if (ret != QLA_SUCCESS)
diff --git a/drivers/scsi/qla2xxx/qla_isr.c b/drivers/scsi/qla2xxx/qla_isr.c
index 6dc14cd..1f3991b 100644
--- a/drivers/scsi/qla2xxx/qla_isr.c
+++ b/drivers/scsi/qla2xxx/qla_isr.c
@@ -2992,9 +2992,9 @@ qla24xx_enable_msix(struct qla_hw_data *ha, struct 
rsp_que *rsp)
"MSI-X: Failed to enable support "
"-- %d/%d\n Retry with %d vectors.\n",
ha->msix_count, ret, ret);
+   ha->msix_count = ret;
+   ha->max_rsp_queues = ha->msix_count - 1;
}
-   ha->msix_count = ret;
-   ha->max_rsp_queues = ha->msix_count - 1;
ha->msix_entries = kzalloc(sizeof(struct qla_msix_entry) *
ha->msix_count, GFP_KERNEL);
if (!ha->msix_entries) {
diff --git a/drivers/scsi/qla2xxx/qla_mid.c b/drivers/scsi/qla2xxx/qla_mid.c
index cc94192..63abed1 100644
--- a/drivers/scsi/qla2xxx/qla_mid.c
+++ b/drivers/scsi/qla2xxx/qla_mid.c
@@ -601,7 +601,7 @@ qla25xx_delete_queues(struct scsi_qla_host *vha)
/* Delete request queues */
for (cnt = 1; cnt < ha->max_req_queues; cnt++) {
req = ha->req_q_map[cnt];
-   if (req) {
+   if (req && test_bit(cnt, ha->req_qid_map)) {
ret = qla25xx_delete_req_que(vha, req);
if (ret != QLA_SUCCESS) {
ql_log(ql_log_warn, vha, 0x00ea,
@@ -615,7 +615,7 @@ qla25xx_delete_queues(struct scsi_qla_host *vha)
/* Delete response queues */
for (cnt = 1; cnt < ha->max_rsp_queues; cnt++) {
rsp = ha->rsp_q_map[cnt];
-   if (rsp) {
+   if (rsp && test_bit(cnt, ha->rsp_qid_map)) {
ret = qla25xx_delete_rsp_que(vha, rsp);
if (ret != QLA_SUCC

Re: [PATCH v9 3/3] add support for DWC UFS Host Controller

2016-03-02 Thread Arnd Bergmann
On Wednesday 02 March 2016 16:46:47 Joao Pinto wrote:
> On 2/19/2016 3:03 PM, Arnd Bergmann wrote:
> > On Thursday 18 February 2016 17:20:27 Joao Pinto wrote:
> >> diff --git a/Documentation/devicetree/bindings/ufs/ufs-dwc.txt 
> >> b/Documentation/devicetree/bindings/ufs/ufs-dwc.txt

> > Please for the last time (!) add a proper version number of the
> > specific IP block to the compatible strings so you can identify
> > what hardware you are talking to.
> > 
> > You earlier confused the version number of the UFS standard with
> > the version number of the controller, and that has been clarified
> > now, but you really still need to use a version for the hardware
> > as well.
> 
> Ok, we can have a "snps,ufshcd-dwc-1.1" and a "snps,ufshcd-dwc-2.0". Agree?

I have no idea what versions of the dwc hardware block exist, but
I find it highly suspicious that the numbers happen to be the
same as the UFS protocol numbers, so I'd say that is probably
not the version of the IP block.

There are a few things you could try to find out the actual
version:

* If you are able to contact the team that worked on the test chip,
  please ask them what hardware revision you have.

* if you have some form of documentation of the hardware, look
  on the first few pages of the manual that describes the registers
  to see if the document has a revision history.

* If you have access to the hardware design files, look at the
  file names.

On https://www.synopsys.com/dw/ipdir.php?ds=ufs, I see a
version "1.30a" listed, which follows the typical numbering
scheme that your employer uses, a single digit followed by
a dot and a two-digit number and then a letter.

There is also a test chip with version 1.10a listed on the
same page, and that follows the same numbering system.

See if you can find a version number that fits into that scheme
in the documents you have available.

> >> +config SCSI_UFS_DWC_TC
> >> +  bool "Support for the Synopsys Test Chip"
> >> +  depends on SCSI_UFS_DWC_HOOKS && (SCSI_UFSHCD_PCI || SCSI_UFS_DWC_PLAT)
> >> +  ---help---
> >> +Synopsys Test Chip is a Phy for prototyping purposes.
> >> +This selects the support for the Synopsys Test Chip.
> >> +
> >> +Select this if you have a Synopsys Test Chip.
> >> +If unsure, say N.
> >> +
> >> +config SCSI_UFS_DWC_20BIT_RMMI
> >> +  bool "20-bit RMMI support"
> >> +  depends on SCSI_UFS_DWC_TC
> >> +  ---help---
> >> +This specifies that the Synopsys Test Chip supports 20-bit RMMI.
> >> +
> >> +Select this if you are using a 20-bit RMMI Synopsys Test Chip.
> >> +If unsure, say N.
> >> +
> >> +config SCSI_UFS_DWC_40BIT_RMMI
> >> +  bool "40-bit RMMI support"
> >> +  depends on SCSI_UFS_DWC_TC
> >> +  ---help---
> >> +Synopsys Test Chip is a Phy for prototyping purposes.
> >> +
> >> +Select this if you are using a 40-bit RMMI Synopsys Test Chip.
> >> +If unsure, say N.
> > 
> > I think it would be better to remove the SCSI_UFS_DWC_20BIT_RMMI
> > and SCSI_UFS_DWC_40BIT_RMMI configuration options now, and always
> > support both. There is not really much need for the options
> > as this is just a test chip, and nobody is going to care much
> > about saving a few bytes of object code.
> 
> We need to know if we are dealing with a 20-bit test chip or a 40-bit test 
> chip
> because the initialization is different. That can be made in the device tree 
> as
> you say bellow, but we can also use a setup in which the host is a PC (so no
> device tree) connected by pci bus to an fpga containing the UFS controller.
> Having this, I think that the only way is to choose the 20/40bit stuff in the
> menuconfig.

NAK.

Mutually exclusive compile-time configuration options are always wrong.

If the PCI hosts both have the same PCI device ID and there is no other
identification register that lets you find out which one you have,
maybe you can use a module parameter that defaults to invalid and that
has to be set explicitly when loading the PCI driver?

Are both test chips the same way? I see that the driver supports two
distinct PCI device IDs, so please check of they both come in variations
for the two PHYs, or if at least one of them always uses the same PHY
so you don't need the module parameter for that.

> >> diff --git a/drivers/scsi/ufs/Makefile b/drivers/scsi/ufs/Makefile
> >> index 8303bcc..bab8c05 100644
> >> --- a/drivers/scsi/ufs/Makefile
> >> +++ b/drivers/scsi/ufs/Makefile
> >> @@ -1,4 +1,7 @@
> >>  # UFSHCD makefile
> >> +obj-$(CONFIG_SCSI_UFS_DWC_HOOKS) += ufshcd-dwc.o
> >> +obj-$(CONFIG_SCSI_UFS_DWC_PLAT) += ufs-dwc.o
> >> +obj-$(CONFIG_SCSI_UFS_DWC_PCI) += ufs-dwc-pci.o
> >>  obj-$(CONFIG_SCSI_UFS_QCOM) += ufs-qcom.o
> >>  obj-$(CONFIG_SCSI_UFSHCD) += ufshcd.o
> >>  obj-$(CONFIG_SCSI_UFSHCD_PCI) += ufshcd-pci.o
> > 
> > However, please split out the SCSI_UFS_DWC_TC specific bits into
> > a separate file, and put the module_platform_driver() bits into
> > that file, to get the right abstraction where the most specific
> > drive

Re: aicasm: fix kbuild for separated build directories

2016-03-02 Thread James Bottomley
On Wed, 2016-03-02 at 17:25 +0100, Michal Marek wrote:
> On 2016-03-02 17:17, James Bottomley wrote:
> > On Thu, 2016-02-18 at 00:46 +0100, Michal Marek wrote:
> > > On Fri, Feb 12, 2016 at 02:42:26PM -0800, James Bottomley wrote:
> > > > I've recently been experimenting with building in emulated 
> > > > architecture containers which allow me to build natively on my 
> > > > laptop a kernel for any architecture which qemu will emulate. 
> > > >  To 
> > > > do this, I've been building into build/$(uname -m) and this
> > > > caused 
> > > > the aicasm stuff to fail to build (using
> > > > CONFIG_AIC7XXX_BUILD_FIRMW
> > > > ARE=y).  I think this patch corrects the problem, but I'm not 
> > > > hugely familiar with the kbuild infrastructure so I cc'd an
> > > > expert
> > > > for a second opinion.
> > > 
> > > Hi James,
> > > 
> > > Sorry for the late reply. Letting kbuild handle the aicasm
> > > directory 
> > > is a step in the right direction. However, it still failed for me
> > > and
> > > instead of trying to understand how the rules work, I removed
> > > them 
> > > and used the existing kbuild infrastructure. Please try the patch
> > > below on top of yours.
> > 
> > Sorry for the late testing.  This patch causes the build to fail
> > again
> > for me:
> > 
> > make[5]: *** No rule to make target
> > 'drivers/scsi/aic7xxx/aicasm/aicasm_scan.lex.c', needed by
> > 'drivers/scsi/aic7xxx/aicasm/aicasm_scan.lex.o'.  Stop.
> > /home/jejb/git/scsi-misc/drivers/scsi/aic7xxx/Makefile:85: recipe
> > for
> > target 'drivers/scsi/aic7xxx/aicasm/aicasm' failed
> > make[4]: *** [drivers/scsi/aic7xxx/aicasm/aicasm] Error 2
> > /home/jejb/git/scsi-misc/scripts/Makefile.build:407: recipe for
> > target
> > 'drivers/scsi/aic7xxx' failed
> > make[3]: *** [drivers/scsi/aic7xxx] Error 2
> > make[3]: *** Waiting for unfinished jobs
> > /home/jejb/git/scsi-misc/scripts/Makefile.build:407: recipe for
> > target
> > 'drivers/scsi' failed
> > 
> > I think the problem is simply that we now have two separate options
> > for
> > building the firmware: REGENERATE_PARSERS and
> >  CONFIG_AIC7XXX_BUILD_FIRMWARE and the latter needs to be
> > eliminated. 
> >  I'll see if I can work out what's missing.
> 
> Did you run the build with REGENERATE_PARSERS=1? It needs to be done
> once and the _shipped files need to be added to git. I did not
> include
> them in my patch for the sake of brevity.

The Build with REGENERATE_PARSERS works.  The build with this make line

make -j 4 O=build/x86_64 

Fails if these config options are set:

CONFIG_AIC7XXX_BUILD_FIRMWARE=y
CONFIG_AIC79XX_BUILD_FIRMWARE=y

I think the fix is just to excise those options from the build system.

James

--
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 v9 3/3] add support for DWC UFS Host Controller

2016-03-02 Thread Joao Pinto

Hi Arnd,

On 2/19/2016 3:03 PM, Arnd Bergmann wrote:
> On Thursday 18 February 2016 17:20:27 Joao Pinto wrote:
>>
>>  Documentation/devicetree/bindings/ufs/ufs-dwc.txt |  19 +
>>  MAINTAINERS   |   6 +
>>  drivers/scsi/ufs/Kconfig  |  51 +++
>>  drivers/scsi/ufs/Makefile |   3 +
>>  drivers/scsi/ufs/ufs-dwc-pci.c| 172 +
>>  drivers/scsi/ufs/ufs-dwc.c|  96 +
>>  drivers/scsi/ufs/ufshcd-dwc.c | 431 
>> ++
>>  drivers/scsi/ufs/ufshcd-dwc.h |  18 +
>>  drivers/scsi/ufs/ufshci-dwc.h |  42 +++
>>  drivers/scsi/ufs/unipro.h |  39 ++
> 
> I still think this can be split up further. From your previous
> explanation, I understand that there is a specific test chip
> that uses the "snps,ufshcd-dwc" implementation along with some
> other glue logic.
> 
> Please split this out so you have anything related to the test
> chips separate from the patch that implements core logic.

Ok, I will split more the patches.

>  
>> diff --git a/Documentation/devicetree/bindings/ufs/ufs-dwc.txt 
>> b/Documentation/devicetree/bindings/ufs/ufs-dwc.txt
>> new file mode 100644
>> index 000..59e9822
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/ufs/ufs-dwc.txt
>> @@ -0,0 +1,19 @@
>> +* Universal Flash Storage (UFS) DesignWare Host Controller
>> +
>> +DWC_UFSHC nodes are defined to describe on-chip UFS host controllers.
>> +Each UFS controller instance should have its own node.
>> +
>> +Required properties:
>> +- compatible: compatible list must contain "snps,ufshcd-dwc" and 
>> should
>> +  also contain the JEDEC version of the controller:
>> +"jedec,ufs-1.1"
>> +"jedec,ufs-2.0"
>> +- reg   : 
>> +- interrupts: 
> 
> 
> Based on your last reply to me (sorry for coming back to this so
> late), I think having just "snps,ufshcd-dwc" as the compatible
> string is not appropriate: This assumes that all "snps,ufshcd-dwc"
> instances are 100% compatible, but your code makes it very clear
> that this is not the case.
> 
> Please for the last time (!) add a proper version number of the
> specific IP block to the compatible strings so you can identify
> what hardware you are talking to.
> 
> You earlier confused the version number of the UFS standard with
> the version number of the controller, and that has been clarified
> now, but you really still need to use a version for the hardware
> as well.

Ok, we can have a "snps,ufshcd-dwc-1.1" and a "snps,ufshcd-dwc-2.0". Agree?

> 
>> +config SCSI_UFS_DWC_TC
>> +bool "Support for the Synopsys Test Chip"
>> +depends on SCSI_UFS_DWC_HOOKS && (SCSI_UFSHCD_PCI || SCSI_UFS_DWC_PLAT)
>> +---help---
>> +  Synopsys Test Chip is a Phy for prototyping purposes.
>> +  This selects the support for the Synopsys Test Chip.
>> +
>> +  Select this if you have a Synopsys Test Chip.
>> +  If unsure, say N.
>> +
>> +config SCSI_UFS_DWC_20BIT_RMMI
>> +bool "20-bit RMMI support"
>> +depends on SCSI_UFS_DWC_TC
>> +---help---
>> +  This specifies that the Synopsys Test Chip supports 20-bit RMMI.
>> +
>> +  Select this if you are using a 20-bit RMMI Synopsys Test Chip.
>> +  If unsure, say N.
>> +
>> +config SCSI_UFS_DWC_40BIT_RMMI
>> +bool "40-bit RMMI support"
>> +depends on SCSI_UFS_DWC_TC
>> +---help---
>> +  Synopsys Test Chip is a Phy for prototyping purposes.
>> +
>> +  Select this if you are using a 40-bit RMMI Synopsys Test Chip.
>> +  If unsure, say N.
> 
> I think it would be better to remove the SCSI_UFS_DWC_20BIT_RMMI
> and SCSI_UFS_DWC_40BIT_RMMI configuration options now, and always
> support both. There is not really much need for the options
> as this is just a test chip, and nobody is going to care much
> about saving a few bytes of object code.

We need to know if we are dealing with a 20-bit test chip or a 40-bit test chip
because the initialization is different. That can be made in the device tree as
you say bellow, but we can also use a setup in which the host is a PC (so no
device tree) connected by pci bus to an fpga containing the UFS controller.
Having this, I think that the only way is to choose the 20/40bit stuff in the
menuconfig.

> 
>> diff --git a/drivers/scsi/ufs/Makefile b/drivers/scsi/ufs/Makefile
>> index 8303bcc..bab8c05 100644
>> --- a/drivers/scsi/ufs/Makefile
>> +++ b/drivers/scsi/ufs/Makefile
>> @@ -1,4 +1,7 @@
>>  # UFSHCD makefile
>> +obj-$(CONFIG_SCSI_UFS_DWC_HOOKS) += ufshcd-dwc.o
>> +obj-$(CONFIG_SCSI_UFS_DWC_PLAT) += ufs-dwc.o
>> +obj-$(CONFIG_SCSI_UFS_DWC_PCI) += ufs-dwc-pci.o
>>  obj-$(CONFIG_SCSI_UFS_QCOM) += ufs-qcom.o
>>  obj-$(CONFIG_SCSI_UFSHCD) += ufshcd.o
>>  obj-$(CONFIG_SCSI_UFSHCD_PCI) += ufshcd-pci.o
> 
> However, please split out the SCSI_UFS_DWC_TC 

Re: aicasm: fix kbuild for separated build directories

2016-03-02 Thread Michal Marek
On 2016-03-02 17:17, James Bottomley wrote:
> On Thu, 2016-02-18 at 00:46 +0100, Michal Marek wrote:
>> On Fri, Feb 12, 2016 at 02:42:26PM -0800, James Bottomley wrote:
>>> I've recently been experimenting with building in emulated 
>>> architecture containers which allow me to build natively on my 
>>> laptop a kernel for any architecture which qemu will emulate.  To 
>>> do this, I've been building into build/$(uname -m) and this caused 
>>> the aicasm stuff to fail to build (using CONFIG_AIC7XXX_BUILD_FIRMW
>>> ARE=y).  I think this patch corrects the problem, but I'm not 
>>> hugely familiar with the kbuild infrastructure so I cc'd an expert
>>> for a second opinion.
>>
>> Hi James,
>>
>> Sorry for the late reply. Letting kbuild handle the aicasm directory 
>> is a step in the right direction. However, it still failed for me and
>> instead of trying to understand how the rules work, I removed them 
>> and used the existing kbuild infrastructure. Please try the patch 
>> below on top of yours.
> 
> Sorry for the late testing.  This patch causes the build to fail again
> for me:
> 
> make[5]: *** No rule to make target
> 'drivers/scsi/aic7xxx/aicasm/aicasm_scan.lex.c', needed by
> 'drivers/scsi/aic7xxx/aicasm/aicasm_scan.lex.o'.  Stop.
> /home/jejb/git/scsi-misc/drivers/scsi/aic7xxx/Makefile:85: recipe for
> target 'drivers/scsi/aic7xxx/aicasm/aicasm' failed
> make[4]: *** [drivers/scsi/aic7xxx/aicasm/aicasm] Error 2
> /home/jejb/git/scsi-misc/scripts/Makefile.build:407: recipe for target
> 'drivers/scsi/aic7xxx' failed
> make[3]: *** [drivers/scsi/aic7xxx] Error 2
> make[3]: *** Waiting for unfinished jobs
> /home/jejb/git/scsi-misc/scripts/Makefile.build:407: recipe for target
> 'drivers/scsi' failed
> 
> I think the problem is simply that we now have two separate options for
> building the firmware: REGENERATE_PARSERS and
>  CONFIG_AIC7XXX_BUILD_FIRMWARE and the latter needs to be eliminated. 
>  I'll see if I can work out what's missing.

Did you run the build with REGENERATE_PARSERS=1? It needs to be done
once and the _shipped files need to be added to git. I did not include
them in my patch for the sake of brevity.

Michal
--
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: aicasm: fix kbuild for separated build directories

2016-03-02 Thread James Bottomley
On Thu, 2016-02-18 at 00:46 +0100, Michal Marek wrote:
> On Fri, Feb 12, 2016 at 02:42:26PM -0800, James Bottomley wrote:
> > I've recently been experimenting with building in emulated 
> > architecture containers which allow me to build natively on my 
> > laptop a kernel for any architecture which qemu will emulate.  To 
> > do this, I've been building into build/$(uname -m) and this caused 
> > the aicasm stuff to fail to build (using CONFIG_AIC7XXX_BUILD_FIRMW
> > ARE=y).  I think this patch corrects the problem, but I'm not 
> > hugely familiar with the kbuild infrastructure so I cc'd an expert
> > for a second opinion.
> 
> Hi James,
> 
> Sorry for the late reply. Letting kbuild handle the aicasm directory 
> is a step in the right direction. However, it still failed for me and
> instead of trying to understand how the rules work, I removed them 
> and used the existing kbuild infrastructure. Please try the patch 
> below on top of yours.

Sorry for the late testing.  This patch causes the build to fail again
for me:

make[5]: *** No rule to make target
'drivers/scsi/aic7xxx/aicasm/aicasm_scan.lex.c', needed by
'drivers/scsi/aic7xxx/aicasm/aicasm_scan.lex.o'.  Stop.
/home/jejb/git/scsi-misc/drivers/scsi/aic7xxx/Makefile:85: recipe for
target 'drivers/scsi/aic7xxx/aicasm/aicasm' failed
make[4]: *** [drivers/scsi/aic7xxx/aicasm/aicasm] Error 2
/home/jejb/git/scsi-misc/scripts/Makefile.build:407: recipe for target
'drivers/scsi/aic7xxx' failed
make[3]: *** [drivers/scsi/aic7xxx] Error 2
make[3]: *** Waiting for unfinished jobs
/home/jejb/git/scsi-misc/scripts/Makefile.build:407: recipe for target
'drivers/scsi' failed

I think the problem is simply that we now have two separate options for
building the firmware: REGENERATE_PARSERS and
 CONFIG_AIC7XXX_BUILD_FIRMWARE and the latter needs to be eliminated. 
 I'll see if I can work out what's missing.

James

--
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 00/14] drivers: use __maybe_unused to hide pm functions

2016-03-02 Thread Arnd Bergmann
I found many variations of the bug in these device drivers (and some
USB drivers I already send patches for in a separate series).

In each case, the power management operations structure conditionally
references suspend/resume functions, but the functions are hidden
in an incorrect #ifdef or not hidden at all.

We could try to correct the #ifdefs, but it seems easier to just
mark those functions as __maybe_unused, which has the same effect
but provides better compile-time test coverage and (subjectively)
looks a bit nicer.

I have a patch series that avoids all warnings in ARM randconfig
builds, and I have verified that all these patches fix a warning that
is still present in today's linux-next, and that they do not
introduce new warnings in any configuration I found.

Note that all these drivers are ARM specific, so I assume that
all portable drivers got fixed already when someone rand into
the problem on x86.

There are no dependencies between the patches, so I'd appreciate
subsystem maintainers to put them directly into their git trees.

Arnd

Arnd Bergmann (14):
  pinctrl: at91: use __maybe_unused to hide pm functions
  irqchip: st: use __maybe_unused to hide st_irq_syscfg_resume
  power: ipaq-micro-battery: use __maybe_unused to hide pm functions
  power: pm2301-charger: use __maybe_unused to hide pm functions
  mfd: ipaq-micro: use __maybe_unused to hide pm functions
  dma: sirf: use __maybe_unused to hide pm functions
  hw_random: exynos: use __maybe_unused to hide pm functions
  scsi: mvumi: use __maybe_unused to hide pm functions
  amd-xgbe: use __maybe_unused to hide pm functions
  wireless: cw1200: use __maybe_unused to hide pm functions_
  input: spear-keyboard: use __maybe_unused to hide pm functions
  keyboard: snvs-pwrkey: use __maybe_unused to hide pm functions
  [media] omap3isp: use IS_ENABLED() to hide pm functions
  ASoC: rockchip: use __maybe_unused to hide st_irq_syscfg_resume

 drivers/char/hw_random/exynos-rng.c | 10 --
 drivers/dma/sirf-dma.c  | 10 --
 drivers/input/keyboard/snvs_pwrkey.c|  4 ++--
 drivers/input/keyboard/spear-keyboard.c |  6 ++
 drivers/irqchip/irq-st.c|  2 +-
 drivers/media/platform/omap3isp/isp.c   | 13 +
 drivers/mfd/ipaq-micro.c|  2 +-
 drivers/net/ethernet/amd/xgbe/xgbe-main.c   |  6 ++
 drivers/net/wireless/st/cw1200/cw1200_spi.c |  9 ++---
 drivers/net/wireless/st/cw1200/pm.h |  9 +++--
 drivers/pinctrl/pinctrl-at91-pio4.c |  4 ++--
 drivers/power/ipaq_micro_battery.c  |  4 ++--
 drivers/power/pm2301_charger.c  | 22 ++
 drivers/scsi/mvumi.c|  4 ++--
 sound/soc/rockchip/rockchip_spdif.c |  4 ++--
 15 files changed, 40 insertions(+), 69 deletions(-)

-- 
2.7.0
Cc: herb...@gondor.apana.org.au
Cc: k.kozlow...@samsung.com
Cc: dan.j.willi...@intel.com
Cc: vinod.k...@intel.com
Cc: bao...@kernel.org
Cc: dmitry.torok...@gmail.com
Cc: t...@linutronix.de
Cc: ja...@lakedaemon.net
Cc: marc.zyng...@arm.com
Cc: laurent.pinch...@ideasonboard.com
Cc: mche...@osg.samsung.com
Cc: lee.jo...@linaro.org
Cc: kv...@codeaurora.org
Cc: ludovic.desroc...@atmel.com
Cc: linus.wall...@linaro.org
Cc: s...@kernel.org
Cc: dbarysh...@gmail.com
Cc: jbottom...@odin.com
Cc: martin.peter...@oracle.com
Cc: broo...@kernel.org
Cc: linux-cry...@vger.kernel.org
Cc: linux-arm-ker...@lists.infradead.org
Cc: linux-samsung-...@vger.kernel.org
Cc: linux-ker...@vger.kernel.org
Cc: dmaeng...@vger.kernel.org
Cc: linux-in...@vger.kernel.org
Cc: linux-me...@vger.kernel.org
Cc: net...@vger.kernel.org
Cc: linux-wirel...@vger.kernel.org
Cc: linux-g...@vger.kernel.org
Cc: linux...@vger.kernel.org
Cc: linux-scsi@vger.kernel.org
Cc: alsa-de...@alsa-project.org
Cc: linux-rockc...@lists.infradead.org
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 08/14] scsi: mvumi: use __maybe_unused to hide pm functions

2016-03-02 Thread Arnd Bergmann
The mvumi scsi hides the references to its suspend/resume functions
in an #ifdef but does not hide the implementation the same way:

drivers/scsi/mvumi.c:2632:12: error: 'mvumi_suspend' defined but not used 
[-Werror=unused-function]
drivers/scsi/mvumi.c:2651:12: error: 'mvumi_resume' defined but not used 
[-Werror=unused-function]

This adds __maybe_unused annotations so the compiler knows
it can silently drop them instead of warning, while avoiding
the addition of another #ifdef.

Signed-off-by: Arnd Bergmann 
---
 drivers/scsi/mvumi.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/mvumi.c b/drivers/scsi/mvumi.c
index 02360de6b7e0..39285070f3b5 100644
--- a/drivers/scsi/mvumi.c
+++ b/drivers/scsi/mvumi.c
@@ -2629,7 +2629,7 @@ static void mvumi_shutdown(struct pci_dev *pdev)
mvumi_flush_cache(mhba);
 }
 
-static int mvumi_suspend(struct pci_dev *pdev, pm_message_t state)
+static int __maybe_unused mvumi_suspend(struct pci_dev *pdev, pm_message_t 
state)
 {
struct mvumi_hba *mhba = NULL;
 
@@ -2648,7 +2648,7 @@ static int mvumi_suspend(struct pci_dev *pdev, 
pm_message_t state)
return 0;
 }
 
-static int mvumi_resume(struct pci_dev *pdev)
+static int __maybe_unused mvumi_resume(struct pci_dev *pdev)
 {
int ret;
struct mvumi_hba *mhba = NULL;
-- 
2.7.0

--
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: [RFC 24/34] iscsi-target: validate conn operational parameters

2016-03-02 Thread Varun Prakash
On Tue, Mar 01, 2016 at 08:33:18PM +0530, Christoph Hellwig wrote:
> On Sun, Feb 14, 2016 at 11:15:31PM +0530, Varun Prakash wrote:
> > call validate params function if registered
> > by transport driver before starting negotiation,
> > so that transport driver can validate and update
> > the value of parameters.
> 
> Should go together with the introduction of the method.

Ok, I will update it in v2 series. 
--
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: [RFC 21/34] iscsi-target: release transport driver resources

2016-03-02 Thread Varun Prakash
On Tue, Mar 01, 2016 at 08:29:53PM +0530, Christoph Hellwig wrote:
> On Sun, Feb 14, 2016 at 11:12:15PM +0530, Varun Prakash wrote:
> > transport driver may allocate resources for an
> > iSCSI cmd, to free that resources iscsi target
> > must call release function registered by transport
> > driver.
> > 
> > ISCSI_TCP_CXGB4 frees DDP resource associated
> > with a WRITE cmd in the callback function.
> 
> This should go with the patch introducing the method.

Ok, I will update it in v2 series.
--
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: [RFC 15/34] iscsi-target: export symbols from iscsi_target.c

2016-03-02 Thread Varun Prakash
On Tue, Mar 01, 2016 at 08:19:48PM +0530, Christoph Hellwig wrote:
> On Sun, Feb 14, 2016 at 11:12:09PM +0530, Varun Prakash wrote:
> > export symbols from iscsi_target.c for
> > ISCSI_TCP_CXGB4 transport driver.
> 
> What exactly is the reason for the split between this and the previous
> patch?

To avoid a single big patch I created two separate patch,
also there are large number of functions exported from iscsi_target.c
as compared to other 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: [RFC 14/34] iscsi-target: export symbols

2016-03-02 Thread Varun Prakash
On Tue, Mar 01, 2016 at 08:19:21PM +0530, Christoph Hellwig wrote:
> This looks like pretty random exports and not something like a well
> defined interface to me :(

I have exported functions which works on iscsi-target data structures
and can be reused by offload drivers.
--
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: [RFC 13/34] iscsi-target: add new transport type

2016-03-02 Thread Varun Prakash
On Tue, Mar 01, 2016 at 08:18:48PM +0530, Christoph Hellwig wrote:
> This really looks like an odd interface.  I think everyone will
> be much happpier in the long run if you do a generic offload interface
> instead of special casing each possible driver.

Common offload transport type is a better option, but we
have to do many changes in iscsi-target to 
support multiple offload drivers simultaneously.
--
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: [RFC 12/34] cxgb4: update Kconfig and Makefile

2016-03-02 Thread Varun Prakash
On Tue, Mar 01, 2016 at 08:17:06PM +0530, Christoph Hellwig wrote:
> > +config CHELSIO_T4_UWIRE
> > +   bool "Unified Wire Support for Chelsio T5 cards"
> > +   default n
> > +   depends on CHELSIO_T4
> > +   ---help---
> > + Enable unified-wire offload features.
> > + Say Y here if you want to enable unified-wire over Ethernet
> > + in the driver.
> 
> And what the hell is "unified-wire over Ethernet".  A little more
> explanation would be very helpful for the user facing this question.

Ok, I will update the help in v2 series.
--
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] tcm_loop: use after free on error

2016-03-02 Thread Dan Carpenter
We dereference "tl_nexus" to get the error code.

Fixes: 1b418a8fcbc0 ('target: Convert demo-mode only drivers to 
target_alloc_session')
Signed-off-by: Dan Carpenter 

diff --git a/drivers/target/loopback/tcm_loop.c 
b/drivers/target/loopback/tcm_loop.c
index 0216c75..e0ffb03 100644
--- a/drivers/target/loopback/tcm_loop.c
+++ b/drivers/target/loopback/tcm_loop.c
@@ -808,6 +808,7 @@ static int tcm_loop_make_nexus(
 {
struct tcm_loop_hba *tl_hba = tl_tpg->tl_hba;
struct tcm_loop_nexus *tl_nexus;
+   int ret;
 
if (tl_tpg->tl_nexus) {
pr_debug("tl_tpg->tl_nexus already exists\n");
@@ -824,8 +825,9 @@ static int tcm_loop_make_nexus(
TARGET_PROT_DIN_PASS | 
TARGET_PROT_DOUT_PASS,
name, tl_nexus, NULL);
if (IS_ERR(tl_nexus->se_sess)) {
+   ret = PTR_ERR(tl_nexus->se_sess);
kfree(tl_nexus);
-   return PTR_ERR(tl_nexus->se_sess);
+   return ret;
}
 
tl_tpg->tl_nexus = tl_nexus;
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/2] scsi_transport_fc: implement 'disable_target_scan' module parameter

2016-03-02 Thread Hannes Reinecke
On 03/02/2016 05:18 PM, Christoph Hellwig wrote:
> On Wed, Mar 02, 2016 at 01:59:39PM +0800, Hannes Reinecke wrote:
>> Main reasons being that
>> a) it's most commonly on FC where you end up having thousands of LUNs;
>> SAS domains are of finite size and you'd need to string quite a few SAS
>> expanders together to achieve a similar setup. And iSCSI is ... well.
>> b) FC already has the required infrastructure in place where you easily
>> can suppress a target scan, making the patch really trivial.
>>
>> So rather than trying to come up with a generic solution and an overly
>> complicated patch I opted for the simpler version :-)
> 
> Having a global disable just for all FC is still odd.  Either you
> do it for all devices and always initiate a manual scan, or you
> need to do it on a per-host basis somehow.
> 
Understood.

Will be resubmitting with a global switch for all SCSI hosts.

Cheers,

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


Re: [PATCH][RESEND] scsi_sysfs: add 'is_bin_visible' callback

2016-03-02 Thread Hannes Reinecke
On 03/02/2016 05:19 PM, Christoph Hellwig wrote:
> On Wed, Mar 02, 2016 at 03:33:14PM +0800, Hannes Reinecke wrote:
 +  rcu_read_lock();
 +  if (attr == &dev_attr_vpd_pg80 &&
 +  !rcu_dereference(sdev->vpd_pg80)) {
 +  rcu_read_unlock();
 +  return 0;
 +  }
 +  if (attr == &dev_attr_vpd_pg83 &&
 +  !rcu_dereference(sdev->vpd_pg83)) {
 +  rcu_read_unlock();
 +  return 0;
 +  }
 +  rcu_read_unlock();
>>>
>>> We are only checking the pointers for being non-zero.  No need for the
>>> rcu_read_lock() or rcu_dereference() here.
>>>
>> Better to be same than sorry; some overly clever code analysis tool
>> might trip over it otherwise.
> 
> It shouldn't.  There is no dereference going on here.
> 
Ok.

>>
>>> Otherwise this looks fine to me.
>>
>> Reviewed-by: ?
> 
> Only without the cargo culted rcu magic.
> 
Okay, will be resending it.

Cheers,

Hannes
--
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 v1 2/3] PM / sleep: try to runtime suspend for direct complete

2016-03-02 Thread Ulf Hansson
On 2 March 2016 at 09:31, dbasehore .  wrote:
>
> On Mar 1, 2016 12:41, "Ulf Hansson"  wrote:
>>
>> On 25 February 2016 at 01:11, Derek Basehore 
>> wrote:
>> > This tries to runtime suspend devices that are still active for direct
>> > complete. This is for cases such as autosuspend delays which leaves
>> > devices able to runtime suspend but still active. It's beneficial in
>> > this case to runtime suspend the device to take advantage of direct
>> > complete when possible.
>>
>> Unfortunate this doesn't work. In the device_prepare() phase the PM
>> core prevents runtime suspend via a call to pm_runtime_get_noresume().
>>
>> Kind regards
>> Uffe
>>
>
> A call to pm_runtime_put_sync_suspend should work. We can't do this
> earlier, because we don't know if the device will actually use
> direct_complete due to its children. Calling pm_runtime_put_noidle may
> allow another pm_runtime_put to runtime suspend the device, but we're
> trying to do that anyways. Also, we can't just call ...put_sync since that
> might not suspend due to autosuspend delays.
>
> We just need to balance the call with another get_noresume after. Am I
> missing anything?

Yes.

The important part that you need to take into consideration is the
sysfs intererface for runtime PM of devices. It allows userspace to
prevent runtime PM suspend (pm_runtime_forbid()). Therefore, you
simply can not rely on the regular runtime PM APIs to put a device
into low power state during system PM.

In cases of a runtime PM centric subsystem/driver, it can instead use
pm_runtime_force_suspend().

Kind regards
Uffe
--
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] scsi_sysfs: add 'is_bin_visible' callback

2016-03-02 Thread Christoph Hellwig
On Wed, Mar 02, 2016 at 03:33:14PM +0800, Hannes Reinecke wrote:
> >> +  rcu_read_lock();
> >> +  if (attr == &dev_attr_vpd_pg80 &&
> >> +  !rcu_dereference(sdev->vpd_pg80)) {
> >> +  rcu_read_unlock();
> >> +  return 0;
> >> +  }
> >> +  if (attr == &dev_attr_vpd_pg83 &&
> >> +  !rcu_dereference(sdev->vpd_pg83)) {
> >> +  rcu_read_unlock();
> >> +  return 0;
> >> +  }
> >> +  rcu_read_unlock();
> > 
> > We are only checking the pointers for being non-zero.  No need for the
> > rcu_read_lock() or rcu_dereference() here.
> > 
> Better to be same than sorry; some overly clever code analysis tool
> might trip over it otherwise.

It shouldn't.  There is no dereference going on here.

> 
> > Otherwise this looks fine to me.
> 
> Reviewed-by: ?

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


Re: [PATCH 1/2] scsi_transport_fc: implement 'disable_target_scan' module parameter

2016-03-02 Thread Christoph Hellwig
On Wed, Mar 02, 2016 at 01:59:39PM +0800, Hannes Reinecke wrote:
> Main reasons being that
> a) it's most commonly on FC where you end up having thousands of LUNs;
> SAS domains are of finite size and you'd need to string quite a few SAS
> expanders together to achieve a similar setup. And iSCSI is ... well.
> b) FC already has the required infrastructure in place where you easily
> can suppress a target scan, making the patch really trivial.
> 
> So rather than trying to come up with a generic solution and an overly
> complicated patch I opted for the simpler version :-)

Having a global disable just for all FC is still odd.  Either you
do it for all devices and always initiate a manual scan, or you
need to do it on a per-host basis somehow.
--
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] snic: correctly check for array overrun on overly long version number

2016-03-02 Thread Narsimhulu Musini (nmusini)
On 26/02/16 4:28 am, "Colin King"  wrote:

>From: Colin Ian King 
>
>The snic version number is expected to be 4 decimals in the form like
>a netmask string with each number stored in an element in array v.
>However, there is an off-by-one check on the number of elements in v
>allowing one to pass a 5 decimal version number causing v[4] to be
>referenced, causing a buffer overrun.  Fix the off-by-one error by
>comparing to i > 3 rather than 4.
Acked-by: Narsimhulu Musini 

>
>Signed-off-by: Colin Ian King 
>---
> drivers/scsi/snic/snic_ctl.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/drivers/scsi/snic/snic_ctl.c b/drivers/scsi/snic/snic_ctl.c
>index aebe753..ab0e06b 100644
>--- a/drivers/scsi/snic/snic_ctl.c
>+++ b/drivers/scsi/snic/snic_ctl.c
>@@ -75,7 +75,7 @@ snic_ver_enc(const char *s)
>   continue;
>   }
> 
>-  if (i > 4 || !isdigit(c))
>+  if (i > 3 || !isdigit(c))
>   goto end;
> 
>   v[i] = v[i] * 10 + (c - '0');
>-- 
>2.7.0
>

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