* Re: [PATCH V2] blktrace: log dropped REQ_OP_ZONE_XXX events ver1
From: Chaitanya Kulkarni @ 2025-12-19 23:50 UTC (permalink / raw)
To: Johannes Thumshirn, Chaitanya Kulkarni
Cc: axboe@kernel.dk, dlemoal@kernel.org, rostedt@goodmis.org,
mhiramat@kernel.org, mathieu.desnoyers@efficios.com,
martin.petersen@oracle.com, linux-block@vger.kernel.org,
linux-trace-kernel@vger.kernel.org, Chaitanya Kulkarni
In-Reply-To: <fc5f6ed4-5bd3-4c4d-82c3-cdab26dcbf94@wdc.com>
On 10/28/25 23:47, Johannes Thumshirn wrote:
> On 10/28/25 6:32 PM, Chaitanya Kulkarni wrote:
>> + if (version == 1 && IS_ENABLED(CONFIG_BLK_DEV_ZONED)) {
>> + pr_info("%s: blktrace events for REQ_OP_ZONE_XXX will be dropped\n",
>> + name);
>> + pr_info("use blktrace tools version >= 2 to track REQ_OP_ZONE_XXX\n");
> Jens hasn't merged the user-space part yet and I'm not sure he'll assign
> a dedicated version number for it. So I'd say hold this until he has.
>
Now Jen's has merged the userspace part, may I please get a reviewed-by if this
looks good?
-ck
^ permalink raw reply
* Re: [PATCH v4 8/8] nfsd: freeze c/mtime updates with outstanding WRITE_ATTRS delegation
From: Olga Kornievskaia @ 2025-12-19 17:43 UTC (permalink / raw)
To: Jeff Layton
Cc: Alexander Viro, Christian Brauner, Jan Kara, Steven Rostedt,
Masami Hiramatsu, Mathieu Desnoyers, Chuck Lever, NeilBrown,
Olga Kornievskaia, Dai Ngo, Tom Talpey, Trond Myklebust,
Anna Schumaker, linux-fsdevel, linux-kernel, linux-trace-kernel,
linux-nfs
In-Reply-To: <ca86b70c1a4a25c2f084bfce53ed864a557ebfed.camel@kernel.org>
On Fri, Dec 19, 2025 at 12:25 PM Jeff Layton <jlayton@kernel.org> wrote:
>
> On Fri, 2025-12-19 at 10:58 -0500, Olga Kornievskaia wrote:
> > Hi Jeff,
> >
> > I narrowed down the upstream failure for generic/215 and generic/407
> > to this commit.
> >
> > Let's consider first where the kernel is compiled with delegated
> > attributes off (but it also fails just the same if the delegated
> > attributes are compiled in).
> >
> > I don't understand why the code unconditionally changed to call
> > nfsd4_finalize_deleg_timestamps() which I think the main driver behind
> > the failure.
> >
> > Running generic/407 there is an OPEN (which gives out a write
> > delegation) and returns a change id, then on this filehandle there is
> > a SETATTR (with a getattr) which returns a new changeid. Then there is
> > a CLONE where the filehandle is the destination filehandle on which
> > there is a getattr which returns unchanged changeid/modify time (bad).
> > Then there is a DELEGRETURN (with a getattr) which again returns same
> > change id. Test fails.
> >
> > Prior to this commit. The changeid/modify time is different in CLONE
> > and DELEGRETURN -- test passes.
> >
> > Now let me describe what happens with delegated attributes enabled.
> > OPEN returns delegated attributes delegation, included getattr return
> > a changeid. Then CLONE is done, the included gettattr returns a
> > different (from open's) changeid (different time_modify). Then there
> > is SETATTR+GEATTR+DELEGRETURN compound from the client (which carries
> > a time_deleg_modify value different from above). Server in getattr
> > replies with changeid same as in clone and mtime with the value client
> > provided. So I'm not sure exactly why the test fails here but that's a
> > different problem as my focus is on "delegation attribute off option"
> > at the moment.
> >
> > I don't know if this is the correct fix or not but perhaps we
> > shouldn't unconditionally be setting this mode? (note this fix only
> > fixes the delegattributes off. however i have no claims that this
> > patch is what broke 215/407 for delegated attributes on. Something
> > else is in play there). If this solution is acceptable, I can send a
> > patch.
> >
> > diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> > index 81fa7cc6c77b..624cc6ab2802 100644
> > --- a/fs/nfsd/nfs4state.c
> > +++ b/fs/nfsd/nfs4state.c
> > @@ -6318,7 +6318,8 @@ nfs4_open_delegation(struct svc_rqst *rqstp,
> > struct nfsd4_open *open,
> > dp->dl_ctime = stat.ctime;
> > dp->dl_mtime = stat.mtime;
> > spin_lock(&f->f_lock);
> > - f->f_mode |= FMODE_NOCMTIME;
> > + if (deleg_ts)
> > + f->f_mode |= FMODE_NOCMTIME;
> > spin_unlock(&f->f_lock);
> > trace_nfsd_deleg_write(&dp->dl_stid.sc_stateid);
> > } else {
> >
> >
>
> That patch does look correct to me -- nice catch. Have you validated
> that it fixes 215 and 407?
Yes, it does fix 215 and 407 for me.
>
> Thanks,
> Jeff
> --
> Jeff Layton <jlayton@kernel.org>
^ permalink raw reply
* Re: [PATCH v4 8/8] nfsd: freeze c/mtime updates with outstanding WRITE_ATTRS delegation
From: Jeff Layton @ 2025-12-19 17:25 UTC (permalink / raw)
To: Olga Kornievskaia
Cc: Alexander Viro, Christian Brauner, Jan Kara, Steven Rostedt,
Masami Hiramatsu, Mathieu Desnoyers, Chuck Lever, NeilBrown,
Olga Kornievskaia, Dai Ngo, Tom Talpey, Trond Myklebust,
Anna Schumaker, linux-fsdevel, linux-kernel, linux-trace-kernel,
linux-nfs
In-Reply-To: <CAN-5tyEjYRFrJ7Gc4S8KwAZUuF-uz6ovPa4-_ynt+GGVqJHN_A@mail.gmail.com>
On Fri, 2025-12-19 at 10:58 -0500, Olga Kornievskaia wrote:
> Hi Jeff,
>
> I narrowed down the upstream failure for generic/215 and generic/407
> to this commit.
>
> Let's consider first where the kernel is compiled with delegated
> attributes off (but it also fails just the same if the delegated
> attributes are compiled in).
>
> I don't understand why the code unconditionally changed to call
> nfsd4_finalize_deleg_timestamps() which I think the main driver behind
> the failure.
>
> Running generic/407 there is an OPEN (which gives out a write
> delegation) and returns a change id, then on this filehandle there is
> a SETATTR (with a getattr) which returns a new changeid. Then there is
> a CLONE where the filehandle is the destination filehandle on which
> there is a getattr which returns unchanged changeid/modify time (bad).
> Then there is a DELEGRETURN (with a getattr) which again returns same
> change id. Test fails.
>
> Prior to this commit. The changeid/modify time is different in CLONE
> and DELEGRETURN -- test passes.
>
> Now let me describe what happens with delegated attributes enabled.
> OPEN returns delegated attributes delegation, included getattr return
> a changeid. Then CLONE is done, the included gettattr returns a
> different (from open's) changeid (different time_modify). Then there
> is SETATTR+GEATTR+DELEGRETURN compound from the client (which carries
> a time_deleg_modify value different from above). Server in getattr
> replies with changeid same as in clone and mtime with the value client
> provided. So I'm not sure exactly why the test fails here but that's a
> different problem as my focus is on "delegation attribute off option"
> at the moment.
>
> I don't know if this is the correct fix or not but perhaps we
> shouldn't unconditionally be setting this mode? (note this fix only
> fixes the delegattributes off. however i have no claims that this
> patch is what broke 215/407 for delegated attributes on. Something
> else is in play there). If this solution is acceptable, I can send a
> patch.
>
> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> index 81fa7cc6c77b..624cc6ab2802 100644
> --- a/fs/nfsd/nfs4state.c
> +++ b/fs/nfsd/nfs4state.c
> @@ -6318,7 +6318,8 @@ nfs4_open_delegation(struct svc_rqst *rqstp,
> struct nfsd4_open *open,
> dp->dl_ctime = stat.ctime;
> dp->dl_mtime = stat.mtime;
> spin_lock(&f->f_lock);
> - f->f_mode |= FMODE_NOCMTIME;
> + if (deleg_ts)
> + f->f_mode |= FMODE_NOCMTIME;
> spin_unlock(&f->f_lock);
> trace_nfsd_deleg_write(&dp->dl_stid.sc_stateid);
> } else {
>
>
That patch does look correct to me -- nice catch. Have you validated
that it fixes 215 and 407?
Thanks,
Jeff
--
Jeff Layton <jlayton@kernel.org>
^ permalink raw reply
* Re: [PATCH 1/3] mm/khugepaged: remove unnecessary goto 'skip' label
From: Zi Yan @ 2025-12-19 16:07 UTC (permalink / raw)
To: Konstantin Ryabitsev, Garg, Shivank
Cc: Andrew Morton, David Hildenbrand, Lorenzo Stoakes, Baolin Wang,
Liam R . Howlett, Nico Pache, Ryan Roberts, Dev Jain, Barry Song,
Lance Yang, linux-mm, linux-kernel, linux-trace-kernel
In-Reply-To: <20251219-flying-congenial-harrier-a7e4f5@lemur>
On 19 Dec 2025, at 11:00, Konstantin Ryabitsev wrote:
> On Fri, Dec 19, 2025 at 05:50:39PM +0530, Garg, Shivank wrote:
>> I noticed that b4 didn't pick up your Reviewed-by tag automatically (possibly because
>> it was on the same line as "LGTM.").
>
> That is correct -- code review trailers must always be on a separate line for
> any tooling to properly recognize them.
Got it. Will do that from now on. Thank you both for pointing this out.
Best Regards,
Yan, Zi
^ permalink raw reply
* Re: [PATCH v10 2/3] mm: Fix OOM killer inaccuracy on large many-core systems
From: Mathieu Desnoyers @ 2025-12-19 16:01 UTC (permalink / raw)
To: Mark Brown
Cc: Thomas Gleixner, Andrew Morton, linux-kernel, Paul E. McKenney,
Steven Rostedt, Masami Hiramatsu, Dennis Zhou, Tejun Heo,
Christoph Lameter, Martin Liu, David Rientjes, christian.koenig,
Shakeel Butt, SeongJae Park, Michal Hocko, Johannes Weiner,
Sweet Tea Dorminy, Lorenzo Stoakes, Liam R . Howlett,
Mike Rapoport, Suren Baghdasaryan, Vlastimil Babka,
Christian Brauner, Wei Yang, David Hildenbrand, Miaohe Lin,
Al Viro, linux-mm, linux-trace-kernel, Yu Zhao, Roman Gushchin,
Mateusz Guzik, Matthew Wilcox, Baolin Wang, Aboorva Devarajan,
Aishwarya TCV
In-Reply-To: <2a772584-2269-4008-809a-6123e226ebe3@sirena.org.uk>
On 2025-12-19 04:31, Mark Brown wrote:
> On Thu, Dec 18, 2025 at 05:18:04PM -0500, Mathieu Desnoyers wrote:
>> On 2025-12-18 13:00, Mark Brown wrote:
>
>> An ugly work-around that may work (and then we can improve on this),
>> at the end of mm/init-mm.c:init_mm (completely untested):
>
>> .cpu_bitmap = { [0 ... ((3*BITS_TO_LONGS(NR_CPUS))-1 + ((69905 * NR_MM_COUNTERS * 64) / BYTES_PER_LONG))] = 0UL },
>
> That doesn't seem to fix the FVP unfortunately (BYTES_PER_LONG doesn't
> exist, but even just deleting the division entirely fails in the same
> way).
I just noticed that there is another static instance of mm_struct:
drivers/firmware/efi/efi.c struct mm_struct efi_mm
we need to apply the same fix to it as well. It seems to fit
with the currently running task when the oops happens:
[ 2.482454] CPU: 2 UID: 0 PID: 12 Comm: kworker/u32:0 Not tainted 6.19.0-rc1-next-20251218 #1 PREEMPT
[ 2.482609] Workqueue: efi_rts_wq efi_call_rts
[...]
[ 2.485094] acct_account_cputime+0x40/0xa4 (P)
[...]
[ 2.487172] el1h_64_irq+0x6c/0x70
[...]
[ 2.487853] __efi_rt_asm_wrapper+0x50/0x74
Thanks,
Mathieu
--
Mathieu Desnoyers
EfficiOS Inc.
https://www.efficios.com
^ permalink raw reply
* Re: [PATCH 1/3] mm/khugepaged: remove unnecessary goto 'skip' label
From: Konstantin Ryabitsev @ 2025-12-19 16:00 UTC (permalink / raw)
To: Garg, Shivank
Cc: Zi Yan, Andrew Morton, David Hildenbrand, Lorenzo Stoakes,
Baolin Wang, Liam R . Howlett, Nico Pache, Ryan Roberts, Dev Jain,
Barry Song, Lance Yang, linux-mm, linux-kernel,
linux-trace-kernel
In-Reply-To: <32f3f9dd-f84c-4113-b356-01750b2af80a@amd.com>
On Fri, Dec 19, 2025 at 05:50:39PM +0530, Garg, Shivank wrote:
> I noticed that b4 didn't pick up your Reviewed-by tag automatically (possibly because
> it was on the same line as "LGTM.").
That is correct -- code review trailers must always be on a separate line for
any tooling to properly recognize them.
Best regards,
-K
^ permalink raw reply
* Re: [PATCH v4 8/8] nfsd: freeze c/mtime updates with outstanding WRITE_ATTRS delegation
From: Olga Kornievskaia @ 2025-12-19 15:58 UTC (permalink / raw)
To: Jeff Layton
Cc: Alexander Viro, Christian Brauner, Jan Kara, Steven Rostedt,
Masami Hiramatsu, Mathieu Desnoyers, Chuck Lever, NeilBrown,
Olga Kornievskaia, Dai Ngo, Tom Talpey, Trond Myklebust,
Anna Schumaker, linux-fsdevel, linux-kernel, linux-trace-kernel,
linux-nfs
In-Reply-To: <20250730-nfsd-testing-v4-8-7f5730570a52@kernel.org>
Hi Jeff,
I narrowed down the upstream failure for generic/215 and generic/407
to this commit.
Let's consider first where the kernel is compiled with delegated
attributes off (but it also fails just the same if the delegated
attributes are compiled in).
I don't understand why the code unconditionally changed to call
nfsd4_finalize_deleg_timestamps() which I think the main driver behind
the failure.
Running generic/407 there is an OPEN (which gives out a write
delegation) and returns a change id, then on this filehandle there is
a SETATTR (with a getattr) which returns a new changeid. Then there is
a CLONE where the filehandle is the destination filehandle on which
there is a getattr which returns unchanged changeid/modify time (bad).
Then there is a DELEGRETURN (with a getattr) which again returns same
change id. Test fails.
Prior to this commit. The changeid/modify time is different in CLONE
and DELEGRETURN -- test passes.
Now let me describe what happens with delegated attributes enabled.
OPEN returns delegated attributes delegation, included getattr return
a changeid. Then CLONE is done, the included gettattr returns a
different (from open's) changeid (different time_modify). Then there
is SETATTR+GEATTR+DELEGRETURN compound from the client (which carries
a time_deleg_modify value different from above). Server in getattr
replies with changeid same as in clone and mtime with the value client
provided. So I'm not sure exactly why the test fails here but that's a
different problem as my focus is on "delegation attribute off option"
at the moment.
I don't know if this is the correct fix or not but perhaps we
shouldn't unconditionally be setting this mode? (note this fix only
fixes the delegattributes off. however i have no claims that this
patch is what broke 215/407 for delegated attributes on. Something
else is in play there). If this solution is acceptable, I can send a
patch.
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 81fa7cc6c77b..624cc6ab2802 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -6318,7 +6318,8 @@ nfs4_open_delegation(struct svc_rqst *rqstp,
struct nfsd4_open *open,
dp->dl_ctime = stat.ctime;
dp->dl_mtime = stat.mtime;
spin_lock(&f->f_lock);
- f->f_mode |= FMODE_NOCMTIME;
+ if (deleg_ts)
+ f->f_mode |= FMODE_NOCMTIME;
spin_unlock(&f->f_lock);
trace_nfsd_deleg_write(&dp->dl_stid.sc_stateid);
} else {
On Wed, Jul 30, 2025 at 9:27 AM Jeff Layton <jlayton@kernel.org> wrote:
>
> Instead of allowing the ctime to roll backward with a WRITE_ATTRS
> delegation, set FMODE_NOCMTIME on the file and have it skip mtime and
> ctime updates.
>
> It is possible that the client will never send a SETATTR to set the
> times before returning the delegation. Add two new bools to struct
> nfs4_delegation:
>
> dl_written: tracks whether the file has been written since the
> delegation was granted. This is set in the WRITE and LAYOUTCOMMIT
> handlers.
>
> dl_setattr: tracks whether the client has sent at least one valid
> mtime that can also update the ctime in a SETATTR.
>
> When unlocking the lease for the delegation, clear FMODE_NOCMTIME. If
> the file has been written, but no setattr for the delegated mtime and
> ctime has been done, update the timestamps to current_time().
>
> Suggested-by: NeilBrown <neil@brown.name>
> Signed-off-by: Jeff Layton <jlayton@kernel.org>
> ---
> fs/nfsd/nfs4proc.c | 26 ++++++++++++++++++++++++--
> fs/nfsd/nfs4state.c | 42 ++++++++++++++++++++++++++++++++++++++++++
> fs/nfsd/state.h | 4 +++-
> 3 files changed, 69 insertions(+), 3 deletions(-)
>
> diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
> index aacd912a5fbe29ba5ccac206d13243308f36b7fa..bfebe6e25638a76d3607bb79a239bdc92e42e7b5 100644
> --- a/fs/nfsd/nfs4proc.c
> +++ b/fs/nfsd/nfs4proc.c
> @@ -1151,7 +1151,9 @@ vet_deleg_attrs(struct nfsd4_setattr *setattr, struct nfs4_delegation *dp)
> if (setattr->sa_bmval[2] & FATTR4_WORD2_TIME_DELEG_MODIFY) {
> if (nfsd4_vet_deleg_time(&iattr->ia_mtime, &dp->dl_mtime, &now)) {
> iattr->ia_ctime = iattr->ia_mtime;
> - if (!nfsd4_vet_deleg_time(&iattr->ia_ctime, &dp->dl_ctime, &now))
> + if (nfsd4_vet_deleg_time(&iattr->ia_ctime, &dp->dl_ctime, &now))
> + dp->dl_setattr = true;
> + else
> iattr->ia_valid &= ~(ATTR_CTIME | ATTR_CTIME_SET);
> } else {
> iattr->ia_valid &= ~(ATTR_CTIME | ATTR_CTIME_SET |
> @@ -1238,12 +1240,26 @@ nfsd4_setattr(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
> return status;
> }
>
> +static void nfsd4_file_mark_deleg_written(struct nfs4_file *fi)
> +{
> + spin_lock(&fi->fi_lock);
> + if (!list_empty(&fi->fi_delegations)) {
> + struct nfs4_delegation *dp = list_first_entry(&fi->fi_delegations,
> + struct nfs4_delegation, dl_perfile);
> +
> + if (dp->dl_type == OPEN_DELEGATE_WRITE_ATTRS_DELEG)
> + dp->dl_written = true;
> + }
> + spin_unlock(&fi->fi_lock);
> +}
> +
> static __be32
> nfsd4_write(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
> union nfsd4_op_u *u)
> {
> struct nfsd4_write *write = &u->write;
> stateid_t *stateid = &write->wr_stateid;
> + struct nfs4_stid *stid = NULL;
> struct nfsd_file *nf = NULL;
> __be32 status = nfs_ok;
> unsigned long cnt;
> @@ -1256,10 +1272,15 @@ nfsd4_write(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
> trace_nfsd_write_start(rqstp, &cstate->current_fh,
> write->wr_offset, cnt);
> status = nfs4_preprocess_stateid_op(rqstp, cstate, &cstate->current_fh,
> - stateid, WR_STATE, &nf, NULL);
> + stateid, WR_STATE, &nf, &stid);
> if (status)
> return status;
>
> + if (stid) {
> + nfsd4_file_mark_deleg_written(stid->sc_file);
> + nfs4_put_stid(stid);
> + }
> +
> write->wr_how_written = write->wr_stable_how;
> status = nfsd_vfs_write(rqstp, &cstate->current_fh, nf,
> write->wr_offset, &write->wr_payload,
> @@ -2550,6 +2571,7 @@ nfsd4_layoutcommit(struct svc_rqst *rqstp,
> mutex_unlock(&ls->ls_mutex);
>
> nfserr = ops->proc_layoutcommit(inode, rqstp, lcp);
> + nfsd4_file_mark_deleg_written(ls->ls_stid.sc_file);
> nfs4_put_stid(&ls->ls_stid);
> out:
> return nfserr;
> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> index 205ee8cc6fa2b9f74d08f7938b323d03bdf8286c..81fa7cc6c77b3cdc5ff22bc60ab0654f95dc258d 100644
> --- a/fs/nfsd/nfs4state.c
> +++ b/fs/nfsd/nfs4state.c
> @@ -1222,6 +1222,42 @@ static void put_deleg_file(struct nfs4_file *fp)
> nfs4_file_put_access(fp, NFS4_SHARE_ACCESS_READ);
> }
>
> +static void nfsd4_finalize_deleg_timestamps(struct nfs4_delegation *dp, struct file *f)
> +{
> + struct iattr ia = { .ia_valid = ATTR_ATIME | ATTR_CTIME | ATTR_MTIME };
> + struct inode *inode = file_inode(f);
> + int ret;
> +
> + /* don't do anything if FMODE_NOCMTIME isn't set */
> + if ((READ_ONCE(f->f_mode) & FMODE_NOCMTIME) == 0)
> + return;
> +
> + spin_lock(&f->f_lock);
> + f->f_mode &= ~FMODE_NOCMTIME;
> + spin_unlock(&f->f_lock);
> +
> + /* was it never written? */
> + if (!dp->dl_written)
> + return;
> +
> + /* did it get a setattr for the timestamps at some point? */
> + if (dp->dl_setattr)
> + return;
> +
> + /* Stamp everything to "now" */
> + inode_lock(inode);
> + ret = notify_change(&nop_mnt_idmap, f->f_path.dentry, &ia, NULL);
> + inode_unlock(inode);
> + if (ret) {
> + struct inode *inode = file_inode(f);
> +
> + pr_notice_ratelimited("Unable to update timestamps on inode %02x:%02x:%lu: %d\n",
> + MAJOR(inode->i_sb->s_dev),
> + MINOR(inode->i_sb->s_dev),
> + inode->i_ino, ret);
> + }
> +}
> +
> static void nfs4_unlock_deleg_lease(struct nfs4_delegation *dp)
> {
> struct nfs4_file *fp = dp->dl_stid.sc_file;
> @@ -1229,6 +1265,7 @@ static void nfs4_unlock_deleg_lease(struct nfs4_delegation *dp)
>
> WARN_ON_ONCE(!fp->fi_delegees);
>
> + nfsd4_finalize_deleg_timestamps(dp, nf->nf_file);
> kernel_setlease(nf->nf_file, F_UNLCK, NULL, (void **)&dp);
> put_deleg_file(fp);
> }
> @@ -6265,6 +6302,8 @@ nfs4_open_delegation(struct svc_rqst *rqstp, struct nfsd4_open *open,
> memcpy(&open->op_delegate_stateid, &dp->dl_stid.sc_stateid, sizeof(dp->dl_stid.sc_stateid));
>
> if (open->op_share_access & NFS4_SHARE_ACCESS_WRITE) {
> + struct file *f = dp->dl_stid.sc_file->fi_deleg_file->nf_file;
> +
> if (!nfsd4_add_rdaccess_to_wrdeleg(rqstp, open, fh, stp) ||
> !nfs4_delegation_stat(dp, currentfh, &stat)) {
> nfs4_put_stid(&dp->dl_stid);
> @@ -6278,6 +6317,9 @@ nfs4_open_delegation(struct svc_rqst *rqstp, struct nfsd4_open *open,
> dp->dl_atime = stat.atime;
> dp->dl_ctime = stat.ctime;
> dp->dl_mtime = stat.mtime;
> + spin_lock(&f->f_lock);
> + f->f_mode |= FMODE_NOCMTIME;
> + spin_unlock(&f->f_lock);
> trace_nfsd_deleg_write(&dp->dl_stid.sc_stateid);
> } else {
> open->op_delegate_type = deleg_ts && nfs4_delegation_stat(dp, currentfh, &stat) ?
> diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h
> index bf9436cdb93c5dd5502ecf83433ea311e3678711..b6ac0f37e9cdfcfddde5861c8c0c51bad60101b7 100644
> --- a/fs/nfsd/state.h
> +++ b/fs/nfsd/state.h
> @@ -217,10 +217,12 @@ struct nfs4_delegation {
> struct nfs4_clnt_odstate *dl_clnt_odstate;
> time64_t dl_time;
> u32 dl_type;
> -/* For recall: */
> + /* For recall: */
> int dl_retries;
> struct nfsd4_callback dl_recall;
> bool dl_recalled;
> + bool dl_written;
> + bool dl_setattr;
>
> /* for CB_GETATTR */
> struct nfs4_cb_fattr dl_cb_fattr;
>
> --
> 2.50.1
>
>
^ permalink raw reply related
* Re: [PATCH bpf-next v3 3/6] bpf: fix the usage of BPF_TRAMP_F_SKIP_FRAME
From: Menglong Dong @ 2025-12-19 14:54 UTC (permalink / raw)
To: Andreas Schwab
Cc: Menglong Dong, ast, rostedt, daniel, john.fastabend, andrii,
martin.lau, eddyz87, song, yonghong.song, kpsingh, sdf, haoluo,
jolsa, mhiramat, mark.rutland, mathieu.desnoyers, jiang.biao, bpf,
linux-kernel, linux-trace-kernel, linux-riscv
In-Reply-To: <878qeyedaw.fsf@igel.home>
On Fri, Dec 19, 2025 at 10:50 PM Andreas Schwab <schwab@linux-m68k.org> wrote:
>
> On Dez 19 2025, Menglong Dong wrote:
>
> > @@ -1171,6 +1167,8 @@ static int __arch_prepare_bpf_trampoline(struct
> > bpf_tramp_image *im,
> > }
> >
> > if (flags & BPF_TRAMP_F_CALL_ORIG) {
> > + /* skip to actual body of traced function */
> > + orig_call += RV_FENTRY_NINSNS * 4;
>
> Before this line, orig_call still contains the same value as func_addr,
> with the latter being dead, so there is not much point in using a copy.
Yeah, we can use "func_addr + RV_FENTRY_NINSNS * 4" here directly.
>
> --
> Andreas Schwab, schwab@linux-m68k.org
> GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510 2552 DF73 E780 A9DA AEC1
> "And now for something completely different."
^ permalink raw reply
* Re: [PATCH bpf-next v3 3/6] bpf: fix the usage of BPF_TRAMP_F_SKIP_FRAME
From: Andreas Schwab @ 2025-12-19 14:50 UTC (permalink / raw)
To: Menglong Dong
Cc: Menglong Dong, ast, rostedt, daniel, john.fastabend, andrii,
martin.lau, eddyz87, song, yonghong.song, kpsingh, sdf, haoluo,
jolsa, mhiramat, mark.rutland, mathieu.desnoyers, jiang.biao, bpf,
linux-kernel, linux-trace-kernel, linux-riscv
In-Reply-To: <CADxym3Y098836fHHRSjeryxCp=CPB8sDU19TBBVs07VZOERJXw@mail.gmail.com>
On Dez 19 2025, Menglong Dong wrote:
> @@ -1171,6 +1167,8 @@ static int __arch_prepare_bpf_trampoline(struct
> bpf_tramp_image *im,
> }
>
> if (flags & BPF_TRAMP_F_CALL_ORIG) {
> + /* skip to actual body of traced function */
> + orig_call += RV_FENTRY_NINSNS * 4;
Before this line, orig_call still contains the same value as func_addr,
with the latter being dead, so there is not much point in using a copy.
--
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510 2552 DF73 E780 A9DA AEC1
"And now for something completely different."
^ permalink raw reply
* Re: [PATCH bpf-next v3 3/6] bpf: fix the usage of BPF_TRAMP_F_SKIP_FRAME
From: Menglong Dong @ 2025-12-19 14:04 UTC (permalink / raw)
To: Andreas Schwab
Cc: Menglong Dong, ast, rostedt, daniel, john.fastabend, andrii,
martin.lau, eddyz87, song, yonghong.song, kpsingh, sdf, haoluo,
jolsa, mhiramat, mark.rutland, mathieu.desnoyers, jiang.biao, bpf,
linux-kernel, linux-trace-kernel, linux-riscv
In-Reply-To: <87cy4aeg56.fsf@igel.home>
On Fri, Dec 19, 2025 at 9:49 PM Andreas Schwab <schwab@linux-m68k.org> wrote:
>
> On Dez 19 2025, Menglong Dong wrote:
>
> > diff --git a/arch/riscv/net/bpf_jit_comp64.c b/arch/riscv/net/bpf_jit_comp64.c
> > index 5f9457e910e8..09b70bf362d3 100644
> > --- a/arch/riscv/net/bpf_jit_comp64.c
> > +++ b/arch/riscv/net/bpf_jit_comp64.c
> > @@ -1134,7 +1134,7 @@ static int __arch_prepare_bpf_trampoline(struct bpf_tramp_image *im,
> > store_args(nr_arg_slots, args_off, ctx);
> >
> > /* skip to actual body of traced function */
> > - if (flags & BPF_TRAMP_F_ORIG_STACK)
> > + if (flags & BPF_TRAMP_F_CALL_ORIG)
> > orig_call += RV_FENTRY_NINSNS * 4;
>
> There are now three occurrences of that condition, and only the third
> one uses orig_call. How about merging them?
Yeah, I think we can merge it to the third one, like this:
diff --git a/arch/riscv/net/bpf_jit_comp64.c b/arch/riscv/net/bpf_jit_comp64.c
index 5f9457e910e8..37888abee70c 100644
--- a/arch/riscv/net/bpf_jit_comp64.c
+++ b/arch/riscv/net/bpf_jit_comp64.c
@@ -1133,10 +1133,6 @@ static int __arch_prepare_bpf_trampoline(struct
bpf_tramp_image *im,
store_args(nr_arg_slots, args_off, ctx);
- /* skip to actual body of traced function */
- if (flags & BPF_TRAMP_F_ORIG_STACK)
- orig_call += RV_FENTRY_NINSNS * 4;
-
if (flags & BPF_TRAMP_F_CALL_ORIG) {
emit_imm(RV_REG_A0, ctx->insns ? (const s64)im :
RV_MAX_COUNT_IMM, ctx);
ret = emit_call((const u64)__bpf_tramp_enter, true, ctx);
@@ -1171,6 +1167,8 @@ static int __arch_prepare_bpf_trampoline(struct
bpf_tramp_image *im,
}
if (flags & BPF_TRAMP_F_CALL_ORIG) {
+ /* skip to actual body of traced function */
+ orig_call += RV_FENTRY_NINSNS * 4;
restore_args(min_t(int, nr_arg_slots,
RV_MAX_REG_ARGS), args_off, ctx);
restore_stack_args(nr_arg_slots - RV_MAX_REG_ARGS,
args_off, stk_arg_off, ctx);
ret = emit_call((const u64)orig_call, true, ctx);
>
> --
> Andreas Schwab, schwab@linux-m68k.org
> GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510 2552 DF73 E780 A9DA AEC1
> "And now for something completely different."
^ permalink raw reply related
* Re: [PATCH bpf-next v3 3/6] bpf: fix the usage of BPF_TRAMP_F_SKIP_FRAME
From: Andreas Schwab @ 2025-12-19 13:48 UTC (permalink / raw)
To: Menglong Dong
Cc: ast, Menglong Dong, rostedt, daniel, john.fastabend, andrii,
martin.lau, eddyz87, song, yonghong.song, kpsingh, sdf, haoluo,
jolsa, mhiramat, mark.rutland, mathieu.desnoyers, jiang.biao, bpf,
linux-kernel, linux-trace-kernel, linux-riscv
In-Reply-To: <1948844.tdWV9SEqCh@7950hx>
On Dez 19 2025, Menglong Dong wrote:
> diff --git a/arch/riscv/net/bpf_jit_comp64.c b/arch/riscv/net/bpf_jit_comp64.c
> index 5f9457e910e8..09b70bf362d3 100644
> --- a/arch/riscv/net/bpf_jit_comp64.c
> +++ b/arch/riscv/net/bpf_jit_comp64.c
> @@ -1134,7 +1134,7 @@ static int __arch_prepare_bpf_trampoline(struct bpf_tramp_image *im,
> store_args(nr_arg_slots, args_off, ctx);
>
> /* skip to actual body of traced function */
> - if (flags & BPF_TRAMP_F_ORIG_STACK)
> + if (flags & BPF_TRAMP_F_CALL_ORIG)
> orig_call += RV_FENTRY_NINSNS * 4;
There are now three occurrences of that condition, and only the third
one uses orig_call. How about merging them?
--
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510 2552 DF73 E780 A9DA AEC1
"And now for something completely different."
^ permalink raw reply
* Re: [PATCH bpf-next v3 3/6] bpf: fix the usage of BPF_TRAMP_F_SKIP_FRAME
From: Menglong Dong @ 2025-12-19 13:31 UTC (permalink / raw)
To: ast
Cc: Andreas Schwab, Menglong Dong, rostedt, daniel, john.fastabend,
andrii, martin.lau, eddyz87, song, yonghong.song, kpsingh, sdf,
haoluo, jolsa, mhiramat, mark.rutland, mathieu.desnoyers,
jiang.biao, bpf, linux-kernel, linux-trace-kernel, linux-riscv
In-Reply-To: <5070743.31r3eYUQgx@7950hx>
On 2025/12/19 20:27, Menglong Dong wrote:
> On 2025/12/19 19:41, Andreas Schwab wrote:
> > On Dez 19 2025, Menglong Dong wrote:
> >
> > > BPF_TRAMP_F_ORIG_STACK
> >
> > How can that ever be set?
>
> Oops, my bad! It should be BPF_TRAMP_F_CALL_ORIG here. I think
> it is some kind of copy-paste mistake. I'll send a fix for it.
I sent the following patch twice, but I didn't see it in the
mail list. I suspect there is something wrong with my gmail.
Hi, Alexei. Can you see my patch?
-->patch<--
From 5dbae5dcba3aa7fa10e506e9fd1a28a6802d9b00 Mon Sep 17 00:00:00 2001
From: Menglong Dong <dongml2@chinatelecom.cn>
Date: Fri, 19 Dec 2025 20:33:10 +0800
Subject: [PATCH RESEND bpf] riscv, bpf: fix incorrect usage of BPF_TRAMP_F_ORIG_STACK
The usage of BPF_TRAMP_F_ORIG_STACK in __arch_prepare_bpf_trampoline() is
wrong, and it should be BPF_TRAMP_F_CALL_ORIG, which caused crash as
Andreas reported:
Insufficient stack space to handle exception!
Task stack: [0xff20000000010000..0xff20000000014000]
Overflow stack: [0xff600000ffdad070..0xff600000ffdae070]
CPU: 1 UID: 0 PID: 1 Comm: systemd Not tainted 6.18.0-rc5+ #15 PREEMPT(voluntary)
Hardware name: riscv-virtio qemu/qemu, BIOS 2025.10 10/01/2025
epc : copy_from_kernel_nofault+0xa/0x198
ra : bpf_probe_read_kernel+0x20/0x60
epc : ffffffff802b732a ra : ffffffff801e6070 sp : ff2000000000ffe0
gp : ffffffff82262ed0 tp : 0000000000000000 t0 : ffffffff80022320
t1 : ffffffff801e6056 t2 : 0000000000000000 s0 : ff20000000010040
s1 : 0000000000000008 a0 : ff20000000010050 a1 : ff60000083b3d320
a2 : 0000000000000008 a3 : 0000000000000097 a4 : 0000000000000000
a5 : 0000000000000000 a6 : 0000000000000021 a7 : 0000000000000003
s2 : ff20000000010050 s3 : ff6000008459fc18 s4 : ff60000083b3d340
s5 : ff20000000010060 s6 : 0000000000000000 s7 : ff20000000013aa8
s8 : 0000000000000000 s9 : 0000000000008000 s10: 000000000058dcb0
s11: 000000000058dca7 t3 : 000000006925116d t4 : ff6000008090f026
t5 : 00007fff9b0cbaa8 t6 : 0000000000000016
status: 0000000200000120 badaddr: 0000000000000000 cause: 8000000000000005
Kernel panic - not syncing: Kernel stack overflow
CPU: 1 UID: 0 PID: 1 Comm: systemd Not tainted 6.18.0-rc5+ #15 PREEMPT(voluntary)
Hardware name: riscv-virtio qemu/qemu, BIOS 2025.10 10/01/2025
Call Trace:
[<ffffffff8001a1f8>] dump_backtrace+0x28/0x38
[<ffffffff80002502>] show_stack+0x3a/0x50
[<ffffffff800122be>] dump_stack_lvl+0x56/0x80
[<ffffffff80012300>] dump_stack+0x18/0x22
[<ffffffff80002abe>] vpanic+0xf6/0x328
[<ffffffff80002d2e>] panic+0x3e/0x40
[<ffffffff80019ef0>] handle_bad_stack+0x98/0xa0
[<ffffffff801e6070>] bpf_probe_read_kernel+0x20/0x60
Just fix it.
Fixes: 47c9214dcbea ("bpf: fix the usage of BPF_TRAMP_F_SKIP_FRAME")
Reported-by: Andreas Schwab <schwab@linux-m68k.org>
Closes: https://lore.kernel.org/bpf/874ipnkfvt.fsf@igel.home/
Signed-off-by: Menglong Dong <dongml2@chinatelecom.cn>
---
arch/riscv/net/bpf_jit_comp64.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/riscv/net/bpf_jit_comp64.c b/arch/riscv/net/bpf_jit_comp64.c
index 5f9457e910e8..09b70bf362d3 100644
--- a/arch/riscv/net/bpf_jit_comp64.c
+++ b/arch/riscv/net/bpf_jit_comp64.c
@@ -1134,7 +1134,7 @@ static int __arch_prepare_bpf_trampoline(struct bpf_tramp_image *im,
store_args(nr_arg_slots, args_off, ctx);
/* skip to actual body of traced function */
- if (flags & BPF_TRAMP_F_ORIG_STACK)
+ if (flags & BPF_TRAMP_F_CALL_ORIG)
orig_call += RV_FENTRY_NINSNS * 4;
if (flags & BPF_TRAMP_F_CALL_ORIG) {
--
2.52.0
--<patch>--
>
> Thanks!
> Menglong Dong
>
> >
> > if (flags & (BPF_TRAMP_F_ORIG_STACK | BPF_TRAMP_F_SHARE_IPMODIFY))
> > return -ENOTSUPP;
> >
> >
>
>
>
>
>
>
^ permalink raw reply related
* Re: [PATCH bpf-next v3 3/6] bpf: fix the usage of BPF_TRAMP_F_SKIP_FRAME
From: Menglong Dong @ 2025-12-19 12:27 UTC (permalink / raw)
To: Andreas Schwab
Cc: Menglong Dong, ast, rostedt, daniel, john.fastabend, andrii,
martin.lau, eddyz87, song, yonghong.song, kpsingh, sdf, haoluo,
jolsa, mhiramat, mark.rutland, mathieu.desnoyers, jiang.biao, bpf,
linux-kernel, linux-trace-kernel, linux-riscv
In-Reply-To: <875xa2g0m0.fsf@igel.home>
On 2025/12/19 19:41, Andreas Schwab wrote:
> On Dez 19 2025, Menglong Dong wrote:
>
> > BPF_TRAMP_F_ORIG_STACK
>
> How can that ever be set?
Oops, my bad! It should be BPF_TRAMP_F_CALL_ORIG here. I think
it is some kind of copy-paste mistake. I'll send a fix for it.
Thanks!
Menglong Dong
>
> if (flags & (BPF_TRAMP_F_ORIG_STACK | BPF_TRAMP_F_SHARE_IPMODIFY))
> return -ENOTSUPP;
>
>
^ permalink raw reply
* Re: [PATCH 1/3] mm/khugepaged: remove unnecessary goto 'skip' label
From: Garg, Shivank @ 2025-12-19 12:20 UTC (permalink / raw)
To: Zi Yan
Cc: Andrew Morton, David Hildenbrand, Lorenzo Stoakes, Baolin Wang,
Liam R . Howlett, Nico Pache, Ryan Roberts, Dev Jain, Barry Song,
Lance Yang, linux-mm, linux-kernel, linux-trace-kernel
In-Reply-To: <848E8F85-CD24-491C-8A3B-474E4FCBF995@nvidia.com>
On 12/16/2025 8:57 PM, Zi Yan wrote:
> On 16 Dec 2025, at 6:11, Shivank Garg wrote:
>
>> Replace 'goto skip' with actual logic for better code readability.
>>
>> No functional change.
>>
>> Signed-off-by: Shivank Garg <shivankg@amd.com>
>> ---
>> mm/khugepaged.c | 7 ++++---
>> 1 file changed, 4 insertions(+), 3 deletions(-)
>>
> LGTM. Reviewed-by: Zi Yan <ziy@nvidia.com>
>
Hi Zi,
I noticed that b4 didn't pick up your Reviewed-by tag automatically (possibly because
it was on the same line as "LGTM."). Just thought I'd mention it in case it's helpful,
Since maintainers may be using b4 or similar tools and your tags might occasionally
get missed.
Of course, I'll make sure to add your tag manually when sending v2.
Thanks,
Shivank
^ permalink raw reply
* Re: [PATCH bpf-next v3 3/6] bpf: fix the usage of BPF_TRAMP_F_SKIP_FRAME
From: Andreas Schwab @ 2025-12-19 11:41 UTC (permalink / raw)
To: Menglong Dong
Cc: Menglong Dong, ast, rostedt, daniel, john.fastabend, andrii,
martin.lau, eddyz87, song, yonghong.song, kpsingh, sdf, haoluo,
jolsa, mhiramat, mark.rutland, mathieu.desnoyers, jiang.biao, bpf,
linux-kernel, linux-trace-kernel, linux-riscv
In-Reply-To: <3730454.R56niFO833@7940hx>
On Dez 19 2025, Menglong Dong wrote:
> BPF_TRAMP_F_ORIG_STACK
How can that ever be set?
if (flags & (BPF_TRAMP_F_ORIG_STACK | BPF_TRAMP_F_SHARE_IPMODIFY))
return -ENOTSUPP;
--
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510 2552 DF73 E780 A9DA AEC1
"And now for something completely different."
^ permalink raw reply
* Re: [PATCH 2/3] mm/khugepaged: use enum scan_result for result variables
From: Garg, Shivank @ 2025-12-19 9:45 UTC (permalink / raw)
To: Zi Yan, David Hildenbrand (Red Hat)
Cc: Andrew Morton, Lorenzo Stoakes, Baolin Wang, Liam R . Howlett,
Nico Pache, Ryan Roberts, Dev Jain, Barry Song, Lance Yang,
linux-mm, linux-kernel, linux-trace-kernel
In-Reply-To: <B6AB9DAB-7985-44F0-A0D0-9FE77D3853B5@nvidia.com>
On 12/18/2025 9:24 PM, Zi Yan wrote:
> On 18 Dec 2025, at 4:17, David Hildenbrand (Red Hat) wrote:
>
>> On 12/18/25 06:40, Garg, Shivank wrote:
>>>
>>>
>>> On 12/16/2025 9:08 PM, Zi Yan wrote:
>>>> On 16 Dec 2025, at 6:11, Shivank Garg wrote:
>>>>
>>>
>>>>
>>>>> int collapse_pte_mapped_thp(struct mm_struct *mm, unsigned long addr,
>>>>> bool install_pmd)
>>>>> {
>>>>
>>>> And here.
>>> Since this function is declared in khugepaged.h, I need to
>>> move the enum definition to that header. I see two options for handling
>>> the CONFIG_TRANSPARENT_HUGEPAGE check:
>>>
>>> 1. Define enum OUTSIDE the ifdef: This allows the static inline stub
>>> to also return enum scan_result, keeping the API consistent.
>>> 2. Define enum INSIDE the ifdef: The enum is hidden when THP is disabled,
>>> forcing the stub to return int 0 instead.
>>>
>>> The only external caller (uprobes.c) of collapse_pte_mapped_thp currently
>>> ignores the return value.
>>
>> Probably best to not expose that enum (especially when nobody cares ...) and instead expose a new void function for uprobe purposes.
>>
>> Maybe
>>
>> void collapse_pte_mapped_thp(...)
>> {
>> try_collapse_pte_mapped_thp();
>> }
>>
>> Maybe something like that?
>
> Sounds good to me.
Thanks, this makes sense to me.
I'll do this.
Best Regards,
Shivank
^ permalink raw reply
* Re: [PATCH v10 2/3] mm: Fix OOM killer inaccuracy on large many-core systems
From: Mark Brown @ 2025-12-19 9:31 UTC (permalink / raw)
To: Mathieu Desnoyers
Cc: Thomas Gleixner, Andrew Morton, linux-kernel, Paul E. McKenney,
Steven Rostedt, Masami Hiramatsu, Dennis Zhou, Tejun Heo,
Christoph Lameter, Martin Liu, David Rientjes, christian.koenig,
Shakeel Butt, SeongJae Park, Michal Hocko, Johannes Weiner,
Sweet Tea Dorminy, Lorenzo Stoakes, Liam R . Howlett,
Mike Rapoport, Suren Baghdasaryan, Vlastimil Babka,
Christian Brauner, Wei Yang, David Hildenbrand, Miaohe Lin,
Al Viro, linux-mm, linux-trace-kernel, Yu Zhao, Roman Gushchin,
Mateusz Guzik, Matthew Wilcox, Baolin Wang, Aboorva Devarajan,
Aishwarya TCV
In-Reply-To: <72ad1fed-574e-4850-beac-91626e5aaa0e@efficios.com>
[-- Attachment #1: Type: text/plain, Size: 517 bytes --]
On Thu, Dec 18, 2025 at 05:18:04PM -0500, Mathieu Desnoyers wrote:
> On 2025-12-18 13:00, Mark Brown wrote:
> An ugly work-around that may work (and then we can improve on this),
> at the end of mm/init-mm.c:init_mm (completely untested):
> .cpu_bitmap = { [0 ... ((3*BITS_TO_LONGS(NR_CPUS))-1 + ((69905 * NR_MM_COUNTERS * 64) / BYTES_PER_LONG))] = 0UL },
That doesn't seem to fix the FVP unfortunately (BYTES_PER_LONG doesn't
exist, but even just deleting the division entirely fails in the same
way).
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [PATCHv5 bpf-next 9/9] bpf,x86: Use single ftrace_ops for direct calls
From: Jiri Olsa @ 2025-12-19 9:27 UTC (permalink / raw)
To: Steven Rostedt
Cc: Steven Rostedt, Florent Revest, Mark Rutland, bpf, linux-kernel,
linux-trace-kernel, linux-arm-kernel, Alexei Starovoitov,
Daniel Borkmann, Andrii Nakryiko, Menglong Dong, Song Liu
In-Reply-To: <20251218112608.11a14a4a@gandalf.local.home>
On Thu, Dec 18, 2025 at 11:26:08AM -0500, Steven Rostedt wrote:
> On Mon, 15 Dec 2025 22:14:02 +0100
> Jiri Olsa <jolsa@kernel.org> wrote:
>
> > Using single ftrace_ops for direct calls update instead of allocating
> > ftrace_ops object for each trampoline.
> >
> > With single ftrace_ops object we can use update_ftrace_direct_* api
> > that allows multiple ip sites updates on single ftrace_ops object.
> >
> > Adding HAVE_SINGLE_FTRACE_DIRECT_OPS config option to be enabled on
> > each arch that supports this.
> >
> > At the moment we can enable this only on x86 arch, because arm relies
> > on ftrace_ops object representing just single trampoline image (stored
> > in ftrace_ops::direct_call). Ach that do not support this will continue
>
> My back "Ach" and doesn't support me well. ;-)
heh, should have been 'Archs' ;-)
>
> > to use *_ftrace_direct api.
> >
> > Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> > ---
> > arch/x86/Kconfig | 1 +
> > kernel/bpf/trampoline.c | 195 ++++++++++++++++++++++++++++++++++------
> > kernel/trace/Kconfig | 3 +
> > kernel/trace/ftrace.c | 7 +-
> > 4 files changed, 177 insertions(+), 29 deletions(-)
> >
> > diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> > index 17a107cc5244..d0c36e49e66e 100644
> > --- a/arch/x86/Kconfig
> > +++ b/arch/x86/Kconfig
> > @@ -335,6 +335,7 @@ config X86
> > select SCHED_SMT if SMP
> > select ARCH_SUPPORTS_SCHED_CLUSTER if SMP
> > select ARCH_SUPPORTS_SCHED_MC if SMP
> > + select HAVE_SINGLE_FTRACE_DIRECT_OPS if X86_64 && DYNAMIC_FTRACE_WITH_DIRECT_CALLS
>
> You can remove the "&& DYNAMIC_FTRACE_WITH_DIRECT_CALLS" part by having the
> config depend on it (see below).
...
>
> >
> > config INSTRUCTION_DECODER
> > def_bool y
> > diff --git a/kernel/bpf/trampoline.c b/kernel/bpf/trampoline.c
> > index 17af2aad8382..02371db3db3e 100644
> > --- a/kernel/bpf/trampoline.c
> > +++ b/kernel/bpf/trampoline.c
> > @@ -33,12 +33,40 @@ static DEFINE_MUTEX(trampoline_mutex);
> > #ifdef CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS
> > static int bpf_trampoline_update(struct bpf_trampoline *tr, bool lock_direct_mutex);
> >
> > +#ifdef CONFIG_HAVE_SINGLE_FTRACE_DIRECT_OPS
>
> Make this:
>
> #ifdef CONFIG_SINGLE_FTRACE_DIRECT_OPS
>
> for the suggested modification in the Kconfig below.
>
> > +static struct bpf_trampoline *direct_ops_ip_lookup(struct ftrace_ops *ops, unsigned long ip)
> > +{
> > + struct hlist_head *head_ip;
> > + struct bpf_trampoline *tr;
> > +
> > + mutex_lock(&trampoline_mutex);
>
> guard(mutex)(&trampoline_mutex);
right, will change
>
> > + head_ip = &trampoline_ip_table[hash_64(ip, TRAMPOLINE_HASH_BITS)];
> > + hlist_for_each_entry(tr, head_ip, hlist_ip) {
> > + if (tr->ip == ip)
>
> return NULL;
>
> > + goto out;
> > + }
>
>
> > + tr = NULL;
> > +out:
> > + mutex_unlock(&trampoline_mutex);
>
> No need for the above
yep
>
> > + return tr;
> > +}
> > +#else
> > +static struct bpf_trampoline *direct_ops_ip_lookup(struct ftrace_ops *ops, unsigned long ip)
> > +{
> > + return ops->private;
> > +}
> > +#endif /* CONFIG_HAVE_SINGLE_FTRACE_DIRECT_OPS */
> > +
> > static int bpf_tramp_ftrace_ops_func(struct ftrace_ops *ops, unsigned long ip,
> > enum ftrace_ops_cmd cmd)
> > {
> > - struct bpf_trampoline *tr = ops->private;
> > + struct bpf_trampoline *tr;
> > int ret = 0;
> >
> > + tr = direct_ops_ip_lookup(ops, ip);
> > + if (!tr)
> > + return -EINVAL;
> > +
> > if (cmd == FTRACE_OPS_CMD_ENABLE_SHARE_IPMODIFY_SELF) {
> > /* This is called inside register_ftrace_direct_multi(), so
> > * tr->mutex is already locked.
> > @@ -137,6 +165,139 @@ void bpf_image_ksym_del(struct bpf_ksym *ksym)
> > PAGE_SIZE, true, ksym->name);
> > }
> >
> > +#ifdef CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS
> > +#ifdef CONFIG_HAVE_SINGLE_FTRACE_DIRECT_OPS
>
> Replace the above two with:
>
> #ifdef CONFIG_SINGLE_FTRACE_DIRECT_OPS
...
>
> > +/*
> > + * We have only single direct_ops which contains all the direct call
> > + * sites and is the only global ftrace_ops for all trampolines.
> > + *
> > + * We use 'update_ftrace_direct_*' api for attachment.
> > + */
> > +struct ftrace_ops direct_ops = {
> > + .ops_func = bpf_tramp_ftrace_ops_func,
> > +};
> > +
> > +static int direct_ops_alloc(struct bpf_trampoline *tr)
> > +{
> > + tr->fops = &direct_ops;
> > + return 0;
> > +}
> > +
> > +static void direct_ops_free(struct bpf_trampoline *tr) { }
> > +
> > +static struct ftrace_hash *hash_from_ip(struct bpf_trampoline *tr, void *ptr)
> > +{
> > + unsigned long ip, addr = (unsigned long) ptr;
> > + struct ftrace_hash *hash;
> > +
> > + ip = ftrace_location(tr->ip);
> > + if (!ip)
> > + return NULL;
> > + hash = alloc_ftrace_hash(FTRACE_HASH_DEFAULT_BITS);
> > + if (!hash)
> > + return NULL;
> > + if (bpf_trampoline_use_jmp(tr->flags))
> > + addr = ftrace_jmp_set(addr);
> > + if (!add_hash_entry_direct(hash, ip, addr)) {
> > + free_ftrace_hash(hash);
> > + return NULL;
> > + }
> > + return hash;
> > +}
> > +
> > +static int direct_ops_add(struct bpf_trampoline *tr, void *addr)
> > +{
> > + struct ftrace_hash *hash = hash_from_ip(tr, addr);
> > + int err = -ENOMEM;
> > +
> > + if (hash)
> > + err = update_ftrace_direct_add(tr->fops, hash);
> > + free_ftrace_hash(hash);
> > + return err;
> > +}
>
> I think these functions would be cleaner as:
>
> {
> struct ftrace_hash *hash = hash_from_ip(tr, addr);
> int err;
>
> if (!hash)
> return -ENOMEM;
>
> err = update_ftrace_direct_*(tr->fops, hash);
> free_ftrace_hash(hash);
> return err;
> }
np, will change
>
>
> > +
> > +static int direct_ops_del(struct bpf_trampoline *tr, void *addr)
> > +{
> > + struct ftrace_hash *hash = hash_from_ip(tr, addr);
> > + int err = -ENOMEM;
> > +
> > + if (hash)
> > + err = update_ftrace_direct_del(tr->fops, hash);
> > + free_ftrace_hash(hash);
> > + return err;
> > +}
> > +
> > +static int direct_ops_mod(struct bpf_trampoline *tr, void *addr, bool lock_direct_mutex)
> > +{
> > + struct ftrace_hash *hash = hash_from_ip(tr, addr);
> > + int err = -ENOMEM;
> > +
> > + if (hash)
> > + err = update_ftrace_direct_mod(tr->fops, hash, lock_direct_mutex);
> > + free_ftrace_hash(hash);
> > + return err;
> > +}
> > +#else
> > +/*
> > + * We allocate ftrace_ops object for each trampoline and it contains
> > + * call site specific for that trampoline.
> > + *
> > + * We use *_ftrace_direct api for attachment.
> > + */
> > +static int direct_ops_alloc(struct bpf_trampoline *tr)
> > +{
> > + tr->fops = kzalloc(sizeof(struct ftrace_ops), GFP_KERNEL);
> > + if (!tr->fops)
> > + return -ENOMEM;
> > + tr->fops->private = tr;
> > + tr->fops->ops_func = bpf_tramp_ftrace_ops_func;
> > + return 0;
> > +}
> > +
> > +static void direct_ops_free(struct bpf_trampoline *tr)
> > +{
> > + if (tr->fops) {
> > + ftrace_free_filter(tr->fops);
> > + kfree(tr->fops);
> > + }
> > +}
>
> Why not:
>
> static void direct_ops_free(struct bpf_trampoline *tr)
> {
> if (!tr->fops)
> return;
>
> ftrace_free_filter(tr->fops);
> kfree(tr->fops);
> }
same pattern like above, ok
>
> ?
>
> > +
> > +static int direct_ops_add(struct bpf_trampoline *tr, void *ptr)
> > +{
> > + unsigned long addr = (unsigned long) ptr;
> > + struct ftrace_ops *ops = tr->fops;
> > + int ret;
> > +
> > + if (bpf_trampoline_use_jmp(tr->flags))
> > + addr = ftrace_jmp_set(addr);
> > +
> > + ret = ftrace_set_filter_ip(ops, tr->ip, 0, 1);
> > + if (ret)
> > + return ret;
> > + return register_ftrace_direct(ops, addr);
> > +}
> > +
> > +static int direct_ops_del(struct bpf_trampoline *tr, void *addr)
> > +{
> > + return unregister_ftrace_direct(tr->fops, (long)addr, false);
> > +}
> > +
> > +static int direct_ops_mod(struct bpf_trampoline *tr, void *ptr, bool lock_direct_mutex)
> > +{
> > + unsigned long addr = (unsigned long) ptr;
> > + struct ftrace_ops *ops = tr->fops;
> > +
> > + if (bpf_trampoline_use_jmp(tr->flags))
> > + addr = ftrace_jmp_set(addr);
> > + if (lock_direct_mutex)
> > + return modify_ftrace_direct(ops, addr);
> > + return modify_ftrace_direct_nolock(ops, addr);
> > +}
> > +#endif /* CONFIG_HAVE_SINGLE_FTRACE_DIRECT_OPS */
> > +#else
> > +static void direct_ops_free(struct bpf_trampoline *tr) { }
>
> This is somewhat inconsistent with direct_ops_alloc() that has:
>
> #ifdef CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS
> if (direct_ops_alloc(tr)) {
> kfree(tr);
> tr = NULL;
> goto out;
> }
> #endif
>
> Now, if you wrap the direct_ops_free() too, we can remove the
> #ifdef CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS
> part with my kconfig suggestion. Otherwise keep the kconfig as is, but I
> would add a stub function for direct_ops_alloc() too.
ah right.. I think let's do the kconfig change you suggest to make
this simpler
>
> > +#endif /* CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS */
> > +
> > static struct bpf_trampoline *bpf_trampoline_lookup(u64 key, unsigned long ip)
> > {
> > struct bpf_trampoline *tr;
> > @@ -155,14 +316,11 @@ static struct bpf_trampoline *bpf_trampoline_lookup(u64 key, unsigned long ip)
> > if (!tr)
> > goto out;
> > #ifdef CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS
> > - tr->fops = kzalloc(sizeof(struct ftrace_ops), GFP_KERNEL);
> > - if (!tr->fops) {
> > + if (direct_ops_alloc(tr)) {
> > kfree(tr);
> > tr = NULL;
> > goto out;
> > }
> > - tr->fops->private = tr;
> > - tr->fops->ops_func = bpf_tramp_ftrace_ops_func;
> > #endif
> >
> > tr->key = key;
> > @@ -206,7 +364,7 @@ static int unregister_fentry(struct bpf_trampoline *tr, u32 orig_flags,
> > int ret;
> >
> > if (tr->func.ftrace_managed)
> > - ret = unregister_ftrace_direct(tr->fops, (long)old_addr, false);
> > + ret = direct_ops_del(tr, old_addr);
>
> Doesn't this need a wrapper too?
yep
>
> > else
> > ret = bpf_trampoline_update_fentry(tr, orig_flags, old_addr, NULL);
> >
> > @@ -220,15 +378,7 @@ static int modify_fentry(struct bpf_trampoline *tr, u32 orig_flags,
> > int ret;
> >
> > if (tr->func.ftrace_managed) {
> > - unsigned long addr = (unsigned long) new_addr;
> > -
> > - if (bpf_trampoline_use_jmp(tr->flags))
> > - addr = ftrace_jmp_set(addr);
> > -
> > - if (lock_direct_mutex)
> > - ret = modify_ftrace_direct(tr->fops, addr);
> > - else
> > - ret = modify_ftrace_direct_nolock(tr->fops, addr);
> > + ret = direct_ops_mod(tr, new_addr, lock_direct_mutex);
>
> and this.
>
> > } else {
> > ret = bpf_trampoline_update_fentry(tr, orig_flags, old_addr,
> > new_addr);
> > @@ -251,15 +401,7 @@ static int register_fentry(struct bpf_trampoline *tr, void *new_addr)
> > }
> >
> > if (tr->func.ftrace_managed) {
> > - unsigned long addr = (unsigned long) new_addr;
> > -
> > - if (bpf_trampoline_use_jmp(tr->flags))
> > - addr = ftrace_jmp_set(addr);
> > -
> > - ret = ftrace_set_filter_ip(tr->fops, (unsigned long)ip, 0, 1);
> > - if (ret)
> > - return ret;
> > - ret = register_ftrace_direct(tr->fops, addr);
> > + ret = direct_ops_add(tr, new_addr);
>
> Ditto.
yes
>
> > } else {
> > ret = bpf_trampoline_update_fentry(tr, 0, NULL, new_addr);
> > }
> > @@ -910,10 +1052,7 @@ void bpf_trampoline_put(struct bpf_trampoline *tr)
> > */
> > hlist_del(&tr->hlist_key);
> > hlist_del(&tr->hlist_ip);
> > - if (tr->fops) {
> > - ftrace_free_filter(tr->fops);
> > - kfree(tr->fops);
> > - }
> > + direct_ops_free(tr);
> > kfree(tr);
> > out:
> > mutex_unlock(&trampoline_mutex);
> > diff --git a/kernel/trace/Kconfig b/kernel/trace/Kconfig
> > index 4661b9e606e0..1ad2e307c834 100644
> > --- a/kernel/trace/Kconfig
> > +++ b/kernel/trace/Kconfig
> > @@ -50,6 +50,9 @@ config HAVE_DYNAMIC_FTRACE_WITH_REGS
> > config HAVE_DYNAMIC_FTRACE_WITH_DIRECT_CALLS
> > bool
> >
> > +config HAVE_SINGLE_FTRACE_DIRECT_OPS
> > + bool
> > +
>
> Now you could add:
>
> config SINGLE_FTRACE_DIRECT_OPS
> bool
> default y
> depends on HAVE_SINGLE_FTRACE_DIRECT_OPS && DYNAMIC_FTRACE_WITH_DIRECT_CALLS
ok, the dependency is more ovbvious, will change
thanks,
jirka
^ permalink raw reply
* Re: [PATCHv5 bpf-next 6/9] ftrace: Add update_ftrace_direct_mod function
From: Jiri Olsa @ 2025-12-19 9:27 UTC (permalink / raw)
To: Steven Rostedt
Cc: Steven Rostedt, Florent Revest, Mark Rutland, bpf, linux-kernel,
linux-trace-kernel, linux-arm-kernel, Alexei Starovoitov,
Daniel Borkmann, Andrii Nakryiko, Menglong Dong, Song Liu
In-Reply-To: <20251218101942.0716efd6@gandalf.local.home>
On Thu, Dec 18, 2025 at 10:19:42AM -0500, Steven Rostedt wrote:
> On Mon, 15 Dec 2025 22:13:59 +0100
> Jiri Olsa <jolsa@kernel.org> wrote:
>
> > diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
> > index 48dc0de5f2ce..95a38fb18ed7 100644
> > --- a/kernel/trace/ftrace.c
> > +++ b/kernel/trace/ftrace.c
> > @@ -6489,6 +6489,78 @@ int update_ftrace_direct_del(struct ftrace_ops *ops, struct ftrace_hash *hash)
> > return err;
> > }
> >
>
> Kerneldoc needed.
will add
>
> > +int update_ftrace_direct_mod(struct ftrace_ops *ops, struct ftrace_hash *hash, bool do_direct_lock)
> > +{
> > + struct ftrace_func_entry *entry, *tmp;
> > + static struct ftrace_ops tmp_ops = {
> > + .func = ftrace_stub,
> > + .flags = FTRACE_OPS_FL_STUB,
> > + };
> > + struct ftrace_hash *orig_hash;
> > + unsigned long size, i;
> > + int err = -EINVAL;
> > +
> > + if (!hash_count(hash))
> > + return -EINVAL;
> > + if (check_direct_multi(ops))
> > + return -EINVAL;
> > + if (!(ops->flags & FTRACE_OPS_FL_ENABLED))
> > + return -EINVAL;
> > + if (direct_functions == EMPTY_HASH)
> > + return -EINVAL;
> > +
> > + if (do_direct_lock)
> > + mutex_lock(&direct_mutex);
>
> This optional taking of the direct_mutex lock needs some serious rationale
> and documentation.
it mirrors the use of modify_ftrace_direct/modify_ftrace_direct_nolock
when we do trampoline update from within ftrace_ops->ops_func callback
I'll add comments with more details
>
> > +
> > + orig_hash = ops->func_hash ? ops->func_hash->filter_hash : NULL;
> > + if (!orig_hash)
> > + goto unlock;
> > +
> > + /* Enable the tmp_ops to have the same functions as the direct ops */
> > + ftrace_ops_init(&tmp_ops);
> > + tmp_ops.func_hash = ops->func_hash;
> > +
> > + err = register_ftrace_function_nolock(&tmp_ops);
> > + if (err)
> > + goto unlock;
> > +
> > + /*
> > + * Call __ftrace_hash_update_ipmodify() here, so that we can call
> > + * ops->ops_func for the ops. This is needed because the above
> > + * register_ftrace_function_nolock() worked on tmp_ops.
> > + */
> > + err = __ftrace_hash_update_ipmodify(ops, orig_hash, orig_hash, true);
> > + if (err)
> > + goto out;
> > +
> > + /*
> > + * Now the ftrace_ops_list_func() is called to do the direct callers.
> > + * We can safely change the direct functions attached to each entry.
> > + */
> > + mutex_lock(&ftrace_lock);
>
> I'm going to need some time staring at this code. It looks like it may be
> relying on some internals here.
ok, thanks
jirka
^ permalink raw reply
* Re: [PATCHv5 bpf-next 5/9] ftrace: Add update_ftrace_direct_del function
From: Jiri Olsa @ 2025-12-19 9:27 UTC (permalink / raw)
To: Steven Rostedt
Cc: Florent Revest, Mark Rutland, bpf, linux-kernel,
linux-trace-kernel, linux-arm-kernel, Alexei Starovoitov,
Daniel Borkmann, Andrii Nakryiko, Menglong Dong, Song Liu
In-Reply-To: <20251217204814.38756224@robin>
On Wed, Dec 17, 2025 at 08:48:14PM -0500, Steven Rostedt wrote:
> On Mon, 15 Dec 2025 22:13:58 +0100
> Jiri Olsa <jolsa@kernel.org> wrote:
> > +/**
> > + * hash_sub - substracts @b from @a and returns the result
> > + * @a: struct ftrace_hash object
> > + * @b: struct ftrace_hash object
> > + *
> > + * Returns struct ftrace_hash object on success, NULL on error.
> > + */
> > +static struct ftrace_hash *hash_sub(struct ftrace_hash *a, struct ftrace_hash *b)
> > +{
> > + struct ftrace_func_entry *entry, *del;
> > + struct ftrace_hash *sub;
> > + int size, i;
> > +
> > + sub = alloc_and_copy_ftrace_hash(a->size_bits, a);
> > + if (!sub)
> > + goto error;
>
> Again, this can be just return NULL;
ok
>
> > +
> > + size = 1 << b->size_bits;
> > + for (i = 0; i < size; i++) {
>
> You can make this for (int i = 0; ...) too.
ok
>
> > + hlist_for_each_entry(entry, &b->buckets[i], hlist) {
> > + del = __ftrace_lookup_ip(sub, entry->ip);
> > + if (WARN_ON_ONCE(!del))
> > + goto error;
>
> And you can remove the error label here too:
ok
>
> if (WARN_ON_ONCE(!del)) {
> free_ftrace_hash(sub);
> return NULL;
> }
>
>
> > + remove_hash_entry(sub, del);
> > + kfree(del);
> > + }
> > + }
> > + return sub;
> > +
> > + error:
> > + free_ftrace_hash(sub);
> > + return NULL;
> > +}
> > +
> > +int update_ftrace_direct_del(struct ftrace_ops *ops, struct ftrace_hash *hash)
> > +{
> > + struct ftrace_hash *old_direct_functions = NULL, *new_direct_functions;
> > + struct ftrace_hash *old_filter_hash, *new_filter_hash = NULL;
> > + struct ftrace_func_entry *del, *entry;
>
> One variable per line.
ok
>
> > + unsigned long size, i;
> > + int err = -EINVAL;
> > +
> > + if (!hash_count(hash))
> > + return -EINVAL;
> > + if (check_direct_multi(ops))
> > + return -EINVAL;
> > + if (!(ops->flags & FTRACE_OPS_FL_ENABLED))
> > + return -EINVAL;
> > + if (direct_functions == EMPTY_HASH)
> > + return -EINVAL;
> > +
> > + mutex_lock(&direct_mutex);
> > +
> > + old_filter_hash = ops->func_hash ? ops->func_hash->filter_hash : NULL;
> > +
> > + if (!hash_count(old_filter_hash))
> > + goto out_unlock;
> > +
> > + /* Make sure requested entries are already registered. */
> > + size = 1 << hash->size_bits;
> > + for (i = 0; i < size; i++) {
>
> for (int i = 0; ...)
ack
thanks,
jirka
^ permalink raw reply
* Re: [PATCHv5 bpf-next 4/9] ftrace: Add update_ftrace_direct_add function
From: Jiri Olsa @ 2025-12-19 9:27 UTC (permalink / raw)
To: Steven Rostedt
Cc: Florent Revest, Mark Rutland, bpf, linux-kernel,
linux-trace-kernel, linux-arm-kernel, Alexei Starovoitov,
Daniel Borkmann, Andrii Nakryiko, Menglong Dong, Song Liu
In-Reply-To: <20251217203909.474ae959@robin>
On Wed, Dec 17, 2025 at 08:39:09PM -0500, Steven Rostedt wrote:
> On Mon, 15 Dec 2025 22:13:57 +0100
> Jiri Olsa <jolsa@kernel.org> wrote:
>
>
> > +/**
> > + * hash_add - adds two struct ftrace_hash and returns the result
> > + * @a: struct ftrace_hash object
> > + * @b: struct ftrace_hash object
> > + *
> > + * Returns struct ftrace_hash object on success, NULL on error.
> > + */
> > +static struct ftrace_hash *hash_add(struct ftrace_hash *a, struct ftrace_hash *b)
> > +{
> > + struct ftrace_func_entry *entry;
> > + struct ftrace_hash *add;
> > + int size, i;
> > +
> > + size = hash_count(a) + hash_count(b);
> > + if (size > 32)
> > + size = 32;
> > +
> > + add = alloc_and_copy_ftrace_hash(fls(size), a);
> > + if (!add)
> > + goto error;
>
> You can just return NULL here, as add is NULL.
ok
>
> > +
> > + size = 1 << b->size_bits;
> > + for (i = 0; i < size; i++) {
> > + hlist_for_each_entry(entry, &b->buckets[i], hlist) {
> > + if (add_hash_entry_direct(add, entry->ip, entry->direct) == NULL)
> > + goto error;
>
> Could remove the error and have:
>
> if (add_hash_entry_direct(add, entry->ip, entry->direct) == NULL) {
> free_ftrace_hash(add);
> return NULL;
> }
ok
>
>
> > + }
> > + }
> > + return add;
> > +
> > + error:
> > + free_ftrace_hash(add);
> > + return NULL;
> > +}
> > +
>
> Non static functions require a kerneldoc header.
ah right, will add
>
> > +int update_ftrace_direct_add(struct ftrace_ops *ops, struct ftrace_hash *hash)
> > +{
> > + struct ftrace_hash *old_direct_functions = NULL, *new_direct_functions;
> > + struct ftrace_hash *old_filter_hash, *new_filter_hash = NULL;
>
> BTW, I prefer to not double up on variables. That is to have each on
> their own lines. Makes it easier to read for me.
>
> > + struct ftrace_func_entry *entry;
> > + int i, size, err = -EINVAL;
>
> Even here.
ok, will split
>
> > + bool reg;
> > +
> > + if (!hash_count(hash))
> > + return -EINVAL;
> > +
> > + mutex_lock(&direct_mutex);
> > +
> > + /* Make sure requested entries are not already registered. */
> > + size = 1 << hash->size_bits;
> > + for (i = 0; i < size; i++) {
>
> If you want, you can remove the i declaration and use for(int i = 0; ... here.
ok
SNIP
> > + }
> > +
> > + if (err) {
> > + /* reset direct_functions and free the new one */
> > + rcu_assign_pointer(direct_functions, old_direct_functions);
> > + old_direct_functions = new_direct_functions;
> > + }
> > +
> > + out_unlock:
> > + mutex_unlock(&direct_mutex);
> > +
> > + if (old_direct_functions && old_direct_functions != EMPTY_HASH)
> > + call_rcu_tasks(&old_direct_functions->rcu, register_ftrace_direct_cb);
> > + if (new_filter_hash)
>
> free_ftrace_hash() checks for NULL, so you don't need the above if statement.
ok
thanks,
jirka
^ permalink raw reply
* Re: [PATCHv5 bpf-next 3/9] ftrace: Export some of hash related functions
From: Jiri Olsa @ 2025-12-19 9:27 UTC (permalink / raw)
To: Steven Rostedt
Cc: Florent Revest, Mark Rutland, bpf, linux-kernel,
linux-trace-kernel, linux-arm-kernel, Alexei Starovoitov,
Daniel Borkmann, Andrii Nakryiko, Menglong Dong, Song Liu
In-Reply-To: <20251217200712.606a9a7a@robin>
On Wed, Dec 17, 2025 at 08:07:12PM -0500, Steven Rostedt wrote:
> On Mon, 15 Dec 2025 22:13:56 +0100
> Jiri Olsa <jolsa@kernel.org> wrote:
>
> > index 505b7d3f5641..c0a72fcae1f6 100644
> > --- a/include/linux/ftrace.h
> > +++ b/include/linux/ftrace.h
> > @@ -82,6 +82,7 @@ static inline void early_trace_init(void) { }
> >
> > struct module;
> > struct ftrace_hash;
> > +struct ftrace_func_entry;
> >
> > #if defined(CONFIG_FUNCTION_TRACER) && defined(CONFIG_MODULES) && \
> > defined(CONFIG_DYNAMIC_FTRACE)
> > @@ -405,6 +406,14 @@ enum ftrace_ops_cmd {
> > typedef int (*ftrace_ops_func_t)(struct ftrace_ops *op, enum ftrace_ops_cmd cmd);
> >
> > #ifdef CONFIG_DYNAMIC_FTRACE
> > +
> > +#define FTRACE_HASH_DEFAULT_BITS 10
> > +
> > +struct ftrace_hash *alloc_ftrace_hash(int size_bits);
> > +void free_ftrace_hash(struct ftrace_hash *hash);
> > +struct ftrace_func_entry *add_hash_entry_direct(struct ftrace_hash *hash,
>
> As this is no longer static and is exported to other users within the
> kernel, it should be renamed to: add_ftrace_hash_entry_direct()
> to keep the namespace unique.
ok, will change
jirka
>
> -- Steve
>
> > + unsigned long ip, unsigned long direct);
> > +
> > /* The hash used to know what functions callbacks trace */
> > struct ftrace_ops_hash {
> > struct ftrace_hash __rcu *notrace_hash;
^ permalink raw reply
* Re: [PATCH 1/3] mm/khugepaged: remove unnecessary goto 'skip' label
From: Garg, Shivank @ 2025-12-19 9:24 UTC (permalink / raw)
To: Wei Yang
Cc: Andrew Morton, David Hildenbrand, Lorenzo Stoakes, Zi Yan,
Baolin Wang, Liam R . Howlett, Nico Pache, Ryan Roberts, Dev Jain,
Barry Song, Lance Yang, linux-mm, linux-kernel,
linux-trace-kernel
In-Reply-To: <20251217024818.ngoime34bxeatqij@master>
On 12/17/2025 8:18 AM, Wei Yang wrote:
> On Tue, Dec 16, 2025 at 11:11:38AM +0000, Shivank Garg wrote:
>> Replace 'goto skip' with actual logic for better code readability.
>>
>> No functional change.
>>
>> Signed-off-by: Shivank Garg <shivankg@amd.com>
>> ---
>> mm/khugepaged.c | 7 ++++---
>> 1 file changed, 4 insertions(+), 3 deletions(-)
>>
>> diff --git a/mm/khugepaged.c b/mm/khugepaged.c
>> index 6c8c35d3e0c9..107146f012b1 100644
>> --- a/mm/khugepaged.c
>> +++ b/mm/khugepaged.c
>> @@ -2442,14 +2442,15 @@ static unsigned int khugepaged_scan_mm_slot(unsigned int pages, int *result,
>> break;
>> }
>> if (!thp_vma_allowable_order(vma, vma->vm_flags, TVA_KHUGEPAGED, PMD_ORDER)) {
>> -skip:
>> progress++;
>> continue;
>> }
>> hstart = round_up(vma->vm_start, HPAGE_PMD_SIZE);
>> hend = round_down(vma->vm_end, HPAGE_PMD_SIZE);
>> - if (khugepaged_scan.address > hend)
>> - goto skip;
>> + if (khugepaged_scan.address > hend) {
>> + progress++;
>> + continue;
>> + }
>
> Hi, Shivank
>
> The change here looks good, while I come up with an question.
>
> The @progress here seems record two things:
>
> * number of pages scaned
> * number of vma skipped
>
Three things: number of mm. It's incremented 1 for whole khugepaged_scan_mm_slot().
> While in very rare case, we may miss to count the second case.
>
> For example, we have following vmas in a process:
>
> vma1 vma2
> +----------------+------------+
> |2M |1M |
> +----------------+------------+
>
> Let's assume vma1 is exactly HPAGE_PMD_SIZE and also HPAGE_PMD_SIZE aligned.
> But vma2 is only half of HPAGE_PMD_SIZE.
>
> When scan finish vma1 and start on vma2, we would have hstart = hend =
> address. So we continue here but would not do real scan, since address == hend.
>
> I am thinking whether this could handle it:
>
> if (khugepaged_scan.address > hend || hend <= hstart) {
> progress++;
> continue;
> }
>
> Do you thinks I am correct on this?
I think you're correct.
IIUC, @progress acts as rate limiter here.
It is increasing +1 for whole, and then increases by +1 per VMA (if skipped),
or by +HPAGE_PMD_NR (if actually scanned).
So, progress ensuring the hugepaged_do_scan run only until (progress >= pages)
at which point it yields and sleeps (wait_event_freezable).
Without your suggested fix, if a process contains a large number of small VMAs (where
round_up hstart >= round_down(hend), it will unfairly consume more CPU cycles before
yielding compared to a process with fewer or aligned VMAs.
I think your suggestion is ensuring fairness by charging 'progress' count correctly.
Thanks,
Shivank
^ permalink raw reply
* Re: [PATCH v7 00/31] context_tracking,x86: Defer some IPIs until a user->kernel transition
From: patchwork-bot+linux-riscv @ 2025-12-19 8:10 UTC (permalink / raw)
To: Valentin Schneider
Cc: linux-riscv, linux-kernel, linux-mm, rcu, x86, linux-arm-kernel,
loongarch, linux-arch, linux-trace-kernel, tglx, mingo, bp,
dave.hansen, hpa, luto, peterz, acme, jpoimboe, pbonzini, arnd,
frederic, paulmck, jbaron, rostedt, ardb, samitolvanen, davem,
neeraj.upadhyay, joelagnelf, josh, boqun.feng, urezki,
mathieu.desnoyers, mgorman, akpm, masahiroy, shenhan, riel, jannh,
dan.carpenter, oleg, juri.lelli, williams, ypodemsk, mtosatti,
dwagner, ptesarik, sshegde
In-Reply-To: <20251114150133.1056710-1-vschneid@redhat.com>
Hello:
This series was applied to riscv/linux.git (fixes)
by Frederic Weisbecker <frederic@kernel.org>:
On Fri, 14 Nov 2025 16:01:02 +0100 you wrote:
> Context
> =======
>
> We've observed within Red Hat that isolated, NOHZ_FULL CPUs running a
> pure-userspace application get regularly interrupted by IPIs sent from
> housekeeping CPUs. Those IPIs are caused by activity on the housekeeping CPUs
> leading to various on_each_cpu() calls, e.g.:
>
> [...]
Here is the summary with links:
- [v7,01/31] objtool: Make validate_call() recognize indirect calls to pv_ops[]
(no matching commit)
- [v7,02/31] objtool: Flesh out warning related to pv_ops[] calls
(no matching commit)
- [v7,03/31] rcu: Add a small-width RCU watching counter debug option
https://git.kernel.org/riscv/c/d1e6d2773898
- [v7,04/31] rcutorture: Make TREE04 use CONFIG_RCU_DYNTICKS_TORTURE
https://git.kernel.org/riscv/c/82a224498005
- [v7,05/31] jump_label: Add annotations for validating noinstr usage
(no matching commit)
- [v7,06/31] static_call: Add read-only-after-init static calls
(no matching commit)
- [v7,07/31] x86/paravirt: Mark pv_sched_clock static call as __ro_after_init
(no matching commit)
- [v7,08/31] x86/idle: Mark x86_idle static call as __ro_after_init
(no matching commit)
- [v7,09/31] x86/paravirt: Mark pv_steal_clock static call as __ro_after_init
(no matching commit)
- [v7,10/31] riscv/paravirt: Mark pv_steal_clock static call as __ro_after_init
(no matching commit)
- [v7,11/31] loongarch/paravirt: Mark pv_steal_clock static call as __ro_after_init
(no matching commit)
- [v7,12/31] arm64/paravirt: Mark pv_steal_clock static call as __ro_after_init
(no matching commit)
- [v7,13/31] arm/paravirt: Mark pv_steal_clock static call as __ro_after_init
(no matching commit)
- [v7,14/31] perf/x86/amd: Mark perf_lopwr_cb static call as __ro_after_init
(no matching commit)
- [v7,15/31] sched/clock: Mark sched_clock_running key as __ro_after_init
(no matching commit)
- [v7,16/31] KVM: VMX: Mark __kvm_is_using_evmcs static key as __ro_after_init
(no matching commit)
- [v7,17/31] x86/bugs: Mark cpu_buf_vm_clear key as allowed in .noinstr
(no matching commit)
- [v7,18/31] x86/speculation/mds: Mark cpu_buf_idle_clear key as allowed in .noinstr
(no matching commit)
- [v7,19/31] sched/clock, x86: Mark __sched_clock_stable key as allowed in .noinstr
(no matching commit)
- [v7,20/31] KVM: VMX: Mark vmx_l1d_should flush and vmx_l1d_flush_cond keys as allowed in .noinstr
(no matching commit)
- [v7,21/31] stackleack: Mark stack_erasing_bypass key as allowed in .noinstr
(no matching commit)
- [v7,22/31] objtool: Add noinstr validation for static branches/calls
(no matching commit)
- [v7,23/31] module: Add MOD_NOINSTR_TEXT mem_type
(no matching commit)
- [v7,24/31] context-tracking: Introduce work deferral infrastructure
(no matching commit)
- [v7,25/31] context_tracking,x86: Defer kernel text patching IPIs
(no matching commit)
- [v7,26/31] x86/jump_label: Add ASM support for static_branch_likely()
(no matching commit)
- [v7,27/31] x86/mm: Make INVPCID type macros available to assembly
(no matching commit)
- [RFC,v7,28/31] x86/mm/pti: Introduce a kernel/user CR3 software signal
(no matching commit)
- [RFC,v7,29/31] x86/mm/pti: Implement a TLB flush immediately after a switch to kernel CR3
(no matching commit)
- [RFC,v7,30/31] x86/mm, mm/vmalloc: Defer kernel TLB flush IPIs under CONFIG_COALESCE_TLBI=y
(no matching commit)
- [RFC,v7,31/31] x86/entry: Add an option to coalesce TLB flushes
(no matching commit)
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply
* Re: [PATCH bpf-next v3 3/6] bpf: fix the usage of BPF_TRAMP_F_SKIP_FRAME
From: Menglong Dong @ 2025-12-19 2:22 UTC (permalink / raw)
To: Menglong Dong, Andreas Schwab
Cc: ast, rostedt, daniel, john.fastabend, andrii, martin.lau, eddyz87,
song, yonghong.song, kpsingh, sdf, haoluo, jolsa, mhiramat,
mark.rutland, mathieu.desnoyers, jiang.biao, bpf, linux-kernel,
linux-trace-kernel, linux-riscv
In-Reply-To: <874ipnkfvt.fsf@igel.home>
On 2025/12/18 22:45 Andreas Schwab <schwab@linux-m68k.org> write:
> On Nov 18 2025, Menglong Dong wrote:
>
> > Some places calculate the origin_call by checking if
> > BPF_TRAMP_F_SKIP_FRAME is set. However, it should use
> > BPF_TRAMP_F_ORIG_STACK for this propose. Just fix them.
> >
> > Signed-off-by: Menglong Dong <dongml2@chinatelecom.cn>
> > Acked-by: Alexei Starovoitov <ast@kernel.org>
>
> This breaks RISC-V:
Hi, Andreas. Can you offer more information here? After my analysis,
I didn't see the problem. BPF_TRAMP_F_SKIP_FRAME and
BPF_TRAMP_F_ORIG_STACK are set together all the time in RISC-V, so
I changed BPF_TRAMP_F_SKIP_FRAME to BPF_TRAMP_F_ORIG_STACK
*should* have no influence.
Thanks!
Menglong Dong
>
> [ 8.584381][ T1] systemd[1]: bpf-restrict-fs: LSM BPF program attached
> [ 8.588359][ T1] Insufficient stack space to handle exception!
> [ 8.588823][ T1] Task stack: [0xff20000000010000..0xff20000000014000]
> [ 8.589219][ T1] Overflow stack: [0xff600000ffdad070..0xff600000ffdae070]
> [ 8.590133][ T1] CPU: 1 UID: 0 PID: 1 Comm: systemd Not tainted 6.18.0-rc5+ #15 PREEMPT(voluntary) c900881ed1c1988ec5cf3e914d0edeb1b4d83ca3
> [ 8.590898][ T1] Hardware name: riscv-virtio qemu/qemu, BIOS 2025.10 10/01/2025
> [ 8.591494][ T1] epc : copy_from_kernel_nofault+0xa/0x198
> [ 8.592292][ T1] ra : bpf_probe_read_kernel+0x20/0x60
> [ 8.592658][ T1] epc : ffffffff802b732a ra : ffffffff801e6070 sp : ff2000000000ffe0
> [ 8.593121][ T1] gp : ffffffff82262ed0 tp : 0000000000000000 t0 : ffffffff80022320
> [ 8.593566][ T1] t1 : ffffffff801e6056 t2 : 0000000000000000 s0 : ff20000000010040
> [ 8.593997][ T1] s1 : 0000000000000008 a0 : ff20000000010050 a1 : ff60000083b3d320
> [ 8.594446][ T1] a2 : 0000000000000008 a3 : 0000000000000097 a4 : 0000000000000000
> [ 8.594940][ T1] a5 : 0000000000000000 a6 : 0000000000000021 a7 : 0000000000000003
> [ 8.595396][ T1] s2 : ff20000000010050 s3 : ff6000008459fc18 s4 : ff60000083b3d340
> [ 8.595831][ T1] s5 : ff20000000010060 s6 : 0000000000000000 s7 : ff20000000013aa8
> [ 8.596215][ T1] s8 : 0000000000000000 s9 : 0000000000008000 s10: 000000000058dcb0
> [ 8.596641][ T1] s11: 000000000058dca7 t3 : 000000006925116d t4 : ff6000008090f026
> [ 8.597065][ T1] t5 : 00007fff9b0cbaa8 t6 : 0000000000000016
> [ 8.597363][ T1] status: 0000000200000120 badaddr: 0000000000000000 cause: 8000000000000005
> [ 8.598033][ T1] Kernel panic - not syncing: Kernel stack overflow
> [ 8.598597][ T1] CPU: 1 UID: 0 PID: 1 Comm: systemd Not tainted 6.18.0-rc5+ #15 PREEMPT(voluntary) c900881ed1c1988ec5cf3e914d0edeb1b4d83ca3
> [ 8.599244][ T1] Hardware name: riscv-virtio qemu/qemu, BIOS 2025.10 10/01/2025
> [ 8.599659][ T1] Call Trace:
> [ 8.600117][ T1] [<ffffffff8001a1f8>] dump_backtrace+0x28/0x38
> [ 8.600517][ T1] [<ffffffff80002502>] show_stack+0x3a/0x50
> [ 8.600844][ T1] [<ffffffff800122be>] dump_stack_lvl+0x56/0x80
> [ 8.601176][ T1] [<ffffffff80012300>] dump_stack+0x18/0x22
> [ 8.601518][ T1] [<ffffffff80002abe>] vpanic+0xf6/0x328
> [ 8.601819][ T1] [<ffffffff80002d2e>] panic+0x3e/0x40
> [ 8.602088][ T1] [<ffffffff80019ef0>] handle_bad_stack+0x98/0xa0
> [ 8.602395][ T1] [<ffffffff801e6070>] bpf_probe_read_kernel+0x20/0x60
>
> --
> Andreas Schwab, schwab@linux-m68k.org
> GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510 2552 DF73 E780 A9DA AEC1
> "And now for something completely different."
>
>
^ 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