All of lore.kernel.org
 help / color / mirror / Atom feed
From: Theodore Ts'o <tytso@mit.edu>
To: Xiaoguang Wang <wangxg.fnst@cn.fujitsu.com>
Cc: linux-ext4@vger.kernel.org
Subject: Re: [PATCH] ext4: issue WARN_ON_ONCE() if page gets truncated in ext4_write_[da_]begin()
Date: Sun, 17 May 2015 13:51:18 -0400	[thread overview]
Message-ID: <20150517175118.GI4489@thunk.org> (raw)
In-Reply-To: <1430990243-8568-1-git-send-email-wangxg.fnst@cn.fujitsu.com>

On Thu, May 07, 2015 at 05:17:23PM +0800, Xiaoguang Wang wrote:
> In ext4_write_[da_]begin(), when we get a page successfully by calling
> grab_cache_page_write_begin(), then unless this page is truncated by
> ext4_truncate_failed_write(), which is called when ext4_write_[da_]begin()
> run into some errors, otherwise I think this page won't be truncated by
> other kernel path because we're holding i_mutex.
> 
> In this patch, if page is truncated by ext4_truncate_failed_write(), then
> we just put it and call grab_cache_page_write_begin() again to get a new
> page, then this check 'if (page->mapping != mapping)' won't be necessary.
> We can remove this check, but according to Jan Kara's suggestion, we issue
> WARN_ON_ONCE() here if page is truncated, that means something is wrong.
> (Thanks to Jan Kara for his help)

It would be helpful if the commit description explained why this
change is a benefit.  There's a very detailed explanation about why
it's safe, but why do we need to release the page only to try again to
grab it by jumping to retry_grab instead of retry_journal?  What
problem are you trying to solve?

Thanks, regards,

					- Ted

      reply	other threads:[~2015-05-17 17:51 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-05-07  9:17 [PATCH] ext4: issue WARN_ON_ONCE() if page gets truncated in ext4_write_[da_]begin() Xiaoguang Wang
2015-05-17 17:51 ` Theodore Ts'o [this message]

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=20150517175118.GI4489@thunk.org \
    --to=tytso@mit.edu \
    --cc=linux-ext4@vger.kernel.org \
    --cc=wangxg.fnst@cn.fujitsu.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.