Re: [PATCH 3/6] xfs: add DAX file operations support

Boaz Harrosh
2015-03-24T08:13:57Z

On 03/24/2015 06:27 AM, Dave Chinner wrote:
> On Thu, Mar 05, 2015 at 09:03:48AM +1100, Dave Chinner wrote:
>> On Wed, Mar 04, 2015 at 04:54:50PM +0200, Boaz Harrosh wrote:
>>> On 03/04/2015 03:01 PM, Dave Chinner wrote:
>>>> On Wed, Mar 04, 2015 at 12:09:40PM +0200, Boaz Harrosh wrote:
>>> <>
>>>>
>>>> So, we definitely need splice to/from DAX enabled inodes to be
>>>> rejected. I'll have a look at that...
>>>>
>>>
>>> default_file_splice_read uses kernel_readv which I think might actually
>>> work. Do you know what xfstest(s) exercise splice?
>>
>> We have a rudimentary one only because I discovered a while back
>> none existed at all. i.e. splice is effectively untested by
>> xfstests. If you want to write some tests to execise it, that'd be
>> great....
>
> Turns out there's no great need to write splice tests for xfstests -
> the current loopback device uses splice, and so all of the tests
> that run on loopback are exercising the splice path through the
> filesystem.
>
> I found this out by disabling splice on dax altogether, and then finding out
> that lots of tests failed badly, then narrowing it down to:
>
> $ sudo mount -o dax /dev/ram0 /mnt/test
> $ sudo mkfs.xfs -dfile,name=/mnt/test/foo1,size=1g
> meta-data=/mnt/test/foo1 isize=512 agcount=4, agsize=65536 blks
> = sectsz=512 attr=2, projid32bit=1
> = crc=1 finobt=1
> data = bsize=4096 blocks=262144, imaxpct=25
> = sunit=0 swidth=0 blks
> naming =version 2 bsize=4096 ascii-ci=0 ftype=1
> log =internal log bsize=4096 blocks=2560, version=2
> = sectsz=512 sunit=0 blks, lazy-count=1
> realtime =none extsz=4096 blocks=0, rtextents=0
> $ sudo mount -o loop /mnt/test/foo1 /mnt/test/foo
> mount: /dev/loop0: can't read superblock
> $
>
> because the splice read returned EINVAL rather than data. So, yes,
> splice canbe made to work with dax if we pass it through the paths
> that aren't interacting directly with the page cache.
>

Cool so current dax code actually does support splice by using
default_file_splice_read/write indirectly.

therefor I think there is merit in keeping just the one
file_operations vector pointing to an internal function and
doing an if (IS_DAX()) default_file_splice_read/write()
at run time.

Because with current code, if CONFIG_FS_DAX is enabled at compile
time, then also the regular HD none dax mounts will use the slow
default_file_splice_read instead of what ever something better
that the FS is doing.

Do you think we should do the IS_DAX() switch at
generic_file_splice_read and iter_file_splice_write to
fix all the Fss in one go or point to an internal FS function
and do the switch there? Please advise?

I will send up a patch that fixes up ext2 ext4, to see how it
looks like.

> Cheers,
> Dave.
>

Thanks Dave
Boaz