xfs
[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: Tue, 1 Oct 2013 17:19:04 -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 x86_64; 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
corruption.

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?

Phil worked on this for a long time. To my knowledge the corruption occured with multistream dumps only. I did not change his code but we have had reports from the field on corruption when using multistream dumps which does not occur with this fix.

Not sure what else Phil did to reproduce this other than a multistream dump/restore (20 streams), which fails.

Testcase?? Good point I need to submit a testcase for both patches.


(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?)

I did not look into this further, I will see if there is a better explaination in our bug writeup.

Thanks,
-Eric

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,
                          &mwhdrp->mh_mediaid,
                          mwhdrp->mh_medialabel,
@@ -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
xfs@xxxxxxxxxxx
http://oss.sgi.com/mailman/listinfo/xfs



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