On Tue, Jun 16, 2020 at 03:57:08PM +0200, Andreas Gruenbacher wrote:
> On Tue, Jun 16, 2020 at 3:23 PM Matthew Wilcox <willy@xxxxxxxxxxxxx> wrote:
> > On Tue, Jun 16, 2020 at 08:17:28AM -0400, Bob Peterson wrote:
> > > ----- Original Message -----
> > > > > I'd assume Andreas is looking at converting a filesystem to use iomap,
> > > > > since this problem only occurs for filesystems which have returned an
> > > > > invalid extent.
> > > >
> > > > Well, I can assume it's gfs2, but you know what happens when you
> > > > assume something....
> > >
> > > Yes, it's gfs2, which already has iomap. I found the bug while just browsing
> > > the code: gfs2 takes a lock in the begin code. If there's an error,
> > > however unlikely, the end code is never called, so we would never unlock.
> > > It doesn't matter to me whether the error is -EIO because it's very unlikely
> > > in the first place. I haven't looked back to see where the problem was
> > > introduced, but I suspect it should be ported back to stable releases.
> >
> > It shouldn't just be "unlikely", it should be impossible. This is the
> > iomap code checking whether you've returned an extent which doesn't cover
> > the range asked for. I don't think it needs to be backported, and I'm
> > pretty neutral on whether it needs to be applied.
>
> Right, when these warnings trigger, the filesystem has already screwed
> up; this fix only makes things less bad. Those kinds of issues are
> very likely to be fixed long before the code hits users, so it
> shouldn't be backported.
>
> This bug was in iomap_apply right from the start, so:
>
> Fixes: ae259a9c8593 ("fs: introduce iomap infrastructure")
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
> Thanks,
> Andreas
>
