Received-SPF: pass (google.com: domain of linux-kernel+bounces-192300-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:40f1:3f00::1 as permitted sender) client-ip=2604:1380:40f1:3f00::1;
From: Radoslaw Zielonek <radoslaw.zielonek@gmail.com>
To: vladimir.oltean@nxp.com
Cc: davem@davemloft.net,
	edumazet@google.com,
	kuba@kernel.org,
	linux-kernel@vger.kernel.org,
	netdev@vger.kernel.org,
	pabeni@redhat.com,
	syzbot+a7d2b1d5d1af83035567@syzkaller.appspotmail.com,
	syzkaller-bugs@googlegroups.com,
	vinicius.gomes@intel.com,
	willemdebruijn.kernel@gmail.com,
	Radoslaw Zielonek <radoslaw.zielonek@gmail.com>
Subject: Re: [syzbot] [net?] INFO: rcu detected stall in packet_release
Date: Tue, 28 May 2024 14:25:58 +0200
Message-ID: <20240528122610.21393-2-radoslaw.zielonek@gmail.com>
In-Reply-To: <20240527140145.tlkyayvvmsmnid32@skbuf>
References: <20240527140145.tlkyayvvmsmnid32@skbuf>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=y
Content-Transfer-Encoding: 8bit

Hello,

I'm working on similar taprio bug:
	https://syzkaller.appspot.com/bug?extid=c4c6c3dc10cc96bcf723 
I think I know what is the root cause.

The function advance_sched()
[https://elixir.bootlin.com/linux/v5.10.173/source/net/sched/sch_taprio.c#L696]
runs repeatedly. It is executed using HRTimer.
In every call to advance_sched(), end_time is calculated,
and the timer is set so that the next execution will be at end_time.
To achieve this, first, the expiration time is set using hrtimer_set_expires(),
and second, HRTIMER_RESTART is returned.
This means that the timer is re-enqueued with the adjusted expiration time.
The issue is that end_time is set far before the current time (now),
causing advance_sched() to execute immediately without a context switch.

__hrtimer_run_queues()
[https://elixir.bootlin.com/linux/v5.10.173/source/kernel/time/hrtimer.c#L1615]
is a function with a long loop.
First, please note that now is calculated once and not updated within this function.
We can see the statement basenow = now + base->offset,
but this statement is outside the loop (and in my case, the offset is 0).
The loop will terminate when the queue is empty or the next entry in the queue
has an expiration time in the future.
The issue here is that the queue can be updated within __run_timer().
In my case, __run_timer() adds a new entry to the queue with advance_sched() function.
Since the expiration time is before now, we need to execute advance_sched() again.
The loop is very long because, in our case, the cycle is set to 3ns.

My idea is to create throttling mechanism.
When advance_sched() sets the hrtimer expiration time to before the current time
for X consecutive times, we can postpone the new advance_sched().
You can see my PoC here: https://lore.kernel.org/all/00000000000089...@google.com/T/

Could you take a look at it? What do you think?
Is it acceptable, or is it too aggressive with too much impact on the TAPRIO scheduler?

Radosław.