* Re: [PATCH] powerpc/mm: Limit allocation of SWIOTLB on server machines
From: Konrad Rzeszutek Wilk @ 2021-01-21 15:54 UTC (permalink / raw)
To: Thiago Jung Bauermann
Cc: linuxppc-dev, Satheesh Rajendran, Ram Pai, linux-kernel
In-Reply-To: <87bldzlzu2.fsf@manicouagan.localdomain>
On Fri, Jan 08, 2021 at 09:27:01PM -0300, Thiago Jung Bauermann wrote:
>
> Ram Pai <linuxram@us.ibm.com> writes:
>
> > On Wed, Dec 23, 2020 at 09:06:01PM -0300, Thiago Jung Bauermann wrote:
> >>
> >> Hi Ram,
> >>
> >> Thanks for reviewing this patch.
> >>
> >> Ram Pai <linuxram@us.ibm.com> writes:
> >>
> >> > On Fri, Dec 18, 2020 at 03:21:03AM -0300, Thiago Jung Bauermann wrote:
> >> >> On server-class POWER machines, we don't need the SWIOTLB unless we're a
> >> >> secure VM. Nevertheless, if CONFIG_SWIOTLB is enabled we unconditionally
> >> >> allocate it.
> >> >>
> >> >> In most cases this is harmless, but on a few machine configurations (e.g.,
> >> >> POWER9 powernv systems with 4 GB area reserved for crashdump kernel) it can
> >> >> happen that memblock can't find a 64 MB chunk of memory for the SWIOTLB and
> >> >> fails with a scary-looking WARN_ONCE:
> >> >>
> >> >> ------------[ cut here ]------------
> >> >> memblock: bottom-up allocation failed, memory hotremove may be affected
> >> >> WARNING: CPU: 0 PID: 0 at mm/memblock.c:332 memblock_find_in_range_node+0x328/0x340
> >> >> Modules linked in:
> >> >> CPU: 0 PID: 0 Comm: swapper Not tainted 5.10.0-rc2-orig+ #6
> >> >> NIP: c000000000442f38 LR: c000000000442f34 CTR: c0000000001e0080
> >> >> REGS: c000000001def900 TRAP: 0700 Not tainted (5.10.0-rc2-orig+)
> >> >> MSR: 9000000002021033 <SF,HV,VEC,ME,IR,DR,RI,LE> CR: 28022222 XER: 20040000
> >> >> CFAR: c00000000014b7b4 IRQMASK: 1
> >> >> GPR00: c000000000442f34 c000000001defba0 c000000001deff00 0000000000000047
> >> >> GPR04: 00000000ffff7fff c000000001def828 c000000001def820 0000000000000000
> >> >> GPR08: 0000001ffc3e0000 c000000001b75478 c000000001b75478 0000000000000001
> >> >> GPR12: 0000000000002000 c000000002030000 0000000000000000 0000000000000000
> >> >> GPR16: 0000000000000000 0000000000000000 0000000000000000 0000000002030000
> >> >> GPR20: 0000000000000000 0000000000010000 0000000000010000 c000000001defc10
> >> >> GPR24: c000000001defc08 c000000001c91868 c000000001defc18 c000000001c91890
> >> >> GPR28: 0000000000000000 ffffffffffffffff 0000000004000000 00000000ffffffff
> >> >> NIP [c000000000442f38] memblock_find_in_range_node+0x328/0x340
> >> >> LR [c000000000442f34] memblock_find_in_range_node+0x324/0x340
> >> >> Call Trace:
> >> >> [c000000001defba0] [c000000000442f34] memblock_find_in_range_node+0x324/0x340 (unreliable)
> >> >> [c000000001defc90] [c0000000015ac088] memblock_alloc_range_nid+0xec/0x1b0
> >> >> [c000000001defd40] [c0000000015ac1f8] memblock_alloc_internal+0xac/0x110
> >> >> [c000000001defda0] [c0000000015ac4d0] memblock_alloc_try_nid+0x94/0xcc
> >> >> [c000000001defe30] [c00000000159c3c8] swiotlb_init+0x78/0x104
> >> >> [c000000001defea0] [c00000000158378c] mem_init+0x4c/0x98
> >> >> [c000000001defec0] [c00000000157457c] start_kernel+0x714/0xac8
> >> >> [c000000001deff90] [c00000000000d244] start_here_common+0x1c/0x58
> >> >> Instruction dump:
> >> >> 2c230000 4182ffd4 ea610088 ea810090 4bfffe84 39200001 3d42fff4 3c62ff60
> >> >> 3863c560 992a8bfc 4bd0881d 60000000 <0fe00000> ea610088 4bfffd94 60000000
> >> >> random: get_random_bytes called from __warn+0x128/0x184 with crng_init=0
> >> >> ---[ end trace 0000000000000000 ]---
> >> >> software IO TLB: Cannot allocate buffer
> >> >>
> >> >> Unless this is a secure VM the message can actually be ignored, because the
> >> >> SWIOTLB isn't needed. Therefore, let's avoid the SWIOTLB in those cases.
> >> >
> >> > The above warn_on is conveying a genuine warning. Should it be silenced?
> >>
> >> Not sure I understand your point. This patch doesn't silence the
> >> warning, it avoids the problem it is warning about.
> >
> > Sorry, I should have explained it better. My point is...
> >
> > If CONFIG_SWIOTLB is enabled, it means that the kernel is
> > promising the bounce buffering capability. I know, currently we
> > do not have any kernel subsystems that use bounce buffers on
> > non-secure-pseries-kernel or powernv-kernel. But that does not
> > mean, there wont be any. In case there is such a third-party
> > module needing bounce buffering, it wont be able to operate,
> > because of the proposed change in your patch.
> >
> > Is that a good thing or a bad thing, I do not know. I will let
> > the experts opine.
>
> Ping? Does anyone else has an opinion on this? The other option I can
> think of is changing the crashkernel code to not reserve so much memory
> below 4 GB. Other people are considering this option, but it's not
> planned for the near future.
That seems a more suitable solution regardless, but there is always
the danger of not being enough or being too big.
There was some autocrashkernel allocation patches going around
for x86 and ARM that perhaps could be re-used?
>
> Also, there's a patch currently in linux-next which removes the scary
> warning because of unrelated reasons:
>
> https://lore.kernel.org/lkml/20201217201214.3414100-2-guro@fb.com
>
> So assuming that the patch above goes in and keeping the assumption that
> the swiotlb won't be needed in the powernv machines where I've seen the
> warning happen, we can just leave things as they are now.
If that solves the problem, then that is OK.
>
> --
> Thiago Jung Bauermann
> IBM Linux Technology Center
^ permalink raw reply
* Re: [RFC PATCH v3 5/6] dt-bindings: of: Add restricted DMA pool
From: Rob Herring @ 2021-01-21 15:48 UTC (permalink / raw)
To: Robin Murphy
Cc: Heikki Krogerus, Peter Zijlstra, Grant Likely, Paul Mackerras,
Frank Rowand, Ingo Molnar, Marek Szyprowski, Stefano Stabellini,
Saravana Kannan, Heinrich Schuchardt, Joerg Roedel,
Wysocki, Rafael J, Christoph Hellwig, Bartosz Golaszewski,
xen-devel, Thierry Reding, devicetree, Will Deacon,
Konrad Rzeszutek Wilk, Dan Williams, Nicolas Boichat,
Claire Chang, Boris Ostrovsky, Andy Shevchenko, Juergen Gross,
Greg Kroah-Hartman, Randy Dunlap, linux-kernel@vger.kernel.org,
Tomasz Figa, Linux IOMMU, linuxppc-dev, Thiago Jung Bauermann
In-Reply-To: <c0d631de-8840-4f6e-aebf-41bb8449f78c@arm.com>
On Wed, Jan 20, 2021 at 7:10 PM Robin Murphy <robin.murphy@arm.com> wrote:
>
> On 2021-01-20 21:31, Rob Herring wrote:
> > On Wed, Jan 20, 2021 at 11:30 AM Robin Murphy <robin.murphy@arm.com> wrote:
> >>
> >> On 2021-01-20 16:53, Rob Herring wrote:
> >>> On Wed, Jan 06, 2021 at 11:41:23AM +0800, Claire Chang wrote:
> >>>> Introduce the new compatible string, restricted-dma-pool, for restricted
> >>>> DMA. One can specify the address and length of the restricted DMA memory
> >>>> region by restricted-dma-pool in the device tree.
> >>>
> >>> If this goes into DT, I think we should be able to use dma-ranges for
> >>> this purpose instead. Normally, 'dma-ranges' is for physical bus
> >>> restrictions, but there's no reason it can't be used for policy or to
> >>> express restrictions the firmware has enabled.
> >>
> >> There would still need to be some way to tell SWIOTLB to pick up the
> >> corresponding chunk of memory and to prevent the kernel from using it
> >> for anything else, though.
> >
> > Don't we already have that problem if dma-ranges had a very small
> > range? We just get lucky because the restriction is generally much
> > more RAM than needed.
>
> Not really - if a device has a naturally tiny addressing capability that
> doesn't even cover ZONE_DMA32 where the regular SWIOTLB buffer will be
> allocated then it's unlikely to work well, but that's just crap system
> design. Yes, memory pressure in ZONE_DMA{32} is particularly problematic
> for such limited devices, but it's irrelevant to the issue at hand here.
Yesterday's crap system design is today's security feature. Couldn't
this feature make crap system design work better?
> What we have here is a device that's not allowed to see *kernel* memory
> at all. It's been artificially constrained to a particular region by a
> TZASC or similar, and the only data which should ever be placed in that
May have been constrained, but that's entirely optional.
In the optional case where the setup is entirely up to the OS, I don't
think this belongs in the DT at all. Perhaps that should be solved
first.
> region is data intended for that device to see. That way if it tries to
> go rogue it physically can't start slurping data intended for other
> devices or not mapped for DMA at all. The bouncing is an important part
> of this - I forget the title off-hand but there was an interesting paper
> a few years ago which demonstrated that even with an IOMMU, streaming
> DMA of in-place buffers could reveal enough adjacent data from the same
> page to mount an attack on the system. Memory pressure should be
> immaterial since the size of each bounce pool carveout will presumably
> be tuned for the needs of the given device.
>
> > In any case, wouldn't finding all the dma-ranges do this? We're
> > already walking the tree to find the max DMA address now.
>
> If all you can see are two "dma-ranges" properties, how do you propose
> to tell that one means "this is the extent of what I can address, please
> set my masks and dma-range-map accordingly and try to allocate things
> where I can reach them" while the other means "take this output range
> away from the page allocator and hook it up as my dedicated bounce pool,
> because it is Serious Security Time"? Especially since getting that
> choice wrong either way would be a Bad Thing.
Either we have some heuristic based on the size or we add some hint.
The point is let's build on what we already have for defining DMA
accessible memory in DT rather than some parallel mechanism.
Rob
^ permalink raw reply
* Re: [PATCH 6/6] powerpc/rtas: constrain user region allocation to RMA
From: Nathan Lynch @ 2021-01-21 15:27 UTC (permalink / raw)
To: Michael Ellerman, Alexey Kardashevskiy, linuxppc-dev
Cc: tyreld, brking, ajd, aneesh.kumar
In-Reply-To: <87ft2vrew6.fsf@mpe.ellerman.id.au>
Michael Ellerman <mpe@ellerman.id.au> writes:
> Nathan Lynch <nathanl@linux.ibm.com> writes:
>> Alexey Kardashevskiy <aik@ozlabs.ru> writes:
>>> On 16/01/2021 02:38, Nathan Lynch wrote:
>>>> Alexey Kardashevskiy <aik@ozlabs.ru> writes:
>>>>> On 15/01/2021 09:00, Nathan Lynch wrote:
>>>>>> Memory locations passed as arguments from the OS to RTAS usually need
>>>>>> to be addressable in 32-bit mode and must reside in the Real Mode
>>>>>> Area. On PAPR guests, the RMA starts at logical address 0 and is the
>>>>>> first logical memory block reported in the LPAR’s device tree.
>>>>>>
>>>>>> On powerpc targets with RTAS, Linux makes available to user space a
>>>>>> region of memory suitable for arguments to be passed to RTAS via
>>>>>> sys_rtas(). This region (rtas_rmo_buf) is allocated via the memblock
>>>>>> API during boot in order to ensure that it satisfies the requirements
>>>>>> described above.
>>>>>>
>>>>>> With radix MMU, the upper limit supplied to the memblock allocation
>>>>>> can exceed the bounds of the first logical memory block, since
>>>>>> ppc64_rma_size is ULONG_MAX and RTAS_INSTANTIATE_MAX is 1GB. (512MB is
>>>>>> a common size of the first memory block according to a small sample of
>>>>>> LPARs I have checked.) This leads to failures when user space invokes
>>>>>> an RTAS function that uses a work area, such as
>>>>>> ibm,configure-connector.
>>>>>>
>>>>>> Alter the determination of the upper limit for rtas_rmo_buf's
>>>>>> allocation to consult the device tree directly, ensuring placement
>>>>>> within the RMA regardless of the MMU in use.
>>>>>
>>>>> Can we tie this with RTAS (which also needs to be in RMA) and simply add
>>>>> extra 64K in prom_instantiate_rtas() and advertise this address
>>>>> (ALIGH_UP(rtas-base + rtas-size, PAGE_SIZE)) to the user space? We do
>>>>> not need this RMO area before that point.
>>>>
>>>> Can you explain more about what advantage that would bring? I'm not
>>>> seeing it. It's a more significant change than what I've written
>>>> here.
>>>
>>>
>>> We already allocate space for RTAS and (like RMO) it needs to be in RMA,
>>> and RMO is useless without RTAS. We can reuse RTAS allocation code for
>>> RMO like this:
>>
>> When you say RMO I assume you are referring to rtas_rmo_buf? (I don't
>> think it is well-named.)
> ...
>
> RMO (Real mode offset) is the old term we used to use to refer to what
> is now called the RMA (Real mode area). There are still many references
> to RMO in Linux, but they almost certainly all refer to what we now call
> the RMA.
Yes... but I think in this discussion Alexey was using RMO to stand in
for rtas_rmo_buf, which was what I was trying to clarify.
>>> May be store in the FDT as "linux,rmo-base" next to "linux,rtas-base",
>>> for clarity, as sharing symbols between prom and main kernel is a bit
>>> tricky.
>>>
>>> The benefit is that we do not do the same thing (== find 64K in RMA)
>>> in 2 different ways and if the RMO allocated my way is broken - we'll
>>> know it much sooner as RTAS itself will break too.
>>
>> Implementation details aside... I'll grant that combining the
>> allocations into one in prom_init reduces some duplication in the sense
>> that both are subject to the same constraints (mostly - the RTAS data
>> area must not cross a 256MB boundary, while the user region may). But
>> they really are distinct concerns. The RTAS private data area is
>> specified in the platform architecture, the OS is obligated to allocate
>> it and pass it to instantiate-rtas, etc etc. However the user region
>> (rtas_rmo_buf) is purely a Linux construct which is there to support
>> sys_rtas.
>>
>> Now, there are multiple sites in the kernel proper that must allocate
>> memory suitable for passing to RTAS. Obviously there is value in
>> consolidating the logic for that purpose in one place, so I'll work on
>> adding that in v2. OK?
>
> I don't think we want to move any allocations into prom_init.c unless we
> have to.
>
> It's best thought of as a trampoline, that runs before the kernel
> proper, to transition from live OF to a flat DT environment. One thing
> that must be done as part of that is instantiating RTAS, because it's
> basically a runtime copy of the live OF. But any other allocs are for
> Linux to handle later, IMHO.
Agreed.
^ permalink raw reply
* Re: [PATCH 5/6] powerpc/rtas: rename RTAS_RMOBUF_MAX to RTAS_USER_REGION_SIZE
From: Nathan Lynch @ 2021-01-21 15:17 UTC (permalink / raw)
To: Alexey Kardashevskiy, linuxppc-dev; +Cc: tyreld, brking, ajd, aneesh.kumar
In-Reply-To: <7988dce5-6cf3-df79-1276-7bc91ce7c8b2@ozlabs.ru>
Alexey Kardashevskiy <aik@ozlabs.ru> writes:
> On 20/01/2021 12:17, Nathan Lynch wrote:
>> Alexey Kardashevskiy <aik@ozlabs.ru> writes:
>>> On 16/01/2021 02:56, Nathan Lynch wrote:
>>>> Alexey Kardashevskiy <aik@ozlabs.ru> writes:
>>>>> On 15/01/2021 09:00, Nathan Lynch wrote:
>>>>>> +#define RTAS_WORK_AREA_SIZE 4096
>>>>>> +
>>>>>> +/* Work areas allocated for user space access. */
>>>>>> +#define RTAS_USER_REGION_SIZE (RTAS_WORK_AREA_SIZE * 16)
>>>>>
>>>>> This is still 64K but no clarity why. There is 16 of something, what
>>>>> is it?
>>>>
>>>> There are 16 4KB work areas in the region. I can name it
>>>> RTAS_NR_USER_WORK_AREAS or similar.
>>>
>>>
>>> Why 16? PAPR (then add "per PAPR") or we just like 16 ("should be
>>> enough")?
>>
>> PAPR doesn't know anything about the user region; it's a Linux
>> construct. It's been 64KB since pre-git days and I'm not sure what the
>> original reason is. At this point, maintaining a kernel-user ABI seems
>> like enough justification for the value.
>
> I am not arguing keeping the numbers but you are replacing one magic
> number with another and for neither it is horribly obvious where they
> came from.
When I wrote it I viewed it as changing one of the factors in (64 *
1024) to a named constant that better expresses how the region is used
and adjusting the remaining factor to arrive at the same end result. I
considered it a net improvement even if we're not sure how 64K was
arrived at in the first place, although I suspect it was chosen to
support multiple concurrent users, and to be compatible with both 4K
and 64K page sizes. Then again 64K pages came a bit after this was
introduced.
The change that introduced RTAS_RMOBUF_MAX (here renamed to
RTAS_USER_REGION_SIZE) does not explain how the value was derived:
================
Author: Andrew Morton <akpm@osdl.org>
Date: Sun Jan 18 18:17:30 2004 -0800
[PATCH] ppc64: add rtas syscall, from John Rose
From: Anton Blanchard <anton@samba.org>
Added RTAS syscall. Reserved lowmem rtas_rmo_buf for userspace use. Created
"rmo_buffer" proc file to export bounds of rtas_rmo_buf.
[...]
diff --git a/include/asm-ppc64/rtas.h b/include/asm-ppc64/rtas.h
index 42a0b484077c..d9e426161044 100644
--- a/include/asm-ppc64/rtas.h
+++ b/include/asm-ppc64/rtas.h
@@ -19,6 +19,9 @@
#define RTAS_UNKNOWN_SERVICE (-1)
#define RTAS_INSTANTIATE_MAX (1UL<<30) /* Don't instantiate rtas at/above this value */
+/* Buffer size for ppc_rtas system call. */
+#define RTAS_RMOBUF_MAX (64 * 1024)
+
================
The comment "Buffer size for ppc_rtas system call" (removed by my
change) is not really appropriate because 1. not all sys_rtas
invocations use the buffer, and 2. no callers use the entire buffer.
> Is 16 the max number of concurrently running sys_rtas system
> calls? Does the userspace ensure there is no more than 16?
No and no; not all calls to sys_rtas need to use a work area. However,
librtas uses record locking to arbitrate access to the user region, and
the unit of allocation is 4KB. This is a reasonable choice: many RTAS
calls which take a work area require 4KB alignment. But some do not
(ibm,get-system-parameter), and librtas conceivably could be made to
perform finer-grained allocations.
It's not the kernel's concern how librtas partitions the user region, so
I'm inclined to leave the (64 * 1024) expression alone now. Thanks for
your review.
> btw where is that userspace code? I thought
> https://github.com/power-ras/ppc64-diag.git but no. Thanks,
librtas, of which ppc64-diag and powerpc-utils are users:
https://github.com/ibm-power-utilities/librtas
^ permalink raw reply related
* Re: [PATCH] powerpc/64: prevent replayed interrupt handlers from running softirqs
From: Michael Ellerman @ 2021-01-21 12:50 UTC (permalink / raw)
To: Nicholas Piggin, linuxppc-dev; +Cc: Nicholas Piggin
In-Reply-To: <20210120075005.1678565-1-npiggin@gmail.com>
Nicholas Piggin <npiggin@gmail.com> writes:
> Running softirqs enables interrupts, which can then end up recursing
> into the irq soft-mask code we're adjusting, including replaying
> interrupts itself, which might be theoretically unbounded.
>
> This abridged trace shows how this can occur:
>
> ! NIP replay_soft_interrupts
> LR interrupt_exit_kernel_prepare
> Call Trace:
> interrupt_exit_kernel_prepare (unreliable)
> interrupt_return
> --- interrupt: ea0 at __rb_reserve_next
> NIP __rb_reserve_next
> LR __rb_reserve_next
> Call Trace:
> ring_buffer_lock_reserve
> trace_function
> function_trace_call
> ftrace_call
> __do_softirq
> irq_exit
> timer_interrupt
> ! replay_soft_interrupts
> interrupt_exit_kernel_prepare
> interrupt_return
> --- interrupt: ea0 at arch_local_irq_restore
>
> Fix this by disabling bhs (softirqs) around the interrupt replay.
>
> I don't know that commit 3282a3da25bd ("powerpc/64: Implement soft
> interrupt replay in C") actually introduced the problem. Prior to that
> change, the interrupt replay seems like it should still be subect to
> this recusion, however it's done after all the irq state has been fixed
> up at the end of the replay, so it seems reasonable to fix back to this
> commit.
This seems very unhappy for me (on P9 bare metal):
[ 0.038571] Mountpoint-cache hash table entries: 131072 (order: 4, 1048576 bytes, linear)
[ 0.040194] ------------[ cut here ]------------
[ 0.040228] WARNING: CPU: 0 PID: 0 at kernel/softirq.c:176 __local_bh_enable_ip+0x150/0x210
[ 0.040263] Modules linked in:
[ 0.040280] CPU: 0 PID: 0 Comm: swapper/0 Not tainted 5.11.0-rc2-00008-g4899f32e4f2a #1
[ 0.040321] NIP: c000000000114bc0 LR: c0000000000172a0 CTR: c00000000002a020
[ 0.040360] REGS: c00000000177f670 TRAP: 0700 Not tainted (5.11.0-rc2-00008-g4899f32e4f2a)
[ 0.040410] MSR: 9000000002021033 <SF,HV,VEC,ME,IR,DR,RI,LE> CR: 28000224 XER: 20040000
[ 0.040472] CFAR: c000000000114ae8 IRQMASK: 3
GPR00: c0000000000172a0 c00000000177f910 c000000001783900 c000000000017290
GPR04: 0000000000000200 4000000000000000 0000000000000002 00000001312d0000
GPR08: 0000000000000000 c0000000016f3480 0000000000000202 0000000000000000
GPR12: c00000000002a020 c0000000023a0000 0000000000000000 0000000000000000
GPR16: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
GPR20: 0000000000000001 00000000100051c6 0000000000000000 0000000000000009
GPR24: 0000000000000e60 0000000000000900 0000000000000500 0000000000000a00
GPR28: 0000000000000f00 0000000000000002 0000000000000003 0000000000000200
[ 0.040824] NIP [c000000000114bc0] __local_bh_enable_ip+0x150/0x210
[ 0.040863] LR [c0000000000172a0] replay_soft_interrupts+0x2e0/0x340
[ 0.040904] Call Trace:
[ 0.040926] [c00000000177f910] [0000000000000500] 0x500 (unreliable)
[ 0.040962] [c00000000177f950] [c0000000000172a0] replay_soft_interrupts+0x2e0/0x340
[ 0.041008] [c00000000177fb50] [c000000000017370] arch_local_irq_restore+0x70/0xe0
[ 0.041042] [c00000000177fb80] [c000000000476514] kmem_cache_alloc+0x474/0x520
[ 0.041066] [c00000000177fc00] [c0000000004e394c] __d_alloc+0x4c/0x2e0
[ 0.041109] [c00000000177fc50] [c0000000004e40ac] d_make_root+0x3c/0xa0
[ 0.041142] [c00000000177fc80] [c000000000679ce0] ramfs_fill_super+0x80/0xb0
[ 0.041186] [c00000000177fcb0] [c0000000004c1b04] get_tree_nodev+0xb4/0x130
[ 0.041230] [c00000000177fcf0] [c000000000679578] ramfs_get_tree+0x28/0x40
[ 0.041282] [c00000000177fd10] [c0000000004bee78] vfs_get_tree+0x48/0x120
[ 0.041325] [c00000000177fd80] [c0000000004f7fe0] vfs_kern_mount.part.0+0xd0/0x130
[ 0.041368] [c00000000177fdc0] [c000000001366700] mnt_init+0x1c8/0x2fc
[ 0.041420] [c00000000177fe60] [c000000001366178] vfs_caches_init+0x110/0x138
[ 0.041454] [c00000000177fee0] [c000000001331020] start_kernel+0x6d8/0x780
[ 0.041497] [c00000000177ff90] [c00000000000d354] start_here_common+0x1c/0x5c8
[ 0.041539] Instruction dump:
[ 0.041555] e9490002 394a0001 91490000 e90d0028 3d42ffcc 394a4730 7d0a42aa e9490002
[ 0.041608] 2c280000 394affff 91490000 4082ff30 <0fe00000> 892d0988 39400001 994d0988
[ 0.041660] irq event stamp: 555
[ 0.041674] hardirqs last enabled at (553): [<c00000000047654c>] kmem_cache_alloc+0x4ac/0x520
[ 0.041707] hardirqs last disabled at (554): [<c000000000017368>] arch_local_irq_restore+0x68/0xe0
[ 0.041750] softirqs last enabled at (0): [<0000000000000000>] 0x0
[ 0.041778] softirqs last disabled at (555): [<c000000000016fd0>] replay_soft_interrupts+0x10/0x340
[ 0.041824] ---[ end trace aa6f9769e07e43db ]---
And lots and lots of these, or similar:
[ 14.369838] =============================
[ 14.369839] WARNING: suspicious RCU usage
[ 14.369841] 5.11.0-rc2-00008-g4899f32e4f2a #1 Tainted: G W
[ 14.369843] -----------------------------
[ 14.369844] include/linux/rcupdate.h:692 rcu_read_unlock() used illegally while idle!
[ 14.369846]
other info that might help us debug this:
[ 14.369848]
rcu_scheduler_active = 2, debug_locks = 1
[ 14.369850] RCU used illegally from extended quiescent state!
[ 14.369851] 2 locks held by swapper/32/0:
[ 14.369853] #0: c0000000015e6fc0 (rcu_callback){....}-{0:0}, at: rcu_core+0x2e0/0x990
[ 14.369864] #1: c0000000015e6f30 (rcu_read_lock){....}-{1:3}, at: kmem_cache_free+0x7cc/0x7e0
[ 14.369874]
stack backtrace:
[ 14.369876] CPU: 32 PID: 0 Comm: swapper/32 Tainted: G W 5.11.0-rc2-00008-g4899f32e4f2a #1
[ 14.369879] Call Trace:
[ 14.369880] [c000001fff557c10] [c0000000008630b8] dump_stack+0xec/0x144 (unreliable)
[ 14.369886] [c000001fff557c60] [c0000000001ad2d0] lockdep_rcu_suspicious+0x124/0x144
[ 14.369890] [c000001fff557cf0] [c00000000047783c] kmem_cache_free+0x2ac/0x7e0
[ 14.369894] [c000001fff557db0] [c0000000004bdeac] file_free_rcu+0x5c/0xa0
[ 14.369898] [c000001fff557de0] [c0000000001e214c] rcu_core+0x33c/0x990
[ 14.369902] [c000001fff557e90] [c000000000f496d0] __do_softirq+0x180/0x688
[ 14.369906] [c000001fff557f90] [c0000000000307bc] call_do_softirq+0x14/0x24
[ 14.369911] [c000000002e1fab0] [c000000000017418] do_softirq_own_stack+0x38/0x50
[ 14.369916] [c000000002e1fad0] [c000000000114a60] do_softirq+0x120/0x130
[ 14.369920] [c000000002e1fb00] [c000000000114c64] __local_bh_enable_ip+0x1f4/0x210
[ 14.369924] [c000000002e1fb40] [c0000000000172a0] replay_soft_interrupts+0x2e0/0x340
[ 14.369928] [c000000002e1fd40] [c000000000017370] arch_local_irq_restore+0x70/0xe0
[ 14.369933] [c000000002e1fd70] [c000000000c87184] snooze_loop+0x64/0x2e4
[ 14.369937] [c000000002e1fdb0] [c000000000c84204] cpuidle_enter_state+0x2e4/0x550
[ 14.369941] [c000000002e1fe10] [c000000000c8450c] cpuidle_enter+0x4c/0x70
[ 14.369946] [c000000002e1fe50] [c00000000016892c] call_cpuidle+0x4c/0x90
[ 14.369949] [c000000002e1fe70] [c000000000168d74] do_idle+0x2f4/0x380
[ 14.369953] [c000000002e1ff10] [c000000000169208] cpu_startup_entry+0x38/0x40
[ 14.369957] [c000000002e1ff40] [c000000000053484] start_secondary+0x2a4/0x2b0
[ 14.369961] [c000000002e1ff90] [c00000000000d254] start_secondary_prolog+0x10/0x14
cheers
^ permalink raw reply
* Re: [PATCH] PCI: dwc: layerscape: convert to builtin_platform_driver()
From: Geert Uytterhoeven @ 2021-01-21 11:01 UTC (permalink / raw)
To: Saravana Kannan
Cc: Roy Zang, Lorenzo Pieralisi, PCI, LKML, Minghuan Lian,
Michael Walle, linux-arm-kernel, Greg Kroah-Hartman,
Bjorn Helgaas, linuxppc-dev, Mingkai Hu
In-Reply-To: <CAGETcx8_6Hp+MWFOhRohXwdWFSfCc7A=zpb5QYNHZE5zv0bDig@mail.gmail.com>
Hi Saravana,
On Thu, Jan 21, 2021 at 1:05 AM Saravana Kannan <saravanak@google.com> wrote:
> On Wed, Jan 20, 2021 at 3:53 PM Michael Walle <michael@walle.cc> wrote:
> > Am 2021-01-20 20:47, schrieb Saravana Kannan:
> > > On Wed, Jan 20, 2021 at 11:28 AM Michael Walle <michael@walle.cc>
> > > wrote:
> > >>
> > >> [RESEND, fat-fingered the buttons of my mail client and converted
> > >> all CCs to BCCs :(]
> > >>
> > >> Am 2021-01-20 20:02, schrieb Saravana Kannan:
> > >> > On Wed, Jan 20, 2021 at 6:24 AM Rob Herring <robh@kernel.org> wrote:
> > >> >>
> > >> >> On Wed, Jan 20, 2021 at 4:53 AM Michael Walle <michael@walle.cc>
> > >> >> wrote:
> > >> >> >
> > >> >> > fw_devlink will defer the probe until all suppliers are ready. We can't
> > >> >> > use builtin_platform_driver_probe() because it doesn't retry after probe
> > >> >> > deferral. Convert it to builtin_platform_driver().
> > >> >>
> > >> >> If builtin_platform_driver_probe() doesn't work with fw_devlink, then
> > >> >> shouldn't it be fixed or removed?
> > >> >
> > >> > I was actually thinking about this too. The problem with fixing
> > >> > builtin_platform_driver_probe() to behave like
> > >> > builtin_platform_driver() is that these probe functions could be
> > >> > marked with __init. But there are also only 20 instances of
> > >> > builtin_platform_driver_probe() in the kernel:
> > >> > $ git grep ^builtin_platform_driver_probe | wc -l
> > >> > 20
> > >> >
> > >> > So it might be easier to just fix them to not use
> > >> > builtin_platform_driver_probe().
> > >> >
> > >> > Michael,
> > >> >
> > >> > Any chance you'd be willing to help me by converting all these to
> > >> > builtin_platform_driver() and delete builtin_platform_driver_probe()?
> > >>
> > >> If it just moving the probe function to the _driver struct and
> > >> remove the __init annotations. I could look into that.
> > >
> > > Yup. That's pretty much it AFAICT.
> > >
> > > builtin_platform_driver_probe() also makes sure the driver doesn't ask
> > > for async probe, etc. But I doubt anyone is actually setting async
> > > flags and still using builtin_platform_driver_probe().
> >
> > Hasn't module_platform_driver_probe() the same problem? And there
> > are ~80 drivers which uses that.
>
> Yeah. The biggest problem with all of these is the __init markers.
> Maybe some familiar with coccinelle can help?
And dropping them will increase memory usage.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
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
^ permalink raw reply
* Re: [PATCH 02/13] module: add a module_loaded helper
From: Christophe Leroy @ 2021-01-21 10:17 UTC (permalink / raw)
To: David Laight, linuxppc-dev@lists.ozlabs.org, Christoph Hellwig
In-Reply-To: <39a4c883684c418ba324c3db702802b6@AcuMS.aculab.com>
Le 21/01/2021 à 11:13, David Laight a écrit :
> From: Christophe Leroy
Sorry I hit "Reply to the list" instead of "Reply all"
Cc Christoph H. who is the author.
>> Sent: 21 January 2021 10:00
>>
>> Le 21/01/2021 à 08:49, Christoph Hellwig a écrit :
>>> Add a helper that takes modules_mutex and uses find_module to check if a
>>> given module is loaded. This provides a better abstraction for the two
>>> callers, and allows to unexport modules_mutex and find_module.
>>>
>>> Signed-off-by: Christoph Hellwig <hch@lst.de>
>>> ---
>>> drivers/gpu/drm/drm_fb_helper.c | 7 +------
>>> include/linux/module.h | 3 +++
>>> kernel/module.c | 14 ++++++++++++--
>>> kernel/trace/trace_kprobe.c | 4 +---
>>> 4 files changed, 17 insertions(+), 11 deletions(-)
>>>
>>
>>> diff --git a/include/linux/module.h b/include/linux/module.h
>>> index 7a0bcb5b1ffccd..b4654f8a408134 100644
>>> --- a/include/linux/module.h
>>> +++ b/include/linux/module.h
>>> @@ -589,6 +589,9 @@ static inline bool within_module(unsigned long addr, const struct module *mod)
>>> /* Search for module by name: must hold module_mutex. */
>>> struct module *find_module(const char *name);
>>>
>>> +/* Check if a module is loaded. */
>>> +bool module_loaded(const char *name);
>>
>> Maybe module_is_loaded() would be a better name.
>
> I can't see the original patch.
You have it there
https://patchwork.ozlabs.org/project/linuxppc-dev/patch/20210121074959.313333-3-hch@lst.de/
>
> What is the point of the function.
> By the time it returns the information is stale - so mostly useless.
>
> Surely you need to use try_module_get() instead?
>
> David
>
> -
> Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
> Registration No: 1397386 (Wales)
>
^ permalink raw reply
* RE: [PATCH 02/13] module: add a module_loaded helper
From: David Laight @ 2021-01-21 10:13 UTC (permalink / raw)
To: 'Christophe Leroy', linuxppc-dev@lists.ozlabs.org
In-Reply-To: <9052b54a-e05a-1534-9e0f-c73c8b3509bd@csgroup.eu>
From: Christophe Leroy
> Sent: 21 January 2021 10:00
>
> Le 21/01/2021 à 08:49, Christoph Hellwig a écrit :
> > Add a helper that takes modules_mutex and uses find_module to check if a
> > given module is loaded. This provides a better abstraction for the two
> > callers, and allows to unexport modules_mutex and find_module.
> >
> > Signed-off-by: Christoph Hellwig <hch@lst.de>
> > ---
> > drivers/gpu/drm/drm_fb_helper.c | 7 +------
> > include/linux/module.h | 3 +++
> > kernel/module.c | 14 ++++++++++++--
> > kernel/trace/trace_kprobe.c | 4 +---
> > 4 files changed, 17 insertions(+), 11 deletions(-)
> >
>
> > diff --git a/include/linux/module.h b/include/linux/module.h
> > index 7a0bcb5b1ffccd..b4654f8a408134 100644
> > --- a/include/linux/module.h
> > +++ b/include/linux/module.h
> > @@ -589,6 +589,9 @@ static inline bool within_module(unsigned long addr, const struct module *mod)
> > /* Search for module by name: must hold module_mutex. */
> > struct module *find_module(const char *name);
> >
> > +/* Check if a module is loaded. */
> > +bool module_loaded(const char *name);
>
> Maybe module_is_loaded() would be a better name.
I can't see the original patch.
What is the point of the function.
By the time it returns the information is stale - so mostly useless.
Surely you need to use try_module_get() instead?
David
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
^ permalink raw reply
* Re: [PATCH 1/2] crypto: talitos - Work around SEC6 ERRATA (AES-CTR mode data size error)
From: Christophe Leroy @ 2021-01-21 10:12 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: Linux Crypto Mailing List, Linux Kernel Mailing List,
open list:LINUX FOR POWERPC (32-BIT AND 64-BIT), Herbert Xu,
David S. Miller
In-Reply-To: <CAMj1kXFNZba9T45RaB_W58Z+4sdAyUDVJM_ZZPk+Y6Mf9DZUQw@mail.gmail.com>
Le 21/01/2021 à 11:02, Ard Biesheuvel a écrit :
> On Thu, 21 Jan 2021 at 10:54, Christophe Leroy
> <christophe.leroy@csgroup.eu> wrote:
>>
>>
>>
>> Le 21/01/2021 à 08:31, Ard Biesheuvel a écrit :
>>> On Thu, 21 Jan 2021 at 06:35, Christophe Leroy
>>> <christophe.leroy@csgroup.eu> wrote:
>>>>
>>>>
>>>>
>>>> Le 20/01/2021 à 23:23, Ard Biesheuvel a écrit :
>>>>> On Wed, 20 Jan 2021 at 19:59, Christophe Leroy
>>>>> <christophe.leroy@csgroup.eu> wrote:
>>>>>>
>>>>>> Talitos Security Engine AESU considers any input
>>>>>> data size that is not a multiple of 16 bytes to be an error.
>>>>>> This is not a problem in general, except for Counter mode
>>>>>> that is a stream cipher and can have an input of any size.
>>>>>>
>>>>>> Test Manager for ctr(aes) fails on 4th test vector which has
>>>>>> a length of 499 while all previous vectors which have a 16 bytes
>>>>>> multiple length succeed.
>>>>>>
>>>>>> As suggested by Freescale, round up the input data length to the
>>>>>> nearest 16 bytes.
>>>>>>
>>>>>> Fixes: 5e75ae1b3cef ("crypto: talitos - add new crypto modes")
>>>>>> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
>>>>>
>>>>> Doesn't this cause the hardware to write outside the given buffer?
>>>>
>>>>
>>>> Only the input length is modified. Not the output length.
>>>>
>>>> The ERRATA says:
>>>>
>>>> The input data length (in the descriptor) can be rounded up to the nearest 16B. Set the
>>>> data-in length (in the descriptor) to include X bytes of data beyond the payload. Set the
>>>> data-out length to only output the relevant payload (don't need to output the padding).
>>>> SEC reads from memory are not destructive, so the extra bytes included in the AES-CTR
>>>> operation can be whatever bytes are contiguously trailing the payload.
>>>
>>> So what happens if the input is not 16 byte aligned, and rounding it
>>> up causes it to extend across a page boundary into a page that is not
>>> mapped by the IOMMU/SMMU?
>>>
>>
>> What is the IOMMU/SMMU ?
>>
>> The mpc8xx, mpc82xx and mpc83xx which embed the Talitos Security Engine don't have such thing, the
>> security engine uses DMA and has direct access to the memory bus for reading and writing.
>>
>
> OK, good. So the only case where this could break is when the DMA
> access spills over into a page that does not exist, and I suppose this
> could only happen if the transfer involves a buffer located at the
> very top of DRAM, right?
>
Right.
Christophe
^ permalink raw reply
* Re: [PATCH 1/2] crypto: talitos - Work around SEC6 ERRATA (AES-CTR mode data size error)
From: Ard Biesheuvel @ 2021-01-21 10:02 UTC (permalink / raw)
To: Christophe Leroy
Cc: Linux Crypto Mailing List, Linux Kernel Mailing List,
open list:LINUX FOR POWERPC (32-BIT AND 64-BIT), Herbert Xu,
David S. Miller
In-Reply-To: <ecdd07b3-afca-7e26-b6b6-3a3a985bc5a1@csgroup.eu>
On Thu, 21 Jan 2021 at 10:54, Christophe Leroy
<christophe.leroy@csgroup.eu> wrote:
>
>
>
> Le 21/01/2021 à 08:31, Ard Biesheuvel a écrit :
> > On Thu, 21 Jan 2021 at 06:35, Christophe Leroy
> > <christophe.leroy@csgroup.eu> wrote:
> >>
> >>
> >>
> >> Le 20/01/2021 à 23:23, Ard Biesheuvel a écrit :
> >>> On Wed, 20 Jan 2021 at 19:59, Christophe Leroy
> >>> <christophe.leroy@csgroup.eu> wrote:
> >>>>
> >>>> Talitos Security Engine AESU considers any input
> >>>> data size that is not a multiple of 16 bytes to be an error.
> >>>> This is not a problem in general, except for Counter mode
> >>>> that is a stream cipher and can have an input of any size.
> >>>>
> >>>> Test Manager for ctr(aes) fails on 4th test vector which has
> >>>> a length of 499 while all previous vectors which have a 16 bytes
> >>>> multiple length succeed.
> >>>>
> >>>> As suggested by Freescale, round up the input data length to the
> >>>> nearest 16 bytes.
> >>>>
> >>>> Fixes: 5e75ae1b3cef ("crypto: talitos - add new crypto modes")
> >>>> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
> >>>
> >>> Doesn't this cause the hardware to write outside the given buffer?
> >>
> >>
> >> Only the input length is modified. Not the output length.
> >>
> >> The ERRATA says:
> >>
> >> The input data length (in the descriptor) can be rounded up to the nearest 16B. Set the
> >> data-in length (in the descriptor) to include X bytes of data beyond the payload. Set the
> >> data-out length to only output the relevant payload (don't need to output the padding).
> >> SEC reads from memory are not destructive, so the extra bytes included in the AES-CTR
> >> operation can be whatever bytes are contiguously trailing the payload.
> >
> > So what happens if the input is not 16 byte aligned, and rounding it
> > up causes it to extend across a page boundary into a page that is not
> > mapped by the IOMMU/SMMU?
> >
>
> What is the IOMMU/SMMU ?
>
> The mpc8xx, mpc82xx and mpc83xx which embed the Talitos Security Engine don't have such thing, the
> security engine uses DMA and has direct access to the memory bus for reading and writing.
>
OK, good. So the only case where this could break is when the DMA
access spills over into a page that does not exist, and I suppose this
could only happen if the transfer involves a buffer located at the
very top of DRAM, right?
^ permalink raw reply
* Re: [PATCH 02/13] module: add a module_loaded helper
From: Christophe Leroy @ 2021-01-21 10:00 UTC (permalink / raw)
To: Christoph Hellwig, Frederic Barrat, Andrew Donnellan,
Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Daniel Vetter, Jessica Yu, Josh Poimboeuf, Jiri Kosina,
Miroslav Benes, Petr Mladek, Joe Lawrence
Cc: Michal Marek, linux-kbuild, Masahiro Yamada, linux-kernel,
dri-devel, live-patching, linuxppc-dev
In-Reply-To: <20210121074959.313333-3-hch@lst.de>
Le 21/01/2021 à 08:49, Christoph Hellwig a écrit :
> Add a helper that takes modules_mutex and uses find_module to check if a
> given module is loaded. This provides a better abstraction for the two
> callers, and allows to unexport modules_mutex and find_module.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
> drivers/gpu/drm/drm_fb_helper.c | 7 +------
> include/linux/module.h | 3 +++
> kernel/module.c | 14 ++++++++++++--
> kernel/trace/trace_kprobe.c | 4 +---
> 4 files changed, 17 insertions(+), 11 deletions(-)
>
> diff --git a/include/linux/module.h b/include/linux/module.h
> index 7a0bcb5b1ffccd..b4654f8a408134 100644
> --- a/include/linux/module.h
> +++ b/include/linux/module.h
> @@ -589,6 +589,9 @@ static inline bool within_module(unsigned long addr, const struct module *mod)
> /* Search for module by name: must hold module_mutex. */
> struct module *find_module(const char *name);
> +/* Check if a module is loaded. */
> +bool module_loaded(const char *name);
Maybe module_is_loaded() would be a better name.
^ permalink raw reply
* Re: [PATCH 02/13] module: add a module_loaded helper
From: Christophe Leroy @ 2021-01-21 9:59 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <20210121074959.313333-3-hch@lst.de>
Le 21/01/2021 à 08:49, Christoph Hellwig a écrit :
> Add a helper that takes modules_mutex and uses find_module to check if a
> given module is loaded. This provides a better abstraction for the two
> callers, and allows to unexport modules_mutex and find_module.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
> drivers/gpu/drm/drm_fb_helper.c | 7 +------
> include/linux/module.h | 3 +++
> kernel/module.c | 14 ++++++++++++--
> kernel/trace/trace_kprobe.c | 4 +---
> 4 files changed, 17 insertions(+), 11 deletions(-)
>
> diff --git a/include/linux/module.h b/include/linux/module.h
> index 7a0bcb5b1ffccd..b4654f8a408134 100644
> --- a/include/linux/module.h
> +++ b/include/linux/module.h
> @@ -589,6 +589,9 @@ static inline bool within_module(unsigned long addr, const struct module *mod)
> /* Search for module by name: must hold module_mutex. */
> struct module *find_module(const char *name);
>
> +/* Check if a module is loaded. */
> +bool module_loaded(const char *name);
Maybe module_is_loaded() would be a better name.
^ permalink raw reply
* Re: [PATCH 1/2] crypto: talitos - Work around SEC6 ERRATA (AES-CTR mode data size error)
From: Christophe Leroy @ 2021-01-21 9:54 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: Linux Crypto Mailing List, Linux Kernel Mailing List,
open list:LINUX FOR POWERPC (32-BIT AND 64-BIT), Herbert Xu,
David S. Miller
In-Reply-To: <CAMj1kXHz8LdDgfOcifcB-MBMM9-TbymOU_psT3JBFQfyvQ=EjQ@mail.gmail.com>
Le 21/01/2021 à 08:31, Ard Biesheuvel a écrit :
> On Thu, 21 Jan 2021 at 06:35, Christophe Leroy
> <christophe.leroy@csgroup.eu> wrote:
>>
>>
>>
>> Le 20/01/2021 à 23:23, Ard Biesheuvel a écrit :
>>> On Wed, 20 Jan 2021 at 19:59, Christophe Leroy
>>> <christophe.leroy@csgroup.eu> wrote:
>>>>
>>>> Talitos Security Engine AESU considers any input
>>>> data size that is not a multiple of 16 bytes to be an error.
>>>> This is not a problem in general, except for Counter mode
>>>> that is a stream cipher and can have an input of any size.
>>>>
>>>> Test Manager for ctr(aes) fails on 4th test vector which has
>>>> a length of 499 while all previous vectors which have a 16 bytes
>>>> multiple length succeed.
>>>>
>>>> As suggested by Freescale, round up the input data length to the
>>>> nearest 16 bytes.
>>>>
>>>> Fixes: 5e75ae1b3cef ("crypto: talitos - add new crypto modes")
>>>> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
>>>
>>> Doesn't this cause the hardware to write outside the given buffer?
>>
>>
>> Only the input length is modified. Not the output length.
>>
>> The ERRATA says:
>>
>> The input data length (in the descriptor) can be rounded up to the nearest 16B. Set the
>> data-in length (in the descriptor) to include X bytes of data beyond the payload. Set the
>> data-out length to only output the relevant payload (don't need to output the padding).
>> SEC reads from memory are not destructive, so the extra bytes included in the AES-CTR
>> operation can be whatever bytes are contiguously trailing the payload.
>
> So what happens if the input is not 16 byte aligned, and rounding it
> up causes it to extend across a page boundary into a page that is not
> mapped by the IOMMU/SMMU?
>
What is the IOMMU/SMMU ?
The mpc8xx, mpc82xx and mpc83xx which embed the Talitos Security Engine don't have such thing, the
security engine uses DMA and has direct access to the memory bus for reading and writing.
Christophe
^ permalink raw reply
* Re: [PATCH 01/13] powerpc/powernv: remove get_cxl_module
From: Andrew Donnellan @ 2021-01-21 9:09 UTC (permalink / raw)
To: Christoph Hellwig, Frederic Barrat, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, David Airlie, Daniel Vetter,
Jessica Yu, Josh Poimboeuf, Jiri Kosina, Miroslav Benes,
Petr Mladek, Joe Lawrence
Cc: Michal Marek, linux-kbuild, Masahiro Yamada, linux-kernel,
dri-devel, live-patching, linuxppc-dev
In-Reply-To: <20210121074959.313333-2-hch@lst.de>
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
^ permalink raw reply
* Re: [PATCH 08/13] drm: remove drm_fb_helper_modinit
From: Daniel Vetter @ 2021-01-21 8:39 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Petr Mladek, Jiri Kosina, Andrew Donnellan, linux-kbuild,
David Airlie, Masahiro Yamada, Josh Poimboeuf, Maarten Lankhorst,
Linux Kernel Mailing List, Maxime Ripard, Michal Marek,
Joe Lawrence, dri-devel, Thomas Zimmermann, Jessica Yu,
Frederic Barrat, live-patching, Miroslav Benes, linuxppc-dev
In-Reply-To: <20210121082820.GA25719@lst.de>
On Thu, Jan 21, 2021 at 9:28 AM Christoph Hellwig <hch@lst.de> wrote:
>
> On Thu, Jan 21, 2021 at 09:25:40AM +0100, Daniel Vetter wrote:
> > On Thu, Jan 21, 2021 at 8:55 AM Christoph Hellwig <hch@lst.de> wrote:
> > >
> > > drm_fb_helper_modinit has a lot of boilerplate for what is not very
> > > simple functionality. Just open code it in the only caller using
> > > IS_ENABLED and IS_MODULE.
> > >
> > > Signed-off-by: Christoph Hellwig <hch@lst.de>
> >
> > I didn't spot any dependencies with your series, should I just merge
> > this through drm trees? Or do you want an ack?
>
> I'd prefer an ACK - module_loaded() is only introduced earlier in this
> series.
I was looking for that but didn't find the hunk touching drm somehow ...
Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Cheers, Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
^ permalink raw reply
* Re: [PATCH 08/13] drm: remove drm_fb_helper_modinit
From: Daniel Vetter @ 2021-01-21 8:25 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Petr Mladek, Jiri Kosina, Andrew Donnellan, linux-kbuild,
David Airlie, Masahiro Yamada, Josh Poimboeuf, Maarten Lankhorst,
Linux Kernel Mailing List, Maxime Ripard, Michal Marek,
Joe Lawrence, dri-devel, Thomas Zimmermann, Jessica Yu,
Frederic Barrat, live-patching, Miroslav Benes, linuxppc-dev
In-Reply-To: <20210121074959.313333-9-hch@lst.de>
On Thu, Jan 21, 2021 at 8:55 AM Christoph Hellwig <hch@lst.de> wrote:
>
> drm_fb_helper_modinit has a lot of boilerplate for what is not very
> simple functionality. Just open code it in the only caller using
> IS_ENABLED and IS_MODULE.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
I didn't spot any dependencies with your series, should I just merge
this through drm trees? Or do you want an ack?
-Daniel
> ---
> drivers/gpu/drm/drm_crtc_helper_internal.h | 10 ---------
> drivers/gpu/drm/drm_fb_helper.c | 16 -------------
> drivers/gpu/drm/drm_kms_helper_common.c | 26 +++++++++++-----------
> 3 files changed, 13 insertions(+), 39 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_crtc_helper_internal.h b/drivers/gpu/drm/drm_crtc_helper_internal.h
> index 25ce42e799952c..61e09f8a8d0ff0 100644
> --- a/drivers/gpu/drm/drm_crtc_helper_internal.h
> +++ b/drivers/gpu/drm/drm_crtc_helper_internal.h
> @@ -32,16 +32,6 @@
> #include <drm/drm_encoder.h>
> #include <drm/drm_modes.h>
>
> -/* drm_fb_helper.c */
> -#ifdef CONFIG_DRM_FBDEV_EMULATION
> -int drm_fb_helper_modinit(void);
> -#else
> -static inline int drm_fb_helper_modinit(void)
> -{
> - return 0;
> -}
> -#endif
> -
> /* drm_dp_aux_dev.c */
> #ifdef CONFIG_DRM_DP_AUX_CHARDEV
> int drm_dp_aux_dev_init(void);
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index ce6d63ca75c32a..0b9f1ae1b7864c 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -2499,19 +2499,3 @@ void drm_fbdev_generic_setup(struct drm_device *dev,
> drm_client_register(&fb_helper->client);
> }
> EXPORT_SYMBOL(drm_fbdev_generic_setup);
> -
> -/* The Kconfig DRM_KMS_HELPER selects FRAMEBUFFER_CONSOLE (if !EXPERT)
> - * but the module doesn't depend on any fb console symbols. At least
> - * attempt to load fbcon to avoid leaving the system without a usable console.
> - */
> -int __init drm_fb_helper_modinit(void)
> -{
> -#if defined(CONFIG_FRAMEBUFFER_CONSOLE_MODULE) && !defined(CONFIG_EXPERT)
> - const char name[] = "fbcon";
> -
> - if (!module_loaded(name))
> - request_module_nowait(name);
> -#endif
> - return 0;
> -}
> -EXPORT_SYMBOL(drm_fb_helper_modinit);
> diff --git a/drivers/gpu/drm/drm_kms_helper_common.c b/drivers/gpu/drm/drm_kms_helper_common.c
> index 221a8528c9937a..b694a7da632eae 100644
> --- a/drivers/gpu/drm/drm_kms_helper_common.c
> +++ b/drivers/gpu/drm/drm_kms_helper_common.c
> @@ -64,19 +64,19 @@ MODULE_PARM_DESC(edid_firmware,
>
> static int __init drm_kms_helper_init(void)
> {
> - int ret;
> -
> - /* Call init functions from specific kms helpers here */
> - ret = drm_fb_helper_modinit();
> - if (ret < 0)
> - goto out;
> -
> - ret = drm_dp_aux_dev_init();
> - if (ret < 0)
> - goto out;
> -
> -out:
> - return ret;
> + /*
> + * The Kconfig DRM_KMS_HELPER selects FRAMEBUFFER_CONSOLE (if !EXPERT)
> + * but the module doesn't depend on any fb console symbols. At least
> + * attempt to load fbcon to avoid leaving the system without a usable
> + * console.
> + */
> + if (IS_ENABLED(CONFIG_DRM_FBDEV_EMULATION) &&
> + IS_MODULE(CONFIG_FRAMEBUFFER_CONSOLE) &&
> + !IS_ENABLED(CONFIG_EXPERT) &&
> + !module_loaded("fbcon"))
> + request_module_nowait("fbcon");
> +
> + return drm_dp_aux_dev_init();
> }
>
> static void __exit drm_kms_helper_exit(void)
> --
> 2.29.2
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
^ permalink raw reply
* Re: [PATCH 08/13] drm: remove drm_fb_helper_modinit
From: Christoph Hellwig @ 2021-01-21 8:28 UTC (permalink / raw)
To: Daniel Vetter
Cc: Petr Mladek, Jiri Kosina, Andrew Donnellan, linux-kbuild,
David Airlie, Masahiro Yamada, Josh Poimboeuf, Maarten Lankhorst,
Linux Kernel Mailing List, Maxime Ripard, Michal Marek,
Joe Lawrence, dri-devel, Thomas Zimmermann, Jessica Yu,
Frederic Barrat, live-patching, Miroslav Benes, linuxppc-dev,
Christoph Hellwig
In-Reply-To: <CAKMK7uFo3epNAUdcp0vvW=VyWMMTZghGyRTPbz_Z37S6nem_2A@mail.gmail.com>
On Thu, Jan 21, 2021 at 09:25:40AM +0100, Daniel Vetter wrote:
> On Thu, Jan 21, 2021 at 8:55 AM Christoph Hellwig <hch@lst.de> wrote:
> >
> > drm_fb_helper_modinit has a lot of boilerplate for what is not very
> > simple functionality. Just open code it in the only caller using
> > IS_ENABLED and IS_MODULE.
> >
> > Signed-off-by: Christoph Hellwig <hch@lst.de>
>
> I didn't spot any dependencies with your series, should I just merge
> this through drm trees? Or do you want an ack?
I'd prefer an ACK - module_loaded() is only introduced earlier in this
series.
^ permalink raw reply
* [PATCH 13/13] module: remove EXPORY_UNUSED_SYMBOL*
From: Christoph Hellwig @ 2021-01-21 7:49 UTC (permalink / raw)
To: Frederic Barrat, Andrew Donnellan, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, David Airlie, Daniel Vetter,
Jessica Yu, Josh Poimboeuf, Jiri Kosina, Miroslav Benes,
Petr Mladek, Joe Lawrence
Cc: Michal Marek, linux-kbuild, Masahiro Yamada, linux-kernel,
dri-devel, live-patching, linuxppc-dev
In-Reply-To: <20210121074959.313333-1-hch@lst.de>
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>
---
arch/arm/configs/bcm2835_defconfig | 1 -
arch/arm/configs/mxs_defconfig | 1 -
arch/mips/configs/nlm_xlp_defconfig | 1 -
arch/mips/configs/nlm_xlr_defconfig | 1 -
arch/parisc/configs/generic-32bit_defconfig | 1 -
arch/parisc/configs/generic-64bit_defconfig | 1 -
arch/powerpc/configs/ppc6xx_defconfig | 1 -
arch/s390/configs/debug_defconfig | 1 -
arch/s390/configs/defconfig | 1 -
arch/sh/configs/edosk7760_defconfig | 1 -
arch/sh/configs/sdk7780_defconfig | 1 -
arch/x86/configs/i386_defconfig | 1 -
arch/x86/configs/x86_64_defconfig | 1 -
arch/x86/tools/relocs.c | 4 +-
include/asm-generic/vmlinux.lds.h | 28 ---------
include/linux/export.h | 8 ---
include/linux/module.h | 13 ----
init/Kconfig | 17 -----
kernel/module.c | 69 ++-------------------
scripts/checkpatch.pl | 6 +-
scripts/mod/modpost.c | 39 +-----------
scripts/mod/modpost.h | 2 -
scripts/module.lds.S | 4 --
tools/include/linux/export.h | 2 -
24 files changed, 13 insertions(+), 192 deletions(-)
diff --git a/arch/arm/configs/bcm2835_defconfig b/arch/arm/configs/bcm2835_defconfig
index 44ff9cd88d8161..d6c6c2e031c43a 100644
--- a/arch/arm/configs/bcm2835_defconfig
+++ b/arch/arm/configs/bcm2835_defconfig
@@ -177,7 +177,6 @@ CONFIG_BOOT_PRINTK_DELAY=y
CONFIG_DYNAMIC_DEBUG=y
CONFIG_DEBUG_INFO=y
# CONFIG_ENABLE_MUST_CHECK is not set
-CONFIG_UNUSED_SYMBOLS=y
CONFIG_DEBUG_MEMORY_INIT=y
CONFIG_LOCKUP_DETECTOR=y
CONFIG_SCHED_TRACER=y
diff --git a/arch/arm/configs/mxs_defconfig b/arch/arm/configs/mxs_defconfig
index a9c6f32a9b1c9d..ca32446b187f5d 100644
--- a/arch/arm/configs/mxs_defconfig
+++ b/arch/arm/configs/mxs_defconfig
@@ -164,7 +164,6 @@ CONFIG_FONTS=y
CONFIG_PRINTK_TIME=y
CONFIG_DEBUG_INFO=y
CONFIG_FRAME_WARN=2048
-CONFIG_UNUSED_SYMBOLS=y
CONFIG_MAGIC_SYSRQ=y
CONFIG_DEBUG_KERNEL=y
CONFIG_SOFTLOCKUP_DETECTOR=y
diff --git a/arch/mips/configs/nlm_xlp_defconfig b/arch/mips/configs/nlm_xlp_defconfig
index 72a211d2d556fd..32c29061172325 100644
--- a/arch/mips/configs/nlm_xlp_defconfig
+++ b/arch/mips/configs/nlm_xlp_defconfig
@@ -549,7 +549,6 @@ CONFIG_PRINTK_TIME=y
CONFIG_DEBUG_INFO=y
# CONFIG_ENABLE_MUST_CHECK is not set
CONFIG_FRAME_WARN=1024
-CONFIG_UNUSED_SYMBOLS=y
CONFIG_DEBUG_MEMORY_INIT=y
CONFIG_DETECT_HUNG_TASK=y
CONFIG_SCHEDSTATS=y
diff --git a/arch/mips/configs/nlm_xlr_defconfig b/arch/mips/configs/nlm_xlr_defconfig
index 4ecb157e56d427..bf9b9244929ecd 100644
--- a/arch/mips/configs/nlm_xlr_defconfig
+++ b/arch/mips/configs/nlm_xlr_defconfig
@@ -500,7 +500,6 @@ CONFIG_CRC7=m
CONFIG_PRINTK_TIME=y
CONFIG_DEBUG_INFO=y
# CONFIG_ENABLE_MUST_CHECK is not set
-CONFIG_UNUSED_SYMBOLS=y
CONFIG_DEBUG_MEMORY_INIT=y
CONFIG_DETECT_HUNG_TASK=y
CONFIG_SCHEDSTATS=y
diff --git a/arch/parisc/configs/generic-32bit_defconfig b/arch/parisc/configs/generic-32bit_defconfig
index 3cbcfad5f7249d..7611d48c599e01 100644
--- a/arch/parisc/configs/generic-32bit_defconfig
+++ b/arch/parisc/configs/generic-32bit_defconfig
@@ -22,7 +22,6 @@ CONFIG_PCI_LBA=y
CONFIG_MODULES=y
CONFIG_MODULE_UNLOAD=y
CONFIG_MODULE_FORCE_UNLOAD=y
-CONFIG_UNUSED_SYMBOLS=y
# CONFIG_BLK_DEV_BSG is not set
# CONFIG_CORE_DUMP_DEFAULT_ELF_HEADERS is not set
CONFIG_BINFMT_MISC=m
diff --git a/arch/parisc/configs/generic-64bit_defconfig b/arch/parisc/configs/generic-64bit_defconfig
index 8f81fcbf04c413..53054b81461a10 100644
--- a/arch/parisc/configs/generic-64bit_defconfig
+++ b/arch/parisc/configs/generic-64bit_defconfig
@@ -31,7 +31,6 @@ CONFIG_MODULE_FORCE_LOAD=y
CONFIG_MODULE_UNLOAD=y
CONFIG_MODULE_FORCE_UNLOAD=y
CONFIG_MODVERSIONS=y
-CONFIG_UNUSED_SYMBOLS=y
CONFIG_BLK_DEV_INTEGRITY=y
CONFIG_BINFMT_MISC=m
# CONFIG_COMPACTION is not set
diff --git a/arch/powerpc/configs/ppc6xx_defconfig b/arch/powerpc/configs/ppc6xx_defconfig
index ef09f3cce1fa85..34c3859040f9f7 100644
--- a/arch/powerpc/configs/ppc6xx_defconfig
+++ b/arch/powerpc/configs/ppc6xx_defconfig
@@ -1072,7 +1072,6 @@ CONFIG_NLS_ISO8859_15=m
CONFIG_NLS_KOI8_R=m
CONFIG_NLS_KOI8_U=m
CONFIG_DEBUG_INFO=y
-CONFIG_UNUSED_SYMBOLS=y
CONFIG_HEADERS_INSTALL=y
CONFIG_MAGIC_SYSRQ=y
CONFIG_DEBUG_KERNEL=y
diff --git a/arch/s390/configs/debug_defconfig b/arch/s390/configs/debug_defconfig
index c4f6ff98a612cd..58e54d17e3154b 100644
--- a/arch/s390/configs/debug_defconfig
+++ b/arch/s390/configs/debug_defconfig
@@ -71,7 +71,6 @@ CONFIG_MODULE_FORCE_UNLOAD=y
CONFIG_MODVERSIONS=y
CONFIG_MODULE_SRCVERSION_ALL=y
CONFIG_MODULE_SIG_SHA256=y
-CONFIG_UNUSED_SYMBOLS=y
CONFIG_BLK_DEV_INTEGRITY=y
CONFIG_BLK_DEV_THROTTLING=y
CONFIG_BLK_WBT=y
diff --git a/arch/s390/configs/defconfig b/arch/s390/configs/defconfig
index 51135893cffe34..b5e62c0d3e23e0 100644
--- a/arch/s390/configs/defconfig
+++ b/arch/s390/configs/defconfig
@@ -66,7 +66,6 @@ CONFIG_MODULE_FORCE_UNLOAD=y
CONFIG_MODVERSIONS=y
CONFIG_MODULE_SRCVERSION_ALL=y
CONFIG_MODULE_SIG_SHA256=y
-CONFIG_UNUSED_SYMBOLS=y
CONFIG_BLK_DEV_THROTTLING=y
CONFIG_BLK_WBT=y
CONFIG_BLK_CGROUP_IOLATENCY=y
diff --git a/arch/sh/configs/edosk7760_defconfig b/arch/sh/configs/edosk7760_defconfig
index 02ba622985769d..d77f54e906fd04 100644
--- a/arch/sh/configs/edosk7760_defconfig
+++ b/arch/sh/configs/edosk7760_defconfig
@@ -102,7 +102,6 @@ CONFIG_NLS_UTF8=y
CONFIG_PRINTK_TIME=y
# CONFIG_ENABLE_MUST_CHECK is not set
CONFIG_MAGIC_SYSRQ=y
-CONFIG_UNUSED_SYMBOLS=y
CONFIG_DEBUG_KERNEL=y
CONFIG_DEBUG_SHIRQ=y
CONFIG_DETECT_HUNG_TASK=y
diff --git a/arch/sh/configs/sdk7780_defconfig b/arch/sh/configs/sdk7780_defconfig
index d10a0414123a51..d53c4595fb2e98 100644
--- a/arch/sh/configs/sdk7780_defconfig
+++ b/arch/sh/configs/sdk7780_defconfig
@@ -130,7 +130,6 @@ CONFIG_NLS_ISO8859_15=y
CONFIG_NLS_UTF8=y
# CONFIG_ENABLE_MUST_CHECK is not set
CONFIG_MAGIC_SYSRQ=y
-CONFIG_UNUSED_SYMBOLS=y
CONFIG_DEBUG_KERNEL=y
CONFIG_DETECT_HUNG_TASK=y
# CONFIG_SCHED_DEBUG is not set
diff --git a/arch/x86/configs/i386_defconfig b/arch/x86/configs/i386_defconfig
index 78210793d357cf..9c9c4a888b1dbf 100644
--- a/arch/x86/configs/i386_defconfig
+++ b/arch/x86/configs/i386_defconfig
@@ -50,7 +50,6 @@ CONFIG_JUMP_LABEL=y
CONFIG_MODULES=y
CONFIG_MODULE_UNLOAD=y
CONFIG_MODULE_FORCE_UNLOAD=y
-# CONFIG_UNUSED_SYMBOLS is not set
CONFIG_BINFMT_MISC=y
CONFIG_NET=y
CONFIG_PACKET=y
diff --git a/arch/x86/configs/x86_64_defconfig b/arch/x86/configs/x86_64_defconfig
index 9936528e19393a..b60bd2d8603499 100644
--- a/arch/x86/configs/x86_64_defconfig
+++ b/arch/x86/configs/x86_64_defconfig
@@ -48,7 +48,6 @@ CONFIG_JUMP_LABEL=y
CONFIG_MODULES=y
CONFIG_MODULE_UNLOAD=y
CONFIG_MODULE_FORCE_UNLOAD=y
-# CONFIG_UNUSED_SYMBOLS is not set
CONFIG_BINFMT_MISC=y
CONFIG_NET=y
CONFIG_PACKET=y
diff --git a/arch/x86/tools/relocs.c b/arch/x86/tools/relocs.c
index 0d210d0e83e241..b9c577a3cacca6 100644
--- a/arch/x86/tools/relocs.c
+++ b/arch/x86/tools/relocs.c
@@ -61,8 +61,8 @@ static const char * const sym_regex_kernel[S_NSYMTYPES] = {
"(__iommu_table|__apicdrivers|__smp_locks)(|_end)|"
"__(start|end)_pci_.*|"
"__(start|end)_builtin_fw|"
- "__(start|stop)___ksymtab(|_gpl|_unused|_unused_gpl)|"
- "__(start|stop)___kcrctab(|_gpl|_unused|_unused_gpl)|"
+ "__(start|stop)___ksymtab(|_gpl)|"
+ "__(start|stop)___kcrctab(|_gpl)|"
"__(start|stop)___param|"
"__(start|stop)___modver|"
"__(start|stop)___bug_table|"
diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
index 83243506e68b00..1fa338ac6a5477 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -481,20 +481,6 @@
__stop___ksymtab_gpl = .; \
} \
\
- /* Kernel symbol table: Normal unused symbols */ \
- __ksymtab_unused : AT(ADDR(__ksymtab_unused) - LOAD_OFFSET) { \
- __start___ksymtab_unused = .; \
- KEEP(*(SORT(___ksymtab_unused+*))) \
- __stop___ksymtab_unused = .; \
- } \
- \
- /* Kernel symbol table: GPL-only unused symbols */ \
- __ksymtab_unused_gpl : AT(ADDR(__ksymtab_unused_gpl) - LOAD_OFFSET) { \
- __start___ksymtab_unused_gpl = .; \
- KEEP(*(SORT(___ksymtab_unused_gpl+*))) \
- __stop___ksymtab_unused_gpl = .; \
- } \
- \
/* Kernel symbol table: Normal symbols */ \
__kcrctab : AT(ADDR(__kcrctab) - LOAD_OFFSET) { \
__start___kcrctab = .; \
@@ -509,20 +495,6 @@
__stop___kcrctab_gpl = .; \
} \
\
- /* Kernel symbol table: Normal unused symbols */ \
- __kcrctab_unused : AT(ADDR(__kcrctab_unused) - LOAD_OFFSET) { \
- __start___kcrctab_unused = .; \
- KEEP(*(SORT(___kcrctab_unused+*))) \
- __stop___kcrctab_unused = .; \
- } \
- \
- /* Kernel symbol table: GPL-only unused symbols */ \
- __kcrctab_unused_gpl : AT(ADDR(__kcrctab_unused_gpl) - LOAD_OFFSET) { \
- __start___kcrctab_unused_gpl = .; \
- KEEP(*(SORT(___kcrctab_unused_gpl+*))) \
- __stop___kcrctab_unused_gpl = .; \
- } \
- \
/* Kernel symbol table: strings */ \
__ksymtab_strings : AT(ADDR(__ksymtab_strings) - LOAD_OFFSET) { \
*(__ksymtab_strings) \
diff --git a/include/linux/export.h b/include/linux/export.h
index 362b64f8d4a7c2..6271a5d9c988fa 100644
--- a/include/linux/export.h
+++ b/include/linux/export.h
@@ -160,14 +160,6 @@ struct kernel_symbol {
#define EXPORT_SYMBOL_NS(sym, ns) __EXPORT_SYMBOL(sym, "", #ns)
#define EXPORT_SYMBOL_NS_GPL(sym, ns) __EXPORT_SYMBOL(sym, "_gpl", #ns)
-#ifdef CONFIG_UNUSED_SYMBOLS
-#define EXPORT_UNUSED_SYMBOL(sym) _EXPORT_SYMBOL(sym, "_unused")
-#define EXPORT_UNUSED_SYMBOL_GPL(sym) _EXPORT_SYMBOL(sym, "_unused_gpl")
-#else
-#define EXPORT_UNUSED_SYMBOL(sym)
-#define EXPORT_UNUSED_SYMBOL_GPL(sym)
-#endif
-
#endif /* !__ASSEMBLY__ */
#endif /* _LINUX_EXPORT_H */
diff --git a/include/linux/module.h b/include/linux/module.h
index 8f4d577d4707c2..0e70596c9a704a 100644
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -392,18 +392,6 @@ struct module {
const s32 *gpl_crcs;
bool using_gplonly_symbols;
-#ifdef CONFIG_UNUSED_SYMBOLS
- /* unused exported symbols. */
- const struct kernel_symbol *unused_syms;
- const s32 *unused_crcs;
- unsigned int num_unused_syms;
-
- /* GPL-only, unused exported symbols. */
- unsigned int num_unused_gpl_syms;
- const struct kernel_symbol *unused_gpl_syms;
- const s32 *unused_gpl_crcs;
-#endif
-
#ifdef CONFIG_MODULE_SIG
/* Signature was verified. */
bool sig_ok;
@@ -592,7 +580,6 @@ struct symsearch {
GPL_ONLY,
WILL_BE_GPL_ONLY,
} license;
- bool unused;
};
/* Returns 0 and fills in value, defined and namebuf, or -ERANGE if
diff --git a/init/Kconfig b/init/Kconfig
index b77c60f8b963d4..11b803b45c1995 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -2262,25 +2262,8 @@ config MODULE_ALLOW_MISSING_NAMESPACE_IMPORTS
If unsure, say N.
-config UNUSED_SYMBOLS
- bool "Enable unused/obsolete exported symbols"
- default y if X86
- help
- Unused but exported symbols make the kernel needlessly bigger. For
- that reason most of these unused exports will soon be removed. This
- option is provided temporarily to provide a transition period in case
- some external kernel module needs one of these symbols anyway. If you
- encounter such a case in your module, consider if you are actually
- using the right API. (rationale: since nobody in the kernel is using
- this in a module, there is a pretty good chance it's actually the
- wrong interface to use). If you really need the symbol, please send a
- mail to the linux kernel mailing list mentioning the symbol and why
- you really need it, and what the merge plan to the mainline kernel for
- your module is.
-
config TRIM_UNUSED_KSYMS
bool "Trim unused exported kernel symbols"
- depends on !UNUSED_SYMBOLS
help
The kernel and some modules make many symbols available for
other modules to use via EXPORT_SYMBOL() and variants. Depending
diff --git a/kernel/module.c b/kernel/module.c
index 917fd1b5c95a42..f725ff99b64f54 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -415,14 +415,6 @@ extern const struct kernel_symbol __start___ksymtab_gpl[];
extern const struct kernel_symbol __stop___ksymtab_gpl[];
extern const s32 __start___kcrctab[];
extern const s32 __start___kcrctab_gpl[];
-#ifdef CONFIG_UNUSED_SYMBOLS
-extern const struct kernel_symbol __start___ksymtab_unused[];
-extern const struct kernel_symbol __stop___ksymtab_unused[];
-extern const struct kernel_symbol __start___ksymtab_unused_gpl[];
-extern const struct kernel_symbol __stop___ksymtab_unused_gpl[];
-extern const s32 __start___kcrctab_unused[];
-extern const s32 __start___kcrctab_unused_gpl[];
-#endif
#ifndef CONFIG_MODVERSIONS
#define symversion(base, idx) NULL
@@ -459,18 +451,6 @@ static bool check_exported_symbol(const struct symsearch *syms,
}
}
-#ifdef CONFIG_UNUSED_SYMBOLS
- if (syms->unused && fsa->warn) {
- pr_warn("Symbol %s is marked as UNUSED, however this module is "
- "using it.\n", fsa->name);
- pr_warn("This symbol will go away in the future.\n");
- pr_warn("Please evaluate if this is the right api to use and "
- "if it really is, submit a report to the linux kernel "
- "mailing list together with submitting your code for "
- "inclusion.\n");
- }
-#endif
-
fsa->owner = owner;
fsa->crc = symversion(syms->crcs, symnum);
fsa->sym = &syms->start[symnum];
@@ -537,18 +517,10 @@ static bool find_symbol(struct find_symbol_arg *fsa)
{
static const struct symsearch arr[] = {
{ __start___ksymtab, __stop___ksymtab, __start___kcrctab,
- NOT_GPL_ONLY, false },
+ NOT_GPL_ONLY },
{ __start___ksymtab_gpl, __stop___ksymtab_gpl,
__start___kcrctab_gpl,
- GPL_ONLY, false },
-#ifdef CONFIG_UNUSED_SYMBOLS
- { __start___ksymtab_unused, __stop___ksymtab_unused,
- __start___kcrctab_unused,
- NOT_GPL_ONLY, true },
- { __start___ksymtab_unused_gpl, __stop___ksymtab_unused_gpl,
- __start___kcrctab_unused_gpl,
- GPL_ONLY, true },
-#endif
+ GPL_ONLY },
};
struct module *mod;
unsigned int i;
@@ -563,20 +535,10 @@ static bool find_symbol(struct find_symbol_arg *fsa)
lockdep_is_held(&module_mutex)) {
struct symsearch arr[] = {
{ mod->syms, mod->syms + mod->num_syms, mod->crcs,
- NOT_GPL_ONLY, false },
+ NOT_GPL_ONLY },
{ mod->gpl_syms, mod->gpl_syms + mod->num_gpl_syms,
mod->gpl_crcs,
- GPL_ONLY, false },
-#ifdef CONFIG_UNUSED_SYMBOLS
- { mod->unused_syms,
- mod->unused_syms + mod->num_unused_syms,
- mod->unused_crcs,
- NOT_GPL_ONLY, true },
- { mod->unused_gpl_syms,
- mod->unused_gpl_syms + mod->num_unused_gpl_syms,
- mod->unused_gpl_crcs,
- GPL_ONLY, true },
-#endif
+ GPL_ONLY },
};
if (mod->state == MODULE_STATE_UNFORMED)
@@ -2304,10 +2266,6 @@ static int verify_exported_symbols(struct module *mod)
} arr[] = {
{ mod->syms, mod->num_syms },
{ mod->gpl_syms, mod->num_gpl_syms },
-#ifdef CONFIG_UNUSED_SYMBOLS
- { mod->unused_syms, mod->num_unused_syms },
- { mod->unused_gpl_syms, mod->num_unused_gpl_syms },
-#endif
};
for (i = 0; i < ARRAY_SIZE(arr); i++) {
@@ -3222,16 +3180,6 @@ static int find_module_sections(struct module *mod, struct load_info *info)
&mod->num_gpl_syms);
mod->gpl_crcs = section_addr(info, "__kcrctab_gpl");
-#ifdef CONFIG_UNUSED_SYMBOLS
- mod->unused_syms = section_objs(info, "__ksymtab_unused",
- sizeof(*mod->unused_syms),
- &mod->num_unused_syms);
- mod->unused_crcs = section_addr(info, "__kcrctab_unused");
- mod->unused_gpl_syms = section_objs(info, "__ksymtab_unused_gpl",
- sizeof(*mod->unused_gpl_syms),
- &mod->num_unused_gpl_syms);
- mod->unused_gpl_crcs = section_addr(info, "__kcrctab_unused_gpl");
-#endif
#ifdef CONFIG_CONSTRUCTORS
mod->ctors = section_objs(info, ".ctors",
sizeof(*mod->ctors), &mod->num_ctors);
@@ -3412,13 +3360,8 @@ static int check_module_license_and_versions(struct module *mod)
pr_warn("%s: module license taints kernel.\n", mod->name);
#ifdef CONFIG_MODVERSIONS
- if ((mod->num_syms && !mod->crcs)
- || (mod->num_gpl_syms && !mod->gpl_crcs)
-#ifdef CONFIG_UNUSED_SYMBOLS
- || (mod->num_unused_syms && !mod->unused_crcs)
- || (mod->num_unused_gpl_syms && !mod->unused_gpl_crcs)
-#endif
- ) {
+ if ((mod->num_syms && !mod->crcs) ||
+ (mod->num_gpl_syms && !mod->gpl_crcs)) {
return try_to_force_load(mod,
"no versions for exported symbols");
}
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 92e888ed939f98..eabd2d5467b156 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -4290,8 +4290,7 @@ sub process {
if (defined $realline_next &&
exists $lines[$realline_next - 1] &&
!defined $suppress_export{$realline_next} &&
- ($lines[$realline_next - 1] =~ /EXPORT_SYMBOL.*\((.*)\)/ ||
- $lines[$realline_next - 1] =~ /EXPORT_UNUSED_SYMBOL.*\((.*)\)/)) {
+ ($lines[$realline_next - 1] =~ /EXPORT_SYMBOL.*\((.*)\)/)) {
# Handle definitions which produce identifiers with
# a prefix:
# XXX(foo);
@@ -4318,8 +4317,7 @@ sub process {
}
if (!defined $suppress_export{$linenr} &&
$prevline =~ /^.\s*$/ &&
- ($line =~ /EXPORT_SYMBOL.*\((.*)\)/ ||
- $line =~ /EXPORT_UNUSED_SYMBOL.*\((.*)\)/)) {
+ ($line =~ /EXPORT_SYMBOL.*\((.*)\)/)) {
#print "FOO B <$lines[$linenr - 1]>\n";
$suppress_export{$linenr} = 2;
}
diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index 25c1446055d16b..20fc57837881ab 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -43,8 +43,9 @@ static int allow_missing_ns_imports;
static bool error_occurred;
enum export {
- export_plain, export_unused, export_gpl,
- export_unused_gpl, export_unknown
+ export_plain,
+ export_gpl,
+ export_unknown
};
/* In kernel, this size is defined in linux/module.h;
@@ -301,9 +302,7 @@ static const struct {
enum export export;
} export_list[] = {
{ .str = "EXPORT_SYMBOL", .export = export_plain },
- { .str = "EXPORT_UNUSED_SYMBOL", .export = export_unused },
{ .str = "EXPORT_SYMBOL_GPL", .export = export_gpl },
- { .str = "EXPORT_UNUSED_SYMBOL_GPL", .export = export_unused_gpl },
{ .str = "(unknown)", .export = export_unknown },
};
@@ -362,12 +361,8 @@ static enum export export_from_secname(struct elf_info *elf, unsigned int sec)
if (strstarts(secname, "___ksymtab+"))
return export_plain;
- else if (strstarts(secname, "___ksymtab_unused+"))
- return export_unused;
else if (strstarts(secname, "___ksymtab_gpl+"))
return export_gpl;
- else if (strstarts(secname, "___ksymtab_unused_gpl+"))
- return export_unused_gpl;
else
return export_unknown;
}
@@ -376,12 +371,8 @@ static enum export export_from_sec(struct elf_info *elf, unsigned int sec)
{
if (sec == elf->export_sec)
return export_plain;
- else if (sec == elf->export_unused_sec)
- return export_unused;
else if (sec == elf->export_gpl_sec)
return export_gpl;
- else if (sec == elf->export_unused_gpl_sec)
- return export_unused_gpl;
else
return export_unknown;
}
@@ -585,12 +576,8 @@ static int parse_elf(struct elf_info *info, const char *filename)
info->modinfo_len = sechdrs[i].sh_size;
} else if (strcmp(secname, "__ksymtab") == 0)
info->export_sec = i;
- else if (strcmp(secname, "__ksymtab_unused") == 0)
- info->export_unused_sec = i;
else if (strcmp(secname, "__ksymtab_gpl") == 0)
info->export_gpl_sec = i;
- else if (strcmp(secname, "__ksymtab_unused_gpl") == 0)
- info->export_unused_gpl_sec = i;
if (sechdrs[i].sh_type == SHT_SYMTAB) {
unsigned int sh_link_idx;
@@ -2141,32 +2128,13 @@ static void check_for_gpl_usage(enum export exp, const char *m, const char *s)
error("GPL-incompatible module %s.ko uses GPL-only symbol '%s'\n",
m, s);
break;
- case export_unused_gpl:
- error("GPL-incompatible module %s.ko uses GPL-only symbol marked UNUSED '%s'\n",
- m, s);
- break;
case export_plain:
- case export_unused:
case export_unknown:
/* ignore */
break;
}
}
-static void check_for_unused(enum export exp, const char *m, const char *s)
-{
- switch (exp) {
- case export_unused:
- case export_unused_gpl:
- warn("module %s.ko uses symbol '%s' marked UNUSED\n",
- m, s);
- break;
- default:
- /* ignore */
- break;
- }
-}
-
static void check_exports(struct module *mod)
{
struct symbol *s, *exp;
@@ -2197,7 +2165,6 @@ static void check_exports(struct module *mod)
if (!mod->gpl_compatible)
check_for_gpl_usage(exp->export, basename, exp->name);
- check_for_unused(exp->export, basename, exp->name);
}
}
diff --git a/scripts/mod/modpost.h b/scripts/mod/modpost.h
index 834220de002bd1..0c47ff95c0e227 100644
--- a/scripts/mod/modpost.h
+++ b/scripts/mod/modpost.h
@@ -139,9 +139,7 @@ struct elf_info {
Elf_Sym *symtab_start;
Elf_Sym *symtab_stop;
Elf_Section export_sec;
- Elf_Section export_unused_sec;
Elf_Section export_gpl_sec;
- Elf_Section export_unused_gpl_sec;
char *strtab;
char *modinfo;
unsigned int modinfo_len;
diff --git a/scripts/module.lds.S b/scripts/module.lds.S
index d82b452e8a7168..24e8af579ce378 100644
--- a/scripts/module.lds.S
+++ b/scripts/module.lds.S
@@ -11,12 +11,8 @@ SECTIONS {
__ksymtab 0 : { *(SORT(___ksymtab+*)) }
__ksymtab_gpl 0 : { *(SORT(___ksymtab_gpl+*)) }
- __ksymtab_unused 0 : { *(SORT(___ksymtab_unused+*)) }
- __ksymtab_unused_gpl 0 : { *(SORT(___ksymtab_unused_gpl+*)) }
__kcrctab 0 : { *(SORT(___kcrctab+*)) }
__kcrctab_gpl 0 : { *(SORT(___kcrctab_gpl+*)) }
- __kcrctab_unused 0 : { *(SORT(___kcrctab_unused+*)) }
- __kcrctab_unused_gpl 0 : { *(SORT(___kcrctab_unused_gpl+*)) }
.init_array 0 : ALIGN(8) { *(SORT(.init_array.*)) *(.init_array) }
diff --git a/tools/include/linux/export.h b/tools/include/linux/export.h
index 9f61349a8944e1..acb6f4daa2f0b4 100644
--- a/tools/include/linux/export.h
+++ b/tools/include/linux/export.h
@@ -3,7 +3,5 @@
#define EXPORT_SYMBOL(sym)
#define EXPORT_SYMBOL_GPL(sym)
-#define EXPORT_UNUSED_SYMBOL(sym)
-#define EXPORT_UNUSED_SYMBOL_GPL(sym)
#endif
--
2.29.2
^ permalink raw reply related
* [PATCH 12/13] module: remove EXPORT_SYMBOL_GPL_FUTURE
From: Christoph Hellwig @ 2021-01-21 7:49 UTC (permalink / raw)
To: Frederic Barrat, Andrew Donnellan, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, David Airlie, Daniel Vetter,
Jessica Yu, Josh Poimboeuf, Jiri Kosina, Miroslav Benes,
Petr Mladek, Joe Lawrence
Cc: Michal Marek, linux-kbuild, Masahiro Yamada, linux-kernel,
dri-devel, live-patching, linuxppc-dev
In-Reply-To: <20210121074959.313333-1-hch@lst.de>
As far as I can tell this has never been used at all, and certainly
not any time recently.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
arch/x86/tools/relocs.c | 4 ++--
include/asm-generic/vmlinux.lds.h | 14 --------------
include/linux/export.h | 1 -
include/linux/module.h | 5 -----
kernel/module.c | 17 -----------------
scripts/mod/modpost.c | 13 +------------
scripts/mod/modpost.h | 1 -
scripts/module.lds.S | 2 --
tools/include/linux/export.h | 1 -
9 files changed, 3 insertions(+), 55 deletions(-)
diff --git a/arch/x86/tools/relocs.c b/arch/x86/tools/relocs.c
index ce7188cbdae58a..0d210d0e83e241 100644
--- a/arch/x86/tools/relocs.c
+++ b/arch/x86/tools/relocs.c
@@ -61,8 +61,8 @@ static const char * const sym_regex_kernel[S_NSYMTYPES] = {
"(__iommu_table|__apicdrivers|__smp_locks)(|_end)|"
"__(start|end)_pci_.*|"
"__(start|end)_builtin_fw|"
- "__(start|stop)___ksymtab(|_gpl|_unused|_unused_gpl|_gpl_future)|"
- "__(start|stop)___kcrctab(|_gpl|_unused|_unused_gpl|_gpl_future)|"
+ "__(start|stop)___ksymtab(|_gpl|_unused|_unused_gpl)|"
+ "__(start|stop)___kcrctab(|_gpl|_unused|_unused_gpl)|"
"__(start|stop)___param|"
"__(start|stop)___modver|"
"__(start|stop)___bug_table|"
diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
index b2b3d81b1535a5..83243506e68b00 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -495,13 +495,6 @@
__stop___ksymtab_unused_gpl = .; \
} \
\
- /* Kernel symbol table: GPL-future-only symbols */ \
- __ksymtab_gpl_future : AT(ADDR(__ksymtab_gpl_future) - LOAD_OFFSET) { \
- __start___ksymtab_gpl_future = .; \
- KEEP(*(SORT(___ksymtab_gpl_future+*))) \
- __stop___ksymtab_gpl_future = .; \
- } \
- \
/* Kernel symbol table: Normal symbols */ \
__kcrctab : AT(ADDR(__kcrctab) - LOAD_OFFSET) { \
__start___kcrctab = .; \
@@ -530,13 +523,6 @@
__stop___kcrctab_unused_gpl = .; \
} \
\
- /* Kernel symbol table: GPL-future-only symbols */ \
- __kcrctab_gpl_future : AT(ADDR(__kcrctab_gpl_future) - LOAD_OFFSET) { \
- __start___kcrctab_gpl_future = .; \
- KEEP(*(SORT(___kcrctab_gpl_future+*))) \
- __stop___kcrctab_gpl_future = .; \
- } \
- \
/* Kernel symbol table: strings */ \
__ksymtab_strings : AT(ADDR(__ksymtab_strings) - LOAD_OFFSET) { \
*(__ksymtab_strings) \
diff --git a/include/linux/export.h b/include/linux/export.h
index fceb5e85571711..362b64f8d4a7c2 100644
--- a/include/linux/export.h
+++ b/include/linux/export.h
@@ -157,7 +157,6 @@ struct kernel_symbol {
#define EXPORT_SYMBOL(sym) _EXPORT_SYMBOL(sym, "")
#define EXPORT_SYMBOL_GPL(sym) _EXPORT_SYMBOL(sym, "_gpl")
-#define EXPORT_SYMBOL_GPL_FUTURE(sym) _EXPORT_SYMBOL(sym, "_gpl_future")
#define EXPORT_SYMBOL_NS(sym, ns) __EXPORT_SYMBOL(sym, "", #ns)
#define EXPORT_SYMBOL_NS_GPL(sym, ns) __EXPORT_SYMBOL(sym, "_gpl", #ns)
diff --git a/include/linux/module.h b/include/linux/module.h
index c92c30a285144f..8f4d577d4707c2 100644
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -411,11 +411,6 @@ struct module {
bool async_probe_requested;
- /* symbols that will be GPL-only in the near future. */
- const struct kernel_symbol *gpl_future_syms;
- const s32 *gpl_future_crcs;
- unsigned int num_gpl_future_syms;
-
/* Exception table */
unsigned int num_exentries;
struct exception_table_entry *extable;
diff --git a/kernel/module.c b/kernel/module.c
index 7a88b71736ff5c..917fd1b5c95a42 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -413,11 +413,8 @@ extern const struct kernel_symbol __start___ksymtab[];
extern const struct kernel_symbol __stop___ksymtab[];
extern const struct kernel_symbol __start___ksymtab_gpl[];
extern const struct kernel_symbol __stop___ksymtab_gpl[];
-extern const struct kernel_symbol __start___ksymtab_gpl_future[];
-extern const struct kernel_symbol __stop___ksymtab_gpl_future[];
extern const s32 __start___kcrctab[];
extern const s32 __start___kcrctab_gpl[];
-extern const s32 __start___kcrctab_gpl_future[];
#ifdef CONFIG_UNUSED_SYMBOLS
extern const struct kernel_symbol __start___ksymtab_unused[];
extern const struct kernel_symbol __stop___ksymtab_unused[];
@@ -544,9 +541,6 @@ static bool find_symbol(struct find_symbol_arg *fsa)
{ __start___ksymtab_gpl, __stop___ksymtab_gpl,
__start___kcrctab_gpl,
GPL_ONLY, false },
- { __start___ksymtab_gpl_future, __stop___ksymtab_gpl_future,
- __start___kcrctab_gpl_future,
- WILL_BE_GPL_ONLY, false },
#ifdef CONFIG_UNUSED_SYMBOLS
{ __start___ksymtab_unused, __stop___ksymtab_unused,
__start___kcrctab_unused,
@@ -573,10 +567,6 @@ static bool find_symbol(struct find_symbol_arg *fsa)
{ mod->gpl_syms, mod->gpl_syms + mod->num_gpl_syms,
mod->gpl_crcs,
GPL_ONLY, false },
- { mod->gpl_future_syms,
- mod->gpl_future_syms + mod->num_gpl_future_syms,
- mod->gpl_future_crcs,
- WILL_BE_GPL_ONLY, false },
#ifdef CONFIG_UNUSED_SYMBOLS
{ mod->unused_syms,
mod->unused_syms + mod->num_unused_syms,
@@ -2314,7 +2304,6 @@ static int verify_exported_symbols(struct module *mod)
} arr[] = {
{ mod->syms, mod->num_syms },
{ mod->gpl_syms, mod->num_gpl_syms },
- { mod->gpl_future_syms, mod->num_gpl_future_syms },
#ifdef CONFIG_UNUSED_SYMBOLS
{ mod->unused_syms, mod->num_unused_syms },
{ mod->unused_gpl_syms, mod->num_unused_gpl_syms },
@@ -3232,11 +3221,6 @@ static int find_module_sections(struct module *mod, struct load_info *info)
sizeof(*mod->gpl_syms),
&mod->num_gpl_syms);
mod->gpl_crcs = section_addr(info, "__kcrctab_gpl");
- mod->gpl_future_syms = section_objs(info,
- "__ksymtab_gpl_future",
- sizeof(*mod->gpl_future_syms),
- &mod->num_gpl_future_syms);
- mod->gpl_future_crcs = section_addr(info, "__kcrctab_gpl_future");
#ifdef CONFIG_UNUSED_SYMBOLS
mod->unused_syms = section_objs(info, "__ksymtab_unused",
@@ -3430,7 +3414,6 @@ static int check_module_license_and_versions(struct module *mod)
#ifdef CONFIG_MODVERSIONS
if ((mod->num_syms && !mod->crcs)
|| (mod->num_gpl_syms && !mod->gpl_crcs)
- || (mod->num_gpl_future_syms && !mod->gpl_future_crcs)
#ifdef CONFIG_UNUSED_SYMBOLS
|| (mod->num_unused_syms && !mod->unused_crcs)
|| (mod->num_unused_gpl_syms && !mod->unused_gpl_crcs)
diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index d6c81657d69550..25c1446055d16b 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -44,7 +44,7 @@ static bool error_occurred;
enum export {
export_plain, export_unused, export_gpl,
- export_unused_gpl, export_gpl_future, export_unknown
+ export_unused_gpl, export_unknown
};
/* In kernel, this size is defined in linux/module.h;
@@ -304,7 +304,6 @@ static const struct {
{ .str = "EXPORT_UNUSED_SYMBOL", .export = export_unused },
{ .str = "EXPORT_SYMBOL_GPL", .export = export_gpl },
{ .str = "EXPORT_UNUSED_SYMBOL_GPL", .export = export_unused_gpl },
- { .str = "EXPORT_SYMBOL_GPL_FUTURE", .export = export_gpl_future },
{ .str = "(unknown)", .export = export_unknown },
};
@@ -369,8 +368,6 @@ static enum export export_from_secname(struct elf_info *elf, unsigned int sec)
return export_gpl;
else if (strstarts(secname, "___ksymtab_unused_gpl+"))
return export_unused_gpl;
- else if (strstarts(secname, "___ksymtab_gpl_future+"))
- return export_gpl_future;
else
return export_unknown;
}
@@ -385,8 +382,6 @@ static enum export export_from_sec(struct elf_info *elf, unsigned int sec)
return export_gpl;
else if (sec == elf->export_unused_gpl_sec)
return export_unused_gpl;
- else if (sec == elf->export_gpl_future_sec)
- return export_gpl_future;
else
return export_unknown;
}
@@ -596,8 +591,6 @@ static int parse_elf(struct elf_info *info, const char *filename)
info->export_gpl_sec = i;
else if (strcmp(secname, "__ksymtab_unused_gpl") == 0)
info->export_unused_gpl_sec = i;
- else if (strcmp(secname, "__ksymtab_gpl_future") == 0)
- info->export_gpl_future_sec = i;
if (sechdrs[i].sh_type == SHT_SYMTAB) {
unsigned int sh_link_idx;
@@ -2152,10 +2145,6 @@ static void check_for_gpl_usage(enum export exp, const char *m, const char *s)
error("GPL-incompatible module %s.ko uses GPL-only symbol marked UNUSED '%s'\n",
m, s);
break;
- case export_gpl_future:
- warn("GPL-incompatible module %s.ko uses future GPL-only symbol '%s'\n",
- m, s);
- break;
case export_plain:
case export_unused:
case export_unknown:
diff --git a/scripts/mod/modpost.h b/scripts/mod/modpost.h
index e6f46eee0af02f..834220de002bd1 100644
--- a/scripts/mod/modpost.h
+++ b/scripts/mod/modpost.h
@@ -142,7 +142,6 @@ struct elf_info {
Elf_Section export_unused_sec;
Elf_Section export_gpl_sec;
Elf_Section export_unused_gpl_sec;
- Elf_Section export_gpl_future_sec;
char *strtab;
char *modinfo;
unsigned int modinfo_len;
diff --git a/scripts/module.lds.S b/scripts/module.lds.S
index 69b9b71a6a4731..d82b452e8a7168 100644
--- a/scripts/module.lds.S
+++ b/scripts/module.lds.S
@@ -13,12 +13,10 @@ SECTIONS {
__ksymtab_gpl 0 : { *(SORT(___ksymtab_gpl+*)) }
__ksymtab_unused 0 : { *(SORT(___ksymtab_unused+*)) }
__ksymtab_unused_gpl 0 : { *(SORT(___ksymtab_unused_gpl+*)) }
- __ksymtab_gpl_future 0 : { *(SORT(___ksymtab_gpl_future+*)) }
__kcrctab 0 : { *(SORT(___kcrctab+*)) }
__kcrctab_gpl 0 : { *(SORT(___kcrctab_gpl+*)) }
__kcrctab_unused 0 : { *(SORT(___kcrctab_unused+*)) }
__kcrctab_unused_gpl 0 : { *(SORT(___kcrctab_unused_gpl+*)) }
- __kcrctab_gpl_future 0 : { *(SORT(___kcrctab_gpl_future+*)) }
.init_array 0 : ALIGN(8) { *(SORT(.init_array.*)) *(.init_array) }
diff --git a/tools/include/linux/export.h b/tools/include/linux/export.h
index d07e586b9ba0ec..9f61349a8944e1 100644
--- a/tools/include/linux/export.h
+++ b/tools/include/linux/export.h
@@ -3,7 +3,6 @@
#define EXPORT_SYMBOL(sym)
#define EXPORT_SYMBOL_GPL(sym)
-#define EXPORT_SYMBOL_GPL_FUTURE(sym)
#define EXPORT_UNUSED_SYMBOL(sym)
#define EXPORT_UNUSED_SYMBOL_GPL(sym)
--
2.29.2
^ permalink raw reply related
* [PATCH 11/13] module: pass struct find_symbol_args to find_symbol
From: Christoph Hellwig @ 2021-01-21 7:49 UTC (permalink / raw)
To: Frederic Barrat, Andrew Donnellan, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, David Airlie, Daniel Vetter,
Jessica Yu, Josh Poimboeuf, Jiri Kosina, Miroslav Benes,
Petr Mladek, Joe Lawrence
Cc: Michal Marek, linux-kbuild, Masahiro Yamada, linux-kernel,
dri-devel, live-patching, linuxppc-dev
In-Reply-To: <20210121074959.313333-1-hch@lst.de>
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>
---
kernel/module.c | 113 ++++++++++++++++++++++--------------------------
1 file changed, 52 insertions(+), 61 deletions(-)
diff --git a/kernel/module.c b/kernel/module.c
index 644dda52dae38c..7a88b71736ff5c 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -536,12 +536,7 @@ static bool find_exported_symbol_in_section(const struct symsearch *syms,
* Find an exported symbol and return it, along with, (optional) crc and
* (optional) module which owns it. Needs preempt disabled or module_mutex.
*/
-static const struct kernel_symbol *find_symbol(const char *name,
- struct module **owner,
- const s32 **crc,
- enum mod_license *license,
- bool gplok,
- bool warn)
+static bool find_symbol(struct find_symbol_arg *fsa)
{
static const struct symsearch arr[] = {
{ __start___ksymtab, __stop___ksymtab, __start___kcrctab,
@@ -561,19 +556,14 @@ static const struct kernel_symbol *find_symbol(const char *name,
GPL_ONLY, true },
#endif
};
- struct find_symbol_arg fsa = {
- .name = name,
- .gplok = gplok,
- .warn = warn,
- };
struct module *mod;
unsigned int i;
module_assert_mutex_or_preempt();
for (i = 0; i < ARRAY_SIZE(arr); i++)
- if (find_exported_symbol_in_section(&arr[i], NULL, &fsa))
- goto found;
+ if (find_exported_symbol_in_section(&arr[i], NULL, fsa))
+ return true;
list_for_each_entry_rcu(mod, &modules, list,
lockdep_is_held(&module_mutex)) {
@@ -603,21 +593,12 @@ static const struct kernel_symbol *find_symbol(const char *name,
continue;
for (i = 0; i < ARRAY_SIZE(arr); i++)
- if (find_exported_symbol_in_section(&arr[i], mod, &fsa))
- goto found;
+ if (find_exported_symbol_in_section(&arr[i], mod, fsa))
+ return true;
}
- pr_debug("Failed to find symbol %s\n", name);
- return NULL;
-
-found:
- if (owner)
- *owner = fsa.owner;
- if (crc)
- *crc = fsa.crc;
- if (license)
- *license = fsa.license;
- return fsa.sym;
+ pr_debug("Failed to find symbol %s\n", fsa->name);
+ return false;
}
/*
@@ -1107,12 +1088,15 @@ static inline void print_unload_info(struct seq_file *m, struct module *mod)
void __symbol_put(const char *symbol)
{
- struct module *owner;
+ struct find_symbol_arg fsa = {
+ .name = symbol,
+ .gplok = true,
+ };
preempt_disable();
- if (!find_symbol(symbol, &owner, NULL, NULL, true, false))
+ if (!find_symbol(&fsa))
BUG();
- module_put(owner);
+ module_put(fsa.owner);
preempt_enable();
}
EXPORT_SYMBOL(__symbol_put);
@@ -1381,19 +1365,22 @@ static int check_version(const struct load_info *info,
static inline int check_modstruct_version(const struct load_info *info,
struct module *mod)
{
- const s32 *crc;
+ struct find_symbol_arg fsa = {
+ .name = "module_layout",
+ .gplok = true,
+ };
/*
* Since this should be found in kernel (which can't be removed), no
* locking is necessary -- use preempt_disable() to placate lockdep.
*/
preempt_disable();
- if (!find_symbol("module_layout", NULL, &crc, NULL, true, false)) {
+ if (!find_symbol(&fsa)) {
preempt_enable();
BUG();
}
preempt_enable();
- return check_version(info, "module_layout", mod, crc);
+ return check_version(info, "module_layout", mod, fsa.crc);
}
/* First part is kernel version, which we ignore if module has crcs. */
@@ -1487,10 +1474,11 @@ static const struct kernel_symbol *resolve_symbol(struct module *mod,
const char *name,
char ownername[])
{
- struct module *owner;
- const struct kernel_symbol *sym;
- const s32 *crc;
- enum mod_license license;
+ struct find_symbol_arg fsa = {
+ .name = name,
+ .gplok = !(mod->taints & (1 << TAINT_PROPRIETARY_MODULE)),
+ .warn = true,
+ };
int err;
/*
@@ -1500,42 +1488,40 @@ static const struct kernel_symbol *resolve_symbol(struct module *mod,
*/
sched_annotate_sleep();
mutex_lock(&module_mutex);
- sym = find_symbol(name, &owner, &crc, &license,
- !(mod->taints & (1 << TAINT_PROPRIETARY_MODULE)), true);
- if (!sym)
+ if (!find_symbol(&fsa))
goto unlock;
- if (license == GPL_ONLY)
+ if (fsa.license == GPL_ONLY)
mod->using_gplonly_symbols = true;
- if (!inherit_taint(mod, owner)) {
- sym = NULL;
+ if (!inherit_taint(mod, fsa.owner)) {
+ fsa.sym = NULL;
goto getname;
}
- if (!check_version(info, name, mod, crc)) {
- sym = ERR_PTR(-EINVAL);
+ if (!check_version(info, name, mod, fsa.crc)) {
+ fsa.sym = ERR_PTR(-EINVAL);
goto getname;
}
- err = verify_namespace_is_imported(info, sym, mod);
+ err = verify_namespace_is_imported(info, fsa.sym, mod);
if (err) {
- sym = ERR_PTR(err);
+ fsa.sym = ERR_PTR(err);
goto getname;
}
- err = ref_module(mod, owner);
+ err = ref_module(mod, fsa.owner);
if (err) {
- sym = ERR_PTR(err);
+ fsa.sym = ERR_PTR(err);
goto getname;
}
getname:
/* We must make copy under the lock if we failed to get ref. */
- strncpy(ownername, module_name(owner), MODULE_NAME_LEN);
+ strncpy(ownername, module_name(fsa.owner), MODULE_NAME_LEN);
unlock:
mutex_unlock(&module_mutex);
- return sym;
+ return fsa.sym;
}
static const struct kernel_symbol *
@@ -2296,16 +2282,19 @@ static void free_module(struct module *mod)
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)) {
+ 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);
@@ -2318,7 +2307,6 @@ EXPORT_SYMBOL_GPL(__symbol_get);
static int verify_exported_symbols(struct module *mod)
{
unsigned int i;
- struct module *owner;
const struct kernel_symbol *s;
struct {
const struct kernel_symbol *sym;
@@ -2335,12 +2323,15 @@ static int verify_exported_symbols(struct module *mod)
for (i = 0; i < ARRAY_SIZE(arr); i++) {
for (s = arr[i].sym; s < arr[i].sym + arr[i].num; s++) {
- if (find_symbol(kernel_symbol_name(s), &owner, NULL,
- NULL, true, false)) {
+ struct find_symbol_arg fsa = {
+ .name = kernel_symbol_name(s),
+ .gplok = true,
+ };
+ if (find_symbol(&fsa)) {
pr_err("%s: exports duplicate symbol %s"
" (owned by %s)\n",
mod->name, kernel_symbol_name(s),
- module_name(owner));
+ module_name(fsa.owner));
return -ENOEXEC;
}
}
--
2.29.2
^ permalink raw reply related
* [PATCH 10/13] module: merge each_symbol_section into find_symbol
From: Christoph Hellwig @ 2021-01-21 7:49 UTC (permalink / raw)
To: Frederic Barrat, Andrew Donnellan, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, David Airlie, Daniel Vetter,
Jessica Yu, Josh Poimboeuf, Jiri Kosina, Miroslav Benes,
Petr Mladek, Joe Lawrence
Cc: Michal Marek, linux-kbuild, Masahiro Yamada, linux-kernel,
dri-devel, live-patching, linuxppc-dev
In-Reply-To: <20210121074959.313333-1-hch@lst.de>
each_symbol_section is only called by find_symbol, so merge the two
functions.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
kernel/module.c | 148 ++++++++++++++++++++++--------------------------
1 file changed, 69 insertions(+), 79 deletions(-)
diff --git a/kernel/module.c b/kernel/module.c
index a9d092765c4eab..644dda52dae38c 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -433,73 +433,6 @@ extern const s32 __start___kcrctab_unused_gpl[];
#define symversion(base, idx) ((base != NULL) ? ((base) + (idx)) : NULL)
#endif
-/* Returns true as soon as fn returns true, otherwise false. */
-static bool each_symbol_section(bool (*fn)(const struct symsearch *arr,
- struct module *owner,
- void *data),
- void *data)
-{
- unsigned int i;
- struct module *mod;
- static const struct symsearch arr[] = {
- { __start___ksymtab, __stop___ksymtab, __start___kcrctab,
- NOT_GPL_ONLY, false },
- { __start___ksymtab_gpl, __stop___ksymtab_gpl,
- __start___kcrctab_gpl,
- GPL_ONLY, false },
- { __start___ksymtab_gpl_future, __stop___ksymtab_gpl_future,
- __start___kcrctab_gpl_future,
- WILL_BE_GPL_ONLY, false },
-#ifdef CONFIG_UNUSED_SYMBOLS
- { __start___ksymtab_unused, __stop___ksymtab_unused,
- __start___kcrctab_unused,
- NOT_GPL_ONLY, true },
- { __start___ksymtab_unused_gpl, __stop___ksymtab_unused_gpl,
- __start___kcrctab_unused_gpl,
- GPL_ONLY, true },
-#endif
- };
-
- module_assert_mutex_or_preempt();
-
- for (i = 0; i < ARRAY_SIZE(arr); i++)
- if (fn(&arr[i], NULL, data))
- return true;
-
- list_for_each_entry_rcu(mod, &modules, list,
- lockdep_is_held(&module_mutex)) {
- struct symsearch arr[] = {
- { mod->syms, mod->syms + mod->num_syms, mod->crcs,
- NOT_GPL_ONLY, false },
- { mod->gpl_syms, mod->gpl_syms + mod->num_gpl_syms,
- mod->gpl_crcs,
- GPL_ONLY, false },
- { mod->gpl_future_syms,
- mod->gpl_future_syms + mod->num_gpl_future_syms,
- mod->gpl_future_crcs,
- WILL_BE_GPL_ONLY, false },
-#ifdef CONFIG_UNUSED_SYMBOLS
- { mod->unused_syms,
- mod->unused_syms + mod->num_unused_syms,
- mod->unused_crcs,
- NOT_GPL_ONLY, true },
- { mod->unused_gpl_syms,
- mod->unused_gpl_syms + mod->num_unused_gpl_syms,
- mod->unused_gpl_crcs,
- GPL_ONLY, true },
-#endif
- };
-
- if (mod->state == MODULE_STATE_UNFORMED)
- continue;
-
- for (i = 0; i < ARRAY_SIZE(arr); i++)
- if (fn(&arr[i], mod, data))
- return true;
- }
- return false;
-}
-
struct find_symbol_arg {
/* Input */
const char *name;
@@ -610,24 +543,81 @@ static const struct kernel_symbol *find_symbol(const char *name,
bool gplok,
bool warn)
{
- struct find_symbol_arg fsa;
+ static const struct symsearch arr[] = {
+ { __start___ksymtab, __stop___ksymtab, __start___kcrctab,
+ NOT_GPL_ONLY, false },
+ { __start___ksymtab_gpl, __stop___ksymtab_gpl,
+ __start___kcrctab_gpl,
+ GPL_ONLY, false },
+ { __start___ksymtab_gpl_future, __stop___ksymtab_gpl_future,
+ __start___kcrctab_gpl_future,
+ WILL_BE_GPL_ONLY, false },
+#ifdef CONFIG_UNUSED_SYMBOLS
+ { __start___ksymtab_unused, __stop___ksymtab_unused,
+ __start___kcrctab_unused,
+ NOT_GPL_ONLY, true },
+ { __start___ksymtab_unused_gpl, __stop___ksymtab_unused_gpl,
+ __start___kcrctab_unused_gpl,
+ GPL_ONLY, true },
+#endif
+ };
+ struct find_symbol_arg fsa = {
+ .name = name,
+ .gplok = gplok,
+ .warn = warn,
+ };
+ struct module *mod;
+ unsigned int i;
+
+ module_assert_mutex_or_preempt();
+
+ for (i = 0; i < ARRAY_SIZE(arr); i++)
+ if (find_exported_symbol_in_section(&arr[i], NULL, &fsa))
+ goto found;
- fsa.name = name;
- fsa.gplok = gplok;
- fsa.warn = warn;
+ list_for_each_entry_rcu(mod, &modules, list,
+ lockdep_is_held(&module_mutex)) {
+ struct symsearch arr[] = {
+ { mod->syms, mod->syms + mod->num_syms, mod->crcs,
+ NOT_GPL_ONLY, false },
+ { mod->gpl_syms, mod->gpl_syms + mod->num_gpl_syms,
+ mod->gpl_crcs,
+ GPL_ONLY, false },
+ { mod->gpl_future_syms,
+ mod->gpl_future_syms + mod->num_gpl_future_syms,
+ mod->gpl_future_crcs,
+ WILL_BE_GPL_ONLY, false },
+#ifdef CONFIG_UNUSED_SYMBOLS
+ { mod->unused_syms,
+ mod->unused_syms + mod->num_unused_syms,
+ mod->unused_crcs,
+ NOT_GPL_ONLY, true },
+ { mod->unused_gpl_syms,
+ mod->unused_gpl_syms + mod->num_unused_gpl_syms,
+ mod->unused_gpl_crcs,
+ GPL_ONLY, true },
+#endif
+ };
- if (each_symbol_section(find_exported_symbol_in_section, &fsa)) {
- if (owner)
- *owner = fsa.owner;
- if (crc)
- *crc = fsa.crc;
- if (license)
- *license = fsa.license;
- return fsa.sym;
+ if (mod->state == MODULE_STATE_UNFORMED)
+ continue;
+
+ for (i = 0; i < ARRAY_SIZE(arr); i++)
+ if (find_exported_symbol_in_section(&arr[i], mod, &fsa))
+ goto found;
}
pr_debug("Failed to find symbol %s\n", name);
return NULL;
+
+found:
+ if (owner)
+ *owner = fsa.owner;
+ if (crc)
+ *crc = fsa.crc;
+ if (license)
+ *license = fsa.license;
+ return fsa.sym;
}
/*
--
2.29.2
^ permalink raw reply related
* [PATCH 09/13] module: remove each_symbol_in_section
From: Christoph Hellwig @ 2021-01-21 7:49 UTC (permalink / raw)
To: Frederic Barrat, Andrew Donnellan, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, David Airlie, Daniel Vetter,
Jessica Yu, Josh Poimboeuf, Jiri Kosina, Miroslav Benes,
Petr Mladek, Joe Lawrence
Cc: Michal Marek, linux-kbuild, Masahiro Yamada, linux-kernel,
dri-devel, live-patching, linuxppc-dev
In-Reply-To: <20210121074959.313333-1-hch@lst.de>
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>
---
kernel/module.c | 29 +++++++----------------------
1 file changed, 7 insertions(+), 22 deletions(-)
diff --git a/kernel/module.c b/kernel/module.c
index d163c78ca8ed69..a9d092765c4eab 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -433,30 +433,13 @@ extern const s32 __start___kcrctab_unused_gpl[];
#define symversion(base, idx) ((base != NULL) ? ((base) + (idx)) : NULL)
#endif
-static bool each_symbol_in_section(const struct symsearch *arr,
- unsigned int arrsize,
- struct module *owner,
- bool (*fn)(const struct symsearch *syms,
- struct module *owner,
- void *data),
- void *data)
-{
- unsigned int j;
-
- for (j = 0; j < arrsize; j++) {
- if (fn(&arr[j], owner, data))
- return true;
- }
-
- return false;
-}
-
/* Returns true as soon as fn returns true, otherwise false. */
static bool each_symbol_section(bool (*fn)(const struct symsearch *arr,
struct module *owner,
void *data),
void *data)
{
+ unsigned int i;
struct module *mod;
static const struct symsearch arr[] = {
{ __start___ksymtab, __stop___ksymtab, __start___kcrctab,
@@ -479,8 +462,9 @@ static bool each_symbol_section(bool (*fn)(const struct symsearch *arr,
module_assert_mutex_or_preempt();
- if (each_symbol_in_section(arr, ARRAY_SIZE(arr), NULL, fn, data))
- return true;
+ for (i = 0; i < ARRAY_SIZE(arr); i++)
+ if (fn(&arr[i], NULL, data))
+ return true;
list_for_each_entry_rcu(mod, &modules, list,
lockdep_is_held(&module_mutex)) {
@@ -509,8 +493,9 @@ static bool each_symbol_section(bool (*fn)(const struct symsearch *arr,
if (mod->state == MODULE_STATE_UNFORMED)
continue;
- if (each_symbol_in_section(arr, ARRAY_SIZE(arr), mod, fn, data))
- return true;
+ for (i = 0; i < ARRAY_SIZE(arr); i++)
+ if (fn(&arr[i], mod, data))
+ return true;
}
return false;
}
--
2.29.2
^ permalink raw reply related
* [PATCH 08/13] drm: remove drm_fb_helper_modinit
From: Christoph Hellwig @ 2021-01-21 7:49 UTC (permalink / raw)
To: Frederic Barrat, Andrew Donnellan, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, David Airlie, Daniel Vetter,
Jessica Yu, Josh Poimboeuf, Jiri Kosina, Miroslav Benes,
Petr Mladek, Joe Lawrence
Cc: Michal Marek, linux-kbuild, Masahiro Yamada, linux-kernel,
dri-devel, live-patching, linuxppc-dev
In-Reply-To: <20210121074959.313333-1-hch@lst.de>
drm_fb_helper_modinit has a lot of boilerplate for what is not very
simple functionality. Just open code it in the only caller using
IS_ENABLED and IS_MODULE.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
drivers/gpu/drm/drm_crtc_helper_internal.h | 10 ---------
drivers/gpu/drm/drm_fb_helper.c | 16 -------------
drivers/gpu/drm/drm_kms_helper_common.c | 26 +++++++++++-----------
3 files changed, 13 insertions(+), 39 deletions(-)
diff --git a/drivers/gpu/drm/drm_crtc_helper_internal.h b/drivers/gpu/drm/drm_crtc_helper_internal.h
index 25ce42e799952c..61e09f8a8d0ff0 100644
--- a/drivers/gpu/drm/drm_crtc_helper_internal.h
+++ b/drivers/gpu/drm/drm_crtc_helper_internal.h
@@ -32,16 +32,6 @@
#include <drm/drm_encoder.h>
#include <drm/drm_modes.h>
-/* drm_fb_helper.c */
-#ifdef CONFIG_DRM_FBDEV_EMULATION
-int drm_fb_helper_modinit(void);
-#else
-static inline int drm_fb_helper_modinit(void)
-{
- return 0;
-}
-#endif
-
/* drm_dp_aux_dev.c */
#ifdef CONFIG_DRM_DP_AUX_CHARDEV
int drm_dp_aux_dev_init(void);
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index ce6d63ca75c32a..0b9f1ae1b7864c 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -2499,19 +2499,3 @@ void drm_fbdev_generic_setup(struct drm_device *dev,
drm_client_register(&fb_helper->client);
}
EXPORT_SYMBOL(drm_fbdev_generic_setup);
-
-/* The Kconfig DRM_KMS_HELPER selects FRAMEBUFFER_CONSOLE (if !EXPERT)
- * but the module doesn't depend on any fb console symbols. At least
- * attempt to load fbcon to avoid leaving the system without a usable console.
- */
-int __init drm_fb_helper_modinit(void)
-{
-#if defined(CONFIG_FRAMEBUFFER_CONSOLE_MODULE) && !defined(CONFIG_EXPERT)
- const char name[] = "fbcon";
-
- if (!module_loaded(name))
- request_module_nowait(name);
-#endif
- return 0;
-}
-EXPORT_SYMBOL(drm_fb_helper_modinit);
diff --git a/drivers/gpu/drm/drm_kms_helper_common.c b/drivers/gpu/drm/drm_kms_helper_common.c
index 221a8528c9937a..b694a7da632eae 100644
--- a/drivers/gpu/drm/drm_kms_helper_common.c
+++ b/drivers/gpu/drm/drm_kms_helper_common.c
@@ -64,19 +64,19 @@ MODULE_PARM_DESC(edid_firmware,
static int __init drm_kms_helper_init(void)
{
- int ret;
-
- /* Call init functions from specific kms helpers here */
- ret = drm_fb_helper_modinit();
- if (ret < 0)
- goto out;
-
- ret = drm_dp_aux_dev_init();
- if (ret < 0)
- goto out;
-
-out:
- return ret;
+ /*
+ * The Kconfig DRM_KMS_HELPER selects FRAMEBUFFER_CONSOLE (if !EXPERT)
+ * but the module doesn't depend on any fb console symbols. At least
+ * attempt to load fbcon to avoid leaving the system without a usable
+ * console.
+ */
+ if (IS_ENABLED(CONFIG_DRM_FBDEV_EMULATION) &&
+ IS_MODULE(CONFIG_FRAMEBUFFER_CONSOLE) &&
+ !IS_ENABLED(CONFIG_EXPERT) &&
+ !module_loaded("fbcon"))
+ request_module_nowait("fbcon");
+
+ return drm_dp_aux_dev_init();
}
static void __exit drm_kms_helper_exit(void)
--
2.29.2
^ permalink raw reply related
* [PATCH 07/13] module: mark module_mutex static
From: Christoph Hellwig @ 2021-01-21 7:49 UTC (permalink / raw)
To: Frederic Barrat, Andrew Donnellan, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, David Airlie, Daniel Vetter,
Jessica Yu, Josh Poimboeuf, Jiri Kosina, Miroslav Benes,
Petr Mladek, Joe Lawrence
Cc: Michal Marek, linux-kbuild, Masahiro Yamada, linux-kernel,
dri-devel, live-patching, linuxppc-dev
In-Reply-To: <20210121074959.313333-1-hch@lst.de>
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>
---
include/linux/module.h | 2 --
kernel/module.c | 2 +-
lib/bug.c | 3 ---
3 files changed, 1 insertion(+), 6 deletions(-)
diff --git a/include/linux/module.h b/include/linux/module.h
index 695f127745af10..c92c30a285144f 100644
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -550,8 +550,6 @@ static inline unsigned long kallsyms_symbol_value(const Elf_Sym *sym)
}
#endif
-extern struct mutex module_mutex;
-
/* FIXME: It'd be nice to isolate modules during init, too, so they
aren't used before they (may) fail. But presently too much code
(IDE & SCSI) require entry into the module during init.*/
diff --git a/kernel/module.c b/kernel/module.c
index e141e5d1d7beaf..d163c78ca8ed69 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -87,7 +87,7 @@
* 3) module_addr_min/module_addr_max.
* (delete and add uses RCU list operations).
*/
-DEFINE_MUTEX(module_mutex);
+static DEFINE_MUTEX(module_mutex);
static LIST_HEAD(modules);
/* Work queue for freeing init sections in success case */
diff --git a/lib/bug.c b/lib/bug.c
index 7103440c0ee1af..8f9d537bfb2a59 100644
--- a/lib/bug.c
+++ b/lib/bug.c
@@ -91,8 +91,6 @@ void module_bug_finalize(const Elf_Ehdr *hdr, const Elf_Shdr *sechdrs,
char *secstrings;
unsigned int i;
- lockdep_assert_held(&module_mutex);
-
mod->bug_table = NULL;
mod->num_bugs = 0;
@@ -118,7 +116,6 @@ void module_bug_finalize(const Elf_Ehdr *hdr, const Elf_Shdr *sechdrs,
void module_bug_cleanup(struct module *mod)
{
- lockdep_assert_held(&module_mutex);
list_del_rcu(&mod->bug_list);
}
--
2.29.2
^ permalink raw reply related
* [PATCH 06/13] kallsyms: only build {, module_}kallsyms_on_each_symbol when required
From: Christoph Hellwig @ 2021-01-21 7:49 UTC (permalink / raw)
To: Frederic Barrat, Andrew Donnellan, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, David Airlie, Daniel Vetter,
Jessica Yu, Josh Poimboeuf, Jiri Kosina, Miroslav Benes,
Petr Mladek, Joe Lawrence
Cc: Michal Marek, linux-kbuild, Masahiro Yamada, linux-kernel,
dri-devel, live-patching, linuxppc-dev
In-Reply-To: <20210121074959.313333-1-hch@lst.de>
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>
---
include/linux/kallsyms.h | 17 ++++-------------
include/linux/module.h | 16 ++++------------
kernel/kallsyms.c | 2 ++
kernel/module.c | 2 ++
4 files changed, 12 insertions(+), 25 deletions(-)
diff --git a/include/linux/kallsyms.h b/include/linux/kallsyms.h
index 481273f0c72d42..465060acc9816f 100644
--- a/include/linux/kallsyms.h
+++ b/include/linux/kallsyms.h
@@ -71,15 +71,14 @@ static inline void *dereference_symbol_descriptor(void *ptr)
return ptr;
}
-#ifdef CONFIG_KALLSYMS
-/* Lookup the address for a symbol. Returns 0 if not found. */
-unsigned long kallsyms_lookup_name(const char *name);
-
-/* Call a function on each kallsyms symbol in the core kernel */
int kallsyms_on_each_symbol(int (*fn)(void *, const char *, struct module *,
unsigned long),
void *data);
+#ifdef CONFIG_KALLSYMS
+/* Lookup the address for a symbol. Returns 0 if not found. */
+unsigned long kallsyms_lookup_name(const char *name);
+
extern int kallsyms_lookup_size_offset(unsigned long addr,
unsigned long *symbolsize,
unsigned long *offset);
@@ -108,14 +107,6 @@ static inline unsigned long kallsyms_lookup_name(const char *name)
return 0;
}
-static inline int kallsyms_on_each_symbol(int (*fn)(void *, const char *,
- struct module *,
- unsigned long),
- void *data)
-{
- return 0;
-}
-
static inline int kallsyms_lookup_size_offset(unsigned long addr,
unsigned long *symbolsize,
unsigned long *offset)
diff --git a/include/linux/module.h b/include/linux/module.h
index 8588482bde4116..695f127745af10 100644
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -610,10 +610,6 @@ int module_get_kallsym(unsigned int symnum, unsigned long *value, char *type,
/* Look for this name: can be of form module:name. */
unsigned long module_kallsyms_lookup_name(const char *name);
-int module_kallsyms_on_each_symbol(int (*fn)(void *, const char *,
- struct module *, unsigned long),
- void *data);
-
extern void __noreturn __module_put_and_exit(struct module *mod,
long code);
#define module_put_and_exit(code) __module_put_and_exit(THIS_MODULE, code)
@@ -797,14 +793,6 @@ static inline unsigned long module_kallsyms_lookup_name(const char *name)
return 0;
}
-static inline int module_kallsyms_on_each_symbol(int (*fn)(void *, const char *,
- struct module *,
- unsigned long),
- void *data)
-{
- return 0;
-}
-
static inline int register_module_notifier(struct notifier_block *nb)
{
/* no events will happen anyway, so this can always succeed */
@@ -893,4 +881,8 @@ static inline bool module_sig_ok(struct module *module)
}
#endif /* CONFIG_MODULE_SIG */
+int module_kallsyms_on_each_symbol(int (*fn)(void *, const char *,
+ struct module *, unsigned long),
+ void *data);
+
#endif /* _LINUX_MODULE_H */
diff --git a/kernel/kallsyms.c b/kernel/kallsyms.c
index a0d3f0865916f9..8043a90aa50ed3 100644
--- a/kernel/kallsyms.c
+++ b/kernel/kallsyms.c
@@ -177,6 +177,7 @@ unsigned long kallsyms_lookup_name(const char *name)
return module_kallsyms_lookup_name(name);
}
+#ifdef CONFIG_LIVEPATCH
/*
* Iterate over all symbols in vmlinux. For symbols from modules use
* module_kallsyms_on_each_symbol instead.
@@ -198,6 +199,7 @@ int kallsyms_on_each_symbol(int (*fn)(void *, const char *, struct module *,
}
return 0;
}
+#endif /* CONFIG_LIVEPATCH */
static unsigned long get_symbol_pos(unsigned long addr,
unsigned long *symbolsize,
diff --git a/kernel/module.c b/kernel/module.c
index 885feec64c1b6f..e141e5d1d7beaf 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -4399,6 +4399,7 @@ unsigned long module_kallsyms_lookup_name(const char *name)
return ret;
}
+#ifdef CONFIG_LIVEPATCH
int module_kallsyms_on_each_symbol(int (*fn)(void *, const char *,
struct module *, unsigned long),
void *data)
@@ -4429,6 +4430,7 @@ int module_kallsyms_on_each_symbol(int (*fn)(void *, const char *,
mutex_unlock(&module_mutex);
return ret;
}
+#endif /* CONFIG_LIVEPATCH */
#endif /* CONFIG_KALLSYMS */
/* Maximum number of characters written by module_flags() */
--
2.29.2
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox