On Thu 31-05-12 07:08:41, Greg KH wrote:
> On Wed, May 30, 2012 at 03:44:58PM +0200, Jan Kara wrote:
> > On Wed 30-05-12 14:24:12, Dmitry Monakhov wrote:
> > > If remount failed due to some reason we have to fallback quota to it's
> > > original state
> > > otherwise it will stay in suspended state which is incorrect.
> > >
> > > #TEST_CASE
> > > dd if=/dev/zero of=/tmp/imgfile bs=1M count=100
> > > # any fs with quota are affected
> > > mkfs.ext4 -F /tmp/imgfile
> > > mount /tmp/imgfile /mnt -oloop,quota
> > > quotacheck -cug /mnt
> > > quotaon /mnt
> > > # remount to RO state , and explicitly provide bad option
> > > mount /mnt -oremount,ro,some-bad-option
> > > # At this point fs is in broken state because it is RW, but quota is in
> > > # suspended state. Later activity will trigger NULL pointer dereference
> > > mount /mnt -oremount,ro,some-bad-option
> > >
> > > Bug was silently fixed in: e0ccfd959cd890 v2.6.34-7101-ge0ccfd9, All
> > > previous
> > > kernel versions are affected.
> > Ah, OK. I was wondering for a while how come your test case fails with
> > current kernel ;)
> >
> > > diff --git a/fs/super.c b/fs/super.c
> > > index 1fb63e3..457e70d 100644
> > > --- a/fs/super.c
> > > +++ b/fs/super.c
> > > @@ -607,8 +607,13 @@ int do_remount_sb(struct super_block *sb, int flags,
> > > void *data, int force)
> > >
> > > if (sb->s_op->remount_fs) {
> > > retval = sb->s_op->remount_fs(sb, &flags, data);
> > > - if (retval)
> > > + printk("remount_fs %s ret:%d\n", sb->s_id, retval);
> > This is a debugging leftover isn't it?
> >
> > > + if (retval) {
> > > + /* Remount failed, fallback quota to original state */
> > > + if (remount_ro)
> > > + vfs_dq_quota_on_remount(sb);
> > > return retval;
> > > + }
> > Otherwise the patch looks OK. The issue doesn't seem too serious though,
> > so I'm not sure how eagerly -stable maintainers will take the fix
> > especially given mainline has fixed the problem differently.
>
> Why not just backport the fix that was done in mainline instead?
> Especially as it's only for really old kernels (.34 and older), so not
> many people really care about them :)
Well, I can see a reason why Dmitry didn't backport the upstream fix
(e0ccfd959cd890). That was a reorganization of quota code dependent on
some other changes and all the changes would be far too risky for a -stable
kernel. So using upstream fix is not really viable. Whether maintainers of
2.6.27, 2.6.32, and 2.6.34 -stable trees want to take this patch is upto
them. After removing the debug printk, I can give it my ack.
Honza
--
Jan Kara <[email protected]>
SUSE Labs, CR
--
To unsubscribe from this list: send the line "unsubscribe stable" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
