* Re: [PATCH mm-unstable v19 11/14] mm/khugepaged: Introduce mTHP collapse support
From: David Hildenbrand (Arm) @ 2026-06-09 9:25 UTC (permalink / raw)
To: Nico Pache, Lance Yang
Cc: linux-doc, linux-kernel, linux-mm, linux-trace-kernel, aarcange,
akpm, anshuman.khandual, apopple, baohua, baolin.wang, byungchul,
catalin.marinas, cl, corbet, dave.hansen, dev.jain, gourry,
hannes, hughd, jack, jackmanb, jannh, jglisse, joshua.hahnjy, kas,
liam, ljs, mathieu.desnoyers, matthew.brost, mhiramat, mhocko,
peterx, pfalcato, rakie.kim, raquini, rdunlap, richard.weiyang,
rientjes, rostedt, rppt, ryan.roberts, shivankg, sunnanyong,
surenb, thomas.hellstrom, tiwai, usamaarif642, vbabka,
vishal.moola, wangkefeng.wang, will, willy, yang, ying.huang, ziy,
zokeefe
In-Reply-To: <CAA1CXcBY_2372eJru8VoCq90rUMxn7w23hHou68MmXRv48NRXg@mail.gmail.com>
On 6/9/26 11:06, Nico Pache wrote:
> On Mon, Jun 8, 2026 at 8:57 AM David Hildenbrand (Arm) <david@kernel.org> wrote:
>>
>> On 6/6/26 12:28, Lance Yang wrote:
>>>
>>>
>>> Looks broken for swap PTEs in PMD collapse ...
>>>
>>> collapse_scan_pmd() allows them up to max_ptes_swap and record them in
>>> unmapped, but they don't get a bit in mthp_present_ptes. And then
>>> mthp_collapse() does the check above:
>>
>> Right. I assumed this is implicitly handled by the optimization in collapse_scan_pmd:
>>
>> if (enabled_orders != BIT(HPAGE_PMD_ORDER))
>> max_ptes_none = KHUGEPAGED_MAX_PTES_LIMIT;
>>
>> But we perform the check a second time.
>>
>>>
>>> nr_occupied_ptes >= nr_ptes - max_ptes_none
>>>
>>> So max_ptes_none=0 + 511 present PTEs + one allowed swap PTE won't even
>>> call collapse_huge_page() for PMD order.
>>>
>>> Shouldn't we account for them in the PMD-order check? Something like:
>>>
>>> if (is_pmd_order(order))
>>> nr_occupied_ptes += unmapped;
>
> This solution seems good for a temporary fixup. but longterm we may
> want something else. I'm still not sure how we plan on supporting
> swapin without causing creep. So I'd be ok with adding a fix for
> legacy PMD behavior until we know how to handle mTHP creep correctly.
>
>> As an alternative, we could either 1) skip the check there for
>> pmd order (as the check was already done); or 2) introduce+maintain
>> a bitmap that tracks non-present PTEs.
>>
>> @@ -1475,7 +1477,9 @@ static enum scan_result mthp_collapse(struct mm_struct *mm,
>> nr_occupied_ptes = bitmap_weight_from(cc->mthp_present_ptes, offset,
>> offset + nr_ptes);
>>
>> - if (nr_occupied_ptes >= nr_ptes - max_ptes_none) {
>> + /* Check was already done in the caller. */
>> + if (is_pmd_order(order) ||
>> + nr_occupied_ptes >= nr_ptes - max_ptes_none) {
>> enum scan_result ret;
>>
>> collapse_address = address + offset * PAGE_SIZE;
>>
>> 2) would probably be cleanest long-term.
>
> That would be best for future swapin support in mTHP, but I still
> don't think it solves the creep issue.
It wouldn't, we'd simply maintain the state we collect + rely on in separate
bitmaps. On swapin, we'd have to update/refresh bitmaps I guess.
> Perhaps we could combine the
> two bitmaps to determine if it would make the future collapse eligible
> again? Not sure but ill start thinking about it.
>
> Should I send a fixup for this using Lance's solution? Or does Lance
> want to send a patch out with the fixes tag?
If Lance could send a fixup, explaining the situation, that would be nice.
--
Cheers,
David
^ permalink raw reply
* Re: [PATCH v8 2/6] mm/memory-failure: surface unhandlable kernel pages as -ENOTRECOVERABLE
From: Lance Yang @ 2026-06-09 9:08 UTC (permalink / raw)
To: David Hildenbrand (Arm), Miaohe Lin, Breno Leitao
Cc: linux-mm, linux-kernel, linux-doc, linux-kselftest,
linux-trace-kernel, kernel-team, Andrew Morton, Lorenzo Stoakes,
Vlastimil Babka, Mike Rapoport, Suren Baghdasaryan, Michal Hocko,
Shuah Khan, Naoya Horiguchi, Steven Rostedt, Masami Hiramatsu,
Mathieu Desnoyers, Jonathan Corbet, Shuah Khan, Liam R. Howlett
In-Reply-To: <f2a4d5c8-3d7d-4fc3-8769-66e0c24866fb@kernel.org>
On 2026/6/9 15:09, David Hildenbrand (Arm) wrote:
> On 6/9/26 04:39, Miaohe Lin wrote:
>> On 2026/6/8 22:15, Breno Leitao wrote:
>>> On Fri, Jun 05, 2026 at 11:42:53AM +0200, David Hildenbrand (Arm) wrote:
>>>>
>>>> I mean, any such races can currently already happen one way or the other?
>>>>
>>>> Really, the only way to not get races is to tryget the (compound)page,
>>>> revalidate that the page is still part of the compound page.
>>>>
>>>> I'm not sure if that's really a good idea.
>>>>
>>>> But my memory is a bit vague in which scenarios we already hold a page reference
>>>> here to prevent any concurrent freeing?
>>>
>>> No, we don't hold one here in the case that matters.
>>>
>>> HWPoisonKernelOwned() runs at the very top of get_any_page(), before
>>> try_again: and before __get_hwpoison_page(). The first refcount taken in
>>> the whole path is the folio_try_get() inside __get_hwpoison_page(), which
>>> runs *after* the short-circuit.
>>>
>>> So get_any_page() itself never holds a reference at the check -- the only way
>>> one exists is if the caller passed MF_COUNT_INCREASED (count_increased ==
>>> true).
>>>
>>> So on the MCE/GHES path -- the one this panic option exists for -- no
>>> reference is held when HWPoisonKernelOwned() does its compound_head() +
>>> PageSlab()/PageTable()/PageLargeKmalloc() checks.
>>>
>>> Given that, I'd rather keep it racy and take no refcount than add a
>>> tryget + revalidate purely for this check. As I've said earleir, an operator
>>
>> Would it be acceptable to add a simple recheck? Something like below:
>>
>> retry:
>> head = compound_head(page);
>> PageSlab()/PageTable()/PageLargeKmalloc() checks
>> if (head != compound_head(page))
>> goto retry
>
> Sure. I guess it could still be racy in some weird scenarios where we
> free+allocate+free in-between.
+1, sounds reasonable to me. Still racy, but acceptable here I guess :D
^ permalink raw reply
* Re: [PATCH mm-unstable v19 11/14] mm/khugepaged: Introduce mTHP collapse support
From: Nico Pache @ 2026-06-09 9:06 UTC (permalink / raw)
To: David Hildenbrand (Arm), Lance Yang
Cc: linux-doc, linux-kernel, linux-mm, linux-trace-kernel, aarcange,
akpm, anshuman.khandual, apopple, baohua, baolin.wang, byungchul,
catalin.marinas, cl, corbet, dave.hansen, dev.jain, gourry,
hannes, hughd, jack, jackmanb, jannh, jglisse, joshua.hahnjy, kas,
liam, ljs, mathieu.desnoyers, matthew.brost, mhiramat, mhocko,
peterx, pfalcato, rakie.kim, raquini, rdunlap, richard.weiyang,
rientjes, rostedt, rppt, ryan.roberts, shivankg, sunnanyong,
surenb, thomas.hellstrom, tiwai, usamaarif642, vbabka,
vishal.moola, wangkefeng.wang, will, willy, yang, ying.huang, ziy,
zokeefe
In-Reply-To: <2553caae-9e0e-42a7-8b61-d1216f1e81fa@kernel.org>
On Mon, Jun 8, 2026 at 8:57 AM David Hildenbrand (Arm) <david@kernel.org> wrote:
>
> On 6/6/26 12:28, Lance Yang wrote:
> >
> > On Fri, Jun 05, 2026 at 10:14:18AM -0600, Nico Pache wrote:
> >> Enable khugepaged to collapse to mTHP orders. This patch implements the
> >> main scanning logic using a bitmap to track occupied pages and the
> >> algorithm to find optimal collapse sizes.
> >>
> >> Previous to this patch, PMD collapse had 3 main phases, a light weight
> >> scanning phase (mmap_read_lock) that determines a potential PMD
> >> collapse, an alloc phase (mmap unlocked), then finally heavier collapse
> >> phase (mmap_write_lock).
> >>
> >> To enabled mTHP collapse we make the following changes:
> >>
> >> During PMD scan phase, track occupied pages in a bitmap. When mTHP
> >> orders are enabled, we remove the restriction of max_ptes_none during the
> >> scan phase to avoid missing potential mTHP collapse candidates. Once we
> >> have scanned the full PMD range and updated the bitmap to track occupied
> >> pages, we use the bitmap to find the optimal mTHP size.
> >>
> >> Implement mthp_collapse() to walk forward through the bitmap and
> >> determine the best eligible order for each naturally-aligned region. The
> >> algorithm starts at the beginning of the PMD range and, for each offset,
> >> tries the highest order that fits the alignment. If the number of
> >> occupied PTEs in that region satisfies the max_ptes_none threshold for
> >> that order, a collapse is attempted. On failure, the order is
> >> decremented and the same offset is retried at the next smaller size. Once
> >> the smallest enabled order is exhausted (or a collapse succeeds), the
> >> offset advances past the region just processed, and the next attempt
> >> starts at the highest order permitted by the new offset's natural
> >> alignment.
> >>
> >> The algorithm works as follows:
> >> 1) set offset=0 and order=HPAGE_PMD_ORDER
> >> 2) if the order is not enabled, go to step (5)
> >> 3) count occupied PTEs in the (offset, order) range using
> >> bitmap_weight_from()
> >> 4) if the count satisfies the max_ptes_none threshold, attempt
> >> collapse; on success, advance to step (6)
> >> 5) if a smaller enabled order exists, decrement order and retry
> >> from step (2) at the same offset
> >> 6) advance offset past the current region and compute the next
> >> order from the new offset's natural alignment via __ffs(offset),
> >> capped at HPAGE_PMD_ORDER
> >> 7) repeat from step (2) until the full PMD range is covered
> >>
> >> mTHP collapses reject regions containing swapped out or shared pages.
> >> This is because adding new entries can lead to new none pages, and these
> >> may lead to constant promotion into a higher order mTHP. A similar
> >> issue can occur with "max_ptes_none > HPAGE_PMD_NR/2" due to a collapse
> >> introducing at least 2x the number of pages, and on a future scan will
> >> satisfy the promotion condition once again. This issue is prevented via
> >> the collapse_max_ptes_none() function which imposes the max_ptes_none
> >> restrictions above.
> >>
> >> We currently only support mTHP collapse for max_ptes_none values of 0
> >> and HPAGE_PMD_NR - 1. resulting in the following behavior:
> >>
> >> - max_ptes_none=0: Never introduce new empty pages during collapse
> >> - max_ptes_none=HPAGE_PMD_NR-1: Always try collapse to the highest
> >> available mTHP order
> >>
> >> Any other max_ptes_none value will emit a warning and default mTHP
> >> collapse to max_ptes_none=0. There should be no behavior change for PMD
> >> collapse.
> >>
> >> Once we determine what mTHP sizes fits best in that PMD range a collapse
> >> is attempted. A minimum collapse order of 2 is used as this is the lowest
> >> order supported by anon memory as defined by THP_ORDERS_ALL_ANON.
> >>
> >> Currently madv_collapse is not supported and will only attempt PMD
> >> collapse.
> >>
> >> We can also remove the check for is_khugepaged inside the PMD scan as
> >> the collapse_max_ptes_none() function handles this logic now.
> >>
> >> Signed-off-by: Nico Pache <npache@redhat.com>
> >> ---
> >> mm/khugepaged.c | 146 +++++++++++++++++++++++++++++++++++++++++++++---
> >> 1 file changed, 138 insertions(+), 8 deletions(-)
> >>
> >> diff --git a/mm/khugepaged.c b/mm/khugepaged.c
> >> index ec886a031952..430047316f43 100644
> >> --- a/mm/khugepaged.c
> >> +++ b/mm/khugepaged.c
> >> @@ -99,6 +99,8 @@ static DEFINE_READ_MOSTLY_HASHTABLE(mm_slots_hash, MM_SLOTS_HASH_BITS);
> >>
> >> static struct kmem_cache *mm_slot_cache __ro_after_init;
> >>
> >> +#define KHUGEPAGED_MIN_MTHP_ORDER 2
> >> +
> >> struct collapse_control {
> >> bool is_khugepaged;
> >>
> >> @@ -110,6 +112,9 @@ struct collapse_control {
> >>
> >> /* nodemask for allocation fallback */
> >> nodemask_t alloc_nmask;
> >> +
> >> + /* Each bit represents a single occupied (!none/zero) page. */
> >> + DECLARE_BITMAP(mthp_present_ptes, MAX_PTRS_PER_PTE);
> >> };
> >>
> >> /**
> >> @@ -1440,20 +1445,130 @@ static enum scan_result collapse_huge_page(struct mm_struct *mm, unsigned long s
> >> return result;
> >> }
> >>
> >> +/* Return the highest naturally aligned order that fits at @offset within a PMD. */
> >> +static unsigned int max_order_from_offset(unsigned int offset)
> >> +{
> >> + if (offset == 0)
> >> + return HPAGE_PMD_ORDER;
> >> +
> >> + return min_t(unsigned int, __ffs(offset), HPAGE_PMD_ORDER);
> >> +}
> >> +
> >> +/*
> >> + * mthp_collapse() consumes the bitmap that is generated during
> >> + * collapse_scan_pmd() to determine what regions and mTHP orders fit best.
> >> + *
> >> + * Each bit in cc->mthp_present_ptes represents a single occupied (!none/zero)
> >> + * page. We start at the PMD order and check if it is eligible for collapse;
> >> + * if not, we check the left and right halves of the PTE page table we are
> >> + * examining at a lower order.
> >> + *
> >> + * For each of these, we determine how many PTE entries are occupied in the
> >> + * range of PTE entries we propose to collapse, then we compare this to a
> >> + * threshold number of PTE entries which would need to be occupied for a
> >> + * collapse to be permitted at that order (accounting for max_ptes_none).
> >> + *
> >> + * If a collapse is permitted, we attempt to collapse the PTE range into a
> >> + * mTHP.
> >> + */
> >> +static enum scan_result mthp_collapse(struct mm_struct *mm,
> >> + unsigned long address, int referenced, int unmapped,
> >> + struct collapse_control *cc, unsigned long enabled_orders)
> >> +{
> >> + unsigned int nr_occupied_ptes, nr_ptes, max_ptes_none;
> >> + enum scan_result last_result = SCAN_FAIL;
> >> + int collapsed = 0;
> >> + bool alloc_failed = false;
> >> + unsigned long collapse_address;
> >> + unsigned int offset = 0;
> >> + unsigned int order = HPAGE_PMD_ORDER;
> >> +
> >> + while (offset < HPAGE_PMD_NR) {
> >> + nr_ptes = 1UL << order;
> >> +
> >> + if (!test_bit(order, &enabled_orders))
> >> + goto next_order;
> >> +
> >> + max_ptes_none = collapse_max_ptes_none(cc, NULL, order);
> >> + nr_occupied_ptes = bitmap_weight_from(cc->mthp_present_ptes, offset,
> >> + offset + nr_ptes);
> >> +
> >> + if (nr_occupied_ptes >= nr_ptes - max_ptes_none) {
> >
> > Looks broken for swap PTEs in PMD collapse ...
> >
> > collapse_scan_pmd() allows them up to max_ptes_swap and record them in
> > unmapped, but they don't get a bit in mthp_present_ptes. And then
> > mthp_collapse() does the check above:
>
> Right. I assumed this is implicitly handled by the optimization in collapse_scan_pmd:
>
> if (enabled_orders != BIT(HPAGE_PMD_ORDER))
> max_ptes_none = KHUGEPAGED_MAX_PTES_LIMIT;
>
> But we perform the check a second time.
>
> >
> > nr_occupied_ptes >= nr_ptes - max_ptes_none
> >
> > So max_ptes_none=0 + 511 present PTEs + one allowed swap PTE won't even
> > call collapse_huge_page() for PMD order.
> >
> > Shouldn't we account for them in the PMD-order check? Something like:
> >
> > if (is_pmd_order(order))
> > nr_occupied_ptes += unmapped;
This solution seems good for a temporary fixup. but longterm we may
want something else. I'm still not sure how we plan on supporting
swapin without causing creep. So I'd be ok with adding a fix for
legacy PMD behavior until we know how to handle mTHP creep correctly.
> As an alternative, we could either 1) skip the check there for
> pmd order (as the check was already done); or 2) introduce+maintain
> a bitmap that tracks non-present PTEs.
>
> @@ -1475,7 +1477,9 @@ static enum scan_result mthp_collapse(struct mm_struct *mm,
> nr_occupied_ptes = bitmap_weight_from(cc->mthp_present_ptes, offset,
> offset + nr_ptes);
>
> - if (nr_occupied_ptes >= nr_ptes - max_ptes_none) {
> + /* Check was already done in the caller. */
> + if (is_pmd_order(order) ||
> + nr_occupied_ptes >= nr_ptes - max_ptes_none) {
> enum scan_result ret;
>
> collapse_address = address + offset * PAGE_SIZE;
>
> 2) would probably be cleanest long-term.
That would be best for future swapin support in mTHP, but I still
don't think it solves the creep issue. Perhaps we could combine the
two bitmaps to determine if it would make the future collapse eligible
again? Not sure but ill start thinking about it.
Should I send a fixup for this using Lance's solution? Or does Lance
want to send a patch out with the fixes tag?
>
> --
> Cheers,
>
> David
>
^ permalink raw reply
* Re: [PATCH mm-unstable v19 11/14] mm/khugepaged: Introduce mTHP collapse support
From: Nico Pache @ 2026-06-09 9:01 UTC (permalink / raw)
To: Lorenzo Stoakes
Cc: linux-doc, linux-kernel, linux-mm, linux-trace-kernel, aarcange,
akpm, anshuman.khandual, apopple, baohua, baolin.wang, byungchul,
catalin.marinas, cl, corbet, dave.hansen, david, dev.jain, gourry,
hannes, hughd, jack, jackmanb, jannh, jglisse, joshua.hahnjy, kas,
lance.yang, liam, mathieu.desnoyers, matthew.brost, mhiramat,
mhocko, peterx, pfalcato, rakie.kim, raquini, rdunlap,
richard.weiyang, rientjes, rostedt, rppt, ryan.roberts, shivankg,
sunnanyong, surenb, thomas.hellstrom, tiwai, usamaarif642, vbabka,
vishal.moola, wangkefeng.wang, will, willy, yang, ying.huang, ziy,
zokeefe
In-Reply-To: <aiMTuXKQ5qxKYo60@lucifer>
On Fri, Jun 5, 2026 at 12:38 PM Lorenzo Stoakes <ljs@kernel.org> wrote:
>
> On Fri, Jun 05, 2026 at 10:14:18AM -0600, Nico Pache wrote:
> > Enable khugepaged to collapse to mTHP orders. This patch implements the
> > main scanning logic using a bitmap to track occupied pages and the
> > algorithm to find optimal collapse sizes.
> >
> > Previous to this patch, PMD collapse had 3 main phases, a light weight
> > scanning phase (mmap_read_lock) that determines a potential PMD
> > collapse, an alloc phase (mmap unlocked), then finally heavier collapse
> > phase (mmap_write_lock).
> >
> > To enabled mTHP collapse we make the following changes:
> >
> > During PMD scan phase, track occupied pages in a bitmap. When mTHP
> > orders are enabled, we remove the restriction of max_ptes_none during the
> > scan phase to avoid missing potential mTHP collapse candidates. Once we
> > have scanned the full PMD range and updated the bitmap to track occupied
> > pages, we use the bitmap to find the optimal mTHP size.
> >
> > Implement mthp_collapse() to walk forward through the bitmap and
> > determine the best eligible order for each naturally-aligned region. The
> > algorithm starts at the beginning of the PMD range and, for each offset,
> > tries the highest order that fits the alignment. If the number of
> > occupied PTEs in that region satisfies the max_ptes_none threshold for
> > that order, a collapse is attempted. On failure, the order is
> > decremented and the same offset is retried at the next smaller size. Once
> > the smallest enabled order is exhausted (or a collapse succeeds), the
> > offset advances past the region just processed, and the next attempt
> > starts at the highest order permitted by the new offset's natural
> > alignment.
>
> I think still it might have been nice to discuss why we are not
> e.g. greedily trying to find the biggest possible mTHP size (if we did, we
> would try the highest offset first), but we can save that for adding some
> documentation somewhere later tbh.
We are, the algorithm tries PMD, then order 8, then order 7, and so
on. Due to the required alignment, if the N-1 order succeeds, we try
the same order at the neighboring offset.
So if we collapse a order 8, the following collapse attempt will be
order 8 at 256. We always try the highest order allowed for a given
offset :)
>
> This commit message is long enough as it is :>)
>
> >
> > The algorithm works as follows:
> > 1) set offset=0 and order=HPAGE_PMD_ORDER
> > 2) if the order is not enabled, go to step (5)
> > 3) count occupied PTEs in the (offset, order) range using
> > bitmap_weight_from()
> > 4) if the count satisfies the max_ptes_none threshold, attempt
> > collapse; on success, advance to step (6)
> > 5) if a smaller enabled order exists, decrement order and retry
> > from step (2) at the same offset
> > 6) advance offset past the current region and compute the next
> > order from the new offset's natural alignment via __ffs(offset),
> > capped at HPAGE_PMD_ORDER
> > 7) repeat from step (2) until the full PMD range is covered
> >
> > mTHP collapses reject regions containing swapped out or shared pages.
> > This is because adding new entries can lead to new none pages, and these
> > may lead to constant promotion into a higher order mTHP. A similar
> > issue can occur with "max_ptes_none > HPAGE_PMD_NR/2" due to a collapse
> > introducing at least 2x the number of pages, and on a future scan will
> > satisfy the promotion condition once again. This issue is prevented via
> > the collapse_max_ptes_none() function which imposes the max_ptes_none
> > restrictions above.
> >
> > We currently only support mTHP collapse for max_ptes_none values of 0
> > and HPAGE_PMD_NR - 1. resulting in the following behavior:
> >
> > - max_ptes_none=0: Never introduce new empty pages during collapse
> > - max_ptes_none=HPAGE_PMD_NR-1: Always try collapse to the highest
> > available mTHP order
> >
> > Any other max_ptes_none value will emit a warning and default mTHP
> > collapse to max_ptes_none=0. There should be no behavior change for PMD
> > collapse.
> >
> > Once we determine what mTHP sizes fits best in that PMD range a collapse
> > is attempted. A minimum collapse order of 2 is used as this is the lowest
> > order supported by anon memory as defined by THP_ORDERS_ALL_ANON.
> >
> > Currently madv_collapse is not supported and will only attempt PMD
> > collapse.
> >
> > We can also remove the check for is_khugepaged inside the PMD scan as
> > the collapse_max_ptes_none() function handles this logic now.
>
> It'd be nice to have kept the ASCII diagram here too :'( but this is fine,
>
> >
> > Signed-off-by: Nico Pache <npache@redhat.com>
>
> This all LGTM, and we can fix up any issues that arise later if anything
> does break. So:
>
> Reviewed-by: Lorenzo Stoakes <ljs@kernel.org>
Thanks for reviewing :)
>
> > ---
> > mm/khugepaged.c | 146 +++++++++++++++++++++++++++++++++++++++++++++---
> > 1 file changed, 138 insertions(+), 8 deletions(-)
> >
> > diff --git a/mm/khugepaged.c b/mm/khugepaged.c
> > index ec886a031952..430047316f43 100644
> > --- a/mm/khugepaged.c
> > +++ b/mm/khugepaged.c
> > @@ -99,6 +99,8 @@ static DEFINE_READ_MOSTLY_HASHTABLE(mm_slots_hash, MM_SLOTS_HASH_BITS);
> >
> > static struct kmem_cache *mm_slot_cache __ro_after_init;
> >
> > +#define KHUGEPAGED_MIN_MTHP_ORDER 2
> > +
> > struct collapse_control {
> > bool is_khugepaged;
> >
> > @@ -110,6 +112,9 @@ struct collapse_control {
> >
> > /* nodemask for allocation fallback */
> > nodemask_t alloc_nmask;
> > +
> > + /* Each bit represents a single occupied (!none/zero) page. */
> > + DECLARE_BITMAP(mthp_present_ptes, MAX_PTRS_PER_PTE);
> > };
> >
> > /**
> > @@ -1440,20 +1445,130 @@ static enum scan_result collapse_huge_page(struct mm_struct *mm, unsigned long s
> > return result;
> > }
> >
> > +/* Return the highest naturally aligned order that fits at @offset within a PMD. */
> > +static unsigned int max_order_from_offset(unsigned int offset)
> > +{
> > + if (offset == 0)
> > + return HPAGE_PMD_ORDER;
> > +
> > + return min_t(unsigned int, __ffs(offset), HPAGE_PMD_ORDER);
> > +}
>
> Thanks this is better! I wonder if we can ever actually see an
> __ffs(offset) that's > HPAGE_PMD_ORDER but probably better safe than sorry
> here with the min_t.
I don't think so unless offset somehow exceeds 512 (it shouldn't), but
like you said, better safe than sorry.
>
> > +
> > +/*
> > + * mthp_collapse() consumes the bitmap that is generated during
> > + * collapse_scan_pmd() to determine what regions and mTHP orders fit best.
> > + *
> > + * Each bit in cc->mthp_present_ptes represents a single occupied (!none/zero)
> > + * page. We start at the PMD order and check if it is eligible for collapse;
> > + * if not, we check the left and right halves of the PTE page table we are
> > + * examining at a lower order.
> > + *
> > + * For each of these, we determine how many PTE entries are occupied in the
> > + * range of PTE entries we propose to collapse, then we compare this to a
> > + * threshold number of PTE entries which would need to be occupied for a
> > + * collapse to be permitted at that order (accounting for max_ptes_none).
> > + *
> > + * If a collapse is permitted, we attempt to collapse the PTE range into a
> > + * mTHP.
> > + */
> > +static enum scan_result mthp_collapse(struct mm_struct *mm,
> > + unsigned long address, int referenced, int unmapped,
> > + struct collapse_control *cc, unsigned long enabled_orders)
> > +{
> > + unsigned int nr_occupied_ptes, nr_ptes, max_ptes_none;
> > + enum scan_result last_result = SCAN_FAIL;
> > + int collapsed = 0;
> > + bool alloc_failed = false;
> > + unsigned long collapse_address;
> > + unsigned int offset = 0;
> > + unsigned int order = HPAGE_PMD_ORDER;
> > +
> > + while (offset < HPAGE_PMD_NR) {
> > + nr_ptes = 1UL << order;
> > +
> > + if (!test_bit(order, &enabled_orders))
> > + goto next_order;
> > +
> > + max_ptes_none = collapse_max_ptes_none(cc, NULL, order);
> > + nr_occupied_ptes = bitmap_weight_from(cc->mthp_present_ptes, offset,
> > + offset + nr_ptes);
> > +
> > + if (nr_occupied_ptes >= nr_ptes - max_ptes_none) {
> > + enum scan_result ret;
> > +
> > + collapse_address = address + offset * PAGE_SIZE;
> > + ret = collapse_huge_page(mm, collapse_address, referenced,
> > + unmapped, cc, order);
> > + switch (ret) {
> > + /* Cases where we continue to next collapse candidate */
> > + case SCAN_SUCCEED:
> > + collapsed += nr_ptes;
> > + fallthrough;
> > + case SCAN_PTE_MAPPED_HUGEPAGE:
> > + goto next_offset;
> > + /* Cases where lower orders might still succeed */
> > + case SCAN_ALLOC_HUGE_PAGE_FAIL:
> > + alloc_failed = true;
> > + last_result = ret;
> > + goto next_order;
> > + /* Cases where no further collapse is possible */
> > + case SCAN_PMD_MAPPED:
> > + fallthrough;
> > + default:
> > + last_result = ret;
> > + goto done;
> > + }
> > + }
> > +
> > +next_order:
> > + /*
> > + * Continue with the next smaller order if there is still
> > + * any smaller order enabled. When at the smallest order
> > + * we must always move to the next offset.
> > + */
> > + if (order > KHUGEPAGED_MIN_MTHP_ORDER &&
> > + (enabled_orders & GENMASK(order - 1, 0))) {
>
> Honestly wasn't aware of GENMASK() before :)
I wasn't either! (thanks David ;) )
>
> > + order--;
> > + continue;
> > + }
> > +next_offset:
> > + /*
> > + * Advance past the region we just processed and determine the
> > + * highest order we can attempt next. Since huge pages must be
> > + * naturally aligned, the max order we can attempt next is
> > + * limited by the alignment of the new offset.
> > + * E.g. if we collapsed a order-2 mTHP at offset 0, offset
> > + * becomes 4 and __ffs(4) == 2, so the next attempt starts at
> > + * order 2.
> > + */
>
> Great comment thanks!
>
> > + offset += nr_ptes;
> > + order = max_order_from_offset(offset);
> > + }
> > +done:
> > + if (collapsed)
> > + return SCAN_SUCCEED;
> > + if (alloc_failed)
> > + return SCAN_ALLOC_HUGE_PAGE_FAIL;
> > + return last_result;
> > +}
> > +
> > static enum scan_result collapse_scan_pmd(struct mm_struct *mm,
> > struct vm_area_struct *vma, unsigned long start_addr,
> > bool *lock_dropped, struct collapse_control *cc)
> > {
> > - const unsigned int max_ptes_none = collapse_max_ptes_none(cc, vma, HPAGE_PMD_ORDER);
> > const unsigned int max_ptes_shared = collapse_max_ptes_shared(cc, HPAGE_PMD_ORDER);
> > const unsigned int max_ptes_swap = collapse_max_ptes_swap(cc, HPAGE_PMD_ORDER);
> > + unsigned int max_ptes_none = collapse_max_ptes_none(cc, vma, HPAGE_PMD_ORDER);
> > + enum tva_type tva_flags = cc->is_khugepaged ? TVA_KHUGEPAGED : TVA_FORCED_COLLAPSE;
> > pmd_t *pmd;
> > - pte_t *pte, *_pte;
> > + pte_t *pte, *_pte, pteval;
> > + int i;
> > int none_or_zero = 0, shared = 0, referenced = 0;
> > enum scan_result result = SCAN_FAIL;
> > struct page *page = NULL;
> > struct folio *folio = NULL;
> > unsigned long addr;
> > + unsigned long enabled_orders;
> > spinlock_t *ptl;
> > int node = NUMA_NO_NODE, unmapped = 0;
> >
> > @@ -1465,8 +1580,19 @@ static enum scan_result collapse_scan_pmd(struct mm_struct *mm,
> > goto out;
> > }
> >
> > + bitmap_zero(cc->mthp_present_ptes, MAX_PTRS_PER_PTE);
> > memset(cc->node_load, 0, sizeof(cc->node_load));
> > nodes_clear(cc->alloc_nmask);
> > +
> > + enabled_orders = collapse_possible_orders(vma, vma->vm_flags, tva_flags);
> > +
> > + /*
> > + * If PMD is the only enabled order, enforce max_ptes_none, otherwise
> > + * scan all pages to populate the bitmap for mTHP collapse.
> > + */
> > + if (enabled_orders != BIT(HPAGE_PMD_ORDER))
> > + max_ptes_none = KHUGEPAGED_MAX_PTES_LIMIT;
> > +
> > pte = pte_offset_map_lock(mm, pmd, start_addr, &ptl);
> > if (!pte) {
> > cc->progress++;
> > @@ -1474,11 +1600,13 @@ static enum scan_result collapse_scan_pmd(struct mm_struct *mm,
> > goto out;
> > }
> >
> > - for (addr = start_addr, _pte = pte; _pte < pte + HPAGE_PMD_NR;
> > - _pte++, addr += PAGE_SIZE) {
> > + for (i = 0; i < HPAGE_PMD_NR; i++) {
> > + _pte = pte + i;
> > + addr = start_addr + i * PAGE_SIZE;
> > + pteval = ptep_get(_pte);
> > +
> > cc->progress++;
> >
> > - pte_t pteval = ptep_get(_pte);
> > if (pte_none_or_zero(pteval)) {
> > if (++none_or_zero > max_ptes_none) {
> > result = SCAN_EXCEED_NONE_PTE;
> > @@ -1558,6 +1686,8 @@ static enum scan_result collapse_scan_pmd(struct mm_struct *mm,
> > }
> > }
> >
> > + /* Set bit for occupied pages */
> > + __set_bit(i, cc->mthp_present_ptes);
> > /*
> > * Record which node the original page is from and save this
> > * information to cc->node_load[].
> > @@ -1616,9 +1746,9 @@ static enum scan_result collapse_scan_pmd(struct mm_struct *mm,
> > if (result == SCAN_SUCCEED) {
> > /* collapse_huge_page expects the lock to be dropped before calling */
> > mmap_read_unlock(mm);
> > - result = collapse_huge_page(mm, start_addr, referenced,
> > - unmapped, cc, HPAGE_PMD_ORDER);
> > - /* collapse_huge_page will return with the mmap_lock released */
> > + result = mthp_collapse(mm, start_addr, referenced,
> > + unmapped, cc, enabled_orders);
> > + /* mmap_lock was released above, set lock_dropped */
> > *lock_dropped = true;
> > }
> > out:
> > --
> > 2.54.0
> >
>
> Cheers, Lorenzo
>
^ permalink raw reply
* Re: [PATCH v2] rethook: Remove the running task check in rethook_find_ret_addr()
From: Tengda Wu @ 2026-06-09 8:53 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Masami Hiramatsu, Steven Rostedt, Mathieu Desnoyers,
Alexei Starovoitov, linux-trace-kernel, linux-kernel
In-Reply-To: <20260609071412.GG3102624@noisy.programming.kicks-ass.net>
On 2026/6/9 15:14, Peter Zijlstra wrote:
> On Tue, Jun 09, 2026 at 08:57:28AM +0800, Tengda Wu wrote:
>> The current check in rethook_find_ret_addr() prevents obtaining a return
>> address when the target task is marked as running. However, this condition
>> is both insufficient for safety and unnecessary for its intended purpose.
>
> Depends on what safety means. If safety means not crashing, it is
> entirely superfluous. If safety means correctness, then yes, it is
> insufficient.
>
>> The check is inherently racy: a task can begin running on another CPU
>> immediately after task_is_running() returns false, potentially leading to
>> concurrent modification of rethook data structures while the iteration is
>> in progress.
>>
>> Rather than attempting to fix this unreliable check deep in the unwinding
>> path, remove it entirely. Callers that require consistency are expected
>> to provide a safe context.
>
> Perhaps also note that unwind_next() will hold RCU and the rethook_node
> things are RCU freed, so while the iteration might go off the rails and
> return invalid information, it will not crash.
>
>
>> Fixes: 54ecbe6f1ed5 ("rethook: Add a generic return hook")
>> Signed-off-by: Tengda Wu <wutengda@huaweicloud.com>
>
> With clarifications:
>
> Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
>
Thank you for the review and suggestions, Peter.
I have incorporated your feedback into v3. The patch has been sent out.
https://lore.kernel.org/all/20260609084953.901576-1-wutengda@huaweicloud.com/
Best regards,
Tengda
^ permalink raw reply
* [PATCH v3] rethook: Remove the running task check in rethook_find_ret_addr()
From: Tengda Wu @ 2026-06-09 8:49 UTC (permalink / raw)
To: Masami Hiramatsu, Peter Zijlstra
Cc: Steven Rostedt, Mathieu Desnoyers, Alexei Starovoitov,
linux-trace-kernel, linux-kernel, Tengda Wu
The current check in rethook_find_ret_addr() prevents obtaining a return
address when the target task is marked as running. However, this condition
is both insufficient for correctness and unnecessary for its intended
purpose.
The check is inherently racy: a task can begin running on another CPU
immediately after task_is_running() returns false, potentially leading to
concurrent modification of rethook data structures while the iteration is
in progress.
Rather than trying to fix this unreliable check deep in the unwinding
path, simply remove it. The iteration is already safe from crashes because
unwind_next_frame() holds RCU and rethook_node structures are RCU-freed;
even if the iteration goes off the rails and returns invalid information,
it will not crash. Callers that require consistency must provide a safe
context themselves.
Fixes: 54ecbe6f1ed5 ("rethook: Add a generic return hook")
Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Signed-off-by: Tengda Wu <wutengda@huaweicloud.com>
---
v3: Improve commit message: clarify safety semantics and document that RCU guarantees no crash.
v2: https://lore.kernel.org/all/20260609005728.458962-1-wutengda@huaweicloud.com/
v1: https://lore.kernel.org/all/20260525132253.1889726-1-wutengda@huaweicloud.com/
kernel/trace/rethook.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/kernel/trace/rethook.c b/kernel/trace/rethook.c
index 5a8bdf88999a..f70f11bc6c91 100644
--- a/kernel/trace/rethook.c
+++ b/kernel/trace/rethook.c
@@ -250,9 +250,6 @@ unsigned long rethook_find_ret_addr(struct task_struct *tsk, unsigned long frame
if (WARN_ON_ONCE(!cur))
return 0;
- if (tsk != current && task_is_running(tsk))
- return 0;
-
do {
ret = __rethook_find_ret_addr(tsk, cur);
if (!ret)
--
2.34.1
^ permalink raw reply related
* Re: [PATCH] mm/lruvec: trace LRU add drains and drain-all queuing
From: Barry Song @ 2026-06-09 7:44 UTC (permalink / raw)
To: JP Kobryn
Cc: linux-mm, willy, shakeel.butt, usama.arif, akpm, vbabka, mhocko,
rostedt, mhiramat, mathieu.desnoyers, kasong, qi.zheng,
axelrasmussen, yuanchu, weixugc, chrisl, shikemeng, nphamcs,
baoquan.he, youngjun.park, linux-kernel, linux-trace-kernel
In-Reply-To: <20260609041156.31127-1-jp.kobryn@linux.dev>
On Tue, Jun 9, 2026 at 12:12 PM JP Kobryn <jp.kobryn@linux.dev> wrote:
>
> LRU add batches can be drained before they reach capacity. This can be a
> source of LRU lock contention, but it is not currently possible to
> attribute these drains to callers with existing tracepoints.
>
> Add mm_lru_add_drain to report the CPU and lru_add batch count when an
> lru_add batch is drained. This allows tracing to distinguish full drains
> from partial drains and attribute them to the calling stack.
>
> Add mm_lru_drain_all_queue to report when lru_add_drain_all() queues
> per-CPU drain work. This captures the requester stack and target CPU for
> remote drain work. The event is named as a drain-all queue event because
> the queued work can be needed for batches other than lru_add.
>
> Signed-off-by: JP Kobryn <jp.kobryn@linux.dev>
> ---
> include/trace/events/pagemap.h | 40 ++++++++++++++++++++++++++++++++++
> mm/swap.c | 6 ++++-
> 2 files changed, 45 insertions(+), 1 deletion(-)
>
> diff --git a/include/trace/events/pagemap.h b/include/trace/events/pagemap.h
> index 171524d3526d..ea8fc46bedb0 100644
> --- a/include/trace/events/pagemap.h
> +++ b/include/trace/events/pagemap.h
> @@ -77,6 +77,46 @@ TRACE_EVENT(mm_lru_activate,
> TP_printk("folio=%p pfn=0x%lx", __entry->folio, __entry->pfn)
> );
>
> +TRACE_EVENT(mm_lru_add_drain,
> +
> + TP_PROTO(int cpu, unsigned int nr),
> +
> + TP_ARGS(cpu, nr),
> +
> + TP_STRUCT__entry(
> + __field(int, cpu )
> + __field(unsigned int, nr )
> + ),
> +
> + TP_fast_assign(
> + __entry->cpu = cpu;
> + __entry->nr = nr;
> + ),
> +
> + TP_printk("cpu=%d nr=%u", __entry->cpu, __entry->nr)
> +);
> +
> +TRACE_EVENT(mm_lru_drain_all_queue,
> +
> + TP_PROTO(int target_cpu, bool force_all_cpus),
> +
> + TP_ARGS(target_cpu, force_all_cpus),
> +
> + TP_STRUCT__entry(
> + __field(int, target_cpu )
> + __field(bool, force_all_cpus )
> + ),
> +
> + TP_fast_assign(
> + __entry->target_cpu = target_cpu;
> + __entry->force_all_cpus = force_all_cpus;
> + ),
> +
> + TP_printk("target_cpu=%d force_all_cpus=%s",
> + __entry->target_cpu,
> + __entry->force_all_cpus ? "true" : "false")
> +);
> +
> #endif /* _TRACE_PAGEMAP_H */
>
> /* This part must be outside protection */
> diff --git a/mm/swap.c b/mm/swap.c
> index 588f50d8f1a8..c385b93582eb 100644
> --- a/mm/swap.c
> +++ b/mm/swap.c
> @@ -694,9 +694,12 @@ void lru_add_drain_cpu(int cpu)
> {
> struct cpu_fbatches *fbatches = &per_cpu(cpu_fbatches, cpu);
> struct folio_batch *fbatch = &fbatches->lru_add;
> + unsigned int nr_folios_add = folio_batch_count(fbatch);
>
> - if (folio_batch_count(fbatch))
> + if (nr_folios_add) {
> folio_batch_move_lru(fbatch, lru_add);
> + trace_mm_lru_add_drain(cpu, nr_folios_add);
> + }
>
> fbatch = &fbatches->lru_move_tail;
> /* Disabling interrupts below acts as a compiler barrier. */
> @@ -928,6 +931,7 @@ static inline void __lru_add_drain_all(bool force_all_cpus)
> if (cpu_needs_drain(cpu)) {
> INIT_WORK(work, lru_add_drain_per_cpu);
> queue_work_on(cpu, mm_percpu_wq, work);
> + trace_mm_lru_drain_all_queue(cpu, force_all_cpus);
Do you need tracing on each CPU individually, or is tracing the
entire __lru_add_drain_all() invocation sufficient?
Do you also need this_gen and lru_drain_gen to be traced?
By the way, I'm not sure drain_all_queue is the best name here.
Why not simply use add_drain_all()? It would match the existing
function name better.
Best Regards
Barry
^ permalink raw reply
* Re: [PATCH v3 1/8] scripts/sorttable: Handle RISC-V patchable ftrace entries
From: Martin Kaiser @ 2026-06-09 7:27 UTC (permalink / raw)
To: Wang Han
Cc: Paul Walmsley, Palmer Dabbelt, Albert Ou, Steven Rostedt,
Alexandre Ghiti, Masami Hiramatsu, Mark Rutland, Catalin Marinas,
Chen Pei, Andy Chiu, Björn Töpel, Deepak Gupta,
Puranjay Mohan, Conor Dooley, Josh Poimboeuf, Jiri Kosina,
Miroslav Benes, Petr Mladek, Joe Lawrence, Shuah Khan,
Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Namhyung Kim, oliver.yang, xueshuai, zhuo.song, jkchen,
linux-riscv, linux-kernel, linux-trace-kernel, live-patching,
linux-kselftest, linux-perf-users
In-Reply-To: <20260609063002.3943001-1-wanghan@linux.alibaba.com>
Thus wrote Wang Han (wanghan@linux.alibaba.com):
> RISC-V uses -fpatchable-function-entry=8,4 when the compressed ISA is
> enabled and -fpatchable-function-entry=4,2 otherwise. In both cases, the
> patchable NOP area starts 8 bytes before the function symbol address.
> The __mcount_loc entries therefore point at the patchable NOP area
> associated with a function, while nm reports the function symbol at the
> entry address used for the function range check.
> After RISC-V selected HAVE_BUILDTIME_MCOUNT_SORT, sorttable started
> applying that range check at build time. Without allowing entries just
> before the reported function address, the mcount sorter treats valid
> RISC-V ftrace callsites as invalid weak-function entries and writes
> them back as zero. The resulting kernel boots with no ftrace entries,
> breaking dynamic ftrace and users such as livepatch.
> The failure is silent during the final link because zeroing weak-function
> entries is an expected sorttable operation. At boot, those zero entries
> are skipped by ftrace_process_locs(), so the only obvious symptom is that
> the vmlinux ftrace table has lost valid callsites and ftrace users cannot
> attach to them.
> CONFIG_FTRACE_SORT_STARTUP_TEST also reports the table as sorted in this
> state: it only checks that the __mcount_loc entries are in ascending
> order, which a fully zeroed table trivially satisfies. The original
> commit relied on this check and did not see the regression.
> On an affected RISC-V QEMU boot with both CONFIG_FTRACE_SORT_STARTUP_TEST
> and CONFIG_FTRACE_STARTUP_TEST enabled, the sort check still passes
> while ftrace reports zero usable entries and the early selftests fail:
> [ 0.000000] ftrace section at ffffffff8101da98 sorted properly
> [ 0.000000] ftrace: allocating 0 entries in 128 pages
> [ 0.054999] Testing tracer function: .. no entries found ..FAILED!
> [ 0.172407] tracer: function failed selftest, disabling
> [ 0.178186] Failed to init function_graph tracer, init returned -19
> Handle RISC-V like arm64 for the function-range check and allow
> patchable entries up to 8 bytes before the function address.
> With this fix, a RISC-V QEMU smoke boot with ftrace startup tests shows
> the vmlinux ftrace table is populated and dynamic ftrace still works:
> [ 0.000000] ftrace: allocating 46749 entries in 184 pages
> [ 0.051115] Testing tracer function: PASSED
> [ 1.283782] Testing dynamic ftrace: PASSED
> [ 6.275456] Testing tracer function_graph: PASSED
> Fixes: 0ca1724b56af ("riscv: ftrace: select HAVE_BUILDTIME_MCOUNT_SORT")
> Suggested-by: Steven Rostedt (Google) <rostedt@goodmis.org>
> Reviewed-by: Steven Rostedt <rostedt@goodmis.org>
> Reviewed-by: Shuai Xue <xueshuai@linux.alibaba.com>
> Reviewed-by: Chen Pei <cp0613@linux.alibaba.com>
> Link: https://lore.kernel.org/all/20260527113028.4b21a5de@fedora/
> Signed-off-by: Wang Han <wanghan@linux.alibaba.com>
> ---
> scripts/sorttable.c | 11 ++++++++---
> 1 file changed, 8 insertions(+), 3 deletions(-)
> diff --git a/scripts/sorttable.c b/scripts/sorttable.c
> index e8ed11c680c6..d8dc2a1b7c31 100644
> --- a/scripts/sorttable.c
> +++ b/scripts/sorttable.c
> @@ -891,17 +891,22 @@ static int do_file(char const *const fname, void *addr)
> table_sort_t custom_sort = NULL;
> switch (elf_map_machine(ehdr)) {
> - case EM_AARCH64:
> #ifdef MCOUNT_SORT_ENABLED
> + case EM_AARCH64:
> + /* arm64 also needs RELA-based weak-function fixups. */
> sort_reloc = true;
> rela_type = 0x403;
> - /* arm64 uses patchable function entry placing before function */
> + /* fallthrough */
> + case EM_RISCV:
> + /* arm64 and RISC-V place patchable entries before the function. */
> before_func = 8;
> +#else
> + case EM_AARCH64:
> + case EM_RISCV:
> #endif
> /* fallthrough */
> case EM_386:
> case EM_LOONGARCH:
> - case EM_RISCV:
> case EM_S390:
> case EM_X86_64:
> custom_sort = sort_relative_table_with_data;
> --
> 2.43.0
I ran into this problem and came up with pretty much the same fix.
Reviewed-by: Martin Kaiser <martin@kaiser.cx>
^ permalink raw reply
* Re: [PATCH v2] rethook: Remove the running task check in rethook_find_ret_addr()
From: Peter Zijlstra @ 2026-06-09 7:14 UTC (permalink / raw)
To: Tengda Wu
Cc: Masami Hiramatsu, Steven Rostedt, Mathieu Desnoyers,
Alexei Starovoitov, linux-trace-kernel, linux-kernel
In-Reply-To: <20260609005728.458962-1-wutengda@huaweicloud.com>
On Tue, Jun 09, 2026 at 08:57:28AM +0800, Tengda Wu wrote:
> The current check in rethook_find_ret_addr() prevents obtaining a return
> address when the target task is marked as running. However, this condition
> is both insufficient for safety and unnecessary for its intended purpose.
Depends on what safety means. If safety means not crashing, it is
entirely superfluous. If safety means correctness, then yes, it is
insufficient.
> The check is inherently racy: a task can begin running on another CPU
> immediately after task_is_running() returns false, potentially leading to
> concurrent modification of rethook data structures while the iteration is
> in progress.
>
> Rather than attempting to fix this unreliable check deep in the unwinding
> path, remove it entirely. Callers that require consistency are expected
> to provide a safe context.
Perhaps also note that unwind_next() will hold RCU and the rethook_node
things are RCU freed, so while the iteration might go off the rails and
return invalid information, it will not crash.
> Fixes: 54ecbe6f1ed5 ("rethook: Add a generic return hook")
> Signed-off-by: Tengda Wu <wutengda@huaweicloud.com>
With clarifications:
Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> ---
> v2: Remove the running task check.
> v1: https://lore.kernel.org/all/20260525132253.1889726-1-wutengda@huaweicloud.com/
>
> kernel/trace/rethook.c | 3 ---
> 1 file changed, 3 deletions(-)
>
> diff --git a/kernel/trace/rethook.c b/kernel/trace/rethook.c
> index 5a8bdf88999a..f70f11bc6c91 100644
> --- a/kernel/trace/rethook.c
> +++ b/kernel/trace/rethook.c
> @@ -250,9 +250,6 @@ unsigned long rethook_find_ret_addr(struct task_struct *tsk, unsigned long frame
> if (WARN_ON_ONCE(!cur))
> return 0;
>
> - if (tsk != current && task_is_running(tsk))
> - return 0;
> -
> do {
> ret = __rethook_find_ret_addr(tsk, cur);
> if (!ret)
> --
> 2.34.1
>
^ permalink raw reply
* Re: [PATCH v8 2/6] mm/memory-failure: surface unhandlable kernel pages as -ENOTRECOVERABLE
From: David Hildenbrand (Arm) @ 2026-06-09 7:09 UTC (permalink / raw)
To: Miaohe Lin, Breno Leitao
Cc: linux-mm, linux-kernel, linux-doc, linux-kselftest,
linux-trace-kernel, kernel-team, Lance Yang, Andrew Morton,
Lorenzo Stoakes, Vlastimil Babka, Mike Rapoport,
Suren Baghdasaryan, Michal Hocko, Shuah Khan, Naoya Horiguchi,
Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
Jonathan Corbet, Shuah Khan, Liam R. Howlett
In-Reply-To: <4953bcee-5a0f-2bc5-7295-63e5e7513e8b@huawei.com>
On 6/9/26 04:39, Miaohe Lin wrote:
> On 2026/6/8 22:15, Breno Leitao wrote:
>> On Fri, Jun 05, 2026 at 11:42:53AM +0200, David Hildenbrand (Arm) wrote:
>>>
>>> I mean, any such races can currently already happen one way or the other?
>>>
>>> Really, the only way to not get races is to tryget the (compound)page,
>>> revalidate that the page is still part of the compound page.
>>>
>>> I'm not sure if that's really a good idea.
>>>
>>> But my memory is a bit vague in which scenarios we already hold a page reference
>>> here to prevent any concurrent freeing?
>>
>> No, we don't hold one here in the case that matters.
>>
>> HWPoisonKernelOwned() runs at the very top of get_any_page(), before
>> try_again: and before __get_hwpoison_page(). The first refcount taken in
>> the whole path is the folio_try_get() inside __get_hwpoison_page(), which
>> runs *after* the short-circuit.
>>
>> So get_any_page() itself never holds a reference at the check -- the only way
>> one exists is if the caller passed MF_COUNT_INCREASED (count_increased ==
>> true).
>>
>> So on the MCE/GHES path -- the one this panic option exists for -- no
>> reference is held when HWPoisonKernelOwned() does its compound_head() +
>> PageSlab()/PageTable()/PageLargeKmalloc() checks.
>>
>> Given that, I'd rather keep it racy and take no refcount than add a
>> tryget + revalidate purely for this check. As I've said earleir, an operator
>
> Would it be acceptable to add a simple recheck? Something like below:
>
> retry:
> head = compound_head(page);
> PageSlab()/PageTable()/PageLargeKmalloc() checks
> if (head != compound_head(page))
> goto retry
Sure. I guess it could still be racy in some weird scenarios where we
free+allocate+free in-between.
--
Cheers,
David
^ permalink raw reply
* Re: [PATCH] rethook: Use tsk->on_cpu to check task execution state
From: Peter Zijlstra @ 2026-06-09 7:05 UTC (permalink / raw)
To: Masami Hiramatsu
Cc: bpf, Tengda Wu, Steven Rostedt, Mathieu Desnoyers,
Alexei Starovoitov, linux-trace-kernel, linux-kernel,
Josh Poimboeuf, jikos, mbenes, pmladek
In-Reply-To: <20260609134153.d06aef367a366e5d976cba62@kernel.org>
On Tue, Jun 09, 2026 at 01:41:53PM +0900, Masami Hiramatsu wrote:
> > This, you cannot take locks in unwinding. The only thing you can do is
> > try to do the best you can without crashing.
> >
> > Typically unwind only happens on self -- this is natural, a task crashes
> > and unwinds itself, or a task does something (takes a lock, hits a
> > tracepoint, etc) and takes a snapshot of its own stack, and this is
> > safe.
> >
> > Things like live-patch use task_call_func(), which ensures the callback
> > function is done while holding sufficient locks for the task to not
> > change state.
>
> Hmm, is there any way to ensure the function is called from task_call_func()?
Nope. And you shouldn't want to.
> (Maybe checking p->pi_lock, but this is not sure the lock owner is this
> context?) If not, I need to make this available only for current task
> (anyway it just return kretprobe trampoline address, no critical issue)
> or, introduce a spinlock.
>
> Or, eventually it may be better to replace kretprobe/rethook with
> fprobe return handler.
I'm not sure where you're wanting to go. AFAICT the current rethook
stuff won't crash when called on an active task, it might just not give
the right results -- but that is true for the entire unwind, so who
cares?
Those who call unwind on active tasks get to keep the pieces, not our
problem etc.
^ permalink raw reply
* [PATCH v3 7/8] riscv: Kconfig: enable HAVE_RELIABLE_STACKTRACE and HAVE_LIVEPATCH
From: Wang Han @ 2026-06-09 6:29 UTC (permalink / raw)
To: Paul Walmsley, Palmer Dabbelt, Albert Ou
Cc: Steven Rostedt, Alexandre Ghiti, Masami Hiramatsu, Mark Rutland,
Catalin Marinas, Chen Pei, Andy Chiu, Björn Töpel,
Deepak Gupta, Puranjay Mohan, Conor Dooley, Josh Poimboeuf,
Jiri Kosina, Miroslav Benes, Petr Mladek, Joe Lawrence,
Shuah Khan, Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Namhyung Kim, oliver.yang, xueshuai, zhuo.song, jkchen,
linux-riscv, linux-kernel, linux-trace-kernel, live-patching,
linux-kselftest, linux-perf-users
In-Reply-To: <cover.194d76e3a15b.v3.riscv-livepatch.wanghan@linux.alibaba.com>
Now that the metadata frame records, the kunwind state machine and
arch_stack_walk_reliable() are all in place, advertise the capability
to the rest of the kernel:
* select HAVE_RELIABLE_STACKTRACE under FRAME_POINTER && 64BIT, so
only the configurations with the tested metadata records and
FP-based reliable walker enable it.
* select HAVE_LIVEPATCH under the same condition and source
kernel/livepatch/Kconfig so the livepatch menu is reachable from
the RISC-V configuration.
The 64BIT dependency is conservative scoping rather than a hard
technical requirement: the metadata frame record, kunwind state machine
and arch_stack_walk_reliable() also build on RV32, and the IRQ-stack
frame-record adjustment fixes a latent RV32 issue. However, the syscall
livepatch selftest and module relocation path have only been exercised
on RV64 QEMU virt so far. The 64BIT gate can be relaxed in a follow-up
once RV32 has equivalent coverage.
This is split out from the unwinder change so the policy decision and
the implementation can be reviewed and reverted independently.
Reviewed-by: Shuai Xue <xueshuai@linux.alibaba.com>
Signed-off-by: Wang Han <wanghan@linux.alibaba.com>
---
arch/riscv/Kconfig | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index 674044754378..2921680d2132 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -185,6 +185,7 @@ config RISCV
select HAVE_KRETPROBES
# https://github.com/ClangBuiltLinux/linux/issues/1881
select HAVE_LD_DEAD_CODE_DATA_ELIMINATION if !LD_IS_LLD
+ select HAVE_LIVEPATCH if FRAME_POINTER && 64BIT
select HAVE_MOVE_PMD
select HAVE_MOVE_PUD
select HAVE_PAGE_SIZE_4KB
@@ -195,6 +196,7 @@ config RISCV
select HAVE_POSIX_CPU_TIMERS_TASK_WORK
select HAVE_PREEMPT_DYNAMIC_KEY
select HAVE_REGS_AND_STACK_ACCESS_API
+ select HAVE_RELIABLE_STACKTRACE if FRAME_POINTER && 64BIT
select HAVE_RETHOOK
select HAVE_RSEQ
select HAVE_RUST if RUSTC_SUPPORTS_RISCV && CC_IS_CLANG
@@ -1394,3 +1396,5 @@ endmenu # "CPU Power Management"
source "arch/riscv/kvm/Kconfig"
source "drivers/acpi/Kconfig"
+
+source "kernel/livepatch/Kconfig"
--
2.43.0
^ permalink raw reply related
* [PATCH v3 6/8] riscv: stacktrace: switch to frame-pointer based unwinder
From: Wang Han @ 2026-06-09 6:29 UTC (permalink / raw)
To: Paul Walmsley, Palmer Dabbelt, Albert Ou
Cc: Steven Rostedt, Alexandre Ghiti, Masami Hiramatsu, Mark Rutland,
Catalin Marinas, Chen Pei, Andy Chiu, Björn Töpel,
Deepak Gupta, Puranjay Mohan, Conor Dooley, Josh Poimboeuf,
Jiri Kosina, Miroslav Benes, Petr Mladek, Joe Lawrence,
Shuah Khan, Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Namhyung Kim, oliver.yang, xueshuai, zhuo.song, jkchen,
linux-riscv, linux-kernel, linux-trace-kernel, live-patching,
linux-kselftest, linux-perf-users
In-Reply-To: <cover.194d76e3a15b.v3.riscv-livepatch.wanghan@linux.alibaba.com>
Replace the open-coded frame-pointer walker in arch_stack_walk() with a
robust kunwind state machine, modelled on arch/arm64/kernel/stacktrace.c
and retargeted to the RISC-V {fp, ra} frame record convention. The new
walker tracks stack bounds, consumes frame records monotonically,
understands the metadata pt_regs records added in the previous frame
record metadata patch, and recovers return addresses replaced by
function graph tracing and kretprobes.
This commit introduces arch_stack_walk_reliable() but does not yet
select HAVE_RELIABLE_STACKTRACE; that is done in a follow-up Kconfig
patch so this commit can be reviewed and bisected as a pure unwinder
replacement. Until that Kconfig change lands, livepatch is not yet
enabled and arch_stack_walk_reliable() has no in-tree caller.
Three related callers are updated to keep the same frame-record
assumptions everywhere:
* Function graph tracing: the old RISC-V unwinder matched function
graph return-stack entries by the saved return-address slot. That
was consistent with the static mcount path, but not with the dynamic
ftrace path where the parent slot is ftrace_regs::ra. Use the
architectural frame pointer as the function graph return-address
cookie, matching the kunwind walker.
* Perf callchains: route kernel callchain collection through
arch_stack_walk() so perf sees the same frame-pointer unwind
behaviour as dump_stack() and the upcoming livepatch path.
* dump_backtrace() / __get_wchan() / show_stack(): these now go
through arch_stack_walk(); the explicit "Call Trace:" header is
moved into dump_backtrace() to preserve the original output.
The non-frame-pointer fallback walker is kept untouched for
!CONFIG_FRAME_POINTER builds.
Signed-off-by: Wang Han <wanghan@linux.alibaba.com>
---
arch/riscv/kernel/ftrace.c | 6 +-
arch/riscv/kernel/perf_callchain.c | 2 +-
arch/riscv/kernel/stacktrace.c | 559 ++++++++++++++++++++++++-----
3 files changed, 471 insertions(+), 96 deletions(-)
diff --git a/arch/riscv/kernel/ftrace.c b/arch/riscv/kernel/ftrace.c
index b430edfb83f4..5d55199a9230 100644
--- a/arch/riscv/kernel/ftrace.c
+++ b/arch/riscv/kernel/ftrace.c
@@ -242,7 +242,8 @@ void prepare_ftrace_return(unsigned long *parent, unsigned long self_addr,
*/
old = *parent;
- if (!function_graph_enter(old, self_addr, frame_pointer, parent))
+ if (!function_graph_enter(old, self_addr, frame_pointer,
+ (void *)frame_pointer))
*parent = return_hooker;
}
@@ -264,7 +265,8 @@ void ftrace_graph_func(unsigned long ip, unsigned long parent_ip,
*/
old = *parent;
- if (!function_graph_enter_regs(old, ip, frame_pointer, parent, fregs))
+ if (!function_graph_enter_regs(old, ip, frame_pointer,
+ (void *)frame_pointer, fregs))
*parent = return_hooker;
}
#endif /* CONFIG_DYNAMIC_FTRACE */
diff --git a/arch/riscv/kernel/perf_callchain.c b/arch/riscv/kernel/perf_callchain.c
index b465bc9eb870..436af96ea59c 100644
--- a/arch/riscv/kernel/perf_callchain.c
+++ b/arch/riscv/kernel/perf_callchain.c
@@ -44,5 +44,5 @@ void perf_callchain_kernel(struct perf_callchain_entry_ctx *entry,
return;
}
- walk_stackframe(NULL, regs, fill_callchain, entry);
+ arch_stack_walk(fill_callchain, entry, NULL, regs);
}
diff --git a/arch/riscv/kernel/stacktrace.c b/arch/riscv/kernel/stacktrace.c
index 2692d3a06afa..8fcf23b046d5 100644
--- a/arch/riscv/kernel/stacktrace.c
+++ b/arch/riscv/kernel/stacktrace.c
@@ -11,98 +11,16 @@
#include <linux/sched/task_stack.h>
#include <linux/stacktrace.h>
#include <linux/ftrace.h>
+#include <linux/kprobes.h>
+#include <linux/llist.h>
#include <asm/stacktrace.h>
-#ifdef CONFIG_FRAME_POINTER
-
/*
- * This disables KASAN checking when reading a value from another task's stack,
- * since the other task could be running on another CPU and could have poisoned
- * the stack in the meantime.
+ * Non-frame-pointer fallback unwinder.
+ * Only compiled when CONFIG_FRAME_POINTER is not enabled.
*/
-#define READ_ONCE_TASK_STACK(task, x) \
-({ \
- unsigned long val; \
- unsigned long addr = x; \
- if ((task) == current) \
- val = READ_ONCE(addr); \
- else \
- val = READ_ONCE_NOCHECK(addr); \
- val; \
-})
-
-extern asmlinkage void handle_exception(void);
-extern unsigned long ret_from_exception_end;
-
-static inline int fp_is_valid(unsigned long fp, unsigned long sp)
-{
- unsigned long low, high;
-
- low = sp + sizeof(struct stackframe);
- high = ALIGN(sp, THREAD_SIZE);
-
- return !(fp < low || fp > high || fp & 0x07);
-}
-
-void notrace walk_stackframe(struct task_struct *task, struct pt_regs *regs,
- bool (*fn)(void *, unsigned long), void *arg)
-{
- unsigned long fp, sp, pc;
- int graph_idx = 0;
- int level = 0;
-
- if (regs) {
- fp = frame_pointer(regs);
- sp = user_stack_pointer(regs);
- pc = instruction_pointer(regs);
- } else if (task == NULL || task == current) {
- fp = (unsigned long)__builtin_frame_address(0);
- sp = current_stack_pointer;
- pc = (unsigned long)walk_stackframe;
- level = -1;
- } else {
- /* task blocked in __switch_to */
- fp = task->thread.s[0];
- sp = task->thread.sp;
- pc = task->thread.ra;
- }
-
- for (;;) {
- struct stackframe *frame;
-
- if (unlikely(!__kernel_text_address(pc) || (level++ >= 0 && !fn(arg, pc))))
- break;
-
- if (unlikely(!fp_is_valid(fp, sp)))
- break;
-
- /* Unwind stack frame */
- frame = (struct stackframe *)fp - 1;
- sp = fp;
- if (regs && (regs->epc == pc) && fp_is_valid(frame->ra, sp)) {
- /* We hit function where ra is not saved on the stack */
- fp = frame->ra;
- pc = regs->ra;
- } else {
- fp = READ_ONCE_TASK_STACK(task, frame->fp);
- pc = READ_ONCE_TASK_STACK(task, frame->ra);
- pc = ftrace_graph_ret_addr(task, &graph_idx, pc,
- &frame->ra);
- if (pc >= (unsigned long)handle_exception &&
- pc < (unsigned long)&ret_from_exception_end) {
- if (unlikely(!fn(arg, pc)))
- break;
-
- pc = ((struct pt_regs *)sp)->epc;
- fp = ((struct pt_regs *)sp)->s0;
- }
- }
-
- }
-}
-
-#else /* !CONFIG_FRAME_POINTER */
+#ifndef CONFIG_FRAME_POINTER
void notrace walk_stackframe(struct task_struct *task,
struct pt_regs *regs, bool (*fn)(void *, unsigned long), void *arg)
@@ -133,7 +51,12 @@ void notrace walk_stackframe(struct task_struct *task,
}
}
-#endif /* CONFIG_FRAME_POINTER */
+#endif /* !CONFIG_FRAME_POINTER */
+
+/*
+ * Common trace helpers.
+ * These are used by both the FP (kunwind) and non-FP (walk_stackframe) paths.
+ */
static bool print_trace_address(void *arg, unsigned long pc)
{
@@ -146,12 +69,12 @@ static bool print_trace_address(void *arg, unsigned long pc)
noinline void dump_backtrace(struct pt_regs *regs, struct task_struct *task,
const char *loglvl)
{
- walk_stackframe(task, regs, print_trace_address, (void *)loglvl);
+ printk("%sCall Trace:\n", loglvl);
+ arch_stack_walk(print_trace_address, (void *)loglvl, task, regs);
}
void show_stack(struct task_struct *task, unsigned long *sp, const char *loglvl)
{
- pr_cont("%sCall Trace:\n", loglvl);
dump_backtrace(NULL, task, loglvl);
}
@@ -171,17 +94,467 @@ unsigned long __get_wchan(struct task_struct *task)
if (!try_get_task_stack(task))
return 0;
- walk_stackframe(task, NULL, save_wchan, &pc);
+ arch_stack_walk(save_wchan, &pc, task, NULL);
put_task_stack(task);
return pc;
}
-noinline noinstr void arch_stack_walk(stack_trace_consume_fn consume_entry, void *cookie,
- struct task_struct *task, struct pt_regs *regs)
+/*
+ * Frame-pointer-based kernel unwind infrastructure.
+ * Only compiled when CONFIG_FRAME_POINTER is enabled.
+ *
+ * See: arch/arm64/kernel/stacktrace.c for the reference implementation.
+ */
+#ifdef CONFIG_FRAME_POINTER
+
+/*
+ * Per-cpu stacks are only accessible when unwinding the current task in a
+ * non-preemptible context.
+ */
+#define STACKINFO_CPU(task, name) \
+ ({ \
+ (((task) == current) && !preemptible()) \
+ ? stackinfo_get_##name() \
+ : stackinfo_get_unknown(); \
+ })
+
+enum kunwind_source {
+ KUNWIND_SOURCE_UNKNOWN,
+ KUNWIND_SOURCE_FRAME,
+ KUNWIND_SOURCE_CALLER,
+ KUNWIND_SOURCE_TASK,
+ KUNWIND_SOURCE_REGS_PC,
+};
+
+union unwind_flags {
+ unsigned long all;
+ struct {
+ unsigned long fgraph : 1,
+ kretprobe : 1;
+ };
+};
+
+/*
+ * Kernel unwind state
+ *
+ * @common: Common unwind state.
+ * @task: The task being unwound.
+ * @graph_idx: Used by ftrace_graph_ret_addr() for optimized stack unwinding.
+ * @kr_cur: When KRETPROBES is selected, holds the kretprobe instance
+ * associated with the most recently encountered replacement ra
+ * value.
+ */
+struct kunwind_state {
+ struct unwind_state common;
+ struct task_struct *task;
+ int graph_idx;
+#ifdef CONFIG_KRETPROBES
+ struct llist_node *kr_cur;
+#endif
+ enum kunwind_source source;
+ union unwind_flags flags;
+ struct pt_regs *regs;
+};
+
+static __always_inline void
+kunwind_init(struct kunwind_state *state,
+ struct task_struct *task)
+{
+ unwind_init_common(&state->common);
+ state->task = task;
+ state->source = KUNWIND_SOURCE_UNKNOWN;
+ state->flags.all = 0;
+ state->regs = NULL;
+}
+
+/*
+ * Start an unwind from a pt_regs.
+ *
+ * The unwind will begin at the PC within the regs.
+ *
+ * The regs must be on a stack currently owned by the calling task.
+ */
+static __always_inline void
+kunwind_init_from_regs(struct kunwind_state *state,
+ struct pt_regs *regs)
+{
+ kunwind_init(state, current);
+
+ state->regs = regs;
+ state->common.fp = frame_pointer(regs);
+ state->common.pc = instruction_pointer(regs);
+ state->source = KUNWIND_SOURCE_REGS_PC;
+}
+
+/*
+ * Start an unwind from a caller.
+ *
+ * The unwind will begin at the caller of whichever function this is inlined
+ * into.
+ *
+ * The function which invokes this must be noinline.
+ */
+static __always_inline void
+kunwind_init_from_caller(struct kunwind_state *state)
+{
+ unsigned long fp = (unsigned long)__builtin_frame_address(0);
+ struct frame_record *record = (struct frame_record *)fp - 1;
+
+ kunwind_init(state, current);
+
+ state->common.fp = READ_ONCE(record->fp);
+ state->common.pc = READ_ONCE(record->ra);
+ state->source = KUNWIND_SOURCE_CALLER;
+}
+
+/*
+ * Start an unwind from a blocked task.
+ *
+ * The unwind will begin at the blocked task's saved PC (i.e. the caller of
+ * __switch_to).
+ *
+ * The caller should ensure the task is blocked in __switch_to for the
+ * duration of the unwind, or the unwind will be bogus. It is never valid to
+ * call this for the current task.
+ */
+static __always_inline void
+kunwind_init_from_task(struct kunwind_state *state,
+ struct task_struct *task)
+{
+ kunwind_init(state, task);
+
+ state->common.fp = task->thread.s[0];
+ state->common.pc = task->thread.ra;
+ state->source = KUNWIND_SOURCE_TASK;
+}
+
+static __always_inline int
+kunwind_recover_return_address(struct kunwind_state *state)
+{
+#ifdef CONFIG_FUNCTION_GRAPH_TRACER
+ if (state->task->ret_stack &&
+ state->common.pc == (unsigned long)return_to_handler) {
+ unsigned long orig_pc;
+
+ orig_pc = ftrace_graph_ret_addr(state->task, &state->graph_idx,
+ state->common.pc,
+ (void *)state->common.fp);
+ if (state->common.pc == orig_pc) {
+ WARN_ON_ONCE(state->task == current);
+ return -EINVAL;
+ }
+ state->common.pc = orig_pc;
+ state->flags.fgraph = 1;
+ }
+#endif /* CONFIG_FUNCTION_GRAPH_TRACER */
+
+#ifdef CONFIG_KRETPROBES
+ if (is_kretprobe_trampoline(state->common.pc)) {
+ unsigned long orig_pc;
+
+ orig_pc = kretprobe_find_ret_addr(state->task,
+ (void *)state->common.fp,
+ &state->kr_cur);
+ if (!orig_pc)
+ return -EINVAL;
+ state->common.pc = orig_pc;
+ state->flags.kretprobe = 1;
+ }
+#endif /* CONFIG_KRETPROBES */
+
+ return 0;
+}
+
+/*
+ * When we reach an exception boundary marked by a metadata frame record,
+ * extract pt_regs from the stack and continue unwinding from the saved
+ * context (epc and s0/fp).
+ *
+ * On RISC-V, fp points above the metadata record, so the record's
+ * frame_record portion is at fp - sizeof(struct frame_record).
+ */
+static __always_inline int
+kunwind_next_regs_pc(struct kunwind_state *state)
+{
+ struct stack_info *info;
+ unsigned long fp = state->common.fp;
+ struct pt_regs *regs;
+
+ regs = container_of((unsigned long *)(fp - sizeof(struct frame_record)),
+ struct pt_regs, stackframe.record.fp);
+
+ info = unwind_find_stack(&state->common, (unsigned long)regs,
+ sizeof(*regs));
+ if (!info)
+ return -EINVAL;
+
+ unwind_consume_stack(&state->common, info, (unsigned long)regs,
+ sizeof(*regs));
+
+ state->regs = regs;
+ state->common.pc = regs->epc;
+ state->common.fp = frame_pointer(regs);
+ state->source = KUNWIND_SOURCE_REGS_PC;
+ return 0;
+}
+
+/*
+ * Handle a metadata frame record embedded in pt_regs.
+ *
+ * On RISC-V, fp points above the record (fp = metadata + 16), so the
+ * frame_record_meta starts at fp - sizeof(struct frame_record).
+ *
+ * FRAME_META_TYPE_FINAL: This is the outermost exception entry
+ * (user -> kernel). Unwinding terminates successfully.
+ * FRAME_META_TYPE_PT_REGS: This is a nested exception entry
+ * (kernel -> kernel). Continue unwinding from the saved context.
+ */
+static __always_inline int
+kunwind_next_frame_record_meta(struct kunwind_state *state)
+{
+ struct task_struct *tsk = state->task;
+ unsigned long fp = state->common.fp;
+ unsigned long meta_base = fp - sizeof(struct frame_record);
+ struct frame_record_meta *meta;
+ struct stack_info *info;
+
+ info = unwind_find_stack(&state->common, meta_base, sizeof(*meta));
+ if (!info)
+ return -EINVAL;
+
+ meta = (struct frame_record_meta *)meta_base;
+ switch (READ_ONCE(meta->type)) {
+ case FRAME_META_TYPE_FINAL:
+ if (meta == &task_pt_regs(tsk)->stackframe)
+ return -ENOENT;
+ WARN_ON_ONCE(tsk == current);
+ return -EINVAL;
+ case FRAME_META_TYPE_PT_REGS:
+ return kunwind_next_regs_pc(state);
+ default:
+ WARN_ON_ONCE(tsk == current);
+ return -EINVAL;
+ }
+}
+
+/*
+ * Unwind from one frame record to the next.
+ *
+ * On RISC-V, the frame record sits at fp - sizeof(struct frame_record),
+ * immediately below the address pointed to by fp/s0. This applies to both
+ * normal frame records and metadata frame records (embedded in pt_regs).
+ *
+ * A metadata record is identified by both fp and ra being zero in the
+ * frame_record portion, with a type value following at fp + 16.
+ */
+static __always_inline int
+kunwind_next_frame_record(struct kunwind_state *state)
+{
+ unsigned long fp = state->common.fp;
+ struct frame_record *record;
+ struct stack_info *info;
+ unsigned long new_fp, new_pc;
+ unsigned long record_base;
+
+ if (fp & 0x7)
+ return -EINVAL;
+
+ record_base = fp - sizeof(*record);
+
+ info = unwind_find_stack(&state->common, record_base, sizeof(*record));
+ if (!info)
+ return -EINVAL;
+
+ record = (struct frame_record *)record_base;
+ new_fp = READ_ONCE(record->fp);
+ new_pc = READ_ONCE(record->ra);
+
+ if (!new_fp && !new_pc)
+ return kunwind_next_frame_record_meta(state);
+
+ unwind_consume_stack(&state->common, info, record_base,
+ sizeof(*record));
+
+ state->common.fp = new_fp;
+ state->common.pc = new_pc;
+ state->source = KUNWIND_SOURCE_FRAME;
+
+ return 0;
+}
+
+/*
+ * Unwind from one frame record (A) to the next frame record (B).
+ *
+ * We terminate early if the location of B indicates a malformed chain of frame
+ * records (e.g. a cycle), determined based on the location and fp value of A
+ * and the location (but not the fp value) of B.
+ */
+static __always_inline int
+kunwind_next(struct kunwind_state *state)
+{
+ int err;
+
+ state->flags.all = 0;
+
+ switch (state->source) {
+ case KUNWIND_SOURCE_FRAME:
+ case KUNWIND_SOURCE_CALLER:
+ case KUNWIND_SOURCE_TASK:
+ case KUNWIND_SOURCE_REGS_PC:
+ err = kunwind_next_frame_record(state);
+ break;
+ default:
+ err = -EINVAL;
+ }
+
+ if (err)
+ return err;
+
+ return kunwind_recover_return_address(state);
+}
+
+typedef bool (*kunwind_consume_fn)(const struct kunwind_state *state, void *cookie);
+
+static __always_inline int
+do_kunwind(struct kunwind_state *state, kunwind_consume_fn consume_state,
+ void *cookie)
+{
+ int ret;
+
+ ret = kunwind_recover_return_address(state);
+ if (ret)
+ return ret;
+
+ while (1) {
+ if (!consume_state(state, cookie))
+ return -EINVAL;
+ ret = kunwind_next(state);
+ if (ret == -ENOENT)
+ return 0;
+ if (ret < 0)
+ return ret;
+ }
+}
+
+static __always_inline int
+kunwind_stack_walk(kunwind_consume_fn consume_state,
+ void *cookie, struct task_struct *task,
+ struct pt_regs *regs)
+{
+ struct task_struct *tsk = task ?: current;
+ struct stack_info stacks[] = {
+ stackinfo_get_task(tsk),
+ STACKINFO_CPU(tsk, irq),
+#ifdef CONFIG_VMAP_STACK
+ STACKINFO_CPU(tsk, overflow),
+#endif
+ };
+ struct kunwind_state state = {
+ .common = {
+ .stacks = stacks,
+ .nr_stacks = ARRAY_SIZE(stacks),
+ },
+ };
+
+ if (regs) {
+ if (tsk != current)
+ return -EINVAL;
+ kunwind_init_from_regs(&state, regs);
+ } else if (tsk == current) {
+ kunwind_init_from_caller(&state);
+ } else {
+ kunwind_init_from_task(&state, tsk);
+ }
+
+ return do_kunwind(&state, consume_state, cookie);
+}
+
+struct kunwind_consume_entry_data {
+ stack_trace_consume_fn consume_entry;
+ void *cookie;
+};
+
+static __always_inline bool
+arch_kunwind_consume_entry(const struct kunwind_state *state, void *cookie)
+{
+ struct kunwind_consume_entry_data *data = cookie;
+
+ return data->consume_entry(data->cookie, state->common.pc);
+}
+
+static __always_inline bool
+arch_reliable_kunwind_consume_entry(const struct kunwind_state *state, void *cookie)
+{
+ /*
+ * At an exception boundary we can reliably consume the saved PC. We do
+ * not know whether ra was live when the exception was taken, and
+ * so we cannot perform the next unwind step reliably.
+ *
+ * All that matters is whether the *entire* unwind is reliable, so give
+ * up as soon as we hit an exception boundary.
+ */
+ if (state->source == KUNWIND_SOURCE_REGS_PC)
+ return false;
+
+ return arch_kunwind_consume_entry(state, cookie);
+}
+
+#endif /* CONFIG_FRAME_POINTER */
+
+/*
+ * arch_stack_walk - dual implementation.
+ *
+ * When CONFIG_FRAME_POINTER is enabled, uses the kunwind infrastructure for
+ * robust frame-pointer-based unwinding, consistent with arch_stack_walk_reliable.
+ *
+ * When CONFIG_FRAME_POINTER is disabled, falls back to the simple stack scan
+ * in walk_stackframe().
+ */
+#ifdef CONFIG_FRAME_POINTER
+
+noinline noinstr void arch_stack_walk(stack_trace_consume_fn consume_entry,
+ void *cookie, struct task_struct *task,
+ struct pt_regs *regs)
+{
+ struct kunwind_consume_entry_data data = {
+ .consume_entry = consume_entry,
+ .cookie = cookie,
+ };
+
+ kunwind_stack_walk(arch_kunwind_consume_entry, &data, task, regs);
+}
+
+#else
+
+noinline noinstr void arch_stack_walk(stack_trace_consume_fn consume_entry,
+ void *cookie, struct task_struct *task,
+ struct pt_regs *regs)
{
walk_stackframe(task, regs, consume_entry, cookie);
}
+#endif /* CONFIG_FRAME_POINTER */
+
+/*
+ * Reliable stack walk for livepatch (CONFIG_FRAME_POINTER only).
+ */
+#ifdef CONFIG_FRAME_POINTER
+
+noinline noinstr int arch_stack_walk_reliable(stack_trace_consume_fn consume_entry,
+ void *cookie,
+ struct task_struct *task)
+{
+ struct kunwind_consume_entry_data data = {
+ .consume_entry = consume_entry,
+ .cookie = cookie,
+ };
+
+ return kunwind_stack_walk(arch_reliable_kunwind_consume_entry, &data,
+ task, NULL);
+}
+
+#endif /* CONFIG_FRAME_POINTER */
+
/*
* Get the return address for a single stackframe and return a pointer to the
* next frame tail.
--
2.43.0
^ permalink raw reply related
* [PATCH v3 5/8] riscv: stacktrace: introduce stack-bound tracking helpers
From: Wang Han @ 2026-06-09 6:29 UTC (permalink / raw)
To: Paul Walmsley, Palmer Dabbelt, Albert Ou
Cc: Steven Rostedt, Alexandre Ghiti, Masami Hiramatsu, Mark Rutland,
Catalin Marinas, Chen Pei, Andy Chiu, Björn Töpel,
Deepak Gupta, Puranjay Mohan, Conor Dooley, Josh Poimboeuf,
Jiri Kosina, Miroslav Benes, Petr Mladek, Joe Lawrence,
Shuah Khan, Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Namhyung Kim, oliver.yang, xueshuai, zhuo.song, jkchen,
linux-riscv, linux-kernel, linux-trace-kernel, live-patching,
linux-kselftest, linux-perf-users
In-Reply-To: <cover.194d76e3a15b.v3.riscv-livepatch.wanghan@linux.alibaba.com>
A reliable unwinder needs to validate that every frame record it reads
is fully contained in a known kernel stack, and it needs to refuse to
walk back into a stack it has already left. Add the building blocks
for that:
* struct stack_info / struct unwind_state in a new
asm/stacktrace/common.h, modelled on the arm64 reference
implementation.
* stackinfo_get_irq() / stackinfo_get_task() / stackinfo_get_overflow()
plus the corresponding on_*_stack() predicates in asm/stacktrace.h,
so callers can ask "is this object on stack X?" by stack kind
rather than open-coded address arithmetic.
* unwind_init_common(), unwind_find_stack() and
unwind_consume_stack() helpers that enforce the
forward-progress-only invariant required for reliability.
No existing user is wired up to these helpers in this commit; the
unwinder switch comes in a follow-up. The header changes leave
on_thread_stack() with the same semantics as before, just expressed in
terms of the new helpers.
Reviewed-by: Shuai Xue <xueshuai@linux.alibaba.com>
Signed-off-by: Wang Han <wanghan@linux.alibaba.com>
---
arch/riscv/include/asm/stacktrace.h | 65 ++++++++-
arch/riscv/include/asm/stacktrace/common.h | 159 +++++++++++++++++++++
2 files changed, 222 insertions(+), 2 deletions(-)
create mode 100644 arch/riscv/include/asm/stacktrace/common.h
diff --git a/arch/riscv/include/asm/stacktrace.h b/arch/riscv/include/asm/stacktrace.h
index b1495a7e06ce..bc87c4940379 100644
--- a/arch/riscv/include/asm/stacktrace.h
+++ b/arch/riscv/include/asm/stacktrace.h
@@ -3,8 +3,13 @@
#ifndef _ASM_RISCV_STACKTRACE_H
#define _ASM_RISCV_STACKTRACE_H
+#include <linux/percpu.h>
#include <linux/sched.h>
+#include <linux/sched/task_stack.h>
+
+#include <asm/irq_stack.h>
#include <asm/ptrace.h>
+#include <asm/stacktrace/common.h>
struct stackframe {
unsigned long fp;
@@ -16,14 +21,70 @@ extern void notrace walk_stackframe(struct task_struct *task, struct pt_regs *re
extern void dump_backtrace(struct pt_regs *regs, struct task_struct *task,
const char *loglvl);
-static inline bool on_thread_stack(void)
+/*
+ * IRQ stack accessors
+ */
+static inline struct stack_info stackinfo_get_irq(void)
+{
+ unsigned long low = (unsigned long)raw_cpu_read(irq_stack_ptr);
+ unsigned long high = low + IRQ_STACK_SIZE;
+
+ return (struct stack_info) {
+ .low = low,
+ .high = high,
+ };
+}
+
+static inline bool on_irq_stack(unsigned long sp, unsigned long size)
+{
+ struct stack_info info = stackinfo_get_irq();
+
+ return stackinfo_on_stack(&info, sp, size);
+}
+
+/*
+ * Task stack accessors
+ */
+static inline struct stack_info stackinfo_get_task(const struct task_struct *tsk)
{
- return !(((unsigned long)(current->stack) ^ current_stack_pointer) & ~(THREAD_SIZE - 1));
+ unsigned long low = (unsigned long)task_stack_page(tsk);
+ unsigned long high = low + THREAD_SIZE;
+
+ return (struct stack_info) {
+ .low = low,
+ .high = high,
+ };
+}
+
+static inline bool on_task_stack(const struct task_struct *tsk,
+ unsigned long sp, unsigned long size)
+{
+ struct stack_info info = stackinfo_get_task(tsk);
+
+ return stackinfo_on_stack(&info, sp, size);
}
+/*
+ * Cast is necessary since current->stack is an opaque ptr.
+ */
+#define on_thread_stack() (on_task_stack(current, current_stack_pointer, 1))
+/*
+ * Overflow stack accessors
+ */
#ifdef CONFIG_VMAP_STACK
DECLARE_PER_CPU(unsigned long [OVERFLOW_STACK_SIZE/sizeof(long)], overflow_stack);
+
+static inline struct stack_info stackinfo_get_overflow(void)
+{
+ unsigned long low = (unsigned long)raw_cpu_ptr(overflow_stack);
+ unsigned long high = low + OVERFLOW_STACK_SIZE;
+
+ return (struct stack_info) {
+ .low = low,
+ .high = high,
+ };
+}
#endif /* CONFIG_VMAP_STACK */
#endif /* _ASM_RISCV_STACKTRACE_H */
diff --git a/arch/riscv/include/asm/stacktrace/common.h b/arch/riscv/include/asm/stacktrace/common.h
new file mode 100644
index 000000000000..360a26e34349
--- /dev/null
+++ b/arch/riscv/include/asm/stacktrace/common.h
@@ -0,0 +1,159 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * RISC-V common stack unwinder types and helpers.
+ *
+ * See: arch/arm64/include/asm/stacktrace/common.h for the reference
+ * implementation.
+ *
+ * Copyright (C) 2026
+ */
+#ifndef __ASM_RISCV_STACKTRACE_COMMON_H
+#define __ASM_RISCV_STACKTRACE_COMMON_H
+
+#include <linux/compiler.h>
+#include <linux/errno.h>
+#include <linux/types.h>
+
+#include <asm/stacktrace/frame.h>
+
+/**
+ * struct stack_info - describes the bounds of a stack.
+ *
+ * @low: The lowest valid address on the stack.
+ * @high: The highest valid address on the stack.
+ */
+struct stack_info {
+ unsigned long low;
+ unsigned long high;
+};
+
+/**
+ * struct unwind_state - state used for robust unwinding.
+ *
+ * @fp: The fp value in the frame record (or the real fp).
+ * @pc: The ra value in the frame record (or the real ra).
+ *
+ * @stack: The stack currently being unwound.
+ * @stacks: An array of stacks which can be unwound.
+ * @nr_stacks: The number of stacks in @stacks.
+ */
+struct unwind_state {
+ unsigned long fp;
+ unsigned long pc;
+
+ struct stack_info stack;
+ struct stack_info *stacks;
+ int nr_stacks;
+};
+
+/**
+ * stackinfo_get_unknown() - Get an unknown stack_info.
+ *
+ * Return: a stack_info with low and high set to 0.
+ */
+static inline struct stack_info stackinfo_get_unknown(void)
+{
+ return (struct stack_info) {
+ .low = 0,
+ .high = 0,
+ };
+}
+
+/**
+ * stackinfo_on_stack() - Check whether an object is fully within a stack.
+ *
+ * @info: The stack to check against.
+ * @sp: The base address of the object.
+ * @size: The size of the object.
+ *
+ * Return: true if the object is fully contained within the stack.
+ */
+static inline bool stackinfo_on_stack(const struct stack_info *info,
+ unsigned long sp, unsigned long size)
+{
+ if (!info->low)
+ return false;
+
+ if (sp < info->low || sp + size < sp || sp + size > info->high)
+ return false;
+
+ return true;
+}
+
+/**
+ * unwind_init_common() - Initialize the common parts of the unwind state.
+ *
+ * @state: the unwind state to initialize.
+ */
+static inline void unwind_init_common(struct unwind_state *state)
+{
+ state->stack = stackinfo_get_unknown();
+}
+
+/**
+ * unwind_find_stack() - Find the accessible stack which entirely contains an
+ * object.
+ *
+ * @state: the current unwind state.
+ * @sp: the base address of the object.
+ * @size: the size of the object.
+ *
+ * Return: a pointer to the relevant stack_info if found; NULL otherwise.
+ */
+static inline struct stack_info *unwind_find_stack(struct unwind_state *state,
+ unsigned long sp,
+ unsigned long size)
+{
+ struct stack_info *info = &state->stack;
+
+ if (stackinfo_on_stack(info, sp, size))
+ return info;
+
+ for (int i = 0; i < state->nr_stacks; i++) {
+ info = &state->stacks[i];
+ if (stackinfo_on_stack(info, sp, size))
+ return info;
+ }
+
+ return NULL;
+}
+
+/**
+ * unwind_consume_stack() - Update stack boundaries so that future unwind steps
+ * cannot consume this object again.
+ *
+ * @state: the current unwind state.
+ * @info: the stack_info of the stack containing the object.
+ * @sp: the base address of the object.
+ * @size: the size of the object.
+ *
+ * Stack transitions are strictly one-way, and once we've
+ * transitioned from one stack to another, it's never valid to
+ * unwind back to the old stack.
+ *
+ * Note that stacks can nest in several valid orders, e.g.
+ *
+ * TASK -> IRQ -> OVERFLOW
+ *
+ * ... so we do not check the specific order of stack
+ * transitions.
+ */
+static inline void unwind_consume_stack(struct unwind_state *state,
+ struct stack_info *info,
+ unsigned long sp,
+ unsigned long size)
+{
+ struct stack_info tmp;
+
+ tmp = *info;
+ *info = stackinfo_get_unknown();
+ state->stack = tmp;
+
+ /*
+ * Future unwind steps can only consume stack above this frame record.
+ * Update the current stack to start immediately above it.
+ */
+ state->stack.low = sp + size;
+}
+
+#endif /* __ASM_RISCV_STACKTRACE_COMMON_H */
--
2.43.0
^ permalink raw reply related
* [PATCH v3 8/8] selftests/livepatch: Add RISC-V syscall wrapper prefix
From: Wang Han @ 2026-06-09 6:29 UTC (permalink / raw)
To: Paul Walmsley, Palmer Dabbelt, Albert Ou
Cc: Steven Rostedt, Alexandre Ghiti, Masami Hiramatsu, Mark Rutland,
Catalin Marinas, Chen Pei, Andy Chiu, Björn Töpel,
Deepak Gupta, Puranjay Mohan, Conor Dooley, Josh Poimboeuf,
Jiri Kosina, Miroslav Benes, Petr Mladek, Joe Lawrence,
Shuah Khan, Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Namhyung Kim, oliver.yang, xueshuai, zhuo.song, jkchen,
linux-riscv, linux-kernel, linux-trace-kernel, live-patching,
linux-kselftest, linux-perf-users
In-Reply-To: <cover.194d76e3a15b.v3.riscv-livepatch.wanghan@linux.alibaba.com>
The syscall livepatch selftest resolves and patches a syscall wrapper
symbol. To use that test for RISC-V livepatch validation, add the
RISC-V FN_PREFIX definition for ARCH_HAS_SYSCALL_WRAPPER.
Without this macro, the syscall livepatch selftest cannot resolve the
RISC-V target symbol, and the syscall-related livepatch test fails on
RISC-V.
Reviewed-by: Marcos Paulo de Souza <mpdesouza@suse.com>
Reviewed-by: Shuai Xue <xueshuai@linux.alibaba.com>
Signed-off-by: Wang Han <wanghan@linux.alibaba.com>
---
.../testing/selftests/livepatch/test_modules/test_klp_syscall.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/tools/testing/selftests/livepatch/test_modules/test_klp_syscall.c b/tools/testing/selftests/livepatch/test_modules/test_klp_syscall.c
index dd802783ea84..275e4b10cf59 100644
--- a/tools/testing/selftests/livepatch/test_modules/test_klp_syscall.c
+++ b/tools/testing/selftests/livepatch/test_modules/test_klp_syscall.c
@@ -18,6 +18,8 @@
#define FN_PREFIX __s390x_
#elif defined(__aarch64__)
#define FN_PREFIX __arm64_
+#elif defined(__riscv)
+#define FN_PREFIX __riscv_
#else
/* powerpc does not select ARCH_HAS_SYSCALL_WRAPPER */
#define FN_PREFIX
--
2.43.0
^ permalink raw reply related
* [PATCH v3 2/8] riscv: stacktrace: Add frame record metadata
From: Wang Han @ 2026-06-09 6:29 UTC (permalink / raw)
To: Paul Walmsley, Palmer Dabbelt, Albert Ou
Cc: Steven Rostedt, Alexandre Ghiti, Masami Hiramatsu, Mark Rutland,
Catalin Marinas, Chen Pei, Andy Chiu, Björn Töpel,
Deepak Gupta, Puranjay Mohan, Conor Dooley, Josh Poimboeuf,
Jiri Kosina, Miroslav Benes, Petr Mladek, Joe Lawrence,
Shuah Khan, Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Namhyung Kim, oliver.yang, xueshuai, zhuo.song, jkchen,
linux-riscv, linux-kernel, linux-trace-kernel, live-patching,
linux-kselftest, linux-perf-users
In-Reply-To: <cover.194d76e3a15b.v3.riscv-livepatch.wanghan@linux.alibaba.com>
Reliable frame-pointer unwinding needs an explicit way to identify
exception boundaries and the final entry frame. The existing unwinder
infers those boundaries from return addresses, which is too loose for a
future reliable unwinder.
Add a small metadata frame record to pt_regs and initialize it on
exception entry, kernel stack overflow, kernel thread fork, user fork,
and early idle task setup. The record uses a zero {fp, ra} sentinel plus
a type field so a later unwinder can distinguish a final user-to-kernel
boundary from a nested kernel pt_regs boundary.
This follows the arm64 metadata frame-record model, adapted to the
RISC-V {fp, ra} frame record convention.
The metadata is established at the RISC-V entry boundaries that need an
explicit unwind marker:
* exception entry clears the metadata {fp, ra} pair and uses SPP
(or MPP in M-mode) to record whether the pt_regs frame is the final
user-to-kernel boundary or a nested kernel boundary;
* the kernel stack overflow path builds a nested pt_regs metadata
record on the overflow stack so an unwinder can resume from the
pre-overflow s0 saved in PT_S0;
* _start_kernel builds the init task's final metadata record, while
the secondary CPU path sets up s0 before smp_callin() so idle-task
unwinding does not inherit an undefined caller frame;
* copy_thread creates matching final metadata records for new kernel
and user tasks, and keeps s0 available for the frame-pointer chain;
* call_on_irq_stack still reserves an aligned stack slot, but links the
saved {fp, ra} with the raw frame-record size so s0 points at the
RISC-V frame record rather than past the alignment padding.
The call_on_irq_stack adjustment fixes a latent RV32 issue. On RV64,
sizeof(struct stackframe) is equal to the stack alignment, so the old
s0 value happened to point just above the saved {fp, ra}. On RV32, the
raw frame record is 8 bytes while the reserved stack slot is 16-byte
aligned, so the old s0 value pointed into the padding. Using the raw
record size makes s0 point above the saved frame record on both RV32
and RV64 while still reserving the aligned slot.
These changes keep s0 reserved for the frame-pointer chain at task and
stack-switch boundaries.
Signed-off-by: Wang Han <wanghan@linux.alibaba.com>
---
arch/riscv/include/asm/ptrace.h | 9 ++++
arch/riscv/include/asm/stacktrace/frame.h | 53 +++++++++++++++++++++++
arch/riscv/kernel/asm-offsets.c | 4 ++
arch/riscv/kernel/entry.S | 43 ++++++++++++++++--
arch/riscv/kernel/head.S | 23 ++++++++++
arch/riscv/kernel/process.c | 33 +++++++++++++-
6 files changed, 159 insertions(+), 6 deletions(-)
create mode 100644 arch/riscv/include/asm/stacktrace/frame.h
diff --git a/arch/riscv/include/asm/ptrace.h b/arch/riscv/include/asm/ptrace.h
index addc8188152f..4b9b0f279214 100644
--- a/arch/riscv/include/asm/ptrace.h
+++ b/arch/riscv/include/asm/ptrace.h
@@ -8,6 +8,7 @@
#include <uapi/asm/ptrace.h>
#include <asm/csr.h>
+#include <asm/stacktrace/frame.h>
#include <linux/compiler.h>
#ifndef __ASSEMBLER__
@@ -53,6 +54,14 @@ struct pt_regs {
unsigned long cause;
/* a0 value before the syscall */
unsigned long orig_a0;
+
+ /*
+ * This frame record is entirely zeroed on exception entry, allowing the
+ * unwinder to identify exception boundaries. The type field encodes
+ * whether the exception was taken from user (FINAL) or kernel (PT_REGS)
+ * mode.
+ */
+ struct frame_record_meta stackframe;
};
#define PTRACE_SYSEMU 0x1f
diff --git a/arch/riscv/include/asm/stacktrace/frame.h b/arch/riscv/include/asm/stacktrace/frame.h
new file mode 100644
index 000000000000..5720a6c65fe8
--- /dev/null
+++ b/arch/riscv/include/asm/stacktrace/frame.h
@@ -0,0 +1,53 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+#ifndef __ASM_RISCV_STACKTRACE_FRAME_H
+#define __ASM_RISCV_STACKTRACE_FRAME_H
+
+/*
+ * See: arch/arm64/include/asm/stacktrace/frame.h for the reference
+ * implementation.
+ */
+
+/*
+ * - FRAME_META_TYPE_NONE
+ *
+ * This value is reserved.
+ *
+ * - FRAME_META_TYPE_FINAL
+ *
+ * The record is the last entry on the stack.
+ * Unwinding should terminate successfully.
+ *
+ * - FRAME_META_TYPE_PT_REGS
+ *
+ * The record is embedded within a struct pt_regs, recording the registers at
+ * an arbitrary point in time.
+ * Unwinding should consume pt_regs::epc, followed by pt_regs::ra.
+ *
+ * Note: all other values are reserved and should result in unwinding
+ * terminating with an error.
+ */
+#define FRAME_META_TYPE_NONE 0
+#define FRAME_META_TYPE_FINAL 1
+#define FRAME_META_TYPE_PT_REGS 2
+
+#ifndef __ASSEMBLER__
+/*
+ * A standard RISC-V frame record.
+ */
+struct frame_record {
+ unsigned long fp;
+ unsigned long ra;
+};
+
+/*
+ * A metadata frame record indicating a special unwind.
+ * The record::{fp,ra} fields must be zero to indicate the presence of
+ * metadata.
+ */
+struct frame_record_meta {
+ struct frame_record record;
+ unsigned long type;
+};
+#endif /* __ASSEMBLER__ */
+
+#endif /* __ASM_RISCV_STACKTRACE_FRAME_H */
diff --git a/arch/riscv/kernel/asm-offsets.c b/arch/riscv/kernel/asm-offsets.c
index af827448a609..8dfcb5a44bb8 100644
--- a/arch/riscv/kernel/asm-offsets.c
+++ b/arch/riscv/kernel/asm-offsets.c
@@ -131,6 +131,9 @@ void asm_offsets(void)
OFFSET(PT_BADADDR, pt_regs, badaddr);
OFFSET(PT_CAUSE, pt_regs, cause);
+ DEFINE(S_STACKFRAME, offsetof(struct pt_regs, stackframe));
+ DEFINE(S_STACKFRAME_TYPE, offsetof(struct pt_regs, stackframe.type));
+
OFFSET(SUSPEND_CONTEXT_REGS, suspend_context, regs);
OFFSET(HIBERN_PBE_ADDR, pbe, address);
@@ -501,6 +504,7 @@ void asm_offsets(void)
OFFSET(SBI_HART_BOOT_STACK_PTR_OFFSET, sbi_hart_boot_data, stack_ptr);
DEFINE(STACKFRAME_SIZE_ON_STACK, ALIGN(sizeof(struct stackframe), STACK_ALIGN));
+ DEFINE(STACKFRAME_RECORD_SIZE, sizeof(struct stackframe));
OFFSET(STACKFRAME_FP, stackframe, fp);
OFFSET(STACKFRAME_RA, stackframe, ra);
#ifdef CONFIG_FUNCTION_TRACER
diff --git a/arch/riscv/kernel/entry.S b/arch/riscv/kernel/entry.S
index d011fb51c59a..8df74d14b551 100644
--- a/arch/riscv/kernel/entry.S
+++ b/arch/riscv/kernel/entry.S
@@ -11,6 +11,7 @@
#include <asm/asm.h>
#include <asm/csr.h>
#include <asm/scs.h>
+#include <asm/stacktrace/frame.h>
#include <asm/unistd.h>
#include <asm/page.h>
#include <asm/thread_info.h>
@@ -193,6 +194,27 @@ SYM_CODE_START(handle_exception)
REG_S s4, PT_CAUSE(sp)
REG_S s5, PT_TP(sp)
+ /*
+ * Create a metadata frame record. The unwinder will use this to
+ * identify and unwind exception boundaries.
+ */
+ REG_S zero, (S_STACKFRAME + STACKFRAME_FP)(sp) /* stackframe.record.fp = 0 */
+ REG_S zero, (S_STACKFRAME + STACKFRAME_RA)(sp) /* stackframe.record.ra = 0 */
+#ifdef CONFIG_RISCV_M_MODE
+ li t0, SR_MPP
+ and t0, s1, t0
+#else
+ andi t0, s1, SR_SPP
+#endif
+ bnez t0, 1f
+ li t0, FRAME_META_TYPE_FINAL
+ j 2f
+1:
+ li t0, FRAME_META_TYPE_PT_REGS
+2:
+ REG_S t0, S_STACKFRAME_TYPE(sp)
+ addi s0, sp, S_STACKFRAME + STACKFRAME_RECORD_SIZE
+
/*
* Set the scratch register to 0, so that if a recursive exception
* occurs, the exception vector knows it came from the kernel
@@ -349,6 +371,19 @@ SYM_CODE_START_LOCAL(handle_kernel_stack_overflow)
REG_S s3, PT_BADADDR(sp)
REG_S s4, PT_CAUSE(sp)
REG_S s5, PT_TP(sp)
+
+ /*
+ * Create a metadata frame record for the overflow pt_regs. The
+ * overflow path is entered from kernel context, so this is a nested
+ * pt_regs boundary and the unwinder can resume from the pre-overflow
+ * frame pointer saved in PT_S0.
+ */
+ REG_S zero, (S_STACKFRAME + STACKFRAME_FP)(sp)
+ REG_S zero, (S_STACKFRAME + STACKFRAME_RA)(sp)
+ li t0, FRAME_META_TYPE_PT_REGS
+ REG_S t0, S_STACKFRAME_TYPE(sp)
+ addi s0, sp, S_STACKFRAME + STACKFRAME_RECORD_SIZE
+
move a0, sp
tail handle_bad_stack
SYM_CODE_END(handle_kernel_stack_overflow)
@@ -357,8 +392,8 @@ ASM_NOKPROBE(handle_kernel_stack_overflow)
SYM_CODE_START(ret_from_fork_kernel_asm)
call schedule_tail
- move a0, s1 /* fn_arg */
- move a1, s0 /* fn */
+ move a0, s3 /* fn_arg */
+ move a1, s2 /* fn */
move a2, sp /* pt_regs */
call ret_from_fork_kernel
j ret_from_exception
@@ -383,7 +418,7 @@ SYM_FUNC_START(call_on_irq_stack)
addi sp, sp, -STACKFRAME_SIZE_ON_STACK
REG_S ra, STACKFRAME_RA(sp)
REG_S s0, STACKFRAME_FP(sp)
- addi s0, sp, STACKFRAME_SIZE_ON_STACK
+ addi s0, sp, STACKFRAME_RECORD_SIZE
/* Switch to the per-CPU shadow call stack */
scs_save_current
@@ -399,7 +434,7 @@ SYM_FUNC_START(call_on_irq_stack)
scs_load_current
/* Switch back to the thread stack and restore ra and s0 */
- addi sp, s0, -STACKFRAME_SIZE_ON_STACK
+ addi sp, s0, -STACKFRAME_RECORD_SIZE
REG_L ra, STACKFRAME_RA(sp)
REG_L s0, STACKFRAME_FP(sp)
addi sp, sp, STACKFRAME_SIZE_ON_STACK
diff --git a/arch/riscv/kernel/head.S b/arch/riscv/kernel/head.S
index f6a8ca49e627..00e16a24f149 100644
--- a/arch/riscv/kernel/head.S
+++ b/arch/riscv/kernel/head.S
@@ -14,6 +14,7 @@
#include <asm/hwcap.h>
#include <asm/image.h>
#include <asm/scs.h>
+#include <asm/stacktrace/frame.h>
#include <asm/usercfi.h>
#include "efi-header.S"
@@ -177,6 +178,14 @@ secondary_start_sbi:
REG_S a0, (a1)
1:
#endif
+
+ /*
+ * Set up the frame pointer for the secondary idle task so reliable
+ * stack unwinding terminates at the metadata frame in task_pt_regs().
+ * Without this, the first frame records can inherit an undefined caller
+ * fp and unwind past smp_callin() into .Lsecondary_park.
+ */
+ addi s0, sp, S_STACKFRAME + STACKFRAME_RECORD_SIZE
scs_load_current
call smp_callin
#endif /* CONFIG_SMP */
@@ -305,6 +314,20 @@ SYM_CODE_START(_start_kernel)
la tp, init_task
la sp, init_thread_union + THREAD_SIZE
addi sp, sp, -PT_SIZE_ON_STACK
+
+ /*
+ * Set up a metadata frame record for the init task so that
+ * the unwinder can identify the outermost frame by its
+ * {fp, ra} = {0, 0} sentinel at the bottom of pt_regs.
+ * fp/s0 points above the metadata record (RISC-V
+ * convention).
+ */
+ REG_S zero, (S_STACKFRAME + STACKFRAME_FP)(sp)
+ REG_S zero, (S_STACKFRAME + STACKFRAME_RA)(sp)
+ li t0, FRAME_META_TYPE_FINAL
+ REG_S t0, S_STACKFRAME_TYPE(sp)
+ addi s0, sp, S_STACKFRAME + STACKFRAME_RECORD_SIZE
+
#if defined(CONFIG_RISCV_SBI) && defined(CONFIG_RISCV_USER_CFI)
li a7, SBI_EXT_FWFT
li a6, SBI_EXT_FWFT_SET
diff --git a/arch/riscv/kernel/process.c b/arch/riscv/kernel/process.c
index b2df7f72241a..0dc90bf7a652 100644
--- a/arch/riscv/kernel/process.c
+++ b/arch/riscv/kernel/process.c
@@ -258,8 +258,23 @@ int copy_thread(struct task_struct *p, const struct kernel_clone_args *args)
/* Supervisor/Machine, irqs on: */
childregs->status = SR_PP | SR_PIE;
- p->thread.s[0] = (unsigned long)args->fn;
- p->thread.s[1] = (unsigned long)args->fn_arg;
+ /*
+ * Set up a metadata frame record at the bottom of the
+ * stack for the unwinder. Use FRAME_META_TYPE_FINAL
+ * since this is the outermost kernel entry for the new
+ * task. The frame_record::{fp,ra} are already zero from
+ * memset().
+ *
+ * fp/s0 points above the metadata record (RISC-V
+ * convention). fn and fn_arg are passed via s2/s3,
+ * keeping s0 available for the frame pointer chain.
+ */
+ childregs->stackframe.type = FRAME_META_TYPE_FINAL;
+
+ p->thread.s[0] = (unsigned long)(&childregs->stackframe)
+ + sizeof(struct frame_record);
+ p->thread.s[2] = (unsigned long)args->fn;
+ p->thread.s[3] = (unsigned long)args->fn_arg;
p->thread.ra = (unsigned long)ret_from_fork_kernel_asm;
} else {
/* allocate new shadow stack if needed. In case of CLONE_VM we have to */
@@ -278,6 +293,20 @@ int copy_thread(struct task_struct *p, const struct kernel_clone_args *args)
if (clone_flags & CLONE_SETTLS)
childregs->tp = tls;
childregs->a0 = 0; /* Return value of fork() */
+
+ /*
+ * Set up the unwind boundary: ensure the metadata
+ * frame record has its {fp,ra} sentinel zeroed and
+ * point fp/s0 above the metadata record. Mark it as
+ * FINAL since this is the outermost kernel entry for
+ * the new task.
+ */
+ childregs->stackframe.record.fp = 0;
+ childregs->stackframe.record.ra = 0;
+ childregs->stackframe.type = FRAME_META_TYPE_FINAL;
+ p->thread.s[0] = (unsigned long)(&childregs->stackframe)
+ + sizeof(struct frame_record);
+
p->thread.ra = (unsigned long)ret_from_fork_user_asm;
}
p->thread.riscv_v_flags = 0;
--
2.43.0
^ permalink raw reply related
* [PATCH v3 4/8] riscv: ftrace: always preserve s0 in dynamic ftrace register frame
From: Wang Han @ 2026-06-09 6:29 UTC (permalink / raw)
To: Paul Walmsley, Palmer Dabbelt, Albert Ou
Cc: Steven Rostedt, Alexandre Ghiti, Masami Hiramatsu, Mark Rutland,
Catalin Marinas, Chen Pei, Andy Chiu, Björn Töpel,
Deepak Gupta, Puranjay Mohan, Conor Dooley, Josh Poimboeuf,
Jiri Kosina, Miroslav Benes, Petr Mladek, Joe Lawrence,
Shuah Khan, Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Namhyung Kim, oliver.yang, xueshuai, zhuo.song, jkchen,
linux-riscv, linux-kernel, linux-trace-kernel, live-patching,
linux-kselftest, linux-perf-users
In-Reply-To: <cover.194d76e3a15b.v3.riscv-livepatch.wanghan@linux.alibaba.com>
struct __arch_ftrace_regs declares s0 unconditionally, and both
ftrace_regs_get_frame_pointer() and ftrace_partial_regs() read it
unconditionally. But the SAVE_ABI_REGS / RESTORE_ABI_REGS macros in
mcount-dyn.S only stored s0 under HAVE_FUNCTION_GRAPH_FP_TEST
(CONFIG_FUNCTION_GRAPH_TRACER && CONFIG_FRAME_POINTER). With
CONFIG_FRAME_POINTER=n the slot held whatever was on the stack before,
so any callback going through ftrace_partial_regs() saw a garbage
regs->s0. RISC-V kernels default to FRAME_POINTER=y, which is why this
has not bitten in practice.
Save and restore s0 unconditionally in the dynamic ftrace ABI register
frame. This fixes the latent garbage-s0 case, brings the dynamic ftrace
path in line with the static _mcount path (mcount.S SAVE_ABI_STATE
already saves s0 unconditionally), and matches the frame layout already
documented in the comment above SAVE_ABI_REGS. It is also a prerequisite
for the upcoming reliable unwinder, which reads
ftrace_regs_get_frame_pointer(fregs) directly.
The cost is one extra REG_S/REG_L pair per traced call, negligible
compared to the overall ftrace cost; the existing FREGS_SIZE_ON_STACK
already reserved the slot, so no extra stack space is used.
Reviewed-by: Shuai Xue <xueshuai@linux.alibaba.com>
Signed-off-by: Wang Han <wanghan@linux.alibaba.com>
---
arch/riscv/kernel/mcount-dyn.S | 4 ----
1 file changed, 4 deletions(-)
diff --git a/arch/riscv/kernel/mcount-dyn.S b/arch/riscv/kernel/mcount-dyn.S
index 082fe0b0e3c0..26c55fba8fec 100644
--- a/arch/riscv/kernel/mcount-dyn.S
+++ b/arch/riscv/kernel/mcount-dyn.S
@@ -85,9 +85,7 @@
addi sp, sp, -FREGS_SIZE_ON_STACK
REG_S t0, FREGS_EPC(sp)
REG_S x1, FREGS_RA(sp)
-#ifdef HAVE_FUNCTION_GRAPH_FP_TEST
REG_S x8, FREGS_S0(sp)
-#endif
REG_S x6, FREGS_T1(sp)
#ifdef CONFIG_CC_IS_CLANG
REG_S x7, FREGS_T2(sp)
@@ -113,9 +111,7 @@
.macro RESTORE_ABI_REGS
REG_L t0, FREGS_EPC(sp)
REG_L x1, FREGS_RA(sp)
-#ifdef HAVE_FUNCTION_GRAPH_FP_TEST
REG_L x8, FREGS_S0(sp)
-#endif
REG_L x6, FREGS_T1(sp)
#ifdef CONFIG_CC_IS_CLANG
REG_L x7, FREGS_T2(sp)
--
2.43.0
^ permalink raw reply related
* [PATCH v3 1/8] scripts/sorttable: Handle RISC-V patchable ftrace entries
From: Wang Han @ 2026-06-09 6:29 UTC (permalink / raw)
To: Paul Walmsley, Palmer Dabbelt, Albert Ou
Cc: Steven Rostedt, Alexandre Ghiti, Masami Hiramatsu, Mark Rutland,
Catalin Marinas, Chen Pei, Andy Chiu, Björn Töpel,
Deepak Gupta, Puranjay Mohan, Conor Dooley, Josh Poimboeuf,
Jiri Kosina, Miroslav Benes, Petr Mladek, Joe Lawrence,
Shuah Khan, Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Namhyung Kim, oliver.yang, xueshuai, zhuo.song, jkchen,
linux-riscv, linux-kernel, linux-trace-kernel, live-patching,
linux-kselftest, linux-perf-users
In-Reply-To: <cover.194d76e3a15b.v3.riscv-livepatch.wanghan@linux.alibaba.com>
RISC-V uses -fpatchable-function-entry=8,4 when the compressed ISA is
enabled and -fpatchable-function-entry=4,2 otherwise. In both cases, the
patchable NOP area starts 8 bytes before the function symbol address.
The __mcount_loc entries therefore point at the patchable NOP area
associated with a function, while nm reports the function symbol at the
entry address used for the function range check.
After RISC-V selected HAVE_BUILDTIME_MCOUNT_SORT, sorttable started
applying that range check at build time. Without allowing entries just
before the reported function address, the mcount sorter treats valid
RISC-V ftrace callsites as invalid weak-function entries and writes
them back as zero. The resulting kernel boots with no ftrace entries,
breaking dynamic ftrace and users such as livepatch.
The failure is silent during the final link because zeroing weak-function
entries is an expected sorttable operation. At boot, those zero entries
are skipped by ftrace_process_locs(), so the only obvious symptom is that
the vmlinux ftrace table has lost valid callsites and ftrace users cannot
attach to them.
CONFIG_FTRACE_SORT_STARTUP_TEST also reports the table as sorted in this
state: it only checks that the __mcount_loc entries are in ascending
order, which a fully zeroed table trivially satisfies. The original
commit relied on this check and did not see the regression.
On an affected RISC-V QEMU boot with both CONFIG_FTRACE_SORT_STARTUP_TEST
and CONFIG_FTRACE_STARTUP_TEST enabled, the sort check still passes
while ftrace reports zero usable entries and the early selftests fail:
[ 0.000000] ftrace section at ffffffff8101da98 sorted properly
[ 0.000000] ftrace: allocating 0 entries in 128 pages
[ 0.054999] Testing tracer function: .. no entries found ..FAILED!
[ 0.172407] tracer: function failed selftest, disabling
[ 0.178186] Failed to init function_graph tracer, init returned -19
Handle RISC-V like arm64 for the function-range check and allow
patchable entries up to 8 bytes before the function address.
With this fix, a RISC-V QEMU smoke boot with ftrace startup tests shows
the vmlinux ftrace table is populated and dynamic ftrace still works:
[ 0.000000] ftrace: allocating 46749 entries in 184 pages
[ 0.051115] Testing tracer function: PASSED
[ 1.283782] Testing dynamic ftrace: PASSED
[ 6.275456] Testing tracer function_graph: PASSED
Fixes: 0ca1724b56af ("riscv: ftrace: select HAVE_BUILDTIME_MCOUNT_SORT")
Suggested-by: Steven Rostedt (Google) <rostedt@goodmis.org>
Reviewed-by: Steven Rostedt <rostedt@goodmis.org>
Reviewed-by: Shuai Xue <xueshuai@linux.alibaba.com>
Reviewed-by: Chen Pei <cp0613@linux.alibaba.com>
Link: https://lore.kernel.org/all/20260527113028.4b21a5de@fedora/
Signed-off-by: Wang Han <wanghan@linux.alibaba.com>
---
scripts/sorttable.c | 11 ++++++++---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/scripts/sorttable.c b/scripts/sorttable.c
index e8ed11c680c6..d8dc2a1b7c31 100644
--- a/scripts/sorttable.c
+++ b/scripts/sorttable.c
@@ -891,17 +891,22 @@ static int do_file(char const *const fname, void *addr)
table_sort_t custom_sort = NULL;
switch (elf_map_machine(ehdr)) {
- case EM_AARCH64:
#ifdef MCOUNT_SORT_ENABLED
+ case EM_AARCH64:
+ /* arm64 also needs RELA-based weak-function fixups. */
sort_reloc = true;
rela_type = 0x403;
- /* arm64 uses patchable function entry placing before function */
+ /* fallthrough */
+ case EM_RISCV:
+ /* arm64 and RISC-V place patchable entries before the function. */
before_func = 8;
+#else
+ case EM_AARCH64:
+ case EM_RISCV:
#endif
/* fallthrough */
case EM_386:
case EM_LOONGARCH:
- case EM_RISCV:
case EM_S390:
case EM_X86_64:
custom_sort = sort_relative_table_with_data;
--
2.43.0
^ permalink raw reply related
* [PATCH v3 3/8] riscv: stacktrace: disable KASAN and KCOV instrumentation for stacktrace.o
From: Wang Han @ 2026-06-09 6:29 UTC (permalink / raw)
To: Paul Walmsley, Palmer Dabbelt, Albert Ou
Cc: Steven Rostedt, Alexandre Ghiti, Masami Hiramatsu, Mark Rutland,
Catalin Marinas, Chen Pei, Andy Chiu, Björn Töpel,
Deepak Gupta, Puranjay Mohan, Conor Dooley, Josh Poimboeuf,
Jiri Kosina, Miroslav Benes, Petr Mladek, Joe Lawrence,
Shuah Khan, Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Namhyung Kim, oliver.yang, xueshuai, zhuo.song, jkchen,
linux-riscv, linux-kernel, linux-trace-kernel, live-patching,
linux-kselftest, linux-perf-users
In-Reply-To: <cover.194d76e3a15b.v3.riscv-livepatch.wanghan@linux.alibaba.com>
KASAN records stack traces for every alloc/free, which means it walks
the unwinder very frequently. Instrumenting the stack trace collection
code itself adds substantial overhead and makes the traces themselves
noisier.
KCOV instruments every basic-block edge. The unwinder is a hot path,
especially with KASAN enabled, so KCOV instrumentation has the same kind
of cost and noise problem here.
Mark stacktrace.o as not KASAN- or KCOV-instrumented, matching the x86
treatment of its stack unwinding code. RISC-V keeps the relevant unwinder
code in stacktrace.o, so a single translation-unit annotation covers the
equivalent scope. This is a prerequisite preference for the upcoming
reliable unwinder, but the change is valid on its own.
Signed-off-by: Wang Han <wanghan@linux.alibaba.com>
---
arch/riscv/kernel/Makefile | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/arch/riscv/kernel/Makefile b/arch/riscv/kernel/Makefile
index cabb99cadfb6..c565a72a36f3 100644
--- a/arch/riscv/kernel/Makefile
+++ b/arch/riscv/kernel/Makefile
@@ -44,6 +44,12 @@ CFLAGS_REMOVE_return_address.o = $(CC_FLAGS_FTRACE)
CFLAGS_REMOVE_sbi_ecall.o = $(CC_FLAGS_FTRACE)
endif
+# When KASAN is enabled, a stack trace is recorded for every alloc/free, which
+# can significantly impact performance. Avoid instrumenting the stack trace
+# collection code to minimize this impact.
+KASAN_SANITIZE_stacktrace.o := n
+KCOV_INSTRUMENT_stacktrace.o := n
+
always-$(KBUILD_BUILTIN) += vmlinux.lds
obj-y += head.o
--
2.43.0
^ permalink raw reply related
* [PATCH v3 0/8] riscv: Add reliable stack unwinding for livepatch
From: Wang Han @ 2026-06-09 6:29 UTC (permalink / raw)
To: Paul Walmsley, Palmer Dabbelt, Albert Ou
Cc: Steven Rostedt, Alexandre Ghiti, Masami Hiramatsu, Mark Rutland,
Catalin Marinas, Chen Pei, Andy Chiu, Björn Töpel,
Deepak Gupta, Puranjay Mohan, Conor Dooley, Josh Poimboeuf,
Jiri Kosina, Miroslav Benes, Petr Mladek, Joe Lawrence,
Shuah Khan, Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Namhyung Kim, oliver.yang, xueshuai, zhuo.song, jkchen,
linux-riscv, linux-kernel, linux-trace-kernel, live-patching,
linux-kselftest, linux-perf-users
In-Reply-To: <20260528082310.1994388-1-wanghan@linux.alibaba.com>
Hi,
This is v3 of the RISC-V reliable stack unwinding series for livepatch.
The series is still based on riscv/for-next commit 0ca1724b56af
("riscv: ftrace: select HAVE_BUILDTIME_MCOUNT_SORT").
Patch 1 fixes the build-time mcount sorting regression for RISC-V
patchable function entries. It is independent from the livepatch
enablement work and can be picked separately if that is preferred.
Patches 2-7 add the reliable frame-pointer unwinder in reviewable
steps, following the arm64 metadata-frame-record and kunwind model but
using the RISC-V {fp, ra} frame-record convention.
Patch 8 adds the RISC-V syscall wrapper prefix used by the livepatch
selftest module.
Problem
=======
Livepatch relies on HAVE_RELIABLE_STACKTRACE to decide whether a task
can safely switch to a patched implementation. RISC-V has a
frame-pointer stack walker, but it is not yet reliable enough for
livepatch. Three pieces are missing:
* arch_stack_walk_reliable() itself, plus the strict stack-bound
checks and forward-progress invariants a reliable unwinder needs.
* Explicit unwind metadata at exception, task-entry and IRQ-stack
boundaries, so the unwinder can distinguish a final user-to-kernel
transition from a nested kernel pt_regs frame instead of guessing
from return addresses.
* Agreement between the ftrace function-graph, perf callchain and
mcount paths and the same frame-record assumptions used by the
reliable unwinder.
There is also a prerequisite ftrace issue on the current riscv/for-next
base. Commit 0ca1724b56af ("riscv: ftrace: select
HAVE_BUILDTIME_MCOUNT_SORT") enabled build-time sorting of the mcount
table. RISC-V uses patchable function entries, and the recorded patch
site is placed before the function symbol. scripts/sorttable currently
does not take that RISC-V layout into account, so valid ftrace sites
can be filtered out before the kernel boots.
Solution
========
Patch 1 fixes scripts/sorttable so the RISC-V build-time mcount sort
path accepts patchable function entries which precede the function
symbol. The fix carries a Fixes: tag for commit 0ca1724b56af ("riscv:
ftrace: select HAVE_BUILDTIME_MCOUNT_SORT") and is otherwise
independent.
Patches 2-7 add the reliable unwinder in small, individually
reviewable steps. The design follows the same FP + metadata model
arm64 already uses for livepatch in production: the metadata frame
record in pt_regs, the unwind-state stack-bound bookkeeping, the
exception boundary handling, and the fgraph / kretprobe return-address
recovery are direct adaptations of arch/arm64/kernel/stacktrace.c,
retargeted to the RISC-V {fp, ra} frame record convention.
Changes since v2
================
* Patch 1:
- Split the arm64-only RELA weak-function fixup comment from the
arm64/RISC-V shared patchable-entry offset handling.
- Add Reviewed-by tags from Steven, Shuai and Chen Pei.
* Patch 2:
- Initialize frame-record metadata in the kernel stack overflow
path as FRAME_META_TYPE_PT_REGS.
- Explicitly set user-fork pt_regs metadata to
FRAME_META_TYPE_FINAL.
- Expand the commit log to document that the call_on_irq_stack
frame-record adjustment fixes a latent RV32 issue where the
aligned stack slot is larger than the raw {fp, ra} record.
* Patch 3:
- Disable KCOV instrumentation for stacktrace.o as well, and update
the subject and commit log accordingly.
* Patch 4:
- Clarify the s0 preservation rationale in the commit log.
- Add Shuai's Reviewed-by tag.
* Patch 5:
- Fix the new header copyright year.
- Add Shuai's Reviewed-by tag.
* Patch 6:
- Keep state->regs set after kunwind_next_regs_pc(), matching
kunwind_init_from_regs() and the arm64 reference.
- Use RISC-V "ra" terminology instead of "LR" in a reliable
unwinder comment.
* Patch 7:
- Document that the 64BIT dependency is a tested-scope guard rather
than a hard technical requirement, and can be relaxed after RV32
receives equivalent coverage.
- Add Shuai's Reviewed-by tag.
* Patch 8:
- Add Reviewed-by tags from Marcos and Shuai.
v2: https://lore.kernel.org/all/20260528082310.1994388-1-wanghan@linux.alibaba.com/
v1: https://lore.kernel.org/all/20260527123530.2593918-1-wanghan@linux.alibaba.com/
Wang Han (8):
scripts/sorttable: Handle RISC-V patchable ftrace entries
riscv: stacktrace: Add frame record metadata
riscv: stacktrace: disable KASAN and KCOV instrumentation for
stacktrace.o
riscv: ftrace: always preserve s0 in dynamic ftrace register frame
riscv: stacktrace: introduce stack-bound tracking helpers
riscv: stacktrace: switch to frame-pointer based unwinder
riscv: Kconfig: enable HAVE_RELIABLE_STACKTRACE and HAVE_LIVEPATCH
selftests/livepatch: Add RISC-V syscall wrapper prefix
arch/riscv/Kconfig | 4 +
arch/riscv/include/asm/ptrace.h | 9 +
arch/riscv/include/asm/stacktrace.h | 65 +-
arch/riscv/include/asm/stacktrace/common.h | 159 +++++
arch/riscv/include/asm/stacktrace/frame.h | 53 ++
arch/riscv/kernel/Makefile | 6 +
arch/riscv/kernel/asm-offsets.c | 4 +
arch/riscv/kernel/entry.S | 43 +-
arch/riscv/kernel/ftrace.c | 6 +-
arch/riscv/kernel/head.S | 23 +
arch/riscv/kernel/mcount-dyn.S | 4 -
arch/riscv/kernel/perf_callchain.c | 2 +-
arch/riscv/kernel/process.c | 33 +-
arch/riscv/kernel/stacktrace.c | 559 +++++++++++++++---
scripts/sorttable.c | 11 +-
.../livepatch/test_modules/test_klp_syscall.c | 2 +
16 files changed, 872 insertions(+), 111 deletions(-)
create mode 100644 arch/riscv/include/asm/stacktrace/common.h
create mode 100644 arch/riscv/include/asm/stacktrace/frame.h
Range-diff against v2:
1: 42147458c15b ! 1: e93530c5718e scripts/sorttable: Handle RISC-V patchable ftrace entries
@@ Commit message
Fixes: 0ca1724b56af ("riscv: ftrace: select HAVE_BUILDTIME_MCOUNT_SORT")
Suggested-by: Steven Rostedt (Google) <rostedt@goodmis.org>
+ Reviewed-by: Steven Rostedt <rostedt@goodmis.org>
+ Reviewed-by: Shuai Xue <xueshuai@linux.alibaba.com>
+ Reviewed-by: Chen Pei <cp0613@linux.alibaba.com>
Link: https://lore.kernel.org/all/20260527113028.4b21a5de@fedora/
Signed-off-by: Wang Han <wanghan@linux.alibaba.com>
@@ scripts/sorttable.c: static int do_file(char const *const fname, void *addr)
- case EM_AARCH64:
#ifdef MCOUNT_SORT_ENABLED
+ case EM_AARCH64:
++ /* arm64 also needs RELA-based weak-function fixups. */
sort_reloc = true;
rela_type = 0x403;
- /* arm64 uses patchable function entry placing before function */
+ /* fallthrough */
+ case EM_RISCV:
-+ /* arm64 and RISC-V place patchable entries before the function */
++ /* arm64 and RISC-V place patchable entries before the function. */
before_func = 8;
+#else
+ case EM_AARCH64:
2: 9f6a4bf60d10 ! 2: 5b6b411e4d9a riscv: stacktrace: Add frame record metadata
@@ Commit message
future reliable unwinder.
Add a small metadata frame record to pt_regs and initialize it on
- exception entry, kernel thread fork, user fork, and early idle task
- setup. The record uses a zero {fp, ra} sentinel plus a type field so a
- later unwinder can distinguish a final user-to-kernel boundary from a
- nested kernel pt_regs boundary.
+ exception entry, kernel stack overflow, kernel thread fork, user fork,
+ and early idle task setup. The record uses a zero {fp, ra} sentinel plus
+ a type field so a later unwinder can distinguish a final user-to-kernel
+ boundary from a nested kernel pt_regs boundary.
This follows the arm64 metadata frame-record model, adapted to the
RISC-V {fp, ra} frame record convention.
@@ Commit message
* exception entry clears the metadata {fp, ra} pair and uses SPP
(or MPP in M-mode) to record whether the pt_regs frame is the final
user-to-kernel boundary or a nested kernel boundary;
+ * the kernel stack overflow path builds a nested pt_regs metadata
+ record on the overflow stack so an unwinder can resume from the
+ pre-overflow s0 saved in PT_S0;
* _start_kernel builds the init task's final metadata record, while
the secondary CPU path sets up s0 before smp_callin() so idle-task
unwinding does not inherit an undefined caller frame;
@@ Commit message
saved {fp, ra} with the raw frame-record size so s0 points at the
RISC-V frame record rather than past the alignment padding.
+ The call_on_irq_stack adjustment fixes a latent RV32 issue. On RV64,
+ sizeof(struct stackframe) is equal to the stack alignment, so the old
+ s0 value happened to point just above the saved {fp, ra}. On RV32, the
+ raw frame record is 8 bytes while the reserved stack slot is 16-byte
+ aligned, so the old s0 value pointed into the padding. Using the raw
+ record size makes s0 point above the saved frame record on both RV32
+ and RV64 while still reserving the aligned slot.
+
These changes keep s0 reserved for the frame-pointer chain at task and
stack-switch boundaries.
@@ arch/riscv/kernel/entry.S: SYM_CODE_START(handle_exception)
/*
* Set the scratch register to 0, so that if a recursive exception
* occurs, the exception vector knows it came from the kernel
+@@ arch/riscv/kernel/entry.S: SYM_CODE_START_LOCAL(handle_kernel_stack_overflow)
+ REG_S s3, PT_BADADDR(sp)
+ REG_S s4, PT_CAUSE(sp)
+ REG_S s5, PT_TP(sp)
++
++ /*
++ * Create a metadata frame record for the overflow pt_regs. The
++ * overflow path is entered from kernel context, so this is a nested
++ * pt_regs boundary and the unwinder can resume from the pre-overflow
++ * frame pointer saved in PT_S0.
++ */
++ REG_S zero, (S_STACKFRAME + STACKFRAME_FP)(sp)
++ REG_S zero, (S_STACKFRAME + STACKFRAME_RA)(sp)
++ li t0, FRAME_META_TYPE_PT_REGS
++ REG_S t0, S_STACKFRAME_TYPE(sp)
++ addi s0, sp, S_STACKFRAME + STACKFRAME_RECORD_SIZE
++
+ move a0, sp
+ tail handle_bad_stack
+ SYM_CODE_END(handle_kernel_stack_overflow)
@@ arch/riscv/kernel/entry.S: ASM_NOKPROBE(handle_kernel_stack_overflow)
SYM_CODE_START(ret_from_fork_kernel_asm)
@@ arch/riscv/kernel/process.c: int copy_thread(struct task_struct *p, const struct
+ /*
+ * Set up the unwind boundary: ensure the metadata
+ * frame record has its {fp,ra} sentinel zeroed and
-+ * point fp/s0 above the metadata record. The type
-+ * field is inherited from the parent's pt_regs.
++ * point fp/s0 above the metadata record. Mark it as
++ * FINAL since this is the outermost kernel entry for
++ * the new task.
+ */
+ childregs->stackframe.record.fp = 0;
+ childregs->stackframe.record.ra = 0;
++ childregs->stackframe.type = FRAME_META_TYPE_FINAL;
+ p->thread.s[0] = (unsigned long)(&childregs->stackframe)
+ + sizeof(struct frame_record);
+
3: c1cc1fdba771 ! 3: dc86baa5b148 riscv: stacktrace: disable KASAN instrumentation for stacktrace.o
@@ Metadata
Author: Wang Han <wanghan@linux.alibaba.com>
## Commit message ##
- riscv: stacktrace: disable KASAN instrumentation for stacktrace.o
+ riscv: stacktrace: disable KASAN and KCOV instrumentation for stacktrace.o
KASAN records stack traces for every alloc/free, which means it walks
the unwinder very frequently. Instrumenting the stack trace collection
code itself adds substantial overhead and makes the traces themselves
noisier.
- Mark stacktrace.o as not KASAN-instrumented, matching the arm, arm64
- and x86 treatment of their stack unwinding code. This is a prerequisite
- preference for the upcoming reliable unwinder, but the change is valid
- on its own.
+ KCOV instruments every basic-block edge. The unwinder is a hot path,
+ especially with KASAN enabled, so KCOV instrumentation has the same kind
+ of cost and noise problem here.
+
+ Mark stacktrace.o as not KASAN- or KCOV-instrumented, matching the x86
+ treatment of its stack unwinding code. RISC-V keeps the relevant unwinder
+ code in stacktrace.o, so a single translation-unit annotation covers the
+ equivalent scope. This is a prerequisite preference for the upcoming
+ reliable unwinder, but the change is valid on its own.
Signed-off-by: Wang Han <wanghan@linux.alibaba.com>
@@ arch/riscv/kernel/Makefile: CFLAGS_REMOVE_return_address.o = $(CC_FLAGS_FTRACE)
+# can significantly impact performance. Avoid instrumenting the stack trace
+# collection code to minimize this impact.
+KASAN_SANITIZE_stacktrace.o := n
++KCOV_INSTRUMENT_stacktrace.o := n
+
always-$(KBUILD_BUILTIN) += vmlinux.lds
4: 8960c3c96143 ! 4: a2d474a996f9 riscv: ftrace: always preserve s0 in dynamic ftrace register frame
@@ Metadata
## Commit message ##
riscv: ftrace: always preserve s0 in dynamic ftrace register frame
- The dynamic ftrace entry/exit only saved s0 (the architectural frame
- pointer) when HAVE_FUNCTION_GRAPH_FP_TEST was selected. The upcoming
- reliable frame-pointer unwinder needs s0 to be present in
- ftrace_regs unconditionally so it can use the frame pointer as the
- function-graph return-address cookie regardless of FP_TEST.
+ struct __arch_ftrace_regs declares s0 unconditionally, and both
+ ftrace_regs_get_frame_pointer() and ftrace_partial_regs() read it
+ unconditionally. But the SAVE_ABI_REGS / RESTORE_ABI_REGS macros in
+ mcount-dyn.S only stored s0 under HAVE_FUNCTION_GRAPH_FP_TEST
+ (CONFIG_FUNCTION_GRAPH_TRACER && CONFIG_FRAME_POINTER). With
+ CONFIG_FRAME_POINTER=n the slot held whatever was on the stack before,
+ so any callback going through ftrace_partial_regs() saw a garbage
+ regs->s0. RISC-V kernels default to FRAME_POINTER=y, which is why this
+ has not bitten in practice.
Save and restore s0 unconditionally in the dynamic ftrace ABI register
- frame. The cost is one extra REG_S/REG_L pair per traced call, which is
- negligible compared to the overall ftrace cost; the benefit is a
- consistent ftrace_regs layout for the unwinder.
+ frame. This fixes the latent garbage-s0 case, brings the dynamic ftrace
+ path in line with the static _mcount path (mcount.S SAVE_ABI_STATE
+ already saves s0 unconditionally), and matches the frame layout already
+ documented in the comment above SAVE_ABI_REGS. It is also a prerequisite
+ for the upcoming reliable unwinder, which reads
+ ftrace_regs_get_frame_pointer(fregs) directly.
+ The cost is one extra REG_S/REG_L pair per traced call, negligible
+ compared to the overall ftrace cost; the existing FREGS_SIZE_ON_STACK
+ already reserved the slot, so no extra stack space is used.
+
+ Reviewed-by: Shuai Xue <xueshuai@linux.alibaba.com>
Signed-off-by: Wang Han <wanghan@linux.alibaba.com>
## arch/riscv/kernel/mcount-dyn.S ##
5: 5fb2633c7e6e ! 5: b74577e4a6b1 riscv: stacktrace: introduce stack-bound tracking helpers
@@ Commit message
on_thread_stack() with the same semantics as before, just expressed in
terms of the new helpers.
+ Reviewed-by: Shuai Xue <xueshuai@linux.alibaba.com>
Signed-off-by: Wang Han <wanghan@linux.alibaba.com>
## arch/riscv/include/asm/stacktrace.h ##
@@ arch/riscv/include/asm/stacktrace/common.h (new)
+ * See: arch/arm64/include/asm/stacktrace/common.h for the reference
+ * implementation.
+ *
-+ * Copyright (C) 2024
++ * Copyright (C) 2026
+ */
+#ifndef __ASM_RISCV_STACKTRACE_COMMON_H
+#define __ASM_RISCV_STACKTRACE_COMMON_H
6: 6b3ec0c98cd8 ! 6: ac01a5cf8317 riscv: stacktrace: switch to frame-pointer based unwinder
@@ arch/riscv/kernel/stacktrace.c: unsigned long __get_wchan(struct task_struct *ta
+ state->regs = regs;
+ state->common.pc = regs->epc;
+ state->common.fp = frame_pointer(regs);
-+ state->regs = NULL;
+ state->source = KUNWIND_SOURCE_REGS_PC;
+ return 0;
+}
@@ arch/riscv/kernel/stacktrace.c: unsigned long __get_wchan(struct task_struct *ta
+{
+ /*
+ * At an exception boundary we can reliably consume the saved PC. We do
-+ * not know whether the LR was live when the exception was taken, and
++ * not know whether ra was live when the exception was taken, and
+ * so we cannot perform the next unwind step reliably.
+ *
+ * All that matters is whether the *entire* unwind is reliable, so give
7: 90fcaa590d57 ! 7: cd40c6ddb5d1 riscv: Kconfig: enable HAVE_RELIABLE_STACKTRACE and HAVE_LIVEPATCH
@@ Commit message
to the rest of the kernel:
* select HAVE_RELIABLE_STACKTRACE under FRAME_POINTER && 64BIT, so
- only the configurations that actually have the metadata records
- and the FP-based reliable walker enable it.
+ only the configurations with the tested metadata records and
+ FP-based reliable walker enable it.
* select HAVE_LIVEPATCH under the same condition and source
kernel/livepatch/Kconfig so the livepatch menu is reachable from
the RISC-V configuration.
+ The 64BIT dependency is conservative scoping rather than a hard
+ technical requirement: the metadata frame record, kunwind state machine
+ and arch_stack_walk_reliable() also build on RV32, and the IRQ-stack
+ frame-record adjustment fixes a latent RV32 issue. However, the syscall
+ livepatch selftest and module relocation path have only been exercised
+ on RV64 QEMU virt so far. The 64BIT gate can be relaxed in a follow-up
+ once RV32 has equivalent coverage.
+
This is split out from the unwinder change so the policy decision and
the implementation can be reviewed and reverted independently.
+ Reviewed-by: Shuai Xue <xueshuai@linux.alibaba.com>
Signed-off-by: Wang Han <wanghan@linux.alibaba.com>
## arch/riscv/Kconfig ##
8: 9590be5df884 ! 8: 194d76e3a15b selftests/livepatch: Add RISC-V syscall wrapper prefix
@@ Commit message
RISC-V target symbol, and the syscall-related livepatch test fails on
RISC-V.
+ Reviewed-by: Marcos Paulo de Souza <mpdesouza@suse.com>
+ Reviewed-by: Shuai Xue <xueshuai@linux.alibaba.com>
Signed-off-by: Wang Han <wanghan@linux.alibaba.com>
## tools/testing/selftests/livepatch/test_modules/test_klp_syscall.c ##
base-commit: 0ca1724b56af054e304a9f3f60623b02a81aba3f
--
2.43.0
^ permalink raw reply
* [PATCH 0/2] arm64: ftrace: support DIRECT_CALLS without CALL_OPS
From: Jose Fernandez (Anthropic) @ 2026-06-09 5:19 UTC (permalink / raw)
To: Steven Rostedt, Masami Hiramatsu, Mark Rutland, Catalin Marinas,
Will Deacon, Nathan Chancellor, Nick Desaulniers, Bill Wendling,
Justin Stitt
Cc: linux-kernel, linux-trace-kernel, linux-arm-kernel, llvm, bpf,
Florent Revest, Puranjay Mohan, Xu Kuohai,
Jose Fernandez (Anthropic)
On arm64, HAVE_DYNAMIC_FTRACE_WITH_DIRECT_CALLS is currently selected
only when DYNAMIC_FTRACE_WITH_CALL_OPS is available. CALL_OPS, in
turn, is mutually exclusive with kCFI: the pre-function NOPs it needs
would change the offset of the pre-function type hash (see
baaf553d3bc3 ("arm64: Implement HAVE_DYNAMIC_FTRACE_WITH_CALL_OPS")),
and the compiler support needed to reconcile the two does not exist
yet.
The result is that a CONFIG_CFI=y arm64 kernel has no
ftrace direct calls at all, so register_fentry() fails with -ENOTSUPP
and no BPF trampoline can attach: fentry/fexit, fmod_ret and BPF LSM
programs are all unavailable. Deployments that want both kCFI
hardening and BPF-based security monitoring currently have to give
one of them up. systemd's bpf-restrict-fs feature hits this today:
https://lore.kernel.org/all/20250610232418.GA3544567@ax162/
CALL_OPS is an optimization for direct calls, not a dependency.
In-BL-range trampolines are reached by a direct branch without
consulting the ops pointer, and out-of-range trampolines already
fall back to ftrace_caller, where the DIRECT_CALLS machinery
(call_direct_funcs() storing the trampoline in ftrace_regs, the
ftrace_caller tail-call) is gated on DIRECT_CALLS alone. s390 and
loongarch ship HAVE_DYNAMIC_FTRACE_WITH_DIRECT_CALLS this way,
without having CALL_OPS at all.
Patch 1 prepares ftrace_modify_call() to build without CALL_OPS by
widening its #ifdef and using the existing ftrace_rec_update_ops()
wrapper (no functional change for current configurations). Patch 2
drops the CALL_OPS requirement from the DIRECT_CALLS select.
Configurations that keep CALL_OPS (clang !CFI, and GCC without
CC_OPTIMIZE_FOR_SIZE) are unchanged. We verified this: in an arm64
clang build, every object file is byte-identical before and after
the series except ftrace.o itself, and its disassembly is identical.
CFI builds (and GCC -Os builds) gain working direct calls, with
out-of-range attachments taking the ftrace_caller dispatch path
instead of the per-callsite fast path.
We tested on a 6.18.y-based kernel and on this base with clang
kCFI builds (CONFIG_CFI=y, enforcing) under qemu (TCG, and KVM on an
arm64 host) and on GB200-based arm64 hardware: fentry/fexit, fmod_ret
and BPF LSM programs load, attach and execute; the ftrace-direct
sample modules (including both modify samples, exercising
ftrace_modify_call()) run cleanly; no CFI violations observed. The
fentry_test, fexit_test, fentry_fexit, fexit_sleep, fexit_stress,
modify_return, tracing_struct, lsm and trampoline_count selftests and
the ftrace direct-call selftests (test.d/direct) pass on the new
configuration with results identical to a CALL_OPS kernel built from
the same tree, and a broader test_progs sweep showed no differences
attributable to this series. Without the series, all of the above
fail at attach time with -ENOTSUPP.
riscv has the same gap (its DIRECT_CALLS select also requires
CALL_OPS, and its CALL_OPS is likewise !CFI); if this approach is
acceptable for arm64 we can follow up there.
---
Jose Fernandez (Anthropic) (2):
arm64: ftrace: prepare ftrace_modify_call() for use without CALL_OPS
arm64: ftrace: allow DIRECT_CALLS without CALL_OPS
arch/arm64/Kconfig | 2 +-
arch/arm64/kernel/ftrace.c | 5 +++--
2 files changed, 4 insertions(+), 3 deletions(-)
---
base-commit: 254f49634ee16a731174d2ae34bc50bd5f45e731
change-id: 20260607-arm64-ftrace-direct-calls-152230ef7077
Best regards,
--
Jose Fernandez (Anthropic) <jose.fernandez@linux.dev>
^ permalink raw reply
* [PATCH 2/2] arm64: ftrace: allow DIRECT_CALLS without CALL_OPS
From: Jose Fernandez (Anthropic) @ 2026-06-09 5:19 UTC (permalink / raw)
To: Steven Rostedt, Masami Hiramatsu, Mark Rutland, Catalin Marinas,
Will Deacon, Nathan Chancellor, Nick Desaulniers, Bill Wendling,
Justin Stitt
Cc: linux-kernel, linux-trace-kernel, linux-arm-kernel, llvm, bpf,
Florent Revest, Puranjay Mohan, Xu Kuohai,
Jose Fernandez (Anthropic)
In-Reply-To: <20260609-arm64-ftrace-direct-calls-v1-0-4a46f266697f@linux.dev>
arm64 gained ftrace direct calls in commit 2aa6ac03516d ("arm64:
ftrace: Add direct call support") on top of
DYNAMIC_FTRACE_WITH_CALL_OPS, using the per-callsite ops pointer as a
fast path to reach the direct trampoline. Since commit baaf553d3bc3
("arm64: Implement HAVE_DYNAMIC_FTRACE_WITH_CALL_OPS"), CALL_OPS is
mutually exclusive with CFI: the pre-function NOPs would change the
offset of the pre-function kCFI type hash, and the compiler support
needed to keep that offset consistent does not exist yet.
The result is that a CONFIG_CFI=y kernel loses CALL_OPS, and with it
DIRECT_CALLS, and with it every BPF trampoline attachment to kernel
functions: register_fentry() returns -ENOTSUPP, so fentry/fexit,
fmod_ret and BPF LSM programs cannot attach at all. This is a real
problem for hardened arm64 deployments that rely on BPF LSM for
security monitoring while keeping kCFI enabled.
CALL_OPS is an optimization for direct calls, not a dependency. When
the direct trampoline is within BL range, the callsite branches
straight to it and ftrace_caller is not involved. When it is out of
range, ftrace_find_callable_addr() already falls back to
ftrace_caller, and the DIRECT_CALLS machinery there
(FREGS_DIRECT_TRAMP, ftrace_caller_direct_late) is gated on
DIRECT_CALLS alone, not CALL_OPS: the ops dispatch invokes
call_direct_funcs(), which stores the trampoline address in
ftrace_regs, and ftrace_caller tail-calls it. s390 and loongarch use
this same mechanism for HAVE_DYNAMIC_FTRACE_WITH_DIRECT_CALLS without
having CALL_OPS at all, and DYNAMIC_FTRACE_WITH_ARGS without CALL_OPS
is already a supported arm64 configuration (GCC builds with
CC_OPTIMIZE_FOR_SIZE do not satisfy the CALL_OPS select condition).
Drop the CALL_OPS requirement from the
HAVE_DYNAMIC_FTRACE_WITH_DIRECT_CALLS select. Configurations that
keep CALL_OPS (!CFI clang builds, and GCC builds without
CC_OPTIMIZE_FOR_SIZE) are unchanged. CALL_OPS-less configurations
take the ftrace_caller ops-dispatch path for out-of-range direct
calls, trading the per-callsite fast path for working BPF
trampolines; in-range attachments still branch directly with no
overhead. GCC -Os builds also gain DIRECT_CALLS as a side effect.
That is intended: s390 and loongarch already ship DIRECT_CALLS
without any per-callsite fast path.
Assisted-by: Claude:unspecified
Signed-off-by: Jose Fernandez (Anthropic) <jose.fernandez@linux.dev>
---
arch/arm64/Kconfig | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index fe60738e5943b..2cd7d536671c9 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -188,7 +188,7 @@ config ARM64
if (GCC_SUPPORTS_DYNAMIC_FTRACE_WITH_ARGS || \
CLANG_SUPPORTS_DYNAMIC_FTRACE_WITH_ARGS)
select HAVE_DYNAMIC_FTRACE_WITH_DIRECT_CALLS \
- if DYNAMIC_FTRACE_WITH_ARGS && DYNAMIC_FTRACE_WITH_CALL_OPS
+ if DYNAMIC_FTRACE_WITH_ARGS
select HAVE_DYNAMIC_FTRACE_WITH_CALL_OPS \
if (DYNAMIC_FTRACE_WITH_ARGS && !CFI && \
(CC_IS_CLANG || !CC_OPTIMIZE_FOR_SIZE))
--
2.52.0
^ permalink raw reply related
* [PATCH 1/2] arm64: ftrace: prepare ftrace_modify_call() for use without CALL_OPS
From: Jose Fernandez (Anthropic) @ 2026-06-09 5:19 UTC (permalink / raw)
To: Steven Rostedt, Masami Hiramatsu, Mark Rutland, Catalin Marinas,
Will Deacon, Nathan Chancellor, Nick Desaulniers, Bill Wendling,
Justin Stitt
Cc: linux-kernel, linux-trace-kernel, linux-arm-kernel, llvm, bpf,
Florent Revest, Puranjay Mohan, Xu Kuohai,
Jose Fernandez (Anthropic)
In-Reply-To: <20260609-arm64-ftrace-direct-calls-v1-0-4a46f266697f@linux.dev>
ftrace_modify_call() is guarded by CONFIG_DYNAMIC_FTRACE_WITH_CALL_OPS
and calls ftrace_rec_set_ops(rec, arm64_rec_get_ops(rec)) directly,
which only exists when CALL_OPS is enabled.
Generic ftrace also needs ftrace_modify_call() when
CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS is enabled, to retarget a
callsite between two non-FTRACE_ADDR destinations, as happens when a
direct trampoline is modified. The next patch allows DIRECT_CALLS without
CALL_OPS, so widen the guard to cover both configurations and switch
the body to the ftrace_rec_update_ops() wrapper, which already has a
stub for the !CALL_OPS case. ftrace_make_call() already uses the same
wrapper today.
No functional change: with CALL_OPS enabled, ftrace_rec_update_ops()
expands to the exact call this replaces.
Assisted-by: Claude:unspecified
Signed-off-by: Jose Fernandez (Anthropic) <jose.fernandez@linux.dev>
---
arch/arm64/kernel/ftrace.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/arch/arm64/kernel/ftrace.c b/arch/arm64/kernel/ftrace.c
index 5a1554a441628..e1a3c0b3a0514 100644
--- a/arch/arm64/kernel/ftrace.c
+++ b/arch/arm64/kernel/ftrace.c
@@ -409,7 +409,8 @@ int ftrace_make_call(struct dyn_ftrace *rec, unsigned long addr)
return ftrace_modify_code(pc, old, new, true);
}
-#ifdef CONFIG_DYNAMIC_FTRACE_WITH_CALL_OPS
+#if defined(CONFIG_DYNAMIC_FTRACE_WITH_CALL_OPS) || \
+ defined(CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS)
int ftrace_modify_call(struct dyn_ftrace *rec, unsigned long old_addr,
unsigned long addr)
{
@@ -417,7 +418,7 @@ int ftrace_modify_call(struct dyn_ftrace *rec, unsigned long old_addr,
u32 old, new;
int ret;
- ret = ftrace_rec_set_ops(rec, arm64_rec_get_ops(rec));
+ ret = ftrace_rec_update_ops(rec);
if (ret)
return ret;
--
2.52.0
^ permalink raw reply related
* [PATCH] tracing/osnoise: Call synchronize_rcu() when unregistering
From: Crystal Wood @ 2026-06-09 4:54 UTC (permalink / raw)
To: Steven Rostedt
Cc: linux-trace-kernel, John Kacur, Tomas Glozar, Costa Shulyupin,
Wander Lairson Costa, Crystal Wood
This ensures that any RCU readers traversing the instance list
have finished, before releasing the reference on the tracer that
the instance points to.
Suggested-by: Steven Rostedt <rostedt@goodmis.org>
Signed-off-by: Crystal Wood <crwood@redhat.com>
---
kernel/trace/trace_osnoise.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/kernel/trace/trace_osnoise.c b/kernel/trace/trace_osnoise.c
index 5e83c4f6f2b4..0e1265acd1cc 100644
--- a/kernel/trace/trace_osnoise.c
+++ b/kernel/trace/trace_osnoise.c
@@ -179,7 +179,9 @@ static void osnoise_unregister_instance(struct trace_array *tr)
if (!found)
return;
- kvfree_rcu_mightsleep(inst);
+ /* Do a full sync to ensure that tr remains valid, not just inst */
+ synchronize_rcu();
+ kvfree(inst);
}
/*
--
2.54.0
^ permalink raw reply related
* Re: [PATCH] rethook: Use tsk->on_cpu to check task execution state
From: Masami Hiramatsu @ 2026-06-09 4:41 UTC (permalink / raw)
To: Peter Zijlstra
Cc: bpf, Tengda Wu, Steven Rostedt, Mathieu Desnoyers,
Alexei Starovoitov, linux-trace-kernel, linux-kernel,
Josh Poimboeuf, jikos, mbenes, pmladek
In-Reply-To: <20260608140654.GE3102624@noisy.programming.kicks-ass.net>
On Mon, 8 Jun 2026 16:06:54 +0200
Peter Zijlstra <peterz@infradead.org> wrote:
> On Mon, Jun 08, 2026 at 10:08:11PM +0900, Masami Hiramatsu wrote:
>
> > > > Anyway, I'm wondering what the purpose of this check here is, there is
> > > > no real comment, and commit 5120d167e21c ("rethook: Remove warning
> > > > messages printed for finding return address of a frame.") is just pure
> > > > voodoo as well.
> > >
> > > FWIW, you should have had this discussion then.
> >
> > Indeed. The rethook is making a shadow stack by list, thus caller must
> > guarantee the target process is blocked at least during this function.
> >
> > The commit messages suggest that when BPF takes a backtrace, it also
> > includes other running tasks. Is that safe?
>
> Well, you get to keep the pieces. At this point safe only pertains to
> 'doesn't-crash', all correctness is out the window.
>
> I always forget the crazy BPF does ;-)
>
> > > > Also, note the comment that goes with the usage of
> > > > task_on_another_cpu(); that thing is racy as all heck.
> > > >
> > > > So it really comes down to what the purpose of this check is.
> >
> > This check has been introduced when it is copied from
> > kretprobe_find_ret_addr(). It has the comment:
> >
> > * The @tsk must be 'current' or a task which is not running. @fp is a hint
> >
> > IIRC, I added this check to explicitly verify this condition.
>
> Right, but it is a prescriptive comment, not an explanatory one. That
> is, it doesn't explain the condition.
>
> > > > I suspect the issue at hand is that tsk->rethook elements, such as
> > > > iterated by __rethook_find_ret_addr() are not safe to be accessed for a
> > > > running task.
> > > >
> > > > Notably while rethook_recycle() has some RCU thing on, that objpool
> > > > thing (and the recycle name itself) seems to strongly suggest iterating
> > > > these things is not sound (you could start with things from this task,
> > > > hit a recycled entry and continue iterating rethooks from another task).
> > > >
> > > > Also note that the current check is also racy, nothing really prevents a
> > > > wakeup from happening right after you observe task_is_running() being
> > > > false. The task can then get scheduled in on another CPU and tear down
> > > > its rethooks concurrent with __rethook_find_ret_addr().
> >
> > Yeah, but is there any way to ensure the task is blocked? Even if it is
> > blocked, like TASK_UNINTERRUPTIBLE, unless holding the actual lock in
> > the rethook, it may not be possible to ensure it?
> >
> > Of course, we could give up on checking within this function and leave
> > everything to the caller to guarantee - as kretprobe does.
> >
> > BTW, the reason why we made it possible to pass tasks other than current
> > is that the stack unwinding code itself supported unwinding tasks other
> > than current, so we had no choice but to create this interface.
> >
> > However, it is a bad idea to check this in deep inside of unwinding.
>
> This, you cannot take locks in unwinding. The only thing you can do is
> try to do the best you can without crashing.
>
> Typically unwind only happens on self -- this is natural, a task crashes
> and unwinds itself, or a task does something (takes a lock, hits a
> tracepoint, etc) and takes a snapshot of its own stack, and this is
> safe.
>
> Things like live-patch use task_call_func(), which ensures the callback
> function is done while holding sufficient locks for the task to not
> change state.
Hmm, is there any way to ensure the function is called from task_call_func()?
(Maybe checking p->pi_lock, but this is not sure the lock owner is this
context?) If not, I need to make this available only for current task
(anyway it just return kretprobe trampoline address, no critical issue)
or, introduce a spinlock.
Or, eventually it may be better to replace kretprobe/rethook with
fprobe return handler.
Thank you,
--
Masami Hiramatsu (Google) <mhiramat@kernel.org>
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox