From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jan Kara Subject: Re: [PATCH] quota: add per-inode reservaton space sanity checks. Date: Wed, 31 Mar 2010 16:29:41 +0200 Message-ID: <20100331142940.GA3322@quack.suse.cz> References: <1269959128-9626-1-git-send-email-dmonakhov@openvz.org> <20100330153933.GC3424@quack.suse.cz> <87ljd9dbnz.fsf@openvz.org> <87aatpcbji.fsf@openvz.org> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: Jan Kara , linux-fsdevel@vger.kernel.org To: Dmitry Monakhov Return-path: Received: from cantor.suse.de ([195.135.220.2]:37468 "EHLO mx1.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751770Ab0CaO3f (ORCPT ); Wed, 31 Mar 2010 10:29:35 -0400 Content-Disposition: inline In-Reply-To: <87aatpcbji.fsf@openvz.org> Sender: linux-fsdevel-owner@vger.kernel.org List-ID: On Wed 31-03-10 09:20:17, Dmitry Monakhov wrote: > dmonakhov@openvz.org writes: > > > Jan Kara writes: > > > >> On Tue 30-03-10 18:25:28, Dmitry Monakhov wrote: > >>> We already has per-dquot sanity checks, but with per-inode checks > >>> quota leakage investigation becomes much easier. > >>> > >>> Signed-off-by: Dmitry Monakhov > >>> --- > >>> fs/quota/dquot.c | 4 ++++ > >>> 1 files changed, 4 insertions(+), 0 deletions(-) > >>> > >>> diff --git a/fs/quota/dquot.c b/fs/quota/dquot.c > >>> index e0b870f..4db57b7 100644 > >>> --- a/fs/quota/dquot.c > >>> +++ b/fs/quota/dquot.c > >>> @@ -1428,6 +1428,8 @@ EXPORT_SYMBOL(inode_add_rsv_space); > >>> void inode_claim_rsv_space(struct inode *inode, qsize_t number) > >>> { > >>> spin_lock(&inode->i_lock); > >>> + if (*inode_reserved_space(inode) < number) > >>> + WARN_ON_ONCE(1); > >> Maybe just: WARN_ON_ONCE(*inode_reserved_space(inode) < number) > Ohh. While writing testcase for automatic-quota consistency check. > I've failed to reliably detect condition where quota goes inconsistent. > And it appears to be not what easy. We have hided real problems too deeply. > The only reliable way is to grep dmesg for hard-coded "fs/quota/dquot.c" > string which produced by WARN_ON_ONCE. > > After some thoughts (sometimes it happens with me too) > i'm think what it is reasonable rewrite all quota error messages logic. > Make it similar to ext4 > quota_error(...) { > printk("QUOTA error (dev:%s)", ...); > handle_quota_error(sb) /* let sb to know what quota is corrupted */ > } > quota_warn(...) { > printk("QUOTA warn (dev:%s)", ...); > } > In fact it is very important to let filesystem to now that it's quota > is corrupted(quota == fs-metadata). It may set fsck_needed flag on > super block for later correction. Yes, it looks like a good idea. > So please ignore this patch for now. I'll prepare another one which > redesign error conditions handling logic. OK. Honza -- Jan Kara SUSE Labs, CR