linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Artem Bityutskiy <dedekind1@gmail.com>
To: Jan Kara <jack@suse.cz>
Cc: Ted Tso <tytso@mit.edu>,
	Ext4 Mailing List <linux-ext4@vger.kernel.org>,
	Linux FS Maling List <linux-fsdevel@vger.kernel.org>,
	Linux Kernel Maling List <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v1 0/9] do not use s_dirt in ext4
Date: Fri, 30 Mar 2012 18:23:55 +0300	[thread overview]
Message-ID: <1333121035.5440.49.camel@sauron.fi.intel.com> (raw)
In-Reply-To: <20120327201401.GF5020@quack.suse.cz>

[-- Attachment #1: Type: text/plain, Size: 7068 bytes --]

On Tue, 2012-03-27 at 22:14 +0200, Jan Kara wrote:
> On Tue 27-03-12 16:29:58, Artem Bityutskiy wrote:
> > On Thu, 2012-03-22 at 11:33 +0100, Jan Kara wrote:
> > >   Then we have ext4_mark_super_dirty() call from 4 places - I forgot about
> > > these originally... I kind of miss their purpose. Originally they were used
> > > so that we write total number of free blocks and inodes in the superblock
> > > but when we do not maintain them in the journal mode I don't see a reason
> > > to periodically sync them in no-journal mode. Ted, what is the purpose of
> > > these calls?
> > 
> > I do not understand what's the fundamental difference between journal
> > and non-journal mode. Why when we do have the journal we do not mark the
> > super-block as dirty in many places (e.g., in 'ext4_file_open()' - if we
> > do have the journal, when do we make sure we save the mount point path
> > change?).
>   We write it at least during ext4_put_super().
> 
> > May be it has something to do with behaving like the ext2 driver when
> > mounting ext2-formatted media with the the ext4 driver?
>   I'm not really sure about this...
> 
> > Jan, since Ted did not answer, may be you can figure out the reasons
> > from this commit message, which actually introduced the
> > 'ext4_mark_super_dirty()' function?
>   Anyway, attached are two patches which you can include in your series
> and which should make your cleanups simpler.

I amended the second patch:
-                               ext4_journal_stop(sb);
+                               ext4_journal_stop(sbi->s_sbh);

Extensive testing with xfstests found a problem with these patches:

[23500.238579] ------------[ cut here ]------------
[23500.238720] kernel BUG at fs/buffer.c:2871!
[23500.238842] invalid opcode: 0000 [#1] SMP 
[23500.239279] CPU 11 
[23500.239338] Modules linked in: [last unloaded: scsi_wait_scan]
[23500.239442] 
[23500.239442] Pid: 15799, comm: fsstress Not tainted 3.3.0+ #43 Bochs Bochs
[23500.239442] RIP: 0010:[<ffffffff811a9add>]  [<ffffffff811a9add>] submit_bh+0x10d/0x120
[23500.239442] RSP: 0018:ffff880273a41b58  EFLAGS: 00010202
[23500.239442] RAX: 000000000004d025 RBX: ffff8802469e5a90 RCX: 0000000000000000
[23500.239442] RDX: ffff880273a41fd8 RSI: ffff8802469e5a90 RDI: 0000000000000211
[23500.239442] RBP: ffff880273a41b78 R08: ffff880409645d80 R09: 0000000000000001
[23500.239442] R10: 0000000000000001 R11: ffff880178439000 R12: 0000000000000211
[23500.239442] R13: ffff880273a41c34 R14: ffff8804059b7000 R15: ffff880273a41fd8
[23500.239442] FS:  00007fc8731e7700(0000) GS:ffff88041fd60000(0000) knlGS:0000000000000000
[23500.239442] CS:  0010 DS: 0000 ES: 0000 CR0: 000000008005003b
[23500.239442] CR2: 00007fc873082008 CR3: 000000012456a000 CR4: 00000000000006e0
[23500.239442] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[23500.239442] DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
[23500.239442] Process fsstress (pid: 15799, threadinfo ffff880273a40000, task ffff880150dd8000)
[23500.239442] Stack:
[23500.239442]  ffff8804059b7000 ffff8802469e5a90 0000000000000211 ffff880273a41c34
[23500.239442]  ffff880273a41b98 ffffffff811ab59d 0000000000000002 ffff8804059b7128
[23500.239442]  ffff880273a41bf8 ffffffff8123be1d 0000000091827364 ffff88010e9a7e30
[23500.239442] Call Trace:
[23500.239442]  [<ffffffff811ab59d>] write_dirty_buffer+0x4d/0x80
[23500.239442]  [<ffffffff8123be1d>] __flush_batch+0x4d/0xa0
[23500.239442]  [<ffffffff8123c605>] jbd2_log_do_checkpoint+0xf5/0x4f0
[23500.239442]  [<ffffffff8123ca89>] __jbd2_log_wait_for_space+0x89/0x190
[23500.239442]  [<ffffffff81237a98>] start_this_handle+0x3a8/0x4e0
[23500.239442]  [<ffffffff810799e0>] ? remove_wait_queue+0x50/0x50
[23500.239442]  [<ffffffff81237c93>] jbd2__journal_start+0xc3/0x100
[23500.239442]  [<ffffffff81237ce3>] jbd2_journal_start+0x13/0x20
[23500.239442]  [<ffffffff8121743f>] ext4_journal_start_sb+0x7f/0x1d0
[23500.239442]  [<ffffffff81224a24>] ? ext4_fallocate+0x1a4/0x530
[23500.239442]  [<ffffffff811f64c5>] ? ext4_meta_trans_blocks+0xa5/0xb0
[23500.239442]  [<ffffffff81224a24>] ext4_fallocate+0x1a4/0x530
[23500.239442]  [<ffffffff8117a092>] do_fallocate+0xf2/0x160
[23500.239442]  [<ffffffff8117a14b>] sys_fallocate+0x4b/0x70
[23500.239442]  [<ffffffff815e6d69>] system_call_fastpath+0x16/0x1b
[23500.239442] Code: ee 44 89 e7 e8 35 1f 0f 00 49 8b 5d 18 4c 89 ef e8 19 4e 00 00 48 83 c4 08 c1 e3 18 c1 fb 1f 83 e3 a1 89 d8 5b 41 5c 41 5d 5d c3 <0f> 0b 0f 0b 0f 0b 0f 0b 0f 0b 66 0f 1f 84 00 00 00 00 00 55 48 
[23500.239442] RIP  [<ffffffff811a9add>] submit_bh+0x10d/0x120
[23500.239442]  RSP <ffff880273a41b58>
[23500.261657] ---[ end trace 3db7a7a7ae953551 ]---
[23500.262131] ------------[ cut here ]------------
[23500.262577] WARNING: at kernel/exit.c:897 do_exit+0x55/0x870()
[23500.263070] Hardware name: Bochs
[23500.263467] Modules linked in: [last unloaded: scsi_wait_scan]
[23500.264129] Pid: 15799, comm: fsstress Tainted: G      D      3.3.0+ #43
[23500.264623] Call Trace:
[23500.265066]  [<ffffffff810572bf>] warn_slowpath_common+0x7f/0xc0
[23500.265542]  [<ffffffff8105731a>] warn_slowpath_null+0x1a/0x20
[23500.266034]  [<ffffffff8105b1c5>] do_exit+0x55/0x870
[23500.266482]  [<ffffffff81058f6c>] ? kmsg_dump+0x5c/0xf0
[23500.266932]  [<ffffffff815dfeac>] oops_end+0xac/0xf0
[23500.267418]  [<ffffffff81017928>] die+0x58/0x90
[23500.267852]  [<ffffffff815df7a4>] do_trap+0xc4/0x170
[23500.268343]  [<ffffffff81014fa5>] do_invalid_op+0x95/0xb0
[23500.268803]  [<ffffffff811a9add>] ? submit_bh+0x10d/0x120
[23500.269344]  [<ffffffff81299db4>] ? drive_stat_acct+0x114/0x190
[23500.269825]  [<ffffffff8129ec36>] ? blk_queue_bio+0x106/0x400
[23500.270324]  [<ffffffff815e7f9b>] invalid_op+0x1b/0x20
[23500.270778]  [<ffffffff811a9add>] ? submit_bh+0x10d/0x120
[23500.271263]  [<ffffffff811a9ac7>] ? submit_bh+0xf7/0x120
[23500.271723]  [<ffffffff811ab59d>] write_dirty_buffer+0x4d/0x80
[23500.272217]  [<ffffffff8123be1d>] __flush_batch+0x4d/0xa0
[23500.272675]  [<ffffffff8123c605>] jbd2_log_do_checkpoint+0xf5/0x4f0
[23500.273226]  [<ffffffff8123ca89>] __jbd2_log_wait_for_space+0x89/0x190
[23500.273714]  [<ffffffff81237a98>] start_this_handle+0x3a8/0x4e0
[23500.274213]  [<ffffffff810799e0>] ? remove_wait_queue+0x50/0x50
[23500.274686]  [<ffffffff81237c93>] jbd2__journal_start+0xc3/0x100
[23500.275193]  [<ffffffff81237ce3>] jbd2_journal_start+0x13/0x20
[23500.275664]  [<ffffffff8121743f>] ext4_journal_start_sb+0x7f/0x1d0
[23500.276166]  [<ffffffff81224a24>] ? ext4_fallocate+0x1a4/0x530
[23500.276638]  [<ffffffff811f64c5>] ? ext4_meta_trans_blocks+0xa5/0xb0
[23500.277186]  [<ffffffff81224a24>] ext4_fallocate+0x1a4/0x530
[23500.277654]  [<ffffffff8117a092>] do_fallocate+0xf2/0x160
[23500.278164]  [<ffffffff8117a14b>] sys_fallocate+0x4b/0x70
[23500.278624]  [<ffffffff815e6d69>] system_call_fastpath+0x16/0x1b
[23500.279124] ---[ end trace 3db7a7a7ae953552 ]---

-- 
Best Regards,
Artem Bityutskiy

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

  parent reply	other threads:[~2012-03-30 15:21 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-03-20 14:41 [PATCH v1 0/9] do not use s_dirt in ext4 Artem Bityutskiy
2012-03-20 14:41 ` [PATCH v1 1/9] ext4: do not mark superblock as dirty unnecessarily Artem Bityutskiy
2012-03-22  9:58   ` Jan Kara
2012-03-20 14:41 ` [PATCH v1 2/9] ext4: write superblock only once on unmount Artem Bityutskiy
2012-03-22  9:59   ` Jan Kara
2012-03-20 14:41 ` [PATCH v1 3/9] ext4: remove useless s_dirt assignment Artem Bityutskiy
2012-03-22 10:02   ` Jan Kara
2012-03-20 14:41 ` [PATCH v1 4/9] mm: export dirty_writeback_interval Artem Bityutskiy
2012-03-20 14:41 ` [PATCH v1 5/9] VFS: remove unused superblock helpers Artem Bityutskiy
2012-03-20 14:41 ` [PATCH v1 6/9] ext4: introduce __ext4_mark_super_dirty Artem Bityutskiy
2012-03-20 14:41 ` [PATCH v1 7/9] ext4: stop using VFS for dirty superblock management Artem Bityutskiy
2012-03-21  8:26   ` Artem Bityutskiy
2012-03-20 14:41 ` [PATCH v1 8/9] ext4: small cleanup in ext4_commit_super Artem Bityutskiy
2012-03-22 10:11   ` Jan Kara
2012-03-20 14:41 ` [PATCH v1 9/9] ext4: introduce own superblock dirty flag Artem Bityutskiy
2012-03-22  9:53 ` [PATCH v1 0/9] do not use s_dirt in ext4 Jan Kara
2012-03-22 10:05   ` Artem Bityutskiy
2012-03-22 10:33     ` Jan Kara
2012-03-22 11:25       ` Artem Bityutskiy
2012-03-22 13:42         ` Jan Kara
2012-03-22 13:59           ` Artem Bityutskiy
2012-03-27 13:29       ` Artem Bityutskiy
2012-03-27 20:14         ` Jan Kara
2012-03-28  8:44           ` Artem Bityutskiy
2012-03-28 10:15             ` Jan Kara
2012-03-30 15:23           ` Artem Bityutskiy [this message]
2012-03-30 15:35             ` Jan Kara
2012-03-30 15:43               ` Artem Bityutskiy
2012-03-31 11:49                 ` Jan Kara
2012-04-02 13:46                   ` Artem Bityutskiy
2012-03-31 12:25               ` Artem Bityutskiy
2012-03-22 13:35 ` Ted Ts'o
2012-03-22 13:56   ` Artem Bityutskiy
2012-03-22 15:06     ` Ted Ts'o
2012-03-23  8:55       ` Artem Bityutskiy
2012-03-23 14:23         ` Ted Ts'o

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1333121035.5440.49.camel@sauron.fi.intel.com \
    --to=dedekind1@gmail.com \
    --cc=jack@suse.cz \
    --cc=linux-ext4@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=tytso@mit.edu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).