linux-scsi.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Bart Van Assche <bvanassche@acm.org>
To: Can Guo <cang@codeaurora.org>
Cc: "Martin K . Petersen" <martin.petersen@oracle.com>,
	"James E . J . Bottomley" <jejb@linux.vnet.ibm.com>,
	linux-scsi@vger.kernel.org, Bean Huo <beanhuo@micron.com>,
	Avri Altman <avri.altman@wdc.com>,
	Stanley Chu <stanley.chu@mediatek.com>,
	Tomas Winkler <tomas.winkler@intel.com>
Subject: Re: [PATCH 4/6] ufs: Fix a race condition in the tracing code
Date: Thu, 26 Dec 2019 09:35:16 -0800	[thread overview]
Message-ID: <27fdf41d-9ec1-380b-e481-7d76a2906c3d@acm.org> (raw)
In-Reply-To: <39b63e90a34a294957c89bd30d0e4912@codeaurora.org>

On 12/25/19 2:59 AM, Can Guo wrote:
> On 2019-12-25 06:02, Bart Van Assche wrote:
>> Starting execution of a command before tracing a command may cause the
>> completion handler to free data while it is being traced. Fix this race
>> by tracing a command before it is submitted.
>>
>> Cc: Bean Huo <beanhuo@micron.com>
>> Cc: Can Guo <cang@codeaurora.org>
>> Cc: Avri Altman <avri.altman@wdc.com>
>> Cc: Stanley Chu <stanley.chu@mediatek.com>
>> Cc: Tomas Winkler <tomas.winkler@intel.com>
>> Signed-off-by: Bart Van Assche <bvanassche@acm.org>
>> ---
>>  drivers/scsi/ufs/ufshcd.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
>> index 80b028ba39e9..4d9bb1932b39 100644
>> --- a/drivers/scsi/ufs/ufshcd.c
>> +++ b/drivers/scsi/ufs/ufshcd.c
>> @@ -1875,12 +1875,12 @@ void ufshcd_send_command(struct ufs_hba *hba,
>> unsigned int task_tag)
>>  {
>>      hba->lrb[task_tag].issue_time_stamp = ktime_get();
>>      hba->lrb[task_tag].compl_time_stamp = ktime_set(0, 0);
>> +    ufshcd_add_command_trace(hba, task_tag, "send");
> 
> How about moving it after __set_bit(task_tag, &hba->outstanding_reqs);?
> 
> Thanks,
> 
> Can Guo.
> 
>>      ufshcd_clk_scaling_start_busy(hba);
>>      __set_bit(task_tag, &hba->outstanding_reqs);
>>      ufshcd_writel(hba, 1 << task_tag, REG_UTP_TRANSFER_REQ_DOOR_BELL);
>>      /* Make sure that doorbell is committed immediately */
>>      wmb();
>> -    ufshcd_add_command_trace(hba, task_tag, "send");
>>  }

Hi Can,

As far as I can see ufshcd_add_command_trace() does not read 
hba->outstanding_reqs so calling ufshcd_add_command_trace() before 
changing outstanding_reqs should be fine.

The order chosen in the posted patch should minimize energy usage of the 
UFS controller, namely by calling the tracing function before starting 
clock scaling.

Thanks,

Bart.

  reply	other threads:[~2019-12-26 17:35 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-12-24 22:02 [PATCH 0/6] Six UFS patches Bart Van Assche
2019-12-24 22:02 ` [PATCH 1/6] ufs: Fix indentation in ufshcd_query_attr_retry() Bart Van Assche
2019-12-25  7:16   ` Stanley Chu
2019-12-25  8:17   ` Can Guo
2019-12-27  1:13   ` Alim Akhtar
2019-12-24 22:02 ` [PATCH 2/6] ufs: Make ufshcd_add_command_trace() easier to read Bart Van Assche
2019-12-25  7:16   ` Stanley Chu
2019-12-25  8:17   ` Can Guo
2019-12-27  1:17   ` Alim Akhtar
2019-12-24 22:02 ` [PATCH 3/6] ufs: Make ufshcd_prepare_utp_scsi_cmd_upiu() " Bart Van Assche
2019-12-25  7:17   ` Stanley Chu
2019-12-25  8:18   ` Can Guo
2019-12-24 22:02 ` [PATCH 4/6] ufs: Fix a race condition in the tracing code Bart Van Assche
2019-12-25 10:59   ` Can Guo
2019-12-26 17:35     ` Bart Van Assche [this message]
2019-12-27  5:50       ` Can Guo
2019-12-27  1:21   ` Alim Akhtar
2019-12-24 22:02 ` [PATCH 5/6] ufs: Remove superfluous memory barriers Bart Van Assche
2019-12-25 10:31   ` Can Guo
2019-12-26 17:36     ` Bart Van Assche
2019-12-24 22:02 ` [PATCH 6/6] ufs: Remove the SCSI timeout handler Bart Van Assche
2019-12-25 11:02   ` Can Guo
2019-12-27  1:24   ` Alim Akhtar
2020-05-28  9:47   ` Can Guo
2020-05-28 16:12     ` Bart Van Assche
2020-05-29  1:39       ` Can Guo
2020-05-29  3:56         ` Bart Van Assche
2020-01-03  2:58 ` [PATCH 0/6] Six UFS patches Martin K. Petersen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=27fdf41d-9ec1-380b-e481-7d76a2906c3d@acm.org \
    --to=bvanassche@acm.org \
    --cc=avri.altman@wdc.com \
    --cc=beanhuo@micron.com \
    --cc=cang@codeaurora.org \
    --cc=jejb@linux.vnet.ibm.com \
    --cc=linux-scsi@vger.kernel.org \
    --cc=martin.petersen@oracle.com \
    --cc=stanley.chu@mediatek.com \
    --cc=tomas.winkler@intel.com \
    --subject='Re: [PATCH 4/6] ufs: Fix a race condition in the tracing code' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
on how to clone and mirror all data and code used for this inbox