[PATCH v2 4/9] xfsrestore: mmap dirent names for faster lookups

[Bill Kendall]

On 11/12/2010 05:25 PM, Alex Elder wrote:
> On Fri, 2010-11-05 at 11:35 -0500, wkendall at 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<wkendall at sgi.com>
>
> 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<aelder at sgi.com>
>
>