linux-kernel - Re: [PATCH v2 2/2] scsi: ufs: core: move some irq handling back to hardirq (with time limit)

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <c57a2ca7-b0c9-4e52-9d9d-5c06c7f56f1a@acm.org>
Date: Fri, 25 Jul 2025 14:08:37 -0700
From: Bart Van Assche <bvanassche@....org>
To: André Draszik <andre.draszik@...aro.org>,
 Alim Akhtar <alim.akhtar@...sung.com>, Avri Altman <avri.altman@....com>,
 "James E.J. Bottomley" <James.Bottomley@...senPartnership.com>,
 "Martin K. Petersen" <martin.petersen@...cle.com>,
 Neil Armstrong <neil.armstrong@...aro.org>
Cc: Peter Griffin <peter.griffin@...aro.org>,
 Tudor Ambarus <tudor.ambarus@...aro.org>,
 Will McVicker <willmcvicker@...gle.com>,
 Manivannan Sadhasivam <mani@...nel.org>, kernel-team@...roid.com,
 linux-arm-msm@...r.kernel.org, linux-samsung-soc@...r.kernel.org,
 linux-scsi@...r.kernel.org, linux-kernel@...r.kernel.org,
 stable@...r.kernel.org
Subject: Re: [PATCH v2 2/2] scsi: ufs: core: move some irq handling back to
 hardirq (with time limit)

On 7/25/25 7:16 AM, André Draszik wrote:
> -	for_each_set_bit(tag, &completed_reqs, hba->nutrs)
> +	for_each_set_bit(tag, &completed_reqs, hba->nutrs) {
>   		ufshcd_compl_one_cqe(hba, tag, NULL);
> +		__clear_bit(tag, &completed_reqs);
> +		if (time_limit && time_after_eq(jiffies, time_limit))
> +			break;
> +	}

Has it been considered to use time_is_before_eq_jiffies(time_limit)
instead of open-coding it?

> @@ -5636,15 +5670,34 @@ static int ufshcd_poll(struct Scsi_Host *shost, unsigned int queue_num)
>   	WARN_ONCE(completed_reqs & ~hba->outstanding_reqs,
>   		  "completed: %#lx; outstanding: %#lx\n", completed_reqs,
>   		  hba->outstanding_reqs);
> -	hba->outstanding_reqs &= ~completed_reqs;
> +
> +	if (completed_reqs) {
> +		pending = __ufshcd_transfer_req_compl(hba, completed_reqs,
> +						      time_limit);
> +		completed_reqs &= ~pending;
> +		hba->outstanding_reqs &= ~completed_reqs;
> +	}
> +
>   	spin_unlock_irqrestore(&hba->outstanding_lock, flags);
>   
> -	if (completed_reqs)
> -		__ufshcd_transfer_req_compl(hba, completed_reqs);

This change moves the __ufshcd_transfer_req_compl() call from outside to
inside the critical section. I expect this to impact performance
negatively because it makes it significantly more likely that the
command submission code will have to wait while the completion code is
holding hba->outstanding_lock. Can this be avoided, e.g. by limiting the
number of commands that are completed instead of the time spent in
interrupt context? usecs_to_jiffies(HARDIRQ_TIMELIMIT) will round up
the time limit anyway from 20 microseconds to 1/HZ (one millisecond?).

Thanks,

Bart.