[Top] [All Lists]

Re: [PATCH] xfsdump: add locks around the inventory put

To: Eric Sandeen <sandeen@xxxxxxxxxxx>
Subject: Re: [PATCH] xfsdump: add locks around the inventory put
From: Rich Johnston <rjohnston@xxxxxxx>
Date: Thu, 3 Oct 2013 09:17:32 -0500
Cc: xfs-oss <xfs@xxxxxxxxxxx>
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <524B395C.6030705@xxxxxxxxxxx>
References: <524AF8C3.8020904@xxxxxxx> <524B395C.6030705@xxxxxxxxxxx>
User-agent: Mozilla/5.0 (X11; Linux i686; rv:24.0) Gecko/20100101 Thunderbird/24.0
On 10/01/2013 04:06 PM, Eric Sandeen wrote:
hi Rich -

On 10/1/13 11:30 AM, Rich Johnston wrote:
From: Phil White <cerise-xfs@xxxxxxxxxxxx>

Add locks around the inventory put to prevent inventory

Signed-off-by: Phil White <cerise-xfs@xxxxxxxxxxxx>

Similar questions here; it says it's thread-safe, but apparently
not.  But why not?  what happens?  Is there a testcase?

This was reported by a customer, no test case, so we used the customers data to verify the fix. Failed every time the customer performed a 20 stream dump/restore and with single stream. Single stream would fail rarley with a different error. Two storage object files were corrupted, causing xfsinvutil -C to crash. As it is customer data we could not supply it as a test case.

Customer runs fine now with this patch,

Bug info

  BUG Title: xfsinvutil seg faults when trying to use -C

  #0  0x00007fd83a98c9d2 in __strlen_sse2 () from /lib64/libc.so.6
  #1  0x00007fd83a95520d in vfprintf () from /lib64/libc.so.6
  #2  0x00007fd83a9f9a2d in __printf_chk () from /lib64/libc.so.6
#3 0x0000000000402b54 in printf (__fmt=<optimized out>) at /usr/include/bits/stdio2.h:105 #4 CheckAndPruneStObjFile (checkonly=1, StObjFileName=0x7fd83b2e8a00 <Address 0x7fd83b2e8a00 out of bounds>,
    sessionp=0x7ffffc02a150, prunetime=0, r_mf_label=0x0) at invutil.c:759
  #5  0x000000000040329b in CheckAndPruneInvIndexFile (checkonly=1,
idxFileName=0x60f040 "/var/lib/xfsdump/inventory/aeb05cfe-c923-4972-ad18-2968326aba46.InvIndex", sessionp=0x7ffffc02a150,
    prunetime=0, r_mf_label=0x0) at invutil.c:651
#6 0x00000000004039f6 in CheckAndPruneFstab (inv_path=0x60e6e0 "/var/lib/xfsdump/inventory", checkonly=1, mountPt=0x40a9c4 "test", uuidp=0x7ffffc02a160, sessionp=0x7ffffc02a150, prunetime=0, r_mf_label=0x0) at invutil.c:538 #7 0x00000000004043bf in main (argc=2, argv=0x7ffffc02a288) at invutil.c:253

  The corrupted files seem to have struct inv_mediafile or maybe struct
inv_stream overwriting the header of the file, and both seem to include media files from multi stream dumps. 83ab1189-b1a7-4733-bfd6-aa8a4feb188e.StObj has a good header that looks like it was written over a struct inv_stream... I suspect we're looking at some kind of locking issue with multistream xfsdump
  adding stream entries to this file in the inventory.

(you guys probably have longer history in ptools, maybe you
can see what if anything changed since the original comment
was added - or maybe when it was added, etc?)

Looks like it was added as the IRIX original commit

cbullis |1.1 | | /* already thread-safe, don't need to lock
cbullis   |1.1  |                            |                   */


diff --git a/dump/content.c b/dump/content.c
index ac19021..b8977bb 100644
--- a/dump/content.c
+++ b/dump/content.c
@@ -2550,8 +2550,11 @@ decision_more:
                         scwhdrp->cih_startpt.sp_offset );

-            /* already thread-safe, don't need to lock
+            /* Supposedly already thread-safe, according to the
+             * previous revisions, but corruption of inventory
+             * objects can occur.
+            lock();
              ok = inv_put_mediafile( inv_stmt,
@@ -2565,6 +2568,7 @@ decision_more:
                          ! empty_mediafile,
                          BOOL_FALSE );
+            unlock();
              if ( ! ok ) {
                  mlog( MLOG_NORMAL, _(
                        "inventory media file put failed\n") );

xfs mailing list

<Prev in Thread] Current Thread [Next in Thread>