Skip to contentSkip to conversation

mm/kmemleak.c: Use list_for_each_entry_safe to reconstruct function scan_gray_list

PATCH In review majianpeng 2 participants 3 messages May 14, 2013 – Jun 3, 2013

Subsystems (2)
lore
Share & tools
lore

majianpeng

author
#0 lore raw

PATCH 2/3 mm/kmemleak.c: Use list_for_each_entry_safe to reconstruct function scan_gray_list

---
 mm/kmemleak.c | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)
mm/kmemleak.c +1 -7
index b1525db..f0ece93 100644
@@ -1225,22 +1225,16 @@ static void scan_gray_list(void)
      * from inside the loop. The kmemleak objects cannot be freed from
      * outside the loop because their use_count was incremented.
      */
-    object = list_entry(gray_list.next, typeof(*object), gray_list);
-    while (&object->gray_list != &gray_list) {
+    list_for_each_entry_safe(object, tmp, &gray_list, gray_list) {
         cond_resched();
 
         /* may add new objects to the list */
         if (!scan_should_stop())
             scan_object(object);
 
-        tmp = list_entry(object->gray_list.next, typeof(*object),
-                 gray_list);
-
         /* remove the object from the list and release it */
         list_del(&object->gray_list);
         put_object(object);
-
-        object = tmp;
     }
     WARN_ON(!list_empty(&gray_list));
 }
Signature · 1 line
1.8.3.rc1.44.gb387c77
maintainer
#1 lore raw
Quoted 22 lines · majianpeng
---
 mm/kmemleak.c | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)
mm/kmemleak.c +1 -2
index b1525db..f0ece93 100644
@@ -1225,22 +1225,16 @@ static void scan_gray_list(void)
 	 * from inside the loop. The kmemleak objects cannot be freed from
 	 * outside the loop because their use_count was incremented.
 	 */
-	object = list_entry(gray_list.next, typeof(*object), gray_list);
-	while (&object->gray_list != &gray_list) {
+	list_for_each_entry_safe(object, tmp, &gray_list, gray_list) {
 		cond_resched();
 
 		/* may add new objects to the list */
 		if (!scan_should_stop())
 			scan_object(object);
 

mm/kmemleak.c static void scan_gray_list(void) · line 1239

Quoted 10 lines
mm/kmemleak.c +0 -5
-		tmp = list_entry(object->gray_list.next, typeof(*object),
-				 gray_list);
-
 		/* remove the object from the list and release it */
 		list_del(&object->gray_list);
 		put_object(object);
-
-		object = tmp;
 	}
 	WARN_ON(!list_empty(&gray_list));

I tried this patch for a few days and I hit the WARN_ON after the loop. During scanning, new entries may be added at the end of the loop but we need to loop until all the entries have been removed. I probably had a reason why I had the 'while' loop.

The key difference is that list_for_each_entry_safe() gets the next entry (n or tmp above) before scan_object() and it may hit the end of the list. However, scan_object() may do a list_add_tail(&gray_list) hence we need to get the next entry after this function.

Basically list_for_each_entry_safe() is not safe with tail additions. I'll revert this patch (hasn't reached mainline anyway).

Thanks.

Signature · 7 lines
Catalin

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
author
#2 lore raw

Pk9uIFR1ZSwgTWF5IDE0LCAyMDEzIGF0IDEyOjQ5OjQ0UE0gKzAxMDAsIG1hamlhbnBlbmcgd3Jv dGU6DQo+PiBTaWduZWQtb2ZmLWJ5OiBKaWFucGVuZyBNYSA8bWFqaWFucGVuZ0BnbWFpbC5jb20+ DQo+PiAtLS0NCj4+ICBtbS9rbWVtbGVhay5jIHwgOCArLS0tLS0tLQ0KPj4gIDEgZmlsZSBjaGFu Z2VkLCAxIGluc2VydGlvbigrKSwgNyBkZWxldGlvbnMoLSkNCj4+IA0KPj4gZGlmZiAtLWdpdCBh L21tL2ttZW1sZWFrLmMgYi9tbS9rbWVtbGVhay5jDQo+PiBpbmRleCBiMTUyNWRiLi5mMGVjZTkz IDEwMDY0NA0KPj4gLS0tIGEvbW0va21lbWxlYWsuYw0KPj4gKysrIGIvbW0va21lbWxlYWsuYw0K Pj4gQEAgLTEyMjUsMjIgKzEyMjUsMTYgQEAgc3RhdGljIHZvaWQgc2Nhbl9ncmF5X2xpc3Qodm9p ZCkNCj4+ICAJICogZnJvbSBpbnNpZGUgdGhlIGxvb3AuIFRoZSBrbWVtbGVhayBvYmplY3RzIGNh bm5vdCBiZSBmcmVlZCBmcm9tDQo+PiAgCSAqIG91dHNpZGUgdGhlIGxvb3AgYmVjYXVzZSB0aGVp ciB1c2VfY291bnQgd2FzIGluY3JlbWVudGVkLg0KPj4gIAkgKi8NCj4+IC0Jb2JqZWN0ID0gbGlz dF9lbnRyeShncmF5X2xpc3QubmV4dCwgdHlwZW9mKCpvYmplY3QpLCBncmF5X2xpc3QpOw0KPj4g LQl3aGlsZSAoJm9iamVjdC0+Z3JheV9saXN0ICE9ICZncmF5X2xpc3QpIHsNCj4+ICsJbGlzdF9m b3JfZWFjaF9lbnRyeV9zYWZlKG9iamVjdCwgdG1wLCAmZ3JheV9saXN0LCBncmF5X2xpc3QpIHsN Cj4+ICAJCWNvbmRfcmVzY2hlZCgpOw0KPj4gIA0KPj4gIAkJLyogbWF5IGFkZCBuZXcgb2JqZWN0 cyB0byB0aGUgbGlzdCAqLw0KPj4gIAkJaWYgKCFzY2FuX3Nob3VsZF9zdG9wKCkpDQo+PiAgCQkJ c2Nhbl9vYmplY3Qob2JqZWN0KTsNCj4+ICANCj4+IC0JCXRtcCA9IGxpc3RfZW50cnkob2JqZWN0 LT5ncmF5X2xpc3QubmV4dCwgdHlwZW9mKCpvYmplY3QpLA0KPj4gLQkJCQkgZ3JheV9saXN0KTsN Cj4+IC0NCj4+ICAJCS8qIHJlbW92ZSB0aGUgb2JqZWN0IGZyb20gdGhlIGxpc3QgYW5kIHJlbGVh c2UgaXQgKi8NCj4+ICAJCWxpc3RfZGVsKCZvYmplY3QtPmdyYXlfbGlzdCk7DQo+PiAgCQlwdXRf b2JqZWN0KG9iamVjdCk7DQo+PiAtDQo+PiAtCQlvYmplY3QgPSB0bXA7DQo+PiAgCX0NCj4+ICAJ V0FSTl9PTighbGlzdF9lbXB0eSgmZ3JheV9saXN0KSk7DQo+DQo+SSB0cmllZCB0aGlzIHBhdGNo IGZvciBhIGZldyBkYXlzIGFuZCBJIGhpdCB0aGUgV0FSTl9PTiBhZnRlciB0aGUgbG9vcC4NCj5E dXJpbmcgc2Nhbm5pbmcsIG5ldyBlbnRyaWVzIG1heSBiZSBhZGRlZCBhdCB0aGUgZW5kIG9mIHRo ZSBsb29wIGJ1dCB3ZQ0KPm5lZWQgdG8gbG9vcCB1bnRpbCBhbGwgdGhlIGVudHJpZXMgaGF2ZSBi ZWVuIHJlbW92ZWQuIEkgcHJvYmFibHkgaGFkIGENCj5yZWFzb24gd2h5IEkgaGFkIHRoZSAnd2hp bGUnIGxvb3AuDQo+DQo+VGhlIGtleSBkaWZmZXJlbmNlIGlzIHRoYXQgbGlzdF9mb3JfZWFjaF9l bnRyeV9zYWZlKCkgZ2V0cyB0aGUgbmV4dA0KPmVudHJ5IChuIG9yIHRtcCBhYm92ZSkgYmVmb3Jl IHNjYW5fb2JqZWN0KCkgYW5kIGl0IG1heSBoaXQgdGhlIGVuZCBvZg0KPnRoZSBsaXN0LiBIb3dl dmVyLCBzY2FuX29iamVjdCgpIG1heSBkbyBhIGxpc3RfYWRkX3RhaWwoJmdyYXlfbGlzdCkNCj5o ZW5jZSB3ZSBuZWVkIHRvIGdldCB0aGUgbmV4dCBlbnRyeSBhZnRlciB0aGlzIGZ1bmN0aW9uLg0K Pg0KPkJhc2ljYWxseSBsaXN0X2Zvcl9lYWNoX2VudHJ5X3NhZmUoKSBpcyBub3Qgc2FmZSB3aXRo IHRhaWwgYWRkaXRpb25zLg0KPkknbGwgcmV2ZXJ0IHRoaXMgcGF0Y2ggKGhhc24ndCByZWFjaGVk IG1haW5saW5lIGFueXdheSkuDQo+DQpPaywgaSBzZWUuIA0KVGhhbmtzIQ0KPlRoYW5rcy4NCj4N Cj4tLSANCj5DYXRhbGlu

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>