| Age | Commit message (Collapse) | Author |
|---|
|
sk_clone() sets sk_refcnt of the new socket some lines behind
bpf_sk_storage_clone(). But the elements which that function copies are
linked to their maps at once, and an element leads to its owner: the
iterator over a sk_storage map hands the owner to its program.
bpf_sk_storage_get() with BPF_SK_STORAGE_GET_F_CREATE and
bpf_sk_storage_delete() rely on refcount_inc_not_zero() to tell whether
they may touch the socket.
sk_refcnt is in the part of struct sock which sock_copy() leaves out, and
sk_prot_alloc() does not clear a clone unless init_on_alloc is on. It is
zero in an object which has been a socket before, and anything in one
which has not. If it is not zero, a helper gets a reference which
refcount_set() wipes out, and the helper's sock_put() then drops one of
the two references of the new socket, which is freed too early:
IPv4: Attempt to release TCP socket in state 1
BUG: KASAN: slab-use-after-free in __raw_spin_lock_init+0x47/0x90
In the meantime the helper changes a storage which bpf_sk_storage_clone()
fills without taking its lock.
Nothing else can find the clone at this point. Say that it is not alive
yet, before the first element is linked. Sockets whose parent has no
storage are left alone.
Fixes: a50a85e40c59 ("bpf: Expose bpf_sk_storage_* to iterator programs")
Suggested-by: Josef Bacik <josef@toxicpanda.com>
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
Link: https://patch.msgid.link/20261017163745.3341787-15-alexei.starovoitov@gmail.com
|
|
For SEG6_LOCAL_ACTION_END_B6 and END_B6_ENCAP bpf_lwt_seg6_action() looks
for the SRH again only if bpf_push_seg6_encap() has succeeded. That
function pushes the new header, for which the head of the skb is
reallocated if it has not got the room, and then looks up the route to the
first segment. If there is none, it returns an error, and the pointer to
the SRH in seg6_bpf_srh_states still points into the old head, which has
been freed.
The program goes on behind the error. bpf_lwt_seg6_store_bytes() and
bpf_lwt_seg6_adjust_srh() read the SRH through that pointer.
Look for the SRH whatever bpf_push_seg6_encap() says.
Fixes: 486cdf21583e ("bpf: add End.DT6 action to bpf_lwt_seg6_action helper")
Signed-off-by: Josef Bacik <josef@toxicpanda.com>
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
Link: https://patch.msgid.link/20261017163745.3341787-14-alexei.starovoitov@gmail.com
|
|
bpf_lwt_seg6_action(SEG6_LOCAL_ACTION_END_DT6) pulls the outer headers off
the packet and hands it to seg6_lookup_nexthop(), which reads the inner
IPv6 header at skb->data. Nothing says that this header is in the head of
the skb, or that there is one. If the head ends with the outer headers,
the route is looked up for whatever lies behind the head, and the packet
is dropped or leaves by the wrong route. If the program then pushes an SRH
with SEG6_LOCAL_ACTION_END_B6, seg6_do_srh_inline() pulls an IPv6 header
off a head which has none, and __skb_pull() hits its BUG(): the kernel
panics on a packet from the network.
GRO pulls the inner header of a plain IPv6 in IPv6 packet into the head. It
takes a packet which GRO does not look into that far, one with a fragment
header in front of the inner header for example, or a device without GRO,
and a driver which leaves the inner header in a fragment.
Make sure of the inner header together with the outer ones. The program
goes on behind an error of the helper, so do it before the packet is
changed. A pull which fails can still have moved the head, so look for the
SRH again.
Fixes: 486cdf21583e ("bpf: add End.DT6 action to bpf_lwt_seg6_action helper")
Signed-off-by: Josef Bacik <josef@toxicpanda.com>
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
Link: https://patch.msgid.link/20261017163745.3341787-13-alexei.starovoitov@gmail.com
|
|
input_action_end_bpf() validates the SRH of a packet and pulls it into the
head before the program runs. If the program decapsulates the packet with
bpf_lwt_seg6_action(SEG6_LOCAL_ACTION_END_DT6), bpf_update_srh_state()
looks for a routing header in the inner packet and takes it for the new
SRH as it is.
Nobody has looked at that header, and whoever has sent the packet has
written it. ipv6_find_hdr() finds it in a fragment of the skb as well, but
hdrlen is then read at that offset from skb->data, past the end of the
head. The header need not be an SRH, and its hdrlen need not fit into the
packet, but it is marked as valid, and bpf_lwt_seg6_adjust_srh() and
bpf_lwt_seg6_store_bytes() check their offsets against it.
A program which then adds a TLV at the end of the SRH, where hdrlen has it,
makes bpf_lwt_seg6_adjust_srh() move and write memory behind the head of
the skb, as far as the sender likes within the 2 KB that hdrlen can say:
BUG: KASAN: slab-out-of-bounds in bpf_skb_net_hdr_push+0x185/0x240
Read of size 2088 at addr ffff888107d6f09e by task srh/102
BUG: KASAN: slab-out-of-bounds in bpf_skb_net_hdr_push+0x1b5/0x240
Write of size 8 at addr ffff888107d6f8be by task srh/102
Use seg6_get_srh(), which pulls and validates the header. Without a valid
SRH the state says that there is none, and the packet goes its way as
before. The pull can move the head, so compute the data pointers behind
it.
Fixes: 486cdf21583e ("bpf: add End.DT6 action to bpf_lwt_seg6_action helper")
Signed-off-by: Josef Bacik <josef@toxicpanda.com>
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
Link: https://patch.msgid.link/20261017163745.3341787-12-alexei.starovoitov@gmail.com
|
|
bpf_local_storage_map_free() pins the owner with refcount_inc_not_zero()
while it uncharges an element. The pin counts when
bpf_local_storage_destroy() could not get local_storage->lock: destroy()
then goes on, drops the owner's reference and spins until the count is
zero. So the pin is the last reference whenever it is needed, and
refcount_dec() takes that for a bug: it warns "decrement hit 0; leaking
memory" and saturates the count. Unless destroy() happens to read the
count between the two stores it spins for ever, under rcu_read_lock().
Drop the pin with refcount_dec_and_test().
Fixes: 5d800f87d0a5 ("bpf: Support lockless unlink when freeing map or local storage")
Signed-off-by: Josef Bacik <josef@toxicpanda.com>
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
Link: https://patch.msgid.link/20261017163745.3341787-11-alexei.starovoitov@gmail.com
|
|
For a perf event on a kprobe or uprobe event which was made in tracefs,
bpf_get_kprobe_info() and bpf_get_uprobe_info() look the probe up by
name in the list of dynamic events, and hand a pointer to its symbol or
file name back to the caller. None of the callers holds event_mutex.
The perf event keeps its trace event alive, nothing else. Other dynamic
events are removed from the list and freed during the walk. And a probe
which shares its event with other probes can be deleted while the event
is in use, so even the probe which is found can be freed, together with
the string, before the caller has measured and copied it. The result is
a use-after-free in the walk, in strlen() and in the copy to user space,
or a NULL pointer dereference.
The readers are BPF_OBJ_GET_INFO_BY_FD and fdinfo of a perf link, for
whoever holds a descriptor of the link, and BPF_TASK_FD_QUERY. The other
side is anybody who may write to dynamic_events, kprobe_events or
uprobe_events.
Take event_mutex in bpf_get_perf_event_info() and give the caller a copy
of the name, which it has to free.
Fixes: 41bdc4b40ed6 ("bpf: introduce bpf subcommand BPF_TASK_FD_QUERY")
Signed-off-by: Josef Bacik <josef@toxicpanda.com>
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
Link: https://patch.msgid.link/20261017163745.3341787-10-alexei.starovoitov@gmail.com
|
|
When bpf_skb_adjust_room() grows a TCP GSO packet without
BPF_F_ADJ_ROOM_FIXED_GSO, gso_size is lowered by the number of bytes
added. Nothing looks at how large it is.
gso_size is the size of the segments the peer has sent if the packet
comes out of GRO, and what the sender has written into the virtio header
if it comes from a tun device or a packet socket. If it equals len_diff
the result is 0, and a packet of up to 64 KB is no GSO packet any more. If
it is smaller, the 16 bit value wraps around.
One less than len_diff gives GSO_BY_FRAGS. skb_segment() then takes the
length of each segment from the frag_list. A packet with more than 64 KB
of payload gets that far, and one from a tun device has no frag_list:
BUG: KASAN: null-ptr-deref in skb_segment+0xcf1/0x1a90
BUG: kernel NULL pointer dereference, address: 0000000000000070
tcp_gso_segment+0x1be/0x9f0
inet_gso_segment+0x288/0x740
inet_gso_segment+0x288/0x740
skb_mac_gso_segment+0x179/0x2e0
What is left must not be less than TCP_MIN_GSO_SIZE either. The stack
relies on that for the number of segments to fit into gso_segs, see
commit c27c449d455a ("virtio-net: Ensure that TCP packets don't overflow
gso_segs").
Refuse to grow a packet that would end up with less, with the error that
other packets which cannot be adjusted get.
Fixes: 2be7e212d541 ("bpf: add bpf_skb_adjust_room helper")
Signed-off-by: Josef Bacik <josef@toxicpanda.com>
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
Link: https://patch.msgid.link/20261017163745.3341787-9-alexei.starovoitov@gmail.com
|
|
In the branch of a BPF_JSET which is taken, the verifier can learn
something about the register only if the mask has a single bit: then this
bit is set. regs_refine_cond_op() tests this with is_power_of_2(val) and
then ORs val into var_off.
val is a u64, but is_power_of_2() takes an unsigned long. On a 32-bit
kernel it only sees the lower half of the mask. For
if r1 & 0xffffffff80000000 goto l
which is a BPF_JSET | BPF_K with an immediate of 0x80000000, the lower
half is a power of two. So the verifier takes all 33 bits for set at l,
while the branch is taken if any of them is. A mask of 0x100000001 from a
register does the same for bit 0.
The log of an i386 kernel, for r6 = bpf_get_prandom_u32() << 31:
3: (45) if r6 & 0x80000000 goto pc+2
from 3 to 6: R6=0xffffffff80000000 R10=fp0
6: (77) r6 >>= 63 ; R6=1
When the program runs, r6 >> 63 is 0.
>From then on the verifier works with known bits which the register does
not have at run time. It accepts, for example, an offset into a map value
which it derives from these bits and believes to be constant, and the
program can access memory outside of the map value.
Count the bits of the whole value.
Only 32-bit kernels are affected. On a 64-bit kernel unsigned long holds
the whole mask, and this patch changes nothing.
Fixes: 960ea056561a ("bpf: verifier: teach the verifier to reason about the BPF_JSET instruction")
Signed-off-by: Josef Bacik <josef@toxicpanda.com>
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
Link: https://patch.msgid.link/20261017163745.3341787-8-alexei.starovoitov@gmail.com
|
|
A sleepable program can call bpf_find_vma() and use a sleepable helper
such as bpf_copy_from_user() or bpf_copy_from_user_task() in the
callback. The verifier accepts this, but bpf_find_vma() runs the callback
with the mmap_lock of the mm held for read. If the user copy faults on
that mm, the fault handler has to take mmap_lock again. Lockdep reports
recursive locking, and if a writer queued up in between, the fault blocks
behind the writer, which waits for the read lock the helper already
holds:
sleepable prog other thread of the same mm
bpf_find_vma(current, ...)
mmap_read_trylock(mm)
mmap_write_lock(mm) <- waits
callback: copy_from_user()
page fault
mmap_read_lock(mm) <- queued behind the writer
This is the same problem as with sleepable task_vma iterator programs.
The verifier checks synchronous callbacks in the caller's context, so
in_sleepable_context() is true in the callback of a sleepable program.
Verify the callback like a hash map for_each callback, as running in an
RCU read-side critical section where sleeping is not allowed, and actually
hold rcu_read_lock() around it, so that global functions the callback
calls, which are then verified for a non-sleepable context, see the RCU
protection they are verified with.
Non-sleepable programs already run the callback under RCU, nothing changes
for them. A sleepable program whose bpf_find_vma() callback calls any
helper, kfunc or global function that may sleep is now rejected by the
verifier, including ones that would be harmless under mmap_lock as they
don't touch user memory.
Fixes: 7c7e3d31e785 ("bpf: Introduce helper bpf_find_vma")
Signed-off-by: Josef Bacik <josef@toxicpanda.com>
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
Link: https://patch.msgid.link/20261017163745.3341787-7-alexei.starovoitov@gmail.com
|
|
A sleepable "iter.s/task_vma" program can be attached, but its ->show()
callback runs with the mmap_lock of the iterated mm held for read. As soon
as the program runs, bpf_iter_run_prog() calls might_fault(), and any
bpf_copy_from_user() or bpf_copy_from_user_task() it does may really fault
or take mmap_lock again. With lockdep this is reported as recursive
locking of mmap_lock. Without lockdep, a fault on the reader's own mm, or
access_process_vm() on the iterated mm, has to take mmap_lock for read
again. If a writer queued up in the meantime that blocks behind the
writer, which in turn waits for the read lock the iterator already holds,
and the read() hangs:
reader (iter.s/task_vma) other thread of the same mm
task_vma_seq_get_next()
mmap_read_lock(mm)
mmap_write_lock(mm) <- waits
bpf prog: copy_from_user()
page fault
mmap_read_lock(mm) <- queued behind the writer
The reader waits killable, so the hang lasts until it gets a fatal signal.
bpf_iter_link_attach() allows sleepable programs for the targets that set
BPF_ITER_RESCHED, and task_vma sets it because cond_resched() between VMAs
is fine with mmap_lock held. Rescheduling between objects and running a
sleepable program inside ->show() are different properties, though, and
task_vma only has the first one. The open-coded task_vma iterator does not
have this problem since it hands out a snapshot of the VMA and holds no
lock when it returns to the program.
The vma handed to the program is only stable under mmap_lock, so the lock
can't be dropped around ->show(). Add a feature flag BPF_ITER_NOSLEEP for
targets which can't run sleepable programs although they can reschedule,
and set it for task_vma.
The check at attach time is not enough. BPF_LINK_UPDATE replaces the
program of a link, and bpf_iter_link_replace() only compares the type, the
attach type and the BTF id of the two programs, which are the same for a
sleepable and a non-sleepable one. Do the check there as well. That also
closes this way around the existing rule: so far a sleepable program could
be put on a link of a target that doesn't set BPF_ITER_RESCHED, like the
hash map element iterator, which runs it under rcu_read_lock().
BPF_LINK_CREATE of a sleepable program for the task_vma target, and
BPF_LINK_UPDATE to one, now fail with -EINVAL, also for programs that
don't touch user memory and have been working since sleepable iterator
programs exist. The programs can still be loaded. They have to be
loaded as non-sleepable programs, or use the open-coded iterator.
Fixes: b77fb25dcb34 ("bpf: Add support for bpf iterator programs to use sleepable helpers")
Signed-off-by: Josef Bacik <josef@toxicpanda.com>
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
Link: https://patch.msgid.link/20261017163745.3341787-6-alexei.starovoitov@gmail.com
|
|
When htab_map_update_elem() replaces an element of a preallocated map,
alloc_htab_elem() takes the spare element of this cpu and puts the old
element into the per-cpu slot right away, long before the old element is
unlinked from its bucket. The bucket lock disables interrupts, but it
does not keep out an NMI, or a tracing program on the return of
alloc_htab_elem(). bpf_prog_active only helps if both programs are
kprobe, tracepoint or perf_event programs. A program that updates an
existing key in another bucket of the same map gets to use the slot:
htab_map_update_elem(k1) tc, xdp, fentry, raw_tp, ...
alloc_htab_elem()
l_new = *pl_new
*pl_new = l_old still linked in bucket 1
<NMI>
htab_map_update_elem(k2) perf_event program
l_new = *pl_new the l_old of the outer update
hlist_nulls_add_head_rcu(l_new, bucket 2)
hlist_nulls_del_rcu(k2's old element)
</NMI>
hlist_nulls_add_head_rcu(l_new, bucket 1)
hlist_nulls_del_rcu(l_old)
If l_old was the first element of bucket 1, the outer add points its pprev
at l_new, and the delete takes it out of bucket 1 only. l_new->next now
leads into bucket 2, where l_old stays with a poisoned pprev, on which a
delete of k2 crashes. If it was not the first, the delete takes it out of
bucket 2: k2 is gone from the map, and the chain of bucket 1 still contains
l_old, whose next pointer leads into bucket 2.
Either way a walk of bucket 1 ends on the nulls value of bucket 2.
lookup_nulls_elem_raw() restarts whenever that happens, so every lookup in
bucket 1 which does not find its key spins forever:
rcu: INFO: rcu_preempt detected stalls on CPUs/tasks:
__htab_map_lookup_elem+0x354/0x4d0
BUG: KASAN: wild-memory-access in htab_map_delete_elem+0x30c/0x670
Write of size 8 at addr dead000000000122 by task extra/102
An NMI between the load and the store of the slot makes both updates use
the same element, with much the same result.
Empty the slot when the spare element is taken, in a way that is atomic
on this cpu, and put the old element in only after it has been unlinked
and its timers and the like have been cancelled. A nested update that
finds the slot empty, which is the case that corrupted the map, fails with
-EBUSY.
Fixes: 8c290e60fa2a ("bpf: fix hashmap extra_elems logic")
Signed-off-by: Josef Bacik <josef@toxicpanda.com>
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
Link: https://patch.msgid.link/20261017163745.3341787-5-alexei.starovoitov@gmail.com
|
|
bpf_token_create() allocates a bpffs inode and hands it to
alloc_file_pseudo(). The inode reference only moves to the new file when
that call succeeds: alloc_file_pseudo() leaves the caller's reference alone
on failure, and FD_PREPARE() does not even evaluate the file expression
when it cannot get a descriptor.
Before the conversion to FD_PREPARE() the error path did iput(inode). The
conversion dropped it, so each BPF_TOKEN_CREATE that fails with -EMFILE,
-ENFILE or -ENOMEM at this point leaks an inode. A task that is allowed to
create tokens can hit this at will by lowering RLIMIT_NOFILE, and the
leaked inodes make umount of that bpffs instance complain about busy
inodes.
Put the inode again if no file was created.
Fixes: 981bec8f698b ("bpf: convert bpf_token_create() to FD_PREPARE()")
Signed-off-by: Josef Bacik <josef@toxicpanda.com>
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
Link: https://patch.msgid.link/20261017163745.3341787-4-alexei.starovoitov@gmail.com
|
|
A sleepable program (e.g. SEC("syscall"), lsm.s, fentry.s) can pass a
callback to bpf_for_each_map_elem() on a hash map and call a sleepable
helper or kfunc such as bpf_copy_from_user() from it. The verifier
accepts this, but at run time the callback is invoked from inside an RCU
read-side critical section:
bpf_for_each_hash_elem()
rcu_read_lock();
callback_fn(map, key, val, ctx, 0);
bpf_copy_from_user() <- may fault and sleep
rcu_read_unlock();
bpf_each_rhash_elem() does the same for BPF_MAP_TYPE_RHASH. Sleeping
there triggers the "Voluntary context switch within RCU read-side
critical section!" warning, delays grace periods, and with
CONFIG_PREEMPT_RCU=n silently ends the read-side critical section that
the bucket walk relies on.
Synchronous callbacks are verified with the caller's state, so
kernel/bpf/verifier.c:in_sleepable_context() only looks at the program's
sleepability and the locks the program took itself, and does not know
that the helper holds rcu_read_lock() around the callback.
With a SEC("syscall") program whose callback reads a page that has to be
read from disk first:
BUG: sleeping function called from invalid context at include/linux/uaccess.h:169
RCU nest depth: 1, expected: 0
__might_fault+0x29/0x80
_copy_from_user+0x26/0x90
bpf_copy_from_user+0x23/0x60
bpf_prog_17d44fa39eebd6cc_cb+0x35/0x46
bpf_for_each_hash_elem+0xdb/0x280
Fix by marking the callback frame of hash map iteration as running under
RCU and making in_sleepable_context() false while such a frame is on the
call stack, so sleepable helpers, kfuncs and global functions are
rejected there as inside bpf_rcu_read_lock(). The rules for trusting RCU
protected pointers are left alone, which is conservative and keeps
callbacks that verify today and do not sleep working. The flag is
compared in func_states_equal() so that pruning never mixes the two
contexts. Array map callbacks run without any lock and keep the caller's
context.
Fixes: 314ee05e2fc6 ("bpf: Add hashtab support for bpf_for_each_map_elem() helper")
Fixes: 818e00848227 ("bpf: Implement iteration ops for resizable hashtab")
Signed-off-by: Josef Bacik <josef@toxicpanda.com>
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
Link: https://patch.msgid.link/20261017163745.3341787-3-alexei.starovoitov@gmail.com
|
|
A devmap egress program that removes a whole fragment of a redirected
xdp_frame with bpf_xdp_adjust_tail() or bpf_xdp_pull_data() frees it
with page_frag_free() whatever the frame's memory type is. For a
MEM_TYPE_PAGE_POOL frame from a page_pool based driver this drops the
page reference owned by the pool while the pool still accounts the
fragment ("page_pool leak" bad page, leaked pp reference).
kernel/bpf/devmap.c:dev_map_bpf_prog_run() points xdp.rxq at an
on-stack
struct xdp_rxq_info rxq = { .dev = rx_dev };
whose mem.type is 0, i.e. MEM_TYPE_PAGE_SHARED, and
net/core/filter.c:bpf_xdp_shrink_data() releases frags according to
rxq->mem.type. cpumap, which runs programs on xdp_frames the same way,
already copies xdpf->mem_type into its private rxq for each frame.
With 9000 byte frames from virtio_net and an egress program that cuts the
fragments off:
BUG: Bad page state in process swapper/3 pfn:10660b
page dumped because: page_pool leak
bad_page+0xbd/0x190
__free_frozen_pages+0x5c7/0x900
bpf_xdp_adjust_tail+0x3be/0x480
bpf_prog_dad43f894a3faff7_egress+0x15c/0x1cd
dev_map_bpf_prog_run+0x288/0x610
The pool waits for these pages for ever when its device is gone. That is
the harmless case. A page of the pool can hold more than one fragment
while its refcount is 1: the first page_frag_free() frees it under the
fragment which is still in use, and the second one runs into
page dumped because: VM_BUG_ON_PAGE(page_ref_count(page) == 0)
kernel BUG at include/linux/mm.h:1846!
veth is not affected. It takes references of its own on the pages and
redirects the frame as MEM_TYPE_PAGE_SHARED.
Fix by setting rxq.mem.type from each frame before running the devmap
program, as cpu_map_bpf_prog_run_xdp() does.
Fixes: ca9984c5f0ab ("bpf: devmap: provide rxq after redirect")
Signed-off-by: Josef Bacik <josef@toxicpanda.com>
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
Link: https://patch.msgid.link/20261017163745.3341787-2-alexei.starovoitov@gmail.com
|
|
The commit moving the device initialization to the new .init callback
missed the description of the newly introduced dev field in the driver
data structure - add an entry to the kerneldoc.
No code changes.
Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202610120726.XkN50Smh-lkp@intel.com/
Signed-off-by: Alexey Charkov <alchark@flipper.net>
Link: https://patch.msgid.link/20261015-bq257xx-dev-comment-v1-1-c87ee6a3b251@flipper.net
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
|
|
On SC8280XP and X1E80100 the AC, USB and wireless supplies take their
online state from the charging source in the battery status response.
The firmware sends no notification for those supplies when the charging
source changes, and the driver only announces the battery, so userspace
never learns that the charger was plugged in or out. UPower keeps the
state it read at startup: a laptop booted on the charger never goes on
battery.
Call power_supply_changed() for a supply whose online state changes when
a status response is parsed. A supply that is not registered yet, when
the battery is read during probe, reports its state when it registers.
Tested on a Microsoft Surface Pro 11 (X1E80100): after restarting UPower
on the charger, unplugging now sets OnBattery, and plugging in clears it.
Fixes: 29e8142b5623 ("power: supply: Introduce Qualcomm PMIC GLINK power supply")
Cc: stable@vger.kernel.org
Assisted-by: Claude:claude-opus-5-5
Signed-off-by: Brandon Williams <turbine.bmw@gmail.com>
Link: https://patch.msgid.link/20261013-upstream-battmgr-charger-online-v1-1-731973e133a0@gmail.com
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
|
|
power_supply_am_i_supplied() returns -ENODEV when no registered power
supply supplies the battery. generic-adc-battery's gab_work() tests this
return value as a boolean, so -ENODEV takes the supplied branch and
reports CHARGING, or NOT_CHARGING if the charge-finished GPIO is asserted,
instead of DISCHARGING.
This was noticed while bringing up an out-of-tree Linux port to the
WikiReader: the driver reported CHARGING on a device powered by two AAA
batteries with no charging hardware.
Only a positive return value indicates an online supplier, so treat zero
and negative returns as discharging.
Fixes: 46cecd130d55 ("power: supply: Fix power_supply_am_i_supplied to return -ENODEV when apropriate")
Cc: stable@vger.kernel.org
Signed-off-by: Tim Douglas <me@timdoug.com>
Link: https://patch.msgid.link/20261013022153.6707-1-me@timdoug.com
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
|
|
On SM8350 and SM8550 class firmware the USB power supply reads its
usb_type from the USB_TYPE property. On a Kaanapali (SM8850) tablet that
property is 1 (SDP) whatever is plugged in, while USB_ADAP_TYPE reports
the adapter, using the same numbering as POWER_SUPPLY_USB_TYPE_*:
PC port USB_TYPE 1 USB_ADAP_TYPE 1 (SDP)
USB-C dock, 5 V PD USB_TYPE 1 USB_ADAP_TYPE 6 (PD)
65 W PPS charger USB_TYPE 1 USB_ADAP_TYPE 8 (PD_PPS)
Qualcomm's downstream qti_battery_charger driver also maps
USB_ADAP_TYPE to POWER_SUPPLY_PROP_USB_TYPE.
Request USB_ADAP_TYPE for POWER_SUPPLY_PROP_USB_TYPE instead.
Tested on a Lenovo Legion Tab Y700 gen 5 (qcom,kaanapali-pmic-glink,
which uses the SM8550 path). Only tested on this device; the SM8350
variant shares the property map and was not tested.
Fixes: 29e8142b5623 ("power: supply: Introduce Qualcomm PMIC GLINK power supply")
Reviewed-by: Kamal Wadhwa <kamal.wadhwa@oss.qualcomm.com>
Assisted-by: LLM
Signed-off-by: Joonhoe Kim <26rote@gmail.com>
Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Link: https://patch.msgid.link/20261012130053.442-1-26rote@gmail.com
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
|
|
power_supply_show_enum_with_available() warns every time a driver
reports a value that is not in its declared set of available values.
For usb_type this happens on every sysfs read and on every uevent, so a
driver that reports such a value and calls power_supply_changed()
frequently can flood the kernel log.
This can happen with drivers that pass a firmware-provided value
through unchanged, as qcom_battmgr does for usb_type. On a OnePlus 10
Pro (SM8450), whose firmware reports a vendor-specific usb_type, this
produced ~128 warnings per second and kept systemd-journald at ~30% CPU.
The bug that causes the bad value needs fixing in the driver itself,
but one misbehaving driver should not be able to flood the kernel log.
Use dev_warn_ratelimited(), as power_supply_show_property() already
does for its own errors, so the problem is still reported, just not on
every read.
Signed-off-by: Robert Killick <robert@killick.dev>
Link: https://patch.msgid.link/20261005-ratelimit-enum-warn-v1-1-b77b48de3c47@killick.dev
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
|
|
The EC resets its charge control configuration when the AP powers off,
so configured sustainer thresholds and charge behaviour are lost across
hibernation.
Nothing notices the reset. Commit 4fc88ba435da ("power: supply:
cros_charge-control: adopt EC charge state on probe"), in the
power-supply tree, made the driver adopt the EC's charge state, but only
at probe, and a resume is not a probe. Sysfs therefore keeps reporting
the configured values while the EC charges the battery to full. On a
Framework Laptop 16 (Ryzen AI 300, charge control command version 3)
with charge_control_end_threshold set to 80, a hibernate and resume
cycle leaves the attribute still reading 80 while the battery charges
past it to 100%.
Add a system sleep resume hook that reprograms the EC from the cached
state. The same EC behaviour and the same fix already exist for another
driver, see commit 2643187ccb86 ("platform/x86: ayaneo-ec: Add suspend
hook").
Reprogramming the EC is only strictly required after hibernation, but it
is cheap and idempotent, so hook every resume path rather than rely on
the EC preserving its state across each kind of sleep transition.
Note that re-reading the state from the EC on resume, by reusing
cros_chctl_init_state() or otherwise, would be wrong even on the command
versions that support GET: the EC has been reset to its defaults by the
time the hook runs, so adopting that state would silently discard the
configuration the user asked for. The driver's cached state is the only
remaining record of it, which is why it is pushed to the EC rather than
refreshed from it.
Fixes: c6ed48ef5259 ("power: supply: add ChromeOS EC based charge control driver")
Cc: stable@vger.kernel.org
Assisted-by: Claude:claude-opus-5-5
Signed-off-by: Andrew Pam <andrew@fastmailteam.com>
Reviewed-by: Tzung-Bi Shih <tzungbi@kernel.org>
Acked-by: Thomas Weißschuh <linux@weissschuh.net>
Link: https://patch.msgid.link/20261005043418.243937-1-andrew@fastmailteam.com
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
|
|
The init sequence sets CHARGING_ENABLE_CMD before CHGR_CFG2 selects
software control of the charge enable.
The Motorola Moto X4's bootloader leaves the charger watchdog armed to
disable charging when it bites, which it does after a few minutes in
fastboot. Charging then stays disabled in the next boot until the cable
is replugged. Setting the command after CHGR_CFG2 re-enables it.
Fixes: 8648aeb5d7b7 ("power: supply: add Qualcomm PMI8998 SMB2 Charger driver")
Cc: stable@vger.kernel.org
Signed-off-by: Fabricio Akio <fabricioakio@gmail.com>
Link: https://patch.msgid.link/20261002-qcom-smbx-enable-order-v1-1-7df92bd809db@gmail.com
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
|
|
Using the same cast we use in util/debug.c with tv_usec.
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
|
|
|
|
The dispatch path can request deferred work to be run from a balance
callback and pick_task_scx() queues rq->scx.deferred_bal_cb right after
dispatching. However, a balance callback must be consumed before the rq
lock is released and pick_task_scx() is not necessarily the end of the
pick:
- it returns RETRY_TASK when a higher priority class task has been
enqueued while the dispatch had the rq lock released;
- the dl_server pick can return NULL, in which case the lower classes
are picked next.
In both cases the callback stays queued while the core keeps picking,
and the rq lock can be released again, either by prev_balance() or by
another dispatch. For example, with an RT task as prev:
__schedule()
__pick_next_task()
pick_task_scx()
dispatch_pick()
maybe_queue_balance_callback() <- deferred_bal_cb queued
return RETRY_TASK
prev_balance()
balance_rt()
pull_rt_task()
double_lock_balance() <- rq lock released
Any CPU acquiring the rq lock in that window finds a pending balance
callback and triggers the following warning in rq_pin_lock():
WARNING: kernel/sched/sched.h:1919 at rq_lock+0x7f/0xe0, CPU#10: stress-ng-sleep/533
CPU: 10 UID: 0 PID: 533 Comm: stress-ng-sleep Not tainted 7.2.0-rc6-virtme #61 PREEMPT(full)
Sched_ext: qmap (enabled+all)
Call Trace:
<IRQ>
try_to_wake_up+0x2c1/0x5c0
hrtimer_wakeup+0x22/0x30
__hrtimer_run_queues+0x1d5/0x350
hrtimer_interrupt+0x106/0x1e0
__sysvec_apic_timer_interrupt+0x6f/0x210
sysvec_apic_timer_interrupt+0x71/0x90
</IRQ>
This can be reproduced running scx_qmap -I together with a SCHED_FIFO
workload that frequently sleeps, e.g.:
$ vng -v --cpus 16 --memory 4G -- \
'tools/sched_ext/build/bin/scx_qmap -I >/dev/null & \
stress-ng --sleep 4 --sched fifo --sched-prio 1 -t 30 -q'
Fix this by queueing the balance callbacks only when the pick is final.
Otherwise, defer the pending work to the irq_work path, which doesn't
depend on the current rq lock section, and leave the kick sync wait
pending for the next pick.
Fixes: 4c95380701f5 ("sched/ext: Fold balance_scx() into pick_task_scx()")
Cc: stable@vger.kernel.org # v6.19+
Signed-off-by: Andrea Righi <arighi@nvidia.com>
Signed-off-by: Tejun Heo <tj@kernel.org>
|
|
Add a test that reads the two u64 fields bytes_received and bytes_acked
at offset + 4 with a u32 load, to make sure such a narrower access is
rejected with "invalid tcp_sock access".
Signed-off-by: Jiayuan Chen <jiayuan.chen@linux.dev>
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
Link: https://patch.msgid.link/20261017143819.209510-2-jiayuan.chen@linux.dev
|
|
A BPF program can read the middle of a u64 field in struct bpf_tcp_sock:
tp = bpf_tcp_sock(listener);
/* +0x54 is in the middle of the u64 bytes_received */
unsigned hi = *(unsigned *)((char *)tp + 0x54);
bpf_tcp_sock_is_valid_access() allows this 4-byte read (the default case
only checks size == 4), but bpf_tcp_sock_convert_ctx_access() only knows
the field starts and has no idea how to handle such an offset. It emits
no instruction, cnt stays 0 and the fixup pass trips verifier_bug():
verifier bug: error during ctx access conversion (0)
Just disallow the narrower access by matching the whole u64 range of the
two fields, so it is turned down at check time:
invalid tcp_sock access off=84 size=4
Fixes: 655a51e536c0 ("bpf: Add struct bpf_tcp_sock and BPF_FUNC_tcp_sock")
Reported-by: Kaiyan Mei <M202472210@hust.edu.cn>
Reported-by: Yinhao Hu <dddddd@hust.edu.cn>
Reported-by: Dongliang Mu <dzm91@hust.edu.cn>
Signed-off-by: Jiayuan Chen <jiayuan.chen@linux.dev>
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
Link: https://patch.msgid.link/20261017143819.209510-1-jiayuan.chen@linux.dev
Closes: https://lore.kernel.org/bpf/7a8ff335.1024c.1a14395f3e5.Coremail.kaiyanm@hust.edu.cn/
|
|
Commit 45e1dccc0653 ("x86/insn: Simplify for_each_insn_prefix()") made
for_each_insn_prefix() declare its own loop counter named idx.
get_seg_reg_override_idx() keeps its result in a local of the same name,
so the assignments in the loop body now write the loop counter:
- the function always returns INAT_SEG_REG_DEFAULT, so FS and GS
overrides are ignored;
- a CS override sets the counter to INAT_SEG_REG_CS (2), the increment
makes it 3, and prefixes.bytes[3] holds the last prefix, 0x2e again,
so the loop never terminates.
The UMIP emulation reaches this from the #GP handler for user space: an
SGDT, SIDT or SMSW with a memory operand and a CS prefix leaves the task
spinning in the kernel, never returning to user space, with a SIGKILL
that is never delivered. Any user can thus tie up CPUs until reboot.
With an FS or GS prefix, the emulated result is written to the wrong
address.
Rename the internal macro iterator variable.
[ Lukas' original patch renamed the use site, I changed it to rename the
iterator in the macro instead, but left Lukas' commit message and
authorship - Linus ]
Fixes: 45e1dccc0653 ("x86/insn: Simplify for_each_insn_prefix()")
Signed-off-by: Lukas Zobernig <lukas.zobernig@gmail.com>
Acked-by: Peter Zijlstra <peterz@infradead.org>
Acked-by: Borislav Petkov <bp@alien8.de>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
|
|
__vgic_v5_save_apr() and __vgic_v5_restore_vmcr_apr() remain callable
once pKVM has finalised. pKVM never registers a GICv5 vgic, so neither
has a valid caller in protected mode, and on a GICv3 machine both reach
a register that is UNDEFINED at EL2 and panic the hypervisor.
Move both into the band the dispatcher rejects once pKVM has finalised.
[maz: Dropped hyp-main.c changes]
Fixes: af325e87af5da ("KVM: arm64: gic-v5: Add vgic-v5 save/restore hyp interface")
Reviewed-by: Suzuki K Poulose <suzuki.poulose@arm.com>
Signed-off-by: Fuad Tabba <fuad.tabba@linux.dev>
Link: https://patch.msgid.link/20261012161524.4016464-2-fuad.tabba@linux.dev
Signed-off-by: Marc Zyngier <maz@kernel.org>
|
|
The first time userspace sets an SMCCC filter, kvm_smccc_set_filter()
inserts the reserved architecture ranges into the maple tree and WARNs
if that fails. The insertion allocates with GFP_KERNEL_ACCOUNT, so a
tight memory cgroup limit is enough for userspace to hit the WARN, even
though the -ENOMEM is also returned to it. The WARN dates back to when
the ranges were inserted at VM creation.
Leave -ENOMEM out of the WARN and keep it for every other error. The
two reserved ranges are fixed and disjoint and go into an empty tree
under config_lock, so any other error is a KVM bug.
Fixes: fb88707dd39bd ("KVM: arm64: Use a maple tree to represent the SMCCC filter")
Reported-by: Sashiko <sashiko-bot@kernel.org>
Closes: https://lore.kernel.org/all/20260829055809.29AD31F000E9@smtp.kernel.org/
Suggested-by: Oliver Upton <oupton@kernel.org>
Signed-off-by: Fuad Tabba <fuad.tabba@linux.dev>
Reviewed-by: Suzuki K Poulose <suzuki.poulose@arm.com>
Reviewed-by: Oliver Upton <oupton@kernel.org>
Link: https://patch.msgid.link/20261013164334.1492070-1-fuad.tabba@linux.dev
Signed-off-by: Marc Zyngier <maz@kernel.org>
|
|
|
|
Pull to receive 30fb2b1e4d41 ("sched_ext: Fix the running task keeping its
protection into bypass") and 5809f749fbdb ("sched_ext: Fix a bypassed task
stalling on a cpu its scheduler has no caps on") and resolve their conflicts
with the proxy execution work here.
Conflicts:
- scx_bypass(): 30fb2b1e4d41 drops the protection strip in the runnable_list
walk, which never sees the running task, and strips rq->curr ahead of the
walk instead. 3d6577ea93c9 ("sched_ext: Split curr|donor references
properly") moved the walk's strip to task_current_donor(), as protection
belongs to the scheduling context. set_next_task_scx() takes the donor off
the runnable_list, so that strip is dead too. Dropped it, and the new
strip takes rq->donor.
- scx_bypass(), merged without a marker: 5809f749fbdb evicts the running
task when bypass lifts if it can't stay on the cpu, on rq->curr.
3d6577ea93c9 moved the cap revoke eviction of the running task in
reenq_local() to rq->donor. The lift eviction takes rq->donor too, and
both comments in the block say the current donor.
Signed-off-by: Tejun Heo <tj@kernel.org>
|
|
The transaction layer has some code that can be replaced with the
existing helper function for scheduling error callbacks.
Link: https://lore.kernel.org/r/20261017153339.267337-3-o-takashi@sakamocchi.jp
Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
|
|
The transaction layer has some code that can be replaced with the
existing helper function for scheduling error callbacks.
Link: https://lore.kernel.org/r/20261017153339.267337-3-o-takashi@sakamocchi.jp
Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
|
|
The error callback work is scheduled only when the transaction instance
is no longer on the pending list. Therefore, iterating over the list to
cancel the work is pointless.
Remove the code that cancels the work while iterating over the pending
list. Once scheduled, the work can run and invoke the error callback
without being cancelled.
Link: https://lore.kernel.org/r/20261017153339.267337-2-o-takashi@sakamocchi.jp
Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
|
|
The error callback work is scheduled only when the transaction instance
is no longer on the pending list. Therefore, iterating over the list to
cancel the work is pointless.
Remove the code that cancels the work while iterating over the pending
list. Once scheduled, the work can run and invoke the error callback
without being cancelled.
Link: https://lore.kernel.org/r/20261017153339.267337-2-o-takashi@sakamocchi.jp
Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
|
|
Iomap supports non-bdev I/O through fs-provided callbacks since
commit b2f35ac4146d ("iomap: add caller-provided callbacks for read
and readahead"), making it suitable for file-backed mounts on unencoded
inodes.
Enable iomap for this case to support sub-folio buffered I/Os on
file-backed mounts and simplify the codebase.
Thanks-to: Hongbo Li <hongbohbli@tencent.com>
Reviewed-by: Chao Yu <chao@kernel.org>
Signed-off-by: Gao Xiang (Docker) <xiang@kernel.org>
|
|
Maxim Skokov says:
====================
bpftool: Fail map dump for maps that can't be iterated
"bpftool map dump" treats only ENOENT from bpf_map_get_next_key() as the
end of the map. Map types that can't be iterated (local storage, ringbuf,
bloom filter, arena, queue, stack) fail on the first key, and bpftool
prints an empty dump, "[]" or "Found 0 elements", with only a non-zero
exit status.
Patch 1 adds a helper that prints ENOTSUPP (524), which the kernel
returns from some BPF commands, as "Operation not supported" instead of
"Unknown error 524", and uses it for map create, map element
operations, map freeze and prog run. Patch 2 asks for the first key
before printing anything and fails with the map type and the error.
Patch 3 adds a test_progs case on a socket storage map, and on a queue
map without BTF for the plain output path.
The new test fails without patches 1-2 and passes with them; the other
bpftool_* selftests pass. v5 was tested on x86_64 under a bpf-next
kernel (d2141481a764e). The ENOTSUPP message was also checked by hand
for map getnext, lookup and dump on ringbuf and socket storage maps,
map create of an LRU hash without preallocation, and prog run on a
kprobe program; map delete, pop and freeze use the same helper but
were not checked by hand.
Changes in v5 (patches 1-2 of v4 are now patches 2-3):
- new patch 1: helper for ENOTSUPP, used in map dump and the other
places where bpftool prints the result of a command that can return
it (Daniel)
- patch 2 (map dump) uses the helper; kept Quentin's Acked-by as the
change is only the error string
- selftests: check the full error text for socket storage
- rebase on current bpf-next
Changes in v4:
- selftests: socket storage always has BTF, so the "Found" check could
never fail there; check the count on a queue map without BTF instead,
and fix the comment on create_sk_storage_map() (bpf-ci)
- selftests: split into sk_storage and queue subtests, new subject
- rebase on current bpf-next, patch 1 unchanged
Changes in v3:
- shorter comment on the probe (Quentin)
- selftests: unlink the pin on every exit path, so a killed run doesn't
leave a stale pin that fails every later run with EEXIST (Sashiko)
- collect Acked-by on patch 1
Changes in v2:
- probe the first key before any output instead of printing a hint for
ENOTSUPP only, so every non-iterable type gets an error and stdout gets
no "[]" (Alexei)
- drop the Fixes tag and use a neutral example map (Alexei)
- selftests: check that JSON output is only the error object and plain
stdout has no elements or count; clear the output buffer before each
run (Sashiko)
- multi-line comment style (bpf-ci)
v4: https://lore.kernel.org/bpf/20260928001513.7072-1-skokovmaksimevg@gmail.com/
v3: https://lore.kernel.org/bpf/20260926170149.541352-1-skokovmaksimevg@gmail.com/
v2: https://lore.kernel.org/bpf/20260925162031.27259-1-skokovmaksimevg@gmail.com/
v1: https://lore.kernel.org/bpf/20260924160732.485650-1-skokovmaksimevg@gmail.com/
====================
Link: https://patch.msgid.link/20261016194825.230433-1-skokovmaksimevg@gmail.com
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
|
|
Socket storage can't be iterated from user space. Create a
BPF_MAP_TYPE_SK_STORAGE map, attach storage to a socket, pin it, and
check that "bpftool map dump" fails with nothing but an error: a JSON
error object with -j, no elements on stdout in plain mode, and the
reason on stderr.
Socket storage always has BTF, so its plain dump goes through the BTF
writer and never prints an element count. Check that path with a queue
map created without BTF that holds one element: its plain dump must
fail without a "Found" line, which bpftool printed as "Found 0
elements" before, and give the reason on stderr.
Signed-off-by: Maxim Skokov <skokovmaksimevg@gmail.com>
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
Acked-by: Quentin Monnet <qmo@kernel.org>
Link: https://patch.msgid.link/20261016194825.230433-4-skokovmaksimevg@gmail.com
|
|
"bpftool map dump" walks the map with bpf_map_get_next_key() and treats
ENOENT as the end of the map. Map types that can't be iterated fail that
call on the very first key: local storage and ringbuf with ENOTSUPP,
bloom filter and arena with EOPNOTSUPP, queue and stack with EINVAL.
bpftool then ends the walk without a word and prints what it prints for
an empty map, "[]" with BTF or "Found 0 elements" without, and only the
exit status tells that the map was not read. On a socket storage map
that holds storage for a socket:
# bpftool map dump pinned /sys/fs/bpf/sk_storage
[]
Ask for the first key before printing anything, and when that fails with
anything but ENOENT, fail with the map type and the error instead of a
dump:
# bpftool map dump pinned /sys/fs/bpf/sk_storage
Error: can't dump sk_storage map: Operation not supported
# bpftool -j map dump pinned /sys/fs/bpf/sk_storage
{"error":"can't dump sk_storage map: Operation not supported"}
An error later in the walk is now reported too, as "bpftool map getnext"
reports it, and the element count is printed only after a complete walk.
Signed-off-by: Maxim Skokov <skokovmaksimevg@gmail.com>
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
Acked-by: Quentin Monnet <qmo@kernel.org>
Link: https://patch.msgid.link/20261016194825.230433-3-skokovmaksimevg@gmail.com
|
|
Some BPF commands return ENOTSUPP (524) to user space: iterating local
storage maps, element operations on ringbuf maps, creating an LRU hash
map without preallocation, freezing a struct_ops map, or running a
program type that has no test_run. ENOTSUPP is kernel internal, so
strerror() only gives "Unknown error 524":
# bpftool map getnext pinned /sys/fs/bpf/sk_storage
Error: can't get next key: Unknown error 524
Add a small helper that knows ENOTSUPP and use it where bpftool prints
the error of map create, map element operations, map freeze and prog
run.
Suggested-by: Daniel Borkmann <daniel@iogearbox.net>
Signed-off-by: Maxim Skokov <skokovmaksimevg@gmail.com>
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
Acked-by: Quentin Monnet <qmo@kernel.org>
Link: https://patch.msgid.link/20261016194825.230433-2-skokovmaksimevg@gmail.com
|
|
A bypassing sub-scheduler's tasks are run by its nearest ancestor that isn't
bypassing. The ancestor puts them on the local DSQ under its own caps.
Whether such a task may stay on the cpu and whether a cap revoke reenqueues
it are still checked against the caps of the task's own scheduler.
The bypass load balancer moves tasks between cpus without looking at caps,
so a bypassed task can run on a cpu where its own scheduler holds none. Once
its slice runs out, it can't stay. It goes to the ancestor's bypass DSQ
without a kick, and the cpu can go idle with it queued until something else
reschedules the cpu.
Make both checks against the caps of the scheduler running the task. When
bypass lifts, the running task keeps its slice without a cap check, so evict
it then if it can't stay on the cpu.
Fixes: 75a8c8202c91 ("sched_ext: Add reject DSQ for cap-rejected dispatches")
Fixes: 46a85ae6fe5b ("sched_ext: Tie cpu occupancy to SCX_CAP_BASE through the task slice")
Signed-off-by: Tejun Heo <tj@kernel.org>
Reviewed-by: Andrea Righi <arighi@nvidia.com>
|
|
Protection ends when the scheduler enters bypass, so that bypass scheduling
can cut every task's slice. scx_bypass() clears it for the queued tasks by
cycling them, and strips the running task's in the same walk over
rq->scx.runnable_list. The running task is never on that list.
set_next_task_scx() takes it off and put_prev_task_scx() puts it back, so
the strip never runs. A protected running task then keeps its slice into
bypass, where the tick's attempts to cut it are refused.
Strip the running task's protection outside the walk.
Fixes: 9cfc6ab34a31 ("sched_ext: Add SCX_TASK_PROTECTED")
Signed-off-by: Tejun Heo <tj@kernel.org>
Reviewed-by: Andrea Righi <arighi@nvidia.com>
|
|
The mmap size used when none is asked for is derived from
kernel.perf_event_mlock_kb itself (see perf_event_mlock_kb_in_pages()),
that after rounding down to a power of two number of pages ends up being
about all of a single CPU's share of that per user budget, so with 'perf
test' starting all the selected tests at once, any further mapping done
while another test still has its ring buffers mapped fails with:
Permission error mapping pages.
Consider increasing /proc/sys/kernel/perf_event_mlock_kb,
or try again with a smaller value of -m/--mmap_pages.
that then shows up as a failure or, worse, as an "unsupp" skip in tests
like the attr ones, i.e. as a false pass.
Instead of making those tests exclusive, which would serialize them
against every other test, keep them running in parallel with the tests
that don't map ring buffers and have them take turns using the budget,
via a robust, process-shared mutex (see mutex_init_pshared_robust()) in
a MAP_SHARED|MAP_ANONYMOUS area that is inherited by the forked test
processes: a test killed by a signal doesn't leave the budget locked,
the kernel marks the mutex owner-died and the next locker takes it over;
a signal that is caught rather than kills ends up the same way, with the
longjmp out of child_test_sig_handler() leaving it held until the child
exits and the kernel hands it over.
Perf's struct mutex is used, so the lock discipline is checked in clang
builds via -Wthread-safety; as the thread safety analysis can't verify
conditionally held locks, the tests that map ring buffers run through a
helper taking the mutex unconditionally around the test function call.
Robust mutexes aren't always available, at build time or, as glibc and
musl can do when get_robust_list(2)/set_robust_list(2) aren't usable, at
run time: in that case the budget is not shared and the tests run as
before, failing or skipping on starvation instead of hanging on a lock
left behind by a test killed by a signal.
Taking turns is unnecessary when nothing can exhaust the budget: once it is
spent, whatever maps on top of it is charged against RLIMIT_MEMLOCK and the
mapping is refused with EPERM when that runs out too, but only for the
processes the kernel is paranoid about and that have no CAP_IPC_LOCK to
lift the check, see perf_mmap_calc_limits(). Root, then, has no turns to
take: it never has a mapping refused, so the turnstile only buys it the
wall clock cost measured below, for a budget nothing is contending for.
The area is shared by the tests of one 'perf test' invocation only: a
second invocation running at the same time maps its own, so the two
don't take turns with each other.
The sysctl is left alone. Growing it, to leave room for whatever else
the user has running, would need the original value shared between the
invocations, so that the last one out puts it back, and it only helps a
user who can write the sysctl but lacks CAP_IPC_LOCK, the one capability
that lifts the limit.
Raising the soft RLIMIT_MEMLOCK to the hard one when a mapping is refused
is left for a follow-up: it would buy back the wall clock above without
taking turns, where the hard limit allows the raise.
None of this is done when just one of the selected tests maps ring
buffers, as there is nothing to share the budget with in that case.
Taking turns isn't free: a test that hangs while holding the budget now
holds with it every other test mapping ring buffers, where before it
would fail, or skip, by itself. And there is a cost even when nothing
goes wrong, this being the whole suite on a Ryzen 9 9950X3D, kernel 7.2.7,
32 CPUs, with the default 516 KiB budget, before and after taking turns:
budget not shared: 42 s wall clock, 72 tests Ok, 28 failed
tests take turns: 133 s wall clock, 101 tests Ok, 10 failed
Both are non-root runs of the whole series applied, the shell test markings
included, and none of the ten failures is about the budget: eight of them
pass as root, where it can't be exhausted, and the two that fail there too,
the addr2line inline unwinding one, with no branch stack sampling to ask
this machine for, and the BPF metadata collection one, whose output isn't
what the test expects, are not budget related either.
Reviewed-by: Ian Rogers <irogers@google.com>
Tested-by: Ian Rogers <irogers@google.com>
Assisted-by: LLM
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
|
|
A mutex held by a process that can die before unlocking it, like the
tests forked by 'perf test' that will take turns using the per user
budget set in kernel.perf_event_mlock_kb, has to be taken over by the
next locker instead of staying locked forever.
Robust mutexes, on top of the kernel's robust futexes, do that: when an
owner dies while holding it the kernel marks the futex word owner-died
and the next pthread_mutex_lock() returns EOWNERDEAD, then mutex_lock()
makes it consistent and takes it over; mutex_trylock() and cond_wait()
come back with EOWNERDEAD too, so they take it over the same way, with
mutex_trylock() giving the mutex back when it can't be taken over: it
is held all the same, and a failed trylock must not leak it.
Add mutex_init_pshared_robust(), returning false when robust mutexes
aren't available: from libcs without the API, gated by a feature check,
to musl and glibc refusing them at runtime, when get_robust_list(2) or
set_robust_list(2) aren't usable. glibc only sets a flag in
pthread_mutexattr_setrobust(), refusing the mutex itself in
pthread_mutex_init() with ENOTSUP instead, other errors from it still
being reported. Robust mutexes being unavailable isn't an error, just a
debug message for whoever then falls back.
Reviewed-by: Ian Rogers <irogers@google.com>
Tested-by: Ian Rogers <irogers@google.com>
Assisted-by: LLM
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
|
|
Shell tests run perf, that maps ring buffers, so they take a turn using
the per user budget in kernel.perf_event_mlock_kb by default; the ones
that don't map them, like the ones running just 'perf stat', say
" (no_mlock)" in their description line and thus keep running in parallel
with the ones taking turns.
The 'perf sched stats' tests are among them, they synthesize the samples
from /proc/schedstat without mapping a ring buffer, and the tests already
marked " (exclusive)" don't need it, exclusive implies every other
requisite and keeps them running alone.
Reviewed-by: Ian Rogers <irogers@google.com>
Tested-by: Ian Rogers <irogers@google.com>
Assisted-by: LLM
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
|
|
The C tests that run 'perf record' in the test process itself map ring
buffers and so need room in the per user budget set in
kernel.perf_event_mlock_kb: mark them with the new 'needs_mlock' suite
flag, read by the turnstile added later in the series, so that they
take turns using that budget while the tests that don't map them keep
running in parallel.
Reviewed-by: Ian Rogers <irogers@google.com>
Tested-by: Ian Rogers <irogers@google.com>
Assisted-by: LLM
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
|
|
Each perf instance maps one ring buffer per CPU, all charged to the same
per user budget set in kernel.perf_event_mlock_kb, so suites that map
them need to be told apart from the ones that don't, to later have them
take turns using that budget while the other tests keep running in
parallel.
Add the flag to 'struct test_suite' and a DEFINE_SUITE_NEEDS_MLOCK()
variant for the suites using the DEFINE_SUITE() form.
Shell tests run perf, that maps ring buffers, so they are taken to need
room in that budget by default; the requisites, a comma separated list in
the parens ending their description line, name what a test needs and take
a "no_" prefix to waive it, e.g.:
# perf stat tests (no_mlock)
"mlock" is room in that budget, so stating it only spells the default
out, "no_mlock" waives it, and "exclusive" is the catch-all that keeps a
test from running alongside any other, so it runs alone, in sequence, and
implies every other requisite, which is why it can't be waived either.
More requisites are to come, each stating what a test needs, till the
catch-all isn't needed anymore.
'perf test requisites' lists what each test needs, reusing the listing
routine and taking the same test name fragments and numbers as 'list'.
Reviewed-by: Ian Rogers <irogers@google.com>
Tested-by: Ian Rogers <irogers@google.com>
Assisted-by: LLM
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
|
|
Per skip the default output shows a self contained one liner:
unsupp 'test-record-branch-filter-any' exit 255, expected 1, no attrs recorded to verify
skipped [kernel newer than 6.12] 'test-record-group-sampling'
skipped [arch = !aarch64, machine = aarch64] 'test-record-graph-default'
The extra notes explaining the reason, with a copy pasteable command to
observe the failure, come in the very same stream when the verbosity is
bumped, at -vvv, nothing is written to a separate place for later
consultation, it is just the logging ladder:
detail: '/root/bin/perf record -o /dev/null --no-bpf-event -j any kill'
exited with 255, the expectations want the workload's 1 exit, so there
are no attrs recorded to verify; this is normally a capability problem,
run '/root/bin/perf record -o /dev/null --no-bpf-event -j any kill' to
see why
Note that 'perf test -v/-vv/-vvv attribute' appends a single -v to the
shell test entry (shell_test__run()), so to see the notes one runs the
python module directly, f.i.:
python3 tests/shell/lib/attr.py -vvv -d tests/shell/attr -p ./perf
The skip functions now return the reason fragment when they want to skip
and None when the test can go on.
The command suggested to observe the failure writes to /dev/null, so it
doesn't leave a perf.data in the directory it is run from.
Reviewed-by: Ian Rogers <irogers@google.com>
Tested-by: Ian Rogers <irogers@google.com>
Assisted-by: LLM
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
|
|
read_format's PERF_FORMAT_GROUP bit asks for the group members to be read
along with the leader, which aside from this bit is verified with the
fd/group_fd based group checking in Test.compare(), and with more than
one core PMU a group whose members are pinned to one of them, say
'default_core', leaves the clones for the other core PMUs without any
members:
$ perf stat -e '{cycles,default_core/legacy-hardware-config=3/}:S'
... the A53 leader keeps its cohort, both A73 cycles clones are alone,
read_format 20 instead of 28
so that bit isn't there: whether it is there is a consequence of how the
evlist got expanded to more than one core PMU, not of the groupness the
expectations say about, which is verified structurally, so take it out on
both sides.
That leaves the A73 leader clones, now with:
expected read_format=28|31, got 20
matching the base event expectations again.
With a single core PMU there is nothing to expand, so there is nothing to
explain away: count the core PMUs when the script starts, from what perf
makes of 'cycles', see parse_events_multi_pmu_add(), and take the bit out
only when there is more than one, leaving the machines with a single core
PMU verifying what the expectations say about it. That count is 2 on the
Amlogic A311D, where its A53 and A73 PMUs both answer for 'cycles', and 1
on a machine with a single core PMU. Where there is no binding to ask,
as when attr.py runs without setup_python.sh, the count isn't available
and the bit is taken out as well.
core_pmu_count() takes the count from the same probe core_pmu_available()
uses, so the binding is asked once, and only when a comparison needs the
answer, not at import time.
The sample id bits hide_sample_id() takes out when the expectations ask
for none are another consequence of having more than one core PMU, so the
same single core PMU gate covers them; equating the two id bits, needed
when the evlist has more than one event even with a single core PMU, keeps
applying.
Reviewed-by: Ian Rogers <irogers@google.com>
Tested-by: Ian Rogers <irogers@google.com>
Assisted-by: LLM
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
|
|
On a hybrid machine the default stat metrics are opened on each core PMU,
and the events of the other one have their own encodings: on an i7-14700K
cpu_atom (type 10) opens UOPS_RETIRED.ALL, config 194, which the
expectations, written for cpu_core (type 4), don't name:
expected type=4, got 10
expected config=209|391, got 194
The test already masks the core PMU type out of 'config' for the legacy
events, but raw events keep their type and encoding, so leave out the
events of the core PMUs no expectation names, like the dummy tracking
event. Core PMUs are found as perf finds them: 'cpu' or 'cpum_cf', or a
PMU with a 'cpus' file in sysfs. Nothing is left out unless an
expectation names a core PMU, so single core PMU machines and the tests
with only legacy expectations are not affected.
Reviewed-by: Ian Rogers <irogers@google.com>
Tested-by: Ian Rogers <irogers@google.com>
Assisted-by: LLM
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
|
