linux-kernel - RE: [PATCH V3 3/4] mmc: core: Capture eMMC and SD card errors

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <SJ0PR02MB84491999BFE61B2E8BFE0212CD5F9@SJ0PR02MB8449.namprd02.prod.outlook.com>
Date:   Tue, 25 Jan 2022 18:40:30 +0000
From:   "Sajida Bhanu (Temp) (QUIC)" <quic_c_sbhanu@...cinc.com>
To:     Adrian Hunter <adrian.hunter@...el.com>,
        "Sajida Bhanu (Temp) (QUIC)" <quic_c_sbhanu@...cinc.com>,
        "Asutosh Das (QUIC)" <quic_asutoshd@...cinc.com>,
        "ulf.hansson@...aro.org" <ulf.hansson@...aro.org>,
        "agross@...nel.org" <agross@...nel.org>,
        "bjorn.andersson@...aro.org" <bjorn.andersson@...aro.org>,
        "linux-mmc@...r.kernel.org" <linux-mmc@...r.kernel.org>,
        "linux-arm-msm@...r.kernel.org" <linux-arm-msm@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
CC:     "stummala@...eaurora.org" <stummala@...eaurora.org>,
        "vbadigan@...eaurora.org" <vbadigan@...eaurora.org>,
        "Ram Prakash Gupta (QUIC)" <quic_rampraka@...cinc.com>,
        "Pradeep Pragallapati (QUIC)" <quic_pragalla@...cinc.com>,
        "sartgarg@...eaurora.org" <sartgarg@...eaurora.org>,
        "nitirawa@...eaurora.org" <nitirawa@...eaurora.org>,
        "sayalil@...eaurora.org" <sayalil@...eaurora.org>,
        Liangliang Lu <luliang@...eaurora.org>,
        "Bao D . Nguyen" <nguyenb@...eaurora.org>
Subject: RE: [PATCH V3 3/4] mmc: core: Capture eMMC and SD card errors

Hi,

Thanks for the review.

Please find the inline comments.

Thanks,
Sajida

-----Original Message-----
From: Adrian Hunter <adrian.hunter@...el.com> 
Sent: Friday, January 21, 2022 1:50 PM
To: Sajida Bhanu (Temp) (QUIC) <quic_c_sbhanu@...cinc.com>; Asutosh Das (QUIC) <quic_asutoshd@...cinc.com>; ulf.hansson@...aro.org; agross@...nel.org; bjorn.andersson@...aro.org; linux-mmc@...r.kernel.org; linux-arm-msm@...r.kernel.org; linux-kernel@...r.kernel.org
Cc: stummala@...eaurora.org; vbadigan@...eaurora.org; Ram Prakash Gupta (QUIC) <quic_rampraka@...cinc.com>; Pradeep Pragallapati (QUIC) <quic_pragalla@...cinc.com>; sartgarg@...eaurora.org; nitirawa@...eaurora.org; sayalil@...eaurora.org; Liangliang Lu <luliang@...eaurora.org>; Bao D . Nguyen <nguyenb@...eaurora.org>
Subject: Re: [PATCH V3 3/4] mmc: core: Capture eMMC and SD card errors

On 20/01/2022 19:26, Shaik Sajida Bhanu wrote:
> Add changes to capture eMMC and SD card errors.
> This is useful for debug and testing.
> 
> Signed-off-by: Shaik Sajida Bhanu <quic_c_sbhanu@...cinc.com>
> Signed-off-by: Liangliang Lu <luliang@...eaurora.org>
> Signed-off-by: Sayali Lokhande <sayalil@...eaurora.org>
> Signed-off-by: Bao D. Nguyen <nguyenb@...eaurora.org>
> Signed-off-by: Ram Prakash Gupta <quic_rampraka@...cinc.com>
> ---
>  drivers/mmc/core/core.c  | 8 ++++++++  drivers/mmc/core/queue.c | 3 
> +++
>  2 files changed, 11 insertions(+)
> 
> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c index 
> 368f104..c586d69 100644
> --- a/drivers/mmc/core/core.c
> +++ b/drivers/mmc/core/core.c
> @@ -2242,6 +2242,14 @@ void mmc_rescan(struct work_struct *work)
>  		if (freqs[i] <= host->f_min)
>  			break;
>  	}
> +
> +	/*
> +	 * Ignore the command timeout errors observed during
> +	 * the card init as those are excepted.
> +	 */
> +

Please remove blank line here.

>>>>Sure.

> +	if (host && host->err_stats_enabled)

The condition is not needed.

>>>>Sure .

> +		host->err_stats[MMC_ERR_CMD_TIMEOUT] = 0;

Please put this after successful call to mmc_rescan_try_freq

>>>>>Sure.

>  	mmc_release_host(host);
>  
>   out:
> diff --git a/drivers/mmc/core/queue.c b/drivers/mmc/core/queue.c index 
> c69b2d9..7dc9dfb 100644
> --- a/drivers/mmc/core/queue.c
> +++ b/drivers/mmc/core/queue.c
> @@ -100,6 +100,9 @@ static enum blk_eh_timer_return mmc_cqe_timed_out(struct request *req)
>  	enum mmc_issue_type issue_type = mmc_issue_type(mq, req);
>  	bool recovery_needed = false;
>  
> +	if (host->err_stats_enabled)
> +		mmc_debugfs_err_stats_inc(host, MMC_ERR_CMDQ_REQ_TIMEOUT);

Doesn't this get covered by the drivers.  It seems like this should not be needed.

>>>>>>Okay

> +
>  	switch (issue_type) {
>  	case MMC_ISSUE_ASYNC:
>  	case MMC_ISSUE_DCMD:
>