NFSv4 server tunable hash table sizing
AcceptedPublic
Actions

Authored by rmacklem on May 19 2015, 11:28 PM.

Details

Reviewers

Summary

This patch makes the sizes of hash tables used by the NFSv4 server tunable.
Although I do not see any measurable performance change when the table
sizes are increased, a server under heavy NFSv4 load from several clients
may need larger hash tables for good performance. One email report of
high CPU overheads for an NFSv4 server might have been caused by small
hash tables. By making them tunable, NFSv4 server sysadmins can test larger
hash tables.
The default sizing of the hash tables is not changed by this patch.

Test Plan

Tested on a small single client setup and found to be performance neutral
for that case.

Diff Detail

Lint

Lint Skipped

Unit

Tests Skipped

Event Timeline

rmacklem updated this revision to Diff 5504.May 19 2015, 11:28 PM

rmacklem retitled this revision from to NFSv4 server tunable hash table sizing.

rmacklem updated this object.

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

Konstantin already provided this review via email. I've pasted
it here so he doesn't need to type it again:

I cannot see anything wrong with allocating the hash tables with malloc(9).
Even if the tables are relatively large, i.e. require large contig KVA for
allocation to succeed, it is not a problem during boot, when the kmem
is not fragmented.

One thing which I note in the patch is cosmetic issue with the lc_stateid[]
variadic array. You save one malloc call by the trick of the zero-length
last member, but this is IMO not needed anymore, lc_statid may become
the normal pointer and memory allocated by the malloc(9) after struct
nfsclient is malloced.

Change to using 2 mallocs as Konstantin suggested in his
email review.

The second revision is applied to the first. I guess I should
have submitted an entire new diff. Oh well...

kib added inline comments.May 27 2015, 8:34 AM

fs/nfs/nfsrvstate.h
106	Wouldn't it be cleaner to declare lc_id as u_char *lc_id ? Also, why the type is u_char ?
fs/nfsserver/nfs_nfsdserv.c
3472	Why +1 ?

rmacklem added inline comments.May 27 2015, 12:06 PM

fs/nfs/nfsrvstate.h
106	Well, the structure is allocated large enough to store the actual "id", which is an 8bit byte "string" that the server handles opaquely. (ie. Just bcmp()'s == or !=). It was made lc_id[1], so I didn't need to "+ 1" for the null termination byte when I allocated it. (This code was written long ago. If the diff was against -head, this wouldn't have been a change.) If you think lc_id[0] is clearer, that's easy to change. All that is needed is a "+ 1" on the size allocated via malloc(). This is normally small, so I don't think a 3rd malloc() is justified, but I can do it that way. (If it is defined as *lc_id, I think I need to do a third malloc?) As for u_char, it's just 8bit arbitrary bytes, so that seemed more appropriate to char, but I don't think anything in the code cares what the type is, so long as it stores 8bits.
fs/nfsserver/nfs_nfsdserv.c
3472	It's "+ i", where "i" is the number of non-null bytes stored in lc_id. If it is declared lc_id[0], this needs to change to "+ i + 1". (And the font seems to make it look like a "1" for me too. I love web based stuff;-).)