All of lore.kernel.org
 help / color / mirror / Atom feed
From: Vivek Goyal <vgoyal@redhat.com>
To: Miklos Szeredi <miklos@szeredi.hu>
Cc: overlayfs <linux-unionfs@vger.kernel.org>,
	Amir Goldstein <amir73il@gmail.com>,
	virtio-fs-list <virtio-fs@redhat.com>
Subject: Re: [PATCH] overlayfs: Pass O_TRUNC flag to underlying filesystem
Date: Tue, 21 Apr 2020 15:31:53 -0400	[thread overview]
Message-ID: <20200421193153.GD28740@redhat.com> (raw)
In-Reply-To: <CAJfpeguGqLxA14unqNx-nExCWKs1Tv_0MwqGohHnuW-ugcjDyQ@mail.gmail.com>

On Tue, Apr 21, 2020 at 08:59:24PM +0200, Miklos Szeredi wrote:
> On Tue, Apr 21, 2020 at 8:41 PM Vivek Goyal <vgoyal@redhat.com> wrote:
> >
> > As of now during open(), we don't pass bunch of flags to underlying
> > filesystem. O_TRUNC is one of these. Normally this is not a problem as VFS
> > calls ->setattr() with zero size and underlying filesystem sets file size
> > to 0.
> >
> > But when overlayfs is running on top of virtiofs, it has an optimization
> > where it does not send setattr request to server if dectects that
> > truncation is part of open(O_TRUNC). It assumes that server already zeroed
> > file size as part of open(O_TRUNC).
> >
> > fuse_do_setattr() {
> >         if (attr->ia_valid & ATTR_OPEN) {
> >                 /*
> >                  * No need to send request to userspace, since actual
> >                  * truncation has already been done by OPEN.  But still
> >                  * need to truncate page cache.
> >                  */
> >         }
> > }
> >
> > IOW, fuse expects O_TRUNC to be passed to it as part of open flags.
> >
> > But currently overlayfs does not pass O_TRUNC to underlying filesystem
> > hence fuse/virtiofs breaks. Setup overlayfs on top of virtiofs and
> > following does not zero the file size of a file is either upper only
> > or has already been copied up.
> >
> > fd = open(foo.txt, O_TRUNC | O_WRONLY);
> >
> > Fix it by passing O_TRUNC to underlying filesystem.
> 
> 
> Or clear ATTR_OPEN in ovl_setattr()
> 
> Need to think about side effects of passing O_TRUNC down to underlying
> fs.   Clearing ATTR_OPEN seems obviously safe, so as a quick fix I'd
> rather go with that for now.

Ok, I will send another patch which clears ATTR_OPEN in ovl_setattr().

Thanks
Vivek


WARNING: multiple messages have this Message-ID (diff)
From: Vivek Goyal <vgoyal@redhat.com>
To: Miklos Szeredi <miklos@szeredi.hu>
Cc: virtio-fs-list <virtio-fs@redhat.com>,
	Amir Goldstein <amir73il@gmail.com>,
	overlayfs <linux-unionfs@vger.kernel.org>
Subject: Re: [Virtio-fs] [PATCH] overlayfs: Pass O_TRUNC flag to underlying filesystem
Date: Tue, 21 Apr 2020 15:31:53 -0400	[thread overview]
Message-ID: <20200421193153.GD28740@redhat.com> (raw)
In-Reply-To: <CAJfpeguGqLxA14unqNx-nExCWKs1Tv_0MwqGohHnuW-ugcjDyQ@mail.gmail.com>

On Tue, Apr 21, 2020 at 08:59:24PM +0200, Miklos Szeredi wrote:
> On Tue, Apr 21, 2020 at 8:41 PM Vivek Goyal <vgoyal@redhat.com> wrote:
> >
> > As of now during open(), we don't pass bunch of flags to underlying
> > filesystem. O_TRUNC is one of these. Normally this is not a problem as VFS
> > calls ->setattr() with zero size and underlying filesystem sets file size
> > to 0.
> >
> > But when overlayfs is running on top of virtiofs, it has an optimization
> > where it does not send setattr request to server if dectects that
> > truncation is part of open(O_TRUNC). It assumes that server already zeroed
> > file size as part of open(O_TRUNC).
> >
> > fuse_do_setattr() {
> >         if (attr->ia_valid & ATTR_OPEN) {
> >                 /*
> >                  * No need to send request to userspace, since actual
> >                  * truncation has already been done by OPEN.  But still
> >                  * need to truncate page cache.
> >                  */
> >         }
> > }
> >
> > IOW, fuse expects O_TRUNC to be passed to it as part of open flags.
> >
> > But currently overlayfs does not pass O_TRUNC to underlying filesystem
> > hence fuse/virtiofs breaks. Setup overlayfs on top of virtiofs and
> > following does not zero the file size of a file is either upper only
> > or has already been copied up.
> >
> > fd = open(foo.txt, O_TRUNC | O_WRONLY);
> >
> > Fix it by passing O_TRUNC to underlying filesystem.
> 
> 
> Or clear ATTR_OPEN in ovl_setattr()
> 
> Need to think about side effects of passing O_TRUNC down to underlying
> fs.   Clearing ATTR_OPEN seems obviously safe, so as a quick fix I'd
> rather go with that for now.

Ok, I will send another patch which clears ATTR_OPEN in ovl_setattr().

Thanks
Vivek


  reply	other threads:[~2020-04-21 19:32 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-04-21 18:41 [PATCH] overlayfs: Pass O_TRUNC flag to underlying filesystem Vivek Goyal
2020-04-21 18:41 ` [Virtio-fs] " Vivek Goyal
2020-04-21 18:59 ` Miklos Szeredi
2020-04-21 18:59   ` [Virtio-fs] " Miklos Szeredi
2020-04-21 19:31   ` Vivek Goyal [this message]
2020-04-21 19:31     ` Vivek Goyal
2020-04-21 21:04   ` Vivek Goyal
2020-04-21 21:04     ` [Virtio-fs] " Vivek Goyal
2020-04-22  8:31     ` Miklos Szeredi
2020-04-22  8:31       ` [Virtio-fs] " Miklos Szeredi

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=20200421193153.GD28740@redhat.com \
    --to=vgoyal@redhat.com \
    --cc=amir73il@gmail.com \
    --cc=linux-unionfs@vger.kernel.org \
    --cc=miklos@szeredi.hu \
    --cc=virtio-fs@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.