On Thu, 23 Mar 2023, Geert Uytterhoeven wrote:
On Thu, Mar 23, 2023 at 7:02 AM Finn Thain <fthain@xxxxxxxxxxxxxx> wrote:
Some Nubus cards have many ROM resources. A single Radius video card
produced well over a thousand entries under /proc/bus/nubus/. Populating
/proc/bus/nubus on a slow machine with several such cards installed takes
long enough that the user may think that the system is wedged. All those
procfs entries also consume significant RAM though they are not normally
needed (except by developers). Omit these resources from /proc/bus/nubus/
by default and add a kernel parameter to enable them when needed.
On the test machine, this saved 300 kB and 10 seconds.
Tested-by: Stan Johnson <userm57@xxxxxxxxx>
Signed-off-by: Finn Thain <fthain@xxxxxxxxxxxxxx>
Thanks for your patch!
Thanks for your review.
Checkpatch says "externs should be avoided in .c files" and if this one
appeared twice I would agree. But as it only appears once, I can't see
any advantage to putting it in a new .h file instead of the .c file...
The advantage is that it allows the compiler to check that the
signatures of the declaration and the definition do match, now and in
the future.
Ah yes, I forgot that we use a header file to make sure that the header
file is correct ;-)
--- a/drivers/nubus/nubus.c
+++ b/drivers/nubus/nubus.c
@@ -34,6 +34,9 @@
LIST_HEAD(nubus_func_rsrcs);
+bool procfs_rsrcs;
+module_param(procfs_rsrcs, bool, 0444);
With the expanded functionality, is "rsrcs" still a good name?
Perhaps this should be an integer, so you can define different
levels? E.g.
- 0 = just devices
- 1 = above + boards + public resources
- 2 = above + private resources
That really depends on how the proc entries get used. I think it's
probably going to be developers who would use them so I consider all of
this to be outside of the UAPI and subject to change. But it would be nice
to hear from other developers on that point.
Regarding terminology, the files in /proc/bus/nubus/*/ are termed
"records" or "entries" while the subdirectories may represent boards, slot
resources or tables of entries. So a parameter like "proc_entries" (in
effect nubus.proc_entries) might be more apt than "procfs_rsrcs".
Linux "devices" correspond to the "functional resources" offered by a
card. (Other resources have other purposes.)
I don't know where the "local/private" designation originates from. It's
not to be found in Apple's book, "Designing Cards and Drivers for the
Macintosh". AFAIK, there's no distinction between "public" and "private"
like you might expect to find between the slot resources needed by Apple's
Slot Manager and those needed by 3rd party drivers. E.g. the
NUBUS_RESID_MAC_ADDRESS and NUBUS_RESID_GAMMADIR slot resources were
Apple-defined, even though nubus.c describes them as "local/private".
Note that this patch doesn't affect the /proc/bus/nubus/boards file which
lists category, type, dr_sw, dr_hw for every functional resource
implemented by every board.
(disclaimer: I know nothing about NuBus and the current /proc/nubus
layout)
Here's an example from my LCIII. There's only one card (in slot e) and it
has minimal resources and entries. e/1 is the "vendor info" slot resource,
e/1/3 is the "revision" entry from that slot resource.
# grep -ar . /proc/bus/nubus
/proc/bus/nubus/e/1/1:
/proc/bus/nubus/e/1/2:Ethernet A-Series
/proc/bus/nubus/e/1/20:
/proc/bus/nubus/e/1/24/1:A-Series
/proc/bus/nubus/e/1/24/3:001
/proc/bus/nubus/e/1/24/4:SS DEV
/proc/bus/nubus/e/80/1:
/proc/bus/nubus/e/80/2:Network_Ethernet_A_Series
/proc/bus/nubus/e/80/a:
/proc/bus/nubus/e/80/80:[snipped binary MAC address]
/proc/bus/nubus/devices:e 0004 0001 010c 0100 fe000000
#
With this patch, only the /proc/bus/nubus/devices file appears by default.
nubus.c captures that information for bus matching purposes, but the
/proc/bus/nubus/e/* inodes are unused AFAIK. I think they were put there
so that userland programs could find uses for them. Problem is, it can be
expensive to do that, depending on the card ROM, and hence this patch.
Should this be documented? I know there is currently nothing about NuBus
under Documentation/.
Probably...
+
/* Meaning of "bytelanes":
The card ROM may appear on any or all bytes of each long word in
@@ -574,7 +577,9 @@ nubus_get_functional_resource(struct nubus_board *board, int slot,
default:
/* Local/Private resources have their own
function */
- nubus_get_private_resource(fres, dir.procdir, &ent);
+ if (procfs_rsrcs)
+ nubus_get_private_resource(fres, dir.procdir,
+ &ent);
}
}
diff --git a/drivers/nubus/proc.c b/drivers/nubus/proc.c
index 2c320a84fd72..844e86636798 100644
--- a/drivers/nubus/proc.c
+++ b/drivers/nubus/proc.c
@@ -51,11 +51,13 @@ static struct proc_dir_entry *proc_bus_nubus_dir;
* /proc/bus/nubus/x/ stuff
*/
+extern bool procfs_rsrcs;
+
struct proc_dir_entry *nubus_proc_add_board(struct nubus_board *board)
{
char name[2];
- if (!proc_bus_nubus_dir)
+ if (!proc_bus_nubus_dir || !procfs_rsrcs)
return NULL;
snprintf(name, sizeof(name), "%x", board->slot);
return proc_mkdir(name, proc_bus_nubus_dir);
@@ -72,7 +74,7 @@ struct proc_dir_entry *nubus_proc_add_rsrc_dir(struct proc_dir_entry *procdir,
char name[9];
int lanes = board->lanes;
- if (!procdir)
+ if (!procdir || !procfs_rsrcs)
return NULL;
snprintf(name, sizeof(name), "%x", ent->type);
remove_proc_subtree(name, procdir);
@@ -157,7 +159,7 @@ void nubus_proc_add_rsrc_mem(struct proc_dir_entry *procdir,
char name[9];
struct nubus_proc_pde_data *pded;
- if (!procdir)
+ if (!procdir || !procfs_rsrcs)
return;
snprintf(name, sizeof(name), "%x", ent->type);
@@ -176,7 +178,7 @@ void nubus_proc_add_rsrc(struct proc_dir_entry *procdir,
char name[9];
unsigned char *data = (unsigned char *)ent->data;
- if (!procdir)
+ if (!procdir || !procfs_rsrcs)
return;
snprintf(name, sizeof(name), "%x", ent->type);
