Re: [PATCH] kmemleak: Do not enable KMEMCHECK_PARTIAL_OK if DEBUG_KMEMLEAK

[Catalin Marinas <catalin.marinas@arm.com>]

On Wed, 2010-01-27 at 15:09 +0000, Pekka Enberg wrote:
> Catalin Marinas wrote:
> > diff --git a/mm/kmemleak.c b/mm/kmemleak.c
> > index 5b069e4..dfd39ba 100644
> > --- a/mm/kmemleak.c
> > +++ b/mm/kmemleak.c
> > @@ -948,6 +948,25 @@ EXPORT_SYMBOL(kmemleak_no_scan);
> >  /*
> >   * Update an object's checksum and return true if it was modified.
> >   */
> > +#ifdef CONFIG_KMEMCHECK_PARTIAL_OK
> > +static bool update_checksum(struct kmemleak_object *object)
> > +{
> > +     u32 crc = 0;
> > +     unsigned long ptr;
> > +
> > +     for (ptr = ALIGN(object->pointer, 4);
> > +          ptr < ((object->pointer + object->size) & ~3); ptr += 4) {
> > +             if (!kmemcheck_is_obj_initialized(ptr, 4))
> > +                     continue;
> > +             crc = crc32(crc, (void *)ptr, 4);
> > +     }
> > +
> > +     if (object->checksum == crc)
> > +             return false;
> > +     object->checksum = crc;
> > +     return true;
> 
> I think it would be better to add a function to _kmemcheck_ that checks
> the full object regardless of CONFIG_KMEMCHECK_PARTIAL_OK and use it here.

IIRC, it's only kmemleak using kmemcheck_is_obj_initialized(), we could
convert this to check the full object. Something like below (again, not
tested yet but if you are ok with the idea I'll test it a bit more and
add a commit log):


diff --git a/arch/x86/mm/kmemcheck/kmemcheck.c b/arch/x86/mm/kmemcheck/kmemcheck.c
index 8cc1833..b3b531a 100644
--- a/arch/x86/mm/kmemcheck/kmemcheck.c
+++ b/arch/x86/mm/kmemcheck/kmemcheck.c
@@ -337,7 +337,7 @@ bool kmemcheck_is_obj_initialized(unsigned long addr, size_t size)
 	if (!shadow)
 		return true;
 
-	status = kmemcheck_shadow_test(shadow, size);
+	status = kmemcheck_shadow_test_all(shadow, size);
 
 	return status == KMEMCHECK_SHADOW_INITIALIZED;
 }
diff --git a/arch/x86/mm/kmemcheck/shadow.c b/arch/x86/mm/kmemcheck/shadow.c
index 3f66b82..16c6d9f 100644
--- a/arch/x86/mm/kmemcheck/shadow.c
+++ b/arch/x86/mm/kmemcheck/shadow.c
@@ -139,13 +139,25 @@ enum kmemcheck_shadow kmemcheck_shadow_test(void *shadow, unsigned int size)
 		if (x[i] == KMEMCHECK_SHADOW_INITIALIZED)
 			return x[i];
 	}
+
+	return x[0];
 #else
+	return kmemcheck_shadow_test_all(shadow, size);
+#endif
+}
+
+enum kmemcheck_shadow kmemcheck_shadow_test_all(void *shadow, unsigned int size)
+{
+	uint8_t *x;
+	unsigned int i;
+
+	x = shadow;
+
 	/* All bytes must be initialized. */
 	for (i = 0; i < size; ++i) {
 		if (x[i] != KMEMCHECK_SHADOW_INITIALIZED)
 			return x[i];
 	}
-#endif
 
 	return x[0];
 }
diff --git a/arch/x86/mm/kmemcheck/shadow.h b/arch/x86/mm/kmemcheck/shadow.h
index af46d9a..ff0b2f7 100644
--- a/arch/x86/mm/kmemcheck/shadow.h
+++ b/arch/x86/mm/kmemcheck/shadow.h
@@ -11,6 +11,8 @@ enum kmemcheck_shadow {
 void *kmemcheck_shadow_lookup(unsigned long address);
 
 enum kmemcheck_shadow kmemcheck_shadow_test(void *shadow, unsigned int size);
+enum kmemcheck_shadow kmemcheck_shadow_test_all(void *shadow,
+						unsigned int size);
 void kmemcheck_shadow_set(void *shadow, unsigned int size);
 
 #endif


-- 
Catalin