There is no need to call spin_unlock_irqrestore() in every conditional
block, as release_nodes() can safely be called with an empty list, in
case we hit the "if else" or "else" case.
We do not use a scoped_guard() here to not unnecessarily change the
indentation level.
Reviewed-by: Greg Kroah-Hartman <[email protected]>
Link: https://patch.msgid.link/[email protected]
Signed-off-by: Danilo Krummrich <[email protected]>
Use guard(spinlock_irqsave)(&dev->devres_lock) where it improves the
code.
Some places still use manual spin_lock_irqsave() and spin_unlock() as
changing it to use a scoped_guard() would result in unnecessary churn.
Reviewed-by: Greg Kroah-Hartman <[email protected]>
Link: https://patch.msgid.link/[email protected]
Signed-off-by: Danilo Krummrich <[email protected]>
Currently, there are three "subclasses" of struct devres_node, which are
struct devres, struct devres_group, struct devres_action.
release_nodes(), which only knows about the base struct devres_node,
assumes that for all "subclasses" struct devres_node is the first member
in the structure and calls kfree() on struct devres_node.
While this technically works, we can still improve semantical
correctness and type safety with a corresponding free_node() callback.
Additionally, we will need this callback soon in the Rust Devres code,
to allocate and free the required memory on the Rust side.
Reviewed-by: Greg Kroah-Hartman <[email protected]>
Link: https://patch.msgid.link/[email protected]
Signed-off-by: Danilo Krummrich <[email protected]>
Currently, devres actions are allocated with devres_alloc(), which
allocates a struct devres with a flexible array member for the actual
data of the resource. The flexible array member is aligned to
ARCH_DMA_MINALIGN, which is wasteful for devres actions that only need
to store a struct action_devres.
Introduce struct devres_action to handle devres actions separately from
struct devres, analogous to what we do for struct devres_group.
Speaking of which, without this patch struct devres_group is treated as
struct devres in release_nodes(). While this is not an actual bug, as
release callbacks for devres nodes in struct devres_group are empty
functions anyways, it is a bit messy and can be confusing.
(Note that besides devres actions, the Rust devres code will also make
use of this. The Rust compiler can figure out the correct alignment of T
in Devres<T> itself, i.e. no need to force a minimum alignment.)
Reviewed-by: Greg Kroah-Hartman <[email protected]>
Link: https://patch.msgid.link/[email protected]
[ Add missing node->release check in devres_for_each_res() and
find_dr(); use kzalloc_obj(). - Danilo ]
Signed-off-by: Danilo Krummrich <[email protected]>
Both devres_add() and devres_open_group() acquire the devres_lock and
call add_dr(). Add a helper, devres_node_add(), for this pattern.
Use guard(spinlock_irqsave) to avoid the explicit unlock call and local
flag variables.
Reviewed-by: Greg Kroah-Hartman <[email protected]>
Link: https://patch.msgid.link/[email protected]
Signed-off-by: Danilo Krummrich <[email protected]>
This is the result of running the Coccinelle script from
scripts/coccinelle/api/kmalloc_objs.cocci. The script is designed to
avoid scalar types (which need careful case-by-case checking), and
instead replace kmalloc-family calls that allocate struct or union
object instances:
Single allocations: kmalloc(sizeof(TYPE), ...)
are replaced with: kmalloc_obj(TYPE, ...)
Array allocations: kmalloc_array(COUNT, sizeof(TYPE), ...)
are replaced with: kmalloc_objs(TYPE, COUNT, ...)
Flex array allocations: kmalloc(struct_size(PTR, FAM, COUNT), ...)
are replaced with: kmalloc_flex(*PTR, FAM, COUNT, ...)
(where TYPE may also be *VAR)
The resulting allocations no longer return "void *", instead returning
"TYPE *".
Signed-off-by: Kees Cook <[email protected]>
Provide a function similar to devm_strdup_const() but for copying blocks
of memory that are likely to be placed in .rodata.
Reviewed-by: Andy Shevchenko <[email protected]>
Acked-by: Greg Kroah-Hartman <[email protected]>
Tested-by: Neil Armstrong <[email protected]>
Signed-off-by: Bartosz Golaszewski <[email protected]>
Signed-off-by: Linus Walleij <[email protected]>
Immutable tag for the driver core tree to pull from
devres: Move devm_*_action*() APIs to devres.h
devres: Add devm_is_action_added() helper
* tag 'gpiod-devm-is-action-added-for-v6.16-rc1' of ssh://gitolite.kernel.org/pub/scm/linux/kernel/git/brgl/linux:
devres: Add devm_is_action_added() helper
devres: Move devm_*_action*() APIs to devres.h
When releasing a device, if the release action causes a group to be
released, a warning is emitted because it can't find the group. This
happens because devres_release_all() moves the entire list to a todo
list and also move the group markers. Considering r* normal resource
nodes and g1 a group resource node:
g1 -----------.
v v
r1 -> r2 -> g1[0] -> r3-> g[1] -> r4
After devres_release_all(), dev->devres_head becomes empty and the todo
list it iterates on becomes:
g1
v
r1 -> r2 -> r3-> r4 -> g1[0]
When a call to component_del() is made and takes down the aggregate
device, a warning like this happen:
RIP: 0010:devres_release_group+0x362/0x530
...
Call Trace:
<TASK>
component_unbind+0x156/0x380
component_unbind_all+0x1d0/0x270
mei_component_master_unbind+0x28/0x80 [mei_hdcp]
take_down_aggregate_device+0xc1/0x160
component_del+0x1c6/0x3e0
intel_hdcp_component_fini+0xf1/0x170 [xe]
xe_display_fini+0x1e/0x40 [xe]
Because the devres group corresponding to the hdcp component cannot be
found. Just ignore this corner case: if the dev->devres_head is empty
and the caller is trying to remove a group, it's likely in the process
of device cleanup so just ignore it instead of warning.
Acked-by: Greg Kroah-Hartman <[email protected]>
Reviewed-by: Rodrigo Vivi <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
Signed-off-by: Lucas De Marchi <[email protected]>
devm_remove_action() warns if the action to remove does not exist
(anymore).
The Rust devres abstraction, however, has a use-case to call
devm_remove_action() at a point where it can't be guaranteed that the
corresponding action hasn't been released yet.
In particular, an instance of `Devres<T>` may be dropped after the
action has been released. So far, `Devres<T>` worked around this by
keeping the inner type alive.
Hence, add devm_remove_action_nowarn(), which returns an error code if
the action has been removed already.
A subsequent patch uses devm_remove_action_nowarn() to remove the action
when `Devres<T>` is dropped.
Signed-off-by: Danilo Krummrich <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Greg Kroah-Hartman <[email protected]>
devm_free_percpu() calls devres_release() with a pointer in percpu
address space. devres_release() expects pointers in the generic address
space, so address space needs to be stripped from the argument.
When strict percpu address space checks are enabled, then the current
direct cast from the percpu address space to the generic address space
fails the compilation on x86_64 with:
devres.c:1234:32: error: cast to generic address space pointer from disjoint ‘__seg_gs’ address space pointer
Add intermediate casts to unsigned long to remove address space of
the pointer before casting it to the generic AS, as advised in [1]
and [2].
Side note: sparse still requires __force, although the documentation
[2] allows casts to unsigned long without __force attribute.
Found by GCC's named address space checks.
There were no changes in the resulting object file.
[1] https://gcc.gnu.org/onlinedocs/gcc/Named-Address-Spaces.html#x86-Named-Address-Spaces
[2] https://sparse.docs.kernel.org/en/latest/annotations.html#address-space-name
Signed-off-by: Uros Bizjak <[email protected]>
Cc: Greg Kroah-Hartman <[email protected]>
Cc: Rafael J. Wysocki <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Greg Kroah-Hartman <[email protected]>
It will cause memory leakage when use driver API devm_free_percpu()
to free memory allocated by devm_alloc_percpu(), fixed by using
devres_release() instead of devres_destroy() within devm_free_percpu().
Fixes: ff86aae3b4 ("devres: add devm_alloc_percpu()")
Cc: [email protected]
Signed-off-by: Zijun Hu <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Greg Kroah-Hartman <[email protected]>
Driver API devm_krealloc() calls alloc_dr() with wrong argument
@total_new_size, so causes more memory to be allocated than required
fix this memory waste by using @new_size as the argument for alloc_dr().
Fixes: f82485722e ("devres: provide devm_krealloc()")
Cc: [email protected]
Signed-off-by: Zijun Hu <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Greg Kroah-Hartman <[email protected]>
If the gfp flag used for the memory allocation already has __GFP_ZERO,
then there is no need to explicitly clear the "struct devres_node". It is
already zeroed.
This saves a few cycles when using devm_zalloc() and co.
In the case of devres_alloc() (which calls __devres_alloc_node()), the
compiler could remove the test and the memset() because it should be able
to see that the __GFP_ZERO flag is set.
So this would make the code both faster and smaller.
Signed-off-by: Christophe JAILLET <[email protected]>
Link: https://lore.kernel.org/r/d255bd871484e63cdd628e819f929e2df59afb02.1658352383.git.christophe.jaillet@wanadoo.fr
Signed-off-by: Greg Kroah-Hartman <[email protected]>
In some cases the printf() mechanism is too heavy and can't be used.
For example, when debugging a race condition involving devres API.
When CONFIG_DEBUG_DEVRES is enabled I can't reproduce an issue, and
otherwise it's quite visible with a useful information being collected.
Enable trace events for devres part of the driver core.
Signed-off-by: Andy Shevchenko <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Greg Kroah-Hartman <[email protected]>
It seems for the sake of saving stack memory of couple of pointers,
the locking in release_nodes() callers becomes interesting.
Replace this logic with a straight forward locking and unlocking scheme.
Signed-off-by: Andy Shevchenko <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Greg Kroah-Hartman <[email protected]>
Implement the managed variant of krealloc(). This function works with
all memory allocated by devm_kmalloc() (or devres functions using it
implicitly like devm_kmemdup(), devm_kstrdup() etc.).
Managed realloc'ed chunks can be manually released with devm_kfree().
Signed-off-by: Bartosz Golaszewski <[email protected]>
Reviewed-by: Andy Shevchenko <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Greg Kroah-Hartman <[email protected]>
Patch series "mm/devm_memremap_pages: Fix page release race", v2.
Logan audited the devm_memremap_pages() shutdown path and noticed that
it was possible to proceed to arch_remove_memory() before all potential
page references have been reaped.
Introduce a new ->cleanup() callback to do the work of waiting for any
straggling page references and then perform the percpu_ref_exit() in
devm_memremap_pages_release() context.
For p2pdma this involves some deeper reworks to reference count
resources on a per-instance basis rather than a per pci-device basis. A
modified genalloc api is introduced to convey a driver-private pointer
through gen_pool_{alloc,free}() interfaces. Also, a
devm_memunmap_pages() api is introduced since p2pdma does not
auto-release resources on a setup failure.
The dax and pmem changes pass the nvdimm unit tests, and the p2pdma
changes should now pass testing with the pci_p2pdma_release() fix.
Jrme, how does this look for HMM?
This patch (of 6):
The devm_add_action() facility allows a resource allocation routine to
add custom devm semantics. One such user is devm_memremap_pages().
There is now a need to manually trigger
devm_memremap_pages_release(). Introduce devm_release_action() so the
release action can be triggered via a new devm_memunmap_pages() api in a
follow-on change.
Link: http://lkml.kernel.org/r/155727336530.292046.2926860263201336366.stgit@dwillia2-desk3.amr.corp.intel.com
Signed-off-by: Dan Williams <[email protected]>
Reviewed-by: Ira Weiny <[email protected]>
Reviewed-by: Logan Gunthorpe <[email protected]>
Cc: Bjorn Helgaas <[email protected]>
Cc: Christoph Hellwig <[email protected]>
Cc: Greg Kroah-Hartman <[email protected]>
Cc: "Rafael J. Wysocki" <[email protected]>
Cc: "Jérôme Glisse" <[email protected]>
Signed-off-by: Andrew Morton <[email protected]>
Signed-off-by: Linus Torvalds <[email protected]>
Provide a resource managed version of kstrdup_const(). This variant
internally calls devm_kstrdup() on pointers that are outside of
.rodata section and returns the string as is otherwise.
Make devm_kfree() check if the passed pointer doesn't point to .rodata
and if so - don't actually destroy the resource.
Signed-off-by: Bartosz Golaszewski <[email protected]>
Reviewed-by: Bjorn Andersson <[email protected]>
Acked-by: Mike Rapoport <[email protected]>
Acked-by: Rasmus Villemoes <[email protected]>
Reviewed-by: Geert Uytterhoeven <[email protected]>
Reviewed-by: Andy Shevchenko <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>
Use the overflow helpers both in existing multiplication-using inlines as
well as the addition-overflow case in the core allocation routine.
Signed-off-by: Kees Cook <[email protected]>
Now that the SPDX tag is in all driver core files, that identifies the
license in a specific and legally-defined manner. So the extra GPL text
wording can be removed as it is no longer needed at all.
This is done on a quest to remove the 700+ different ways that files in
the kernel describe the GPL license text. And there's unneeded stuff
like the address (sometimes incorrect) for the FSF which is never
needed.
No copyright headers or other non-license-description text was removed.
Cc: Johannes Berg <[email protected]>
Cc: "Luis R. Rodriguez" <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>
It's good to have SPDX identifiers in all files to make it easier to
audit the kernel tree for correct licenses.
Update the driver core files files with the correct SPDX license
identifier based on the license text in the file itself. The SPDX
identifier is a legally binding shorthand, which can be used instead of
the full boiler plate text.
This work is based on a script and data from Thomas Gleixner, Philippe
Ombredanne, and Kate Stewart.
Cc: Johannes Berg <[email protected]>
Cc: "Luis R. Rodriguez" <[email protected]>
Cc: William Breathitt Gray <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: Kate Stewart <[email protected]>
Cc: Philippe Ombredanne <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>
Introduce managed counterparts for alloc_percpu() and free_percpu().
Add devm_alloc_percpu() and devm_free_percpu() into the managed
interfaces list.
Signed-off-by: Madalin Bucur <[email protected]>
Signed-off-by: David S. Miller <[email protected]>
Danilo Krummrich