Re: [PATCH 1/2] iomap: Add iomap_iter API

[Jan Kara <jack@suse.cz>]

On Wed 01-04-20 11:42:45, Matthew Wilcox wrote:
> On Wed, Apr 01, 2020 at 08:42:48AM -0700, Christoph Hellwig wrote:
> > > +loff_t iomap_iter(struct iomap_iter *iter, loff_t written)
> > > +{
> > > +	const struct iomap_ops *ops = iter->ops;
> > > +	struct iomap *iomap = &iter->iomap;
> > > +	struct iomap *srcmap = &iter->srcmap;
> > 
> > I think it makes sense to only have members in the iter structure
> > that this function modifies.  That is, just pass inode, ops and flags
> > as explicit parameters.
> 
> One of the annoying things we do when looking at the disassembly is
> spend a lot of instructions shuffling arguments around.  Passing as many
> arguments as possible in a struct minimises that.

Somewhat late to the game but ... from the conversions of "explicit
arguments to struct of arguments" I've seen (e.g. in xarray) compilers seem
to generate somewhat slower code when arguments are passed in structs. From
the profiling I did it just seems that when arguments are passed directly,
they are in registers which is generally the fastest access you can get.
When you pass arguments in structs, compilers just fetch the value from
stack which is slower even if its cached. And when the argument is not used
frequently or there's something else cache heavy going on, you may have to
go to L2 or L3 which is when you feel the pain... E.g. I've observed some
of the xarray functions which were "logically" identical to their
radix-tree counterparts generate non-negligible amount of cache misses when
reading their arguments from the passed struct.

I don't think iomap is as CPU sensitive as xarray (generally there's much
heavier work that happens in the filesystem) so I'd just strive for code
simplicity here. But I wanted to mention this so that it's clear that
pushing arguments to structs isn't free either.

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR