Details

Reviewers

markj
kib

Group Reviewers

manpages

Commits

rGa638dc4ebc8e: vfs: Add ioflag to VOP_DEALLOCATE(9)

Summary

The addition of ioflag allows callers passing
IO_SYNC/IO_DATASYNC/IO_DIRECT down to the file system implementation.
The vop_stddeallocate fallback implementation is updated to pass the
ioflag to the file system implementation.
vn_deallocate(9) internally is also changed to pass ioflag to the
VOP_DEALLOCATE call.

Diff Detail

Repository

rS FreeBSD src repository - subversion

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 40991
Build 37880: arc lint + arc unit

Event Timeline

khng created this revision.Aug 11 2021, 7:19 AM

Herald added a reviewer: manpages. · View Herald TranscriptAug 11 2021, 7:19 AM

Herald added a subscriber: imp. · View Herald Transcript

khng requested review of this revision.Aug 11 2021, 7:19 AM

Harbormaster completed remote builds in B40981: Diff 93526.Aug 11 2021, 7:19 AM

kib added inline comments.Aug 11 2021, 8:12 AM

sys/kern/vfs_vnops.c
3457	I suggest you to commit rename ioflg->ioflag as a preliminary change.
3473	Fix style: there should be spaces around '\|'.
3554	!= 0
3555	!= 0
3563	() != 0

Styles.

khng added a child revision: D31505: vfs: Rename ioflg to ioflag in vn_deallocate.Aug 11 2021, 8:28 AM

khng removed a child revision: D31505: vfs: Rename ioflg to ioflag in vn_deallocate.

khng added a parent revision: D31505: vfs: Rename ioflg to ioflag in vn_deallocate.

Harbormaster completed remote builds in B40987: Diff 93532.Aug 11 2021, 8:29 AM

khng mentioned this in D31505: vfs: Rename ioflg to ioflag in vn_deallocate.Aug 11 2021, 9:06 AM

kib added inline comments.Aug 11 2021, 9:07 AM

sys/kern/vfs_vnops.c
3564	This ioflag calculation block was copied/pasted from vn_write(). Put it into a helper used in both places.

IO_SYNC and/or IO_DATASYNC ends up in a helper.
Move IO_NODELOCKED | IO_RANGELOCKED style fix to D31505

Harbormaster completed remote builds in B40989: Diff 93534.Aug 11 2021, 9:43 AM

kib added inline comments.Aug 11 2021, 9:56 AM

sys/kern/vfs_vnops.c
1143	Didn't previous version also handled O_DIRECT?
1145	I do not see why making the invocation of the helper more complicated. Move calculation of mp into it, as you see mp is cleared on the next line.

Let vn_fspacectl check O_DIRECT as well
style

Harbormaster completed remote builds in B40991: Diff 93537.Aug 11 2021, 10:10 AM

Move mp = into the helper.

Harbormaster completed remote builds in B40993: Diff 93542.Aug 11 2021, 12:04 PM

get_sync_ioflag -> get_write_ioflag: Move IO_DIRECT checking into the helper.

Harbormaster completed remote builds in B40994: Diff 93543.Aug 11 2021, 12:11 PM

markj added inline comments.Aug 11 2021, 1:55 PM

share/man/man9/vn_deallocate.9
70	Just copy the definition from VOP_DEALLOCATE?
79–80
sys/kern/vfs_vnops.c
1142–1146	Why not handle FNONBLOCK in the helper too?

It is useful to extract get_write_ioflags() into separate commit. For instance, it can be merged to stable branches independently of other fspacectl changes.

sys/kern/vfs_vnops.c
930	() != 0
933	Same.
942	Same

Dedicated commit for:

vfs: Add get_write_ioflag handler to calculate ioflag

Styles

Harbormaster completed remote builds in B40995: Diff 93544.Aug 11 2021, 3:02 PM

khng marked an inline comment as done.Aug 11 2021, 3:03 PM

I asked for a commit that _only_ introduces get_write_ioflag() and replaces existing place(s) with it. Addition of flags anywhere is clearly a different change.

Separate get_write_ioflag commit (This currently calculates IO_DIRECT/IO_SYNC/IO_DATASYNC). I do not introduce IO_NDELAY calculation because it is clearly not going to be used by vn_fspacectl yet even it is harmless to just let it passthrough for now.

Harbormaster completed remote builds in B41007: Diff 93586.Aug 12 2021, 5:15 AM

khng added a parent revision: D31513: vfs: Add get_write_ioflag helper to calculate ioflag.Aug 12 2021, 5:17 AM

kib accepted this revision.Aug 12 2021, 12:44 PM

This revision is now accepted and ready to land.Aug 12 2021, 12:44 PM

markj accepted this revision.Aug 12 2021, 1:29 PM

Closed by commit rGa638dc4ebc8e: vfs: Add ioflag to VOP_DEALLOCATE(9) (authored by khng). · Explain WhyAug 12 2021, 3:07 PM

This revision was automatically updated to reflect the committed changes.

khng added a commit: rGa638dc4ebc8e: vfs: Add ioflag to VOP_DEALLOCATE(9).