Page MenuHomeFreeBSD

jah (Jason A. Harmening)
User

Projects

User Details

User Since
Mar 25 2015, 11:24 AM (401 w, 1 d)

Recent Activity

Tue, Nov 22

jah updated the diff for D37458: Various fixes related to VV_CROSSLOCK handling.

Style

Tue, Nov 22, 10:30 PM
jah added inline comments to D37458: Various fixes related to VV_CROSSLOCK handling.
Tue, Nov 22, 7:12 PM
jah added inline comments to D37458: Various fixes related to VV_CROSSLOCK handling.
Tue, Nov 22, 7:08 PM
jah added inline comments to D37458: Various fixes related to VV_CROSSLOCK handling.
Tue, Nov 22, 4:00 AM
jah requested review of D37458: Various fixes related to VV_CROSSLOCK handling.
Tue, Nov 22, 3:54 AM

Oct 29 2022

jah accepted D37198: vfs_domount(): ensure that v_mountedhere and VIRF_MOUNTPOINT are set under the vnode lock.
Oct 29 2022, 12:04 AM

Oct 28 2022

jah accepted D37198: vfs_domount(): ensure that v_mountedhere and VIRF_MOUNTPOINT are set under the vnode lock.
Oct 28 2022, 4:47 PM
jah added inline comments to D37198: vfs_domount(): ensure that v_mountedhere and VIRF_MOUNTPOINT are set under the vnode lock.
Oct 28 2022, 3:44 PM
jah accepted D37198: vfs_domount(): ensure that v_mountedhere and VIRF_MOUNTPOINT are set under the vnode lock.
Oct 28 2022, 3:32 PM
jah added a comment to D37198: vfs_domount(): ensure that v_mountedhere and VIRF_MOUNTPOINT are set under the vnode lock.

Oops, I didn't realize that the mount path doesn't hold the covered vnode lock while updating the relevant fields.
It looks as though the unmount path does hold the lock across the relevant operations, so we should be safe from these fields being cleared out from under us, correct?
Would it make sense to add a comment here to that effect?

Oct 28 2022, 3:31 PM

Oct 27 2022

jah committed rGf7833196bd6b: vfs_lookup(): Minor performance optimizations (authored by jah).
vfs_lookup(): Minor performance optimizations
Oct 27 2022, 12:25 AM
jah committed rG4390622c8d16: vfs_busy(): fix wording in comment (authored by jah).
vfs_busy(): fix wording in comment
Oct 27 2022, 12:25 AM
jah committed rG706f15c5fa6b: Remove witness directives from crossmp locking VOPs (authored by jah).
Remove witness directives from crossmp locking VOPs
Oct 27 2022, 12:25 AM
jah committed rG080ef8a41851: Add VV_CROSSLOCK vnode flag to avoid cross-mount lookup LOR (authored by jah).
Add VV_CROSSLOCK vnode flag to avoid cross-mount lookup LOR
Oct 27 2022, 12:25 AM
jah closed D35054: Add VV_CROSSLOCK vnode flag to avoid cross-mount lookup LOR.
Oct 27 2022, 12:24 AM

Oct 21 2022

jah added a comment to D35054: Add VV_CROSSLOCK vnode flag to avoid cross-mount lookup LOR.

Apologies for letting this review stall, $WORK got in the way in a big way for a while.
I've rebased, incorporated some of @mjg's optimization suggestions, and retested.

Oct 21 2022, 3:46 AM
jah updated the diff for D35054: Add VV_CROSSLOCK vnode flag to avoid cross-mount lookup LOR.

Fix wording in comment, incoporate some suggested performance optimizations

Oct 21 2022, 3:43 AM

Sep 21 2022

jah accepted D36625: Improve POSIX compliance for RLIMIT_FSIZE.
Sep 21 2022, 5:00 PM

Aug 13 2022

jah updated the diff for D35054: Add VV_CROSSLOCK vnode flag to avoid cross-mount lookup LOR.

Style

Aug 13 2022, 5:08 AM

Aug 5 2022

jah updated the diff for D35054: Add VV_CROSSLOCK vnode flag to avoid cross-mount lookup LOR.

Remove remaining WITNESS directives and no-longer used variables

Aug 5 2022, 5:49 AM

Jul 29 2022

jah accepted D35946: multimedia/cx88: Remove devclass from DRIVER_MODULE on recent main..
Jul 29 2022, 4:09 AM

Jul 27 2022

jah added inline comments to D35054: Add VV_CROSSLOCK vnode flag to avoid cross-mount lookup LOR.
Jul 27 2022, 3:56 AM
jah added a comment to D35946: multimedia/cx88: Remove devclass from DRIVER_MODULE on recent main..

Is the idea here to proactively update the DRIVER_MODULE because the devclass compat shims will be removed in a future version, or was there some build error on 14?

Jul 27 2022, 3:50 AM

Jul 25 2022

jah added inline comments to D35054: Add VV_CROSSLOCK vnode flag to avoid cross-mount lookup LOR.
Jul 25 2022, 3:41 AM
jah updated the diff for D35054: Add VV_CROSSLOCK vnode flag to avoid cross-mount lookup LOR.
  • Fix typo, restore support for LK_INTERLOCK on vp_crossmp
Jul 25 2022, 3:34 AM

Jul 20 2022

jah added inline comments to D35054: Add VV_CROSSLOCK vnode flag to avoid cross-mount lookup LOR.
Jul 20 2022, 9:27 PM
jah added inline comments to D35054: Add VV_CROSSLOCK vnode flag to avoid cross-mount lookup LOR.
Jul 20 2022, 9:22 PM

Jul 19 2022

jah updated the diff for D35054: Add VV_CROSSLOCK vnode flag to avoid cross-mount lookup LOR.
  • Replace runtime check with KASSERT, add comment on CROSSLOCK behavior
Jul 19 2022, 6:02 PM

Jun 28 2022

jah accepted D35638: UFS quotaoff: start write before unbusying.
Jun 28 2022, 10:09 PM

Jun 14 2022

jah accepted D35477: null_vptocnp(): busy nullfs mp instead of refing it.
Jun 14 2022, 5:39 AM

Jun 10 2022

jah committed R11:88aef5f532d0: multimedia/cx88: update to 1.5.4 (authored by jah).
multimedia/cx88: update to 1.5.4
Jun 10 2022, 3:44 AM

May 15 2022

jah added inline comments to D35054: Add VV_CROSSLOCK vnode flag to avoid cross-mount lookup LOR.
May 15 2022, 2:49 AM

Apr 26 2022

jah added inline comments to D35054: Add VV_CROSSLOCK vnode flag to avoid cross-mount lookup LOR.
Apr 26 2022, 12:00 AM

Apr 25 2022

jah added a comment to D35054: Add VV_CROSSLOCK vnode flag to avoid cross-mount lookup LOR.

I don't really like this change, it feels like a hack to me. At the same time, I don't have another idea that seems clearly better, so I thought I would post this to at least start a discussion.

Apr 25 2022, 3:57 AM
jah requested review of D35054: Add VV_CROSSLOCK vnode flag to avoid cross-mount lookup LOR.
Apr 25 2022, 3:50 AM

Mar 9 2022

jah added inline comments to D34466: shared-locked lookup for ".." on nullfs.
Mar 9 2022, 5:33 PM

Feb 26 2022

jah accepted D34381: rmlock: Add required compiler barriers to _rm_runlock().
Feb 26 2022, 6:26 PM
jah added inline comments to D34381: rmlock: Add required compiler barriers to _rm_runlock().
Feb 26 2022, 12:20 AM

Feb 25 2022

jah accepted D34377: rmlock: Micro-optimize read locking.
Feb 25 2022, 5:15 PM

Feb 24 2022

jah committed rGfcb164742b6f: unionfs: rework unionfs_getwritemount() (authored by jah).
unionfs: rework unionfs_getwritemount()
Feb 24 2022, 4:01 AM
jah closed D34282: unionfs: rework unionfs_getwritemount().
Feb 24 2022, 4:01 AM

Feb 18 2022

jah updated the diff for D34282: unionfs: rework unionfs_getwritemount().

prune whitespace

Feb 18 2022, 4:52 AM
jah updated the diff for D34282: unionfs: rework unionfs_getwritemount().

Remove unnecessary MNT_RDONLY check

Feb 18 2022, 4:51 AM

Feb 17 2022

jah updated the diff for D34282: unionfs: rework unionfs_getwritemount().

Return EACCES if we can't find a suitable upper vnode; add comments and assertion

Feb 17 2022, 8:33 PM

Feb 16 2022

jah added inline comments to D34282: unionfs: rework unionfs_getwritemount().
Feb 16 2022, 7:39 PM
jah added inline comments to D34282: unionfs: rework unionfs_getwritemount().
Feb 16 2022, 5:06 PM
jah added inline comments to D34282: unionfs: rework unionfs_getwritemount().
Feb 16 2022, 2:35 AM

Feb 15 2022

jah added inline comments to D34282: unionfs: rework unionfs_getwritemount().
Feb 15 2022, 8:41 PM
jah added inline comments to D34282: unionfs: rework unionfs_getwritemount().
Feb 15 2022, 8:39 PM
jah added inline comments to D34282: unionfs: rework unionfs_getwritemount().
Feb 15 2022, 7:43 PM
jah added inline comments to D34282: unionfs: rework unionfs_getwritemount().
Feb 15 2022, 4:01 AM
jah updated the summary of D34282: unionfs: rework unionfs_getwritemount().
Feb 15 2022, 3:55 AM
jah added inline comments to D34282: unionfs: rework unionfs_getwritemount().
Feb 15 2022, 3:48 AM
jah requested review of D34282: unionfs: rework unionfs_getwritemount().
Feb 15 2022, 3:45 AM

Feb 10 2022

jah committed rG974efbb3d5c0: unionfs: fix typo in comment (authored by jah).
unionfs: fix typo in comment
Feb 10 2022, 9:10 PM

Feb 3 2022

jah committed rG83d61d5b7300: unionfs: do not force LK_NOWAIT if VI_OWEINACT is set (authored by jah).
unionfs: do not force LK_NOWAIT if VI_OWEINACT is set
Feb 3 2022, 2:59 AM
jah committed rG6ff167aa427c: unionfs: allow lock recursion when reclaiming the root vnode (authored by jah).
unionfs: allow lock recursion when reclaiming the root vnode
Feb 3 2022, 2:59 AM
jah committed rG0cd8f3e958a5: unionfs: fix assertion order in unionfs_lock() (authored by jah).
unionfs: fix assertion order in unionfs_lock()
Feb 3 2022, 2:59 AM
jah closed D34109: assorted small fixes to unionfs_lock().
Feb 3 2022, 2:58 AM

Jan 31 2022

jah updated the diff for D34109: assorted small fixes to unionfs_lock().

KASSERT->VNASSERT

Jan 31 2022, 5:02 PM
jah retitled D34109: assorted small fixes to unionfs_lock() from unionfs: fix assertion order in unionfs_lock() to assorted small fixes to unionfs_lock().
Jan 31 2022, 1:56 AM

Jan 30 2022

jah added inline comments to D34109: assorted small fixes to unionfs_lock().
Jan 30 2022, 9:29 PM
jah added reviewers for D34109: assorted small fixes to unionfs_lock(): kib, markj, pho.
Jan 30 2022, 9:22 PM
jah requested review of D34109: assorted small fixes to unionfs_lock().
Jan 30 2022, 9:14 PM
jah committed rGa01ca46b9b86: unionfs: use VV_ROOT to check for root vnode in unionfs_lock() (authored by jah).
unionfs: use VV_ROOT to check for root vnode in unionfs_lock()
Jan 30 2022, 4:29 AM
jah closed D33914: unionfs: use VV_ROOT to check for root vnode in unionfs_lock().
Jan 30 2022, 4:28 AM
jah updated the diff for D33914: unionfs: use VV_ROOT to check for root vnode in unionfs_lock().

Add comment explaining VV_ROOT check

Jan 30 2022, 12:27 AM
jah added inline comments to D33914: unionfs: use VV_ROOT to check for root vnode in unionfs_lock().
Jan 30 2022, 12:23 AM

Jan 26 2022

jah added inline comments to D33914: unionfs: use VV_ROOT to check for root vnode in unionfs_lock().
Jan 26 2022, 11:57 PM

Jan 25 2022

jah added inline comments to D33914: unionfs: use VV_ROOT to check for root vnode in unionfs_lock().
Jan 25 2022, 3:54 PM
jah added inline comments to D33914: unionfs: use VV_ROOT to check for root vnode in unionfs_lock().
Jan 25 2022, 4:52 AM

Jan 24 2022

jah added inline comments to D33914: unionfs: use VV_ROOT to check for root vnode in unionfs_lock().
Jan 24 2022, 4:14 PM
jah updated the diff for D33914: unionfs: use VV_ROOT to check for root vnode in unionfs_lock().

Restore VV_ROOT management to unionfs_nodeget()

Jan 24 2022, 2:33 AM

Jan 18 2022

jah added a comment to D33914: unionfs: use VV_ROOT to check for root vnode in unionfs_lock().
In D33914#766976, @jah wrote:
In D33914#766971, @kib wrote:

You can see yourself what would be the breakage for dotdots: in vfs_lookup.c, the block starting at line 1038 if (cnp->cn_flags & ISDOTDOT) { checks for VV_ROOT to detect the case when it should walk over the mount point instead of calling VOP_LOOKUP(). Most likely, it would result in either upper or lower filesystem call to VOP_LOOKUP(), which again would either break VFS invariant that dotdot lookup is never done on VV_ROOT, or (if unionfs allows sub-directory covering, nullfs does allow it) causes exposition of the vnodes not supposed to be accessed through this mount.

Right now I think that indeed, there are only forced unmount and debugging sysctls that result in the referenced vnode reclamation. But VFS does not guarantee that, and we might grow additional cases for reclaim without breaking the promises.

The other reason I think this *might* not be an issue in practice is that the unionfs node keeps a reference to the parent directory vnode, which is returned for ISDOTDOT lookups instead of calling unionfs_nodeget().
But it also seems as though there are enough corner cases and things that might change in the future here, that it's probably better to restore the VV_ROOT logic in unionfs_nodeget().

Jan 18 2022, 3:09 AM

Jan 17 2022

jah added a comment to D33914: unionfs: use VV_ROOT to check for root vnode in unionfs_lock().
In D33914#766971, @kib wrote:

You can see yourself what would be the breakage for dotdots: in vfs_lookup.c, the block starting at line 1038 if (cnp->cn_flags & ISDOTDOT) { checks for VV_ROOT to detect the case when it should walk over the mount point instead of calling VOP_LOOKUP(). Most likely, it would result in either upper or lower filesystem call to VOP_LOOKUP(), which again would either break VFS invariant that dotdot lookup is never done on VV_ROOT, or (if unionfs allows sub-directory covering, nullfs does allow it) causes exposition of the vnodes not supposed to be accessed through this mount.

Right now I think that indeed, there are only forced unmount and debugging sysctls that result in the referenced vnode reclamation. But VFS does not guarantee that, and we might grow additional cases for reclaim without breaking the promises.

Jan 17 2022, 3:08 PM
jah added a comment to D33914: unionfs: use VV_ROOT to check for root vnode in unionfs_lock().
In D33914#766955, @kib wrote:
In D33914#766951, @jah wrote:
In D33914#766950, @kib wrote:

What if um_rootvp is reclaimed?

Would that matter as far as the check for VV_ROOT is concerned? It looks as though v_vflag isn't cleared until the vnode is freed, and we always keep a reference to um_rootvp.
Also, unionfs_root() directly returns um_rootvp instead of calling unionfs_nodeget().

Suppose that um_rootvp is reclaimed, and another unionfs vnode is instantiated over corresponding upper or lower vnode. In this case that new vnode does not get VV_ROOT set. This breaks dotdot lookups at least.

Jan 17 2022, 2:25 PM
jah added a comment to D33914: unionfs: use VV_ROOT to check for root vnode in unionfs_lock().
In D33914#766950, @kib wrote:

What if um_rootvp is reclaimed?

Jan 17 2022, 1:58 PM
jah requested review of D33914: unionfs: use VV_ROOT to check for root vnode in unionfs_lock().
Jan 17 2022, 1:02 AM

Jan 12 2022

jah committed rGa565cc81c04e: unionfs: add stress2 scenarios for write references (authored by jah).
unionfs: add stress2 scenarios for write references
Jan 12 2022, 2:37 AM
jah committed rG39a2dc44f8d9: unionfs: allow vnode lock to be held shared during VOP_OPEN (authored by jah).
unionfs: allow vnode lock to be held shared during VOP_OPEN
Jan 12 2022, 2:37 AM
jah closed D33729: unionfs: allow vnode lock to be held shared during VOP_OPEN.
Jan 12 2022, 2:37 AM

Jan 6 2022

jah added a comment to D33729: unionfs: allow vnode lock to be held shared during VOP_OPEN.

I'm sure you've observed this already, but this behaviour of potentially dropping the vnode lock seems dubious: the vnode interface doesn't say anything about whether a particular VOP implementation is allowed to do that, and callers might assume that it doesn't happen. Moreover, lock upgrades will often succeed, so problems that arise from dropping the lock will be rare and hard to debug. Maybe unionfs can use an internal lock to provide mutual exclusion without having to upgrade, but that is probably a simplistic idea, or maybe this is actually not much of a problem in practice.

Seems good to me, but I didn't really look at the tests.

Jan 6 2022, 10:48 PM
jah added inline comments to D33729: unionfs: allow vnode lock to be held shared during VOP_OPEN.
Jan 6 2022, 7:06 PM
jah updated the diff for D33729: unionfs: allow vnode lock to be held shared during VOP_OPEN.

Factor out lock upgrade/downgrade

Jan 6 2022, 6:17 PM

Jan 4 2022

jah updated the diff for D33729: unionfs: allow vnode lock to be held shared during VOP_OPEN.

Check for reclaimed vnodes in various places, chmod +x unionfs1[0-2].sh

Jan 4 2022, 11:29 PM

Jan 3 2022

jah added inline comments to D33729: unionfs: allow vnode lock to be held shared during VOP_OPEN.
Jan 3 2022, 11:22 PM
jah requested review of D33729: unionfs: allow vnode lock to be held shared during VOP_OPEN.
Jan 3 2022, 2:56 PM
jah committed rGd877dd5767bc: unionfs: simplify writecount management (authored by jah).
unionfs: simplify writecount management
Jan 3 2022, 3:45 AM
jah committed rG9e891d43f586: unionfs: implement VOP_SET_TEXT/VOP_UNSET_TEXT (authored by jah).
unionfs: implement VOP_SET_TEXT/VOP_UNSET_TEXT
Jan 3 2022, 3:45 AM
jah closed D33611: unionfs: simplify writecount management.
Jan 3 2022, 3:45 AM

Dec 30 2021

jah updated the diff for D33611: unionfs: simplify writecount management.

Improve assert message

Dec 30 2021, 5:40 AM
jah updated the diff for D33611: unionfs: simplify writecount management.

Also simplify writecount management in unionfs_noderem()

Dec 30 2021, 4:03 AM
jah added inline comments to D33611: unionfs: simplify writecount management.
Dec 30 2021, 4:02 AM

Dec 29 2021

jah updated the diff for D33611: unionfs: simplify writecount management.

Only allow write refs on upper vnode

Dec 29 2021, 3:11 AM

Dec 28 2021

jah added inline comments to D33611: unionfs: simplify writecount management.
Dec 28 2021, 11:35 PM

Dec 27 2021

jah added inline comments to D33611: unionfs: simplify writecount management.
Dec 27 2021, 9:29 PM
jah added inline comments to D33611: unionfs: simplify writecount management.
Dec 27 2021, 6:45 PM

Dec 25 2021

jah retitled D33611: unionfs: simplify writecount management from unionfs: leave writecount management to underlying filesystem to unionfs: simplify writecount management.
Dec 25 2021, 8:28 PM
jah updated the diff for D33611: unionfs: simplify writecount management.

Restore writecount tracking on unionfs vnode, in simplified form

Dec 25 2021, 8:26 PM

Dec 24 2021

jah added a comment to D33611: unionfs: simplify writecount management.
In D33611#759740, @kib wrote:
In D33611#759655, @jah wrote:

Ok, will do. On a related note, I did test nullfs enough to know that it doesn't have the same panic-on-process-termination issue as unionfs. So for nullfs the text ref seems to be managed on the correct vnode even though nullfs also does not implement VOP_[UN]SET_TEXT.
I assume this is because of null_bypass? If so would it be sufficient to simply delete null_add_writecount?

Yes, null_bypass results in text references go onto the lower vnode.

I mostly agree with the note that maintaining writecount on the upper vnode is effectively nop. So I am interested in see if nullfs can work without it, and indeed for nullfs the corresponding change would be to remove the custom bypass.

Dec 24 2021, 3:34 AM