p9fs: Refactor buffer allocations to avoid zeroing large payloads
AcceptedPublic
Actions

Authored by arichardson on Sat, Apr 18, 5:58 AM.

Details

Reviewers

markj
kib

Summary

Allocating large buffers with M_ZERO adds unnecessary overhead since
the data is immediately overwritten. This change embeds the tc and rc
p9_buffer structs directly into p9_req_t so we only zero the small
metadata headers. The actual data payload is allocated with M_NOWAIT.

Embedding the metadata headers by value also allows the p9fs_buf_zone
UMA items to be sized exactly to P9FS_MTU, ensuring they are nicely
aligned.

This also adds proper error handling to p9_get_request() to handle
UMA allocation failures.

Diff Detail

Repository

rG FreeBSD src repository

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 72872
Build 69755: arc lint + arc unit

Event Timeline

arichardson created this revision.Sat, Apr 18, 5:58 AM

Herald added a subscriber: imp. · View Herald TranscriptSat, Apr 18, 5:58 AM

arichardson requested review of this revision.Sat, Apr 18, 5:58 AM

Harbormaster completed remote builds in B72316: Diff 175818.Sat, Apr 18, 5:58 AM

arichardson added a parent revision: D56492: p9fs: Move UMA zone initialization to VFS module lifecycle.Sat, Apr 18, 6:01 AM

arichardson added a child revision: D56496: p9fs: Implement msize mount option and bump default to 128 KiB.

markj added inline comments.Tue, Apr 21, 1:16 PM

sys/fs/p9fs/p9_client.c
113	No need to check for NULL here.
118–119	This comment seems wrong and unhelpful.
149–150	With M_WAITOK the allocation is guaranteed to succeed, so this is dead code.
512	These two lines can be combined.

jhb added a subscriber: jhb.Tue, Apr 21, 6:59 PM

jhb added inline comments.

sys/fs/p9fs/p9_client.c
156	Given that p9_buffer_alloc() uses `M_WAITOK`, this is also dead code. You could just change `p9_buffer_alloc` to return void given its use of `M_WAITOK`.