[fuse] writeback cache triggers read() for O_WRONLY files - bug?

[fuse] writeback cache triggers read() for O_WRONLY files - bug?

[Nikolaus Rath]

Hello,

When enabling writeback cache for SSHFS, appending to files overwrites
data at a different position (cf. https://github.com/libfuse/sshfs/issues/72).

When trying to track this issue down, I noticed that the libfuse
passthrough_ll example also has problems with appending: calling
fuse_reply_data gives a "Bad File Descriptior" error.

This in turn I traced this down to the fact that when writeback caching
is enabled, and userspace calls open(name, O_WRONLY|O_APPEND) the kernel
passes the O_WRONLY flag on to libfuse. But when userspace then writes
data, the kernel issues a read() request to libfuse (presumably to fill
the write cache) - for a file that has been opened write-only.

In the passthrough_ll example, this then fails because the underlying
file has also been opened O_WRONLY and the strace read then fails.

I am not sure what to make of this. Is this behavior of the kernel
correct? Should libfuse be prepared to accept READ requests for files
that have been opened write-only? Or should the kernel never open files
write-only when writeback caching is enabled?

(I am not sure if something like this is also the cause of the
dataloss problem in SSHFS, but it seems worth addressing in any case)

Thanks,
-Nikolaus

-- 
GPG Fingerprint: ED31 791B 2C5C 1613 AF38 8B8A D113 FCAC 3C4E 599F

             »Time flies like an arrow, fruit flies like a Banana.«

Re: [fuse] writeback cache triggers read() for O_WRONLY files - bug?

[Nikolaus Rath]

Hi again,

Small clarification: the O_APPEND flag muddles the water a bit (I'll
write a second mail about that). The behavior that I'm describing here
also happens if userspace opens without O_APPEND but seeks to the end of
the file before writing.

Best,
-Nikolaus

On Aug 04 2017, Nikolaus Rath <Nikolaus@rath.org> wrote:
> Hello,
>
> When enabling writeback cache for SSHFS, appending to files overwrites
> data at a different position (cf. https://github.com/libfuse/sshfs/issues/72).
>
> When trying to track this issue down, I noticed that the libfuse
> passthrough_ll example also has problems with appending: calling
> fuse_reply_data gives a "Bad File Descriptior" error.
>
> This in turn I traced this down to the fact that when writeback caching
> is enabled, and userspace calls open(name, O_WRONLY|O_APPEND) the kernel
> passes the O_WRONLY flag on to libfuse. But when userspace then writes
> data, the kernel issues a read() request to libfuse (presumably to fill
> the write cache) - for a file that has been opened write-only.
>
> In the passthrough_ll example, this then fails because the underlying
> file has also been opened O_WRONLY and the strace read then fails.
>
> I am not sure what to make of this. Is this behavior of the kernel
> correct? Should libfuse be prepared to accept READ requests for files
> that have been opened write-only? Or should the kernel never open files
> write-only when writeback caching is enabled?
>
> (I am not sure if something like this is also the cause of the
> dataloss problem in SSHFS, but it seems worth addressing in any case)
>
> Thanks,
> -Nikolaus
>
> -- 
> GPG Fingerprint: ED31 791B 2C5C 1613 AF38 8B8A D113 FCAC 3C4E 599F
>
>              »Time flies like an arrow, fruit flies like a Banana.«


-- 
GPG Fingerprint: ED31 791B 2C5C 1613 AF38 8B8A D113 FCAC 3C4E 599F

             »Time flies like an arrow, fruit flies like a Banana.«

Re: [fuse] writeback cache triggers read() for O_WRONLY files - bug?

[Maxim Patlasov]

Hi Nikolaus,


If writeback caching is enabled, libfuse must be prepared to get READ 
requests from kernel. Hence, even if an user wants WRONLY, libfuse 
should open RDWR.


Thanks,

Maxim


On 08/04/2017 12:00 PM, Nikolaus Rath wrote:
> Hi again,
>
> Small clarification: the O_APPEND flag muddles the water a bit (I'll
> write a second mail about that). The behavior that I'm describing here
> also happens if userspace opens without O_APPEND but seeks to the end of
> the file before writing.
>
> Best,
> -Nikolaus
>
> On Aug 04 2017, Nikolaus Rath <Nikolaus@rath.org> wrote:
>> Hello,
>>
>> When enabling writeback cache for SSHFS, appending to files overwrites
>> data at a different position (cf. https://github.com/libfuse/sshfs/issues/72).
>>
>> When trying to track this issue down, I noticed that the libfuse
>> passthrough_ll example also has problems with appending: calling
>> fuse_reply_data gives a "Bad File Descriptior" error.
>>
>> This in turn I traced this down to the fact that when writeback caching
>> is enabled, and userspace calls open(name, O_WRONLY|O_APPEND) the kernel
>> passes the O_WRONLY flag on to libfuse. But when userspace then writes
>> data, the kernel issues a read() request to libfuse (presumably to fill
>> the write cache) - for a file that has been opened write-only.
>>
>> In the passthrough_ll example, this then fails because the underlying
>> file has also been opened O_WRONLY and the strace read then fails.
>>
>> I am not sure what to make of this. Is this behavior of the kernel
>> correct? Should libfuse be prepared to accept READ requests for files
>> that have been opened write-only? Or should the kernel never open files
>> write-only when writeback caching is enabled?
>>
>> (I am not sure if something like this is also the cause of the
>> dataloss problem in SSHFS, but it seems worth addressing in any case)
>>
>> Thanks,
>> -Nikolaus
>>
>> -- 
>> GPG Fingerprint: ED31 791B 2C5C 1613 AF38 8B8A D113 FCAC 3C4E 599F
>>
>>               »Time flies like an arrow, fruit flies like a Banana.«
>

Re: [fuse] writeback cache triggers read() for O_WRONLY files - bug?

[Nikolaus Rath]

Hi Maxim,

Any reason the kernel doesn't fix up the flags before sending the open
request?

I assume the kernel will enforce that userspace can't read from a file
opened with O_WRONLY if writeback cache is enabled? Is the same also
true without writeback cache, i.e. can the filesystem safely ignore
O_WRONLY/O_RDONLY at all times?

Thanks!
-Nikolaus

On Aug 04 2017, Maxim Patlasov <mpatlasov@virtuozzo.com> wrote:
> Hi Nikolaus,
>
>
> If writeback caching is enabled, libfuse must be prepared to get READ
> requests from kernel. Hence, even if an user wants WRONLY, libfuse
> should open RDWR.
>
>
> Thanks,
>
> Maxim
>
>
> On 08/04/2017 12:00 PM, Nikolaus Rath wrote:
>> Hi again,
>>
>> Small clarification: the O_APPEND flag muddles the water a bit (I'll
>> write a second mail about that). The behavior that I'm describing here
>> also happens if userspace opens without O_APPEND but seeks to the end of
>> the file before writing.
>>
>> Best,
>> -Nikolaus
>>
>> On Aug 04 2017, Nikolaus Rath <Nikolaus@rath.org> wrote:
>>> Hello,
>>>
>>> When enabling writeback cache for SSHFS, appending to files overwrites
>>> data at a different position (cf. https://github.com/libfuse/sshfs/issues/72).
>>>
>>> When trying to track this issue down, I noticed that the libfuse
>>> passthrough_ll example also has problems with appending: calling
>>> fuse_reply_data gives a "Bad File Descriptior" error.
>>>
>>> This in turn I traced this down to the fact that when writeback caching
>>> is enabled, and userspace calls open(name, O_WRONLY|O_APPEND) the kernel
>>> passes the O_WRONLY flag on to libfuse. But when userspace then writes
>>> data, the kernel issues a read() request to libfuse (presumably to fill
>>> the write cache) - for a file that has been opened write-only.
>>>
>>> In the passthrough_ll example, this then fails because the underlying
>>> file has also been opened O_WRONLY and the strace read then fails.
>>>
>>> I am not sure what to make of this. Is this behavior of the kernel
>>> correct? Should libfuse be prepared to accept READ requests for files
>>> that have been opened write-only? Or should the kernel never open files
>>> write-only when writeback caching is enabled?
>>>
>>> (I am not sure if something like this is also the cause of the
>>> dataloss problem in SSHFS, but it seems worth addressing in any case)
>>>
>>> Thanks,
>>> -Nikolaus
>>>
>>> -- 
>>> GPG Fingerprint: ED31 791B 2C5C 1613 AF38 8B8A D113 FCAC 3C4E 599F
>>>
>>>               »Time flies like an arrow, fruit flies like a Banana.«
>>
>
>


-- 
GPG Fingerprint: ED31 791B 2C5C 1613 AF38 8B8A D113 FCAC 3C4E 599F

             »Time flies like an arrow, fruit flies like a Banana.«

Re: [fuse] writeback cache triggers read() for O_WRONLY files - bug?

[Maxim Patlasov]

On 08/04/2017 12:13 PM, Nikolaus Rath wrote:

> Hi Maxim,
>
> Any reason the kernel doesn't fix up the flags before sending the open
> request?

Because it's more flexible: filesystem knows that the user wants 
O_WRONLY and it knows writeback cache is enabled, hence it can derive 
that READs are expected. Otherwise, we'd loose that knowledge about user 
intention to open a file for write only.

>
> I assume the kernel will enforce that userspace can't read from a file
> opened with O_WRONLY if writeback cache is enabled? Is the same also
> true without writeback cache, i.e. can the filesystem safely ignore
> O_WRONLY/O_RDONLY at all times?

No, without writeback cache, filesystem won't see READs for a file 
opened with O_WRONLY. That's because kernel fuse_send_write_pages() 
calls SetPageUptodate(page) only if given WRITE covers the whole page. 
We can't do the same for writeback cache because by the time of actual 
writeback we wouldn't know which part of page is up-to-date and because 
it would impact performance.


But generally, yes, it seems pretty harmless if libfuse assumed RDWR 
semantics for all O_WRONLY open requests.

Thanks,
Maxim

>
> Thanks!
> -Nikolaus
>
> On Aug 04 2017, Maxim Patlasov <mpatlasov@virtuozzo.com> wrote:
>> Hi Nikolaus,
>>
>>
>> If writeback caching is enabled, libfuse must be prepared to get READ
>> requests from kernel. Hence, even if an user wants WRONLY, libfuse
>> should open RDWR.
>>
>>
>> Thanks,
>>
>> Maxim
>>
>>
>> On 08/04/2017 12:00 PM, Nikolaus Rath wrote:
>>> Hi again,
>>>
>>> Small clarification: the O_APPEND flag muddles the water a bit (I'll
>>> write a second mail about that). The behavior that I'm describing here
>>> also happens if userspace opens without O_APPEND but seeks to the end of
>>> the file before writing.
>>>
>>> Best,
>>> -Nikolaus
>>>
>>> On Aug 04 2017, Nikolaus Rath <Nikolaus@rath.org> wrote:
>>>> Hello,
>>>>
>>>> When enabling writeback cache for SSHFS, appending to files overwrites
>>>> data at a different position (cf. https://github.com/libfuse/sshfs/issues/72).
>>>>
>>>> When trying to track this issue down, I noticed that the libfuse
>>>> passthrough_ll example also has problems with appending: calling
>>>> fuse_reply_data gives a "Bad File Descriptior" error.
>>>>
>>>> This in turn I traced this down to the fact that when writeback caching
>>>> is enabled, and userspace calls open(name, O_WRONLY|O_APPEND) the kernel
>>>> passes the O_WRONLY flag on to libfuse. But when userspace then writes
>>>> data, the kernel issues a read() request to libfuse (presumably to fill
>>>> the write cache) - for a file that has been opened write-only.
>>>>
>>>> In the passthrough_ll example, this then fails because the underlying
>>>> file has also been opened O_WRONLY and the strace read then fails.
>>>>
>>>> I am not sure what to make of this. Is this behavior of the kernel
>>>> correct? Should libfuse be prepared to accept READ requests for files
>>>> that have been opened write-only? Or should the kernel never open files
>>>> write-only when writeback caching is enabled?
>>>>
>>>> (I am not sure if something like this is also the cause of the
>>>> dataloss problem in SSHFS, but it seems worth addressing in any case)
>>>>
>>>> Thanks,
>>>> -Nikolaus
>>>>
>>>> -- 
>>>> GPG Fingerprint: ED31 791B 2C5C 1613 AF38 8B8A D113 FCAC 3C4E 599F
>>>>
>>>>                »Time flies like an arrow, fruit flies like a Banana.«
>>
>