Re: [PATCH 00/20] Error handling fixes

David Sterba
2024-01-29T16:13:36Z

On Fri, Jan 26, 2024 at 08:28:12PM +0800, Anand Jain wrote:
> On 1/25/24 05:17, David Sterba wrote:
> > Get rid of some easy BUG_ONs, there are a few groups fixing the same
> > pattern. At the end there are three patches that move transaction abort
> > to the right place. I have tested it on my setups only, not the CI, but
> > given the type of fix we'd never hit any of them without special
> > instrumentation.
> >
>
> What is our guideline for the error message location in the function
> stack, leaf function or at the root function? IMO, it should be
> in the leaf functions(), so that in the event of a serious error,
> we have substantial logs to pinpoint the issue. Here, despite -EUCLEAN
> we have no errors logged, in some of the patches. Why not also add
> error message where it wont' endup with abort(). Otherwise, debugging
> becomes too difficult when looking at the errors due to corruption.

I'll try to sum up suggestions from the whole thread, the proposed end
result of the error handling should follow this:

- critical errors should be followed by a message at the place where
they happen, typically next to an EUCLEAN
- an exception could be something that is called frequently and the
messages would flood the logs

- transaction abort should be at the place where an error is detected,
assuming that it's clear from the context that it's unrecoverable and
no rollback is possible (ie. free structures, undo changes and only
return an error)
- this also needs inspection for special cases and exceptions

- tree-checker verifies data at the time of reading from disk, we can
make some assumptions about structure consistency

- error handling should cover all possible returned values, as an
example search slot can return < 0, 0 or 1, although some of them
might not be possible on a consistent filesystem, all should be
handled and return EUCLEAN + message for the "impossible" ones

There are already examples that can be followed and copied elsewhere,
however we still lack some helpers for the EUCLEAN and messages
pattern.