All of lore.kernel.org
 help / color / mirror / Atom feed
From: Alex Elder <aelder@sgi.com>
To: wkendall@sgi.com
Cc: xfs@oss.sgi.com
Subject: Re: [PATCH v2 6/9] xfsrestore: fix node table setup
Date: Mon, 15 Nov 2010 14:38:35 -0600	[thread overview]
Message-ID: <1289853515.2199.225.camel@doink> (raw)
In-Reply-To: <20101105163644.182690256@sgi.com>

On Fri, 2010-11-05 at 11:35 -0500, wkendall@sgi.com wrote:
> plain text document attachment (winmap_max_fix)
> The node table setup code unintentionally limits the amount of virtual
> memory that will be used for the node table. Rather than setting a
> limit based on the remaining virtual memory, the node table is limited
> to the amount of memory it thinks it will need based on the dump's
> inode count. But the node table stores directory entries, not inodes,
> and so dumps with lots of hard links end up thrashing mapping and
> unmapping segments of the node table to stay within the self-imposed
> virtual memory limit.
> 
> This patch also changes the node table to ensure that there are a
> power of two nodes per segment. Profiling revealed that while building
> the node table, 50% of the restore cpu time was spent doing division
> and modulo to convert a node index into its segment index and offset
> within the segment. This change prepares the node table for another
> patch which will change the lookup mechanism to use shift and
> bitwise-and.
> 
> Also don't bother passing the estimated segment table size to the
> window abstraction. It has to deal with resizing the table anyway and
> can choose a reasonable starting size.
> 
> 
> Signed-off-by: Bill Kendall <wkendall@sgi.com>

This looks good to me.

In your loop determining how many segments to
use, etc., you might as well start with the
minimum number of segments.  I.e.:

winmapmax = 0;
for ( segcount = WINMAP_MIN; winmapmax < WINMAP_MIN; segcount <<= 1 ) {

I had a feeling there could be a pathological case
in which the the computation of winmapmax, etc. would
get stuck looping indefinitely, but I gave up trying
to see if I could identify such a case...

And although I'm not that confident I understand the way
the windows on the tree file are used, I don't see anything
really wrong with your change.

Reviewed-by: Alex Elder <aelder@sgi.com>


_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

  reply	other threads:[~2010-11-15 20:37 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-11-05 16:35 [PATCH v2 0/9] xfsrestore dirent limitations and scaling issues wkendall
2010-11-05 16:35 ` [PATCH v2 1/9] xfsrestore: turn off NODECHK wkendall
2010-11-12 23:23   ` Alex Elder
2010-11-05 16:35 ` [PATCH v2 2/9] xfsrestore: change nrh_t from 32 to 64 bits wkendall
2010-11-12 23:24   ` Alex Elder
2010-11-05 16:35 ` [PATCH v2 3/9] xfsrestore: cache path lookups wkendall
2010-11-12 23:25   ` Alex Elder
2010-11-05 16:35 ` [PATCH v2 4/9] xfsrestore: mmap dirent names for faster lookups wkendall
2010-11-12 23:25   ` Alex Elder
2010-11-15 21:51     ` Bill Kendall
2010-11-05 16:35 ` [PATCH v2 5/9] xfsrestore: cleanup node allocation wkendall
2010-11-15 20:38   ` Alex Elder
2010-11-15 21:36     ` Bill Kendall
2010-11-05 16:35 ` [PATCH v2 6/9] xfsrestore: fix node table setup wkendall
2010-11-15 20:38   ` Alex Elder [this message]
2010-11-15 21:30     ` Bill Kendall
2010-11-05 16:35 ` [PATCH v2 7/9] xfsrestore: make node lookup more efficient wkendall
2010-11-15 20:38   ` Alex Elder
2010-11-15 22:06     ` Bill Kendall
2010-11-05 16:35 ` [PATCH v2 8/9] xfsrestore: remove nix_t wkendall
2010-11-12 23:25   ` Alex Elder
2010-11-05 16:35 ` [PATCH v2 9/9] xfsrestore: check for compatible xfsrestore wkendall
2010-11-12 23:25   ` Alex Elder
2010-11-12 23:25 ` [PATCH v2 0/9] xfsrestore dirent limitations and scaling issues Alex Elder

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=1289853515.2199.225.camel@doink \
    --to=aelder@sgi.com \
    --cc=wkendall@sgi.com \
    --cc=xfs@oss.sgi.com \
    /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.