Re: [patch] fs: fix superblock iteration race

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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


[Index of Archives]     [Linux Ext4 Filesystem]     [Union Filesystem]     [Filesystem Testing]     [Ceph Users]     [Ecryptfs]     [AutoFS]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux Cachefs]     [Reiser Filesystem]     [Linux RAID]     [Samba]     [Device Mapper]     [CEPH Development]
  Powered by Linux