linux-ext4.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Ted Ts'o <tytso@mit.edu>
To: Toshiyuki Okajima <toshi.okajima@jp.fujitsu.com>
Cc: adilger.kernel@dilger.ca, Yongqiang Yang <xiaoqiangnk@gmail.com>,
	linux-ext4@vger.kernel.org
Subject: Re: [PATCH 2/2 take2] jbd2: delete spin_lock(t_handle_lock) inside wirte_lock(j_state_lock)
Date: Wed, 4 Jan 2012 22:27:54 -0500	[thread overview]
Message-ID: <20120105032754.GD24494@thunk.org> (raw)
In-Reply-To: <20111226101727.1ca09119.toshi.okajima@jp.fujitsu.com>

On Mon, Dec 26, 2011 at 10:17:27AM +0900, Toshiyuki Okajima wrote:
> Delete all spin_lock(t_handle_lock) statements inside write_lock(j_state_lock) 
> because the critical code sections can be protected by write_lock(j_state_lock)
> only.
> 
> Signed-off-by: Toshiyuki Okajima <toshi.okajima@jp.fujitsu.com>
> Reported-by: Yongqiang Yang <xiaoqiangnk@gmail.com>

This may be true today, but I'm worried this is going to make the code
more fragile.  The header file include/linux/jbd2.h defines which
fields are protected by which lock.  This patch is going to make those
comments partially obsolete.  We need to make sure the locking
protocol is clearly defined before we go around removing locks.

This is something I would like to do, but we need to make sure the
code remains maintainable in the long run.

					- Ted

      parent reply	other threads:[~2012-01-05  3:27 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-12-16 11:19 [PATCH][RFC] jbd2: jbd2_journal_stop needs an exclusive control to synchronize around t_updates operations Toshiyuki Okajima
2011-12-16 12:22 ` Yongqiang Yang
2011-12-20 10:44   ` Toshiyuki Okajima
2011-12-22 11:36     ` [PATCH 0/2 take2][RFC] hangup in jbd2_journal_lock_updates Toshiyuki Okajima
2011-12-22 11:56       ` [PATCH 1/2] jbd2: jbd2_journal_stop needs an exclusive control to synchronize around t_update operations Toshiyuki Okajima
2012-01-03 15:32         ` Jan Kara
2012-01-05  3:12           ` Ted Ts'o
2011-12-22 12:00       ` [PATCH 2/2] jbd2: remove all t_handle_lock statements Toshiyuki Okajima
2011-12-22 14:07         ` Yongqiang Yang
2011-12-26  0:20           ` Toshiyuki Okajima
2011-12-26  1:17             ` [PATCH 2/2 take2] jbd2: delete spin_lock(t_handle_lock) inside wirte_lock(j_state_lock) Toshiyuki Okajima
2011-12-31  6:35               ` Yongqiang Yang
2012-01-03 15:35               ` Jan Kara
2012-01-05  3:27               ` Ted 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=20120105032754.GD24494@thunk.org \
    --to=tytso@mit.edu \
    --cc=adilger.kernel@dilger.ca \
    --cc=linux-ext4@vger.kernel.org \
    --cc=toshi.okajima@jp.fujitsu.com \
    --cc=xiaoqiangnk@gmail.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 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).