Re: [PATCH v4 00/12] re-enable DAX PMD support
From: Ross Zwisler
Date: Mon Oct 03 2016 - 14:54:18 EST
On Thu, Sep 29, 2016 at 09:00:55PM -0700, Darrick J. Wong wrote:
> On Thu, Sep 29, 2016 at 09:03:43PM -0600, Ross Zwisler wrote:
> > On Fri, Sep 30, 2016 at 09:43:45AM +1000, Dave Chinner wrote:
> > > Finally: none of the patches in your tree have reviewed-by tags.
> > > That says to me that none of this code has been reviewed yet.
> > > Reviewed-by tags are non-negotiable requirement for anything going
> > > through my trees. I don't have time right now to review this code,
> > > so you're going to need to chase up other reviewers before merging.
> > >
> > > And, really, this is getting very late in the cycle to be merging
> > > new code - we're less than one working day away from the merge
> > > window opening and we've missed the last linux-next build. I'd
> > > suggest that we'd might be best served by slipping this to the PMD
> > > support code to the next cycle when there's no time pressure for
> > > review and we can get a decent linux-next soak on the code.
> >
> > I absolutely support your policy of only sending code to Linux that has passed
> > peer review.
> >
> > However, I do feel compelled to point out that this is not new code. I didn't
> > just spring it on everyone in the hours before the v4.8 merge window. I
> > posted the first version of this patch set on August 15th, *seven weeks ago*:
> >
> > https://lkml.org/lkml/2016/8/15/613
> >
> > This was the day after v4.7-rc2 was released.
> >
> > Since then I have responded promptly to the little review feedback
> > that I've received. I've also reviewed and tested other DAX changes,
> > like the struct iomap changes from Christoph. Those changes were
> > first posted to the mailing list on September 9th, four weeks after
> > mine. Nevertheless, I was happy to rebase my changes on top of his,
> > which meant a full rewrite of the DAX PMD fault handler so it would be
> > based on struct iomap. His changes are going to be merged for v4.9,
> > and mine are not.
>
> I'm not knocking the iomap migration, but it did cause a fair amount of
> churn in the XFS reflink patchset -- and that's for a filesystem that
> already /had/ iomap implemented. It'd be neat to have all(?) the DAX
> filesystems (ext[24], XFS) move over to iomap so that you wouldn't have
> to support multiple ways of talking to FSes. AFAICT ext4 hasn't gotten
> iomap, which complicates things. But that's my opinion, maybe you're
> fine with supporting iomap and not-iomap.
I agree that we want to move everything over to be iomap based. I think
Christoph is already working on moving ext4 over, but as of this set PMD
support explicitly depends on the iomap interface, and I'm itching to remove
the struct buffer_head + get_block_t PTE path and I/O path as well.