On Thu, May 25, 2017 at 04:42:16PM +0100, Catalin Marinas wrote:
> The scan_block() function updates the number of references (pointers) to
> objects, adding them to the gray_list when object->min_count is reached.
> The patch factors out this functionality into a separate update_refs()
> function.
>
> Cc: Michal Hocko <mhocko@xxxxxxxxxx>
> Cc: Andy Lutomirski <luto@xxxxxxxxxxxxxx>
> Cc: "Luis R. Rodriguez" <mcgrof@xxxxxxxxxx>
> Signed-off-by: Catalin Marinas <catalin.marinas@xxxxxxx>
> ---
> mm/kmemleak.c | 43 +++++++++++++++++++++++++------------------
> 1 file changed, 25 insertions(+), 18 deletions(-)
>
> diff --git a/mm/kmemleak.c b/mm/kmemleak.c
> index 964b12eba2c1..266482f460c2 100644
> --- a/mm/kmemleak.c
> +++ b/mm/kmemleak.c
> @@ -1188,6 +1188,30 @@ static bool update_checksum(struct kmemleak_object *object)
> }
>
> /*
> + * Update an object's references. object->lock must be held by the caller.
> + */
> +static void update_refs(struct kmemleak_object *object)
> +{
> + if (!color_white(object)) {
> + /* non-orphan, ignored or new */
> + return;
> + }
> +
> + /*
> + * Increase the object's reference count (number of pointers to the
> + * memory block). If this count reaches the required minimum, the
> + * object's color will become gray and it will be added to the
> + * gray_list.
> + */
> + object->count++;
> + if (color_gray(object)) {
> + /* put_object() called when removing from gray_list */
> + WARN_ON(!get_object(object));
> + list_add_tail(&object->gray_list, &gray_list);
> + }
> +}
> +
> +/*
> * Memory scanning is a long process and it needs to be interruptable. This
> * function checks whether such interrupt condition occurred.
> */
> @@ -1259,24 +1283,7 @@ static void scan_block(void *_start, void *_end,
> * enclosed by scan_mutex.
> */
> spin_lock_nested(&object->lock, SINGLE_DEPTH_NESTING);
> - if (!color_white(object)) {
> - /* non-orphan, ignored or new */
> - spin_unlock(&object->lock);
> - continue;
> - }
> -
> - /*
> - * Increase the object's reference count (number of pointers
> - * to the memory block). If this count reaches the required
> - * minimum, the object's color will become gray and it will be
> - * added to the gray_list.
> - */
> - object->count++;
> - if (color_gray(object)) {
> - /* put_object() called when removing from gray_list */
> - WARN_ON(!get_object(object));
> - list_add_tail(&object->gray_list, &gray_list);
> - }
> + update_refs(object);
> spin_unlock(&object->lock);
FWIW, I've tested this patchset and I don't see kmemleak triggering the
false positives anymore.
I've also done a quick review and couldn't find anything obviously
incorrect, just a question: why didn't you moved the spin_lock/unlock into
update_refs() too? It would save you 2 lines in the next patch :)
Cheers,
--
Luís
> }
> read_unlock_irqrestore(&kmemleak_lock, flags);
>
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@xxxxxxxxx. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>
