* [PATCH v4 4/9] xfsrestore: mmap dirent names for faster lookups
@ 2010-11-19 18:38 Bill Kendall
2010-11-23 13:48 ` Christoph Hellwig
0 siblings, 1 reply; 5+ messages in thread
From: Bill Kendall @ 2010-11-19 18:38 UTC (permalink / raw)
To: xfs
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@sgi.com>
Reviewed-by: Alex Elder <aelder@sgi.com>
---
restore/content.c | 2 -
restore/namreg.c | 98 +++++++++++++++++++++++++++++++++++++-----------------
restore/namreg.h | 6 ++-
3 files changed, 73 insertions(+), 33 deletions(-)
Index: xfsdump-kernel.org/restore/content.c
===================================================================
--- xfsdump-kernel.org.orig/restore/content.c
+++ xfsdump-kernel.org/restore/content.c
@@ -3026,7 +3026,7 @@ applydirdump( drive_t *drivep,
return rv;
}
- if ((rv = namreg_flush()) != RV_OK) {
+ if ((rv = namreg_map()) != RV_OK) {
return rv;
}
Index: xfsdump-kernel.org/restore/namreg.c
===================================================================
--- xfsdump-kernel.org.orig/restore/namreg.c
+++ xfsdump-kernel.org/restore/namreg.c
@@ -57,6 +57,7 @@ typedef struct namreg_pers namreg_pers_t
struct namreg_tran {
char *nt_pathname;
int nt_fd;
+ char *nt_map;
bool_t nt_at_endpr;
size_t nt_off;
char nt_buf[NAMREG_BUFSIZE];
@@ -96,6 +97,7 @@ extern size_t pgsz;
/* forward declarations of locally defined static functions ******************/
+static rv_t namreg_flush( void );
/* definition of locally defined global variables ****************************/
@@ -262,6 +264,7 @@ namreg_add( char *name, size_t namelen )
*/
ASSERT( ntp );
ASSERT( npp );
+ ASSERT( !ntp->nt_map );
/* make sure file pointer is positioned to append
*/
@@ -326,7 +329,7 @@ namreg_del( nrh_t nrh )
*/
}
-rv_t
+static rv_t
namreg_flush( void )
{
ssize_t nwritten;
@@ -367,6 +370,7 @@ namreg_get( nrh_t nrh,
off64_t newoff;
intgen_t nread;
size_t len;
+ char *in_bufp;
static char read_buf[256];
/* long enough for the longest allowed name (255), plus 1 for length */
#ifdef NAMREGCHK
@@ -403,41 +407,51 @@ namreg_get( nrh_t nrh,
lock( );
- if ( ntp->nt_at_endpr && ntp->nt_off ) {
- if (namreg_flush() != RV_OK) {
+ if ( ntp->nt_map ) {
+
+ in_bufp = ntp->nt_map + newoff - NAMREG_PERS_SZ;
+
+ } else {
+
+ if ( ntp->nt_at_endpr && ntp->nt_off ) {
+ if (namreg_flush() != RV_OK) {
+ unlock( );
+ return -3;
+ }
+ }
+
+ /* seek to the name
+ */
+ newoff = lseek64( ntp->nt_fd, newoff, SEEK_SET );
+ if ( newoff == ( off64_t )-1 ) {
unlock( );
+ mlog( MLOG_NORMAL, _(
+ "lseek of namreg failed: %s\n"),
+ strerror( errno ));
return -3;
}
- }
+ ntp->nt_at_endpr = BOOL_FALSE;
- /* seek to the name
- */
- newoff = lseek64( ntp->nt_fd, newoff, SEEK_SET );
- if ( newoff == ( off64_t )-1 ) {
- unlock( );
- mlog( MLOG_NORMAL, _(
- "lseek of namreg failed: %s\n"),
- strerror( errno ));
- return -3;
- }
+ /* read the name length and the name itself in one call
+ * NOTE: assumes read_buf is big enough for the longest
+ * allowed name (255 chars) plus one byte for length.
+ */
+ nread = read( ntp->nt_fd, ( void * )read_buf, sizeof(read_buf) );
+ if ( nread <= 0 ) {
+ unlock( );
+ mlog( MLOG_NORMAL, _(
+ "read of namreg failed: %s (nread = %d)\n"),
+ strerror( errno ),
+ nread );
+ return -3;
+ }
- /* read the name length and the name itself in one call
- * NOTE: assumes read_buf is big enough for the longest
- * allowed name (255 chars) plus one byte for length.
- */
- nread = read( ntp->nt_fd, ( void * )read_buf, sizeof(read_buf) );
- if ( nread <= 0 ) {
- unlock( );
- mlog( MLOG_NORMAL, _(
- "read of namreg failed: %s (nread = %d)\n"),
- strerror( errno ),
- nread );
- return -3;
+ in_bufp = read_buf;
}
/* deal with a short caller-supplied buffer
*/
- len = ( size_t )read_buf[0];
+ len = ( size_t )in_bufp[0];
if ( bufsz < len + 1 ) {
unlock( );
return -1;
@@ -445,7 +459,7 @@ namreg_get( nrh_t nrh,
/* copy the name into the caller-supplied buffer.
*/
- strncpy(bufp, read_buf+1, len);
+ strncpy(bufp, in_bufp+1, len);
#ifdef NAMREGCHK
@@ -460,13 +474,37 @@ namreg_get( nrh_t nrh,
/* null-terminate the string if room
*/
bufp[ len ] = 0;
-
- ntp->nt_at_endpr = BOOL_FALSE;
unlock( );
return ( intgen_t )len;
}
+rv_t
+namreg_map( void )
+{
+ rv_t rv;
+
+ /* ensure all entries have been written */
+ if ( (rv = namreg_flush()) != RV_OK ) {
+ return rv;
+ }
+
+ ntp->nt_map = ( char * ) mmap_autogrow(
+ npp->np_appendoff - NAMREG_PERS_SZ,
+ ntp->nt_fd,
+ NAMREG_PERS_SZ );
+
+ /* it's okay if this fails, just fall back to (the much slower)
+ * seek-and-read lookups.
+ */
+ if ( ntp->nt_map == ( char * )-1 ) {
+ mlog( MLOG_DEBUG, "failed to map namreg: %s\n",
+ strerror( errno ) );
+ ntp->nt_map = NULL;
+ }
+
+ return RV_OK;
+}
/* definition of locally defined static functions ****************************/
Index: xfsdump-kernel.org/restore/namreg.h
===================================================================
--- xfsdump-kernel.org.orig/restore/namreg.h
+++ xfsdump-kernel.org/restore/namreg.h
@@ -49,9 +49,11 @@ extern nrh_t namreg_add( char *name, siz
*/
extern void namreg_del( nrh_t nrh );
-/* namreg_flush - flush namreg I/O buffer. Returns 0 if successful.
+/* namreg_map - mmap the name registry, allowing for much
+ * faster namreg_get() lookups. once called, additional
+ * entries cannot be added.
*/
-extern rv_t namreg_flush( void );
+extern rv_t namreg_map( void );
/* namreg_get - retrieves the name identified by the index.
* fills the buffer with the null-terminated name from the registry.
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v4 4/9] xfsrestore: mmap dirent names for faster lookups
2010-11-19 18:38 [PATCH v4 4/9] xfsrestore: mmap dirent names for faster lookups Bill Kendall
@ 2010-11-23 13:48 ` Christoph Hellwig
2010-11-29 14:28 ` Bill Kendall
0 siblings, 1 reply; 5+ messages in thread
From: Christoph Hellwig @ 2010-11-23 13:48 UTC (permalink / raw)
To: Bill Kendall; +Cc: xfs
On Fri, Nov 19, 2010 at 12:38:37PM -0600, Bill Kendall wrote:
> 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.
This looks much simpler indeed. Is it intentional that the namreg file
is never unmapped any more?
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v4 4/9] xfsrestore: mmap dirent names for faster lookups
2010-11-23 13:48 ` Christoph Hellwig
@ 2010-11-29 14:28 ` Bill Kendall
2010-11-29 14:56 ` Christoph Hellwig
0 siblings, 1 reply; 5+ messages in thread
From: Bill Kendall @ 2010-11-29 14:28 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: xfs
On 11/23/2010 07:48 AM, Christoph Hellwig wrote:
> On Fri, Nov 19, 2010 at 12:38:37PM -0600, Bill Kendall wrote:
>> 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.
>
> This looks much simpler indeed. Is it intentional that the namreg file
> is never unmapped any more?
There are several modules in xfsrestore which map files. None of them
currently contain a destructor/cleanup function for unmapping the files.
It would be a good change to make though.
Bill
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v4 4/9] xfsrestore: mmap dirent names for faster lookups
2010-11-29 14:28 ` Bill Kendall
@ 2010-11-29 14:56 ` Christoph Hellwig
2010-11-29 20:42 ` Alex Elder
0 siblings, 1 reply; 5+ messages in thread
From: Christoph Hellwig @ 2010-11-29 14:56 UTC (permalink / raw)
To: Bill Kendall; +Cc: xfs
On Mon, Nov 29, 2010 at 08:28:05AM -0600, Bill Kendall wrote:
> There are several modules in xfsrestore which map files. None of them
> currently contain a destructor/cleanup function for unmapping the files.
> It would be a good change to make though.
If keeping it mapped later it's fine with me. So ACK from me for the
whole series.
Alex, maybe it's time to push out another xfsdump release after this?
There's quite a few changes that have accumulated.
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v4 4/9] xfsrestore: mmap dirent names for faster lookups
2010-11-29 14:56 ` Christoph Hellwig
@ 2010-11-29 20:42 ` Alex Elder
0 siblings, 0 replies; 5+ messages in thread
From: Alex Elder @ 2010-11-29 20:42 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: xfs
On Mon, 2010-11-29 at 09:56 -0500, Christoph Hellwig wrote:
> On Mon, Nov 29, 2010 at 08:28:05AM -0600, Bill Kendall wrote:
> > There are several modules in xfsrestore which map files. None of them
> > currently contain a destructor/cleanup function for unmapping the files.
> > It would be a good change to make though.
>
> If keeping it mapped later it's fine with me. So ACK from me for the
> whole series.
>
> Alex, maybe it's time to push out another xfsdump release after this?
> There's quite a few changes that have accumulated.
Yes, I had planned to do that once we got Bill's series
committed. I am doing a final test of it today before
pushing it to the -dev tree, for starters.
I also wanted to update my metadump series and, after
getting that committed, mark a new release for xfsprogs
as well. I thought I'd do these releases at the same
time but I guess they don't have to be.
-Alex
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2010-11-29 20:41 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-11-19 18:38 [PATCH v4 4/9] xfsrestore: mmap dirent names for faster lookups Bill Kendall
2010-11-23 13:48 ` Christoph Hellwig
2010-11-29 14:28 ` Bill Kendall
2010-11-29 14:56 ` Christoph Hellwig
2010-11-29 20:42 ` Alex Elder
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox