Re: [PATCH] powerpc/pseries: delete scanlog
From: Michael Ellerman <mpe@ellerman.id.au>
Date: 2021-11-18 00:27:30
Nathan Lynch [off-list ref] writes:
Michael Ellerman [off-list ref] writes:quoted
Nathan Lynch [off-list ref] writes:quoted
Nathan Lynch [off-list ref] writes:quoted
Remove the pseries scanlog driver. This code supports functions from Power4-era servers that are not present on targets currently supported by arch/powerpc. System manuals from this time have this description: Scan Dump data is a set of chip data that the service processor gathers after a system malfunction. It consists of chip scan rings, chip trace arrays, and Scan COM (SCOM) registers. This data is stored in the scan-log partition of the system’s Nonvolatile Random Access Memory (NVRAM). PowerVM partition firmware development doesn't recognize the associated function call or property, and they don't see any references to them in their codebase. It seems to have been specific to non-virtualized pseries.Just bumping this to see if there are any objections.Not an objection, I like nothing better than dropping old unused cruft, but are we sure it's safe to remove the proc file? I see that rtas_errd still looks for it, have you checked that it will handle the absence of the file gracefully and continue doing whatever else it does?Uhh. I will stop forgetting to check ppc64_diag when making such changes. Thanks for pointing this out.
No worries.
quoted
On further inspection it looks like the code that looks for it in rtas_errd is #if 0'ed out (??), so maybe it's dead.Yes it seems so. From rtas_errd's main(): #if 0 /* * Check to see if a new scanlog dump is available; if so, copy it to * the filesystem and associate the dump with the first error processed. */ check_scanlog_dump(); #endif And that's the only entry point into the code that collects the scanlog data. And that dead code appears to deal with the absence of /proc/ppc64/scan-log-dump gracefully. It has been like that since initial git import in 2013.
OK, I guess it came from sourceforge before that. But I'm not going to start digging there, that's long enough ago that it shouldn't matter.
quoted
Anyway if you can test that rtas_errd still works that'd be good.I've verified that it starts normally and logs EPOW events associated with partition migration.
Awesome.
quoted
Presumably there's no other code that cares about the proc file.AFAIK this is right. powerpc-utils and librtas do not use it. librtas has a wrapper for the calling the associated RTAS function directly, but that's fine. I tried using GitHub's search to find instances of "scan-log-dump" that weren't from Linux or ppc64_diag (need to be logged in I think): https://github.com/search?q=%22scan-log-dump%22+-path%3Aarch%2Fpowerpc+-filename%3Ascanlog.c+-extension%3Apatch&type=Code&ref=advsearch&l=&l= This hasn't yielded any unexpected users. There may be better search terms but that's what a few minutes of fiddling with it got me.
I had a look on sourcegraph too, same story, nothing interesting: https://sourcegraph.com/search?q=context:global+scan-log-dump+NOT+file:arch/powerpc/platforms/pseries/scanlog.c+NOT+file:arch/powerpc/kernel/rtas.c+NOT+file:arch/ppc64/kernel/scanlog.c+fork:yes+archived:yes&patternType=literal So that seems OK to me, I'll pick this up. cheers