Re: [PATCH net-next v2 0/5] net/rds: own the fastpath locks across connection teardown

On Thu, 2026-08-20 at 13:38 -0700, Jakub Kicinski wrote:
> On Sat, 15 Aug 2026 17:15:05 -0700 Allison Henderson wrote:
> > This is v2 of the follow-up set to "net/rds: Bug fix ports, part 2"
> > [1] (v1 of this set is at [2]).  During review of part 2, the later
> > half of that series needed more work than a respin, so it was split
> > off into this set together with the companion fixes identified along
> > the way.
> 
> Any thoughts on the AI review?
> https://netdev-ai.bots.linux.dev/sashiko/#/patchset/[email protected]
> IDK enough about RDS to judge the severity of the last one.
> 
> BTW we will take these via net now that the 7.2 final was tagged.
> Don't worry about net-next being closed.

Hi Jakub,

I think the last review in patch 5 isnt as severe as the review makes it
sound. The shutdown worker can hold RDS_IN_XMIT for the duration of the
TCP drain, which in the worst case can be up to 5 sec across 50 retries.  

If a duelling SYN shows up in that time, the accept worker on the global
krdsd queue sleeps waiting for the bit.  That does mean that things in
the same queue have to wait behind it since it's single threaded. It can't
deadlock though since the accept worker runs on a different queue than the
shutdown worker. Shutdowns are on the individual connection path queues.
So the accept workers wait for the shutdown workers, but nothing on the
conn path queues wait for anything on the global queue, so eventually the
drain always finishes and releases the bit.

There is a low-memory fallback where conn paths share the global queue
instead of allocating their own.   That's what they used to do before
commit 4716af3897 gave them their own queue.  But that just means
all the work items are serialized, so they can't race over the bit.

The waiting algorithm itself actually isn't new. The accept path has
waited on this bit from the krdsd work item since 335b48d980f6 back in
2016. This series stretches the worst case from "a sender finishes its
batch" to "the teardown finishes its drain".  Ideally the best fix would
be asynchronous teardown, and there is another Oracle-tree change that
coordinates that.  It's another porting effort on my todo list, but
I try to contain changes in small subsets to keep the reviews manageable
since a lot of rds patch tend to snowball. 

The other commentary nits from the review do have merit though. If you
like, I can send a v3 with those corrected, but I think the code itself
is ok. The patch 1 comment about cong.c is a pre-existing issue similar
to the lost-wakeup issue that patch 1 fixes. So I can send a follow-up
converting those waitqueue_active() calls to wq_has_sleeper().

I hope that helps? Thank you for being accommodating with the branch
targeting.

Allison