Messages in this thread | | | Date | Sat, 8 Jun 2013 03:03:24 +0300 | Subject | Re: [PATCH 1/2] list: add list_for_each_entry_del | From | Andy Shevchenko <> |
| |
On Fri, Jun 7, 2013 at 9:48 PM, Jörn Engel <joern@logfs.org> wrote: > On Fri, 7 June 2013 21:30:16 +0300, Andy Shevchenko wrote: >> > >> > spin_lock >> > list_for_each_entry_safe >> > list_del >> > spin_unlock >> >> Who is doing such thing? > > Replace list_for_each_entry_safe with 'while (!list_empty(...))' and > just grep. My patch is about 'while (!list_empty(...))', not about > list_for_each_entry_safe.
I saw your patch against btrfs. I didn't see locking there.
Any excerpt like
while (!list_empty(&prefs)) { ref = list_first_entry(&prefs, struct __prelim_ref, list); list_del(&ref->list);
could be transformed to struct __prelim_ref *_ref; list_for_each_entry_safe(ref, _ref, &prefs, list) { list_del(&ref->list); ... }
but is it worth to do? (Same question to your approach)
I see two potential issues with while_list_drain_entry() or whatever name you like: - you delete list as a first operation - you limit user to think in that way, if you choose deletion as last operation, who will go to free resources (call kfree() for example)? - you always use the same ordering (list_first_entry() call), someone may not be satisfied by that
So, the approach with while (!list_empty()) { e = list_entry(); list_del(&e->list); ... } actually not bad. If there any bugs in code, better to fix those bugs explicitly.
What do I not understand?
-- With Best Regards, Andy Shevchenko -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |