Page MenuHomeFreeBSD
Differential D22130

zfs: use VOP_NEED_INACTIVE
ClosedPublic
Actions

Authored by mjg on Oct 23 2019, 5:59 PM.
Tags
None
Referenced Files
Unknown Object (File)
Sun, Dec 15, 2:10 AM
Unknown Object (File)
Wed, Dec 11, 7:19 PM
Unknown Object (File)
Tue, Dec 3, 12:35 PM
Unknown Object (File)
Mon, Nov 25, 6:06 PM
Unknown Object (File)
Mon, Nov 25, 7:57 AM
Unknown Object (File)
Sun, Nov 24, 9:29 AM
Unknown Object (File)
Sun, Nov 24, 6:56 AM
Unknown Object (File)
Sun, Nov 24, 3:41 AM
View All Files
Subscribers
delphij
imp
val_packett.cool

Details

Reviewers
mmacy
avg
mav
Commits
rS357281: zfs: use VOP_NEED_INACTIVE
Summary

See r351584 for reasoning.

Sample results from make -j 40 bzImage:

beforeafter
96630750 (lockmgr:zfs)17400445 (rw:vm object)
19971882 (rw:vm object)9008902 (sleep mutex:zio_write_issue)
10328270 (spin mutex:sleepq chain)8802655 (spin mutex:sleepq chain)
6159891 (sleep mutex:vnode interlock)4737875 (sx:zp->z_acl_lock)
5128765 (spin mutex:sched lock 33)4459105 (rw:pmap pv list)
4995571 (spin mutex:sched lock 11)4343860 (sx:rrl->rr_lock)
4942302 (rw:pmap pv list)2906398 (lockmgr:zfs)

Diff Detail

Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

mjg created this revision.Oct 23 2019, 5:59 PM
Herald added subscribers: delphij, imp. · View Herald TranscriptOct 23 2019, 5:59 PM
Harbormaster completed remote builds in B27173: Diff 63594.Oct 23 2019, 5:59 PM
val_packett.cool added a subscriber: val_packett.cool.Nov 3 2019, 11:34 PM
val_packett.cool added a comment.Dec 29 2019, 3:42 PM

I've been testing this patch for a while and it seems like…

it causes files written by LLD >= 9.0 (which uses ftruncate→mmap→rename→close)…

to become all zeroes after reboot.

(and possibly hangs on poweroff/reboot after the bufspacethings are done)

mjg added a comment.EditedDec 29 2019, 3:50 PM

I see why, it misses one check. I'm probably going to fix this in a very different manner though. Thanks for the report!

mjg updated this revision to Diff 66167.Dec 30 2019, 10:42 PM
  • perform a locked check for all conditions
mjg added a comment.Dec 30 2019, 10:42 PM

@greg_unrelenting.technology does the updated patch work for you?

mjg updated this revision to Diff 66169.Dec 30 2019, 11:37 PM
  • trylock since the routine is called with the interlock held
val_packett.cool added a comment.Jan 5 2020, 11:16 PM

Same issues on the latest version.

mjg added a comment.Jan 25 2020, 9:26 PM

@greg_unrelenting.technology can you please retest on top fresh head? (specifically after r357070)

val_packett.cool added a comment.Jan 25 2020, 11:30 PM
In D22130#512307, @mjg wrote:

@greg_unrelenting.technology can you please retest on top fresh head? (specifically after r357070)

No longer hangs on reboot, but zeroed files still happen :(

mjg added a comment.EditedJan 26 2020, 5:24 AM

Can you prepare a short reproducer? Preferably few commands in total.

I ran into problems with previous of the patch but everything works fine for me with this one.

val_packett.cool added a comment.EditedJan 26 2020, 10:49 AM
In D22130#512369, @mjg wrote:

Can you prepare a short reproducer? Preferably few commands in total.

echo 'int main() {}' > hello.c
clang90 -o hello hello.c
./hello # should work
reboot
# cd to same directory
./hello # should report an exec format error or whatever it is, the file will be all zeros

(cc should work as well, current has clang 9.x in base)

mjg added a comment.Jan 26 2020, 10:54 AM

Thanks a lot, I'll look into it.

mjg updated this revision to Diff 67309.Jan 26 2020, 12:42 PM
  • handle mmap

The problem was that the following was not taken into consideration:

if ((obj = vp->v_object) != NULL && (vp->v_vflag & VV_NOSYNC) == 0 &&
    vm_object_mightbedirty(obj)) {
        VM_OBJECT_WLOCK(obj);
        vm_object_page_clean(obj, 0, 0, 0);
        VM_OBJECT_WUNLOCK(obj);
}

Now the testcase passes. Thanks a lot for testing.

This revision was not accepted when it landed; it landed in state Needs Review.Jan 30 2020, 2:14 AM
Closed by commit rS357281: zfs: use VOP_NEED_INACTIVE (authored by mjg). · Explain Why
This revision was automatically updated to reflect the committed changes.
mjg added a commit: rS357281: zfs: use VOP_NEED_INACTIVE.
GitHub <noreply@github.com> mentioned this in rG8a3d9186ba0c: Update zfs_freebsd_need_inactive to fix mmapped writes.Jun 8 2021, 2:47 PM

Revision Contents
Changeset List

PathSize
sys/
cddl/
contrib/
opensolaris/
uts/
common/
fs/
zfs/
20 lines

Diff 66167

View Options

sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_vnops.c

Loading...