linux-kernel - Re: [PATCH] fs: optimise generic_write_check

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <YQ/5mCUBmfCWoyVs@casper.infradead.org>
Date:   Sun, 8 Aug 2021 16:34:48 +0100
From:   Matthew Wilcox <willy@...radead.org>
To:     David Laight <David.Laight@...lab.com>
Cc:     Pavel Begunkov <asml.silence@...il.com>,
        Alexander Viro <viro@...iv.linux.org.uk>,
        "linux-fsdevel@...r.kernel.org" <linux-fsdevel@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] fs: optimise generic_write_check_limits()

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