Re: [PATCH 1/3] RFC quota: add generic quota error handlers.

[Jan Kara <jack@suse.cz>]

On Wed 31-03-10 13:00:35, Dmitry Monakhov wrote:
> @@ -1023,8 +1022,12 @@ static inline void dquot_resv_space(struct dquot *dquot, qsize_t number)
>   */
>  static void dquot_claim_reserved_space(struct dquot *dquot, qsize_t number)
>  {
> -	if (dquot->dq_dqb.dqb_rsvspace < number) {
> -		WARN_ON_ONCE(1);
> +	if (unlikely(dquot->dq_dqb.dqb_rsvspace < number)) {
> +		quota_error(dquot->dq_sb, "Incorrect quota reservation "
> +			"for quota id:%d, rsvspace:%lld, claim:%lld, "
> +			"thus quota information is probably inconsistent. "
> +			"Please run quotacheck(8).",
> +			dquot->dq_id, dquot->dq_dqb.dqb_rsvspace, number);
>  		number = dquot->dq_dqb.dqb_rsvspace;
>  	}
  We shouldn't spam syslog with errors about quotas (which might happen
once quota gets inconsistent). So we could maybe warn at most once per
dquot structure (by using an error flag in dq_flags) or even once per
superblock + type (by using flags in info) - using info would also have
the advantage that it would be also usable for checks about inodes.

> diff --git a/fs/quota/quota.c b/fs/quota/quota.c
> index 95388f9..ebdce30 100644
> --- a/fs/quota/quota.c
> +++ b/fs/quota/quota.c
> @@ -19,6 +19,41 @@
>  #include <linux/types.h>
>  #include <linux/writeback.h>
>  
> +static void quota_handle_error(struct super_block *sb)
> +{
> +
> +	set_bit(_DQUOT_ERROR, &sb_dqopt(sb)->flags);
> +	/* XXX: Currently it is no impossible to signall fs about error */
> +}
  I guess we can make all quota allocation and freeing functions to return
an error (EIO) if they spot some problem. That should be enough for a
filesystem to find out something is wrong...

> +void __quota_error(struct super_block * sb, const char * function,
> +		const char * fmt, ...)
> +{
> +	va_list args;
> +
> +	va_start(args, fmt);
> +	printk(KERN_CRIT "QUOTA: error (device %s): %s: ",sb->s_id, function);
                                                          ^ space after ','

> +void __quota_warning(struct super_block * sb, const char * func,
> +		const char * fmt, ...)
> +{
> +	va_list args;
> +
> +	va_start(args, fmt);
> +	printk(KERN_WARNING "QUOTA: warning (device %s): %s: ",sb->s_id, func);
                                                               ^ space

> diff --git a/fs/quota/quota_tree.c b/fs/quota/quota_tree.c
> index f81f4bc..da0de94 100644
> --- a/fs/quota/quota_tree.c
> +++ b/fs/quota/quota_tree.c
> @@ -44,7 +44,7 @@ static char *getdqbuf(size_t size)
>  	char *buf = kmalloc(size, GFP_NOFS);
>  	if (!buf)
>  		printk(KERN_WARNING
> -		       "VFS: Not enough memory for quota buffers.\n");
> +		       "VFS: Not enough memory for quota buffers.");
  Why have you removed '\n'?

								Honza
-- 
Jan Kara <jack@suse.cz>
SUSE Labs, CR