Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 13 Feb 2014 19:06:07 -0800
From:      Adrian Chadd <adrian@freebsd.org>
To:        Ryan Stone <rysto32@gmail.com>
Cc:        "freebsd-hackers@freebsd.org" <freebsd-hackers@freebsd.org>, "freebsd-arch@freebsd.org" <freebsd-arch@freebsd.org>
Subject:   Re: can the scheduler decide to schedule an interrupted but runnable thread on another CPU core? What are the implications for code?
Message-ID:  <CAJ-Vmommd7w_7RtkqgFGt%2BCVccqcsd8QLNOSXdXSRmmBPKEyYg@mail.gmail.com>
In-Reply-To: <CAFMmRNxFbtegWMxfdD1=t7gHRVg_86FSxh0_R5_%2Bh5JP%2Bpw1Vw@mail.gmail.com>
References:  <CAJ-Vmo=7Nz1jqXy%2BrTQ7u9_ZP7jeFOKUJxU1O51tYJjvTUmWTg@mail.gmail.com> <CAFMmRNxFbtegWMxfdD1=t7gHRVg_86FSxh0_R5_%2Bh5JP%2Bpw1Vw@mail.gmail.com>

next in thread | previous in thread | raw e-mail | index | archive | help
On 13 February 2014 18:37, Ryan Stone <rysto32@gmail.com> wrote:
> On Thu, Feb 13, 2014 at 6:57 PM, Adrian Chadd <adrian@freebsd.org> wrote:
>> sequentually:
>>
>> * lookup occurs on CPU A;
>> * lookup succeeds on CPU A for some almost-expired entry;
>> * preemption occurs, and it gets scheduled to CPU B;
>>
>> then simultaneously:
>>
>> * CPU A's flowtable purge code runs, and decides to purge entries
>> including the current one;
>> * the code now running on CPU B has an item from the CPU A flowtable,
>> and dereferences it as it's being freed, leading to potential badness.
>
> This kind of scenario is definitely possible.  All of the FreeBSD
> kernel code that deals with lockless per-cpu data structures that I
> have seen (e.g. uma) has used critical_enter()/critical_exit() to
> prevent preemption, and have been careful to invalidate their
> references to the per-cpu data if they have to drop the critical
> section.
>
> I don't believe that sched_pin() is good enough because I don't
> believe that it handles the scenario when thread A gets a reference
> and then is preempted, thread B frees the entry, and then A is
> scheduled and uses the now-freed entry.  However I'm really not
> familiar at all with flowtable so maybe there's something preventing
> that.

Oh man, I hate you for bringing that up.

So, both flowtable_lookup_common() and flowtable_insert() does do a
critical_enter() / critical_exit() during the list operations, so
that's ok. It won't be pre-empted.

If I wrap flowtable_lookup() with sched_pin() so it covers both the
lookup path and the insert path, then it won't get scheduled on
another CPU. Then the critical sections around the list operations
guarantee it won't end up with a freed entry. The fl entry uptime is
updated in the critical section so it shouldn't end up overlapping
with the flow purger in a way that has it removed.

So, I think that's okay. I think once i add sched_pin() in the right
spots, this crap won't use a stale entry. Well, as long as the entry
get used before the flow cleaner picks it up.

Phew!



-adrian



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAJ-Vmommd7w_7RtkqgFGt%2BCVccqcsd8QLNOSXdXSRmmBPKEyYg>