All of lore.kernel.org
 help / color / mirror / Atom feed
From: Joel Becker <jlbec@evilplan.org>
To: Theodore Ts'o <tytso@mit.edu>
Cc: Younger Liu <younger.liu@huawei.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	linux-ext4@vger.kernel.org,
	Ocfs2-Devel <ocfs2-devel@oss.oracle.com>,
	Li Zefan <lizefan@huawei.com>,
	jack@suse.cz
Subject: Re: [PATCH] fs/jbd2: t_updates should increase when start_this_handle() failed in jbd2__journal_restart()
Date: Sat, 29 Jun 2013 14:22:29 +0100	[thread overview]
Message-ID: <20130629132229.GJ13405@ZenIV.linux.org.uk> (raw)
In-Reply-To: <20130623173628.GC16620@thunk.org>

On Sun, Jun 23, 2013 at 01:36:28PM -0400, Theodore Ts'o wrote:
> On Fri, Jun 21, 2013 at 09:29:31PM +0800, Younger Liu wrote:
> > 
> > This bug was triggered by the following scenario:
> > In ocfs2 file system, allocate a very large disk space for a small file
> > with ocfs2_fallocate(), while the journal file size is 32M. 
> > 
> > Because there are much many journal blocks needed by jbd2_journal_restart(), 
> > so that nblocks is greater than journal->j_max_transaction_buffers 
> > in start_this_handle(), and then return -ENOSPC.
> 
> Ah, I see.  I have a patch that should prevent the kernel from
> crashing in this situation, and which adds some additional checks to
> make sure no one tries to use the handle after jbd2_journal_restart()
> fails in this circumstance.
> 
> However, you may want to further pursue a fix in ocfs2 so you don't
> actually return ENOSPC to userspace, since it is a very misleading
> error message --- it's not that the file system is out of space, but
> that the journal is too small for the amount of space that you are
> trying to allocate using fallocate().
> 
> I would think a better way of handling this situation would be to log
> a warning message that the journal is probably too small, and then to
> break up the fallocate into smaller chunks, so that it can
> successfully complete despite the fact that the journal was
> unfortunately missized.

	Yes, this solution is a good one.

Joel

> 
> I'll be sending the proposed fix in a moment; could you check and see
> if the patch prevents ocfs2/jbd2 from tripping over the assertion
> given your test case?
> 
> Thanks,
> 
> 					- 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

-- 

WARNING: multiple messages have this Message-ID (diff)
From: Joel Becker <jlbec@evilplan.org>
To: Theodore Ts'o <tytso@mit.edu>
Cc: Younger Liu <younger.liu@huawei.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	linux-ext4@vger.kernel.org,
	Ocfs2-Devel <ocfs2-devel@oss.oracle.com>,
	Li Zefan <lizefan@huawei.com>,
	jack@suse.cz
Subject: [Ocfs2-devel] [PATCH] fs/jbd2: t_updates should increase when start_this_handle() failed in jbd2__journal_restart()
Date: Sat, 29 Jun 2013 14:22:29 +0100	[thread overview]
Message-ID: <20130629132229.GJ13405@ZenIV.linux.org.uk> (raw)
In-Reply-To: <20130623173628.GC16620@thunk.org>

On Sun, Jun 23, 2013 at 01:36:28PM -0400, Theodore Ts'o wrote:
> On Fri, Jun 21, 2013 at 09:29:31PM +0800, Younger Liu wrote:
> > 
> > This bug was triggered by the following scenario:
> > In ocfs2 file system, allocate a very large disk space for a small file
> > with ocfs2_fallocate(), while the journal file size is 32M. 
> > 
> > Because there are much many journal blocks needed by jbd2_journal_restart(), 
> > so that nblocks is greater than journal->j_max_transaction_buffers 
> > in start_this_handle(), and then return -ENOSPC.
> 
> Ah, I see.  I have a patch that should prevent the kernel from
> crashing in this situation, and which adds some additional checks to
> make sure no one tries to use the handle after jbd2_journal_restart()
> fails in this circumstance.
> 
> However, you may want to further pursue a fix in ocfs2 so you don't
> actually return ENOSPC to userspace, since it is a very misleading
> error message --- it's not that the file system is out of space, but
> that the journal is too small for the amount of space that you are
> trying to allocate using fallocate().
> 
> I would think a better way of handling this situation would be to log
> a warning message that the journal is probably too small, and then to
> break up the fallocate into smaller chunks, so that it can
> successfully complete despite the fact that the journal was
> unfortunately missized.

	Yes, this solution is a good one.

Joel

> 
> I'll be sending the proposed fix in a moment; could you check and see
> if the patch prevents ocfs2/jbd2 from tripping over the assertion
> given your test case?
> 
> Thanks,
> 
> 					- Ted
> --
> To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 

  parent reply	other threads:[~2013-06-29 13:22 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-06-19  4:48 [PATCH] fs/jbd2: t_updates should increase when start_this_handle() failed in jbd2__journal_restart() Younger Liu
2013-06-19  4:48 ` [Ocfs2-devel] " Younger Liu
2013-06-20 15:55 ` Theodore Ts'o
2013-06-20 15:55   ` Theodore Ts'o
2013-06-20 15:55   ` [Ocfs2-devel] " Theodore Ts'o
2013-06-20 16:08   ` [PATCH] jbd2: fix theoretical race in jbd2__journal_restart Theodore Ts'o
2013-06-20 16:08     ` [Ocfs2-devel] " Theodore Ts'o
2013-06-20 17:26   ` [PATCH] fs/jbd2: t_updates should increase when start_this_handle() failed in jbd2__journal_restart() Josef Bacik
2013-06-20 17:26     ` [Ocfs2-devel] " Josef Bacik
2013-06-20 18:12     ` Theodore Ts'o
2013-06-20 18:12       ` [Ocfs2-devel] " Theodore Ts'o
2013-06-21 23:26       ` Jan Kara
2013-06-21 23:26         ` [Ocfs2-devel] " Jan Kara
2013-06-21 13:29   ` Younger Liu
2013-06-21 13:29     ` [Ocfs2-devel] " Younger Liu
2013-06-23 17:36     ` Theodore Ts'o
2013-06-23 17:36       ` [Ocfs2-devel] " Theodore Ts'o
2013-06-23 17:44       ` [PATCH] jbd2: invalidate handle if jbd2_journal_restart() fails Theodore Ts'o
2013-06-24  9:53         ` Jan Kara
2013-06-25  9:42         ` Younger Liu
2013-06-29 23:46           ` Theodore Ts'o
2013-07-03 12:22             ` Younger Liu
2013-07-03 12:36               ` Younger Liu
2013-06-25  8:30       ` [PATCH] fs/jbd2: t_updates should increase when start_this_handle() failed in jbd2__journal_restart() Younger Liu
2013-06-25  8:30         ` [Ocfs2-devel] " Younger Liu
2013-06-29 13:22       ` Joel Becker [this message]
2013-06-29 13:22         ` Joel Becker

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=20130629132229.GJ13405@ZenIV.linux.org.uk \
    --to=jlbec@evilplan.org \
    --cc=akpm@linux-foundation.org \
    --cc=jack@suse.cz \
    --cc=linux-ext4@vger.kernel.org \
    --cc=lizefan@huawei.com \
    --cc=ocfs2-devel@oss.oracle.com \
    --cc=tytso@mit.edu \
    --cc=younger.liu@huawei.com \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.