All of lore.kernel.org
 help / color / mirror / Atom feed
From: David Howells <dhowells@redhat.com>
To: Pavel Begunkov <asml.silence@gmail.com>,
	Matthew Wilcox <willy@infradead.org>,
	Jens Axboe <axboe@kernel.dk>,
	Alexander Viro <viro@zeniv.linux.org.uk>
Cc: dhowells@redhat.com,
	Linus Torvalds <torvalds@linux-foundation.org>,
	linux-fsdevel@vger.kernel.org, linux-block@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: [PATCH 00/29] RFC: iov_iter: Switch to using an ops table
Date: Sat, 21 Nov 2020 14:13:21 +0000	[thread overview]
Message-ID: <160596800145.154728.7192318545120181269.stgit@warthog.procyon.org.uk> (raw)


Hi Pavel, Willy, Jens, Al,

I had a go switching the iov_iter stuff away from using a type bitmask to
using an ops table to get rid of the if-if-if-if chains that are all over
the place.  After I pushed it, someone pointed me at Pavel's two patches.

I have another iterator class that I want to add - which would lengthen the
if-if-if-if chains.  A lot of the time, there's a conditional clause at the
beginning of a function that just jumps off to a type-specific handler or
to reject the operation for that type.  An ops table can just point to that
instead.

As far as I can tell, there's no difference in performance in most cases,
though doing AFS-based kernel compiles appears to take less time (down from
3m20 to 2m50), which might make sense as that uses iterators a lot - but
there are too many variables in that for that to be a good benchmark (I'm
dealing with a remote server, for a start).

Can someone recommend a good way to benchmark this properly?  The problem
is that the difference this makes relative to the amount of time taken to
actually do I/O is tiny.

I've tried TCP transfers using the following sink program:

	#include <stdio.h>
	#include <stdlib.h>
	#include <string.h>
	#include <fcntl.h>
	#include <unistd.h>
	#include <netinet/in.h>
	#define OSERROR(X, Y) do { if ((long)(X) == -1) { perror(Y); exit(1); } } while(0)
	static unsigned char buffer[512 * 1024] __attribute__((aligned(4096)));
	int main(int argc, char *argv[])
	{
		struct sockaddr_in sin = { .sin_family = AF_INET, .sin_port = htons(5555) };
		int sfd, afd;
		sfd = socket(AF_INET, SOCK_STREAM, 0);
		OSERROR(sfd, "socket");
		OSERROR(bind(sfd, (struct sockaddr *)&sin, sizeof(sin)), "bind");
		OSERROR(listen(sfd, 1), "listen");
		for (;;) {
			afd = accept(sfd, NULL, NULL);
			if (afd != -1) {
				while (read(afd, buffer, sizeof(buffer)) > 0) {}
				close(afd);
			}
		}
	}

and send program:

	#include <stdio.h>
	#include <stdlib.h>
	#include <string.h>
	#include <fcntl.h>
	#include <unistd.h>
	#include <netdb.h>
	#include <netinet/in.h>
	#include <sys/stat.h>
	#include <sys/sendfile.h>
	#define OSERROR(X, Y) do { if ((long)(X) == -1) { perror(Y); exit(1); } } while(0)
	static unsigned char buffer[512*1024] __attribute__((aligned(4096)));
	int main(int argc, char *argv[])
	{
		struct sockaddr_in sin = { .sin_family = AF_INET, .sin_port = htons(5555) };
		struct hostent *h;
		ssize_t size, r, o;
		int cfd;
		if (argc != 3) {
			fprintf(stderr, "tcp-gen <server> <size>\n");
			exit(2);
		}
		size = strtoul(argv[2], NULL, 0);
		if (size <= 0) {
			fprintf(stderr, "Bad size\n");
			exit(2);
		}
		h = gethostbyname(argv[1]);
		if (!h) {
			fprintf(stderr, "%s: %s\n", argv[1], hstrerror(h_errno));
			exit(3);
		}
		if (!h->h_addr_list[0]) {
			fprintf(stderr, "%s: No addresses\n", argv[1]);
			exit(3);
		}
		memcpy(&sin.sin_addr, h->h_addr_list[0], h->h_length);
		cfd = socket(AF_INET, SOCK_STREAM, 0);
		OSERROR(cfd, "socket");
		OSERROR(connect(cfd, (struct sockaddr *)&sin, sizeof(sin)), "connect");
		do {
			r = size > sizeof(buffer) ? sizeof(buffer) : size;
			size -= r;
			o = 0;
			do {
				ssize_t w = write(cfd, buffer + o, r - o);
				OSERROR(w, "write");
				o += w;
			} while (o < r);
		} while (size > 0);
		OSERROR(close(cfd), "close/c");
		return 0;
	}

since the socket interface uses iterators.  It seems to show no difference.
One side note, though: I've been doing 10GiB same-machine transfers, and it
takes either ~2.5s or ~0.87s and rarely in between, with or without these
patches, alternating apparently randomly between the two times.

The patches can be found here:

	https://git.kernel.org/pub/scm/linux/kernel/git/dhowells/linux-fs.git/log/?h=iov-ops

David
---
David Howells (29):
      iov_iter: Switch to using a table of operations
      iov_iter: Split copy_page_to_iter()
      iov_iter: Split iov_iter_fault_in_readable
      iov_iter: Split the iterate_and_advance() macro
      iov_iter: Split copy_to_iter()
      iov_iter: Split copy_mc_to_iter()
      iov_iter: Split copy_from_iter()
      iov_iter: Split the iterate_all_kinds() macro
      iov_iter: Split copy_from_iter_full()
      iov_iter: Split copy_from_iter_nocache()
      iov_iter: Split copy_from_iter_flushcache()
      iov_iter: Split copy_from_iter_full_nocache()
      iov_iter: Split copy_page_from_iter()
      iov_iter: Split iov_iter_zero()
      iov_iter: Split copy_from_user_atomic()
      iov_iter: Split iov_iter_advance()
      iov_iter: Split iov_iter_revert()
      iov_iter: Split iov_iter_single_seg_count()
      iov_iter: Split iov_iter_alignment()
      iov_iter: Split iov_iter_gap_alignment()
      iov_iter: Split iov_iter_get_pages()
      iov_iter: Split iov_iter_get_pages_alloc()
      iov_iter: Split csum_and_copy_from_iter()
      iov_iter: Split csum_and_copy_from_iter_full()
      iov_iter: Split csum_and_copy_to_iter()
      iov_iter: Split iov_iter_npages()
      iov_iter: Split dup_iter()
      iov_iter: Split iov_iter_for_each_range()
      iov_iter: Remove iterate_all_kinds() and iterate_and_advance()


 lib/iov_iter.c | 1440 +++++++++++++++++++++++++++++++-----------------
 1 file changed, 934 insertions(+), 506 deletions(-)



             reply	other threads:[~2020-11-21 14:13 UTC|newest]

Thread overview: 63+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-11-21 14:13 David Howells [this message]
2020-11-21 14:13 ` [PATCH 01/29] iov_iter: Switch to using a table of operations David Howells
2020-11-21 14:31   ` Pavel Begunkov
2020-11-23 23:21     ` Pavel Begunkov
2020-11-21 18:21   ` Linus Torvalds
2020-12-11  1:30     ` Al Viro
2020-11-22 13:33   ` David Howells
2020-11-22 13:58     ` David Laight
2020-11-22 19:22     ` Linus Torvalds
2020-11-22 22:34       ` David Laight
2020-11-22 22:46   ` David Laight
2020-11-23  8:05   ` Christoph Hellwig
2020-11-23 10:31   ` David Howells
2020-11-23 23:42     ` Pavel Begunkov
2020-11-24 12:50     ` David Howells
2020-11-24 15:30       ` Jens Axboe
2020-11-27 17:14       ` David Howells
2020-11-23 11:14   ` David Howells
2020-12-03  6:45   ` [iov_iter] 9bd0e337c6: will-it-scale.per_process_ops -4.8% regression kernel test robot
2020-12-03  6:45     ` kernel test robot
2020-12-03 17:47     ` Linus Torvalds
2020-12-03 17:47       ` Linus Torvalds
2020-12-03 17:50       ` Jens Axboe
2020-12-03 17:50         ` Jens Axboe
2020-12-04 11:50     ` David Howells
2020-12-04 11:50       ` David Howells
2020-12-04 11:51     ` David Howells
2020-12-04 11:51       ` David Howells
2020-12-07 13:10       ` Oliver Sang
2020-12-07 13:10         ` Oliver Sang
2020-12-07 13:20       ` David Howells
2020-12-07 13:20         ` David Howells
2020-11-21 14:13 ` [PATCH 02/29] iov_iter: Split copy_page_to_iter() David Howells
2020-11-21 14:13 ` [PATCH 03/29] iov_iter: Split iov_iter_fault_in_readable David Howells
2020-11-21 14:13 ` [PATCH 04/29] iov_iter: Split the iterate_and_advance() macro David Howells
2020-11-21 14:14 ` [PATCH 05/29] iov_iter: Split copy_to_iter() David Howells
2020-11-21 14:14 ` [PATCH 06/29] iov_iter: Split copy_mc_to_iter() David Howells
2020-11-21 14:14 ` [PATCH 07/29] iov_iter: Split copy_from_iter() David Howells
2020-11-21 14:14 ` [PATCH 08/29] iov_iter: Split the iterate_all_kinds() macro David Howells
2020-11-21 14:14 ` [PATCH 09/29] iov_iter: Split copy_from_iter_full() David Howells
2020-11-21 14:14 ` [PATCH 10/29] iov_iter: Split copy_from_iter_nocache() David Howells
2020-11-21 14:14 ` [PATCH 11/29] iov_iter: Split copy_from_iter_flushcache() David Howells
2020-11-21 14:14 ` [PATCH 12/29] iov_iter: Split copy_from_iter_full_nocache() David Howells
2020-11-21 14:15 ` [PATCH 13/29] iov_iter: Split copy_page_from_iter() David Howells
2020-11-21 14:15 ` [PATCH 14/29] iov_iter: Split iov_iter_zero() David Howells
2020-11-21 14:15 ` [PATCH 15/29] iov_iter: Split copy_from_user_atomic() David Howells
2020-11-21 14:15 ` [PATCH 16/29] iov_iter: Split iov_iter_advance() David Howells
2020-11-21 14:15 ` [PATCH 17/29] iov_iter: Split iov_iter_revert() David Howells
2020-11-21 14:15 ` [PATCH 18/29] iov_iter: Split iov_iter_single_seg_count() David Howells
2020-11-21 14:15 ` [PATCH 19/29] iov_iter: Split iov_iter_alignment() David Howells
2020-11-21 14:15 ` [PATCH 20/29] iov_iter: Split iov_iter_gap_alignment() David Howells
2020-11-21 14:16 ` [PATCH 21/29] iov_iter: Split iov_iter_get_pages() David Howells
2020-11-21 14:16 ` [PATCH 22/29] iov_iter: Split iov_iter_get_pages_alloc() David Howells
2020-11-21 14:16 ` [PATCH 23/29] iov_iter: Split csum_and_copy_from_iter() David Howells
2020-11-21 14:16 ` [PATCH 24/29] iov_iter: Split csum_and_copy_from_iter_full() David Howells
2020-11-21 14:16 ` [PATCH 25/29] iov_iter: Split csum_and_copy_to_iter() David Howells
2020-11-21 14:16 ` [PATCH 26/29] iov_iter: Split iov_iter_npages() David Howells
2020-11-21 14:16 ` [PATCH 27/29] iov_iter: Split dup_iter() David Howells
2020-11-21 14:17 ` [PATCH 28/29] iov_iter: Split iov_iter_for_each_range() David Howells
2020-11-21 14:17 ` [PATCH 29/29] iov_iter: Remove iterate_all_kinds() and iterate_and_advance() David Howells
2020-11-21 14:34 ` [PATCH 00/29] RFC: iov_iter: Switch to using an ops table Pavel Begunkov
2020-11-21 18:23 ` Linus Torvalds
2020-12-11  3:24 ` Matthew Wilcox

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=160596800145.154728.7192318545120181269.stgit@warthog.procyon.org.uk \
    --to=dhowells@redhat.com \
    --cc=asml.silence@gmail.com \
    --cc=axboe@kernel.dk \
    --cc=linux-block@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.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.