All of lore.kernel.org
 help / color / mirror / Atom feed
From: Matthew Wilcox <willy@infradead.org>
To: David Howells <dhowells@redhat.com>
Cc: Alexander Viro <viro@zeniv.linux.org.uk>,
	Andrew Morton <akpm@linux-foundation.org>,
	linux-fsdevel@vger.kernel.org, linux-mm@kvack.org,
	linux-kernel@vger.kernel.org
Subject: Re: [RFC][PATCH] iov_iter: Add ITER_MAPPING
Date: Wed, 22 Jan 2020 11:33:06 -0800	[thread overview]
Message-ID: <20200122193306.GB4675@bombadil.infradead.org> (raw)
In-Reply-To: <3577430.1579705075@warthog.procyon.org.uk>

On Wed, Jan 22, 2020 at 02:57:55PM +0000, David Howells wrote:
> An alternative could be to have an "ITER_ARRAY" that just has the page
> pointers and not the offset/length info.  This decreases the redundancy and
> increases the max payload-per-array-page to 2M.

We could also have an ITER_XARRAY which you just pass &mapping->i_pages
to.  I don't think you use any other part of the mapping, so that would
be a more generic version that is equally efficient.

> +	rcu_read_lock();						\
> +	for (page = xas_load(&xas); page; page = xas_next(&xas)) {	\
> +		if (xas_retry(&xas, page))				\
> +			continue;					\
> +		if (xa_is_value(page))					\
> +			break;						\

Do you also want to check for !page?  That would be a bug in the caller.

> +		if (PageCompound(page))					\
> +			break;						\

It's perfectly legal to have compound pages in the page cache.  Call
find_subpage(page, xas.xa_index) unconditionally.

> +		if (page_to_pgoff(page) != xas.xa_index)		\
> +			break;						\

... and you can ditch this if the pages are pinned as find_subpage()
will bug in this case.

> +		__v.bv_page = page;					\
> +		offset = (i->mapping_start + skip) & ~PAGE_MASK;	\
> +		seg = PAGE_SIZE - offset;			\
> +		__v.bv_offset = offset;				\
> +		__v.bv_len = min(n, seg);			\
> +		(void)(STEP);					\
> +		n -= __v.bv_len;				\
> +		skip += __v.bv_len;				\

Do we want STEP to be called with PAGE_SIZE chunks, or if they have a
THP, can we have it called with larger than a PAGE_SIZE chunk?

> +#define iterate_all_kinds(i, n, v, I, B, K, M) {		\
>  	if (likely(n)) {					\
>  		size_t skip = i->iov_offset;			\
>  		if (unlikely(i->type & ITER_BVEC)) {		\
> @@ -86,6 +119,9 @@
>  			struct kvec v;				\
>  			iterate_kvec(i, n, v, kvec, skip, (K))	\
>  		} else if (unlikely(i->type & ITER_DISCARD)) {	\
> +		} else if (unlikely(i->type & ITER_MAPPING)) {	\
> +			struct bio_vec v;			\
> +			iterate_mapping(i, n, v, skip, (M));	\

bio_vec?

> -#define iterate_and_advance(i, n, v, I, B, K) {			\
> +#define iterate_and_advance(i, n, v, I, B, K, M) {		\
>  	if (unlikely(i->count < n))				\
>  		n = i->count;					\
>  	if (i->count) {						\
> @@ -119,6 +155,9 @@
>  			i->kvec = kvec;				\
>  		} else if (unlikely(i->type & ITER_DISCARD)) {	\
>  			skip += n;				\
> +		} else if (unlikely(i->type & ITER_MAPPING)) {	\
> +			struct bio_vec v;			\
> +			iterate_mapping(i, n, v, skip, (M))	\

again?


  reply	other threads:[~2020-01-22 19:33 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-01-22 14:57 [RFC][PATCH] iov_iter: Add ITER_MAPPING David Howells
2020-01-22 19:33 ` Matthew Wilcox [this message]
2020-01-22 21:42 ` David Howells
2020-01-23 11:04 ` David Howells
2020-01-24 11:21   ` Matthew Wilcox
2020-01-24 14:58   ` 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=20200122193306.GB4675@bombadil.infradead.org \
    --to=willy@infradead.org \
    --cc=akpm@linux-foundation.org \
    --cc=dhowells@redhat.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.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 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.