Messages in this thread | | | Date | Fri, 11 Sep 2009 16:55:43 -0700 | From | Andrew Morton <> | Subject | Re: [PATCH 2/3] devmem: introduce size_inside_page() |
| |
On Fri, 11 Sep 2009 10:23:35 +0800 Wu Fengguang <fengguang.wu@intel.com> wrote:
> Introduce size_inside_page() to replace duplicate /dev/mem code. > > Also apply it to /dev/kmem, whose alignment logic was buggy. > > > CC: Marcelo Tosatti <mtosatti@redhat.com> > CC: Greg Kroah-Hartman <gregkh@suse.de> > CC: Mark Brown <broonie@opensource.wolfsonmicro.com> > CC: Johannes Berg <johannes@sipsolutions.net> > CC: Avi Kivity <avi@qumranet.com> > Signed-off-by: Wu Fengguang <fengguang.wu@intel.com> > --- > drivers/char/mem.c | 60 +++++++++++++------------------------------ > 1 file changed, 19 insertions(+), 41 deletions(-) > > --- linux.orig/drivers/char/mem.c > +++ linux/drivers/char/mem.c > @@ -35,6 +35,19 @@ > # include <linux/efi.h> > #endif > > +static inline unsigned long size_inside_page(unsigned long start, > + unsigned long size) > +{ > + unsigned long sz; > + > + if (-start & (PAGE_SIZE - 1)) > + sz = -start & (PAGE_SIZE - 1);
What on earth is this doing? Negating an unsigned number?
Can we get rid of these party tricks and use something more conventional here? In a separate patch I guess.
> + else > + sz = PAGE_SIZE; > + > + return min_t(unsigned long, sz, size);
Can use min() here.
> +}
Please have a think about the types. Should we be using unsigned long, or size_t? Which makes more sense? Which maps better onto reality?
I suspect that the min_t which you inherited was added somewhere because someone didn't get the types right: int-vs-size_t or something. If we actually get the types right, this sort of thing goes away.
> @@ -462,10 +451,8 @@ static ssize_t read_kmem(struct file *fi > if (!kbuf) > return -ENOMEM; > while (count > 0) { > - int len = count; > + int len = size_inside_page(p, count);
int?
| |