Re: [PATCH] dio: track and serialise unaligned direct IO

[Dave Chinner <david@fromorbit.com>]

On Tue, Aug 03, 2010 at 05:11:18PM -0700, Mingming Cao wrote:
> On Fri, 2010-07-30 at 08:45 +1000, Dave Chinner wrote:
> > From: Dave Chinner <dchinner@redhat.com>
> > 
> > If we get two unaligned direct IO's to the same filesystem block
> > that is marked as a new allocation (i.e. buffer_new), then both IOs will
> > zero the portion of the block they are not writing data to. As a
> > result, when the IOs complete there will be a portion of the block
> > that contains zeros from the last IO to complete rather than the
> > data that should be there.
> > 
> > This is easily manifested by qemu using aio+dio with an unaligned
> > guest filesystem - every IO is unaligned and fileystem corruption is
> > encountered in the guest filesystem. xfstest 240 (from Eric Sandeen)
> > is also a simple reproducer.
> > 
> > To avoid this problem, track unaligned IO that triggers sub-block zeroing and
> > check new incoming unaligned IO that require sub-block zeroing against that
> > list. If we get an overlap where the start and end of unaligned IOs hit the
> > same filesystem block, then we need to block the incoming IOs until the IO that
> > is zeroing the block completes. The blocked IO can then continue without
> > needing to do any zeroing and hence won't overwrite valid data with zeros.
> > 
> 
> This seems to address both two IOs are unaligned direct IO. If the first
> IO is aligned direct IO, then it is not tracked?
> 
> I am also concerned about the aligned direct IO case...
> 
> 1) first thread aio+dio+aligned write to a hole, there is no zero-out
> submitted from kernel. But the hole remains initialized before all IO
> complete and convert it from uninitialized extent to initialized.
> 2) second thread aio+dio+unalign write to the same hole, this time it is
> unaligned. since buffer is still new (not converted yet), the new
> incoming thread zero out port of data that first thread has written to

That is clearly and unmistakably an application bug - it should not
be issuing concurrent, overlapping IO to the same block(s)
regardless of whether they are unaligned, aligned or a mixture of
both. By using direct IO, the application has assumed responsibility
for preventing data corruption due to overlapping IOs - they are
inherently racy and nothing in the dio code prevents that from
occurring.

The bug I'm fixing is for *non-overlapping* concurrent unaligned IOs
where the kernel direct IO code causes the data corruption, not the
application. The application is not doing something stupid, and as
such needs to be fixed.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com