linux-kernel - Re: [PATCH 2/2] fuse: wait for writeback in fuse_file

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 13 Aug 2013 17:45:47 +0400
From:	Maxim Patlasov <mpatlasov@...allels.com>
To:	Brian Foster <bfoster@...hat.com>
CC:	<miklos@...redi.hu>, <fuse-devel@...ts.sourceforge.net>,
	<linux-kernel@...r.kernel.org>, <devel@...nvz.org>,
	<xemul@...allels.com>
Subject: Re: [PATCH 2/2] fuse: wait for writeback in fuse_file_fallocate()

08/13/2013 05:23 PM, Brian Foster пишет:
> On 08/13/2013 08:56 AM, Maxim Patlasov wrote:
>> 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?
>>
> I agree, though most users probably wouldn't expect that a blatant error
> like EOPNOTSUPP leave the range in a weird state. What's more, it only
> does so if it's the first attempt and behaves more appropriately after
> that.
>
>>> 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?
>>
> I was referring to something like filemap_write_and_wait_range(), for
> example. Then continue to use truncate_pagecache_range() as we do today.
> Thoughts

Looks nice. I'll send updated patch after some testing. Thanks a lot for 
the suggestion!

Maxim
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/