Re: [PATCH 2/2] fuse: wait for writeback in fuse_file_fallocate()

[Maxim Patlasov <mpatlasov@parallels.com>]

Hi,

08/13/2013 04:05 PM, Brian Foster пишет:
> ...
> @@ -2478,8 +2516,11 @@ static long fuse_file_fallocate(struct file *file, int mode, loff_t offset,
>   
>   	if (lock_inode) {
>   		mutex_lock(&inode->i_mutex);
> -		if (mode & FALLOC_FL_PUNCH_HOLE)
> -			fuse_set_nowrite(inode);
> +		if (mode & FALLOC_FL_PUNCH_HOLE) {
> +			truncate_pagecache_range(inode, offset,
> +						 offset + length - 1);
> +			fuse_wait_on_writeback(inode, offset, length);
> +		}
> If this happens to be the first attempt on an fs that doesn't support
> fallocate, we'll return -EOPNOTSUPP after having already punched out the
> data in the pagecache.

Yes, this is unpleasant, but it's not critical, imo. We're returning an 
error code (even though equal to -EOPNOTSUPP) and a sane application 
should not make any assumption about current state of the punched 
region. Also, the application intended to discard given region of the 
file, so why should it pay care for its content afterwards?

> What about replacing the nowrite logic with a
> flush (and still followed by your new writeback wait logic) rather than
> moving the pagecache truncate?

The "flush" you mentioned should firstly flush page cache. 
invalidate_inode_pages2_range() seems to be a candidate. We definitely 
cannot ignore error code from it because it can be fuse_launder_page() 
who got -ENOMEM from fuse_writepage_locked(). In case of err == -ENOMEM, 
we could safely fail fallocate, but what should we do if it's -EBUSY? 
Any ideas?

Thanks,
Maxim