All of lore.kernel.org
 help / color / mirror / Atom feed
From: Peter Staubach <staubach@redhat.com>
To: Trond Myklebust <trond.myklebust@fys.uio.no>
Cc: Steve Dickson <SteveD@redhat.com>,
	nfs@lists.sourceforge.net, linux-fsdevel@vger.kernel.org
Subject: Re: [PATCH][RFC] NFS: Improving the access cache
Date: Wed, 26 Apr 2006 09:14:56 -0400	[thread overview]
Message-ID: <444F7250.2070200@redhat.com> (raw)
In-Reply-To: <1146056601.8177.34.camel@lade.trondhjem.org>

Trond Myklebust wrote:

>On Tue, 2006-04-25 at 21:14 -0400, Steve Dickson wrote:
>  
>
>>Currently the NFS client caches ACCESS information on a per uid basis
>>which fall apart when different process with different uid consistently
>>access the same directory. The end result being a storm of needless
>>ACCESS calls...
>>
>>The attached patch used a hash table to store the nfs_access_entry
>>entires which cause the ACCESS request to only happen when the
>>attributes timeout.. The table is indexed by the addition of the
>>nfs_inode pointer and the cr_uid in the cred structure which should
>>spread things out nicely for some decent scalability (although the
>>locking scheme may need to be reworked a bit). The table has 256 entries
>>of struct list_head giving it a total size of 2k.
>>    
>>
>
>Instead of having the field 'id', why don't you let the nfs_inode keep a
>small (hashed?) list of all the nfs_access_entry objects that refer to
>it? That would speed up searches for cached entries.
>
>I agree with Neil's assessment that we need a bound on the size of the
>cache. In fact, enforcing a bound is pretty much the raison d'être for a
>global table (by which I mean that if we don't need a bound, then we
>might as well cache everything in the nfs_inode).
>How about rather changing that hash table into an LRU list, then adding
>a shrinker callback (using set_shrinker()) to allow the VM to free up
>entries when memory pressure dictates that it must?
>

Previous implementations have shown that a single per inode linear 
linked list
ends up not being scalable enough in certain situations.  There would end up
being too many entries in the list and searching the list would become
a bottleneck.  Adding a set of hash buckets per inode also proved to be
inefficient because in order to have enough hash buckets to make the hashing
efficient, much space was wasted.  Having a single set of hash buckets,
adequately sized, ended up being the best solution.

Why have a limit?  A better solution is to have the cache grow dynamically
as need requires and then have the system reclaim the memory when it starts
to run low on memory.

       ps
-
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

WARNING: multiple messages have this Message-ID (diff)
From: Peter Staubach <staubach@redhat.com>
To: Trond Myklebust <trond.myklebust@fys.uio.no>
Cc: Steve Dickson <SteveD@redhat.com>,
	nfs@lists.sourceforge.net, linux-fsdevel@vger.kernel.org
Subject: Re: [PATCH][RFC] NFS: Improving the access cache
Date: Wed, 26 Apr 2006 09:14:56 -0400	[thread overview]
Message-ID: <444F7250.2070200@redhat.com> (raw)
In-Reply-To: <1146056601.8177.34.camel@lade.trondhjem.org>

Trond Myklebust wrote:

>On Tue, 2006-04-25 at 21:14 -0400, Steve Dickson wrote:
> =20
>
>>Currently the NFS client caches ACCESS information on a per uid basis
>>which fall apart when different process with different uid consistent=
ly
>>access the same directory. The end result being a storm of needless
>>ACCESS calls...
>>
>>The attached patch used a hash table to store the nfs_access_entry
>>entires which cause the ACCESS request to only happen when the
>>attributes timeout.. The table is indexed by the addition of the
>>nfs_inode pointer and the cr_uid in the cred structure which should
>>spread things out nicely for some decent scalability (although the
>>locking scheme may need to be reworked a bit). The table has 256 entr=
ies
>>of struct list_head giving it a total size of 2k.
>>   =20
>>
>
>Instead of having the field 'id', why don't you let the nfs_inode keep=
 a
>small (hashed?) list of all the nfs_access_entry objects that refer to
>it? That would speed up searches for cached entries.
>
>I agree with Neil's assessment that we need a bound on the size of the
>cache. In fact, enforcing a bound is pretty much the raison d'=EAtre f=
or a
>global table (by which I mean that if we don't need a bound, then we
>might as well cache everything in the nfs_inode).
>How about rather changing that hash table into an LRU list, then addin=
g
>a shrinker callback (using set_shrinker()) to allow the VM to free up
>entries when memory pressure dictates that it must?
>

Previous implementations have shown that a single per inode linear=20
linked list
ends up not being scalable enough in certain situations.  There would e=
nd up
being too many entries in the list and searching the list would become
a bottleneck.  Adding a set of hash buckets per inode also proved to be
inefficient because in order to have enough hash buckets to make the ha=
shing
efficient, much space was wasted.  Having a single set of hash buckets,
adequately sized, ended up being the best solution.

Why have a limit?  A better solution is to have the cache grow dynamica=
lly
as need requires and then have the system reclaim the memory when it st=
arts
to run low on memory.

       ps
-
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel=
" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

  reply	other threads:[~2006-04-26 13:15 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2006-04-26  1:14 [PATCH][RFC] NFS: Improving the access cache Steve Dickson
2006-04-26  1:31 ` Matthew Wilcox
2006-04-26  4:55 ` Neil Brown
2006-04-26 14:51   ` Steve Dickson
2006-04-26 22:32     ` Neil Brown
2006-05-02  9:49       ` Steve Dickson
2006-05-02 13:51         ` [NFS] " Peter Staubach
2006-05-02 14:38           ` Steve Dickson
2006-05-02 14:51             ` Peter Staubach
2006-05-02 15:26               ` [NFS] " Ian Kent
2006-05-03  4:42         ` Chuck Lever
2006-05-05 14:07           ` Steve Dickson
2006-05-05 14:53             ` Peter Staubach
2006-05-05 14:59               ` Peter Staubach
2006-05-06 14:35               ` [NFS] " Steve Dickson
2006-05-08 14:07                 ` Peter Staubach
2006-05-08 17:09                   ` Trond Myklebust
2006-05-08 17:20                     ` Peter Staubach
2006-05-08 17:37                     ` Steve Dickson
2006-05-08  2:44           ` [NFS] " Neil Brown
2006-05-08  3:23             ` Chuck Lever
2006-05-08  3:28               ` Neil Brown
2006-04-26 13:03 ` Trond Myklebust
2006-04-26 13:03   ` Trond Myklebust
2006-04-26 13:14   ` Peter Staubach [this message]
2006-04-26 13:14     ` Peter Staubach
2006-04-26 14:01     ` Trond Myklebust
2006-04-26 14:01       ` Trond Myklebust
2006-04-26 14:15       ` Peter Staubach
2006-04-26 14:15         ` Peter Staubach
2006-04-26 15:44         ` Trond Myklebust
2006-04-26 17:01           ` Peter Staubach
2006-04-26 15:03   ` Steve Dickson
2006-04-26 15:03     ` Steve Dickson
2006-04-26 13:17 ` [NFS] " Chuck Lever
2006-04-26 14:19   ` Steve Dickson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=444F7250.2070200@redhat.com \
    --to=staubach@redhat.com \
    --cc=SteveD@redhat.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=nfs@lists.sourceforge.net \
    --cc=trond.myklebust@fys.uio.no \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.