[fuse-devel] Cached attr + mmap query

[Pete M. <pe...@ta...>]

Morning there,

I'm currently working on implementing a test version of a pre-defined 
driver interface for a device which produces large quantities of data 
output - It's an MPEG stream processor, essentially. The drivers allow 
the user to size the buffer, and then mmap it, at which point the device 
starts to fill the buffer with data. The user calls something like the 
following sequence:

Open device node
Call an ioctl to set the size of the buffer
Mmap the device
Poll the device for events
On Data-Ready event, read data from mmap buffer

I've stumbled across something that I'm not sure is quite right with the 
mmap implementation, and I'm interested to know if there's just a 
function or two that I've missed. For reference, I'm on the low level 
interface.

The behaviour that I see -

Device opens successfully. I initially return the size as 0 to any attr 
request.
ioctl gets called. I call fuse_notify_inval_inode to flush the attrs, as 
this size change is outside of FUSEs control.
mmap gets called. All seems happy
Client now tries to read or write the mmap buffer, and everything falls 
apart. On my embedded target, it results in a SIGBUS - On my Ubuntu 
desktop, it results in a hung and unkillable process.

After the ioctl, if the client calls stat, the correct size is returned. 
Similarly, read/write/lseek work as expected. In fact, if you call 
stat/read/write/lseek, then call mmap, everything works perfectly. This 
leads me a little deeper.

It seems that the fuse_file_vm_ops struct for the file hands off fault 
handing to filemap_fault in mm/filemap.c - this uses the inode size to 
determine how it behaves and indeed, if the inode size <= the file 
offset, it'll result in SIGBUS, which makes a whole lot of sense.  So 
far, so good. Looking at the FUSE source, the inode size gets updated 
when the attrs get flushed (fuse_change_attributes and friends), and 
fuse_file_llseek (for example) calls fuse_update_attributes before it 
does anything too serious.

However, mmap doesn't seem to bother updating attrs before getting on 
with business, which means that the inode size that the kernel knows 
about may be stale. If it helps, I've got sample client and driver code 
which can demonstrate this, although I'll have to get it cleaned up 
before I let anyone else see it :)

I attempted a brief hack within the mmap implementation to update the 
attrs before anything else - it's a few lines of change that make the 
top of fuse_file_mmap look like this:

static int fuse_file_mmap(struct file *file, struct vm_area_struct *vma)
{
	struct inode *inode = file_inode(file);
	struct fuse_inode *fi = get_fuse_inode(inode);

	mutex_lock(&inode->i_mutex);
	retval = fuse_update_attributes(inode, NULL, file, NULL);
	mutex_unlock(&inode->i_mutex);

	if ((vma->vm_flags & VM_SHARED) && (vma->vm_flags & VM_MAYWRITE)) {
		struct fuse_conn *fc = get_fuse_conn(inode);
		struct fuse_file *ff = file->private_data;
		/*
		 * file may be written through mmap, so chain it onto the
		 * inodes's write_file list
		 */



And that seems to resolve the issue - I see getattrs getting called, and 
the mmap buffer works beautifully.

So - My question - Is this a minor hole in the mmap implementation (I 
understand that what I'm doing is pretty non-standard, and probably far 
from recommended), or am I missing something stupid? Is there something 
about calling getattr from the middle of mmap that may cause the kernel 
to come crashing down around us? (While I'm comfortable coding 
generally, I've not spent a lot of time in kernel-space :) )

Extra bonus question - Some of our device state machine is moved on when 
the client calls mmap. In current FUSE, the driver process doesn't get 
notified when mmap is called (and I understand that in most cases, the 
underlying FS won't care). Is there any way of getting a notification, 
other than altering the FUSE source to add one? Is there a reason that's 
not already done (other than "don't be daft - the underlying FS 
shouldn't care") that I'm likely to fall over?

Thanks in advance for your help, and massive thanks for maintaining a 
staggeringly handy bit of the kernel :)

Pete