[PATCH 1/2] jbd2: jbd2_log_wait_for_space improve error detetcion

[PATCH 1/2] jbd2: jbd2_log_wait_for_space improve error detetcion

[Dmitry Monakhov]

If EIO happen after we have drop j_state_lock so it is reasonable to move this check under
i_sate_lock critical section. This patch helps to  prevent false positive complain after EIO
#DMESG:
__jbd2_log_wait_for_space: needed 8448 blocks and only had 8386 space available
__jbd2_log_wait_for_space: no way to get more journal space in ram1-8
------------[ cut here ]------------
WARNING: CPU: 15 PID: 6739 at fs/jbd2/checkpoint.c:168 __jbd2_log_wait_for_space+0x188/0x200()
Modules linked in: brd iTCO_wdt lpc_ich mfd_core igb ptp dm_mirror dm_region_hash dm_log dm_mod
CPU: 15 PID: 6739 Comm: fsstress Tainted: G        W      3.17.0-rc2-00429-g684de57 #139
Hardware name: Intel Corporation W2600CR/W2600CR, BIOS SE5C600.86B.99.99.x028.061320111235 06/13/2011
 00000000000000a8 ffff88077aaab878 ffffffff815c1a8c 00000000000000a8
 0000000000000000 ffff88077aaab8b8 ffffffff8106ce8c ffff88077aaab898
 ffff8807c57e6000 ffff8807c57e6028 0000000000002100 ffff8807c57e62f0
Call Trace:
 [<ffffffff815c1a8c>] dump_stack+0x51/0x6d
 [<ffffffff8106ce8c>] warn_slowpath_common+0x8c/0xc0
 [<ffffffff8106ceda>] warn_slowpath_null+0x1a/0x20
 [<ffffffff812419f8>] __jbd2_log_wait_for_space+0x188/0x200
 [<ffffffff8123be9a>] start_this_handle+0x4da/0x7b0
 [<ffffffff810990e5>] ? local_clock+0x25/0x30
 [<ffffffff810aba87>] ? lockdep_init_map+0xe7/0x180
 [<ffffffff8123c5bc>] jbd2__journal_start+0xdc/0x1d0
 [<ffffffff811f2414>] ? __ext4_new_inode+0x7f4/0x1330
 [<ffffffff81222a38>] __ext4_journal_start_sb+0xf8/0x110
 [<ffffffff811f2414>] __ext4_new_inode+0x7f4/0x1330
 [<ffffffff810ac359>] ? lock_release_holdtime+0x29/0x190
 [<ffffffff812025bb>] ext4_create+0x8b/0x150
 [<ffffffff8117fe3b>] vfs_create+0x7b/0xb0
 [<ffffffff8118097b>] do_last+0x7db/0xcf0
 [<ffffffff8117e31d>] ? inode_permission+0x4d/0x50
 [<ffffffff811845d2>] path_openat+0x242/0x590
 [<ffffffff81191a76>] ? __alloc_fd+0x36/0x140
 [<ffffffff81184a6a>] do_filp_open+0x4a/0xb0
 [<ffffffff81191b61>] ? __alloc_fd+0x121/0x140
 [<ffffffff81172f20>] do_sys_open+0x170/0x220
 [<ffffffff8117300e>] SyS_open+0x1e/0x20
 [<ffffffff811715d6>] SyS_creat+0x16/0x20
 [<ffffffff815c7e12>] system_call_fastpath+0x16/0x1b
---[ end trace cd71c831f82059db ]---

Signed-off-by: Dmitry Monakhov <dmonakhov@openvz.org>
---
 fs/jbd2/checkpoint.c |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/fs/jbd2/checkpoint.c b/fs/jbd2/checkpoint.c
index 9ffb19c..1fbf599 100644
--- a/fs/jbd2/checkpoint.c
+++ b/fs/jbd2/checkpoint.c
@@ -115,8 +115,6 @@ void __jbd2_log_wait_for_space(journal_t *journal)
 
 	nblocks = jbd2_space_needed(journal);
 	while (jbd2_log_space_left(journal) < nblocks) {
-		if (journal->j_flags & JBD2_ABORT)
-			return;
 		write_unlock(&journal->j_state_lock);
 		mutex_lock(&journal->j_checkpoint_mutex);
 
@@ -132,6 +130,10 @@ void __jbd2_log_wait_for_space(journal_t *journal)
 		 * trace for forensic evidence.
 		 */
 		write_lock(&journal->j_state_lock);
+		if (journal->j_flags & JBD2_ABORT) {
+			mutex_unlock(&journal->j_checkpoint_mutex);
+			return;
+		}
 		spin_lock(&journal->j_list_lock);
 		nblocks = jbd2_space_needed(journal);
 		space_left = jbd2_log_space_left(journal);
-- 
1.7.1

[PATCH 2/2] ext4: Explicitly inform user about orphan list cleanup.

[Dmitry Monakhov]

Production fs likely compiled/mounted w/o jbd debugging, so orphan list clearing
will be silent.

Signed-off-by: Dmitry Monakhov <dmonakhov@openvz.org>
---
 fs/ext4/super.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index 2632017..028935f 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -2191,7 +2191,7 @@ static void ext4_orphan_cleanup(struct super_block *sb,
 	if (EXT4_SB(sb)->s_mount_state & EXT4_ERROR_FS) {
 		/* don't clear list on RO mount w/ errors */
 		if (es->s_last_orphan && !(s_flags & MS_RDONLY)) {
-			jbd_debug(1, "Errors on filesystem, "
+			ext4_msg(sb, KERN_INFO, "Errors on filesystem, "
 				  "clearing orphan list.\n");
 			es->s_last_orphan = 0;
 		}
-- 
1.7.1

Re: [PATCH 1/2] jbd2: jbd2_log_wait_for_space improve error detetcion

[Theodore Ts'o]

On Sun, Sep 14, 2014 at 04:38:56PM +0400, Dmitry Monakhov wrote:
> If EIO happen after we have drop j_state_lock so it is reasonable to move this check under
> i_sate_lock critical section. This patch helps to  prevent false positive complain after EIO

Thanks, applied.  I adjusted the commit description slightly:

    If EIO happens after we have dropped j_state_lock, we won't notice
    that the journal has been aborted.  So it is reasonable to move this
    check after we have grabbed the j_checkpoint_mutex and re-grabbed the
    j_state_lock.  This patch helps to prevent false positive complain
    after EIO.

Cheers,

					- Ted

P.S.  Welcome back!  Glad to see you are safe.

Re: [PATCH 2/2] ext4: Explicitly inform user about orphan list cleanup.

[Theodore Ts'o]

On Sun, Sep 14, 2014 at 04:38:57PM +0400, Dmitry Monakhov wrote:
> Production fs likely compiled/mounted w/o jbd debugging, so orphan list clearing
> will be silent.
> 
> Signed-off-by: Dmitry Monakhov <dmonakhov@openvz.org>

Thanks, applied.

					- Ted

Re: [PATCH 2/2] ext4: Explicitly inform user about orphan list cleanup.

[Lukáš Czerner]

On Thu, 18 Sep 2014, Theodore Ts'o wrote:

> Date: Thu, 18 Sep 2014 00:29:07 -0400
> From: Theodore Ts'o <tytso@mit.edu>
> To: Dmitry Monakhov <dmonakhov@openvz.org>
> Cc: linux-ext4@vger.kernel.org
> Subject: Re: [PATCH 2/2] ext4: Explicitly inform user about orphan list
>     cleanup.
> 
> On Sun, Sep 14, 2014 at 04:38:57PM +0400, Dmitry Monakhov wrote:
> > Production fs likely compiled/mounted w/o jbd debugging, so orphan list clearing
> > will be silent.
> > 
> > Signed-off-by: Dmitry Monakhov <dmonakhov@openvz.org>
> 
> Thanks, applied.

Hmm did not we just take this out recently with

566370a2e56837be98bc62d012fc2d2d11851db4 ext4: suppress ext4 orphan
messages on mount

and TBH I like it that way.

Thanks!
-Lukas

> 
> 					- Ted
> --
> To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH 2/2] ext4: Explicitly inform user about orphan list cleanup.

[Dmitry Monakhov]

On Thu, 18 Sep 2014 10:49:56 +0200 (CEST), Lukáš Czerner <lczerner@redhat.com> wrote:
> On Thu, 18 Sep 2014, Theodore Ts'o wrote:
> 
> > Date: Thu, 18 Sep 2014 00:29:07 -0400
> > From: Theodore Ts'o <tytso@mit.edu>
> > To: Dmitry Monakhov <dmonakhov@openvz.org>
> > Cc: linux-ext4@vger.kernel.org
> > Subject: Re: [PATCH 2/2] ext4: Explicitly inform user about orphan list
> >     cleanup.
> > 
> > On Sun, Sep 14, 2014 at 04:38:57PM +0400, Dmitry Monakhov wrote:
> > > Production fs likely compiled/mounted w/o jbd debugging, so orphan list clearing
> > > will be silent.
> > > 
> > > Signed-off-by: Dmitry Monakhov <dmonakhov@openvz.org>
> > 
> > Thanks, applied.
> 
> Hmm did not we just take this out recently with
> 
> 566370a2e56837be98bc62d012fc2d2d11851db4 ext4: suppress ext4 orphan
> messages on mount
The message I want to change appears only if file system contains
errors. So IMHO this is reasonable to make this message louder.
This is important because after orphan list will be directly cleared
later fsck will complain about incorrect dtime and corrupted orphan list.
Sane administrator should investigate why this happen. So leaving
precise message in /var/log/messages is good practice from our side.
> 
> and TBH I like it that way.
> 
> Thanks!
> -Lukas
> 
> > 
> > 					- Ted
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > 
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/2] ext4: Explicitly inform user about orphan list cleanup.

[Lukáš Czerner]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 2042 bytes --]

On Thu, 18 Sep 2014, Dmitry Monakhov wrote:

> Date: Thu, 18 Sep 2014 13:30:36 +0400
> From: Dmitry Monakhov <dmonakhov@openvz.org>
> To: Lukáš Czerner <lczerner@redhat.com>, Theodore Ts'o <tytso@mit.edu>
> Cc: linux-ext4@vger.kernel.org
> Subject: Re: [PATCH 2/2] ext4: Explicitly inform user about orphan list
>     cleanup.
> 
> On Thu, 18 Sep 2014 10:49:56 +0200 (CEST), Lukáš Czerner <lczerner@redhat.com> wrote:
> > On Thu, 18 Sep 2014, Theodore Ts'o wrote:
> > 
> > > Date: Thu, 18 Sep 2014 00:29:07 -0400
> > > From: Theodore Ts'o <tytso@mit.edu>
> > > To: Dmitry Monakhov <dmonakhov@openvz.org>
> > > Cc: linux-ext4@vger.kernel.org
> > > Subject: Re: [PATCH 2/2] ext4: Explicitly inform user about orphan list
> > >     cleanup.
> > > 
> > > On Sun, Sep 14, 2014 at 04:38:57PM +0400, Dmitry Monakhov wrote:
> > > > Production fs likely compiled/mounted w/o jbd debugging, so orphan list clearing
> > > > will be silent.
> > > > 
> > > > Signed-off-by: Dmitry Monakhov <dmonakhov@openvz.org>
> > > 
> > > Thanks, applied.
> > 
> > Hmm did not we just take this out recently with
> > 
> > 566370a2e56837be98bc62d012fc2d2d11851db4 ext4: suppress ext4 orphan
> > messages on mount
> The message I want to change appears only if file system contains
> errors. So IMHO this is reasonable to make this message louder.
> This is important because after orphan list will be directly cleared
> later fsck will complain about incorrect dtime and corrupted orphan list.
> Sane administrator should investigate why this happen. So leaving
> precise message in /var/log/messages is good practice from our side.

Ah, yes. It's for the file system with errors so it is fine. I spoke too
soon, sorry about that.

Thanks!
-Lukas

> > 
> > and TBH I like it that way.
> > 
> > Thanks!
> > -Lukas
> > 
> > > 
> > > 					- Ted
> > > --
> > > To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
> > > the body of a message to majordomo@vger.kernel.org
> > > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > > 
>