public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/3] more /dev/[k]mem cleanups
@ 2009-09-12 15:20 Wu Fengguang
  2009-09-12 15:20 ` [PATCH 1/3] devmem: make size_inside_page() logic straight Wu Fengguang
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Wu Fengguang @ 2009-09-12 15:20 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Andi Kleen, Avi Kivity, Greg Kroah-Hartman, Johannes Berg,
	Marcelo Tosatti, Mark Brown, Wu Fengguang, LKML

Andrew,

Here are three more devmem cleanup patches following your comments.

Thanks,
Fengguang
-- 


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 1/3] devmem: make size_inside_page() logic straight
  2009-09-12 15:20 [PATCH 0/3] more /dev/[k]mem cleanups Wu Fengguang
@ 2009-09-12 15:20 ` Wu Fengguang
  2009-09-12 15:20 ` [PATCH 2/3] devmem: remove the "written" variable in write_kmem() Wu Fengguang
  2009-09-12 15:20 ` [PATCH 3/3] devmem: remove redundant parameter from do_write_kmem() Wu Fengguang
  2 siblings, 0 replies; 6+ messages in thread
From: Wu Fengguang @ 2009-09-12 15:20 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Andi Kleen, Avi Kivity, Greg Kroah-Hartman, Johannes Berg,
	Marcelo Tosatti, Mark Brown, Wu Fengguang, LKML

[-- Attachment #1: kmem-size_inside_page.patch --]
[-- Type: text/plain, Size: 2553 bytes --]

Also convert more size_inside_page() users.

Cc: Andi Kleen <ak@linux.intel.com>
Cc: Avi Kivity <avi@qumranet.com>
Cc: Greg Kroah-Hartman <gregkh@suse.de>
Cc: Johannes Berg <johannes@sipsolutions.net>
Cc: Marcelo Tosatti <mtosatti@redhat.com>
Cc: Mark Brown <broonie@opensource.wolfsonmicro.com>
Signed-off-by: Wu Fengguang <fengguang.wu@intel.com>
---
 drivers/char/mem.c |   34 ++++++++++++----------------------
 1 file changed, 12 insertions(+), 22 deletions(-)

--- linux-mm.orig/drivers/char/mem.c	2009-09-12 23:01:24.000000000 +0800
+++ linux-mm/drivers/char/mem.c	2009-09-12 23:01:25.000000000 +0800
@@ -40,12 +40,9 @@ static inline unsigned long size_inside_
 {
 	unsigned long sz;
 
-	if (-start & (PAGE_SIZE - 1))
-		sz = -start & (PAGE_SIZE - 1);
-	else
-		sz = PAGE_SIZE;
+	sz = PAGE_SIZE - (start & (PAGE_SIZE - 1));
 
-	return min_t(unsigned long, sz, size);
+	return min(sz, size);
 }
 
 /*
@@ -137,9 +134,7 @@ static ssize_t read_mem(struct file * fi
 #ifdef __ARCH_HAS_NO_PAGE_ZERO_MAPPED
 	/* we don't have page 0 mapped on sparc and m68k.. */
 	if (p < PAGE_SIZE) {
-		sz = PAGE_SIZE - p;
-		if (sz > count) 
-			sz = count; 
+		sz = size_inside_page(p, count);
 		if (sz > 0) {
 			if (clear_user(buf, sz))
 				return -EFAULT;
@@ -202,9 +197,7 @@ static ssize_t write_mem(struct file * f
 #ifdef __ARCH_HAS_NO_PAGE_ZERO_MAPPED
 	/* we don't have page 0 mapped on sparc and m68k.. */
 	if (p < PAGE_SIZE) {
-		unsigned long sz = PAGE_SIZE - p;
-		if (sz > count)
-			sz = count;
+		sz = size_inside_page(p, count);
 		/* Hmm. Do something? */
 		buf += sz;
 		p += sz;
@@ -416,15 +409,14 @@ static ssize_t read_kmem(struct file *fi
 #ifdef __ARCH_HAS_NO_PAGE_ZERO_MAPPED
 		/* we don't have page 0 mapped on sparc and m68k.. */
 		if (p < PAGE_SIZE && low_count > 0) {
-			size_t tmp = PAGE_SIZE - p;
-			if (tmp > low_count) tmp = low_count;
-			if (clear_user(buf, tmp))
+			sz = size_inside_page(p, low_count);
+			if (clear_user(buf, sz))
 				return -EFAULT;
-			buf += tmp;
-			p += tmp;
-			read += tmp;
-			low_count -= tmp;
-			count -= tmp;
+			buf += sz;
+			p += sz;
+			read += sz;
+			low_count -= sz;
+			count -= sz;
 		}
 #endif
 		while (low_count > 0) {
@@ -484,9 +476,7 @@ do_write_kmem(void *p, unsigned long rea
 #ifdef __ARCH_HAS_NO_PAGE_ZERO_MAPPED
 	/* we don't have page 0 mapped on sparc and m68k.. */
 	if (realp < PAGE_SIZE) {
-		unsigned long sz = PAGE_SIZE - realp;
-		if (sz > count)
-			sz = count;
+		sz = size_inside_page(realp, count);
 		/* Hmm. Do something? */
 		buf += sz;
 		p += sz;

-- 


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 2/3] devmem: remove the "written" variable in write_kmem()
  2009-09-12 15:20 [PATCH 0/3] more /dev/[k]mem cleanups Wu Fengguang
  2009-09-12 15:20 ` [PATCH 1/3] devmem: make size_inside_page() logic straight Wu Fengguang
@ 2009-09-12 15:20 ` Wu Fengguang
  2009-09-12 15:20 ` [PATCH 3/3] devmem: remove redundant parameter from do_write_kmem() Wu Fengguang
  2 siblings, 0 replies; 6+ messages in thread
From: Wu Fengguang @ 2009-09-12 15:20 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Andi Kleen, Avi Kivity, Greg Kroah-Hartman, Johannes Berg,
	Marcelo Tosatti, Mark Brown, Wu Fengguang, LKML

[-- Attachment #1: kmem-remove-written.patch --]
[-- Type: text/plain, Size: 2717 bytes --]

Also rename "len" to "sz". No behavior change.

Cc: Andi Kleen <ak@linux.intel.com>
Cc: Avi Kivity <avi@qumranet.com>
Cc: Greg Kroah-Hartman <gregkh@suse.de>
Cc: Johannes Berg <johannes@sipsolutions.net>
Cc: Marcelo Tosatti <mtosatti@redhat.com>
Cc: Mark Brown <broonie@opensource.wolfsonmicro.com>
Signed-off-by: Wu Fengguang <fengguang.wu@intel.com>
---
 drivers/char/mem.c |   48 ++++++++++++++++++-------------------------
 1 file changed, 21 insertions(+), 27 deletions(-)

--- linux-mm.orig/drivers/char/mem.c	2009-09-12 23:03:41.000000000 +0800
+++ linux-mm/drivers/char/mem.c	2009-09-12 23:14:33.000000000 +0800
@@ -444,19 +444,18 @@ static ssize_t read_kmem(struct file *fi
 		if (!kbuf)
 			return -ENOMEM;
 		while (count > 0) {
-			int len = size_inside_page(p, count);
-
-			len = vread(kbuf, (char *)p, len);
-			if (!len)
+			sz = size_inside_page(p, count);
+			sz = vread(kbuf, (char *)p, sz);
+			if (!sz)
 				break;
-			if (copy_to_user(buf, kbuf, len)) {
+			if (copy_to_user(buf, kbuf, sz)) {
 				free_page((unsigned long)kbuf);
 				return -EFAULT;
 			}
-			count -= len;
-			buf += len;
-			read += len;
-			p += len;
+			count -= sz;
+			buf += sz;
+			read += sz;
+			p += sz;
 		}
 		free_page((unsigned long)kbuf);
 	}
@@ -526,19 +525,14 @@ static ssize_t write_kmem(struct file * 
 	unsigned long p = *ppos;
 	ssize_t wrote = 0;
 	ssize_t virtr = 0;
-	ssize_t written;
 	char * kbuf; /* k-addr because vwrite() takes vmlist_lock rwlock */
 
 	if (p < (unsigned long) high_memory) {
-
-		wrote = count;
-		if (count > (unsigned long) high_memory - p)
-			wrote = (unsigned long) high_memory - p;
-
-		written = do_write_kmem((void*)p, p, buf, wrote, ppos);
-		if (written != wrote)
-			return written;
-		wrote = written;
+		unsigned long to_write = min_t(unsigned long, count,
+					       (unsigned long)high_memory - p);
+		wrote = do_write_kmem((void *)p, p, buf, to_write, ppos);
+		if (wrote != to_write)
+			return wrote;
 		p += wrote;
 		buf += wrote;
 		count -= wrote;
@@ -549,20 +543,20 @@ static ssize_t write_kmem(struct file * 
 		if (!kbuf)
 			return wrote ? wrote : -ENOMEM;
 		while (count > 0) {
-			int len = size_inside_page(p, count);
+			unsigned long sz = size_inside_page(p, count);
 
-			written = copy_from_user(kbuf, buf, len);
-			if (written) {
+			sz = copy_from_user(kbuf, buf, sz);
+			if (sz) {
 				if (wrote + virtr)
 					break;
 				free_page((unsigned long)kbuf);
 				return -EFAULT;
 			}
-			len = vwrite(kbuf, (char *)p, len);
-			count -= len;
-			buf += len;
-			virtr += len;
-			p += len;
+			sz = vwrite(kbuf, (char *)p, sz);
+			count -= sz;
+			buf += sz;
+			virtr += sz;
+			p += sz;
 		}
 		free_page((unsigned long)kbuf);
 	}

-- 


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 3/3] devmem: remove redundant parameter from do_write_kmem()
  2009-09-12 15:20 [PATCH 0/3] more /dev/[k]mem cleanups Wu Fengguang
  2009-09-12 15:20 ` [PATCH 1/3] devmem: make size_inside_page() logic straight Wu Fengguang
  2009-09-12 15:20 ` [PATCH 2/3] devmem: remove the "written" variable in write_kmem() Wu Fengguang
@ 2009-09-12 15:20 ` Wu Fengguang
  2009-09-12 15:30   ` Daniel Walker
  2 siblings, 1 reply; 6+ messages in thread
From: Wu Fengguang @ 2009-09-12 15:20 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Andi Kleen, Avi Kivity, Greg Kroah-Hartman, Johannes Berg,
	Marcelo Tosatti, Mark Brown, Wu Fengguang, LKML

[-- Attachment #1: kmem-remove-realp.patch --]
[-- Type: text/plain, Size: 2233 bytes --]

Cc: Andi Kleen <ak@linux.intel.com>
Cc: Avi Kivity <avi@qumranet.com>
Cc: Greg Kroah-Hartman <gregkh@suse.de>
Cc: Johannes Berg <johannes@sipsolutions.net>
Cc: Marcelo Tosatti <mtosatti@redhat.com>
Cc: Mark Brown <broonie@opensource.wolfsonmicro.com>
Signed-off-by: Wu Fengguang <fengguang.wu@intel.com>
---
 drivers/char/mem.c |   14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

--- linux-mm.orig/drivers/char/mem.c	2009-09-12 23:17:12.000000000 +0800
+++ linux-mm/drivers/char/mem.c	2009-09-12 23:17:13.000000000 +0800
@@ -465,7 +465,7 @@ static ssize_t read_kmem(struct file *fi
 
 
 static inline ssize_t
-do_write_kmem(void *p, unsigned long realp, const char __user * buf,
+do_write_kmem(unsigned long p, const char __user * buf,
 	      size_t count, loff_t *ppos)
 {
 	ssize_t written, sz;
@@ -474,12 +474,11 @@ do_write_kmem(void *p, unsigned long rea
 	written = 0;
 #ifdef __ARCH_HAS_NO_PAGE_ZERO_MAPPED
 	/* we don't have page 0 mapped on sparc and m68k.. */
-	if (realp < PAGE_SIZE) {
-		sz = size_inside_page(realp, count);
+	if (p < PAGE_SIZE) {
+		sz = size_inside_page(p, count);
 		/* Hmm. Do something? */
 		buf += sz;
 		p += sz;
-		realp += sz;
 		count -= sz;
 		written += sz;
 	}
@@ -488,14 +487,14 @@ do_write_kmem(void *p, unsigned long rea
 	while (count > 0) {
 		char *ptr;
 
-		sz = size_inside_page(realp, count);
+		sz = size_inside_page(p, count);
 
 		/*
 		 * On ia64 if a page has been mapped somewhere as
 		 * uncached, then it must also be accessed uncached
 		 * by the kernel or data corruption may occur
 		 */
-		ptr = xlate_dev_kmem_ptr(p);
+		ptr = xlate_dev_kmem_ptr((char *)p);
 
 		copied = copy_from_user(ptr, buf, sz);
 		if (copied) {
@@ -506,7 +505,6 @@ do_write_kmem(void *p, unsigned long rea
 		}
 		buf += sz;
 		p += sz;
-		realp += sz;
 		count -= sz;
 		written += sz;
 	}
@@ -530,7 +528,7 @@ static ssize_t write_kmem(struct file * 
 	if (p < (unsigned long) high_memory) {
 		unsigned long to_write = min_t(unsigned long, count,
 					       (unsigned long)high_memory - p);
-		wrote = do_write_kmem((void *)p, p, buf, to_write, ppos);
+		wrote = do_write_kmem(p, buf, to_write, ppos);
 		if (wrote != to_write)
 			return wrote;
 		p += wrote;

-- 


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 3/3] devmem: remove redundant parameter from do_write_kmem()
  2009-09-12 15:20 ` [PATCH 3/3] devmem: remove redundant parameter from do_write_kmem() Wu Fengguang
@ 2009-09-12 15:30   ` Daniel Walker
  2009-09-12 15:36     ` Wu Fengguang
  0 siblings, 1 reply; 6+ messages in thread
From: Daniel Walker @ 2009-09-12 15:30 UTC (permalink / raw)
  To: Wu Fengguang
  Cc: Andrew Morton, Andi Kleen, Avi Kivity, Greg Kroah-Hartman,
	Johannes Berg, Marcelo Tosatti, Mark Brown, LKML

On Sat, 2009-09-12 at 23:20 +0800, Wu Fengguang wrote:
>  static inline ssize_t
> -do_write_kmem(void *p, unsigned long realp, const char __user * buf,
> +do_write_kmem(unsigned long p, const char __user * buf,
>               size_t count, loff_t *ppos)
>  {

There's one style issue in the block above. Could you run your patches
through checkpatch and fix whatever errors it reports?

Daniel




^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 3/3] devmem: remove redundant parameter from do_write_kmem()
  2009-09-12 15:30   ` Daniel Walker
@ 2009-09-12 15:36     ` Wu Fengguang
  0 siblings, 0 replies; 6+ messages in thread
From: Wu Fengguang @ 2009-09-12 15:36 UTC (permalink / raw)
  To: Daniel Walker
  Cc: Andrew Morton, Andi Kleen, Avi Kivity, Greg Kroah-Hartman,
	Johannes Berg, Marcelo Tosatti, Mark Brown, LKML

On Sat, Sep 12, 2009 at 11:30:54PM +0800, Daniel Walker wrote:
> On Sat, 2009-09-12 at 23:20 +0800, Wu Fengguang wrote:
> >  static inline ssize_t
> > -do_write_kmem(void *p, unsigned long realp, const char __user * buf,
> > +do_write_kmem(unsigned long p, const char __user * buf,
> >               size_t count, loff_t *ppos)
> >  {
> 
> There's one style issue in the block above. Could you run your patches
> through checkpatch and fix whatever errors it reports?

Thanks for catching it!  Here is the updated patch.
(I ran checkpatch against the first two bugs but forgot this last one..)

--
devmem: remove redundant parameter from do_write_kmem()

Cc: Andi Kleen <ak@linux.intel.com>
Cc: Avi Kivity <avi@qumranet.com>
Cc: Greg Kroah-Hartman <gregkh@suse.de>
Cc: Johannes Berg <johannes@sipsolutions.net>
Cc: Marcelo Tosatti <mtosatti@redhat.com>
Cc: Mark Brown <broonie@opensource.wolfsonmicro.com>
Cc: Daniel Walker <dwalker@fifo99.com>
Signed-off-by: Wu Fengguang <fengguang.wu@intel.com>
---
 drivers/char/mem.c |   14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

--- linux-mm.orig/drivers/char/mem.c	2009-09-12 23:17:12.000000000 +0800
+++ linux-mm/drivers/char/mem.c	2009-09-12 23:35:18.000000000 +0800
@@ -465,7 +465,7 @@ static ssize_t read_kmem(struct file *fi
 
 
 static inline ssize_t
-do_write_kmem(void *p, unsigned long realp, const char __user * buf,
+do_write_kmem(unsigned long p, const char __user *buf,
 	      size_t count, loff_t *ppos)
 {
 	ssize_t written, sz;
@@ -474,12 +474,11 @@ do_write_kmem(void *p, unsigned long rea
 	written = 0;
 #ifdef __ARCH_HAS_NO_PAGE_ZERO_MAPPED
 	/* we don't have page 0 mapped on sparc and m68k.. */
-	if (realp < PAGE_SIZE) {
-		sz = size_inside_page(realp, count);
+	if (p < PAGE_SIZE) {
+		sz = size_inside_page(p, count);
 		/* Hmm. Do something? */
 		buf += sz;
 		p += sz;
-		realp += sz;
 		count -= sz;
 		written += sz;
 	}
@@ -488,14 +487,14 @@ do_write_kmem(void *p, unsigned long rea
 	while (count > 0) {
 		char *ptr;
 
-		sz = size_inside_page(realp, count);
+		sz = size_inside_page(p, count);
 
 		/*
 		 * On ia64 if a page has been mapped somewhere as
 		 * uncached, then it must also be accessed uncached
 		 * by the kernel or data corruption may occur
 		 */
-		ptr = xlate_dev_kmem_ptr(p);
+		ptr = xlate_dev_kmem_ptr((char *)p);
 
 		copied = copy_from_user(ptr, buf, sz);
 		if (copied) {
@@ -506,7 +505,6 @@ do_write_kmem(void *p, unsigned long rea
 		}
 		buf += sz;
 		p += sz;
-		realp += sz;
 		count -= sz;
 		written += sz;
 	}
@@ -530,7 +528,7 @@ static ssize_t write_kmem(struct file * 
 	if (p < (unsigned long) high_memory) {
 		unsigned long to_write = min_t(unsigned long, count,
 					       (unsigned long)high_memory - p);
-		wrote = do_write_kmem((void *)p, p, buf, to_write, ppos);
+		wrote = do_write_kmem(p, buf, to_write, ppos);
 		if (wrote != to_write)
 			return wrote;
 		p += wrote;

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2009-09-12 15:36 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-09-12 15:20 [PATCH 0/3] more /dev/[k]mem cleanups Wu Fengguang
2009-09-12 15:20 ` [PATCH 1/3] devmem: make size_inside_page() logic straight Wu Fengguang
2009-09-12 15:20 ` [PATCH 2/3] devmem: remove the "written" variable in write_kmem() Wu Fengguang
2009-09-12 15:20 ` [PATCH 3/3] devmem: remove redundant parameter from do_write_kmem() Wu Fengguang
2009-09-12 15:30   ` Daniel Walker
2009-09-12 15:36     ` Wu Fengguang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox