A problem with ASYNC drain
AbandonedPublic
Actions

Authored by mmacy on Jul 6 2016, 11:08 AM.

Details

Reviewers

glebius
jch
rrs

Group Reviewers

transport

Summary

If a thread is running async-drain (like TCP does) its possible some other thread may attempt to restart
the timer. If that occurs the thread should be given a error return and the timer not started (just like
the synchronous drain).

Note that Gleb's previous patch to the callout system fixes 1/2 of the problem this rarer case is still possible and
needs to be addressed :-)

Test Plan

Run it in a netflix work load as well as create a test module to create the exact circumstance.

Diff Detail

Lint

Lint Skipped

Unit

Tests Skipped

Event Timeline

rrs updated this revision to Diff 18185.Jul 6 2016, 11:08 AM

rrs retitled this revision from to A problem with ASYNC drain.

rrs updated this object.

rrs edited the test plan for this revision. (Show Details)

rrs added reviewers: glebius, transport.

jch added a subscriber: jch.Jul 6 2016, 11:10 AM

jch added a reviewer: jch.Jul 14 2016, 4:58 PM

kbowling added a subscriber: bz.Jul 14 2016, 5:25 PM

kbowling added a subscriber: kbowling.

missed an unlock

Originally I had started working on this before Gleb decided to take over and clean it up. I
had fixed the three issues I saw:

Principle - the start of a new timer was not rejected (like it is for sync drain) for async-drain.
A three way leak in the callout start
An incorrect return on some cases with async_drain and tcp.

I think these were the real issues that Gleb went in to fix when he re-wrote a block of
the code and started breaking thing due to the change in return codes and others.

I decided to go back to this version, which I know works and NF is testing (and will soon release
into production).

If Gleb wants he can hack on his re-write and figure out how to make it work.. or he can use this
here and it will work.. I have no preference either way.. I just know that this code does work :-)

Can I close this?

Yes, see rS302894, rS302997, rS302998, rS303037, rS303096, and rS303098.

Short version: @glebius eventually fixed it.

mmacy commandeered this revision.May 16 2018, 7:04 PM

mmacy abandoned this revision.

mmacy added a reviewer: rrs.

Revision Contents
Changeset List

Path

Size

kern_timeout.c

66 lines

Diff 18543

View Options

A problem with ASYNC drainAbandonedPublicActions