Re: [PATCH] Ext3 online resizing locking issue

[Glauber de Oliveira Costa <gocosta@br.ibm.com>]

> 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
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/