Thread (6 messages) 6 messages, 3 authors, 2021-11-25

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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help