All of lore.kernel.org
 help / color / mirror / Atom feed
From: Toshiyuki Okajima <toshi.okajima@jp.fujitsu.com>
To: Theodore Tso <tytso@mit.edu>
Cc: aneesh.kumar@linux.vnet.ibm.com, balbir@linux.vnet.ibm.com,
	linux-ext4@vger.kernel.org
Subject: Re: [BUG][PATCH 1/4] ext3: fix a cause of __schedule_bug via	blkdev_releasepage
Date: Fri, 12 Dec 2008 09:54:18 +0900	[thread overview]
Message-ID: <4941B63A.3090302@jp.fujitsu.com> (raw)
In-Reply-To: <20081208140138.GA17700@mit.edu>

Ted-san,
Thank you for your reviewing.

 > Toshiyuki-san,
 >
 > My apologies for not having a chance to review your patches; things
 > have been rather busy for me.  There were a couple of shortcomings in
 > your patch series, and I think there is a better way of solving the
 > issue at hand.  First of all, patches #1 and #2 use a new function
 > which is not actually defined until patches #3 and #4, respectively.
 > This can make it difficult for people who are trying to use "git
 > bisect" to try to localize the root cause of a problem.
 >
 > Secondly, the introduction of a large number of wrapper functions
 > increases stack utilization, and makes the call depth deeper (and in
 > the long run, each increase in call depth makes the code that much
 > harder to trace through and understand), and so it should be done only
 > as last resort.  Fortunately, there is a simpler way of fixing this
 > problem.  I include the inter-diff below, but I will fold this into
 > the current blkdev_releasepage() patches that are in the ext4 patch
 > queue.
 >
 > Best regards,
 >
 > 					- Ted
 >
 > P.S.  Note that this patch is functionally identical to what you
 > proposed in your patch series, but since the gfp_wait mask already
 > controlls whether or not log_wait_commit() is called, instead of
 > introducing a new functional parameter, we just mask off __GFP_WAIT
 > before calling jbd2_journal_try_to_free_buffers.  I'll make a similar
 > change to the ext3 patch, and attach the two revised patches to this
 > mail thread.

Through the idea as follows,  I agree to your change.

-----------------------------------------------------------------------------
To tell the truth, at first, I imagined the same patch as yours to fix this
problem. But I have made another patch because I thought that ext3(or ext4)
should not know the contents of the processing of journal_try_to_free_buffers
  in detail. (ext3 should not know there is a possibility to call
journal_wait_for_transaction_sync_data from journal_try_to_free_buffers.)
So, I have made a new function, journal_try_to_free_metadata_buffers
to release only metadata buffer_heads.
(I wanted a function which is the almost same as journal_try_to_free_buffers
except calling journal_wait_for_transaction_sync_data from it.)
However, this new function needed big changes, you know.

I reconsidered what was the most suitable patch to fix this problem
after I read your mail (patch).
And then, I thought that it was important to make the most concise patch
to fix only a root cause. Big patch is not easy to understand even if it is
more logical one.

Therefore, there is the fact that ext3_release_metadata must not sleep because
it can get a spinlock, and then, only changing ext3_release_metadata to the
logic to make it not sleep is the simplest fix for this problem.
-----------------------------------------------------------------------------

Best Regards,
Toshiyuki Okajima


  parent reply	other threads:[~2008-12-12  0:54 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-12-02 11:06 [BUG][PATCH 1/4] ext3: fix a cause of __schedule_bug via blkdev_releasepage Toshiyuki Okajima
2008-12-08 14:01 ` Theodore Tso
2008-12-08 14:06   ` [PATCH -V2] ext3: provide function to release metadata pages under memory pressure Theodore Ts'o
2008-12-08 14:06     ` [PATCH -V2] ext4: " Theodore Ts'o
2008-12-12  0:54   ` Toshiyuki Okajima [this message]
2008-12-12  6:21     ` [BUG][PATCH 1/4] ext3: fix a cause of __schedule_bug via blkdev_releasepage Theodore Tso
2008-12-12 17:52       ` [PATCH -v3] vfs: add releasepages hooks to block devices which can be used by file systems Theodore Ts'o
2008-12-12 17:52         ` [PATCH -v3] ext3: provide function to release metadata pages under memory pressure Theodore Ts'o
2008-12-12 17:52           ` [PATCH -v3] ext4: " Theodore Ts'o
2008-12-17 15:39         ` [PATCH -v3] vfs: add releasepages hooks to block devices which can be used by file systems Jan Kara
2008-12-18  5:15           ` Toshiyuki Okajima
2008-12-18 13:12             ` Jan Kara
2008-12-18 14:54               ` Theodore Tso
2008-12-18 16:38                 ` Jan Kara
2008-12-19  5:15               ` Toshiyuki Okajima
2008-12-26  5:01         ` Al Viro
2009-01-03 15:09           ` Theodore Ts'o
2009-01-03 15:09             ` [PATCH 1/3] add releasepage " Theodore Ts'o
2009-01-03 15:09               ` [PATCH 2/3] ext3: provide function to release metadata pages under memory pressure Theodore Ts'o
2009-01-03 15:09                 ` [PATCH 3/3] ext4: " Theodore Ts'o
2009-01-05  8:16               ` [PATCH 1/3] add releasepage hooks to block devices which can be used by file systems Toshiyuki Okajima
2009-01-05 16:05                 ` Theodore Tso
2009-01-06  4:07                   ` Toshiyuki Okajima
2009-01-06  4:29                     ` Theodore Tso
2008-12-15  2:21       ` [BUG][PATCH 1/4] ext3: fix a cause of __schedule_bug via blkdev_releasepage Toshiyuki Okajima

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=4941B63A.3090302@jp.fujitsu.com \
    --to=toshi.okajima@jp.fujitsu.com \
    --cc=aneesh.kumar@linux.vnet.ibm.com \
    --cc=balbir@linux.vnet.ibm.com \
    --cc=linux-ext4@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 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.