fscache/cachefiles versus btrfs

hi,
fscache cannot currently be used with btrfs as the backing store for the cache (managed by cachefilesd).
This is because cachefiles needs the ->bmap address_space_operation, and btrfs doesn't provide it.

cachefiles only uses this to find out if a particular page is a 'hole' or not. For btrfs, this can be done with 'SEEK_DATA'.

Unfortunately it doesn't seem to be possible to query a filesystem or a file to see if SEEK_DATA is reliable or not, so we cannot simply use SEEK_DATA when reliable, else ->bmap if available.

The following patch make fscache work for me on btrfs. It explicitly checks for BTRFS_SUPER_MAGIC. Not really a nice solution, but all I could think of.

Is there a better way? Could a better way be created? Maybe SEEK_DATA_RELIABLE ??

Comments, suggestions welcome.

Also, if you do try to use fscache on btrfs with 3.19, then nothing gets cached (as expected) and with a heavy load you can lose a race and get an asserting fail in fscache_enqueue_operation

	ASSERT(fscache_object_is_available(op->object));

It looks like the object is being killed before it is available...

[  859.700765] kernel BUG at ../fs/fscache/operation.c:38!
...
[  859.703124] Call Trace:
[  859.703193]  [<ffffffffa0448044>] fscache_run_op.isra.4+0x34/0x80 [fscache]
[  859.703260]  [<ffffffffa0448160>] fscache_start_operations+0xa0/0xf0 [fscache]
[  859.703388]  [<ffffffffa0446cd8>] fscache_kill_object+0x98/0xc0 [fscache]
[  859.703455]  [<ffffffffa04475c1>] fscache_object_work_func+0x151/0x210 [fscache]
[  859.703578]  [<ffffffff81078b07>] process_one_work+0x147/0x3c0
[  859.703642]  [<ffffffff8107929c>] worker_thread+0x20c/0x470

I haven't figured out the cause of that yet.

Thanks,
NeilBrown

index 1e51714eb33e..1389d8483d5d 100644
@@ -20,6 +20,7 @@
 #include <linux/namei.h>
 #include <linux/security.h>
 #include <linux/slab.h>
+#include <linux/magic.h>
 #include "internal.h"
 
 #define CACHEFILES_KEYBUF_SIZE 512

@@ -647,7 +648,8 @@ lookup_again:
 
 			ret = -EPERM;
 			aops = object->dentry->d_inode->i_mapping->a_ops;
-			if (!aops->bmap)
+			if (!aops->bmap &&
+			    object->dentry->d_sb->s_magic != BTRFS_SUPER_MAGIC)
 				goto check_error;
 
 			object->backer = object->dentry;

fiemap() maybe?

Do you have the patches here applied?

http://git.kernel.org/cgit/linux/kernel/git/dhowells/linux-fs.git/log/?h=fscache-fixes

David

fiemap is not reliable for mapping holes - it returns extent info, not whether there is data in a range. i.e. there can be data over a hole (e.g. delayed allocation) and fiemap will return it as a hole. cp made this mistake back when fiemap was first introduced, resulting in corrupt file copies.

SEEK_HOLE/SEEK_DATA is what you want, as they are page cache coherent, not extent based operations. And, really if you need it to really be able to find real holes, then a superblock flag might be a better way of marking filesystems with the required capability.

Cheers,

Dave.

Dave Chinner
[email protected]

I assumed from the doco that FIEMAP_EXTENT_DELALLOC would get returned in this case. I guess not?

btrfs seems to use the same underlying functionality for both fiemap and SEEK_HOLE/SEEK_DATA...

But yes: a new fs_flags flag is probably the go.

Thanks,
NeilBrown

FIEMAP is advisory and implementing features in it are optional. It's also not guaranteed to be an accurate representation of the layout of the file because it can change even before hte FIEMAP ioctl returns to userspace. it's also a representation of the physical layout of the data in the file, but it does not need to be coherent with the current data in the file. i.e. FIEMAP is not required to report the current state of data in the files to the caller, just the working physical layout sitting in memory...

That's just an internal implementation detail - it does not define the behaviour and constraints of the information FIEMAP or SEEK_HOLE/SEEK_DATA APIs are supposed to provide.

Cheers,

Dave.

Dave Chinner
[email protected]

Actually, I wonder if what I want is a kernel_read() that returns ENODATA upon encountering a hole at the beginning of the area to be read.

Of course, what I really, really want is asynchronous, direct read and write within the kernel, where the read will notify you of any holes, but will read all the data it can around those holes.

David

NFS READ_PLUS could also make use of this, but someone needs to actually implement it.

Until we have that lseek SEEK_HOLE/DATA is the way to go, and the horrible ->bmap hack needs to die ASAP, I can't believe you managed to sneak that in in the not too distant past.

Do I don't. I had looked through them and while they did seem to be addressing similar sorts of races, nothing seems like an obvious match. I haven't been able to reproduce the BUG_ON myself. I only have a report of it repeatedly affecting someone else:

https://bugzilla.opensuse.org/show_bug.cgi?id=908706

I'll probably have to be happy with fixing usage on btrfs, and hope the other bug is fixed already or doesn't become a problem.

Thanks,
NeilBrown

I managed to reproduce the bug, and when I applied your patches I cannot any more. So it looks like you've fixed it - thanks.

That just leave the bmap issue. I'll post a patch which causes lseek to be used when the fs says that is OK.

Thanks,
NeilBrown

I hope so too. Now I just hope Linus takes the patches.

Okay, thanks.

David