2023-03-26 03:53:21

by Finn Thain

[permalink] [raw]
Subject: [RFC PATCH v3] nubus: Don't list slot resources by default

Some Nubus card ROMs contain many slot 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.

Cc: Brad Boyer <[email protected]>
Tested-by: Stan Johnson <[email protected]>
Signed-off-by: Finn Thain <[email protected]>
---
drivers/nubus/nubus.c | 12 +++++++++---
drivers/nubus/proc.c | 8 ++++----
include/linux/nubus.h | 1 +
3 files changed, 14 insertions(+), 7 deletions(-)

diff --git a/drivers/nubus/nubus.c b/drivers/nubus/nubus.c
index f70ba58dbc55..d2d2b580f646 100644
--- a/drivers/nubus/nubus.c
+++ b/drivers/nubus/nubus.c
@@ -32,6 +32,12 @@

/* Globals */

+/* This parameter makes slot resources available in procfs. It's deprecated and
+ * disabled by default as procfs is no longer thought to be suitable for that.
+ */
+bool populate_procfs;
+module_param(populate_procfs, bool, 0);
+
LIST_HEAD(nubus_func_rsrcs);

/* Meaning of "bytelanes":
@@ -572,9 +578,9 @@ nubus_get_functional_resource(struct nubus_board *board, int slot,
nubus_proc_add_rsrc(dir.procdir, &ent);
break;
default:
- /* Local/Private resources have their own
- function */
- nubus_get_private_resource(fres, dir.procdir, &ent);
+ if (populate_procfs)
+ nubus_get_private_resource(fres, dir.procdir,
+ &ent);
}
}

diff --git a/drivers/nubus/proc.c b/drivers/nubus/proc.c
index 2c320a84fd72..1808accb8214 100644
--- a/drivers/nubus/proc.c
+++ b/drivers/nubus/proc.c
@@ -55,7 +55,7 @@ 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 || !populate_procfs)
return NULL;
snprintf(name, sizeof(name), "%x", board->slot);
return proc_mkdir(name, proc_bus_nubus_dir);
@@ -72,7 +72,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 || !populate_procfs)
return NULL;
snprintf(name, sizeof(name), "%x", ent->type);
remove_proc_subtree(name, procdir);
@@ -157,7 +157,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 || !populate_procfs)
return;

snprintf(name, sizeof(name), "%x", ent->type);
@@ -176,7 +176,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 || !populate_procfs)
return;

snprintf(name, sizeof(name), "%x", ent->type);
diff --git a/include/linux/nubus.h b/include/linux/nubus.h
index 392fc6c53e96..50c9145808d1 100644
--- a/include/linux/nubus.h
+++ b/include/linux/nubus.h
@@ -93,6 +93,7 @@ extern struct bus_type nubus_bus_type;

/* Generic NuBus interface functions, modelled after the PCI interface */
#ifdef CONFIG_PROC_FS
+extern bool populate_procfs;
void nubus_proc_init(void);
struct proc_dir_entry *nubus_proc_add_board(struct nubus_board *board);
struct proc_dir_entry *nubus_proc_add_rsrc_dir(struct proc_dir_entry *procdir,
--
2.37.5


2023-04-03 06:21:28

by Brad Boyer

[permalink] [raw]
Subject: Re: [RFC PATCH v3] nubus: Don't list slot resources by default

On Sun, Mar 26, 2023 at 02:37:02PM +1100, Finn Thain wrote:
> diff --git a/drivers/nubus/nubus.c b/drivers/nubus/nubus.c
> index f70ba58dbc55..d2d2b580f646 100644
> --- a/drivers/nubus/nubus.c
> +++ b/drivers/nubus/nubus.c
> @@ -32,6 +32,12 @@
>
> /* Globals */
>
> +/* This parameter makes slot resources available in procfs. It's deprecated and
> + * disabled by default as procfs is no longer thought to be suitable for that.
> + */
> +bool populate_procfs;
> +module_param(populate_procfs, bool, 0);
> +
> LIST_HEAD(nubus_func_rsrcs);
>
> /* Meaning of "bytelanes":

Would it be better to give this option a name that indicates that it
is for nubus only? These are effectively a global namespace. Other
than this, I don't see any issues.

Brad Boyer
[email protected]

2023-04-03 22:52:49

by Finn Thain

[permalink] [raw]
Subject: Re: [RFC PATCH v3] nubus: Don't list slot resources by default

On Sun, 2 Apr 2023, Brad Boyer wrote:

> On Sun, Mar 26, 2023 at 02:37:02PM +1100, Finn Thain wrote:
> > diff --git a/drivers/nubus/nubus.c b/drivers/nubus/nubus.c
> > index f70ba58dbc55..d2d2b580f646 100644
> > --- a/drivers/nubus/nubus.c
> > +++ b/drivers/nubus/nubus.c
> > @@ -32,6 +32,12 @@
> >
> > /* Globals */
> >
> > +/* This parameter makes slot resources available in procfs. It's deprecated and
> > + * disabled by default as procfs is no longer thought to be suitable for that.
> > + */
> > +bool populate_procfs;
> > +module_param(populate_procfs, bool, 0);
> > +
> > LIST_HEAD(nubus_func_rsrcs);
> >
> > /* Meaning of "bytelanes":
>
> Would it be better to give this option a name that indicates that it is
> for nubus only? These are effectively a global namespace. Other than
> this, I don't see any issues.
>

The name of the module here is "nubus", so this parameter is specified as
"nubus.populate_procfs" in the kernel parameters. Without the "nubus" it
doesn't work. I should probably add a comment about that.

2023-04-04 21:18:34

by Brad Boyer

[permalink] [raw]
Subject: Re: [RFC PATCH v3] nubus: Don't list slot resources by default

On Tue, Apr 04, 2023 at 08:50:59AM +1000, Finn Thain wrote:
> On Sun, 2 Apr 2023, Brad Boyer wrote:
> > Would it be better to give this option a name that indicates that it is
> > for nubus only? These are effectively a global namespace. Other than
> > this, I don't see any issues.
>
> The name of the module here is "nubus", so this parameter is specified as
> "nubus.populate_procfs" in the kernel parameters. Without the "nubus" it
> doesn't work. I should probably add a comment about that.

OK. A comment would be good. Thank you for the clarification. It's been
a while since I needed to use anything like that myself. The code I
deal with on a regular basis is never built-in, so I missed how that
worked these days. Now that I looked at the headers, I see how that
works to add the prefix depending on the build type.

Brad Boyer
[email protected]