linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Matthew Wilcox <willy@infradead.org>
To: David Laight <David.Laight@aculab.com>
Cc: Pavel Begunkov <asml.silence@gmail.com>,
	Alexander Viro <viro@zeniv.linux.org.uk>,
	"linux-fsdevel@vger.kernel.org" <linux-fsdevel@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] fs: optimise generic_write_check_limits()
Date: Sun, 8 Aug 2021 16:34:48 +0100	[thread overview]
Message-ID: <YQ/5mCUBmfCWoyVs@casper.infradead.org> (raw)
In-Reply-To: <567d7e15f59a45f6ab94428261b3e473@AcuMS.aculab.com>

On Sun, Aug 08, 2021 at 02:41:13PM +0000, David Laight wrote:
> From: Matthew Wilcox
> > Sent: 06 August 2021 14:28
> > 
> > On Fri, Aug 06, 2021 at 12:22:10PM +0100, Pavel Begunkov wrote:
> > > Even though ->s_maxbytes is used by generic_write_check_limits() only in
> > > case of O_LARGEFILE, the value is loaded unconditionally, which is heavy
> > > and takes 4 indirect loads. Optimise it by not touching ->s_maxbytes,
> > > if it's not going to be used.
> > 
> > Is this "optimisation" actually worth anything?  Look at how
> > force_o_largefile() is used.  I would suggest that on the vast majority
> > of machines, O_LARGEFILE is always set.
> 
> An option would be to only determine ->s_maxbytes when the size
> if larger than MAX_NON_LFS.
> 
> So you'd end up with something like:
> 
> 	if (pos >= max_size) {
> 		if (!(file->f_flags & O_LARGEFILE))
> 			return -EFBIG;
> 		inode = file->f_mapping->host;
> 		if (pos >= inode->i_sb->s_maxbytes)
> 			return -EFBIG;
> 	}

You're optimising the part of the function that you can see in the
diff instead of the whole function.  And there's no evidence that
there's much win to be had here ...

  reply	other threads:[~2021-08-08 15:35 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-06 11:22 [PATCH] fs: optimise generic_write_check_limits() Pavel Begunkov
2021-08-06 13:28 ` Matthew Wilcox
2021-08-07 10:05   ` Pavel Begunkov
2021-08-08 14:41   ` David Laight
2021-08-08 15:34     ` Matthew Wilcox [this message]
2021-08-06 13:46 ` Al Viro

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=YQ/5mCUBmfCWoyVs@casper.infradead.org \
    --to=willy@infradead.org \
    --cc=David.Laight@aculab.com \
    --cc=asml.silence@gmail.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=viro@zeniv.linux.org.uk \
    /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).