xfs
[Top] [All Lists]

[RFC PATCH 0/9] xfs: clean up xfs_buf io interfaces

To: xfs@xxxxxxxxxxx
Subject: [RFC PATCH 0/9] xfs: clean up xfs_buf io interfaces
From: Dave Chinner <david@xxxxxxxxxxxxx>
Date: Fri, 15 Aug 2014 16:38:58 +1000
Delivered-to: xfs@xxxxxxxxxxx
Hi folks,

This patch set has come from the conversion Brian and I have been
having over how to fix a use-after-free issue related to IO
completions racing with synchronous IO submission. It seemed to me
we were having trouble grasping all the different things that were
wrong and putting them in a line, so I figured the easiest way to do
with was to write a patchset that fixed the original bug, everything
else we'd noticed, and changed the IO API such that we wouldn't have
problems in the future.

So, this patch contains:

        a) initial fix for the use after free issue (patch 1)
        b) clear separation of IO submission errors at the XFS level
           from IO errors reported through BIO completion callbacks
           (patches 2,3)
        c) removal of some historic cruft around IO submission
           (patch 4)
        d) rework of unnecessarily complex historic error handling
           paths (patches 5,6,7)
        e) a new API for clear and concise asynchronous and
           synchronous IO.  (patch 8)

The last patch fixes a bug I found writing patch 8, and can probably
be moved to first in the series...

I was just going to send this compile tested for comments, but I
threw it at a couple of VMs and it's half way through xfstests now
without any failures. That means I haven't broken anything badly,
though the error hendling changes will be interesting to test.

The error handling changes have something in common - a buffer that
took an IO error was mostly marked stale, but not always. Sometimes
they were marked as "not done" indicating that IO needed to be done
on them before the contents were valid. Some got marked stale and
done, and so on. The thing is, when we mark a buffer stale, it is
essentially the same thing as marking it !XBF_DONE. So all the error
handling paths now stale the buffer and clear XBF_DONE. Nice,
consistent error handling. This enabled some significant cleanups -
xfs_trans_buf_read_map() finally got the love it's been needing for
long time.

It also defines a specific, safe way for waiting on XBF_ASYNC buffer
IO, and makes use of that in the buffer delayed write code. Hence
the patchset also solves the problem of how do async dispatch, some
work, then wait for IO completion as separate steps as opposed to
issuing synchronous IO that is waited on immediately.

Also, the rework of the IO submission interfaces removes the fixes
added in the first patch to solve the use-after-free issue - we no
longer need an extra reference to protect parts of the IO completion
path because the reference we take during submission is held until
after the IO completion wait has occurred.

Anyway, I figured patches would speak louder than any description,
and most especially a diffstat that looks like this:

        9 files changed, 286 insertions(+), 360 deletions(-)

Comments welcome.

-Dave.

<Prev in Thread] Current Thread [Next in Thread>