fuse: Overlay less of struct fid
AbandonedPublic
Actions

Authored by freqlabs on May 30 2020, 9:15 PM.

Details

Reviewers

asomers
rmacklem

Summary

Match the change to vop_vptofh in tmpfs proposed by D25077, for fuse.

Instead of duplicating fields of struct fid in struct fuse_fid, narrow the struct to cover only the unique fields for fuse and assert at compile time that the struct fits in the allotted space. This way we don't have to replicate the offsets of struct fid fields, we just use them directly.

Diff Detail

Repository

rS FreeBSD src repository - subversion

Lint

Lint Skipped

Unit

Tests Skipped

Event Timeline

freqlabs created this revision.May 30 2020, 9:15 PM

Herald added a subscriber: imp. · View Herald TranscriptMay 30 2020, 9:15 PM

freqlabs requested review of this revision.May 30 2020, 9:15 PM

Um, no? That _Static_assert checks that the situation you describe isn't happening. And kgdb confirms it:

(kgdb) print sizeof(struct fuse_fid)
$1 = 16
(kgdb) print sizeof(struct fid)
$2 = 20

What architecture and compiler are you using? Are you actually seeing build failures due to the _Static_assert?

Sorry, you're right the description is bogus. I saw the same pattern that caused a bug with tmpfs, but missed that here the gen field is only 32 bits rather than 64.

Disregarding the nonexistent bug, how do you feel about this change for the sake of matching the change to tmpfs?

freqlabs retitled this revision from fuse: Preserve alignment of struct fid fields to fuse: Overlay less of struct fid.May 30 2020, 9:40 PM

freqlabs edited the summary of this revision. (Show Details)

freqlabs edited the summary of this revision. (Show Details)May 30 2020, 9:43 PM

I can't say I'm a fan of reducing the diff between fusefs and tmpfs. Right now, fusefs is working similarly to UFS, and I think that's a more important reference than tmpfs.

Okay, I had not looked at UFS. Overlaying the structs like this is clearly a fragile design, but I'm not that motivated to fix what isn't broken.