On Tue 02-01-18 13:40:32, Dan Williams wrote:
> On Tue, Jan 2, 2018 at 1:15 PM, Darrick J. Wong <darrick.wong@xxxxxxxxxx> wrote:
> > On Sat, Dec 23, 2017 at 04:57:04PM -0800, Dan Williams wrote:
> >> In preparation for the dax implementation to start associating dax pages
> >> to inodes via page->mapping, we need to provide a 'struct
> >> address_space_operations' instance for dax. Otherwise, direct-I/O
> >> triggers incorrect page cache assumptions and warnings.
> >>
> >> Cc: "Darrick J. Wong" <darrick.wong@xxxxxxxxxx>
> >> Cc: linux-xfs@xxxxxxxxxxxxxxx
> >> Signed-off-by: Dan Williams <dan.j.williams@xxxxxxxxx>
> >> ---
> >> fs/xfs/xfs_aops.c | 2 ++
> >> fs/xfs/xfs_aops.h | 1 +
> >> fs/xfs/xfs_iops.c | 5 ++++-
> >> 3 files changed, 7 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/fs/xfs/xfs_aops.c b/fs/xfs/xfs_aops.c
> >> index 21e2d70884e1..361915d53cef 100644
> >> --- a/fs/xfs/xfs_aops.c
> >> +++ b/fs/xfs/xfs_aops.c
> >> @@ -1492,3 +1492,5 @@ const struct address_space_operations xfs_address_space_operations = {
> >> .is_partially_uptodate = block_is_partially_uptodate,
> >> .error_remove_page = generic_error_remove_page,
> >> };
> >> +
> >> +DEFINE_FSDAX_AOPS(xfs_dax_address_space_operations, xfs_vm_writepages);
> >
> > Hmm, if we ever re-enable changing the DAX flag on the fly, will
> > mapping->a_ops have to change dynamically too?
> >
> > How sure are we that we'll never have to set anything in the dax aops
> > other than ->writepages?
> >
> > (I also kinda wonder why not just make the callers savvy vs. a bunch of
> > dummy aops, but maybe that's been tried in a previous iteration?)
>
> Matthew had similar feedback. I pushed back that I think more IS_DAX
> sprinkling increases the long term maintenance burden, but now that
> you've independently asked for the same thing I'm not opposed to
> changing my mind.
>
> Either way this need to switch the address_space_operations, or
> synchronize against in-flight address_space_operations is going to
> complicate the "dynamic toggle the dax mode" feature.
ext4 already dynamically switches aops for journalled data vs
non-journalled data vs nodelalloc case. That being said I'm not quite sure
this is bug-free ;)
Honza
--
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR
