* Linux 2.4 quota (accounting?) bug ... @ 2003-10-25 16:26 Herbert Poetzl 2003-10-25 16:31 ` Jan Kara 0 siblings, 1 reply; 5+ messages in thread From: Herbert Poetzl @ 2003-10-25 16:26 UTC (permalink / raw) To: Jan Kara; +Cc: linux-kernel, Alex Lyashkov Hi Honza! a friend of mine, made me aware of the following imbalance, which looks like a minor accounting bug to me, but might be a quota bug ... fs/dquot.c : 394 vfs_quota_sync() ----------------------------------------------------- /* Get reference to quota so it won't be invalidated. get_dquot_ref() * is enough since if dquot is locked/modified it can't be * on the free list */ > get_dquot_ref(dquot); if (dquot->dq_flags & DQ_LOCKED) wait_on_dquot(dquot); if (dquot_dirty(dquot)) sb->dq_op->sync_dquot(dquot); > dqput(dquot); goto restart; } best, Herbert ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: Linux 2.4 quota (accounting?) bug ... 2003-10-25 16:26 Linux 2.4 quota (accounting?) bug Herbert Poetzl @ 2003-10-25 16:31 ` Jan Kara 2003-10-25 17:42 ` Herbert Poetzl 0 siblings, 1 reply; 5+ messages in thread From: Jan Kara @ 2003-10-25 16:31 UTC (permalink / raw) To: linux-kernel, Alex Lyashkov Hi, > a friend of mine, made me aware of the following > imbalance, which looks like a minor accounting bug > to me, but might be a quota bug ... Sorry but the code seems correct to me - we get reference to dquot by get_dquot_ref() and than we put the reference by dqput(). dqput() is correct because something nasty might happen in the mean time and so we might be the last holders of the dquot. What do you think is wrong? Honza > > fs/dquot.c : 394 vfs_quota_sync() > ----------------------------------------------------- > /* Get reference to quota so it won't be invalidated. get_dquot_ref() > * is enough since if dquot is locked/modified it can't be > * on the free list */ > > > get_dquot_ref(dquot); > if (dquot->dq_flags & DQ_LOCKED) > wait_on_dquot(dquot); > if (dquot_dirty(dquot)) > sb->dq_op->sync_dquot(dquot); > > dqput(dquot); > goto restart; > } > > > best, > Herbert > > -- Jan Kara <jack@suse.cz> SuSE CR Labs ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: Linux 2.4 quota (accounting?) bug ... 2003-10-25 16:31 ` Jan Kara @ 2003-10-25 17:42 ` Herbert Poetzl 2003-10-28 23:40 ` Jan Kara 2003-11-05 20:36 ` Jan Kara 0 siblings, 2 replies; 5+ messages in thread From: Herbert Poetzl @ 2003-10-25 17:42 UTC (permalink / raw) To: Jan Kara; +Cc: linux-kernel, Alex Lyashkov On Sat, Oct 25, 2003 at 06:31:28PM +0200, Jan Kara wrote: > Hi, > > > a friend of mine, made me aware of the following > > imbalance, which looks like a minor accounting bug > > to me, but might be a quota bug ... > Sorry but the code seems correct to me - we get reference to dquot by > get_dquot_ref() and than we put the reference by dqput(). dqput() is > correct because something nasty might happen in the mean time and so we > might be the last holders of the dquot. What do you think is wrong? dqput() does dqstats.drops++; which isn't correct if this should be the same as put_dquot_ref(), but maybe I'm just irritated by strange statistics on some kernels showing more drops than lookups+allocated after sync/quotaoff best, Herbert > Honza > > > > > fs/dquot.c : 394 vfs_quota_sync() > > ----------------------------------------------------- > > /* Get reference to quota so it won't be invalidated. get_dquot_ref() > > * is enough since if dquot is locked/modified it can't be > > * on the free list */ > > > > > get_dquot_ref(dquot); > > if (dquot->dq_flags & DQ_LOCKED) > > wait_on_dquot(dquot); > > if (dquot_dirty(dquot)) > > sb->dq_op->sync_dquot(dquot); > > > dqput(dquot); > > goto restart; > > } > > > > > > best, > > Herbert > > > > > -- > Jan Kara <jack@suse.cz> > SuSE CR Labs > - > 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/ ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: Linux 2.4 quota (accounting?) bug ... 2003-10-25 17:42 ` Herbert Poetzl @ 2003-10-28 23:40 ` Jan Kara 2003-11-05 20:36 ` Jan Kara 1 sibling, 0 replies; 5+ messages in thread From: Jan Kara @ 2003-10-28 23:40 UTC (permalink / raw) To: Jan Kara, linux-kernel, Alex Lyashkov Hi, > On Sat, Oct 25, 2003 at 06:31:28PM +0200, Jan Kara wrote: > > Hi, > > > > > a friend of mine, made me aware of the following > > > imbalance, which looks like a minor accounting bug > > > to me, but might be a quota bug ... > > Sorry but the code seems correct to me - we get reference to dquot by > > get_dquot_ref() and than we put the reference by dqput(). dqput() is > > correct because something nasty might happen in the mean time and so we > > might be the last holders of the dquot. What do you think is wrong? > > dqput() does dqstats.drops++; > which isn't correct if this should be the same as > put_dquot_ref(), but maybe I'm just irritated by > strange statistics on some kernels showing more > drops than lookups+allocated after sync/quotaoff Oh, now I see... At first I didn't understand that the problem is in the quota statistics. I'll fix that. Honza ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: Linux 2.4 quota (accounting?) bug ... 2003-10-25 17:42 ` Herbert Poetzl 2003-10-28 23:40 ` Jan Kara @ 2003-11-05 20:36 ` Jan Kara 1 sibling, 0 replies; 5+ messages in thread From: Jan Kara @ 2003-11-05 20:36 UTC (permalink / raw) To: linux-kernel, Alex Lyashkov; +Cc: Marcelo Tosatti, Herbert Poetzl Hi, > On Sat, Oct 25, 2003 at 06:31:28PM +0200, Jan Kara wrote: > > Hi, > > > > > a friend of mine, made me aware of the following > > > imbalance, which looks like a minor accounting bug > > > to me, but might be a quota bug ... > > Sorry but the code seems correct to me - we get reference to dquot by > > get_dquot_ref() and than we put the reference by dqput(). dqput() is > > correct because something nasty might happen in the mean time and so we > > might be the last holders of the dquot. What do you think is wrong? > > dqput() does dqstats.drops++; > which isn't correct if this should be the same as > put_dquot_ref(), but maybe I'm just irritated by > strange statistics on some kernels showing more > drops than lookups+allocated after sync/quotaoff So I finally got to fixing the bug. The patch (also with a deletition of unnecessary variable) is attached. Marcello please apply. Honza ---------------------cut here------------------------------------------- diff -ruX ../kerndiffexclude linux-2.4.22-um/fs/dquot.c linux-2.4.22-fixstat/fs/dquot.c --- linux-2.4.22-um/fs/dquot.c Mon Aug 25 13:44:43 2003 +++ linux-2.4.22-fixstat/fs/dquot.c Wed Nov 5 21:21:58 2003 @@ -392,6 +392,7 @@ * is enough since if dquot is locked/modified it can't be * on the free list */ get_dquot_ref(dquot); + dqstats.lookups++; if (dquot->dq_flags & DQ_LOCKED) wait_on_dquot(dquot); if (dquot_dirty(dquot)) @@ -622,7 +623,6 @@ dqput(dquot); return NODQUOT; } - ++dquot->dq_referenced; dqstats.lookups++; return dquot; @@ -642,7 +642,6 @@ if (dquot->dq_flags & DQ_LOCKED) printk(KERN_ERR "VFS: dqduplicate(): Locked quota to be duplicated!\n"); get_dquot_dup_ref(dquot); - dquot->dq_referenced++; dqstats.lookups++; return dquot; Only in linux-2.4.22-fixstat/include/linux: modules diff -ruX ../kerndiffexclude linux-2.4.22-um/include/linux/quota.h linux-2.4.22-fixstat/include/linux/quota.h --- linux-2.4.22-um/include/linux/quota.h Mon Aug 25 13:44:44 2003 +++ linux-2.4.22-fixstat/include/linux/quota.h Wed Nov 5 21:27:44 2003 @@ -222,8 +222,6 @@ loff_t dq_off; /* Offset of dquot on disk */ short dq_type; /* Type of quota */ short dq_flags; /* See DQ_* */ - unsigned long dq_referenced; /* Number of times this dquot was - referenced during its lifetime */ struct mem_dqblk dq_dqb; /* Diskquota usage */ }; ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2003-11-05 20:36 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2003-10-25 16:26 Linux 2.4 quota (accounting?) bug Herbert Poetzl 2003-10-25 16:31 ` Jan Kara 2003-10-25 17:42 ` Herbert Poetzl 2003-10-28 23:40 ` Jan Kara 2003-11-05 20:36 ` Jan Kara
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).