Page MenuHomeFreeBSD

rc.initdiskless: Disable soft-updates in mdmfs (again)
ClosedPublic

Authored by jlduran_gmail.com on Dec 30 2023, 8:14 AM.
Tags
None
Referenced Files
Unknown Object (File)
Wed, May 8, 12:37 AM
Unknown Object (File)
Tue, May 7, 2:15 PM
Unknown Object (File)
Sun, Apr 28, 3:16 AM
Unknown Object (File)
Fri, Apr 26, 2:48 AM
Unknown Object (File)
Apr 20 2024, 9:57 PM
Unknown Object (File)
Apr 20 2024, 9:57 PM
Unknown Object (File)
Apr 19 2024, 11:30 AM
Unknown Object (File)
Apr 19 2024, 10:18 AM

Details

Summary

Re-apply the -S switch to disable soft-updates in memory disks (commit 8b1292ac5219). This might be beneficial when tmpfs(5) is not present in the kernel, as this can cause mdmfs(8)'s auto keyword to fallback to using md(4).

PR: 85558

Test Plan

Don't know who to ask for review, the change back to mdmfs was done in 50e3590c4486925a3694a113f59db7db69eb7679, and the original addition of the -S flag was done in 8b1292ac52192757bf4c5141340cdb8e2d78d821.

I don't think (or know if) it makes sense to also add back -i 4096?

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

Please don't do both pull requests and phab reviews for the same thing.

In D43242#987249, @imp wrote:

Please don't do both pull requests and phab reviews for the same thing.

Sorry about that, the pull request is an "internal" pull request on my GitHub fork (it's just that you were accidentally at-mentioned there).

In D43242#987249, @imp wrote:

Please don't do both pull requests and phab reviews for the same thing.

Sorry about that, the pull request is an "internal" pull request on my GitHub fork (it's just that you were accidentally at-mentioned there).

Yea, I noticed that and had thought it was against the main repo...

tl;dr: ship it Softupdates are useless here: either we use the much better tmpfs (which ignores softupdates) or we're using swap backed md, which lives 100% in the buffer cache so softupdates don't save any I/O.

libexec/rc/rc.initdiskless
213

I think this change is fine. Here's my reasoning.

(1) auto uses tmpfs, which doesn't care: there's no softupdates on tmpfs, so this is a nop for the vast majority of users. All generic kernels have it.
(2) Even if we don't have tmpfs in the kernel we kldload it (though NanoBSD is one place we severely trim the list).
(3) softupdates with md-backed UFS filesystem just wastes memory in this use case. I don't think it saves anything. We're using swap backed md in this script (since we don't set -M or -F). Since they are alloced from buffer memory for the 'swap' operation, we're just adding complexity to do the softdep chains when all the flushing will be instantaneous. No sense doing the softdep things for malloc based even, since that's a memcpy and there's little window to avoid extra writes (though it might make the initial extraction from the tarball/backing disk directory a smidge faster, but since we're not using malloc..)

This revision is now accepted and ready to land.Jan 4 2024, 3:43 PM

Also, dropping the -i 4096 also is likely not worth the bother. Nobody is really using the 'md' backing store anymore for this script.

I kinda think that we should just document the tmpfs dependency and move away from 'auto' and just use tmpfs always. I'm finding it hard to understand when we might want to do md at all for this scenario.

Seems like this should land? We're just missing a commit log message based on the commentary in this review.

jlduran_gmail.com edited the test plan for this revision. (Show Details)

Seems like this should land? We're just missing a commit log message based on the commentary in this review.

Let me know if a more verbose message, based upon Warner's comments, should be added. As it is often the case, very small changes require long explanations.