Messages in this thread | | | Date | Mon, 1 Dec 2008 09:15:33 +0200 | From | "Pekka Enberg" <> | Subject | Re: [PATCH 01/15] kmemleak: Add the base support |
| |
Hi Catalin,
On Sat, Nov 29, 2008 at 12:43 PM, Catalin Marinas <catalin.marinas@arm.com> wrote: > +/* > + * Insert a pointer into the pointer hash table. > + */ > +static inline void create_object(unsigned long ptr, size_t size, int ref_count) > +{
[...]
> + if (ptr < min_addr) > + min_addr = ptr; > + if (ptr + size > max_addr) > + max_addr = ptr + size; > + /* > + * Update the boundaries before inserting the object in the > + * prio search tree. > + */ > + smp_mb();
I'm not sure I understand the purpose of this memory barrier. As soon as some other CPU acquires object_tree_lock, updates to the boundaries will be visible due to the implicit memory barriers in locking functions (see Documentation/memory-barrier.txt for details).
However, I'm wondering why this isn't a smp_wmb() and..
> +/* > + * Scan a block of memory (exclusive range) for pointers and move > + * those found to the gray list. > + */ > +static void scan_block(void *_start, void *_end, struct memleak_object *scanned) > +{ > + unsigned long *ptr; > + unsigned long *start = PTR_ALIGN(_start, BYTES_PER_WORD); > + unsigned long *end = _end - (BYTES_PER_WORD - 1); > + > + for (ptr = start; ptr < end; ptr++) {
...why don't we have the pairing smp_rmb() here before we read min_addr and max_addr?
> + > + /* > + * The boundaries check doesn't need to be precise > + * (hence no locking) since orphan objects need to > + * pass a scanning threshold before being reported. > + */ > + if (pointer < min_addr || pointer >= max_addr) > + continue;
Pekka
| |