Re: [PATCH 07/13] kmemcheck: add the kmemcheck core

[Randy Dunlap <rdunlap@xenotime.net>]

On Fri, 18 Jul 2008 01:23:41 +0200 Vegard Nossum wrote:

> On Thu, Jul 17, 2008 at 11:49 PM, Randy Dunlap <rdunlap@xenotime.net> wrote:
> > On Wed, 16 Jul 2008 02:23:29 +0200 Vegard Nossum wrote:
> >
> >> >From fcd8f514a8962ea22aafb831b9f22a2ea1a13870 Mon Sep 17 00:00:00 2001
> >
> > eh?
> 
> Uhm, probably relative to my previous patch? About the date, I have no
> idea. All my "git format-patch" output looks like this.
> 
> >> +++ b/arch/x86/mm/kmemcheck/error.h
> >> @@ -0,0 +1,15 @@
> >> +#ifndef ARCH__X86__MM__KMEMCHECK__ERROR_H
> >> +#define ARCH__X86__MM__KMEMCHECK__ERROR_H
> >
> > We don't usually use double __ here.
> 
> But it's such a nice convention! How can we differentiate between
> mm/types.h and mm_types.h!
> 
> >> +void *kmemcheck_memset(void *s, int c, size_t n)
> >> +{
> >> +     unsigned long addr;
> >> +     unsigned long start_page, start_offset;
> >> +     unsigned long end_page, end_offset;
> >> +     unsigned long i;
> >> +
> >> +     if (!n)
> >> +             return s;
> >> +
> >> +     if (!slab_is_available()) {
> >> +             __memset(s, c, n);
> >> +             return s;
> >> +     }
> >> +
> >> +     addr = (unsigned long) s;
> >> +
> >> +     start_page = addr & PAGE_MASK;
> >> +     end_page = (addr + n) & PAGE_MASK;
> >> +
> >> +     if (start_page == end_page) {
> >> +             /* The entire area is within the same page. Good, we only
> >> +              * need one memset(). */
> >> +             memset_one_page(s, c, n);
> >> +             return s;
> >> +     }
> >> +
> >> +     start_offset = addr & ~PAGE_MASK;
> >> +     end_offset = (addr + n) & ~PAGE_MASK;
> >> +
> >> +     /* Clear the head, body, and tail of the memory area. */
> >> +     if (start_offset < PAGE_SIZE)
> >> +             memset_one_page(s, c, PAGE_SIZE - start_offset);
> >> +     for (i = start_page + PAGE_SIZE; i < end_page; i += PAGE_SIZE)
> >> +             memset_one_page((void *) i, c, PAGE_SIZE);
> >> +     if (end_offset > 0)
> >> +             memset_one_page((void *) end_page, c, end_offset);
> >> +
> >> +     return s;
> >> +}
> >> +
> >> +EXPORT_SYMBOL(kmemcheck_memset);
> >
> > We would prefer to have kernel-doc on exported functions...
> 
> Oops. I will not add a kernel-doc, but I will add an explanation.
> 
> This function is a drop-in replacement for memset, and the reason for
> the EXPORT is that the real memset() in fact is just a macro that
> calls this function. And memset is needed in modules. This function
> should not actually have any users beside the memset() in x86 headers.
> 
> Thanks for mini-review! (Does this mean the rest was okay? :-D)

The rest of the comments/documentation ... ;)

---
~Randy
Linux Plumbers Conference, 17-19 September 2008, Portland, Oregon USA
http://linuxplumbersconf.org/