Messages in this thread |  | | | Date | Tue, 15 Sep 2009 16:22:00 +0800 | | From | Wu Fengguang <> | | Subject | Re: [PATCH 0/3] /proc/kmem cleanups and hwpoison bits |
| |
On Tue, Sep 15, 2009 at 11:09:39AM +0800, KAMEZAWA Hiroyuki wrote: > On Tue, 15 Sep 2009 10:18:51 +0800 > Wu Fengguang <fengguang.wu@intel.com> wrote: > > > Hi Kame, > > > > Here are 3 more kmem patches in my queue. Comments are welcome. > > If you feel good about them, I can send all recent kmem cleanup > > patches for you. > > > > This is my quick hack. But I don't want to be an obstacle for you. > So, I'll wait for your updates.
Thanks. This is also a bug fix: vmalloc_to_page() will otherwise BUG() on !is_vmalloc_or_module_addr() pages.
> == > Now, /dev/kmem's read/write vmalloc area doesn't do > range-check. Because vread/vwrite traverse vmalloc area list > under system-wide spinlock, it's better to avoid unnecessary > to do unnecessary calls to vread/vwrite.
is_vmalloc_or_module_addr() could be put to either read_kmem() or aligned_vread(), and I'm fine with both.
It looks like vread can be shared by kmem and kcore :)
> And, vread/vwrite returns 0 if we accessed memory holes. > We can avoid copy-to-user in read side, we just ignore at write. > > Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> > --- > drivers/char/mem.c | 27 +++++++++++++++++++-------- > 1 file changed, 19 insertions(+), 8 deletions(-) > > Index: linux-2.6.31/drivers/char/mem.c > =================================================================== > --- linux-2.6.31.orig/drivers/char/mem.c > +++ linux-2.6.31/drivers/char/mem.c > @@ -437,17 +437,19 @@ static ssize_t read_kmem(struct file *fi > } > > if (count > 0) { > - kbuf = (char *)__get_free_page(GFP_KERNEL); > + kbuf = (char *)__get_free_page(GFP_KERNEL | __GFP_ZERO); > if (!kbuf) > return -ENOMEM; > while (count > 0) { > sz = size_inside_page(p, count); > - sz = vread(kbuf, (char *)p, sz); > - if (!sz) > - break; > - if (copy_to_user(buf, kbuf, sz)) { > - free_page((unsigned long)kbuf); > - return -EFAULT; > + if (is_vmalloc_or_module_addr((void*)p)) { > + /* returns Non-zero if a page is found */ > + if (vread(kbuf, (char *)p, sz)) > + if (copy_to_user(buf, kbuf, sz)) { > + free_page((unsigned long)kbuf); > + return -EFAULT; > + } > + } > } > count -= sz; > buf += sz; > @@ -541,6 +543,11 @@ static ssize_t write_kmem(struct file * > unsigned long sz = size_inside_page(p, count); > unsigned long n; > > + if (is_vmalloc_or_module_addr((void*)p)) {
!is_vmalloc_or_module_addr() ?
> + free_page((unsignedl long)kbuf); > + retrun -EFAULT; > + } > + > n = copy_from_user(kbuf, buf, sz); > if (n) { > if (wrote + virtr) > @@ -548,7 +555,11 @@ static ssize_t write_kmem(struct file * > free_page((unsigned long)kbuf); > return -EFAULT; > } > - sz = vwrite(kbuf, (char *)p, sz); > + /* > + * returns non-zero if copied successfully.... > + * But we don't care it. just make a progress. > + */ > + vwrite(kbuf, (char *)p, sz);
vwrite could return error code which should not be ignored.
Thanks, Fengguang
> count -= sz; > buf += sz; > virtr += sz; >
|  |