linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Glauber de Oliveira Costa <gocosta@br.ibm.com>
To: "Stephen C. Tweedie" <sct@redhat.com>
Cc: Glauber de Oliveira Costa <gocosta@br.ibm.com>,
	"ext2-devel@lists.sourceforge.net"
	<ext2-devel@lists.sourceforge.net>,
	ext2resize-devel@lists.sourceforge.net,
	linux-kernel <linux-kernel@vger.kernel.org>,
	linux-fsdevel@vger.kernel.org,
	Andreas Dilger <adilger@clusterfs.com>,
	Andrew Morton <akpm@osdl.org>,
	Al Viro <viro@parcelfarce.linux.theplanet.co.uk>
Subject: Re: [PATCH] Ext3 online resizing locking issue
Date: Thu, 25 Aug 2005 17:43:35 -0300	[thread overview]
Message-ID: <20050825204335.GA1674@br.ibm.com> (raw)
In-Reply-To: <1124996561.1884.212.camel@sisko.sctweedie.blueyonder.co.uk>

 
> NAK, this is wrong:
> 
> > +		lock_super(sb);
> >  		err = ext3_group_extend(sb, EXT3_SB(sb)->s_es, n_blocks_count);
> > +		unlock_super(sb);
> 
> This basically reverses the order of locking between lock_super() and
> journal_start() (the latter acts like a lock because it can block on a
> resource if the journal is too full for the new transaction.)  That's
> the opposite order to normal, and will result in a potential deadlock.
> 
Ooops! Missed that. But I agree with the point. 

 
> But the _right_ fix, if you really want to keep that code, is probably
> to move all the resize locking to a separate lock that ranks outside the
> journal_start.  The easy workaround is to drop the superblock lock and
> reaquire it around the journal_start(); it would be pretty easy to make
> that work robustly as far as ext3 is concerned, but I suspect there may
> be VFS-layer problems if we start dropping the superblock lock in the
> middle of the s_ops->remount() call --- Al?
> 

Just a question here. With s_lock held by the remount code, we're
altering the struct super_block, and believing we're safe. We try to
acquire it inside the resize functions, because we're trying to modify 
this same data. Thus, if we rely on another lock, aren't we probably 
messing  up something ? (for example, both group_extend and remount code 
potentially modify s_flags field. If we ioctl and remount at the same time, 
each one with a different lock, something could go wrong). Am I missing
something here ? 

glauber

  reply	other threads:[~2005-08-25 20:41 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2005-08-24 21:03 [PATCH] Ext3 online resizing locking issue Glauber de Oliveira Costa
2005-08-25 19:02 ` Stephen C. Tweedie
2005-08-25 20:43   ` Glauber de Oliveira Costa [this message]
2005-08-30 14:06     ` Stephen C. Tweedie
2005-08-31 11:35       ` Glauber de Oliveira Costa
2005-08-31 13:30         ` Stephen C. Tweedie
2005-09-01 23:04           ` [PATCH][RFC] Ext3 online resizing locking issue (Again) Glauber de Oliveira Costa

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=20050825204335.GA1674@br.ibm.com \
    --to=gocosta@br.ibm.com \
    --cc=adilger@clusterfs.com \
    --cc=akpm@osdl.org \
    --cc=ext2-devel@lists.sourceforge.net \
    --cc=ext2resize-devel@lists.sourceforge.net \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=sct@redhat.com \
    --cc=viro@parcelfarce.linux.theplanet.co.uk \
    /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).