All of lore.kernel.org
 help / color / mirror / Atom feed
From: Greg Banks <gnb@sgi.com>
To: "J. Bruce Fields" <bfields@fieldses.org>
Cc: Neil Brown <neilb@suse.de>, nfs@lists.sourceforge.net
Subject: Re: [PATCH] nfsd: fix possible read-ahead cache and export table corruption
Date: Fri, 13 Jul 2007 14:10:19 +1000	[thread overview]
Message-ID: <20070713041019.GB20430@sgi.com> (raw)
In-Reply-To: <20070713024420.GA31364@fieldses.org>

On Thu, Jul 12, 2007 at 10:44:20PM -0400, J. Bruce Fields wrote:
> From: J. Bruce Fields <bfields@citi.umich.edu>
> 
> The value of nperbucket calculated here is too small--we should be
> [...]
> diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
> index 7e50da0..dd3604e 100644
> --- a/fs/nfsd/vfs.c
> +++ b/fs/nfsd/vfs.c
> @@ -1885,7 +1885,7 @@ nfsd_racache_init(int cache_size)
>  		raparm_hash[i].pb_head = NULL;
>  		spin_lock_init(&raparm_hash[i].pb_lock);
>  	}
> -	nperbucket = cache_size >> RAPARM_HASH_BITS;
> +	nperbucket = DIV_ROUND_UP(cache_size, RAPARM_HASH_SIZE);
>  	for (i = 0; i < cache_size - 1; i++) {
>  		if (i % nperbucket == 0)
>  			raparm_hash[j++].pb_head = raparml + i;
> -- 
> 1.5.3.rc0.63.gc956

My bad.  The code I added really only works properly if the number
of nfsd threads at startup is a multiple of 8, so that cache_size
is a multiple of 16 and the shift doesn't lose any nonzero bits.
As we use 128 threads in our product, I'd never noticed the bug :-(
Sorry for the trouble, Bruce.

There's a couple of other problems with that code, for which I'll send
patches shortly.  But I think the raparams cache needs a more thorough
renovation than that.  I have a mostly-working patch which rips it out
entirely and replaces it with a cache of open struct files, an idea
of Neil's.  This approach fixes a whole bunch of separate problems
at once, and will be really useful...if I can ever find enough time
between super-urgent dramas to work on it ;-)

Greg.
-- 
Greg Banks, R&D Software Engineer, SGI Australian Software Group.
Apparently, I'm Bedevere.  Which MPHG character are you?
I don't speak for SGI.

-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/
_______________________________________________
NFS maillist  -  NFS@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs

  reply	other threads:[~2007-07-13  4:10 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-07-13  2:44 [PATCH] nfsd: fix possible read-ahead cache and export table corruption J. Bruce Fields
2007-07-13  4:10 ` Greg Banks [this message]
2007-07-13 17:46   ` J. Bruce Fields
2007-07-24 14:40     ` J. Bruce Fields

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=20070713041019.GB20430@sgi.com \
    --to=gnb@sgi.com \
    --cc=bfields@fieldses.org \
    --cc=neilb@suse.de \
    --cc=nfs@lists.sourceforge.net \
    /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.