mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Bart Van Assche <bvanassche@acm.org>
To: palash.kambar@oss.qualcomm.com, mani@kernel.org,
	alim.akhtar@samsung.com, avri.altman@wdc.com,
	peter.griffin@linaro.org, krzk@kernel.org,
	peter.wang@mediatek.com, beanhuo@micron.com,
	quic_nguyenb@quicinc.com, adrian.hunter@intel.com,
	ebiggers@kernel.org, neil.armstrong@linaro.org,
	James.Bottomley@HansenPartnership.com,
	martin.petersen@oracle.com
Cc: linux-arm-msm@vger.kernel.org, linux-scsi@vger.kernel.org,
	linux-kernel@vger.kernel.org, quic_cang@quicinc.com,
	quic_nitirawa@quicinc.com
Subject: Re: [PATCH V1 2/2] ufs: ufs-qcom: Disable AHIT before SQ tail update to prevent race in MCQ mode
Date: Tue, 14 Oct 2025 09:18:50 -0700	[thread overview]
Message-ID: <f2b56041-b418-4ca9-a84a-ac662a850207@acm.org> (raw)
In-Reply-To: <20251014060406.1420475-3-palash.kambar@oss.qualcomm.com>

On 10/13/25 11:04 PM, palash.kambar@oss.qualcomm.com wrote:
> +static void ufs_qcom_send_command(struct ufs_hba *hba,
> +				  struct ufshcd_lrb *lrbp)
> +{
> +	struct ufs_qcom_host *host = ufshcd_get_variant(hba);
> +	unsigned long flags;
> +
> +	if ((host->hw_ver.major == 6 && host->hw_ver.minor == 0 &&
> +	     host->hw_ver.step == 0) && hba->mcq_enabled) {
> +		spin_lock_irqsave(hba->host->host_lock, flags);
> +		if ((++host->active_cmds) == 1)
> +			/* Stop the auto-hiberate idle timer */
> +			ufshcd_writel(hba, 0, REG_AUTO_HIBERNATE_IDLE_TIMER);
> +		spin_unlock_irqrestore(hba->host->host_lock, flags);
> +	}
> +}

Do you realize that obtaining the host lock from the command submission
path introduces a contention point and hence cancels one of the benefits
of MCQ (different CPU cores can submit commands independently)? This
argument still applies if host->active_cmds would be changed into an
atomic counter because making that change would still trigger cache line
ping-pong if UFS commands are submitted from multiple CPU cores
simultaneously. I think this is a strong argument against this patch
series.

An example of how to implement this kind of counter without killing 
performance is available in the block layer (&q->q_usage_counter).
However, implementing a per-cpu counter and checking in an efficient
way whether it has reached zero is much more complicated than the
approach of this patch series.

Is AHIT really that valuable that you want to sacrifice MCQ performance
to enable AHIT? Why not to enable/disable AHIT from the RPM callbacks?

Bart.

  reply	other threads:[~2025-10-14 16:19 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-10-14  6:04 [PATCH V1 0/2] Address race condition in MCQ mode and enhance palash.kambar
2025-10-14  6:04 ` [PATCH V1 1/2] ufs: core:Add vendor-specific callbacks and update setup_xfer_req interface palash.kambar
2025-10-14 16:10   ` Bart Van Assche
2025-10-18  6:49     ` Palash Kambar
2025-10-14  6:04 ` [PATCH V1 2/2] ufs: ufs-qcom: Disable AHIT before SQ tail update to prevent race in MCQ mode palash.kambar
2025-10-14 16:18   ` Bart Van Assche [this message]
     [not found]     ` <CAGbPq5dhUXr59U_J3W4haNHughkaiXpnc4kAZWXB0SjPdFQMhg@mail.gmail.com>
2025-10-15 15:45       ` Bart Van Assche
2025-10-18  6:44         ` Palash Kambar
2025-10-20  6:53           ` Peter Wang (王信友)

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=f2b56041-b418-4ca9-a84a-ac662a850207@acm.org \
    --to=bvanassche@acm.org \
    --cc=James.Bottomley@HansenPartnership.com \
    --cc=adrian.hunter@intel.com \
    --cc=alim.akhtar@samsung.com \
    --cc=avri.altman@wdc.com \
    --cc=beanhuo@micron.com \
    --cc=ebiggers@kernel.org \
    --cc=krzk@kernel.org \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-scsi@vger.kernel.org \
    --cc=mani@kernel.org \
    --cc=martin.petersen@oracle.com \
    --cc=neil.armstrong@linaro.org \
    --cc=palash.kambar@oss.qualcomm.com \
    --cc=peter.griffin@linaro.org \
    --cc=peter.wang@mediatek.com \
    --cc=quic_cang@quicinc.com \
    --cc=quic_nguyenb@quicinc.com \
    --cc=quic_nitirawa@quicinc.com \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®