On Fri, Jun 11, 2010 at 7:50 AM, Nick Piggin <npiggin@xxxxxxx> wrote: > Not sure if this is really the _cleanest_ way to fix it. But open coding > the list walking is a bit annoying too. And I couldn't see any real way to > make the list macro safe. Better ideas? I really think we should open-code the list walking instead. You basically already are doing that, and in a very non-obvious way too (ie you are mixing the non-open-coded list walker with also explicitly playing with the internal variable for that magic walker. So I would get rid of the 'list_for_each_entry_safe' entirely, and replace it with something like struct list_head *list; spin_lock(&sb_lock); list = super_blocks->next; while (list != &super_blocks) { struct super_block *sb = list_entry(next, struct super_block, s_list); list = list->next; if (list_empty(&sb->s_instances)) continue; if (!sb->s_nr_dentry_unused) continue; sb->s_count++; spin_unlock(&sb_lock); .... whatever ... spin_lock(&sb_lock); /* We dropped the lock, need to re-load the next list entry */ list = sb->s_list.next; __put_super(sb); } which isn't that much more complicated, now is it? Sure, it's open-coded, but at least it doesn't play games. And being open-coded, it's a lot more honest about the issue. Maybe even add a comment saying "we can't use the list_for_each[_safe]() macro, because we don't hold the lock and we're not the only ones that may delete things" explaining _why_ it's open-coded. I dunno. Maybe Al disagrees. I just don't like using the "simple helpers" and then changing subtly how they work by knowing their internals. Linus -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html