Re: [PATCH] iomap: Make sure iomap_end is called after iomap_begin

From: Dave Chinner <david@fromorbit.com>
To: Bob Peterson <rpeterso@redhat.com>
Cc: "Darrick J. Wong" <darrick.wong@oracle.com>,
	Andreas Gruenbacher <agruenba@redhat.com>,
	Matthew Wilcox <willy@infradead.org>,
	Christoph Hellwig <hch@infradead.org>,
	linux-xfs@vger.kernel.org,
	linux-fsdevel <linux-fsdevel@vger.kernel.org>
Subject: Re: [PATCH] iomap: Make sure iomap_end is called after iomap_begin
Date: Thu, 18 Jun 2020 09:44:41 +1000	[thread overview]
Message-ID: <20200617234441.GE2005@dread.disaster.area> (raw)
In-Reply-To: <700590041.34131118.1592325518407.JavaMail.zimbra@redhat.com>

On Tue, Jun 16, 2020 at 12:38:38PM -0400, Bob Peterson wrote:
> ----- Original Message -----
> > So... you found this through code inspection, and not because you
> > actually hit this on gfs2, or any of the other iomap users?
> > 
> > I generally think this looks ok, but I want to know if I should be
> > looking deeper. :)
> > 
> > --D
> 
> Correct. Code-inspection only. I never actually hit the problem.
> If those errors are really so unusual and catastrophic, perhaps
> we should just change them to BUG_ONs or something instead.

We do not panic a machine because a detectable data or filesystem
corruption event has occurred. We have a viable error path to tell
userspace a fatal IO error occurred so that is all the generic
infrastructure should be doing.

If a loud warning needs to be issued, then WARN_ON_ONCE() may be
appropriate, though I suspect even that is overkill for this
situation....

> Why bother unlocking if we're already 1.9 meters underground?

Because then a filesystem that has shutdown because it has
recognised that it is walking dead can be unmounted and the user can
then run an autopsy to find and fix the problem without having to
reboot the machine....

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com