On Tue, Nov 09, 2010 at 04:37:40PM +0100, Jan Kara wrote:
> On Thu 28-10-10 03:36:07, Frederic Weisbecker wrote:
> > On Thu, Oct 28, 2010 at 02:28:23AM +0200, Jan Kara wrote:
> > > reiserfs_quota_on() unpacks a tail of quota file in case it has one. But after
> > > BKL conversion, reiserfs_unpack() expects to be called with write_lock held.
> > > So acquire the lock before calling reiserfs_unpack() to avoid assertion
> > > failures.
> > >
> > > Reported-by: markus.gapp@xxxxxxx
> > > Signed-off-by: Jan Kara <jack@xxxxxxx>
> > > ---
> > > fs/reiserfs/super.c | 2 ++
> > > 1 files changed, 2 insertions(+), 0 deletions(-)
> > >
> > > Frederic, would you merge this patch or should I merge it?
> > >
> > > diff --git a/fs/reiserfs/super.c b/fs/reiserfs/super.c
> > > index 6e85cfd..73c000f 100644
> > > --- a/fs/reiserfs/super.c
> > > +++ b/fs/reiserfs/super.c
> > > @@ -2059,7 +2059,9 @@ static int reiserfs_quota_on(struct super_block *sb, int type, int format_id,
> > > inode = path->dentry->d_inode;
> > > /* We must not pack tails for quota files on reiserfs for quota IO to work */
> > > if (!(REISERFS_I(inode)->i_flags & i_nopack_mask)) {
> > > + reiserfs_write_lock(sb);
> > > err = reiserfs_unpack(inode, NULL);
> > > + reiserfs_write_unlock(sb);
> > > if (err) {
> > > reiserfs_warning(sb, "super-6520",
> > > "Unpacking tail of quota file failed"
> > > --
> > > 1.7.1
> > >
> >
> >
> >
> > Yeah. This is due to a recent fix in reiserfs_unpack().
> > And in this fix I assumed reiserfs_unpack() was always called under the
> > reiserfs lock.
> > I was wrong, I thought that reiserfs_quota_on() was ok because it can
> > call joural_begin() which appears to have the same requirements.
> > But no that's probably another bug, journal_begin() should also
> > be called under the reiserfs lock.
> > Anyway that must be another patch.
> >
> >
> > For this specific problem, it might be slightly more proper to do the
> > below. It lowers a bit the reiserfs lock coverage and also fixes
> > a weird lock-unlock ordering in reiserfs_unpack() that was doing:
> >
> > Lock A - Lock B - Unlock A - Unlock B
> >
> > Hmm?
> Looks OK to me. Do you plan to merge it?
Yep, will be sent to Andrew soon.
Thanks!
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
