kern: Add phashinit_flags(), which allows malloc(M_NOWAIT)
ClosedPublic
Actions

Authored by sepherosa_gmail.com on Apr 28 2016, 8:41 AM.

Details

Reviewers

kib
gallatin
howard0su_gmail.com
honzhan_microsoft.com
decui_microsoft.com
jhb
rrs

Commits

rS298956: kern: Add phashinit_flags(), which allows malloc(M_NOWAIT)

Summary

It will be used for the upcoming LRO hash table initialization. And probably will be useful in other cases, when M_WAITOK can't be used.

Diff Detail

Lint

Lint Skipped

Unit

Tests Skipped

Event Timeline

sepherosa_gmail.com updated this revision to Diff 15688.Apr 28 2016, 8:41 AM

sepherosa_gmail.com retitled this revision from to kern: Add phashinit_flags(), which allows malloc(M_NOWAIT).

sepherosa_gmail.com updated this object.

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

sepherosa_gmail.com added reviewers: kib, rrs, gallatin, jhb, decui_microsoft.com, honzhan_microsoft.com, howard0su_gmail.com.

jhb added inline comments.Apr 28 2016, 6:56 PM

sys/sys/systm.h
193	I think you can drop the comment for the variable in the prototype as that isn't typical style in FreeBSD. There is a manpage for hashinit/phashinit that should be updated for this new function as well. That is where the meaning of the flags argument should be documented instead.

kib added inline comments.Apr 28 2016, 8:38 PM

sys/kern/subr_hash.c
111	To my taste, the new name is too long. We often call such enhanced functions e.g. phashinit1(). I understand that you just copied what was done to hashinit.
130	You wrote the assert expression in very compact way. Why spend 4 lines to translate flags ? m_flags = (flags & HASH_NOWAIT) != 0 ? M_NOWAIT : M_WAITOK;

sepherosa_gmail.com added inline comments.Apr 29 2016, 1:46 AM

sys/kern/subr_hash.c
111	Just want to make it consistent w/ hashinit :)
130	KASSERT was copied too :P And, well, when copying the malloc, I just noticed that part duplicates too much code (two malloc(...) lines), so we saw what I posted here. Yeah, compat form is fine w/ me.

Address jhb and kib's concerns

kib accepted this revision.Apr 29 2016, 7:40 AM

kib edited edge metadata.

This revision is now accepted and ready to land.Apr 29 2016, 7:40 AM

Maybe as a followup fix hashinit_flags() to not duplicate the malloc and instead use the logic you used in phashinit_flags(). That's optional though (and I wouldn't include it as part of this change).

In D6138#130946, @jhb wrote:

Maybe as a followup fix hashinit_flags() to not duplicate the malloc and instead use the logic you used in phashinit_flags(). That's optional though (and I wouldn't include it as part of this change).

Yeah, sure.

Closed by commit rS298956: kern: Add phashinit_flags(), which allows malloc(M_NOWAIT) (authored by sephe). · Explain WhyMay 3 2016, 7:17 AM

This revision was automatically updated to reflect the committed changes.