Page MenuHomeFreeBSD

Make the code consistent with the comments
ClosedPublic

Authored by kjopek_gmail.com on May 21 2021, 3:52 PM.
Tags
None
Referenced Files
Unknown Object (File)
Sun, Apr 21, 9:02 PM
Unknown Object (File)
Thu, Apr 18, 4:47 AM
Unknown Object (File)
Thu, Apr 18, 4:34 AM
Unknown Object (File)
Wed, Apr 17, 7:03 PM
Unknown Object (File)
Wed, Apr 17, 6:59 PM
Unknown Object (File)
Wed, Apr 17, 6:58 PM
Unknown Object (File)
Wed, Apr 17, 4:52 PM
Unknown Object (File)
Wed, Apr 17, 6:32 AM

Details

Summary

We need to reserve two segments. Three sectors is just 3/4 of the segment.

This fixes panic with fsck on dirty filesystem.
Actually I also tested version with single segment reserved and it has
shown to be stable. However, I do believe there may be the cases
when the second segment is also required.

Test Plan

Tested on RockPro64.

Diff Detail

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

Event Timeline

kjopek_gmail.com created this revision.

Sorry for long delay. Can you, please, also test "(IDMAC_MAX_SIZE * (IDMAC_DESC_SEGS - 1)) / MMC_SECTOR_SIZE"? Minus one should be sufficient and it passed all my tests.

I performed again test with

(IDMAC_MAX_SIZE * (IDMAC_DESC_SEGS - 1)) / MMC_SECTOR_SIZE

and it worked perfectly:

Starting file system checks:
** SU+J Recovering /dev/mmcsd0s5
** Reading 4194304 byte journal from inode 4.
** Building recovery table.
** Resolving unreferenced inode list.
** Processing journal entries.
** 2 journal records in 512 bytes for 12.50% utilization
** Freed 0 inodes (0 dirs) 0 blocks, and 0 frags.

***** FILE SYSTEM MARKED CLEAN *****
/dev/mmcsd0s6: 8058 files, 18530 used, 3374021 free (37 frags, 421748 blocks, 0.0% fragmentation)

@mmel I think we need to do something with this review. We have at least one bug report: https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=256822 (similar to what I observe).

avg added a subscriber: avg.

Please commit this.
If not too much trouble please also update dwmmc_get_tran_settings (used with MMCCAM) to use the same formula.
Thank you very much!

This revision is now accepted and ready to land.Sep 22 2021, 7:30 AM

I'm not against this being commited but I'd like to understand why it's needed.
IIRC @mmel said this has to do with bounce buffer (and the comment seems to agree, is there somewhere I can read about this ?

@manu: The handling of bouncing buffers itself is correct in this case. I did not observe any issues in sys/arm64/arm64/busdma_bounce.c. However this is needed, because currently it returns invalid value: instead of making reservation for 2 segments it makes reservation for 3/4 segment which in turns leads to crashes in bouncing buffers code. Comment above the line is valid - for sure we need to reserve two segments (it works for one reserved segment too, but I am not sure about all edge cases), but we need to correctly reserve 2 segments, not 3 sectors.

@avg: Surely, I can provide small patch for it this weekend.

kjopek_gmail.com set the repository for this revision to rG FreeBSD src repository.

Update patch according to latest comments.

This revision now requires review to proceed.Sep 26 2021, 11:17 AM
This revision was not accepted when it landed; it landed in state Needs Review.Oct 8 2021, 7:22 AM
This revision was automatically updated to reflect the committed changes.