mm: write iovec cleanup

Hide some of the open-coded nr_segs tests into the iovec helpers. This is all to simplify generic_file_buffered_write, because that gets more complex in the next patch. Signed-off-by: Nick Piggin <npiggin@suse.de> Signed-off-by: Andrew Morton <akpm@linux-foundation.org> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>

mm: write iovec cleanup
Hide some of the open-coded nr_segs tests into the iovec helpers. This is all to simplify generic_file_buffered_write, because that gets more complex in the next patch. Signed-off-by: Nick Piggin <npiggin@suse.de> Signed-off-by: Andrew Morton <akpm@linux-foundation.org> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Nick Piggin · Linus Torvalds
1 parent eb2be18931
Showing 3 changed files with 67 additions and 86 deletions Side-by-side Diff
mm/filemap.c
mm/filemap.h
mm/filemap_xip.c
@@ -1823,12 +1823,7 @@
 	/*
 	 * handle partial DIO write.  Adjust cur_iov if needed.
 	 */
-	if (likely(nr_segs == 1))
-		buf = iov->iov_base + written;
-	else {
-		filemap_set_next_iovec(&cur_iov, &iov_offset, written);
-		buf = cur_iov->iov_base + iov_offset;
-	}
+	filemap_set_next_iovec(&cur_iov, nr_segs, &iov_offset, written);
  
 	do {
 		struct page *page;
@@ -1838,6 +1833,7 @@
 		size_t bytes;		/* Bytes to write to page */
 		size_t copied;		/* Bytes copied from user */
  
+		buf = cur_iov->iov_base + iov_offset;
 		offset = (pos & (PAGE_CACHE_SIZE - 1));
 		index = pos >> PAGE_CACHE_SHIFT;
 		bytes = PAGE_CACHE_SIZE - offset;
  
@@ -1869,13 +1865,10 @@
 		if (unlikely(status))
 			goto fs_write_aop_error;
  
-		if (likely(nr_segs == 1))
-			copied = filemap_copy_from_user(page, offset,
-							buf, bytes);
-		else
-			copied = filemap_copy_from_user_iovec(page, offset,
-						cur_iov, iov_offset, bytes);
+		copied = filemap_copy_from_user(page, offset,
+					cur_iov, nr_segs, iov_offset, bytes);
 		flush_dcache_page(page);
+
 		status = a_ops->commit_write(file, page, offset, offset+bytes);
 		if (unlikely(status < 0 || status == AOP_TRUNCATED_PAGE))
 			goto fs_write_aop_error;
@@ -1886,20 +1879,11 @@
 		if (unlikely(status > 0)) /* filesystem did partial write */
 			copied = status;
  
-		if (likely(copied > 0)) {
-			written += copied;
-			count -= copied;
-			pos += copied;
-			buf += copied;
-			if (unlikely(nr_segs > 1)) {
-				filemap_set_next_iovec(&cur_iov,
-						&iov_offset, copied);
-				if (count)
-					buf = cur_iov->iov_base + iov_offset;
-			} else {
-				iov_offset += copied;
-			}
-		}
+		written += copied;
+		count -= copied;
+		pos += copied;
+		filemap_set_next_iovec(&cur_iov, nr_segs, &iov_offset, copied);
+
 		unlock_page(page);
 		mark_page_accessed(page);
 		page_cache_release(page);
@@ -22,83 +22,83 @@
  
 /*
  * Copy as much as we can into the page and return the number of bytes which
- * were sucessfully copied.  If a fault is encountered then clear the page
- * out to (offset+bytes) and return the number of bytes which were copied.
- *
- * NOTE: For this to work reliably we really want copy_from_user_inatomic_nocache
- * to *NOT* zero any tail of the buffer that it failed to copy.  If it does,
- * and if the following non-atomic copy succeeds, then there is a small window
- * where the target page contains neither the data before the write, nor the
- * data after the write (it contains zero).  A read at this time will see
- * data that is inconsistent with any ordering of the read and the write.
- * (This has been detected in practice).
+ * were sucessfully copied.  If a fault is encountered then return the number of
+ * bytes which were copied.
  */
 static inline size_t
-filemap_copy_from_user(struct page *page, unsigned long offset,
-			const char __user *buf, unsigned bytes)
+filemap_copy_from_user_atomic(struct page *page, unsigned long offset,
+			const struct iovec *iov, unsigned long nr_segs,
+			size_t base, size_t bytes)
 {
 	char *kaddr;
-	int left;
+	size_t copied;
  
 	kaddr = kmap_atomic(page, KM_USER0);
-	left = __copy_from_user_inatomic_nocache(kaddr + offset, buf, bytes);
+	if (likely(nr_segs == 1)) {
+		int left;
+		char __user *buf = iov->iov_base + base;
+		left = __copy_from_user_inatomic_nocache(kaddr + offset,
+							buf, bytes);
+		copied = bytes - left;
+	} else {
+		copied = __filemap_copy_from_user_iovec_inatomic(kaddr + offset,
+							iov, base, bytes);
+	}
 	kunmap_atomic(kaddr, KM_USER0);
  
-	if (left != 0) {
-		/* Do it the slow way */
-		kaddr = kmap(page);
-		left = __copy_from_user_nocache(kaddr + offset, buf, bytes);
-		kunmap(page);
-	}
-	return bytes - left;
+	return copied;
 }
  
 /*
- * This has the same sideeffects and return value as filemap_copy_from_user().
- * The difference is that on a fault we need to memset the remainder of the
- * page (out to offset+bytes), to emulate filemap_copy_from_user()'s
- * single-segment behaviour.
+ * This has the same sideeffects and return value as
+ * filemap_copy_from_user_atomic().
+ * The difference is that it attempts to resolve faults.
  */
 static inline size_t
-filemap_copy_from_user_iovec(struct page *page, unsigned long offset,
-			const struct iovec *iov, size_t base, size_t bytes)
+filemap_copy_from_user(struct page *page, unsigned long offset,
+			const struct iovec *iov, unsigned long nr_segs,
+			 size_t base, size_t bytes)
 {
 	char *kaddr;
 	size_t copied;
  
-	kaddr = kmap_atomic(page, KM_USER0);
-	copied = __filemap_copy_from_user_iovec_inatomic(kaddr + offset, iov,
-							 base, bytes);
-	kunmap_atomic(kaddr, KM_USER0);
-	if (copied != bytes) {
-		kaddr = kmap(page);
-		copied = __filemap_copy_from_user_iovec_inatomic(kaddr + offset, iov,
-								 base, bytes);
-		if (bytes - copied)
-			memset(kaddr + offset + copied, 0, bytes - copied);
-		kunmap(page);
+	kaddr = kmap(page);
+	if (likely(nr_segs == 1)) {
+		int left;
+		char __user *buf = iov->iov_base + base;
+		left = __copy_from_user_nocache(kaddr + offset, buf, bytes);
+		copied = bytes - left;
+	} else {
+		copied = __filemap_copy_from_user_iovec_inatomic(kaddr + offset,
+							iov, base, bytes);
 	}
+	kunmap(page);
 	return copied;
 }
  
 static inline void
-filemap_set_next_iovec(const struct iovec **iovp, size_t *basep, size_t bytes)
+filemap_set_next_iovec(const struct iovec **iovp, unsigned long nr_segs,
+						 size_t *basep, size_t bytes)
 {
-	const struct iovec *iov = *iovp;
-	size_t base = *basep;
+	if (likely(nr_segs == 1)) {
+		*basep += bytes;
+	} else {
+		const struct iovec *iov = *iovp;
+		size_t base = *basep;
  
-	while (bytes) {
-		int copy = min(bytes, iov->iov_len - base);
+		while (bytes) {
+			int copy = min(bytes, iov->iov_len - base);
  
-		bytes -= copy;
-		base += copy;
-		if (iov->iov_len == base) {
-			iov++;
-			base = 0;
+			bytes -= copy;
+			base += copy;
+			if (iov->iov_len == base) {
+				iov++;
+				base = 0;
+			}
 		}
+		*iovp = iov;
+		*basep = base;
 	}
-	*iovp = iov;
-	*basep = base;
 }
 #endif
@@ -15,7 +15,6 @@
 #include <linux/rmap.h>
 #include <linux/sched.h>
 #include <asm/tlbflush.h>
-#include "filemap.h"
  
 /*
  * We do use our own empty page to avoid interference with other users
@@ -288,6 +287,7 @@
 		unsigned long index;
 		unsigned long offset;
 		size_t copied;
+		char *kaddr;
  
 		offset = (pos & (PAGE_CACHE_SIZE -1)); /* Within page */
 		index = pos >> PAGE_CACHE_SHIFT;
@@ -295,14 +295,6 @@
 		if (bytes > count)
 			bytes = count;
  
-		/*
-		 * Bring in the user page that we will copy from _first_.
-		 * Otherwise there's a nasty deadlock on copying from the
-		 * same page as we're writing to, without it being marked
-		 * up-to-date.
-		 */
-		fault_in_pages_readable(buf, bytes);
-
 		page = a_ops->get_xip_page(mapping,
 					   index*(PAGE_SIZE/512), 0);
 		if (IS_ERR(page) && (PTR_ERR(page) == -ENODATA)) {
  
@@ -319,8 +311,13 @@
 			break;
 		}
  
-		copied = filemap_copy_from_user(page, offset, buf, bytes);
+		fault_in_pages_readable(buf, bytes);
+		kaddr = kmap_atomic(page, KM_USER0);
+		copied = bytes -
+			__copy_from_user_inatomic_nocache(kaddr, buf, bytes);
+		kunmap_atomic(kaddr, KM_USER0);
 		flush_dcache_page(page);
+
 		if (likely(copied > 0)) {
 			status = copied;
...	...	@@ -1823,12 +1823,7 @@
1823	1823	/*
1824	1824	* handle partial DIO write. Adjust cur_iov if needed.
1825	1825	*/
1826		- if (likely(nr_segs == 1))
1827		- buf = iov->iov_base + written;
1828		- else {
1829		- filemap_set_next_iovec(&cur_iov, &iov_offset, written);
1830		- buf = cur_iov->iov_base + iov_offset;
1831		- }
	1826	+ filemap_set_next_iovec(&cur_iov, nr_segs, &iov_offset, written);
1832	1827
1833	1828	do {
1834	1829	struct page *page;
...	...	@@ -1838,6 +1833,7 @@
1838	1833	size_t bytes; /* Bytes to write to page */
1839	1834	size_t copied; /* Bytes copied from user */
1840	1835
	1836	+ buf = cur_iov->iov_base + iov_offset;
1841	1837	offset = (pos & (PAGE_CACHE_SIZE - 1));
1842	1838	index = pos >> PAGE_CACHE_SHIFT;
1843	1839	bytes = PAGE_CACHE_SIZE - offset;
1844	1840
...	...	@@ -1869,13 +1865,10 @@
1869	1865	if (unlikely(status))
1870	1866	goto fs_write_aop_error;
1871	1867
1872		- if (likely(nr_segs == 1))
1873		- copied = filemap_copy_from_user(page, offset,
1874		- buf, bytes);
1875		- else
1876		- copied = filemap_copy_from_user_iovec(page, offset,
1877		- cur_iov, iov_offset, bytes);
	1868	+ copied = filemap_copy_from_user(page, offset,
	1869	+ cur_iov, nr_segs, iov_offset, bytes);
1878	1870	flush_dcache_page(page);
	1871	+
1879	1872	status = a_ops->commit_write(file, page, offset, offset+bytes);
1880	1873	if (unlikely(status < 0 \|\| status == AOP_TRUNCATED_PAGE))
1881	1874	goto fs_write_aop_error;
...	...	@@ -1886,20 +1879,11 @@
1886	1879	if (unlikely(status > 0)) /* filesystem did partial write */
1887	1880	copied = status;
1888	1881
1889		- if (likely(copied > 0)) {
1890		- written += copied;
1891		- count -= copied;
1892		- pos += copied;
1893		- buf += copied;
1894		- if (unlikely(nr_segs > 1)) {
1895		- filemap_set_next_iovec(&cur_iov,
1896		- &iov_offset, copied);
1897		- if (count)
1898		- buf = cur_iov->iov_base + iov_offset;
1899		- } else {
1900		- iov_offset += copied;
1901		- }
1902		- }
	1882	+ written += copied;
	1883	+ count -= copied;
	1884	+ pos += copied;
	1885	+ filemap_set_next_iovec(&cur_iov, nr_segs, &iov_offset, copied);
	1886	+
1903	1887	unlock_page(page);
1904	1888	mark_page_accessed(page);
1905	1889	page_cache_release(page);
...	...	@@ -22,83 +22,83 @@
22	22
23	23	/*
24	24	* Copy as much as we can into the page and return the number of bytes which
25		- * were sucessfully copied. If a fault is encountered then clear the page
26		- * out to (offset+bytes) and return the number of bytes which were copied.
27		- *
28		- * NOTE: For this to work reliably we really want copy_from_user_inatomic_nocache
29		- * to NOT zero any tail of the buffer that it failed to copy. If it does,
30		- * and if the following non-atomic copy succeeds, then there is a small window
31		- * where the target page contains neither the data before the write, nor the
32		- * data after the write (it contains zero). A read at this time will see
33		- * data that is inconsistent with any ordering of the read and the write.
34		- * (This has been detected in practice).
	25	+ * were sucessfully copied. If a fault is encountered then return the number of
	26	+ * bytes which were copied.
35	27	*/
36	28	static inline size_t
37		-filemap_copy_from_user(struct page *page, unsigned long offset,
38		- const char __user *buf, unsigned bytes)
	29	+filemap_copy_from_user_atomic(struct page *page, unsigned long offset,
	30	+ const struct iovec *iov, unsigned long nr_segs,
	31	+ size_t base, size_t bytes)
39	32	{
40	33	char *kaddr;
41		- int left;
	34	+ size_t copied;
42	35
43	36	kaddr = kmap_atomic(page, KM_USER0);
44		- left = __copy_from_user_inatomic_nocache(kaddr + offset, buf, bytes);
	37	+ if (likely(nr_segs == 1)) {
	38	+ int left;
	39	+ char __user *buf = iov->iov_base + base;
	40	+ left = __copy_from_user_inatomic_nocache(kaddr + offset,
	41	+ buf, bytes);
	42	+ copied = bytes - left;
	43	+ } else {
	44	+ copied = __filemap_copy_from_user_iovec_inatomic(kaddr + offset,
	45	+ iov, base, bytes);
	46	+ }
45	47	kunmap_atomic(kaddr, KM_USER0);
46	48
47		- if (left != 0) {
48		- /* Do it the slow way */
49		- kaddr = kmap(page);
50		- left = __copy_from_user_nocache(kaddr + offset, buf, bytes);
51		- kunmap(page);
52		- }
53		- return bytes - left;
	49	+ return copied;
54	50	}
55	51
56	52	/*
57		- * This has the same sideeffects and return value as filemap_copy_from_user().
58		- * The difference is that on a fault we need to memset the remainder of the
59		- * page (out to offset+bytes), to emulate filemap_copy_from_user()'s
60		- * single-segment behaviour.
	53	+ * This has the same sideeffects and return value as
	54	+ * filemap_copy_from_user_atomic().
	55	+ * The difference is that it attempts to resolve faults.
61	56	*/
62	57	static inline size_t
63		-filemap_copy_from_user_iovec(struct page *page, unsigned long offset,
64		- const struct iovec *iov, size_t base, size_t bytes)
	58	+filemap_copy_from_user(struct page *page, unsigned long offset,
	59	+ const struct iovec *iov, unsigned long nr_segs,
	60	+ size_t base, size_t bytes)
65	61	{
66	62	char *kaddr;
67	63	size_t copied;
68	64
69		- kaddr = kmap_atomic(page, KM_USER0);
70		- copied = __filemap_copy_from_user_iovec_inatomic(kaddr + offset, iov,
71		- base, bytes);
72		- kunmap_atomic(kaddr, KM_USER0);
73		- if (copied != bytes) {
74		- kaddr = kmap(page);
75		- copied = __filemap_copy_from_user_iovec_inatomic(kaddr + offset, iov,
76		- base, bytes);
77		- if (bytes - copied)
78		- memset(kaddr + offset + copied, 0, bytes - copied);
79		- kunmap(page);
	65	+ kaddr = kmap(page);
	66	+ if (likely(nr_segs == 1)) {
	67	+ int left;
	68	+ char __user *buf = iov->iov_base + base;
	69	+ left = __copy_from_user_nocache(kaddr + offset, buf, bytes);
	70	+ copied = bytes - left;
	71	+ } else {
	72	+ copied = __filemap_copy_from_user_iovec_inatomic(kaddr + offset,
	73	+ iov, base, bytes);
80	74	}
	75	+ kunmap(page);
81	76	return copied;
82	77	}
83	78
84	79	static inline void
85		-filemap_set_next_iovec(const struct iovec *iovp, size_t basep, size_t bytes)
	80	+filemap_set_next_iovec(const struct iovec **iovp, unsigned long nr_segs,
	81	+ size_t *basep, size_t bytes)
86	82	{
87		- const struct iovec iov = iovp;
88		- size_t base = *basep;
	83	+ if (likely(nr_segs == 1)) {
	84	+ *basep += bytes;
	85	+ } else {
	86	+ const struct iovec iov = iovp;
	87	+ size_t base = *basep;
89	88
90		- while (bytes) {
91		- int copy = min(bytes, iov->iov_len - base);
	89	+ while (bytes) {
	90	+ int copy = min(bytes, iov->iov_len - base);
92	91
93		- bytes -= copy;
94		- base += copy;
95		- if (iov->iov_len == base) {
96		- iov++;
97		- base = 0;
	92	+ bytes -= copy;
	93	+ base += copy;
	94	+ if (iov->iov_len == base) {
	95	+ iov++;
	96	+ base = 0;
	97	+ }
98	98	}
	99	+ *iovp = iov;
	100	+ *basep = base;
99	101	}
100		- *iovp = iov;
101		- *basep = base;
102	102	}
103	103	#endif
...	...	@@ -15,7 +15,6 @@
15	15	#include <linux/rmap.h>
16	16	#include <linux/sched.h>
17	17	#include <asm/tlbflush.h>
18		-#include "filemap.h"
19	18
20	19	/*
21	20	* We do use our own empty page to avoid interference with other users
...	...	@@ -288,6 +287,7 @@
288	287	unsigned long index;
289	288	unsigned long offset;
290	289	size_t copied;
	290	+ char *kaddr;
291	291
292	292	offset = (pos & (PAGE_CACHE_SIZE -1)); /* Within page */
293	293	index = pos >> PAGE_CACHE_SHIFT;
...	...	@@ -295,14 +295,6 @@
295	295	if (bytes > count)
296	296	bytes = count;
297	297
298		- /*
299		- * Bring in the user page that we will copy from _first_.
300		- * Otherwise there's a nasty deadlock on copying from the
301		- * same page as we're writing to, without it being marked
302		- * up-to-date.
303		- */
304		- fault_in_pages_readable(buf, bytes);
305		-
306	298	page = a_ops->get_xip_page(mapping,
307	299	index*(PAGE_SIZE/512), 0);
308	300	if (IS_ERR(page) && (PTR_ERR(page) == -ENODATA)) {
309	301
...	...	@@ -319,8 +311,13 @@
319	311	break;
320	312	}
321	313
322		- copied = filemap_copy_from_user(page, offset, buf, bytes);
	314	+ fault_in_pages_readable(buf, bytes);
	315	+ kaddr = kmap_atomic(page, KM_USER0);
	316	+ copied = bytes -
	317	+ __copy_from_user_inatomic_nocache(kaddr, buf, bytes);
	318	+ kunmap_atomic(kaddr, KM_USER0);
323	319	flush_dcache_page(page);
	320	+
324	321	if (likely(copied > 0)) {
325	322	status = copied;
326	323