Page MenuHomeFreeBSD

jah (Jason A. Harmening)
User

Projects

User Details

User Since
Mar 25 2015, 11:24 AM (325 w, 2 d)

Recent Activity

Wed, Jun 16

jah updated the diff for D30748: unionfs: release parent vnodes in deferred context.

Use only CTLFLAG_RD

Wed, Jun 16, 1:54 AM
jah added a comment to D30748: unionfs: release parent vnodes in deferred context.
In D30748#691844, @jah wrote:
In D30748#691598, @jah wrote:
In D30748#691212, @mjg wrote:

If memory serves unionfs panics on mount if DEBUG_VFS_LOCKS is enabled, but the patch needs to be tested with it. I don't remember what stands in the way of fixing it, but bare minimum the triggering assert can be commented out for testing.

I didn't know we had DEBUG_VFS_LOCKS. My guess is that unionfs under stress will fail these checks in a variety of ways, but hopefully none of them will be caused by this change. This seems like another useful tool for evaluating unionfs, so I'll give it a shot as soon as I get some time.

Surprisingly, the panic on mount (new vnode not exclusively locked for insmntque()) is the only issue I've found in testing with DEBUG_VFS_LOCKS so far.

Wed, Jun 16, 12:50 AM

Tue, Jun 15

jah updated the diff for D30748: unionfs: release parent vnodes in deferred context.

Quiesce the taskqueue on module unload

Tue, Jun 15, 1:17 PM
jah added a comment to D30748: unionfs: release parent vnodes in deferred context.
In D30748#691785, @kib wrote:
In D30748#691607, @jah wrote:
In D30748#691229, @kib wrote:

I think it is useful, if not required, to drain the taskqueue on unmount.

This could be helpful in making it more likely for non-forced unmounts to succeed if there have been very recent deletions. Would you consider that a requirement?

My motivation for the suggestion was that the module cannot be unloaded until taskqueue is drained, and it is logical to drain on unmount to ensure that unmount is actually completed, i.e. as much resources related to the filesystem is freed as possible.

But now I think that also you need to terminate and clean up the taskqueue thread on vfs module unload.

Tue, Jun 15, 1:15 PM
jah added a comment to D30748: unionfs: release parent vnodes in deferred context.
In D30748#691598, @jah wrote:
In D30748#691212, @mjg wrote:

If memory serves unionfs panics on mount if DEBUG_VFS_LOCKS is enabled, but the patch needs to be tested with it. I don't remember what stands in the way of fixing it, but bare minimum the triggering assert can be commented out for testing.

I didn't know we had DEBUG_VFS_LOCKS. My guess is that unionfs under stress will fail these checks in a variety of ways, but hopefully none of them will be caused by this change. This seems like another useful tool for evaluating unionfs, so I'll give it a shot as soon as I get some time.

Tue, Jun 15, 1:08 PM

Mon, Jun 14

jah added a comment to D30748: unionfs: release parent vnodes in deferred context.
In D30748#691229, @kib wrote:

I think it is useful, if not required, to drain the taskqueue on unmount.

Mon, Jun 14, 5:44 PM
jah added a comment to D30748: unionfs: release parent vnodes in deferred context.
In D30748#691212, @mjg wrote:

If memory serves unionfs panics on mount if DEBUG_VFS_LOCKS is enabled, but the patch needs to be tested with it. I don't remember what stands in the way of fixing it, but bare minimum the triggering assert can be commented out for testing.

Mon, Jun 14, 5:37 PM

Sun, Jun 13

jah updated the diff for D30748: unionfs: release parent vnodes in deferred context.

FIFO processing for the deferred list

Sun, Jun 13, 2:28 PM
jah updated the diff for D30748: unionfs: release parent vnodes in deferred context.

Use dedicated taskqueue, use STAILQ_CONCAT to reduce list locking

Sun, Jun 13, 1:49 PM
jah added inline comments to D30748: unionfs: release parent vnodes in deferred context.
Sun, Jun 13, 12:25 AM

Sat, Jun 12

jah added a comment to D30748: unionfs: release parent vnodes in deferred context.

This is a naive approach. Instead of using a taskqueue, would I be better of with something completely different? For example, could I just stop holding a reference to the parent and instead lookup the parent through namei in the relatively small number of cases where unionfs needs the parent? Or, if the taskqueue is an acceptable approach, should I instead use a dedicated taskqueue instead of funneling everything through taskqueue_thread?

Sat, Jun 12, 8:41 PM
jah requested review of D30748: unionfs: release parent vnodes in deferred context.
Sat, Jun 12, 8:37 PM

Sun, Jun 6

jah closed D30401: Add a generic mechanism for preventing forced unmount.
Sun, Jun 6, 1:18 AM
jah committed R10:59409cb90fc0: Add a generic mechanism for preventing forced unmount (authored by jah).
Add a generic mechanism for preventing forced unmount
Sun, Jun 6, 1:18 AM

Sun, May 30

jah closed D30556: VFS_QUOTACTL(9): allow implementation to indicate busy state changes.
Sun, May 30, 9:53 PM
jah committed R10:a4b07a2701f5: VFS_QUOTACTL(9): allow implementation to indicate busy state changes (authored by jah).
VFS_QUOTACTL(9): allow implementation to indicate busy state changes
Sun, May 30, 9:53 PM
jah added a comment to D30556: VFS_QUOTACTL(9): allow implementation to indicate busy state changes.
In D30556#686194, @jah wrote:

This is the same change you've already reviewed, with 2 exceptions:
--explicit inclusion of sys/types.h for _KERNEL builds from sys/mount.h, to avoid relying on coincidentally getting the header from elsewhere.
--inclusion of stdbool.h for libprocstat modules whose use of kernel headers otherwise defeats the definition of type bool.

For the second issue, I would be willing to drop the libprocstat changes and instead use _Bool in VFS_QUOTACTL() if anyone insists on it. However, my personal preference is strongly for 'bool' over '_Bool' as a matter of style, and we seem to be slowly standardizing on 'bool' in the kernel. My take is that we shouldn't need to warp KPIs in order to accommodate userspace code that does sketchy things with kernel headers, especially when the code doing said sketchy things can employ a simple workaround.

I tend to agree that we should avoid mixing _Bool and bool in the kernel. It's just confusing. The change you proposed is ok with me, with the caveat that a similar modification may be needed in a small number of ports that do something like libprocstat (lsof is the main one that comes to mind). I'm ok with kib's suggestions as well.

Sun, May 30, 8:59 PM
jah added a comment to D30556: VFS_QUOTACTL(9): allow implementation to indicate busy state changes.

This is the same change you've already reviewed, with 2 exceptions:
--explicit inclusion of sys/types.h for _KERNEL builds from sys/mount.h, to avoid relying on coincidentally getting the header from elsewhere.
--inclusion of stdbool.h for libprocstat modules whose use of kernel headers otherwise defeats the definition of type bool.

Sun, May 30, 6:33 PM
jah requested review of D30556: VFS_QUOTACTL(9): allow implementation to indicate busy state changes.
Sun, May 30, 6:27 PM
jah added a reverting change for R10:6d3e78ad6c11: VFS_QUOTACTL(9): allow implementation to indicate busy state changes: R10:271fcf1c28ef: Revert commits 6d3e78ad6c11 and 54256e7954d7.
Sun, May 30, 12:49 AM
jah added a reverting change for D30218: VFS_QUOTACTL(9): allow implementation to indicate busy state changes: R10:271fcf1c28ef: Revert commits 6d3e78ad6c11 and 54256e7954d7.
Sun, May 30, 12:48 AM
jah added a reverting change for R10:54256e7954d7: Fix userspace build after commit 6d3e78ad6c11: R10:271fcf1c28ef: Revert commits 6d3e78ad6c11 and 54256e7954d7.
Sun, May 30, 12:48 AM
jah committed R10:271fcf1c28ef: Revert commits 6d3e78ad6c11 and 54256e7954d7 (authored by jah).
Revert commits 6d3e78ad6c11 and 54256e7954d7
Sun, May 30, 12:48 AM

Sat, May 29

jah committed R10:54256e7954d7: Fix userspace build after commit 6d3e78ad6c11 (authored by jah).
Fix userspace build after commit 6d3e78ad6c11
Sat, May 29, 9:42 PM
jah committed R10:6d3e78ad6c11: VFS_QUOTACTL(9): allow implementation to indicate busy state changes (authored by jah).
VFS_QUOTACTL(9): allow implementation to indicate busy state changes
Sat, May 29, 9:05 PM
jah closed D30218: VFS_QUOTACTL(9): allow implementation to indicate busy state changes.
Sat, May 29, 9:05 PM

Tue, May 25

jah updated the diff for D30218: VFS_QUOTACTL(9): allow implementation to indicate busy state changes.

Add comments around unbusying requirements, chase __FreeBSD_version changes

Tue, May 25, 6:27 PM
jah abandoned D30152: unionfs: reference the underlying FS' mount objects when using them.
Tue, May 25, 6:06 PM
jah abandoned D30263: nullfs: busy the lower mount before calling VFS_* operations on it.

Superseded by https://reviews.freebsd.org/D30401

Tue, May 25, 6:05 PM
jah planned changes to D30152: unionfs: reference the underlying FS' mount objects when using them.

Superseded by https://reviews.freebsd.org/D30401

Tue, May 25, 6:04 PM
jah updated the diff for D30401: Add a generic mechanism for preventing forced unmount.

Remove stray newline

Tue, May 25, 5:39 PM
jah updated the diff for D30401: Add a generic mechanism for preventing forced unmount.

Add and tweak various asserts

Tue, May 25, 3:14 AM

Sun, May 23

jah requested review of D30401: Add a generic mechanism for preventing forced unmount.
Sun, May 23, 3:32 AM

Fri, May 21

jah updated the diff for D30218: VFS_QUOTACTL(9): allow implementation to indicate busy state changes.

unbusy the unionfs/nullfs mount before calling QUOTACTL on the lower FS

Fri, May 21, 7:45 PM

May 17 2021

jah added inline comments to D30218: VFS_QUOTACTL(9): allow implementation to indicate busy state changes.
May 17 2021, 11:23 PM
jah added a comment to D30218: VFS_QUOTACTL(9): allow implementation to indicate busy state changes.
In D30218#680984, @jah wrote:

Add basic fixes for nullfs and unionfs, conditionalize ZFS changes

May 17 2021, 11:21 PM
jah updated the diff for D30218: VFS_QUOTACTL(9): allow implementation to indicate busy state changes.

Add basic fixes for nullfs and unionfs, conditionalize ZFS changes

May 17 2021, 11:17 PM
jah added inline comments to D30263: nullfs: busy the lower mount before calling VFS_* operations on it.
May 17 2021, 2:06 AM

May 16 2021

jah added inline comments to D30263: nullfs: busy the lower mount before calling VFS_* operations on it.
May 16 2021, 1:44 AM

May 15 2021

jah added inline comments to D30263: nullfs: busy the lower mount before calling VFS_* operations on it.
May 15 2021, 11:12 PM
jah added inline comments to D30263: nullfs: busy the lower mount before calling VFS_* operations on it.
May 15 2021, 9:40 PM
jah added inline comments to D30263: nullfs: busy the lower mount before calling VFS_* operations on it.
May 15 2021, 9:25 PM
jah added inline comments to D30263: nullfs: busy the lower mount before calling VFS_* operations on it.
May 15 2021, 8:55 PM

May 14 2021

jah requested review of D30263: nullfs: busy the lower mount before calling VFS_* operations on it.
May 14 2021, 5:17 PM
jah updated the diff for D30152: unionfs: reference the underlying FS' mount objects when using them.

Replace vfs_ref() with simple atomic load of v_mount

May 14 2021, 12:40 AM
jah added inline comments to D30218: VFS_QUOTACTL(9): allow implementation to indicate busy state changes.
May 14 2021, 12:08 AM

May 13 2021

jah added inline comments to D30218: VFS_QUOTACTL(9): allow implementation to indicate busy state changes.
May 13 2021, 7:37 PM
jah added inline comments to D30152: unionfs: reference the underlying FS' mount objects when using them.
May 13 2021, 3:44 PM
jah added inline comments to D30218: VFS_QUOTACTL(9): allow implementation to indicate busy state changes.
May 13 2021, 2:50 PM
jah updated the diff for D30218: VFS_QUOTACTL(9): allow implementation to indicate busy state changes.

Fix manpage formatting, remove leftover include, fix incorrect deleted line due to fingerslip

May 13 2021, 4:58 AM
jah retitled D30218: VFS_QUOTACTL(9): allow implementation to indicate busy state changes from Follow mount point unbusying requirements in vfs_stdquotactl() to VFS_QUOTACTL(9): allow implementation to indicate busy state changes.
May 13 2021, 2:11 AM
jah updated the diff for D30152: unionfs: reference the underlying FS' mount objects when using them.

Chase D30218

May 13 2021, 2:06 AM
jah added inline comments to D30218: VFS_QUOTACTL(9): allow implementation to indicate busy state changes.
May 13 2021, 1:56 AM
jah updated the diff for D30218: VFS_QUOTACTL(9): allow implementation to indicate busy state changes.

Allow the implementation to direct busy state changes

May 13 2021, 1:55 AM
jah added a comment to D30152: unionfs: reference the underlying FS' mount objects when using them.

Do you plan to revise this change after changing the VFS_QUOTACTL interface? I'm just not sure whether to wait until that's done before re-reading this.

May 13 2021, 1:53 AM

May 12 2021

jah updated the diff for D30152: unionfs: reference the underlying FS' mount objects when using them.

Use correct variable when passing through VFS_QUOTACTL

May 12 2021, 2:43 AM

May 11 2021

jah added inline comments to D30218: VFS_QUOTACTL(9): allow implementation to indicate busy state changes.
May 11 2021, 7:06 PM
jah requested review of D30218: VFS_QUOTACTL(9): allow implementation to indicate busy state changes.
May 11 2021, 7:03 PM
jah updated the diff for D30152: unionfs: reference the underlying FS' mount objects when using them.

Follow the unbusy requirements for vfs_quotactl()

May 11 2021, 7:00 PM

May 10 2021

jah added inline comments to D30152: unionfs: reference the underlying FS' mount objects when using them.
May 10 2021, 11:46 PM
jah updated the diff for D30152: unionfs: reference the underlying FS' mount objects when using them.

Style

May 10 2021, 4:51 PM

May 7 2021

jah updated the diff for D30152: unionfs: reference the underlying FS' mount objects when using them.

Use atomic_load_ptr to enforce single load of vp->v_mount

May 7 2021, 6:06 PM
jah updated the diff for D30152: unionfs: reference the underlying FS' mount objects when using them.

Apply vfs_busy, minor refactoring

May 7 2021, 5:48 AM
jah added a comment to D30152: unionfs: reference the underlying FS' mount objects when using them.
In D30152#677009, @kib wrote:
In D30152#677007, @jah wrote:
In D30152#676937, @jah wrote:

A simpler approach would be to directly store the underlying mount objects in struct unionfs_mount, vfs_ref() them on mount and vfs_rel() them on unmount.
But that would have the effect of blocking forcible unmount of the underlying filesystems in vfs_mount_destroy(). Meanwhile, the underlying FS' vnodes would still be flushed before this happens so the underlying filesystems would be no more usable than they are with this approach.

In D30152#676980, @kib wrote:

Taking a reference on the mount point does not prevent unmount. For instance, calling VFS_QUOTACTL() requires busying mp, which, if successful, prevents unmount until unbusy. In fact, busying fs is currently enforced by VFS_QUOTACTL protocol since UFS (the only actual quotactl provider) in some cases unbusies and then busies the mp.

It certainly doesn't prevent the unmount operations (vflush, etc) from happening, but doesn't it prevent the unmount system call from returning? That's what I observed in testing, and is corroborated by reading vfs_mount_destroy().

Why 'preventing syscall from returning' matters? What matters is that unmount destroys the private mount data pointed to by mnt_data. This data is usually vital for VFS_XXX() methods, and destroying/freeing it while these methods operate causes undefined behavior, a crash in the best case.

May 7 2021, 2:58 AM
jah added a comment to D30152: unionfs: reference the underlying FS' mount objects when using them.
In D30152#676937, @jah wrote:

A simpler approach would be to directly store the underlying mount objects in struct unionfs_mount, vfs_ref() them on mount and vfs_rel() them on unmount.
But that would have the effect of blocking forcible unmount of the underlying filesystems in vfs_mount_destroy(). Meanwhile, the underlying FS' vnodes would still be flushed before this happens so the underlying filesystems would be no more usable than they are with this approach.

May 7 2021, 12:51 AM

May 6 2021

jah added a comment to D30152: unionfs: reference the underlying FS' mount objects when using them.

A simpler approach would be to directly store the underlying mount objects in struct unionfs_mount, vfs_ref() them on mount and vfs_rel() them on unmount.
But that would have the effect of blocking forcible unmount of the underlying filesystems in vfs_mount_destroy(). Meanwhile, the underlying FS' vnodes would still be flushed before this happens so the underlying filesystems would be no more usable than they are with this approach.

May 6 2021, 9:57 PM
jah requested review of D30152: unionfs: reference the underlying FS' mount objects when using them.
May 6 2021, 9:54 PM

Mar 31 2021

jah committed R10:8dc8feb53da0: Clean up a couple of MD warts in vm_fault_populate(): (authored by jah).
Clean up a couple of MD warts in vm_fault_populate():
Mar 31 2021, 1:18 AM
jah closed D29439: Clean up a couple of MD warts in vm_fault_populate():.
Mar 31 2021, 1:18 AM

Mar 27 2021

jah updated the diff for D29439: Clean up a couple of MD warts in vm_fault_populate():.

Adjust wording in comment, rearrange return value assertion

Mar 27 2021, 8:42 PM
jah accepted D29442: vm_fault: handle KERN_PROTECTION_FAILURE.
Mar 27 2021, 2:00 PM
jah added inline comments to D29439: Clean up a couple of MD warts in vm_fault_populate():.
Mar 27 2021, 3:23 AM
jah added reviewers for D29439: Clean up a couple of MD warts in vm_fault_populate():: kib, markj, PowerPC, MIPS.
Mar 27 2021, 3:19 AM
jah requested review of D29439: Clean up a couple of MD warts in vm_fault_populate():.
Mar 27 2021, 3:17 AM
jah added inline comments to D28717: vm_page_grab*: Consolidate common logic into vm_page_grab_release().
Mar 27 2021, 2:46 AM

Mar 21 2021

jah committed R10:d22883d71544: Remove PCPU_INC (authored by jah).
Remove PCPU_INC
Mar 21 2021, 2:30 AM
jah closed D29308: Remove PCPU_INC.
Mar 21 2021, 2:30 AM

Mar 17 2021

jah requested review of D29308: Remove PCPU_INC.
Mar 17 2021, 1:25 AM

Mar 16 2021

jah closed D29151: factor out PT page allocation/freeing.
Mar 16 2021, 12:15 AM
jah committed R10:c2460d7cfe9f: factor out PT page allocation/freeing (authored by jah).
factor out PT page allocation/freeing
Mar 16 2021, 12:15 AM

Mar 14 2021

jah added inline comments to D29151: factor out PT page allocation/freeing.
Mar 14 2021, 3:50 PM

Mar 13 2021

jah added inline comments to D29151: factor out PT page allocation/freeing.
Mar 13 2021, 7:19 PM
jah updated the diff for D29151: factor out PT page allocation/freeing.

Fix la57 table accounting

Mar 13 2021, 5:28 PM
jah added inline comments to D29151: factor out PT page allocation/freeing.
Mar 13 2021, 5:26 PM
jah updated the diff for D29151: factor out PT page allocation/freeing.

--remove unnecessary include
--add la57 bootstrap accounting
--further cleanup to improve code reuse

Mar 13 2021, 6:22 AM
jah added inline comments to D29151: factor out PT page allocation/freeing.
Mar 13 2021, 4:24 AM
jah updated the diff for D29151: factor out PT page allocation/freeing.

Revert accidentally-committed PCID counter removal, incorporate resident_count

Mar 13 2021, 2:40 AM

Mar 10 2021

jah added inline comments to D29151: factor out PT page allocation/freeing.
Mar 10 2021, 4:50 PM
jah added inline comments to D29151: factor out PT page allocation/freeing.
Mar 10 2021, 5:05 AM
jah added inline comments to D29151: factor out PT page allocation/freeing.
Mar 10 2021, 2:59 AM
jah added inline comments to D29151: factor out PT page allocation/freeing.
Mar 10 2021, 2:52 AM

Mar 9 2021

jah added inline comments to D29151: factor out PT page allocation/freeing.
Mar 9 2021, 6:00 PM
jah requested review of D29151: factor out PT page allocation/freeing.
Mar 9 2021, 5:56 PM
jah committed R10:e4b8deb22227: amd64 pmap: convert to counter(9), add PV and pagetable page counts (authored by jah).
amd64 pmap: convert to counter(9), add PV and pagetable page counts
Mar 9 2021, 5:36 PM
jah closed D28923: amd64 pmap: convert to counter(9), add PV and pagetable page counts.
Mar 9 2021, 5:35 PM

Mar 3 2021

jah added a comment to D28923: amd64 pmap: convert to counter(9), add PV and pagetable page counts.
In D28923#650048, @kib wrote:

I probably should have been more explicit, I proposed to drop the pt_page_count at all. But I hope that you will follow up with pmap_pt_page_alloc() patch so this does not matter much.

Mar 3 2021, 4:47 PM
jah updated the diff for D28923: amd64 pmap: convert to counter(9), add PV and pagetable page counts.
  • Revert "Remove PCID save count, factor out PT page allocation/freeing"
  • Fix NULL check in pmap_large_map_getptp_unlocked()
Mar 3 2021, 4:04 AM

Mar 1 2021

jah accepted D28869: multimedia/cx88: Fix build with default PIE..
Mar 1 2021, 8:28 PM
jah updated the diff for D28923: amd64 pmap: convert to counter(9), add PV and pagetable page counts.
  • Remove PCID save count, factor out PT page allocation/freeing
Mar 1 2021, 4:42 PM

Feb 27 2021

jah added inline comments to D28923: amd64 pmap: convert to counter(9), add PV and pagetable page counts.
Feb 27 2021, 6:07 PM