Page MenuHomeFreeBSD
Differential D28899

md: Replace BIO_DELETE emulation with vn_deallocate(9)
ClosedPublic
Actions

Authored by khng on Feb 23 2021, 3:12 PM.
Tags
None
Referenced Files
Unknown Object (File)
Tue, Nov 19, 10:50 AM
Unknown Object (File)
Tue, Nov 19, 10:10 AM
Unknown Object (File)
Mon, Nov 18, 5:06 AM
Unknown Object (File)
Sun, Nov 17, 8:00 AM
Unknown Object (File)
Sun, Nov 17, 7:55 AM
Unknown Object (File)
Sun, Nov 17, 7:52 AM
Unknown Object (File)
Sun, Nov 17, 7:25 AM
Unknown Object (File)
Sun, Nov 17, 7:22 AM
View All Files
Subscribers
imp
lwhsu
Contributor Reviews (src)

Details

Reviewers
markj
Commits
rG78267c2e703c: md: Replace BIO_DELETE emulation with vn_deallocate(9)
Summary

Both zero-filling and/or deallocation can be done with vn_deallocate(9).

Submitted by: Ka Ho Ng <khng@freebsdfoundation.org>
Sponsored by: The FreeBSD Foundation

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 40756
Build 37645: arc lint + arc unit

Event Timeline

khng created this revision.Feb 23 2021, 3:12 PM
Herald added subscribers: Contributor Reviews (src), imp. · View Herald TranscriptFeb 23 2021, 3:12 PM
khng requested review of this revision.Feb 23 2021, 3:12 PM
Harbormaster completed remote builds in B37314: Diff 84553.Feb 23 2021, 3:12 PM
khng added a parent revision: D28347: Add fspacectl(2), vn_deallocate(9) and VOP_DEALLOCATE(9)..Feb 23 2021, 3:13 PM
lwhsu added a subscriber: lwhsu.Feb 23 2021, 3:47 PM
khng updated this revision to Diff 84734.Feb 26 2021, 8:13 AM

Rebased.

Harbormaster completed remote builds in B37399: Diff 84734.Feb 26 2021, 8:14 AM
khng updated this revision to Diff 85986.Mar 18 2021, 7:51 PM

Rebased

Harbormaster completed remote builds in B37927: Diff 85986.Mar 18 2021, 7:51 PM
khng updated this revision to Diff 86012.Mar 19 2021, 8:44 AM

Rebased

Harbormaster completed remote builds in B37943: Diff 86012.Mar 19 2021, 8:45 AM
khng updated this revision to Diff 86187.Mar 23 2021, 10:13 AM

Updated commit message.

Harbormaster completed remote builds in B38010: Diff 86187.Mar 23 2021, 10:14 AM
khng updated this revision to Diff 89087.May 12 2021, 6:32 PM

vn_deallocate KPI changes.

Harbormaster completed remote builds in B39145: Diff 89087.May 12 2021, 6:32 PM
khng updated this revision to Diff 92995.Jul 30 2021, 4:41 PM

vn_deallocate() changed.

Harbormaster completed remote builds in B40756: Diff 92995.Jul 30 2021, 4:41 PM
khng added a reviewer: markj.Aug 10 2021, 3:05 PM
markj added inline comments.Aug 10 2021, 3:17 PM
sys/dev/md/md.c
918

Looks like there is no way to honour MD_SYNC? For writes it is translated to IO_SYNC, but here it looks like it would not be plumbed through VOP_DEALLOCATE.

markj added inline comments.Aug 10 2021, 3:45 PM
sys/dev/md/md.c
886

Harmless, but there's a missing break here.

khng added inline comments.Aug 10 2021, 3:50 PM
sys/dev/md/md.c
918

For the fallback part, do you think it is reasonable if we add a ioflg in VOP_DEALLOCATE that can accept IO_SYNC? Or do we just make it a vn_deallocate + VOP_FSYNC? On the other hand, posix_fallocate is another call that currently bypass IO_SYNC.

markj added inline comments.Aug 10 2021, 4:14 PM
sys/dev/md/md.c
918

I think that adding an additional parameter would be reasonable. Calling VOP_FSYNC() should also work but that seems like a hack, and it can in principle result in unnecessary I/O. I do not insist on fixing this for this review, but I would note it with an XXX comment or so.

So posix_fallocate() does not respect O_SYNC, effectively, which is a bug IMO.

khng updated this revision to Diff 93493.Aug 10 2021, 4:22 PM
  • Gotchas on the current behavior
  • missing break
khng marked 3 inline comments as done.Aug 10 2021, 4:22 PM
Harbormaster completed remote builds in B40967: Diff 93493.Aug 10 2021, 4:22 PM
markj added inline comments.Aug 10 2021, 4:29 PM
sys/dev/md/md.c
920

I believe we need to update bio->bp_resid as well.

khng updated this revision to Diff 93494.Aug 10 2021, 4:39 PM

Update bp->bio_resid.

Harbormaster completed remote builds in B40968: Diff 93494.Aug 10 2021, 4:39 PM
khng marked an inline comment as done.Aug 10 2021, 4:40 PM
markj accepted this revision.Aug 10 2021, 5:24 PM

A test case in sbin/mdconfig/tests would be nice. I believe trim(8) can be used to easily submit DIOCGDELETE requests against a md device.

This revision is now accepted and ready to land.Aug 10 2021, 5:24 PM
khng added a comment.Aug 10 2021, 5:48 PM
In D28899#709851, @markj wrote:

A test case in sbin/mdconfig/tests would be nice. I believe trim(8) can be used to easily submit DIOCGDELETE requests against a md device.

I will make that a separate differential for review.

khng updated this revision to Diff 93621.Aug 12 2021, 3:21 PM
  • Use IO_SYNC for non-MD_ASYNC case.
This revision now requires review to proceed.Aug 12 2021, 3:21 PM
Harbormaster completed remote builds in B41019: Diff 93621.Aug 12 2021, 3:21 PM
markj accepted this revision.Aug 12 2021, 3:52 PM
This revision is now accepted and ready to land.Aug 12 2021, 3:52 PM
Closed by commit rG78267c2e703c: md: Replace BIO_DELETE emulation with vn_deallocate(9) (authored by khng). · Explain WhyAug 19 2021, 10:32 AM
This revision was automatically updated to reflect the committed changes.
khng added a commit: rG78267c2e703c: md: Replace BIO_DELETE emulation with vn_deallocate(9).

Revision Contents
Changeset List

PathSize
sys/
dev/
md/
29 lines

Diff 92995

View Options

sys/dev/md/md.c

Loading...