From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from relay.sgi.com (relay3.corp.sgi.com [198.149.34.15]) by oss.sgi.com (8.14.3/8.14.3/SuSE Linux 0.8) with ESMTP id oAFLoDVQ179839 for ; Mon, 15 Nov 2010 15:50:13 -0600 Received: from estes.americas.sgi.com (estes.americas.sgi.com [128.162.236.10]) by relay3.corp.sgi.com (Postfix) with ESMTP id A5152AC002 for ; Mon, 15 Nov 2010 13:51:45 -0800 (PST) Message-ID: <4CE1AB71.3000708@sgi.com> Date: Mon, 15 Nov 2010 15:51:45 -0600 From: Bill Kendall MIME-Version: 1.0 Subject: Re: [PATCH v2 4/9] xfsrestore: mmap dirent names for faster lookups References: <20101105163500.747192954@sgi.com> <20101105163643.799606284@sgi.com> <1289604322.2315.909.camel@doink> In-Reply-To: <1289604322.2315.909.camel@doink> List-Id: XFS Filesystem from SGI List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset="us-ascii"; Format="flowed" Sender: xfs-bounces@oss.sgi.com Errors-To: xfs-bounces@oss.sgi.com To: aelder@sgi.com Cc: xfs@oss.sgi.com On 11/12/2010 05:25 PM, Alex Elder wrote: > On Fri, 2010-11-05 at 11:35 -0500, wkendall@sgi.com wrote: >> plain text document attachment (namreg_map) >> Pathname resolution in xfsrestore is about 4x faster if the file >> containing dirent names ("namreg") is memory mapped. If xfsrestore is >> unable to map the file (e.g., due to virtual memory constraints) >> fallback to the existing seek-and-read approach. >> >> The file is mapped after all directory entries have been written to >> the "namreg" file. If the caller tries to add additional entries after >> the file has been mapped, it will be unmapped and restore will resort >> back to seek-and-read lookups. >> >> Signed-off-by: Bill Kendall > > I guess I might have created a simple namreg_flush_final() > routine to encapsulate the namreg_flush() and namreg_map() > calls, rather than adding a flag. Then namreg_flush() could > have been made to have static scope. No big deal though. > > Also, you fall back to the non-mmapped method in case a > flush or add happens after you've mapped the file. That > is in some sense comforting, but it would be nice to be > assured it simply won't happen. If you come to a point > someday where you are sure of this it would be nice to > see that code switched to assertions instead. > > You could define the new namreg_*map*() functions earlier > in the file and skip the forward declarations. But the comments told me to put the functions there! ;) xfsdump/restore could benefit from some major refactoring. > > These aren't all that important though, I think it's a > good change. Let me know if you think you will > re-work it but I think it's OK as-is. I'll leave this patch as is, and make a note to look at changing the mmap-checks into asserts. Bill > > Reviewed-by: Alex Elder > > _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs