* [PATCH 2/2] [SCSI] ufs: add SCSI host lock push-down for ufs.
@ 2012-05-01 14:31 Namjae Jeon
2012-05-01 20:59 ` James Bottomley
0 siblings, 1 reply; 3+ messages in thread
From: Namjae Jeon @ 2012-05-01 14:31 UTC (permalink / raw)
To: James.Bottomley, jBottomley, akpm; +Cc: linux-scsi, Namjae Jeon
SCSI host lock push-down patch is not applied in ufs host driver yet.
After adding it, I tried to remove unneeded spin lock in queuecommand also.
Signed-off-by: Namjae Jeon <linkinjeon@gmail.com>
---
drivers/scsi/ufs/ufshcd.c | 20 ++++++++++----------
1 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 3cb7a08..a134738 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -658,23 +658,23 @@ static void ufshcd_compose_upiu(struct ufshcd_lrb *lrbp)
}
/**
- * ufshcd_queuecommand - main entry point for SCSI requests
+ * ufshcd_queuecommand_lck - main entry point for SCSI requests
* @cmd: command from SCSI Midlayer
* @done: call back function
*
* Returns 0 for success, non-zero in case of failure
*/
-static int ufshcd_queuecommand(struct Scsi_Host *host, struct scsi_cmnd *cmd)
+static int
+ufshcd_queuecommand_lck(struct scsi_cmnd *scp, void (*done)(struct scsi_cmnd *))
{
struct ufshcd_lrb *lrbp;
struct ufs_hba *hba;
- unsigned long flags;
int tag;
int err = 0;
- hba = shost_priv(host);
+ hba = shost_priv(scp->device->host);
- tag = cmd->request->tag;
+ tag = scp->request->tag;
if (hba->ufshcd_state != UFSHCD_STATE_OPERATIONAL) {
err = SCSI_MLQUEUE_HOST_BUSY;
@@ -683,11 +683,11 @@ static int ufshcd_queuecommand(struct Scsi_Host *host, struct scsi_cmnd *cmd)
lrbp = &hba->lrb[tag];
- lrbp->cmd = cmd;
+ lrbp->cmd = scp;
lrbp->sense_bufflen = SCSI_SENSE_BUFFERSIZE;
- lrbp->sense_buffer = cmd->sense_buffer;
+ lrbp->sense_buffer = scp->sense_buffer;
lrbp->task_tag = tag;
- lrbp->lun = cmd->device->lun;
+ lrbp->lun = scp->device->lun;
lrbp->command_type = UTP_CMD_TYPE_SCSI;
@@ -698,13 +698,13 @@ static int ufshcd_queuecommand(struct Scsi_Host *host, struct scsi_cmnd *cmd)
goto out;
/* issue command to the controller */
- spin_lock_irqsave(hba->host->host_lock, flags);
ufshcd_send_command(hba, tag);
- spin_unlock_irqrestore(hba->host->host_lock, flags);
out:
return err;
}
+static DEF_SCSI_QCMD(ufshcd_queuecommand);
+
/**
* ufshcd_memory_alloc - allocate memory for host memory space data structures
* @hba: per adapter instance
--
1.7.5.4
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH 2/2] [SCSI] ufs: add SCSI host lock push-down for ufs.
2012-05-01 14:31 [PATCH 2/2] [SCSI] ufs: add SCSI host lock push-down for ufs Namjae Jeon
@ 2012-05-01 20:59 ` James Bottomley
2012-05-01 23:39 ` Namjae Jeon
0 siblings, 1 reply; 3+ messages in thread
From: James Bottomley @ 2012-05-01 20:59 UTC (permalink / raw)
To: Namjae Jeon; +Cc: akpm, linux-scsi
On Tue, 2012-05-01 at 10:31 -0400, Namjae Jeon wrote:
> SCSI host lock push-down patch is not applied in ufs host driver yet.
> After adding it, I tried to remove unneeded spin lock in queuecommand also.
This really isn't the correct thing: you're increasing the lock coverage
not decreasing it.
In the old way, ->queuecommand() was called locked. DEF_SCSI_QCMD() was
a macro we use to avoid converting all the previous methods from locked
to unlocked (it simply acts as a bridge between the new way and the old
way).
In the new way we call ->queuecommand() unlocked and try only to take
the lock when it's needed, if at all. The current ufshcd_queuecommand()
is correct because it only takes the lock when it needs it in
ufshcd_send_command(). A lock push down into that routine might be
possible, but since it's only a couple of bytes, probably not worth it.
James
> Signed-off-by: Namjae Jeon <linkinjeon@gmail.com>
> ---
> drivers/scsi/ufs/ufshcd.c | 20 ++++++++++----------
> 1 files changed, 10 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> index 3cb7a08..a134738 100644
> --- a/drivers/scsi/ufs/ufshcd.c
> +++ b/drivers/scsi/ufs/ufshcd.c
> @@ -658,23 +658,23 @@ static void ufshcd_compose_upiu(struct ufshcd_lrb *lrbp)
> }
>
> /**
> - * ufshcd_queuecommand - main entry point for SCSI requests
> + * ufshcd_queuecommand_lck - main entry point for SCSI requests
> * @cmd: command from SCSI Midlayer
> * @done: call back function
> *
> * Returns 0 for success, non-zero in case of failure
> */
> -static int ufshcd_queuecommand(struct Scsi_Host *host, struct scsi_cmnd *cmd)
> +static int
> +ufshcd_queuecommand_lck(struct scsi_cmnd *scp, void (*done)(struct scsi_cmnd *))
> {
> struct ufshcd_lrb *lrbp;
> struct ufs_hba *hba;
> - unsigned long flags;
> int tag;
> int err = 0;
>
> - hba = shost_priv(host);
> + hba = shost_priv(scp->device->host);
>
> - tag = cmd->request->tag;
> + tag = scp->request->tag;
>
> if (hba->ufshcd_state != UFSHCD_STATE_OPERATIONAL) {
> err = SCSI_MLQUEUE_HOST_BUSY;
> @@ -683,11 +683,11 @@ static int ufshcd_queuecommand(struct Scsi_Host *host, struct scsi_cmnd *cmd)
>
> lrbp = &hba->lrb[tag];
>
> - lrbp->cmd = cmd;
> + lrbp->cmd = scp;
> lrbp->sense_bufflen = SCSI_SENSE_BUFFERSIZE;
> - lrbp->sense_buffer = cmd->sense_buffer;
> + lrbp->sense_buffer = scp->sense_buffer;
> lrbp->task_tag = tag;
> - lrbp->lun = cmd->device->lun;
> + lrbp->lun = scp->device->lun;
>
> lrbp->command_type = UTP_CMD_TYPE_SCSI;
>
> @@ -698,13 +698,13 @@ static int ufshcd_queuecommand(struct Scsi_Host *host, struct scsi_cmnd *cmd)
> goto out;
>
> /* issue command to the controller */
> - spin_lock_irqsave(hba->host->host_lock, flags);
> ufshcd_send_command(hba, tag);
> - spin_unlock_irqrestore(hba->host->host_lock, flags);
> out:
> return err;
> }
>
> +static DEF_SCSI_QCMD(ufshcd_queuecommand);
> +
> /**
> * ufshcd_memory_alloc - allocate memory for host memory space data structures
> * @hba: per adapter instance
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH 2/2] [SCSI] ufs: add SCSI host lock push-down for ufs.
2012-05-01 20:59 ` James Bottomley
@ 2012-05-01 23:39 ` Namjae Jeon
0 siblings, 0 replies; 3+ messages in thread
From: Namjae Jeon @ 2012-05-01 23:39 UTC (permalink / raw)
To: James Bottomley; +Cc: akpm, linux-scsi
2012/5/2, James Bottomley <James.Bottomley@hansenpartnership.com>:
> On Tue, 2012-05-01 at 10:31 -0400, Namjae Jeon wrote:
>> SCSI host lock push-down patch is not applied in ufs host driver yet.
>> After adding it, I tried to remove unneeded spin lock in queuecommand
>> also.
>
> This really isn't the correct thing: you're increasing the lock coverage
> not decreasing it.
>
> In the old way, ->queuecommand() was called locked. DEF_SCSI_QCMD() was
> a macro we use to avoid converting all the previous methods from locked
> to unlocked (it simply acts as a bridge between the new way and the old
> way).
>
> In the new way we call ->queuecommand() unlocked and try only to take
> the lock when it's needed, if at all. The current ufshcd_queuecommand()
> is correct because it only takes the lock when it needs it in
> ufshcd_send_command(). A lock push down into that routine might be
> possible, but since it's only a couple of bytes, probably not worth it.
>
> James
Hi James.
I agree your comment.
Thanks for your explanation.
>
>
>> Signed-off-by: Namjae Jeon <linkinjeon@gmail.com>
>> ---
>> drivers/scsi/ufs/ufshcd.c | 20 ++++++++++----------
>> 1 files changed, 10 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
>> index 3cb7a08..a134738 100644
>> --- a/drivers/scsi/ufs/ufshcd.c
>> +++ b/drivers/scsi/ufs/ufshcd.c
>> @@ -658,23 +658,23 @@ static void ufshcd_compose_upiu(struct ufshcd_lrb
>> *lrbp)
>> }
>>
>> /**
>> - * ufshcd_queuecommand - main entry point for SCSI requests
>> + * ufshcd_queuecommand_lck - main entry point for SCSI requests
>> * @cmd: command from SCSI Midlayer
>> * @done: call back function
>> *
>> * Returns 0 for success, non-zero in case of failure
>> */
>> -static int ufshcd_queuecommand(struct Scsi_Host *host, struct scsi_cmnd
>> *cmd)
>> +static int
>> +ufshcd_queuecommand_lck(struct scsi_cmnd *scp, void (*done)(struct
>> scsi_cmnd *))
>> {
>> struct ufshcd_lrb *lrbp;
>> struct ufs_hba *hba;
>> - unsigned long flags;
>> int tag;
>> int err = 0;
>>
>> - hba = shost_priv(host);
>> + hba = shost_priv(scp->device->host);
>>
>> - tag = cmd->request->tag;
>> + tag = scp->request->tag;
>>
>> if (hba->ufshcd_state != UFSHCD_STATE_OPERATIONAL) {
>> err = SCSI_MLQUEUE_HOST_BUSY;
>> @@ -683,11 +683,11 @@ static int ufshcd_queuecommand(struct Scsi_Host
>> *host, struct scsi_cmnd *cmd)
>>
>> lrbp = &hba->lrb[tag];
>>
>> - lrbp->cmd = cmd;
>> + lrbp->cmd = scp;
>> lrbp->sense_bufflen = SCSI_SENSE_BUFFERSIZE;
>> - lrbp->sense_buffer = cmd->sense_buffer;
>> + lrbp->sense_buffer = scp->sense_buffer;
>> lrbp->task_tag = tag;
>> - lrbp->lun = cmd->device->lun;
>> + lrbp->lun = scp->device->lun;
>>
>> lrbp->command_type = UTP_CMD_TYPE_SCSI;
>>
>> @@ -698,13 +698,13 @@ static int ufshcd_queuecommand(struct Scsi_Host
>> *host, struct scsi_cmnd *cmd)
>> goto out;
>>
>> /* issue command to the controller */
>> - spin_lock_irqsave(hba->host->host_lock, flags);
>> ufshcd_send_command(hba, tag);
>> - spin_unlock_irqrestore(hba->host->host_lock, flags);
>> out:
>> return err;
>> }
>>
>> +static DEF_SCSI_QCMD(ufshcd_queuecommand);
>> +
>> /**
>> * ufshcd_memory_alloc - allocate memory for host memory space data
>> structures
>> * @hba: per adapter instance
>
>
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2012-05-01 23:39 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-05-01 14:31 [PATCH 2/2] [SCSI] ufs: add SCSI host lock push-down for ufs Namjae Jeon
2012-05-01 20:59 ` James Bottomley
2012-05-01 23:39 ` Namjae Jeon
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.