Page MenuHomeFreeBSD

Use M_WRITABLE() rather than direct M_EXT checks
ClosedPublic

Authored by rwatson on Oct 4 2014, 10:02 PM.
Tags
None
Referenced Files
Unknown Object (File)
Fri, Oct 25, 11:52 PM
Unknown Object (File)
Thu, Oct 24, 6:28 AM
Unknown Object (File)
Mon, Oct 14, 8:48 PM
Unknown Object (File)
Sun, Oct 13, 3:23 PM
Unknown Object (File)
Sun, Oct 13, 3:23 PM
Unknown Object (File)
Sun, Oct 13, 3:22 PM
Unknown Object (File)
Sun, Oct 13, 3:22 PM
Unknown Object (File)
Sun, Oct 13, 3:22 PM
Subscribers
None

Details

Summary

Use M_WRITABLE() rather than checking explicitly (and only) for M_EXT in
cases where this was likely the intention (e.g., when deciding whether to
m_pullup() even if there is notionally sufficient data in the mbuf for a
read-write data structure). This reduces knowledge of extern-storage
mechanics outside of the mbuf allocator.

Diff Detail

Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

rwatson retitled this revision from to Use M_WRITABLE() rather than direct M_EXT checks.
rwatson updated this object.
rwatson edited the test plan for this revision. (Show Details)
rwatson added reviewers: bz, glebius.
rwatson set the repository for this revision to rS FreeBSD src repository - subversion.
rwatson updated this revision to Diff 1892.

Closed by commit rS272559 (authored by @rwatson).

Sigh. Wrong URL in my commit message laid to premature closing of this patch.

bz requested changes to this revision.Oct 5 2014, 10:57 AM
bz edited edge metadata.

This is the wrong diff, right?

This revision now requires changes to proceed.Oct 5 2014, 10:57 AM

Argh, I see you committed with the wring D???? number and Phrabicator is too dumb. Can we make diff1 the default again?

Unfortunately, my ability to wield phabricator is weak. Maybe I should just create a new review request. I'll try resubmitting the original patch.

rwatson edited edge metadata.

Re-upload original diff following error in commit message with tricky consequences.

So let's see if I do understand the difference between m_pullup() and m_copyup() correctly.

sys/netinet/igmp.c
1469

This logic doesn't entirely make sense to me. If the mbuf is not writeable how would you do a m_pullup() on it? You would need an m_copyup(), right? It wouldn't matter much in this code path to my understanding as we assume the mbuf to be gone (and no other copy being around) by the time we leave the function to my understanding.

The same applies to all other netinet/ files, ip6_mroute.c, and 2nd case in ip6_output.c.

Side question: is minlen actually smaller than MHLEN? I know this is unrelated ... so just assume that for the moment ;-)

sys/netinet6/icmp6.c
587

Here it seems good.

sys/netinet6/ip6_output.c
1187

M_TRAINLINGSPACE already does the M_WRITEABLE check for you. You can nuke it.

2910

And that's another one of the m_pulls on possibly not writeable mbufs

rwatson updated this revision to Diff 1964.

Closed by commit rS272984 (authored by @rwatson).