Re: [PATCH 3/6] xfs: Don't use unwritten extents for DAX

Brian Foster
2015-10-30T12:37:00Z

On Fri, Oct 30, 2015 at 10:37:56AM +1100, Dave Chinner wrote:
> On Thu, Oct 29, 2015 at 10:29:50AM -0400, Brian Foster wrote:
> > On Mon, Oct 19, 2015 at 02:27:15PM +1100, Dave Chinner wrote:
> > > From: Dave Chinner <dchinner@redhat.com>
> > >
...
> > > + /*
> > > + * For DAX, we do not allocate unwritten extents, but instead we zero
> > > + * the block before we commit the transaction. Ideally we'd like to do
> > > + * this outside the transaction context, but if we commit and then crash
> > > + * we may not have zeroed the blocks and this will be exposed on
> > > + * recovery of the allocation. Hence we must zero before commit.
> > > + * Further, if we are mapping unwritten extents here, we need to zero
> > > + * and convert them to written so that we don't need an unwritten extent
> > > + * callback for DAX. This also means that we need to be able to dip into
> > > + * the reserve block pool if there is no space left but we need to do
> > > + * unwritten extent conversion.
> > > + */
> > > + if (IS_DAX(VFS_I(ip))) {
> > > + bmapi_flags = XFS_BMAPI_CONVERT | XFS_BMAPI_ZERO;
> > > + tp->t_flags |= XFS_TRANS_RESERVE;
> > > + }
> >
> > Am I following the commit log description correctly in that block
> > zeroing is only required for DAX faults? Do we zero blocks for DAX DIO
> > as well to be consistent, or is that also required (because it looks
> > like we still have end_io completion for dio writes anyways)?
>
> DAX DIO will do the zeroing rather than using unwritten extents,
> too. But we still have DIO IO completion as that needs to do file
> size updates.
>

Right, my question is: is the DAX DIO zeroing required to avoid the
races described as the purpose for this patch, or is this just here as a
simplification? In other words, why not do block zeroing only for DAX
faults and not DAX/DIO?

I ask because my understanding is the purpose of this patch is a special
atomic zeroed allocation requirement just for mmap. Unless there is some
special mixed dio/mmap case I'm missing, doing so for DAX/DIO basically
causes a clear_pmem() over every page sized chunk of the target I/O
range for which we already have the data. Perhaps that is fine (for now)
from a performance perspective, but seems unnecessary. Further, we still
have write completion in place which means we can still handle unwritten
conversion just as easily for DAX/DIO as normal DIO.

Thoughts?

Brian

> Cheers,
>
> Dave.
> --
> Dave Chinner
> david@fromorbit.com
>
> _______________________________________________
> xfs mailing list
> xfs@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs