lkml.org 
[lkml]   [2001]   [Sep]   [25]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: Locking comment on shrink_caches()
Quoting Marcelo Tosatti (marcelo@conectiva.com.br):
>
>
> On Tue, 25 Sep 2001, David S. Miller wrote:
>
> > From: Marcelo Tosatti <marcelo@conectiva.com.br>
> > Date: Tue, 25 Sep 2001 14:49:40 -0300 (BRT)
> >
> > Do you really need to do this ?
> >
> > if (unlikely(!spin_trylock(&pagecache_lock))) {
> > /* we hold the page lock so the page cannot go away from under us */
> > spin_unlock(&pagemap_lru_lock);
> >
> > spin_lock(&pagecache_lock);
> > spin_lock(&pagemap_lru_lock);
> > }
> >
> > Have you actually seen bad hold times of pagecache_lock by
> > shrink_caches() ?
> >
> > Marcelo, this is needed because of the spin lock ordering rules.
> > The pagecache_lock must be obtained before the pagemap_lru_lock
> > or else deadlock is possible. The spin_trylock is an optimization.
>
> Not, it is not.
>
> We can simply lock the pagecachelock and the pagemap_lru_lock at the
> beginning of the cleaning function. page_launder() use to do that.

Since your main concern seems to be simplicity, the code can remain
the way it is and be far more readable with, e.g.,

/* Aquire lock1 while holding lock2--reverse order. */
#define spin_reverse_lock(lock1,lock2) \
if (unlikely(!spin_trylock(&lock1))) { \
spin_unlock(&lock2); \
spin_lock(&lock1); \
spin_lock(&lock2); \
}

You can't argue for simple in favor of increasing lock contention,
but you can keep it readable.

-josh
-
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/

\
 
 \ /
  Last update: 2005-03-22 13:03    [W:0.105 / U:1.736 seconds]
©2003-2018 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site