Re: [PATCH 1/2] kmemleak: enable tracking for percpu pointers

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 7/30/24 22:15, Catalin Marinas wrote:
Hi Pavel,

On Thu, Jul 25, 2024 at 12:12:15PM +0800, Pavel Tikhomirov wrote:
@@ -1308,12 +1319,23 @@ static bool update_checksum(struct kmemleak_object *object)
  {
  	u32 old_csum = object->checksum;
- if (WARN_ON_ONCE(object->flags & (OBJECT_PHYS | OBJECT_PERCPU)))
+	if (WARN_ON_ONCE(object->flags & OBJECT_PHYS))
  		return false;
kasan_disable_current();
  	kcsan_disable_current();
-	object->checksum = crc32(0, kasan_reset_tag((void *)object->pointer), object->size);
+	if (object->flags & OBJECT_PERCPU) {
+		unsigned int cpu;
+
+		object->checksum = 0;
+		for_each_possible_cpu(cpu) {
+			void *ptr = per_cpu_ptr((void __percpu *)object->pointer, cpu);
+
+			object->checksum ^= crc32(0, kasan_reset_tag((void *)ptr), object->size);
+		}

Slight worry this may take too long for large-ish objects with a large
number of CPUs. But we can revisit if anyone complains.

+	} else {
+		object->checksum = crc32(0, kasan_reset_tag((void *)object->pointer), object->size);
+	}
  	kasan_enable_current();
  	kcsan_enable_current();
@@ -1365,6 +1387,64 @@ static int scan_should_stop(void)
  	return 0;
  }
+static void scan_pointer(struct kmemleak_object *scanned,
+			 unsigned long pointer, unsigned int objflags)

Nitpick: I'd have called this lookup_pointer or something like that.
When I first saw it, I tried to figure out why it doesn't have a size
argument but it became clear that it's not actually scanning/reading the
location at 'pointer' but simply looking the value up in various trees.
Up to you if you want to change the name to something else. The patch
looks good.

Yes, I agree "scan_" is a bit too confusing. Let's try "pointer_update_refs", as it is more self-explanatory, and represents what the function does. The "lookup_pointer" seems like a function which should return a pointer, e.g. like "lookup_object" returns an object, but we can always switch to "lookup_pointer" if you like.

I will send v2 with this change shortly.


Reviewed-by: Catalin Marinas <catalin.marinas@xxxxxxx>

Thanks.


--
Best regards, Tikhomirov Pavel
Senior Software Developer, Virtuozzo.




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Bugtraq]     [Linux OMAP]     [Linux MIPS]     [eCos]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux