Page MenuHomeFreeBSD

mckusick (Marshall Kirk McKusick)
User

Projects

User Details

User Since
Oct 24 2014, 7:17 PM (353 w, 5 d)

Recent Activity

Thu, Jul 29

mckusick committed R10:a91716efeb68: Clean up orphaned indirdep dependency structures after disk failure. (authored by mckusick).
Clean up orphaned indirdep dependency structures after disk failure.
Thu, Jul 29, 11:33 PM
mckusick committed R10:412b5e40a721: Diagnotic improvement to soft dependency structure management. (authored by mckusick).
Diagnotic improvement to soft dependency structure management.
Thu, Jul 29, 11:15 PM

Mon, Jul 19

mckusick accepted D31016: Allow stacked filesystems to be recursively unmounted.

I am fine with the structure of this patch. I do ask that you await passing Peter Holm's tests before committing.

Mon, Jul 19, 11:42 PM
mckusick added a comment to D31016: Allow stacked filesystems to be recursively unmounted.

This reminds me of something that occurred to me while I was writing the code though: these flags are effectively prevented from being exposed to userspace because they're outside the 32-bit range of unmount_args->flags.
It seems a bit fragile to rely on that though...should we have something like an MNT_KERNEL mask for flags that are only allowed to be passed from kernel-initiated operations, and return EINVAL for any userspace request that includes any of them?

In theory MNT_VISFLAGMASK are externally visible and MNT_CMDFLAGS are internal only. The MNT_VISFLAGMASK flags are the only ones returned in a statfs(2) call but are not checked on system calls as far as I can tell.
Adding some clarity / enforcement would likely be a good idea, but not as part of these changes.

Mon, Jul 19, 11:39 PM
mckusick added a comment to D29754: Augment systat(1) -swap to display large swap space processes.

I tested the 2 kernel changes both applied. In short, it helps since the 2nd refresh.

With or without the kernel changes, systat takes long time to return from the call. During that period, top, ps and few others freeze, although xosview draws memory and swap usage fine. Without the kernel changes, each systat refresh takes same amount of time and causes some others to hung. On the other hand, with the kernel changes, systat redraws significantly faster for the 2nd refresh and thereafter and also others like top don't hung.
So, the first call is an expensive call.

Mon, Jul 19, 10:59 PM

Sat, Jul 17

mckusick added a comment to D31016: Allow stacked filesystems to be recursively unmounted.

Suggestion on how to run forcible unmount test on UFS.

Sat, Jul 17, 12:33 AM

Wed, Jul 14

mckusick added a comment to D31016: Allow stacked filesystems to be recursively unmounted.

@jah - are you ready to have Peter Holm test these changes? If so, I will enlist his help.

Wed, Jul 14, 7:34 PM

Tue, Jul 13

mckusick added inline comments to D31016: Allow stacked filesystems to be recursively unmounted.
Tue, Jul 13, 12:22 AM

Mon, Jul 12

mckusick added a comment to D30442: mprotect.2: Update text for largepages.

For the purposes of this discussion, I think you should just go with your original proposal as it is clearly a useful step forward.

Mon, Jul 12, 5:01 PM

Sat, Jul 10

mckusick accepted D30880: fsck_ufs: fix background fsck in preen mode.

Latest change looks good.

Sat, Jul 10, 5:53 AM
mckusick accepted D31068: geom_label: Partially reinstate old sysinstall(8) workaround.

Code wise it looks good. I am not familiar with the exact layout on disk, but given cy@ has tested it and found it to work, I am happy that those aspects of the code are correct.

Sat, Jul 10, 4:42 AM

Jun 30 2021

mckusick accepted D30825: geom_label: Remove an old sysinstall(8) workaround.

Basing a decision on just the superblock magic number seems a bit weak, but that is what has been used traditionally and it worked, so with that in mind, this change seems reasonable.
Were this still in use, I would recommend more robust checks. For example verifying the check-hash for UFS2 filesystems that have them.

Jun 30 2021, 1:14 AM

Jun 28 2021

mckusick added a comment to D28856: Move struct bufobj out of struct vnode.

The tests that Peter Holm ran (back in March) showed that buildworld ran about 1.5% slower with this change than without it. We were not able to figure out why that slowdown occurred, but unless this can be done in a way that at least has the same running time, I do not think we should do it.

Jun 28 2021, 7:44 PM
mckusick added a comment to D30880: fsck_ufs: fix background fsck in preen mode.

Maybe it'd be nice if the read/write fd's were handled by libufs, but then again maybe not - I couldn't say without studying the code a bit more.

Jun 28 2021, 6:55 PM

Jun 15 2021

mckusick accepted D30041: ufs: avoid unneeded calls to softdep_prerename() and softdep_prelink().

Looks good to go.

Jun 15 2021, 6:54 PM
mckusick added a comment to D30041: ufs: avoid unneeded calls to softdep_prerename() and softdep_prelink().

Good catch on Mark's part.

Jun 15 2021, 12:29 AM

Jun 11 2021

mckusick added a comment to D30041: ufs: avoid unneeded calls to softdep_prerename() and softdep_prelink().

People experiencing this problem report that running the sync(8) command cleared up the slowdown. The reason that it helps is because the problem is triggered when the journal is low on space to make new entries. The journal is written as a circular queue, so running low on space means that the head of the list is nearing the tail of the list. The only way to make more space in the journal is to flush the oldest entries in it so that the tail can advance. What we do in softdep_prelink() is to flush out the entry that we have at hand. But it is almost never causes the oldest entry in the journal to be flushed. So the problem persists and we are just going to keep hitting it until some event causes the oldest entry to be flushed. The reason that sync(8) is so helpful is that it causes the older entries in the journal to be cleared which makes the problem go away.

Jun 11 2021, 2:26 PM

Jun 10 2021

mckusick accepted D30041: ufs: avoid unneeded calls to softdep_prerename() and softdep_prelink().

This looks good and based on testing appears to mitigate effectively. I also recommend changing SUJ_MAX in sys/ufs/ffs/fs.h to 512Mb or at least 256Mb.

Jun 10 2021, 4:46 AM

Jun 7 2021

mckusick added a comment to D30041: ufs: avoid unneeded calls to softdep_prerename() and softdep_prelink().
In D30041#682108, @bz wrote:

FYI I am running the current version now as well. I'll see if my occasional 100% Syncer for a few minutes go away (or get better).
Do you want to track this here or should I follow-up on the PR?

Jun 7 2021, 7:06 PM

Jun 2 2021

mckusick committed R10:02966cbdf03a: Fix fsck_ufs segfault when it needs to rerun. (authored by mckusick).
Fix fsck_ufs segfault when it needs to rerun.
Jun 2 2021, 7:39 PM

May 31 2021

mckusick accepted D30586: ffs: Correct the input size check in sysctl_ffs_fsck().

Good catch, definitely needed.

May 31 2021, 10:53 PM
mckusick committed R10:f9738ecc4115: Fix handling of embedded symbolic links (authored by mckusick).
Fix handling of embedded symbolic links
May 31 2021, 12:53 AM
mckusick committed R10:57d877348b2c: Fix handling of embedded symbolic links (and history lesson). (authored by mckusick).
Fix handling of embedded symbolic links (and history lesson).
May 31 2021, 12:45 AM

May 30 2021

mckusick accepted D30555: vinvalbuf: do not panic if we were unable to flush dirty buffers.

This looks good to me.

May 30 2021, 9:57 PM

May 29 2021

mckusick added a comment to D30442: mprotect.2: Update text for largepages.

If mprotect(2) can force the breakup of pages suitable for superpage mappings, then it opens a vector for a denial of performance by mapping and breaking up all the superpages held by the operating system.

May 29 2021, 9:52 PM
mckusick accepted D30537: fsck_ffs(8): do bufinit() just before gjournal_check().

Given that you rolled back the two changes, your suggested change of just adding a bufinit() in the gjournal case is the easiest and minimal change to solving this bug.

May 29 2021, 3:03 PM
mckusick added a comment to D30537: fsck_ffs(8): do bufinit() just before gjournal_check().

I think a better change is to simply preinitialize fs_bsize in sblock_init to MAXBSIZE which will be valid for all UFS filesystems so that bufinit will do a resonable allocation.

If you're doing that, why not just make bufinit pass MAXBSIZE to malloc instead rather than initialising the in-memory superblock with a dummy value? The latter seems dangerous, it's nice to be able to have malloc junk catch things that read parts of the superblock that haven't been read from disk yet.

May 29 2021, 6:18 AM
mckusick requested changes to D30537: fsck_ffs(8): do bufinit() just before gjournal_check().

I think a better change is to simply preinitialize fs_bsize in sblock_init to MAXBSIZE which will be valid for all UFS filesystems so that bufinit will do a resonable allocation.

May 29 2021, 3:06 AM
mckusick added a comment to D30537: fsck_ffs(8): do bufinit() just before gjournal_check().

I travelled to the north coast where we have been offline for two days, but now back online. Will look into this directly.

May 29 2021, 2:47 AM
mckusick committed R10:5c9e9eb7a27f: Fix fsck_ufs segfault when it needs to rerun. (authored by mckusick).
Fix fsck_ufs segfault when it needs to rerun.
May 29 2021, 2:38 AM

May 22 2021

mckusick added a comment to D29354: systat: Add SPDX-License-Identifier.

Ed, thanks for trying to do the close. Seems like a bug in Phabricator, but not sure to whom to report it. Meanwhile, abandonment seems like the best option.

May 22 2021, 11:46 PM
mckusick committed R10:e198c1dc8f6f: Fix fsck_ffs Pass 1b error exit "bad inode number 256 to nextinode". (authored by mckusick).
Fix fsck_ffs Pass 1b error exit "bad inode number 256 to nextinode".
May 22 2021, 9:01 PM

May 21 2021

mckusick committed R10:f190f9193bc1: Fix fsck_ufs segfaults with gjournal (SU+J) (authored by mckusick).
Fix fsck_ufs segfaults with gjournal (SU+J)
May 21 2021, 8:39 PM
mckusick accepted D30370: fsck_ffs(8): fix divide by zero when debug messages are enabled.

Yes, that is the correct fix.

May 21 2021, 6:05 AM
mckusick added a comment to D30041: ufs: avoid unneeded calls to softdep_prerename() and softdep_prelink().
In D30041#682108, @bz wrote:

FYI I am running the current version now as well. I'll see if my occasional 100% Syncer for a few minutes go away (or get better).
Do you want to track this here or should I follow-up on the PR?

May 21 2021, 5:48 AM

May 20 2021

mckusick committed R10:8ab4e1b95122: Correct assert added to dump program. (authored by mckusick).
Correct assert added to dump program.
May 20 2021, 10:58 PM
mckusick committed R10:c910e015ba8b: Correct assert added to dump program. (authored by mckusick).
Correct assert added to dump program.
May 20 2021, 10:55 PM

May 19 2021

mckusick added a comment to D30041: ufs: avoid unneeded calls to softdep_prerename() and softdep_prelink().

Expanding the journal is just to reduce the incidence of running into this problem. It is still necessary to fix the problem and this looks to be a viable approach.

May 19 2021, 10:15 PM
mckusick committed R10:fe815b88b553: Fix fsck_ffs Pass 1b error exit "bad inode number 256 to nextinode". (authored by mckusick).
Fix fsck_ffs Pass 1b error exit "bad inode number 256 to nextinode".
May 19 2021, 9:36 PM
mckusick accepted D30325: Move mnt_maxsymlinklen into appropriate fs mount data structures.

Looks good to go.

May 19 2021, 9:12 PM

May 18 2021

mckusick committed R10:80e3f1df8ca8: Ensure that files with no allocated blocks are trimmed to zero length. (authored by mckusick).
Ensure that files with no allocated blocks are trimmed to zero length.
May 18 2021, 8:26 PM
mckusick committed R10:e2a49768a4ad: Ensure that files with no allocated blocks are trimmed to zero length. (authored by mckusick).
Ensure that files with no allocated blocks are trimmed to zero length.
May 18 2021, 8:23 PM
mckusick requested changes to D30325: Move mnt_maxsymlinklen into appropriate fs mount data structures.

Other than one requested change, this looks good.

May 18 2021, 6:53 PM

May 17 2021

mckusick committed R10:efe145a7453e: Correct assert added to dump program. (authored by mckusick).
Correct assert added to dump program.
May 17 2021, 11:31 PM
mckusick added a comment to D30041: ufs: avoid unneeded calls to softdep_prerename() and softdep_prelink().

I have been working with folks on these two bug reports which I suspect are caused by this problem:

May 17 2021, 6:43 PM
mckusick added a comment to D29354: systat: Add SPDX-License-Identifier.

This change has now been MFC'ed to 12-stable and 13-stable, so can be closed.

May 17 2021, 5:20 PM
mckusick committed R10:a7e6401ac7cc: Clean up copyright messages. (authored by mckusick).
Clean up copyright messages.
May 17 2021, 6:07 AM
mckusick committed R10:daa020139f1f: Clean up copyright messages. (authored by mckusick).
Clean up copyright messages.
May 17 2021, 6:06 AM
mckusick committed R10:cad6b1e358b4: Clean up fsck_ffs error message output. (authored by mckusick).
Clean up fsck_ffs error message output.
May 17 2021, 5:01 AM
mckusick committed R10:2e762b02521c: Clean up fsck_ffs error message output. (authored by mckusick).
Clean up fsck_ffs error message output.
May 17 2021, 12:39 AM
mckusick committed R10:8f28af0a445f: Make fsck_ffs more persistent in creating a lost+found directory. (authored by mckusick).
Make fsck_ffs more persistent in creating a lost+found directory.
May 17 2021, 12:33 AM
mckusick committed R10:501b4fb50e80: Make fsck_ffs more persistent in creating a lost+found directory. (authored by mckusick).
Make fsck_ffs more persistent in creating a lost+found directory.
May 17 2021, 12:17 AM
mckusick committed R10:9a2fac6ba65f: Fix handling of embedded symbolic links (and history lesson). (authored by mckusick).
Fix handling of embedded symbolic links (and history lesson).
May 17 2021, 12:01 AM

May 11 2021

mckusick committed R10:a3628327e7b6: Ensure that files with no allocated blocks are trimmed to zero length. (authored by mckusick).
Ensure that files with no allocated blocks are trimmed to zero length.
May 11 2021, 9:49 PM
mckusick accepted D29354: systat: Add SPDX-License-Identifier.

This now follows FreeBSD style.

May 11 2021, 6:15 AM
mckusick reopened D29354: systat: Add SPDX-License-Identifier.

forgot to approve before committing.

May 11 2021, 6:15 AM
mckusick committed R10:9e16b9530aab: Clean up copyright messages. (authored by mckusick).
Clean up copyright messages.
May 11 2021, 6:13 AM
mckusick closed D29354: systat: Add SPDX-License-Identifier.
May 11 2021, 6:12 AM

May 9 2021

mckusick added a comment to D30168: Add KPI to dynamically register pager type.

I think that this change provides a useful cleanup of the VM system. As noted it makes it easier to create new types of pagers.

May 9 2021, 11:11 PM

May 7 2021

mckusick added a comment to D30041: ufs: avoid unneeded calls to softdep_prerename() and softdep_prelink().

This change does accurately determine when a vnode does not need to be sync'ed. Does it help solve the stalling problem?

May 7 2021, 11:38 PM

May 5 2021

mckusick added a comment to D29354: systat: Add SPDX-License-Identifier.

Legally "All Rights Reserved." is no longer required, so we no longer add it to copyright messages.

May 5 2021, 5:28 AM

Apr 27 2021

mckusick committed R10:689724cb23c2: Clean up fsck_ffs error message output. (authored by mckusick).
Clean up fsck_ffs error message output.
Apr 27 2021, 1:40 AM

Apr 26 2021

mckusick committed R10:84a0e3f95700: Make fsck_ffs more persistent in creating a lost+found directory. (authored by mckusick).
Make fsck_ffs more persistent in creating a lost+found directory.
Apr 26 2021, 11:45 PM

Apr 19 2021

mckusick accepted D29303: systat: Avoid incorrect reallocation in pigs.c.

This change is a useful improvement.

Apr 19 2021, 6:56 PM

Apr 18 2021

mckusick committed R10:b2b7ad916cb7: Ensure that the mount command shows "with quotas" when quotas are enabled. (authored by mckusick).
Ensure that the mount command shows "with quotas" when quotas are enabled.
Apr 18 2021, 5:09 PM
mckusick committed R10:44f01dbeda67: Ensure that the mount command shows "with quotas" when quotas are enabled. (authored by mckusick).
Ensure that the mount command shows "with quotas" when quotas are enabled.
Apr 18 2021, 5:06 PM

Apr 14 2021

mckusick committed R10:14d0cd7225e2: Ensure that the mount command shows "with quotas" when quotas are enabled. (authored by mckusick).
Ensure that the mount command shows "with quotas" when quotas are enabled.
Apr 14 2021, 10:21 PM

Apr 13 2021

mckusick added a comment to D29695: systat: Implemented per-process swap display on -swap.

Not having seen this review, I started a review at https://reviews.freebsd.org/D29754. I am fine with abandoning my review though I do think you should consider incorporating my version of the manual page.

Apr 13 2021, 11:12 PM
mckusick requested review of D29754: Augment systat(1) -swap to display large swap space processes.
Apr 13 2021, 9:41 PM

Apr 9 2021

mckusick committed R10:27aa4fcbbc73: Ensure that all allocated data structures in fsck_ffs are freed. (authored by mckusick).
Ensure that all allocated data structures in fsck_ffs are freed.
Apr 9 2021, 12:49 AM

Apr 2 2021

mckusick accepted D29563: Remove kgmon(8).

Given that -pg support has been withdrawn from the kernel, it is sensible to remove kgmon(8).

Apr 2 2021, 10:44 PM
mckusick committed R10:343b9e6219e1: Fix fsck_ffs -R finds unfixed duplicate block errors when rerunning. (authored by mckusick).
Fix fsck_ffs -R finds unfixed duplicate block errors when rerunning.
Apr 2 2021, 9:53 PM
mckusick committed R10:fab7c18ce322: Fix fsck_ffs Pass 1b error exit "bad inode number 2 to nextinode". (authored by mckusick).
Fix fsck_ffs Pass 1b error exit "bad inode number 2 to nextinode".
Apr 2 2021, 9:50 PM
mckusick committed R10:fc56fd262d0b: Ensure that all allocated data structures in fsck_ffs are freed. (authored by mckusick).
Ensure that all allocated data structures in fsck_ffs are freed.
Apr 2 2021, 6:56 PM

Mar 31 2021

mckusick accepted D29478: ffsinfo: Update example to avoid to-be-deprecated vinum.

Your change looks good.

Mar 31 2021, 5:34 AM

Mar 26 2021

mckusick added a comment to D28856: Move struct bufobj out of struct vnode.
In D28856#659254, @kib wrote:

The cost of an extra allocation versus the overhead of having to handle low-memory situations and building up and tearing down zones seems like a bad tradeoff.

Why? It is reverse, IMO: the normal system operation performs a lot of vnode allocations and deallocations, while lowmem is rare condition, where we do not worry about system performance at all, only about system liveness. Optimizing for normal path is right, optimizing for lowmem handler is not.

The purpose of this change is to reduce the amount of memory dedicated to vnodes.

Mar 26 2021, 6:19 PM
mckusick added a comment to D28856: Move struct bufobj out of struct vnode.
In D28856#659141, @kib wrote:

There is still overhead as the zone memory has to be cleaned up (locks disposed of) and then new memory initialized (zeroed, lists and queues initialized, locks initialized, etc). Also there is extra work done detecting that we have hit these conditions and making them happen. In general we are going to have more memory tied up and do more work moving it between the zones. If we just had one zone for vnodes and another zone for bufobjs we could avoid all of this. In all likelyhood we would only need occational freeing of memory in the bufobj zone.

I am curious why you are so resistant to having a single vnode zone and a single bufobj zone?

With either (vnode + bufobj, vnode - bufobj), or (vnode - bufobj, bufobj) we still have two zones, and on low memory condition two zones needs to be drained. But for the separate bufobj zone, we additionally punish filesystems that use buffers. Instead of single allocation for vnode, they have to perform two, and also they have to perform two frees.

We have a similar structure in namecache, where {short,long}x{timestamp, no timestamp} allocations use specific zones, instead of allocating nc + path segment + timestamp.

Mar 26 2021, 12:09 AM

Mar 25 2021

mckusick added a comment to D28856: Move struct bufobj out of struct vnode.

I understand that there cannot be more than maxvnodes. What I am concerned about is how much memory is tied up in the two zones. In this example as vnlru() is freeing (vnodes without bufobjs) into the (vnodes without bufobjs) zone. It then allocates memory from (vnodes+bufobj) from the (vnodes+bufobj) zone. That allocation cannot use the memory in the (vnodes without bufobj) zone. So when we are done we have enough memory locked down in the two zones to support 2 * maxvnodes. This is much more wasteful of memory than having a single zone for pure vnodes and a second zone that holds bufobjs each of which will be limited to maxvnodes in size.

Mar 25 2021, 9:18 PM
mckusick committed R10:7848b25edd2a: Fix fsck_ffs -R finds unfixed duplicate block errors when rerunning. (authored by mckusick).
Fix fsck_ffs -R finds unfixed duplicate block errors when rerunning.
Mar 25 2021, 12:22 AM

Mar 24 2021

mckusick committed R10:bc444e2ec6e6: Fix fsck_ffs Pass 1b error exit "bad inode number 2 to nextinode". (authored by mckusick).
Fix fsck_ffs Pass 1b error exit "bad inode number 2 to nextinode".
Mar 24 2021, 11:50 PM
mckusick added a comment to D28856: Move struct bufobj out of struct vnode.
In D28856#658811, @kib wrote:

No, we do not have two pools of vnodes after this change. We have two zones, but zones do not keep vnodes, they cache partially initialized memory for vnodes. Either the current single zone, or two zones after applying the patch, do not have any limits to grow that size of the cache. But it is a cache of memory and not vnodes. Without or with the patch, only maxvnodes constructed vnodes can exist in the system. The constructed vnode is struct vnode which is correctly initialized and has identity belonging to some filesystem, or reclaimed. [In fact in some cases getnewvnodes() is allowed to ignore the limit of maxvnodes, but this is not relevant to the discussion].

Let me try again to explain my perceived issue.

Under this scheme we have two zones. If there is a lot of ZFS activity, the vnode-only zone can be filled with maxvnodes worth of entries. Now suppose activity in ZFS drops but activity in NFS rises. Now the zone with vnodes + bufobj can fill to maxvnodes worth of memory. As I understand it we do not reclaim any of the memory from the vnode-only zone, it just sits there unable to be used. Is that correct?

No, this is not correct. Total number of vnodes (summary of both types) is limited by maxvnodes. After the load shifted from zfs to nfs in your scenario, vnlru starts reclaiming vnodes in LRU order from the global free list, freeing (vnode without bufobj)s, and most of the allocated vnodes would be from the (vnode+bufobj) zone. We do not allow more that maxvnodes total vnodes allocated in the system.

Mar 24 2021, 10:31 PM
mckusick added a comment to D28856: Move struct bufobj out of struct vnode.

No, we do not have two pools of vnodes after this change. We have two zones, but zones do not keep vnodes, they cache partially initialized memory for vnodes. Either the current single zone, or two zones after applying the patch, do not have any limits to grow that size of the cache. But it is a cache of memory and not vnodes. Without or with the patch, only maxvnodes constructed vnodes can exist in the system. The constructed vnode is struct vnode which is correctly initialized and has identity belonging to some filesystem, or reclaimed. [In fact in some cases getnewvnodes() is allowed to ignore the limit of maxvnodes, but this is not relevant to the discussion].

Mar 24 2021, 7:10 PM

Mar 22 2021

mckusick added a comment to D28856: Move struct bufobj out of struct vnode.
In D28856#657865, @kib wrote:

Three inline comments / questions.

In Sun's implementation of vnodes each filesystem type had its own pool. When I adopted the vnode idea into BSD, I created generic vnodes that could be used by all filesystems so that they could move between filesystems based on demand.

This design reverts back to vnodes usable by ZFS and a few other filesystems and vnodes for NFS, UFS, and most other filesystems. This will be a win for systems that run just ZFS. But, systems that are also running NFS or UFS will not be able to share vnode memory and will likely have a bigger memory footprint than if they stuck with the single type of vnode.

There has been no attempt to fix vlrureclaim() so we can end up reclaiming a bunch of vnodes of the wrong type thus reducing the usefulness of the cache without recovering any useful memory. In the worst case, we can end up with each of the two vnode pools using maxvnodes worth of memory.

We probably need to have a separate maxvnodes for each pool. Alternately we could keep track of how many vnodes are in each pool and limit the two pools to a total of maxvnodes. That of course begs the question of how we decide to divide the quota between the two pools. At a minimum, vlrureclaim() needs to have a way to decide which pool needs to have vnodes reclaimed.

We do not have two pools of vnodes after the patch. For very long time, we free vnode after its hold count goes to zero (mod SMR complications).

Mar 22 2021, 11:30 PM
mckusick requested changes to D28856: Move struct bufobj out of struct vnode.

Three inline comments / questions.

Mar 22 2021, 5:20 AM

Mar 16 2021

mckusick committed R10:cf0310dfefee: Fix bug 253158 - Panic: snapacct_ufs2: bad block - mksnap_ffs(8) crash (authored by mckusick).
Fix bug 253158 - Panic: snapacct_ufs2: bad block - mksnap_ffs(8) crash
Mar 16 2021, 12:07 AM

Mar 14 2021

mckusick committed R10:7dd29d256ff7: Do not complain about incorrect cylinder group check-hashes when (authored by mckusick).
Do not complain about incorrect cylinder group check-hashes when
Mar 14 2021, 10:16 PM

Mar 12 2021

mckusick committed R10:6385cabd5be6: Do not complain about incorrect cylinder group check-hashes when (authored by mckusick).
Do not complain about incorrect cylinder group check-hashes when
Mar 12 2021, 6:44 AM

Mar 11 2021

mckusick accepted D29178: UFS SU: handle races on remounts rw<->ro.

Flag definitions look good.
Breakdown of commits is excellent.
Changes should resolve the problem.

Mar 11 2021, 7:39 PM
mckusick accepted D29178: UFS SU: handle races on remounts rw<->ro.

It would help if I had looked at your commit logs before my previous comment. You have in fact separated everything out appropriately.

Mar 11 2021, 6:31 AM
mckusick added a comment to D29178: UFS SU: handle races on remounts rw<->ro.

Nearly all of the changes in ffs_softdep.c are code cleanups and not related to this bug fix. I would prefer to see the code cleanups in a separate commit so that it is easier to see the changes that are needed to fix this problem. That said, this update appears to solve the problem that you describe.

Mar 11 2021, 6:27 AM

Mar 10 2021

mckusick added a comment to D29178: UFS SU: handle races on remounts rw<->ro.

In your summary you say ``rw<->ro remounts are not atomic, filesystem is accessible by other threads during the process. As result, its internal state is inconsistent. Just blocking writers with suspend is not enough.'' Can you elaborate on how having other processes reading the filesystem causes trouble?

Mar 10 2021, 4:48 AM

Mar 3 2021

mckusick accepted D29045: Don't use sleeping allocations for ufs dirhash blocks when holding directory vnode.

This seems like a reasonable solution to the problem. The allocation will fail in a few cases where it previously would have succeeded, but hopefully those will be rare. The effect of failing will simply be slower lookups rather than unexpected errors to applications.

Mar 3 2021, 9:19 PM
mckusick accepted D29021: growfs: allow operation on RW-mounted filesystems.

I agree that this change is appropriate.

Mar 3 2021, 9:02 PM

Mar 2 2021

mckusick added a comment to D28999: FFS extattr: fix handling of the tail.

Sorry for the delayed review. This change looks correct to me.

Mar 2 2021, 5:55 PM
mckusick added a comment to D26999: Remove an extra if_ref()..

Sorry for the delayed review. This fix looks correct to me.

Mar 2 2021, 5:54 PM

Feb 25 2021

mckusick added a comment to D28901: buf: Fix the dirtybufthresh check.

It appears that this change should be MFC'ed to 12.

Feb 25 2021, 12:24 AM

Feb 24 2021

mckusick accepted D28901: buf: Fix the dirtybufthresh check.
Feb 24 2021, 6:11 AM

Feb 20 2021

mckusick accepted D28679: vnode: move write cluster support data to inodes..

Its a wrap!

Feb 20 2021, 1:03 AM

Feb 18 2021

mckusick added a comment to D28679: vnode: move write cluster support data to inodes..

I note that lib/libprocstat/libprocstat.c includes ufs/ufs/inode.h but in fact does not need to do so.

Feb 18 2021, 10:53 PM
mckusick accepted D28679: vnode: move write cluster support data to inodes..

Getting rid of _buf_cluster.h was a lot more work than I expected, but is definitely cleaner and has the added bonus of cleaning up some other cruft as well.

Feb 18 2021, 10:49 PM