All of lore.kernel.org
 help / color / mirror / Atom feed
From: Maxim Patlasov <mpatlasov@virtuozzo.com>
To: Nikolaus Rath <Nikolaus@rath.org>
Cc: Miklos Szeredi <mszeredi@redhat.com>,
	linux-fsdevel <linux-fsdevel@vger.kernel.org>,
	"fuse-devel@lists.sourceforge.net"
	<fuse-devel@lists.sourceforge.net>
Subject: Re: [fuse] writeback cache triggers read() for O_WRONLY files - bug?
Date: Fri, 4 Aug 2017 17:11:07 -0700	[thread overview]
Message-ID: <9f7192d0-067e-933c-deab-bb4771db503a@virtuozzo.com> (raw)
In-Reply-To: <87ini3uoxj.fsf@vostro.rath.org>

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.«
>>
>

      reply	other threads:[~2017-08-05  0:11 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-08-04 18:47 [fuse] writeback cache triggers read() for O_WRONLY files - bug? Nikolaus Rath
2017-08-04 19:00 ` Nikolaus Rath
2017-08-04 19:04   ` Maxim Patlasov
2017-08-04 19:13     ` Nikolaus Rath
2017-08-05  0:11       ` Maxim Patlasov [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=9f7192d0-067e-933c-deab-bb4771db503a@virtuozzo.com \
    --to=mpatlasov@virtuozzo.com \
    --cc=Nikolaus@rath.org \
    --cc=fuse-devel@lists.sourceforge.net \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=mszeredi@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.