ksys_sync_helper

DISCUSSION Christoph Hellwig 4 participants 5 messages Jul 31, 2023 – Aug 7, 2023

Subsystems (3)

DRM drivers VFS Radeon and amdgpu DRM drivers

lore

Share & tools

lore

Christoph Hellwig

author

2023-07-31 07:25 UTC #0 lore raw

Guys,

why did b5dee3130bb4014 add a magic export for sync functionality that wrapps VFS code in a weird way, and then exports it (without
even adding a user in that commit)? This kind of functionality
needs to be exported from the VFS, and only with ACKs? With this and commit d5ea093eebf022e now we end up with a random driver (amdgpu) syncing all file systems for absolutely no good reason.

Wysocki, Rafael J

2023-07-31 18:27 UTC #1 lore raw

Quoted 6 lines · Christoph Hellwig

Guys,
why did b5dee3130bb4014 add a magic export for sync functionality that wrapps VFS code in a weird way, and then exports it (without
even adding a user in that commit)? This kind of functionality
needs to be exported from the VFS, and only with ACKs?

OK, I'll remember about this.

Quoted 3 lines

With this
and commit d5ea093eebf022e now we end up with a random driver (amdgpu) syncing all file systems for absolutely no good reason.

Sorry about that.

The problematic commit should still revert more or less cleanly, so please do that if that's what you need.

Christoph Hellwig

author

2023-08-01 11:12 UTC #2 lore raw

Quoted 4 lines · Rafael J

OK, I'll remember about this.

Quoted 8 lines

Quoted 3 lines
With this
and commit d5ea093eebf022e now we end up with a random driver (amdgpu) syncing all file systems for absolutely no good reason.
Sorry about that.
The problematic commit should still revert more or less cleanly, so please do that if that's what you need.

We'd still need to remove abuse in amdgpu first, though.

Matthew Wilcox

2023-08-01 12:34 UTC #3 lore raw

Quoted 9 lines · Christoph Hellwig

Quoted 8 lines · Rafael J
OK, I'll remember about this.
Quoted 3 lines
With this
and commit d5ea093eebf022e now we end up with a random driver (amdgpu) syncing all file systems for absolutely no good reason.

Quoted 6 lines

Quoted 4 lines
Sorry about that.
The problematic commit should still revert more or less cleanly, so please do that if that's what you need.
We'd still need to remove abuse in amdgpu first, though.

This would effectively revert d5ea093eebf0

drivers/gpu/drm/amd/amdgpu/amdgpu_device.c +0 ~~-12~~

index dc0e5227119b..af04fece37d5 100644
@@ -75,7 +75,6 @@
 #include "amdgpu_fru_eeprom.h"
 #include "amdgpu_reset.h"
 
-#include <linux/suspend.h>
 #include <drm/task_barrier.h>
 #include <linux/pm_runtime.h>

@@ -5225,17 +5224,6 @@ int amdgpu_device_gpu_recover(struct amdgpu_device *adev,
 	 */
 	need_emergency_restart = amdgpu_ras_need_emergency_restart(adev);
 
-	/*
-	 * Flush RAM to disk so that after reboot
-	 * the user can read log and see why the system rebooted.
-	 */
-	if (need_emergency_restart && amdgpu_ras_get_context(adev)->reboot) {
-		DRM_WARN("Emergency reboot.");
-
-		ksys_sync_helper();
-		emergency_restart();
-	}
-
 	dev_info(adev->dev, "GPU %s begin!\n",
 		need_emergency_restart ? "jobs stop":"reset");

Deucher, Alexander

maintainer

2023-08-07 23:01 UTC #4 lore raw

[Public]

Quoted 53 lines

-----Original Message-----
From: Matthew Wilcox <[email protected]>
Sent: Tuesday, August 1, 2023 8:34 AM
To: Christoph Hellwig <[email protected]>
Cc: Wysocki, Rafael J <[email protected]>; Christian Brauner <[email protected]>; Andrey Grodzovsky <[email protected]>; [email protected]; [email protected]; Deucher, Alexander <[email protected]>; Zhang, Hawking <[email protected]>; Harry Pan <[email protected]>; linux- [email protected]
Subject: Re: ksys_sync_helper
Quoted 9 lines · Christoph Hellwig
Quoted 8 lines · Rafael J
OK, I'll remember about this.
Quoted 3 lines
With this
and commit d5ea093eebf022e now we end up with a random driver (amdgpu) syncing all file systems for absolutely no good reason.
Quoted 6 lines
Quoted 4 lines
Sorry about that.
The problematic commit should still revert more or less cleanly, so please do that if that's what you need.
We'd still need to remove abuse in amdgpu first, though.
This would effectively revert d5ea093eebf0
drivers/gpu/drm/amd/amdgpu/amdgpu_device.c +0 -3
index dc0e5227119b..af04fece37d5 100644
@@ -75,7 +75,6 @@
 #include "amdgpu_fru_eeprom.h"
 #include "amdgpu_reset.h"
 
-#include <linux/suspend.h>
 #include <drm/task_barrier.h>
 #include <linux/pm_runtime.h>
 
@@ -5225,17 +5224,6 @@ int amdgpu_device_gpu_recover(struct
 amdgpu_device *adev,
       */
      need_emergency_restart =
 amdgpu_ras_need_emergency_restart(adev);
 
-     /*
-      * Flush RAM to disk so that after reboot

drivers/gpu/drm/amd/amdgpu/amdgpu_device.c int amdgpu_device_gpu_recover(struct · line 5241

Quoted 10 lines

drivers/gpu/drm/amd/amdgpu/amdgpu_device.c +0 -9

-      * the user can read log and see why the system rebooted.
-      */
-     if (need_emergency_restart && amdgpu_ras_get_context(adev)-
 >reboot) {
-             DRM_WARN("Emergency reboot.");
-
-             ksys_sync_helper();
-             emergency_restart();
-     }
-

Was on PTO last week. I think we can drop this. Will try and send out a patch this week to clean this up.

Alex

Quoted 3 lines

      dev_info(adev->dev, "GPU %s begin!\n",
              need_emergency_restart ? "jobs stop":"reset");