Page MenuHomeFreeBSD

Add a safety net to reclaim mbufs when one of the mbuf zones become exhausted
ClosedPublic

Authored by jtl on Oct 10 2015, 2:19 AM.
Referenced Files
Unknown Object (File)
Sat, Mar 23, 12:39 AM
Unknown Object (File)
Sat, Mar 23, 12:01 AM
Unknown Object (File)
Mon, Mar 11, 10:28 AM
Unknown Object (File)
Tue, Mar 5, 1:16 PM
Unknown Object (File)
Jan 30 2024, 5:23 PM
Unknown Object (File)
Jan 26 2024, 3:15 PM
Unknown Object (File)
Jan 26 2024, 3:15 PM
Unknown Object (File)
Jan 26 2024, 3:15 PM

Details

Summary

It is possible for a bug in the code (or, theoretically, even unusual network conditions) to exhaust all possible mbufs or mbuf clusters. When this occurs, things can grind to a halt fairly quickly. However, we currently do not call mb_reclaim() unless the entire system is experiencing a low-memory condition.

While it is best to try to prevent exhaustion of one of the mbuf zones, it would also be useful to have a mechanism to attempt to recover from these situations by freeing "expendable" mbufs.

This patch makes two changes:

a) The patch adds a generic API to the UMA zone allocator to set a function that should be called when an allocation fails because the zone limit has been reached. Because of the way this function can be called, it really should do minimal work; it should be something akin to a signal handler.

b) The patch uses this API to try to free mbufs when an allocation fails from one of the mbuf zones because the zone limit has been reached. The function schedules a callout to run mb_reclaim() in the context of the callout softclock thread.

Test Plan

With only the changes to the UMA system, I saw that the system behaved normally when I exhausted all mbuf clusters.

With the additional changes to run mb_reclaim() when an allocation fails from one of the mbuf zones because the zone limit has been reached, I exhausted all mbufs on a system and saw that mb_reclaim() freed them.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

jonlooney_gmail.com retitled this revision from to Add a safety net to reclaim mbufs when one of the mbuf zones become exhausted.
jonlooney_gmail.com updated this object.
jonlooney_gmail.com edited the test plan for this revision. (Show Details)
jonlooney_gmail.com set the repository for this revision to rS FreeBSD src repository - subversion.
jonlooney_gmail.com added a project: network.
jonlooney_gmail.com added a subscriber: network.
bz added a subscriber: bz.

I am trying to start a proper reviewers list ... ;-)

Should not the callout have some sort of lock?

You declare it MP-SAFE but I see no lock as described in the man pages
for callout_reset and friends..

In D3864#81077, @rrs wrote:

Should not the callout have some sort of lock?

You declare it MP-SAFE but I see no lock as described in the man pages
for callout_reset and friends..

My understanding - perhaps flawed - was that the callout would be mpsafe because the actual actions taken by the callout (mb_reclaim()) acquires appropriate locks before working.

However, I see that the man page specifically talks about the callout structure itself being mpsafe. It is easy enough to add such a lock, so I will do it.

jonlooney_gmail.com edited edge metadata.

Added a lock around the callout calls.

"bump" :-)

If anyone has time to review this, I'd appreciate it.

Thanks!

rrs requested changes to this revision.Nov 9 2015, 3:31 PM
rrs edited edge metadata.

fix my comment and I think you will be good to go

sys/kern/kern_mbuf.c
726 ↗(On Diff #9431)

The return with-in the callout_pending/!callout_active needs to unlock the
lock that you took.

This revision now requires changes to proceed.Nov 9 2015, 3:31 PM
jtl edited edge metadata.

Addressed @rrs's comment.

Also, updated the date in the man page.

sys/kern/kern_mbuf.c
726 ↗(On Diff #10185)

Doh! Thanks! Good catch! This should be fixed now.

jtl marked 2 inline comments as done.Nov 15 2015, 12:39 AM

Switch the locking mechanism to a mutex.

gnn edited edge metadata.

What about using taskqueue for memory reclaiming purposes? The struct taskqueue will live at the end of struct uma_zone. That would allow a less constrained KPI. uma(9) users would not need to do gymnastics with callout(9) and their maxaction function can go directly into freeing memory, since the context doesn't have any locks.

I'm not trying to delay this revision, since I know other things depend on it! If you agree on my suggestion we can do it post-commit.

Hi Gleb,

What about using taskqueue for memory reclaiming purposes? The struct taskqueue will live at the end of struct uma_zone. That would allow a less constrained KPI. uma(9) users would not need to do gymnastics with callout(9) and their maxaction function can go directly into freeing memory, since the context doesn't have any locks.

I have been thinking about something similar. I also want to tighten up the locking model. I think task queues could help with that, too.

I'm not trying to delay this revision, since I know other things depend on it! If you agree on my suggestion we can do it post-commit.

Sounds good. Let's plan on that.

Jonathan

This revision was automatically updated to reflect the committed changes.