Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1448785 > unrolled thread

[PATCH 0/2] mm patches

Back to article view | Back to linux.kernel

Contents

  [PATCH 0/2] mm patches Mikulas Patocka <mpatocka@redhat.com> - 2016-07-22 23:20 +0200
    [PATCH 2/2] mm: optimize copy_page_to/from_iter_iovec Mikulas Patocka <mpatocka@redhat.com> - 2016-07-22 23:20 +0200
      Re: [PATCH 2/2] mm: optimize copy_page_to/from_iter_iovec Andi Kleen <andi@firstfloor.org> - 2016-07-23 23:00 +0200
    [PATCH 1/2] mm: add cond_resched to generic_swapfile_activate Mikulas Patocka <mpatocka@redhat.com> - 2016-07-22 23:20 +0200
      Re: [PATCH 1/2] mm: add cond_resched to generic_swapfile_activate Michal Hocko <mhocko@kernel.org> - 2016-07-25 09:30 +0200

#1448785 — [PATCH 0/2] mm patches

Hi

I'm submitting these two patches for the next merge window.

The first patch adds cond_resched() to generic_swapfile_activate to avoid 
stall when activating unfragmented swapfile.

The second patch removes useless code from copy_page_to_iter_iovec and 
copy_page_from_iter_iovec.

Mikulas

[toc] | [next] | [standalone]

#1448789 — [PATCH 2/2] mm: optimize copy_page_to/from_iter_iovec

The functions copy_page_to_iter_iovec and copy_page_from_iter_iovec copy some
data to userspace or from userspace. These functions have a fast path where they
map a page using kmap_atomic and a slow path where they use kmap.

kmap is slower than kmap_atomic, so the fast path is preferred.

However, on kernels without highmem support, kmap just calls page_address, so
there is no need to avoid kmap. On kernels without highmem support, the fast
path just increases code size (and cache footprint) and it doesn't improve
copy performance in any way.

This patch enables the fast path only if CONFIG_HIGHMEM is defined.

Code size reduced by this patch:
x86 (without highmem)	928
x86-64			960
sparc64			848
alpha			1136
pa-risc			1200

Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>

---
 lib/iov_iter.c |   12 ++++++++++++
 1 file changed, 12 insertions(+)

Index: linux-4.7-rc7/lib/iov_iter.c
===================================================================
--- linux-4.7-rc7.orig/lib/iov_iter.c	2016-05-30 17:34:37.000000000 +0200
+++ linux-4.7-rc7/lib/iov_iter.c	2016-07-11 17:14:03.000000000 +0200
@@ -159,6 +159,7 @@ static size_t copy_page_to_iter_iovec(st
 	buf = iov->iov_base + skip;
 	copy = min(bytes, iov->iov_len - skip);
 
+#ifdef CONFIG_HIGHMEM
 	if (!fault_in_pages_writeable(buf, copy)) {
 		kaddr = kmap_atomic(page);
 		from = kaddr + offset;
@@ -190,6 +191,8 @@ static size_t copy_page_to_iter_iovec(st
 		copy = min(bytes, iov->iov_len - skip);
 	}
 	/* Too bad - revert to non-atomic kmap */
+#endif
+
 	kaddr = kmap(page);
 	from = kaddr + offset;
 	left = __copy_to_user(buf, from, copy);
@@ -208,7 +211,10 @@ static size_t copy_page_to_iter_iovec(st
 		bytes -= copy;
 	}
 	kunmap(page);
+
+#ifdef CONFIG_HIGHMEM
 done:
+#endif
 	if (skip == iov->iov_len) {
 		iov++;
 		skip = 0;
@@ -240,6 +246,7 @@ static size_t copy_page_from_iter_iovec(
 	buf = iov->iov_base + skip;
 	copy = min(bytes, iov->iov_len - skip);
 
+#ifdef CONFIG_HIGHMEM
 	if (!fault_in_pages_readable(buf, copy)) {
 		kaddr = kmap_atomic(page);
 		to = kaddr + offset;
@@ -271,6 +278,8 @@ static size_t copy_page_from_iter_iovec(
 		copy = min(bytes, iov->iov_len - skip);
 	}
 	/* Too bad - revert to non-atomic kmap */
+#endif
+
 	kaddr = kmap(page);
 	to = kaddr + offset;
 	left = __copy_from_user(to, buf, copy);
@@ -289,7 +298,10 @@ static size_t copy_page_from_iter_iovec(
 		bytes -= copy;
 	}
 	kunmap(page);
+
+#ifdef CONFIG_HIGHMEM
 done:
+#endif
 	if (skip == iov->iov_len) {
 		iov++;
 		skip = 0;

[toc] | [prev] | [next] | [standalone]

#1449001 — Re: [PATCH 2/2] mm: optimize copy_page_to/from_iter_iovec

Mikulas Patocka <mpatocka@redhat.com> writes:
>  	copy = min(bytes, iov->iov_len - skip);
>  
> +#ifdef CONFIG_HIGHMEM
>  	if (!fault_in_pages_writeable(buf, copy)) {

If you use IS_ENABLED in the if here ...

>  	kunmap(page);
> +
> +#ifdef CONFIG_HIGHMEM
>  done:
> +#endif

... you don't need this ifdef.

-Andi


-- 
ak@linux.intel.com -- Speaking for myself only

[toc] | [prev] | [next] | [standalone]

#1448791 — [PATCH 1/2] mm: add cond_resched to generic_swapfile_activate

The function generic_swapfile_activate can take quite long time, it iterates
over all blocks of a file, so add cond_resched to it. I observed about 1 second
stalls when activating a swapfile that was almost unfragmented - this patch
fixes it.

Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>

---
 mm/page_io.c |    2 ++
 1 file changed, 2 insertions(+)

Index: linux-4.7-rc7/mm/page_io.c
===================================================================
--- linux-4.7-rc7.orig/mm/page_io.c	2016-05-30 17:34:37.000000000 +0200
+++ linux-4.7-rc7/mm/page_io.c	2016-07-11 17:23:33.000000000 +0200
@@ -166,6 +166,8 @@ int generic_swapfile_activate(struct swa
 		unsigned block_in_page;
 		sector_t first_block;
 
+		cond_resched();
+
 		first_block = bmap(inode, probe_block);
 		if (first_block == 0)
 			goto bad_bmap;

[toc] | [prev] | [next] | [standalone]

#1449322 — Re: [PATCH 1/2] mm: add cond_resched to generic_swapfile_activate

On Fri 22-07-16 17:11:20, Mikulas Patocka wrote:
> The function generic_swapfile_activate can take quite long time, it iterates
> over all blocks of a file, so add cond_resched to it. I observed about 1 second
> stalls when activating a swapfile that was almost unfragmented - this patch
> fixes it.
> 
> Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>

Acked-by: Michal Hocko <mhocko@suse.com>

> 
> ---
>  mm/page_io.c |    2 ++
>  1 file changed, 2 insertions(+)
> 
> Index: linux-4.7-rc7/mm/page_io.c
> ===================================================================
> --- linux-4.7-rc7.orig/mm/page_io.c	2016-05-30 17:34:37.000000000 +0200
> +++ linux-4.7-rc7/mm/page_io.c	2016-07-11 17:23:33.000000000 +0200
> @@ -166,6 +166,8 @@ int generic_swapfile_activate(struct swa
>  		unsigned block_in_page;
>  		sector_t first_block;
>  
> +		cond_resched();
> +
>  		first_block = bmap(inode, probe_block);
>  		if (first_block == 0)
>  			goto bad_bmap;

-- 
Michal Hocko
SUSE Labs

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web