Differential D25478

[fifolog] Wrap to the beginning of the file once the end of the file is reached
ClosedPublic
Actions

Authored by adrian on Jun 26 2020, 9:23 PM.

Details

Reviewers

phk

Commits

rS362718: [fifolog] wrap the recno when we hit the end of the provided file size.

Summary

The fifolog code wasn't wrapping the record number, so once the
file filled up it would just continue filling.

This resets the record to the beginning of the file once it
hits the end.

Test Plan

run on a 256k fifolog, fill with content! Make sure it doesn't overflow!

Diff Detail

Repository

rS FreeBSD src repository - subversion

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

adrian created this revision.Jun 26 2020, 9:23 PM

Herald added a subscriber: imp. · View Herald TranscriptJun 26 2020, 9:23 PM

adrian requested review of this revision.Jun 26 2020, 9:23 PM

Harbormaster completed remote builds in B31994: Diff 73718.Jun 26 2020, 9:23 PM

adrian added a reviewer: phk.Jun 26 2020, 9:23 PM

allanjude added a subscriber: allanjude.Jun 26 2020, 9:26 PM

allanjude added inline comments.

usr.sbin/fifolog/lib/fifolog_write_poll.c
248 ↗	(On Diff #73718)	Is 'recno 1' actually f->recno = 0? or is this a typo and you meant f->recno=2?

adrian added inline comments.Jun 26 2020, 11:25 PM

usr.sbin/fifolog/lib/fifolog_write_poll.c
248 ↗	(On Diff #73718)	Sorry yeah, comment is wrong. recno 0 is header.

Oops - update comment. Thanks jude!

Harbormaster completed remote builds in B31997: Diff 73726.Jun 26 2020, 11:26 PM

I noticed we were off by one; the file size ended up being one record
longer than specified. This fixes it.

Harbormaster completed remote builds in B31998: Diff 73728.Jun 27 2020, 12:11 AM

Yeah, that's clearly an oversight.

In the original application this code came from, the write error propagated up and was used to register how many times the flash had been written.

That code did not become part of the fifolog_writer(1) program, which silently ignores the errors from fifolog_write_record_poll().

Wrapping in the lib code probably makes most sense in this stand alone usage of the code, but fifolog_writer(1) should also report write errors back.

In D25478#562806, @phk wrote:

Yeah, that's clearly an oversight.

In the original application this code came from, the write error propagated up and was used to register how many times the flash had been written.

That code did not become part of the fifolog_writer(1) program, which silently ignores the errors from fifolog_write_record_poll().

Wrapping in the lib code probably makes most sense in this stand alone usage of the code, but fifolog_writer(1) should also report write errors back.

oh! that makes sense!

I'll tackle that as a follow-up if you don't mind. I may use this on some NAND device and having callbacks for write errors and record number wrapping would help.

This revision was not accepted when it landed; it landed in state Needs Review.Jun 28 2020, 6:53 AM

Closed by commit rS362718: [fifolog] wrap the recno when we hit the end of the provided file size. (authored by adrian). · Explain Why

This revision was automatically updated to reflect the committed changes.

adrian added a commit: rS362718: [fifolog] wrap the recno when we hit the end of the provided file size..