linux-kernel - RE: [PATCH 2/2] scsi: ufs: core: Add a trace function calling when uic command error occurs

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: <0ed401dbb01d$2f487720$8dd96560$@samsung.com>
Date: Fri, 18 Apr 2025 13:48:52 +0900
From: "DooHyun Hwang" <dh0421.hwang@...sung.com>
To: "'Bart Van Assche'" <bvanassche@....org>, <linux-scsi@...r.kernel.org>,
	<linux-kernel@...r.kernel.org>, <alim.akhtar@...sung.com>,
	<avri.altman@....com>, <James.Bottomley@...senPartnership.com>,
	<martin.petersen@...cle.com>, <peter.wang@...iatek.com>,
	<manivannan.sadhasivam@...aro.org>, <quic_mnaresh@...cinc.com>
Cc: <grant.jung@...sung.com>, <jt77.jang@...sung.com>,
	<junwoo80.lee@...sung.com>, <jangsub.yi@...sung.com>,
	<sh043.lee@...sung.com>, <cw9316.lee@...sung.com>,
	<sh8267.baek@...sung.com>, <wkon.kim@...sung.com>
Subject: RE: [PATCH 2/2] scsi: ufs: core: Add a trace function calling when
 uic command error occurs

> On 4/16/25 7:34 PM, DooHyun Hwang wrote:
> > When a uic command error occurs, there is no trace function calling.
> > Therefore, trace function calls are added when a uic command error
> happens.
> >
> > Signed-off-by: DooHyun Hwang <dh0421.hwang@...sung.com>
> > ---
> >   drivers/ufs/core/ufshcd.c | 5 +++++
> >   1 file changed, 5 insertions(+)
> >
> > diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
> > index ab98acd982b5..baac1ae94efc 100644
> > --- a/drivers/ufs/core/ufshcd.c
> > +++ b/drivers/ufs/core/ufshcd.c
> > @@ -2534,6 +2534,9 @@ ufshcd_wait_for_uic_cmd(struct ufs_hba *hba,
> struct uic_command *uic_cmd)
> >   	hba->active_uic_cmd = NULL;
> >   	spin_unlock_irqrestore(hba->host->host_lock, flags);
> >
> > +	if (ret)
> > +		ufshcd_add_uic_command_trace(hba, uic_cmd, UFS_CMD_ERR);
> > +
> >   	return ret;
> >   }
> >
> > @@ -4306,6 +4309,8 @@ static int ufshcd_uic_pwr_ctrl(struct ufs_hba *hba,
> struct uic_command *cmd)
> >   	}
> >   out:
> >   	if (ret) {
> > +		ufshcd_add_uic_command_trace(hba, hba->active_uic_cmd,
> > +					     UFS_CMD_ERR);
> >   		ufshcd_print_host_state(hba);
> >   		ufshcd_print_pwr_info(hba);
> >   		ufshcd_print_evt_hist(hba);
> 
> Shouldn't the value of 'ret' be included in the UFS_CMD_ERR trace output?
> 
> Thanks,
> 
> Bart.
Thank you for your insightful feedback.

Upon further consideration, it seems that the existing trace, which captures detailed information through the UICCMDARG1 to UICCMDARG3 fields.
That already provides sufficient context for analyzing command-related issues.
The UFS_CMD_ERR trace output, combined with UICCMDARG* values, should be adequate for diagnosing errors and understanding the command's execution status.

BR,
Thank you.
DooHyun Hwang.