Re: [PATCH 2/3] cgroup: add lockless fast-path checks to cgroup_file_notify()

[Shakeel Butt]

On Tue, Mar 03, 2026 at 11:18:17AM +0800, Chen Ridong wrote:
> 
> 
[...]
> > @@ -4689,6 +4689,12 @@ void cgroup_file_notify(struct cgroup_file *cfile)
> >  	unsigned long flags;
> >  	struct kernfs_node *kn = NULL;
> >  
> > +	if (!READ_ONCE(cfile->kn))
> > +		return;
> > +
> > +	if (timer_pending(&cfile->notify_timer))
> > +		return;
> > +
> 
> The added timer_pending() check seems problematic. According to the function's
> comment, callers must ensure serialization with other timer operations. Here
> we're checking timer_pending() locklessly, which means we might:
> 

That comment seems outdated. Check the commit 90c018942c2ba ("timer: Use
hlist_unhashed_lockless() in timer_pending()").

> 1. See an inconsistent state if another CPU is concurrently modifying the timer
> 

It will not see inconsistent state but it can see stale state which is totally
fine. At worst we will take the lock and recheck but we will never miss the
notifications.

> 2. Race with del_timer() or mod_timer() from other contexts
> 
> >  	spin_lock_irqsave(&cgroup_file_kn_lock, flags);
> >  	if (cfile->kn) {
> >  		unsigned long last = cfile->notified_at;
> 
> -- 
> Best regards,
> Ridong
>