linux-kernel - Re: [patch 11/33] fs: dcache scale subdirs

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1276787615.27822.426.camel@twins>
Date:	Thu, 17 Jun 2010 17:13:35 +0200
From:	Peter Zijlstra <peterz@...radead.org>
To:	npiggin@...e.de
Cc:	linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org,
	john stultz <johnstul@...ibm.com>,
	John Kacur <jkacur@...il.com>,
	Thomas Gleixner <tglx@...utronix.de>
Subject: Re: [patch 11/33] fs: dcache scale subdirs

On Fri, 2009-09-04 at 16:51 +1000, npiggin@...e.de wrote:
> @@ -932,6 +984,7 @@ static int select_parent(struct dentry *
>         int found = 0;
>  
>         spin_lock(&dcache_lock);
> +       spin_lock(&this_parent->d_lock);
>  repeat:
>         next = this_parent->d_subdirs.next;
>  resume:
> @@ -939,8 +992,9 @@ resume:
>                 struct list_head *tmp = next;
>                 struct dentry *dentry = list_entry(tmp, struct dentry, d_u.d_child);
>                 next = tmp->next;
> +               BUG_ON(this_parent == dentry);
>  
> -               spin_lock(&dentry->d_lock);
> +               spin_lock_nested(&dentry->d_lock, DENTRY_D_LOCK_NESTED);

Right, so this isn't going to work well, this dentry recursion is
basically unbounded afaict, so the 2nd subdir will also be locked using
DENRTY_D_LOCKED_NESTED, resulting in the 1st and 2nd subdir both having
the same (sub)class and lockdep doesn't like that much.

Do we really need to keep the whole path locked? One of the comments
seems to suggest we could actually drop some locks and re-acquire.

>                 dentry_lru_del_init(dentry);
>                 /* 
>                  * move only zero ref count dentries to the end 
> @@ -950,33 +1004,45 @@ resume:
>                         dentry_lru_add_tail(dentry);
>                         found++;
>                 }
> -               spin_unlock(&dentry->d_lock);
>  
>                 /*
>                  * We can return to the caller if we have found some (this
>                  * ensures forward progress). We'll be coming back to find
>                  * the rest.
>                  */
> -               if (found && need_resched())
> +               if (found && need_resched()) {
> +                       spin_unlock(&dentry->d_lock);
>                         goto out;
> +               }
>  
>                 /*
>                  * Descend a level if the d_subdirs list is non-empty.
>                  */
>                 if (!list_empty(&dentry->d_subdirs)) {
> +                       spin_unlock(&this_parent->d_lock);
> +                       spin_release(&dentry->d_lock.dep_map, 1, _RET_IP_);
>                         this_parent = dentry;
> +                       spin_acquire(&this_parent->d_lock.dep_map, 0, 1, _RET_IP_);
>                         goto repeat;
>                 }
> +
> +               spin_unlock(&dentry->d_lock);
>         } 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/