Having the ARM PMUv3 driver sitting in arch/arm64/kernel is getting
in the way of being able to use perf on ARMv8 cores running a 32bit
kernel, such as 32bit KVM guests.
This patch moves it into drivers/perf/arm_pmuv3.c, with an include
file in include/linux/perf/arm_pmuv3.h. The only thing left in
arch/arm64 is some mundane perf stuff.
Change-Id: I1928ceb684a107ef991401175458f2ea6c711826
Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
Signed-off-by: Zaid Al-Bassam <zalbassam@google.com>
Tested-by: Florian Fainelli <f.fainelli@gmail.com>
Link: https://lore.kernel.org/r/20230317195027.3746949-2-zalbassam@google.com
Signed-off-by: Will Deacon <will@kernel.org>
(cherry picked from commit 7755cec63a)
Signed-off-by: Danesh Petigara <danesh.petigara@broadcom.com>
The logic in blk_mq_complete_need_ipi() assumes SMP systems where all
CPUs have equal compute capacities and only LLC cache can make
a different on perceived performance. But this assumption falls apart on
HMP systems where LLC is shared, but the CPUs have different capacities.
Staying local then can have a big performance impact if the IO request
was done from a CPU with higher capacity but the interrupt is serviced
on a lower capacity CPU.
Use the new cpus_equal_capacity() function to check if we need to send
an IPI.
Without the patch I see the BLOCK softirq always running on little cores
(where the hardirq is serviced). With it I can see it running on all
cores.
This was noticed after the topology change [1] where now on a big.LITTLE
we truly get that the LLC is shared between all cores where as in the
past it was being misrepresented for historical reasons. The logic
exposed a missing dependency on capacities for such systems where there
can be a big performance difference between the CPUs.
This of course introduced a noticeable change in behavior depending on
how the topology is presented. Leading to regressions in some workloads
as the performance of the BLOCK softirq on littles can be noticeably
worse on some platforms.
Worth noting that we could have checked for capacities being greater
than or equal instead for equality. This will lead to favouring higher
performance always. But opted for equality instead to match the
performance of the requester without making an assumption that can lead
to power trade-offs which these systems tend to be sensitive about. If
the requester would like to run faster, it's better to rely on the
scheduler to give the IO requester via some facility to run on a faster
core; and then if the interrupt triggered on a CPU with different
capacity we'll make sure to match the performance the requester is
supposed to run at.
[1] https://lpc.events/event/16/contributions/1342/attachments/962/1883/LPC-2022-Android-MC-Phantom-Domains.pdf
Bug: 341551538
Signed-off-by: Qais Yousef <qyousef@layalina.io>
Reviewed-by: Bart Van Assche <bvanassche@acm.org>
Link: https://lore.kernel.org/r/20240223155749.2958009-3-qyousef@layalina.io
Signed-off-by: Jens Axboe <axboe@kernel.dk>
(cherry picked from commit af550e4c968294398fc76b075f12d51c76caf753)
Signed-off-by: Qais Yousef <qyousef@google.com>
(cherry picked from https://android-review.googlesource.com/q/commit:d01dc1c7d020f49f31194254a89694bd23ad3dd5)
Merged-In: Iefed6a19b9d25102642e264118431f5e12e23fea
Change-Id: Iefed6a19b9d25102642e264118431f5e12e23fea
FFS based applications can utilize the aio_cancel() callback to dequeue
pending USB requests submitted to the UDC. There is a scenario where the
FFS application issues an AIO cancel call, while the UDC is handling a
soft disconnect. For a DWC3 based implementation, the callstack looks
like the following:
DWC3 Gadget FFS Application
dwc3_gadget_soft_disconnect() ...
--> dwc3_stop_active_transfers()
--> dwc3_gadget_giveback(-ESHUTDOWN)
--> ffs_epfile_async_io_complete() ffs_aio_cancel()
--> usb_ep_free_request() --> usb_ep_dequeue()
There is currently no locking implemented between the AIO completion
handler and AIO cancel, so the issue occurs if the completion routine is
running in parallel to an AIO cancel call coming from the FFS application.
As the completion call frees the USB request (io_data->req) the FFS
application is also referencing it for the usb_ep_dequeue() call. This can
lead to accessing a stale/hanging pointer.
commit b566d38857 ("usb: gadget: f_fs: use io_data->status consistently")
relocated the usb_ep_free_request() into ffs_epfile_async_io_complete().
However, in order to properly implement locking to mitigate this issue, the
spinlock can't be added to ffs_epfile_async_io_complete(), as
usb_ep_dequeue() (if successfully dequeuing a USB request) will call the
function driver's completion handler in the same context. Hence, leading
into a deadlock.
Fix this issue by moving the usb_ep_free_request() back to
ffs_user_copy_worker(), and ensuring that it explicitly sets io_data->req
to NULL after freeing it within the ffs->eps_lock. This resolves the race
condition above, as the ffs_aio_cancel() routine will not continue
attempting to dequeue a request that has already been freed, or the
ffs_user_copy_work() not freeing the USB request until the AIO cancel is
done referencing it.
This fix depends on
commit b566d38857 ("usb: gadget: f_fs: use io_data->status
consistently")
Fixes: 2e4c7553cd ("usb: gadget: f_fs: add aio support")
Cc: stable <stable@kernel.org> # b566d38857 ("usb: gadget: f_fs: use io_data->status consistently")
Signed-off-by: Wesley Cheng <quic_wcheng@quicinc.com>
Link: https://lore.kernel.org/r/20240409014059.6740-1-quic_wcheng@quicinc.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Bug: 334976932
(cherry picked from commit 24729b307eefcd7c476065cd7351c1a018082c19)
Change-Id: I56f6b9d24c239e73edff94e1f9f33ab41a9bd37b
Signed-off-by: Prashanth K <quic_prashk@quicinc.com>
Commit fb1f16d74e ("usb: gadget: f_fs: change ep->status safe in
ffs_epfile_io()") added a new ffs_io_data::status field to fix lifetime
issues in synchronous requests.
While there are no similar lifetime issues for asynchronous requests
(the separate ep member in ffs_io_data avoids them) using the status
field means the USB request can be freed earlier and that there is more
consistency between the synchronous and asynchronous I/O paths.
Cc: Linyu Yuan <quic_linyyuan@quicinc.com>
Signed-off-by: John Keeping <john@metanate.com>
Reviewed-by: Linyu Yuan <quic_linyyuan@quicinc.com>
Link: https://lore.kernel.org/r/20221124170430.3998755-1-john@metanate.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Bug: 334976932
(cherry picked from commit b566d38857)
Change-Id: I8cd4d95ff6ec694adac3881da80eff23f6c679d4
Signed-off-by: Prashanth K <quic_prashk@quicinc.com>
For GKI targets with kmi_symbol_list set,
also set rewrite_absolute_paths_in_config to True
so that CONFIG_UNUSED_KSYMS_WHITELIST is not an
absolute path any more, increasing reproducibility
on different machines.
This is made possible with c6427490ab
"Revert^2 "BACKPORT: FROMGIT: module: allow UNUSED_KSYMS_WHITELIST to be relative against objtree.""
so that Kbuild recognizes relative paths by searching
$(objtree) first.
This does not change x86 builds because it
doesn't have kmi_symbol_list set.
This change does not affect device builds like
db845c, rockpi and fips140. Adding the attribute
to these targets would be okay, but it is not
covered by this change.
Bug: 333769605
Bug: 342390208
Change-Id: I844c251979bae5277474837eaa055f1346b0afa2
Signed-off-by: Yifan Hong <elsk@google.com>
wdev->valid_links is not cleared when upper layer disconnect from a
wdev->AP MLD. It has been observed that this would prevent offchannel
operations like remain-on-channel which would be needed for user space
operations with Public Action frame.
Clear the wdev->valid_links when STA disconnects.
Signed-off-by: Xin Deng <quic_deng@quicinc.com>
Link: https://msgid.link/20240426092501.8592-1-quic_deng@quicinc.com
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
Bug: 344476904
Change-Id: Ie84dda35e26d2792ed39f50d0c3f959682730d89
(cherry picked from commit 9f6d4b8d149af8dc3f9a1e3000168b99ca576390)
Signed-off-by: Xin Deng <quic_deng@quicinc.com>
Add the usb gadget connect/disconnect symbols to ABI to be used by
vendor driver:
usb_gadget_connect
usb_gadget_disconnect
Bug: 344835325
Change-Id: I506d142ed863e6ce0d21b7a0e31c1721f6bdba93
Signed-off-by: Udipto Goswami <quic_ugoswami@quicinc.com>
This is a step toward making __iommu_probe_device() self contained.
It should, under proper locking, check if the device is already associated
with an iommu driver and resolve parallel probes. All but one of the
callers open code this test using two different means, but they all
rely on dev->iommu_group.
Currently the bus_iommu_probe()/probe_iommu_group() and
probe_acpi_namespace_devices() rejects already probed devices with an
unlocked read of dev->iommu_group. The OF and ACPI "replay" functions use
device_iommu_mapped() which is the same read without the pointless
refcount.
Move this test into __iommu_probe_device() and put it under the
iommu_probe_device_lock. The store to dev->iommu_group is in
iommu_group_add_device() which is also called under this lock for iommu
driver devices, making it properly locked.
The only path that didn't have this check is the hotplug path triggered by
BUS_NOTIFY_ADD_DEVICE. The only way to get dev->iommu_group assigned
outside the probe path is via iommu_group_add_device(). Today the only
caller is VFIO no-iommu which never associates with an iommu driver. Thus
adding this additional check is safe.
Bug: 337990354
Reviewed-by: Kevin Tian <kevin.tian@intel.com>
Reviewed-by: Lu Baolu <baolu.lu@linux.intel.com>
Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
Link: https://lore.kernel.org/r/1-v3-328044aa278c+45e49-iommu_probe_jgg@nvidia.com
Signed-off-by: Joerg Roedel <jroedel@suse.de>
(cherry picked from commit 6eb4da8cf5)
Change-Id: I079ee5467e96367a5e1aa2ae5f0d5f6837df597c
[quic_nprakash: Resolved conflicts in drivers/iommu/iommu.c]
Signed-off-by: Nikhil V <quic_nprakash@quicinc.com>
In commit 92f1655aa2b2 ("net: fix __dst_negative_advice() race") the
struct dst_ops callback negative_advice is callback changes function
parameters. But as this pointer is part of a structure that is tracked
in the ABI checker, the tool triggers when this is changed.
However, the callback pointer is internal to the networking stack, so
changing the function type is safe, so needing to preserve this is not
required. To do so, switch the function pointer type back to the old
one so that the checking tools pass, AND then do a hard cast of the
function pointer to the new type when assigning and calling the
function.
[6.1.y backport note, work around --Werror=cast-function-type issue by
abusing void * for function pointer types, despite its best effort, C
still let's us shoot our foot off if we really want to!]
Bug: 343727534
Fixes: 92f1655aa2b2 ("net: fix __dst_negative_advice() race")
Change-Id: I48d4ab4bbd29f8edc8fbd7923828b7f78a23e12e
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
__dst_negative_advice() does not enforce proper RCU rules when
sk->dst_cache must be cleared, leading to possible UAF.
RCU rules are that we must first clear sk->sk_dst_cache,
then call dst_release(old_dst).
Note that sk_dst_reset(sk) is implementing this protocol correctly,
while __dst_negative_advice() uses the wrong order.
Given that ip6_negative_advice() has special logic
against RTF_CACHE, this means each of the three ->negative_advice()
existing methods must perform the sk_dst_reset() themselves.
Note the check against NULL dst is centralized in
__dst_negative_advice(), there is no need to duplicate
it in various callbacks.
Many thanks to Clement Lecigne for tracking this issue.
This old bug became visible after the blamed commit, using UDP sockets.
Bug: 343727534
Fixes: a87cb3e48e ("net: Facility to report route quality of connected sockets")
Reported-by: Clement Lecigne <clecigne@google.com>
Diagnosed-by: Clement Lecigne <clecigne@google.com>
Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: Tom Herbert <tom@herbertland.com>
Reviewed-by: David Ahern <dsahern@kernel.org>
Link: https://lore.kernel.org/r/20240528114353.1794151-1-edumazet@google.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
(cherry picked from commit 92f1655aa2b2294d0b49925f3b875a634bd3b59e)
[Lee: Trivial/unrelated conflict - no change to the patch]
Signed-off-by: Lee Jones <joneslee@google.com>
Change-Id: I293734dca1b81fcb712e1de294f51e96a405f7e4
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
udc device and gadget device are tightly coupled, yet there's no good
way to corelate the two. Add a sysfs link in udc that points to the
corresponding gadget device.
An example use case: userspace configures a f_midi configfs driver and
bind the udc device, then it tries to locate the corresponding midi
device, which is a child device of the gadget device. The gadget device
that's associated to the udc device has to be identified in order to
index the midi device. Having a sysfs link would make things much
easier.
Signed-off-by: Roy Luo <royluo@google.com>
Link: https://lore.kernel.org/r/20240307030922.3573161-1-royluo@google.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Bug: 333778731
Change-Id: I9e3f782543eba5e026a65031aaae754daafb69ab
(cherry picked from commit 0ef40f399aa2be8c04aee9b7430705612c104ce5)
Signed-off-by: Roy Luo <royluo@google.com>
Enabling configs for Tegra SoCs required this module to be included in
the rockpi4 builds. This change fixes the following build issue:
ERROR: The following kernel modules are built but not copied. Add these lines to the module_outs attribute of @@//common:rockpi4_no_kgdb:
"drivers/cpufreq/tegra20-cpufreq.ko",
Bug: 335325897
Change-Id: I898000725287f34956cb977b3b02685f189cd5a0
Signed-off-by: Carlos Llamas <cmllamas@google.com>
Enabling configs for Tegra SoCs required these symbols to be included.
This change fixes the following build issues:
ERROR: modpost: "devm_tegra_memory_controller_get" [drivers/iommu/arm/arm-smmu/arm_smmu.ko] undefined!
ERROR: modpost: "tegra_mc_probe_device" [drivers/iommu/arm/arm-smmu/arm_smmu.ko] undefined!
Also update the STG file accordingly:
2 function symbol(s) added
'struct tegra_mc* devm_tegra_memory_controller_get(struct device*)'
'int tegra_mc_probe_device(struct tegra_mc*, struct device*)'
Bug: 335325897
Change-Id: Ie357116189e074f1dc0e572553b2264abc910bd3
Signed-off-by: Carlos Llamas <cmllamas@google.com>
Build some configs that are tristate and selected by
CONFIG_ARCH_TEGRA as vendor modules rather than built-in.
Bug: 335325897
Change-Id: I7ce4cf975fcbc834bc5fc3c90317d9d81162185a
Signed-off-by: Ahmad Chaudhry <ahmadc@nvidia.com>
To use cpumask_parse_user in vendor code, it will call
bitmap_parse_user.
Adding the following symbols:
- bitmap_parse_user
Bug: 248621289
Change-Id: I4b24d8b1fba71af7b55c7444963a003d50f6d93c
Signed-off-by: Rick Yiu <rickyiu@google.com>
This reverts commit 3f13972470.
Reason for revert: relanding change that should be safe to go in on its own. Below is the original commit message.
BACKPORT: FROMGIT: module: allow UNUSED_KSYMS_WHITELIST to be relative against objtree.
If UNUSED_KSYMS_WHITELIST is a file generated
before Kbuild runs, and the source tree is in
a read-only filesystem, the developer must put
the file somewhere and specify an absolute
path to UNUSED_KSYMS_WHITELIST. This worked,
but if IKCONFIG=y, an absolute path is embedded
into .config and eventually into vmlinux, causing
the build to be less reproducible when building
on a different machine.
This patch makes the handling of
UNUSED_KSYMS_WHITELIST to be similar to
MODULE_SIG_KEY.
First, check if UNUSED_KSYMS_WHITELIST is an
absolute path, just as before this patch. If so,
use the path as is.
If it is a relative path, use wildcard to check
the existence of the file below objtree first.
If it does not exist, fall back to the original
behavior of adding $(srctree)/ before the value.
After this patch, the developer can put the generated
file in objtree, then use a relative path against
objtree in .config, eradicating any absolute paths
that may be evaluated differently on different machines.
Signed-off-by: Yifan Hong <elsk@google.com>
Reviewed-by: Elliot Berman <quic_eberman@quicinc.com>
Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
(cherry picked from commit a2e3c811938b4902725e259c03b2d6c539613992
https://git.kernel.org/pub/scm/linux/kernel/git/mcgrof/linux.git modules-next)
Bug: 333769605
Change-Id: I442bbe5a68fc895619b84b4749f1e97cb88088ea
[elsk: apply change to gen_autoksyms.sh instead because
CONFIG_UNUSED_KSYMS_WHITELIST is parsed there. Revert change
to Makefile.modpost.]
Bug: 342390208
Signed-off-by: Yifan Hong <elsk@google.com>
Wrap 'struct binder_proc' inside 'struct binder_proc_wrap' to add the
alloc->lock equivalent without breaking the KMI. Also, add convenient
apis to access/modify this new spinlock.
Without this patch, the following KMI issues show up:
type 'struct binder_proc' changed
byte size changed from 616 to 576
type 'struct binder_alloc' changed
byte size changed from 152 to 112
member 'spinlock_t lock' was added
member 'struct mutex mutex' was removed
Bug: 254650075
Bug: 319778300
Change-Id: Ic31dc39fb82800a3e47be10a7873cd210f7b60be
Signed-off-by: Carlos Llamas <cmllamas@google.com>
[cmllamas: fixed trivial conflicts]
The correct printk format specifier when calculating buffer offsets
should be "%tx" as it is a pointer difference (a.k.a ptrdiff_t). This
fixes some W=1 build warnings reported by the kernel test robot.
Bug: 329799092
Fixes: 63f7ddea2e ("ANDROID: binder: fix KMI-break due to address type change")
Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202401100511.A4BKMwoq-lkp@intel.com/
Change-Id: Iaa87433897b507c47fe8601464445cb6de4b61db
Signed-off-by: Carlos Llamas <cmllamas@google.com>
In commit ("binder: keep vma addresses type as unsigned long") the vma
address type in 'struct binder_alloc' and 'struct binder_buffer' is
changed from 'void __user *' to 'unsigned long'.
This triggers the following KMI issues:
type 'struct binder_buffer' changed
member changed from 'void* user_data' to 'unsigned long user_data'
type changed from 'void*' to 'unsigned long'
type 'struct binder_alloc' changed
member changed from 'void* buffer' to 'unsigned long buffer'
type changed from 'void*' to 'unsigned long'
This offending commit is being backported as part of a larger patchset
from upstream in [1]. Lets fix these issues by doing a partial revert
that restores the original types and casts to an integer type where
necessary.
Note this approach is preferred over dropping the single KMI-breaking
patch from the backport, as this would have created non-trivial merge
conflicts in the subsequent cherry-picks.
Bug: 254650075
Bug: 319778300
Link: https://lore.kernel.org/all/20231201172212.1813387-1-cmllamas@google.com/ [1]
Change-Id: Ief9de717d0f34642f5954ffa2e306075a5b4e02e
Signed-off-by: Carlos Llamas <cmllamas@google.com>
[cmllamas: fixed trivial conflicts]
This reverts commit 637c8e0d372f1dfff53337a5db89f772577828d7.
Also squash commit db91c5d31a ("ANDROID: vendor_hook: rename the the
name of hooks"), which fixes the length of the vendor hook's name.
Rework the error return for a goto as this has been refactor too.
Finally, also fix spaces vs tabs.
Change-Id: I22c495eb81237c51c0f9f4d4f9f4f1cf0c8438a8
Signed-off-by: Carlos Llamas <cmllamas@google.com>
This reverts commit eeb899e9f54bef5286fd5044db481ecc01e417b4.
Change-Id: I810727a6872c16ccb484023bfbc587daca8a2515
Signed-off-by: Carlos Llamas <cmllamas@google.com>
The alloc->mutex is a highly contended lock that causes performance
issues on Android devices. When a low-priority task is given this lock
and it sleeps, it becomes difficult for the task to wake up and complete
its work. This delays other tasks that are also waiting on the mutex.
The problem gets worse when there is memory pressure in the system,
because this increases the contention on the alloc->mutex while the
shrinker reclaims binder pages.
Switching to a spinlock helps to keep the waiters running and avoids the
overhead of waking up tasks. This significantly improves the transaction
latency when the problematic scenario occurs.
The performance impact of this patchset was measured by stress-testing
the binder alloc contention. In this test, several clients of different
priorities send thousands of transactions of different sizes to a single
server. In parallel, pages get reclaimed using the shinker's debugfs.
The test was run on a Pixel 8, Pixel 6 and qemu machine. The results
were similar on all three devices:
after:
| sched | prio | average | max | min |
|--------+------+---------+-----------+---------|
| fifo | 99 | 0.135ms | 1.197ms | 0.022ms |
| fifo | 01 | 0.136ms | 5.232ms | 0.018ms |
| other | -20 | 0.180ms | 7.403ms | 0.019ms |
| other | 19 | 0.241ms | 58.094ms | 0.018ms |
before:
| sched | prio | average | max | min |
|--------+------+---------+-----------+---------|
| fifo | 99 | 0.350ms | 248.730ms | 0.020ms |
| fifo | 01 | 0.357ms | 248.817ms | 0.024ms |
| other | -20 | 0.399ms | 249.906ms | 0.020ms |
| other | 19 | 0.477ms | 297.756ms | 0.022ms |
The key metrics above are the average and max latencies (wall time).
These improvements should roughly translate to p95-p99 latencies on real
workloads. The response time is up to 200x faster in these scenarios and
there is no penalty in the regular path.
Note that it is only possible to convert this lock after a series of
changes made by previous patches. These mainly include refactoring the
sections that might_sleep() and changing the locking order with the
mmap_lock amongst others.
Reviewed-by: Alice Ryhl <aliceryhl@google.com>
Signed-off-by: Carlos Llamas <cmllamas@google.com>
Link: https://lore.kernel.org/r/20231201172212.1813387-29-cmllamas@google.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Bug: 254650075
(cherry picked from commit 7710e2cca32e7f3958480e8bd44f50e29d0c2509)
Change-Id: I67121be071d5f072ac0e5eb719c95c0f1dee5eb5
Signed-off-by: Carlos Llamas <cmllamas@google.com>
The locking order currently requires the alloc->mutex to be acquired
first followed by the mmap lock. However, the alloc->mutex is converted
into a spinlock in subsequent commits so the order needs to be reversed
to avoid nesting the sleeping mmap lock under the spinlock.
The shrinker's callback binder_alloc_free_page() is the only place that
needs to be reordered since other functions have been refactored and no
longer nest these locks.
Some minor cosmetic changes are also included in this patch.
Signed-off-by: Carlos Llamas <cmllamas@google.com>
Link: https://lore.kernel.org/r/20231201172212.1813387-28-cmllamas@google.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Bug: 254650075
(cherry picked from commit e50f4e6cc9bfaca655d3b6a3506d27cf2caa1d40)
Change-Id: I7f7501945a477ac5571082a5dd2a7934f484b8ab
Signed-off-by: Carlos Llamas <cmllamas@google.com>
Prefer logging vma offsets instead of addresses or simply drop the debug
log altogether if not useful. Note this covers the instances affected by
the switch to store addresses as unsigned long. However, there are other
sections in the driver that could do the same.
Signed-off-by: Carlos Llamas <cmllamas@google.com>
Reviewed-by: Alice Ryhl <aliceryhl@google.com>
Link: https://lore.kernel.org/r/20231201172212.1813387-27-cmllamas@google.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Bug: 254650075
(cherry picked from commit 162c79731448a5a052e93af7753df579dfe0bf7a)
Change-Id: I92b7f409e45d9006492d56302e911ccdd8efc950
Signed-off-by: Carlos Llamas <cmllamas@google.com>
The code to determine the page range for binder_lru_freelist_del() is
quite obscure. It leverages the buffer_size calculated before doing an
oversized buffer split. This is used to figure out if the last page is
being shared with another active buffer. If so, the page gets trimmed
out of the range as it has been previously removed from the freelist.
This would be equivalent to getting the start page of the next in-use
buffer explicitly. However, the code for this is much larger as we can
see in binder_free_buf_locked() routine. Instead, lets settle on
documenting the tricky step and using better names for now.
I believe an ideal solution would be to count the binder_page->users to
determine when a page should be added or removed from the freelist.
However, this is a much bigger change than what I'm willing to risk at
this time.
Signed-off-by: Carlos Llamas <cmllamas@google.com>
Link: https://lore.kernel.org/r/20231201172212.1813387-24-cmllamas@google.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Bug: 254650075
(cherry picked from commit 67dcc880780569ec40391cae4d8299adc1e7a44e)
Change-Id: Iec2466605fe7f8aa338c8313f586cdb7519a36e7
Signed-off-by: Carlos Llamas <cmllamas@google.com>
Now that the page allocation step is done separately we should rename
the binder_free_page_range() and binder_allocate_page_range() functions
to provide a more accurate description of what they do. Lets borrow the
freelist concept used in other parts of the kernel for this.
No functional change here.
Signed-off-by: Carlos Llamas <cmllamas@google.com>
Reviewed-by: Alice Ryhl <aliceryhl@google.com>
Link: https://lore.kernel.org/r/20231201172212.1813387-23-cmllamas@google.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Bug: 254650075
(cherry picked from commit ea9cdbf0c7273b55e251b2ed8f85794cfadab5d5)
Change-Id: I0d0dfcc6f72d54209da310be2ad5e30f3d722652
[cmllamas: fixed trivial conflicts due to missing commits e33c267ab7
95a542da5322e]
Signed-off-by: Carlos Llamas <cmllamas@google.com>
The sections in binder_alloc_new_buf_locked() dealing with oversized
buffers are scattered which makes them difficult to read. Instead,
consolidate this code into a single block to improve readability.
No functional change here.
Signed-off-by: Carlos Llamas <cmllamas@google.com>
Reviewed-by: Alice Ryhl <aliceryhl@google.com>
Link: https://lore.kernel.org/r/20231201172212.1813387-22-cmllamas@google.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Bug: 254650075
(cherry picked from commit de0e6573125f8ea7a01a9b05a45b0c73116c73b2)
Change-Id: I62c2cec7341e13d9174b4f0839a1345df7cfd808
Signed-off-by: Carlos Llamas <cmllamas@google.com>
Split out the insertion of pages to be outside of the alloc->mutex in a
separate binder_install_buffer_pages() routine. Since this is no longer
serialized, we must look at the full range of pages used by the buffers.
The installation is protected with mmap_sem in write mode since multiple
tasks might race to install the same page.
Besides avoiding unnecessary nested locking this helps in preparation of
switching the alloc->mutex into a spinlock_t in subsequent patches.
Signed-off-by: Carlos Llamas <cmllamas@google.com>
Reviewed-by: Alice Ryhl <aliceryhl@google.com>
Link: https://lore.kernel.org/r/20231201172212.1813387-20-cmllamas@google.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Bug: 254650075
(cherry picked from commit 37ebbb4f73a0d299fa0c7dd043932a2f5fbbb779)
Change-Id: I7b0684310b8824194d7e4a51a1fd67944f8ec06a
Signed-off-by: Carlos Llamas <cmllamas@google.com>
Rather than repeatedly initializing some of the binder_lru_page members
during binder_alloc_new_buf(), perform this initialization just once in
binder_alloc_mmap_handler(), after the pages have been created.
Reviewed-by: Alice Ryhl <aliceryhl@google.com>
Signed-off-by: Carlos Llamas <cmllamas@google.com>
Link: https://lore.kernel.org/r/20231201172212.1813387-19-cmllamas@google.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Bug: 254650075
(cherry picked from commit 68aef12d094e4c96d972790f1620415460a4f3cf)
Change-Id: I3197038683f76a5cb98a79d017d1515429df2d73
Signed-off-by: Carlos Llamas <cmllamas@google.com>
Preallocate new_buffer before acquiring the alloc->mutex and hand it
down to binder_alloc_new_buf_locked(). The new buffer will be used in
the vast majority of requests (measured at 98.2% in field data). The
buffer is discarded otherwise. This change is required in preparation
for transitioning alloc->mutex into a spinlock in subsequent commits.
Signed-off-by: Carlos Llamas <cmllamas@google.com>
Reviewed-by: Alice Ryhl <aliceryhl@google.com>
Link: https://lore.kernel.org/r/20231201172212.1813387-18-cmllamas@google.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Bug: 254650075
(cherry picked from commit c7ac30fad18231a1637d38aa8a97d6b4788ed8ad)
Change-Id: Ib7c8eb3c53e8383694a118fabc776a6a22783c75
Signed-off-by: Carlos Llamas <cmllamas@google.com>
Instead of looping through the page range twice to first determine if
the mmap lock is required, simply do it per-page as needed. Split out
all this logic into a separate binder_install_single_page() function.
Reviewed-by: Alice Ryhl <aliceryhl@google.com>
Signed-off-by: Carlos Llamas <cmllamas@google.com>
Link: https://lore.kernel.org/r/20231201172212.1813387-17-cmllamas@google.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Bug: 254650075
(cherry picked from commit ea2735ce19c1c6ce0f6011f813a1eea0272c231d)
Change-Id: Ic057e9cfaeb22754f99bdec2a51076cf58c86855
Signed-off-by: Carlos Llamas <cmllamas@google.com>
Move the low async space calculation to debug_low_async_space_locked().
This logic not only fits better here but also offloads some of the many
tasks currently done in binder_alloc_new_buf_locked().
No functional change in this patch.
Reviewed-by: Alice Ryhl <aliceryhl@google.com>
Signed-off-by: Carlos Llamas <cmllamas@google.com>
Link: https://lore.kernel.org/r/20231201172212.1813387-15-cmllamas@google.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Bug: 254650075
(cherry picked from commit c13500eaabd2343aa4cbb76b54ec624cb0c0ef8d)
Change-Id: I1b396f59f2a5b6640d8664767f2d45a675af7197
Signed-off-by: Carlos Llamas <cmllamas@google.com>
Move the no-space debugging logic into a separate function. Lets also
mark this branch as unlikely in binder_alloc_new_buf_locked() as most
requests will fit without issue.
Also add a few cosmetic changes and suggestions from checkpatch.
Reviewed-by: Alice Ryhl <aliceryhl@google.com>
Signed-off-by: Carlos Llamas <cmllamas@google.com>
Link: https://lore.kernel.org/r/20231201172212.1813387-14-cmllamas@google.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Bug: 254650075
(cherry picked from commit 9409af24e4503d14093b27db9425f7c99e64fef4)
Change-Id: I4ff8ced5728a63815f7d47df9eb9ac85aa0a362d
Signed-off-by: Carlos Llamas <cmllamas@google.com>
Binder attributes the buffer allocation to the current->tgid everytime.
There is no need to pass this as a parameter so drop it.
Also add a few touchups to follow the coding guidelines. No functional
changes are introduced in this patch.
Reviewed-by: Alice Ryhl <aliceryhl@google.com>
Signed-off-by: Carlos Llamas <cmllamas@google.com>
Link: https://lore.kernel.org/r/20231201172212.1813387-13-cmllamas@google.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Bug: 254650075
(cherry picked from commit 89f71743bf42217dd4092fda703a8e4f6f4e55ac)
Change-Id: Ib21fdc5afd7eeb4723b08913ba40ded762421b0b
Signed-off-by: Carlos Llamas <cmllamas@google.com>
Extract non-critical sections from binder_alloc_new_buf_locked() that
don't require holding the alloc->mutex. While we are here, consolidate
the checks for size overflow and zero-sized padding into a separate
sanitized_size() helper function.
Also add a few touchups to follow the coding guidelines.
Signed-off-by: Carlos Llamas <cmllamas@google.com>
Reviewed-by: Alice Ryhl <aliceryhl@google.com>
Link: https://lore.kernel.org/r/20231201172212.1813387-12-cmllamas@google.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Bug: 254650075
(cherry picked from commit 377e1684db7a1e23261f3c3ebf76523c0554d512)
Change-Id: I8fc18c06563ad2c26536633034fb3e94b0aaf510
Signed-off-by: Carlos Llamas <cmllamas@google.com>
The binder_update_page_range() function performs both allocation and
freeing of binder pages. However, these two operations are unrelated and
have no common logic. In fact, when a free operation is requested, the
allocation logic is skipped entirely. This behavior makes the error path
unnecessarily complex. To improve readability of the code, this patch
splits the allocation and freeing operations into separate functions.
No functional changes are introduced by this patch.
Reviewed-by: Alice Ryhl <aliceryhl@google.com>
Signed-off-by: Carlos Llamas <cmllamas@google.com>
Link: https://lore.kernel.org/r/20231201172212.1813387-11-cmllamas@google.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Bug: 254650075
(cherry picked from commit 0d35bf3bf2da8d43fd12fea7699dc936999bf96e)
Change-Id: Iaf64f94564d2017c4633f2421c15b0bdee914738
Signed-off-by: Carlos Llamas <cmllamas@google.com>
