Page MenuHomeFreeBSD

Add option to cluster-align the start of the root directory
ClosedPublic

Authored by delphij on Jun 6 2018, 7:46 AM.
Tags
None
Referenced Files
Unknown Object (File)
Sep 23 2024, 1:08 AM
Unknown Object (File)
Sep 22 2024, 11:37 PM
Unknown Object (File)
Sep 22 2024, 2:13 AM
Unknown Object (File)
Sep 20 2024, 12:41 PM
Unknown Object (File)
Sep 17 2024, 8:11 PM
Unknown Object (File)
Sep 12 2024, 7:51 AM
Unknown Object (File)
Sep 12 2024, 7:51 AM
Unknown Object (File)
Sep 12 2024, 7:51 AM
Subscribers

Details

Summary

Added option to cluster-align the start of the root directory.

This follows the recommendations of the SD-card association.

Obtained from: Android https://android.googlesource.com/platform/system/core/+/052f27562154d175267999106bd6bf18fc8c363e
and https://android.googlesource.com/platform/system/core/+/8218b6aae9cd4a19fa074a8a8203fe9275b35447

Test Plan

newfs_msdos with and without -A and fsck_msdosfs the result

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

sbin/newfs_msdos/mkfs_msdos.c
500 ↗(On Diff #43353)

align it to what? If the SD card association is recommending this, having either their recommendation here (if it is short) or a pointer to (if not) would be good.

512 ↗(On Diff #43353)

wha's so special about 16k?

sbin/newfs_msdos/mkfs_msdos.c
500 ↗(On Diff #43353)

I'll amend the align part to "align to cluster". I'll try to dig out what the recommendation was.

512 ↗(On Diff #43353)

Honestly, I don't know and I haven't researched that in detail as it's not really related to this change. The code tried to say there should be max(x, sectors in a 16K cluster, 4), but I don't see the reasoning, if I was the author, I would probably write MAX(x, 32) where 32 is the "usual" reserved sectors number for FAT32, according to https://en.wikipedia.org/wiki/Design_of_the_FAT_file_system .

Note that the number was from the original code (introduced in rS37446; and line 512 in new code is actually from old code in line 491).

What this change does is merely added the following logic to the existing FAT sizing calculation: after the calculation, check if the root directory would be aligned to a cluster, and pad it if alignment is not satisfied, and repeat the calculation for at most two times. The only real change here is to add "extra_res" to reserved sectors number.

I think modifying this number is beyond the scope of this change, if we really want to change it, we should do it in a separate changeset.

jilles added inline comments.
sbin/newfs_msdos/newfs_msdos.c
95–97 ↗(On Diff #43353)

Hmm, do we need this to be an option or can we determine it automatically?

This is different from the Android code since newfs_msdos is a user interface for us. The change can be a separate commit.

184–185 ↗(On Diff #43353)

In my quick look, there does not seem a reason to prevent -A with -N.

sbin/newfs_msdos/newfs_msdos.8
95–96 ↗(On Diff #43353)

Given that the option is required, this description is not very helpful because it does not say why and when it is needed.

delphij added inline comments.
sbin/newfs_msdos/newfs_msdos.c
95–97 ↗(On Diff #43353)

Let's try to do it in a different change. I'll silent the warning for now.

Address reviewer comments.

This revision was not accepted when it landed; it landed in state Needs Review.Jun 15 2018, 6:04 AM
This revision was automatically updated to reflect the committed changes.