Re: [PATCH v2] nubus: Don't list card resources by default

[Geert Uytterhoeven]

Hi Finn,

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!

> 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.

> --- 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
(disclaimer: I know nothing about NuBus and the current /proc/nubus
 layout)

Should this be documented?
I know there is currently nothing about NuBus under Documentation/.

> +
>  /* 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);

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds