Messages in this thread Patch in this message | | | Date | Mon, 14 Sep 2009 12:34:44 +0800 | From | Wu Fengguang <> | Subject | Re: [PATCH] devmem: handle partial kmem write/read |
| |
Hi Kame,
This patch needs more work. I first intent to fix a bug:
sz = vwrite(kbuf, (char *)p, sz); p += sz; }
So if the returned len is 0, the kbuf/p pointers will mismatch.
Then I realize it changed the write behavior. The current vwrite() behavior is strange, it returns 0 if _whole range_ is hole, otherwise ignores the hole silently. So holes can be treated differently even in the original code.
I'm not really sure about the right behavior. KAME-san, do you have any suggestions?
Thanks, Fengguang
On Mon, Sep 14, 2009 at 11:29:01AM +0800, Wu Fengguang wrote: > Current vwrite silently ignores vwrite() to vmap holes. > Better behavior should be stop the write and return > on such holes. > > Also return on partial read, which may happen in future > (eg. hit hwpoison pages). > > CC: Andi Kleen <andi@firstfloor.org> > Signed-off-by: Wu Fengguang <fengguang.wu@intel.com> > --- > drivers/char/mem.c | 30 ++++++++++++++++++------------ > 1 file changed, 18 insertions(+), 12 deletions(-) > > --- linux-mm.orig/drivers/char/mem.c 2009-09-14 10:59:50.000000000 +0800 > +++ linux-mm/drivers/char/mem.c 2009-09-14 11:06:25.000000000 +0800 > @@ -444,18 +444,22 @@ static ssize_t read_kmem(struct file *fi > if (!kbuf) > return -ENOMEM; > while (count > 0) { > + unsigned long n; > + > sz = size_inside_page(p, count); > - sz = vread(kbuf, (char *)p, sz); > - if (!sz) > + n = vread(kbuf, (char *)p, sz); > + if (!n) > break; > - if (copy_to_user(buf, kbuf, sz)) { > + if (copy_to_user(buf, kbuf, n)) { > free_page((unsigned long)kbuf); > return -EFAULT; > } > - count -= sz; > - buf += sz; > - read += sz; > - p += sz; > + count -= n; > + buf += n; > + read += n; > + p += n; > + if (n < sz) > + break; > } > free_page((unsigned long)kbuf); > } > @@ -551,11 +555,13 @@ static ssize_t write_kmem(struct file * > free_page((unsigned long)kbuf); > return -EFAULT; > } > - sz = vwrite(kbuf, (char *)p, sz); > - count -= sz; > - buf += sz; > - virtr += sz; > - p += sz; > + n = vwrite(kbuf, (char *)p, sz); > + count -= n; > + buf += n; > + virtr += n; > + p += n; > + if (n < sz) > + break; > } > free_page((unsigned long)kbuf); > }
| |