Add fallible allocation methods #325
Conversation
These are all fallible alternatives to all currently used methods that may perform infallible allocation. `assume_fallible` is a semantically marker function to denote that we have proved that the closure passed to it will not perform infallible allocation. Signed-off-by: Gary Guo <gary@garyguo.net>
|
Review of
|
💖 That tool looks really nice! |
|
Nice! Now that allegedly there's no more need for fallible allocations, is it perhaps possible to disable them, i.e. make the build fail if someone tries to use them? And would that be preferable to using a linter for that purpose? |
Ideally we could just disable the infallible methods, but we couldn't do it yet as otherwise we have to re-implement most of So I thought it might be a good compromise to use a linter as a temporary solution until we're ready to disable them completely. P.S. I always find infallible and fallible to be kind of confusing, since infallible allocation means the allocation is not allowed to fail (but allowed to |
|
This is very nice -- Clippy-like kernel lints for the Rust side is something that we definitely want to have. And integrating with the compiler is a better approach than e.g. the regexp one used by some of the tools for C (nothing against that -- I assume they went that way because writing compiler plugins for GCC was not easy many years ago). I was thinking we would have such a tool as a "Clippy fork", i.e. adding our checks there, but I guess an independent tool also makes sense -- not sure about pros/cons. On the allocations: I am not sure about putting the changes in, since we have to remove the allocations soon anyway (I will be focusing on that this week). |
Oh great! Do you plan on using the no_global_oom_handling config and use extension traits or an liballoc fork? I think it's best to avoid forking liballoc as it depends on a ton of nightly features that we don't really want to depend on. Some examples include lang items, fundamental, coerce unsized, custom receiver trait, unsized local, etc etc. |
|
The three times I re-started that (and then got sidetracked), I went the forking way ( But yeah, we will see how it looks. If it is too bad, we can definitely go with |
|
Close as we have #402 now |
Commit 5f64ce5 ("iommu/vt-d: Duplicate iommu_resv_region objects per device list") converted rcu_lock in get_resv_regions to dmar_global_lock to allow sleeping in iommu_alloc_resv_region(). This introduced possible recursive locking if get_resv_regions is called from within a section where intel_iommu_init() already holds dmar_global_lock. Especially, after commit 57365a0 ("iommu: Move bus setup to IOMMU device registration"), below lockdep splats could always be seen. ============================================ WARNING: possible recursive locking detected 6.0.0-rc4+ #325 Tainted: G I -------------------------------------------- swapper/0/1 is trying to acquire lock: ffffffffa8a18c90 (dmar_global_lock){++++}-{3:3}, at: intel_iommu_get_resv_regions+0x25/0x270 but task is already holding lock: ffffffffa8a18c90 (dmar_global_lock){++++}-{3:3}, at: intel_iommu_init+0x36d/0x6ea ... Call Trace: <TASK> dump_stack_lvl+0x48/0x5f __lock_acquire.cold.73+0xad/0x2bb lock_acquire+0xc2/0x2e0 ? intel_iommu_get_resv_regions+0x25/0x270 ? lock_is_held_type+0x9d/0x110 down_read+0x42/0x150 ? intel_iommu_get_resv_regions+0x25/0x270 intel_iommu_get_resv_regions+0x25/0x270 iommu_create_device_direct_mappings.isra.28+0x8d/0x1c0 ? iommu_get_dma_cookie+0x6d/0x90 bus_iommu_probe+0x19f/0x2e0 iommu_device_register+0xd4/0x130 intel_iommu_init+0x3e1/0x6ea ? iommu_setup+0x289/0x289 ? rdinit_setup+0x34/0x34 pci_iommu_init+0x12/0x3a do_one_initcall+0x65/0x320 ? rdinit_setup+0x34/0x34 ? rcu_read_lock_sched_held+0x5a/0x80 kernel_init_freeable+0x28a/0x2f3 ? rest_init+0x1b0/0x1b0 kernel_init+0x1a/0x130 ret_from_fork+0x1f/0x30 </TASK> This rolls back dmar_global_lock to rcu_lock in get_resv_regions to avoid the lockdep splat. Fixes: 57365a0 ("iommu: Move bus setup to IOMMU device registration") Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com> Tested-by: Alex Williamson <alex.williamson@redhat.com> Link: https://lore.kernel.org/r/20220927053109.4053662-3-baolu.lu@linux.intel.com Signed-off-by: Joerg Roedel <jroedel@suse.de>
These are all fallible alternatives to all currently used methods that may perform infallible allocation.
assume_fallibleis a semantically marker function to denote that we have proved that the closure passed to it will not perform infallible allocation.After this change, the code can be verified to not perform any infallible allocation using the tool I have just completed: https://github.com/nbdd0121/klint
(you can try the tool on Linux repo by
rustup run nightly make CLIPPY_DRIVER=klint CLIPPY=1, beware it's quite noisy when compiling libmacros!)