The RTC update work involves runtime resuming the UFS controller. Hence, only start the RTC update work after runtime power management in the UFS driver has been fully initialized. This patch fixes the following kernel crash:
Internal error: Oops: 0000000096000006 [#1] PREEMPT SMP Workqueue: events ufshcd_rtc_work Call trace: _raw_spin_lock_irqsave+0x34/0x8c (P) pm_runtime_get_if_active+0x24/0x9c (L) pm_runtime_get_if_active+0x24/0x9c ufshcd_rtc_work+0x138/0x1b4 process_one_work+0x148/0x288 worker_thread+0x2cc/0x3d4 kthread+0x110/0x114 ret_from_fork+0x10/0x20
Reported-by: Neil Armstrong neil.armstrong@linaro.org Closes: https://lore.kernel.org/linux-scsi/0c0bc528-fdc2-4106-bc99-f23ae377f6f5@lina... Fixes: 6bf999e0eb41 ("scsi: ufs: core: Add UFS RTC support") Cc: Bean Huo beanhuo@micron.com Cc: stable@vger.kernel.org Signed-off-by: Bart Van Assche bvanassche@acm.org --- drivers/ufs/core/ufshcd.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c index 585557eaa9a2..ed82ff329314 100644 --- a/drivers/ufs/core/ufshcd.c +++ b/drivers/ufs/core/ufshcd.c @@ -8633,6 +8633,14 @@ static int ufshcd_add_lus(struct ufs_hba *hba) ufshcd_init_clk_scaling_sysfs(hba); }
+ /* + * The RTC update code accesses the hba->ufs_device_wlun->sdev_gendev + * pointer and hence must only be started after the WLUN pointer has + * been initialized by ufshcd_scsi_add_wlus(). + */ + schedule_delayed_work(&hba->ufs_rtc_update_work, + msecs_to_jiffies(UFS_RTC_UPDATE_INTERVAL_MS)); + ufs_bsg_probe(hba); scsi_scan_host(hba->host);
@@ -8727,8 +8735,6 @@ static int ufshcd_post_device_init(struct ufs_hba *hba) ufshcd_force_reset_auto_bkops(hba);
ufshcd_set_timestamp_attr(hba); - schedule_delayed_work(&hba->ufs_rtc_update_work, - msecs_to_jiffies(UFS_RTC_UPDATE_INTERVAL_MS));
if (!hba->max_pwr_info.is_valid) return 0;
On Thu, 2024-10-31 at 14:26 -0700, Bart Van Assche wrote:
External email : Please do not click links or open attachments until you have verified the sender or the content.
The RTC update work involves runtime resuming the UFS controller. Hence, only start the RTC update work after runtime power management in the UFS driver has been fully initialized. This patch fixes the following kernel crash:
Internal error: Oops: 0000000096000006 [#1] PREEMPT SMP Workqueue: events ufshcd_rtc_work Call trace: _raw_spin_lock_irqsave+0x34/0x8c (P) pm_runtime_get_if_active+0x24/0x9c (L) pm_runtime_get_if_active+0x24/0x9c ufshcd_rtc_work+0x138/0x1b4 process_one_work+0x148/0x288 worker_thread+0x2cc/0x3d4 kthread+0x110/0x114 ret_from_fork+0x10/0x20
Reported-by: Neil Armstrong neil.armstrong@linaro.org Closes: https://lore.kernel.org/linux-scsi/0c0bc528-fdc2-4106-bc99-f23ae377f6f5@lina... Fixes: 6bf999e0eb41 ("scsi: ufs: core: Add UFS RTC support") Cc: Bean Huo beanhuo@micron.com Cc: stable@vger.kernel.org Signed-off-by: Bart Van Assche bvanassche@acm.org
Reviewed-by: Peter Wang peter.wang@mediatek.com
On Thu, Oct 31, 2024 at 02:26:24PM -0700, Bart Van Assche wrote:
The RTC update work involves runtime resuming the UFS controller. Hence, only start the RTC update work after runtime power management in the UFS driver has been fully initialized. This patch fixes the following kernel crash:
Internal error: Oops: 0000000096000006 [#1] PREEMPT SMP Workqueue: events ufshcd_rtc_work Call trace: _raw_spin_lock_irqsave+0x34/0x8c (P) pm_runtime_get_if_active+0x24/0x9c (L) pm_runtime_get_if_active+0x24/0x9c ufshcd_rtc_work+0x138/0x1b4 process_one_work+0x148/0x288 worker_thread+0x2cc/0x3d4 kthread+0x110/0x114 ret_from_fork+0x10/0x20
Reported-by: Neil Armstrong neil.armstrong@linaro.org Closes: https://lore.kernel.org/linux-scsi/0c0bc528-fdc2-4106-bc99-f23ae377f6f5@lina... Fixes: 6bf999e0eb41 ("scsi: ufs: core: Add UFS RTC support") Cc: Bean Huo beanhuo@micron.com Cc: stable@vger.kernel.org Signed-off-by: Bart Van Assche bvanassche@acm.org
Bart, Thanks for the fix! While looking into this patch, I also found the weirdness of the ufshcd_rpm_*() helpers in ufshcd-priv.h. Their naming doesn't seem to indicate whether those helpers are for WLUN or for HBA. Also, I don't see the benefit of these helpers since they just wrap generic pm_runtime* calls. Then there are other open coding instances in the ufshcd.c. Like
pm_runtime_suspended(&hba->ufs_device_wlun->sdev_gendev) pm_runtime_set_active(&hba->ufs_device_wlun->sdev_gendev)
Moreover, we do check for the presence of hba->ufs_device_wlun before calling ufshcd_rpm_get_sync() in ufshcd_remove(). This could be one other way to fix this null ptr dereference even though I wouldn't recommend doing so as calling rtc_work early is pointless.
So I think we should remove these helpers to avoid having these discrepancies. WDYT?
- Mani
linux-stable-mirror@lists.linaro.org