xfs
[Top] [All Lists]

Re: Re: [PATCH v5] Stop xfs_do_force_shutdown / messages fron xfs_log_f

To: Dave Chinner <david@xxxxxxxxxxxxx>
Subject: Re: Re: [PATCH v5] Stop xfs_do_force_shutdown / messages fron xfs_log_force if filesystem is already shutdown.
From: Raghavendra D Prabhu <raghu.prabhu13@xxxxxxxxx>
Date: Sat, 15 Sep 2012 07:47:23 +0530
Cc: xfs@xxxxxxxxxxx
Dkim-signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=date:from:to:cc:subject:message-id:mail-followup-to:references :mime-version:content-type:content-disposition:in-reply-to :x-operating-system:x-editor:user-agent; bh=X7T59u9Vgqkf5X3/SGQMnWjmk48YM1zG6hPofqvZwgk=; b=VS7Hh0CvvnKzu2M/GEEmFkTQiU0crlzazfYZgW5SwEk3rK2Y+lxIuM5tfT/8DMz1MJ pqr0zOBIMRRfGsI2Vmwb3b0zPuRHan2xHXH77US69XXQmSYststOy/XLyWpBa7eKLqou 5H3vziOyzNbo2qySgSjTA+F2fKzAKa7xedtMaTFXayMMhvasTikYYTzheqyS4N2Pb8pE zRJZ40bHIJNwrSB/U3ARRiCzUmd4wH4SvBw52zR8d6sEy81PGUMwHmDVGL9nj1Ka2TJJ iwECCU/CowWjzCq8GIl4IIwEzlUukK/5L8krvVicGLuF6q/b6BpluNam8rX+R1raQF7s 5LIw==
In-reply-to: <20120911210512.GE11511@dastard>
Mail-followup-to: Dave Chinner <david@xxxxxxxxxxxxx>, xfs@xxxxxxxxxxx
References: <76577fa31ab2aa750994c1bc6a1b6f44beb03116.1347002369.git.rprabhu@xxxxxxxxxxx> <20120911210512.GE11511@dastard>
User-agent: Mutt/1.5.21 (2011-07-01)
Hi,


* On Wed, Sep 12, 2012 at 07:05:12AM +1000, Dave Chinner <david@xxxxxxxxxxxxx> 
wrote:
On Sat, Sep 08, 2012 at 03:28:11AM +0530, raghu.prabhu13@xxxxxxxxx wrote:
From: Raghavendra D Prabhu <rprabhu@xxxxxxxxxxx>
.....
....
...


Version 1: Removed calling xfs_syncd_stop from xfs_sync_worker.
Version 2: Removed calling xfs_fs_writable in xfs_sync_worker and 
xfs_flush_worker.
Version 3: Removed calling xfs_syncd_stop in xfs_bwrite.
Version 4: Added return statements to xfs_log_force and xfs_fs_sync_fs.
Version 5: As per suggestion, added xfs_warn to xfs_fs_sync_fs and removed EIO
return in xfs_log_force

 Signed-off-by: Raghavendra D Prabhu <rprabhu@xxxxxxxxxxx>
 Tested-by: Raghavendra D Prabhu <rprabhu@xxxxxxxxxxx>

Signed-off-by implies you tested it. Tested-by is used to
acknowledge third party testers, usually in the case to acknowledge
the fix was Tested-by the Reported-by...
Sorry about that, I meant to add only the Signed-off part.


And the signed-off-by belongs in the commit message (i.e. about the
first ---) not in the section after the commit message that is
stripped by the git tools.

I will fix that.


---
 fs/xfs/xfs_log.c   | 5 ++++-
 fs/xfs/xfs_super.c | 5 +++++
 2 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c
index 7f4f937..161c925 100644
--- a/fs/xfs/xfs_log.c
+++ b/fs/xfs/xfs_log.c
@@ -3002,7 +3002,10 @@ xfs_log_force(

        trace_xfs_log_force(mp, 0);
        error = _xfs_log_force(mp, flags, NULL);
-       if (error)
+       /*
+        * Avoid warning when the filesystem has already shutdown.
+        */
+       if (error && !XFS_FORCED_SHUTDOWN(mp))
                xfs_warn(mp, "%s: error %d returned.", __func__, error);
 }

diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
index bdaf4cb..ebafc99 100644
--- a/fs/xfs/xfs_super.c
+++ b/fs/xfs/xfs_super.c
@@ -943,6 +943,11 @@ xfs_fs_sync_fs(
        if (!wait)
                return 0;

+       if (XFS_FORCED_SHUTDOWN(mp)) {
+               xfs_warn(mp, "Sync called: filesystem already shutdown");
+               return XFS_ERROR(EIO);
+       }

Why should we emit a warning here? There's already information in the
log to indicate the filesystem is shut down...

Actually in a previous review, you had suggested to add information about it since 'sync' is called manually/less often, so better to have more verbosity about it and user gets feedback on it. So, is this fine?


Cheers,

Dave.
--
Dave Chinner
david@xxxxxxxxxxxxx





Regards,
--
Raghavendra Prabhu
GPG Id : 0xD72BE977
Fingerprint: B93F EBCB 8E05 7039 CD3C A4B8 A616 DCA1 D72B E977
www: wnohang.net

Attachment: pgppvuV7iNmYs.pgp
Description: PGP signature

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