linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Vivek Goyal <vgoyal@redhat.com>
To: Bernd Schubert <bschubert@ddn.com>
Cc: Dharmendra Singh <dharamhans87@gmail.com>,
	miklos@szeredi.hu, linux-fsdevel@vger.kernel.org,
	fuse-devel@lists.sourceforge.net, linux-kernel@vger.kernel.org,
	Dharmendra Singh <dsingh@ddn.com>
Subject: Re: [PATCH v3 1/1] FUSE: Allow non-extending parallel direct writes on the same file.
Date: Thu, 26 May 2022 14:14:23 -0400	[thread overview]
Message-ID: <Yo/DfzU7IXZbADK5@redhat.com> (raw)
In-Reply-To: <3350e4e2-bad5-7f2f-2b09-c1807815a29c@ddn.com>

On Wed, May 25, 2022 at 10:49:49PM +0200, Bernd Schubert wrote:
> 
> 
> On 5/25/22 22:31, Vivek Goyal wrote:
> > On Fri, May 20, 2022 at 10:04:43AM +0530, Dharmendra Singh wrote:
> > > From: Dharmendra Singh <dsingh@ddn.com>
> > > 
> > > In general, as of now, in FUSE, direct writes on the same file are
> > > serialized over inode lock i.e we hold inode lock for the full duration
> > > of the write request. I could not found in fuse code a comment which
> > > clearly explains why this exclusive lock is taken for direct writes.
> > > Our guess is some USER space fuse implementations might be relying
> > > on this lock for seralization and also it protects for the issues
> > > arising due to file size assumption or write failures.  This patch
> > > relaxes this exclusive lock in some cases of direct writes.
> > 
> > I have this question as well. My understanding was that in general,
> > reads can do shared lock while writes have to take exclusive lock.
> > And I assumed that extends to both buffered as well as direct
> > writes.
> > 
> > I would also like to understand what's the fundamental restriction
> > and why O_DIRECT is special that this restriction does not apply.
> > 
> > Is any other file system doing this as well?
> > 
> > If fuse server dir is shared with other fuse clients, it is possible
> > that i_size in this client is stale. Will that be a problem. I guess
> > if that's the problem then, even a single write will be a problem
> > because two fuse clients might be trying to write.
> > 
> > Just trying to make sure that it is safe to allow parallel direct
> > writes.
> 
> I think missing in this series is to add a comment when this lock is needed
> at all. Our network file system is log structured - any parallel writes to
> the same file from different remote clients are handled through addition of
> fragments on the network server side - lockless safe due to byte level
> accuracy. With the exception of conflicting writes - last client wins -
> application is then doing 'silly' things - locking would not help either.
> And random parallel writes from the same (network) client are even an ideal
> case for us, as that is handled through shared blocks for different
> fragments (file offset + len). So for us shared writes are totally safe.
> 
> When Dharmendra and I discussed about the lock we came up with a few write
> error handling cases where that lock might be needed - I guess that should
> be added as comment.

Right, please add the changelogs to make thought process clear.

So there are no restrictions on the fuse client side from parallelism
point of view on direct write path? 

Why file extending writes are not safe? Is this a restriction from
fuse client point of view or just being safe from server point of view.
If fuse user space is talking to another filesystem, I guess then it
is not problem because that filesystem will take care of locking as
needed.

I see ext4 is allowing parallel direct writes for certain cases. And
where they can't allow it, they have documented it in comments.
(ext4_dio_write_checks()). I think we need similar rationalization,
especially from fuse client's perspective and have comments in code
and specify when it is ok to have parallel direct writes and when it
is not ok and why. This will help people when they are looking at
the code later.

Thanks
Vivek


      reply	other threads:[~2022-05-26 18:14 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-05-20  4:34 [PATCH v3 0/1] FUSE: Allow non-extending parallel direct writes Dharmendra Singh
2022-05-20  4:34 ` [PATCH v3 1/1] FUSE: Allow non-extending parallel direct writes on the same file Dharmendra Singh
2022-05-25 18:41   ` Vivek Goyal
2022-05-25 19:06     ` Bernd Schubert
2022-05-25 19:12       ` Vivek Goyal
2022-05-25 20:31   ` Vivek Goyal
2022-05-25 20:49     ` Bernd Schubert
2022-05-26 18:14       ` Vivek Goyal [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=Yo/DfzU7IXZbADK5@redhat.com \
    --to=vgoyal@redhat.com \
    --cc=bschubert@ddn.com \
    --cc=dharamhans87@gmail.com \
    --cc=dsingh@ddn.com \
    --cc=fuse-devel@lists.sourceforge.net \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=miklos@szeredi.hu \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).