Re: [PATCH 06/13] kallsyms: only build {, module_}kallsyms_on_each_symbol when required

Re: [PATCH 06/13] kallsyms: only build {, module_}kallsyms_on_each_symbol when required

[Miroslav Benes]

On Tue, 2 Feb 2021, Christoph Hellwig wrote:

> kallsyms_on_each_symbol and module_kallsyms_on_each_symbol are only used
> by the livepatching code, so don't build them if livepatching is not
> enabled.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Reviewed-by: Miroslav Benes <mbenes@suse.cz>

M
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 07/13] module: mark module_mutex static

[Miroslav Benes]

On Tue, 2 Feb 2021, Christoph Hellwig wrote:

> Except for two lockdep asserts module_mutex is only used in module.c.
> Remove the two asserts given that the functions they are in are not
> exported and just called from the module code, and mark module_mutex
> static.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Reviewed-by: Miroslav Benes <mbenes@suse.cz>

M
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 08/13] module: remove each_symbol_in_section

[Miroslav Benes]

On Tue, 2 Feb 2021, Christoph Hellwig wrote:

> each_symbol_in_section just contains a trivial loop over its arguments.
> Just open code the loop in the two callers.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Reviewed-by: Miroslav Benes <mbenes@suse.cz>

M
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 09/13] module: merge each_symbol_section into find_symbol

[Miroslav Benes]

On Tue, 2 Feb 2021, Christoph Hellwig wrote:

> each_symbol_section is only called by find_symbol, so merge the two
> functions.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Reviewed-by: Miroslav Benes <mbenes@suse.cz>

M
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 10/13] module: pass struct find_symbol_args to find_symbol

[Miroslav Benes]

>  void *__symbol_get(const char *symbol)
>  {
> -	struct module *owner;
> -	const struct kernel_symbol *sym;
> +	struct find_symbol_arg fsa = {
> +		.name	= symbol,
> +		.gplok	= true,
> +		.warn	= true,
> +	};
>  
>  	preempt_disable();
> -	sym = find_symbol(symbol, &owner, NULL, NULL, true, true);
> -	if (sym && strong_try_module_get(owner))
> -		sym = NULL;
> +	if (!find_symbol(&fsa) || !strong_try_module_get(fsa.owner)) {

I think this should be in fact

  if (!find_symbol(&fsa) || strong_try_module_get(fsa.owner)) {

to get the logic right (note the missing !). We want to return NULL if 
strong_try_module_get() does not succeed for a found symbol.

> +		preempt_enable();
> +		return NULL;
> +	}
>  	preempt_enable();
> -
> -	return sym ? (void *)kernel_symbol_value(sym) : NULL;
> +	return (void *)kernel_symbol_value(fsa.sym);
>  }
>  EXPORT_SYMBOL_GPL(__symbol_get);

Miroslav
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 11/13] module: move struct symsearch to module.c

[Miroslav Benes]

On Tue, 2 Feb 2021, Christoph Hellwig wrote:

> struct symsearch is only used inside of module.h, so move the definition
> out of module.h.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Reviewed-by: Miroslav Benes <mbenes@suse.cz>

M
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 12/13] module: remove EXPORT_SYMBOL_GPL_FUTURE

[Miroslav Benes]

On Tue, 2 Feb 2021, Christoph Hellwig wrote:

> As far as I can tell this has never been used at all, and certainly
> not any time recently.

Right, I've always wondered about this one.
 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Reviewed-by: Miroslav Benes <mbenes@suse.cz>

M
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 13/13] module: remove EXPORT_UNUSED_SYMBOL*

[Miroslav Benes]

On Tue, 2 Feb 2021, Christoph Hellwig wrote:

> EXPORT_UNUSED_SYMBOL* is not actually used anywhere.  Remove the
> unused functionality as we generally just remove unused code anyway.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Reviewed-by: Miroslav Benes <mbenes@suse.cz>

M
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: module loader dead code removal and cleanups v3

[Jessica Yu]

+++ Christoph Hellwig [02/02/21 13:13 +0100]:
>Hi all,
>
>this series removes support for long term unused export types and
>cleans up various loose ends in the module loader.
>
>Changes since v2:
> - clean up klp_find_object_symbol a bit
> - remove the now unused module_assert_mutex helper
>
>Changes since v1:
> - move struct symsearch to module.c
> - rework drm to not call find_module at all
> - allow RCU-sched locking for find_module
> - keep find_module as a public API instead of module_loaded
> - update a few comments and commit logs

Thanks Christoph for cleaning up all that aged cruft, and thanks everyone
for the reviews.

I was curious about EXPORT_SYMBOL_GPL_FUTURE and EXPORT_UNUSED_SYMBOL
variants, and found that most of that stuff was introduced between
2006 - 2008. All the of the unused symbols were removed and gpl future
symbols were converted to gpl quite a long time ago, and I don't
believe these export types have been used ever since. So I
think it's safe to retire those export types now.

The patchset looks good so far. After Miroslav's comments are
addressed, I'll wait an extra day or two in case there are more
comments before queueing them onto modules-next. I can take the first
two patches as well provided the acks are there (I think patch 2 is
missing Daniel Vetter's ack from v1 of the series, but I'll add that
back in).

Thanks,

Jessica
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: module loader dead code removal and cleanups v3

[Emil Velikov]

Hi Jessica,

On Tue, 2 Feb 2021 at 14:37, Jessica Yu <jeyu@kernel.org> wrote:
>
> +++ Christoph Hellwig [02/02/21 13:13 +0100]:
> >Hi all,
> >
> >this series removes support for long term unused export types and
> >cleans up various loose ends in the module loader.
> >
> >Changes since v2:
> > - clean up klp_find_object_symbol a bit
> > - remove the now unused module_assert_mutex helper
> >
> >Changes since v1:
> > - move struct symsearch to module.c
> > - rework drm to not call find_module at all
> > - allow RCU-sched locking for find_module
> > - keep find_module as a public API instead of module_loaded
> > - update a few comments and commit logs
>
> Thanks Christoph for cleaning up all that aged cruft, and thanks everyone
> for the reviews.
>
> I was curious about EXPORT_SYMBOL_GPL_FUTURE and EXPORT_UNUSED_SYMBOL
> variants, and found that most of that stuff was introduced between
> 2006 - 2008. All the of the unused symbols were removed and gpl future
> symbols were converted to gpl quite a long time ago, and I don't
> believe these export types have been used ever since. So I
> think it's safe to retire those export types now.
>
I believe you're spot on - based on reading through git log and
checking the ML archives.

Shame I didn't get to finish a similar series I had locally. Patches
11-13 match what I have here so:
Reviewed-by: Emil Velikov <emil.l.velikov@gmail.com>

HTH
-Emil
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 05/13] kallsyms: refactor {,module_}kallsyms_on_each_symbol

[Petr Mladek]

On Tue 2021-02-02 13:13:26, Christoph Hellwig wrote:
> Require an explicit call to module_kallsyms_on_each_symbol to look
> for symbols in modules instead of the call from kallsyms_on_each_symbol,
> and acquire module_mutex inside of module_kallsyms_on_each_symbol instead
> of leaving that up to the caller.  Note that this slightly changes the
> behavior for the livepatch code in that the symbols from vmlinux are not
> iterated anymore if objname is set, but that actually is the desired
> behavior in this case.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> Acked-by: Miroslav Benes <mbenes@suse.cz>

Reviewed-by: Petr Mladek <pmladek@suse.com>

Best Regards,
Petr
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 10/13] module: pass struct find_symbol_args to find_symbol

[Miroslav Benes]

On Wed, 3 Feb 2021, Christoph Hellwig wrote:

> FYI, this is the updated version:
> 
> ---
> >From 664ca3378deac7530fe8fc15fe73d583d3333df2 Mon Sep 17 00:00:00 2001
> From: Christoph Hellwig <hch@lst.de>
> Date: Wed, 20 Jan 2021 14:58:27 +0100
> Subject: module: pass struct find_symbol_args to find_symbol
> 
> Simplify the calling convention by passing the find_symbol_args structure
> to find_symbol instead of initializing it inside the function.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Reviewed-by: Miroslav Benes <mbenes@suse.cz>

M
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 01/13] powerpc/powernv: remove get_cxl_module

[Michael Ellerman]

Christoph Hellwig <hch@lst.de> writes:
> The static inline get_cxl_module function is entirely unused since commit
> 8bf6b91a5125a ("Revert "powerpc/powernv: Add support for the cxl kernel
> api on the real phb"), so remove it.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> Reviewed-by: Andrew Donnellan <ajd@linux.ibm.com>
> ---
>  arch/powerpc/platforms/powernv/pci-cxl.c | 22 ----------------------
>  1 file changed, 22 deletions(-)

Acked-by: Michael Ellerman <mpe@ellerman.id.au>

cheers
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: module loader dead code removal and cleanups v3

[Jessica Yu]

+++ Christoph Hellwig [02/02/21 13:13 +0100]:
>Hi all,
>
>this series removes support for long term unused export types and
>cleans up various loose ends in the module loader.
>
>Changes since v2:
> - clean up klp_find_object_symbol a bit
> - remove the now unused module_assert_mutex helper
>
>Changes since v1:
> - move struct symsearch to module.c
> - rework drm to not call find_module at all
> - allow RCU-sched locking for find_module
> - keep find_module as a public API instead of module_loaded
> - update a few comments and commit logs
>
>Diffstat:

Queued on modules-next (along with the updated patch 10).

Thanks everyone,

Jessica
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 01/13] powerpc/powernv: remove get_cxl_module

[Andrew Donnellan]

On 21/1/21 6:49 pm, Christoph Hellwig wrote:
> The static inline get_cxl_module function is entirely unused,
> remove it.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

The one user of this was removed in 8bf6b91a5125a ("Revert 
"powerpc/powernv: Add support for the cxl kernel api on the real phb").

Thanks for picking this up.

Reviewed-by: Andrew Donnellan <ajd@linux.ibm.com>

-- 
Andrew Donnellan              OzLabs, ADL Canberra
ajd@linux.ibm.com             IBM Australia Limited
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel