linux-kernel - Re: [PATCH v5 3/4] scsi: ufs: core: Introduce a new clock

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: <20250121124217.ajerfz3p7iorc2oh@thinkpad>
Date: Tue, 21 Jan 2025 18:12:17 +0530
From: Manivannan Sadhasivam <manisadhasivam.linux@...il.com>
To: Avri Altman <avri.altman@....com>
Cc: "Martin K . Petersen" <martin.petersen@...cle.com>,
	linux-scsi@...r.kernel.org, linux-kernel@...r.kernel.org,
	Bart Van Assche <bvanassche@....org>, Bean Huo <beanhuo@...ron.com>
Subject: Re: [PATCH v5 3/4] scsi: ufs: core: Introduce a new clock_gating lock

+ Dmitry

On Sun, Nov 24, 2024 at 09:08:07AM +0200, Avri Altman wrote:

[...]

> @@ -9162,7 +9159,6 @@ static int ufshcd_setup_clocks(struct ufs_hba *hba, bool on)
>  	int ret = 0;
>  	struct ufs_clk_info *clki;
>  	struct list_head *head = &hba->clk_list_head;
> -	unsigned long flags;
>  	ktime_t start = ktime_get();
>  	bool clk_state_changed = false;
>  
> @@ -9213,11 +9209,10 @@ static int ufshcd_setup_clocks(struct ufs_hba *hba, bool on)
>  				clk_disable_unprepare(clki->clk);
>  		}
>  	} else if (!ret && on) {
> -		spin_lock_irqsave(hba->host->host_lock, flags);
> -		hba->clk_gating.state = CLKS_ON;
> +		scoped_guard(spinlock_irqsave, &hba->clk_gating.lock)

This triggers the following lockdep warning on Qualcomm boards as reported by
Dmitry offline:

[    4.388838] INFO: trying to register non-static key.
[    4.395673] The code is fine but needs lockdep annotation, or maybe
[    4.402118] you didn't initialize this object before use?
[    4.407673] turning off the locking correctness validator.
[    4.413334] CPU: 5 UID: 0 PID: 58 Comm: kworker/u32:1 Not tainted 6.12-rc1 #185
[    4.413343] Hardware name: Qualcomm Technologies, Inc. Robotics RB5 (DT)
[    4.413362] Call trace:
[    4.413364]  show_stack+0x18/0x24 (C)
[    4.413374]  dump_stack_lvl+0x90/0xd0
[    4.413384]  dump_stack+0x18/0x24
[    4.413392]  register_lock_class+0x498/0x4a8
[    4.413400]  __lock_acquire+0xb4/0x1b90
[    4.413406]  lock_acquire+0x114/0x310
[    4.413413]  _raw_spin_lock_irqsave+0x60/0x88
[    4.413423]  ufshcd_setup_clocks+0x2c0/0x490
[    4.413433]  ufshcd_init+0x198/0x10ec
[    4.413437]  ufshcd_pltfrm_init+0x600/0x7c0
[    4.413444]  ufs_qcom_probe+0x20/0x58
[    4.413449]  platform_probe+0x68/0xd8
[    4.413459]  really_probe+0xbc/0x268
[    4.413466]  __driver_probe_device+0x78/0x12c
[    4.413473]  driver_probe_device+0x40/0x11c
[    4.413481]  __device_attach_driver+0xb8/0xf8
[    4.413489]  bus_for_each_drv+0x84/0xe4
[    4.413495]  __device_attach+0xfc/0x18c
[    4.413502]  device_initial_probe+0x14/0x20
[    4.413510]  bus_probe_device+0xb0/0xb4
[    4.413517]  deferred_probe_work_func+0x8c/0xc8
[    4.413524]  process_scheduled_works+0x250/0x658
[    4.413534]  worker_thread+0x15c/0x2c8
[    4.413542]  kthread+0x134/0x200
[    4.413550]  ret_from_fork+0x10/0x20

As lockdep found, clk_gating.lock is uninitialized when ufshcd_setup_clocks() is
called for the first time. I looked into fixing it for a moment, but the overall
locking for 'clk_gating.state' looks fragile i.e., there are instances where the
code is not locked at all. So I'm just reporting to you here hoping that you'd
have some idea to fix it.

While submitting the fix, please add the following reported by tag:

Reported-by: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>

- Mani

-- 
மணிவண்ணன் சதாசிவம்