All of lore.kernel.org
 help / color / mirror / Atom feed
From: David Laight <David.Laight@ACULAB.COM>
To: 'David Howells' <dhowells@redhat.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>,
	Alexander Viro <viro@zeniv.linux.org.uk>,
	Jens Axboe <axboe@kernel.dk>, Christoph Hellwig <hch@lst.de>,
	Christian Brauner <christian@brauner.io>,
	Matthew Wilcox <willy@infradead.org>,
	"jlayton@kernel.org" <jlayton@kernel.org>,
	"linux-block@vger.kernel.org" <linux-block@vger.kernel.org>,
	"linux-fsdevel@vger.kernel.org" <linux-fsdevel@vger.kernel.org>,
	"linux-mm@kvack.org" <linux-mm@kvack.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: RE: [RFC PATCH v2] iov_iter: Convert iterate*() to inline funcs
Date: Wed, 16 Aug 2023 10:17:23 +0000	[thread overview]
Message-ID: <a72036d57d50464ea4fe7fa556ee1a72@AcuMS.aculab.com> (raw)
In-Reply-To: <440141.1692179410@warthog.procyon.org.uk>

From: David Howells
> Sent: Wednesday, August 16, 2023 10:50 AM
> 
> David Laight <David.Laight@ACULAB.COM> wrote:
> 
> > It is harder to compare because of some of the random name changes.
> 
> I wouldn't say 'random' exactly, but if you prefer, some of the name changing
> can be split out into a separate patch.  The macros are kind of the worst
> since they picked up variable names from the callers.
> 
> > The version of the source I found seems to pass priv2 to functions
> > that don't use it?
> 
> That can't be avoided if I convert everything to inline functions and function
> pointers - but the optimiser can get rid of it where it can inline the step
> function.

AFAICT the IOVEC one was only called directly.

> I tried passing the iterator to the step functions instead, but that just made
> things bigger.  memcpy_from_iter_mc() is interesting to deal with.  I would
> prefer to deal with it in the caller so we only do the check once, but that
> might mean duplicating the caller.

You could try something slightly horrid that the compiler
might optimise for you.
Instead of passing in a function pointer pass a number.
Then do something like:
#define call_iter(id, ...) \
	(id == x ? fn_x(__VA_ARGS__) : id == y ? fn_y(__VA_ARGS) ...)
constant folding on the inline should kill the function pointer.
You might get away with putting the args on the end.

...
> > I rather hope the should_fail_usercopy() and instrument_copy_xxx()
> > calls are usually either absent or, at most, nops.
> 
> Okay - it's probably worth marking those too, then.

Thinking I'm sure they are KASAN annotations.
The are few enough calls that I suspect that replicating them
won't affect KASAN (etc) builds.

> > This all seems to have a lot fewer options than last time I looked.
> 
> I'm not sure what you mean by 'a lot fewer options'?

It might just be ITER_PIPE that has gone.

> > Is it worth optimising the KVEC case with a single buffer?
> 
> You mean an equivalent of UBUF?  Maybe.  There are probably a whole bunch of
> netfs places that do single-kvec writes, though I'm trying to convert these
> over to bvec arrays, combining them with their data, and MSG_SPLICE_PAGES.

I'm thinking of what happens with kernel callers of things
like the socket code - especially for address/option buffers.
Probably io_uring and bpf (and my out of tree drivers!).

Could be the equivalent of UBUF, but checking for KVEC with
a count of 1 wouldn't really add any more cmp/jmp pairs.

I've also noticed in the past that some of this code seems
to be optimised for zero length buffers/fragments.
Surely they just need to work?

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)


  reply	other threads:[~2023-08-16 10:18 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-08-14 21:09 [RFC PATCH v2] iov_iter: Convert iterate*() to inline funcs David Howells
2023-08-14 21:40 ` David Howells
2023-08-16  8:30   ` David Laight
2023-08-16  9:50     ` David Howells
2023-08-16 10:17       ` David Laight [this message]
2023-08-16 11:19         ` David Howells

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=a72036d57d50464ea4fe7fa556ee1a72@AcuMS.aculab.com \
    --to=david.laight@aculab.com \
    --cc=axboe@kernel.dk \
    --cc=christian@brauner.io \
    --cc=dhowells@redhat.com \
    --cc=hch@lst.de \
    --cc=jlayton@kernel.org \
    --cc=linux-block@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=torvalds@linux-foundation.org \
    --cc=viro@zeniv.linux.org.uk \
    --cc=willy@infradead.org \
    /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.