Jan Kara <jack@xxxxxxx> writes:
> 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.
Ok i'll stick to dquot->dq_flags approach like follows:
int quota_handle_error(struct super_block *sb, struct dquot *dquot)
{
int ret;
ret = test_and_set_bit(_DQUOT_ERROR, &sb_dqopt(sb)->flags);
if (dquot)
ret = test_and_set_bit(DQ_ERROR_B, dquot->dq_flags);
return ret;
}
and use it like this.
if (!quota_handle_error(sb, dquot) || force_dump)
dump_error(...)
>
>> 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...
This result significant paradigm changes, because almost all fs-related
syscalls result int quota alloc/free which return -EIO (i.e in almost
unusable filesystem). Users with incorrect system-scripts will hate us
for this.
>
>> +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
--
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
