Jan Kara wrote:
>> Because we can badly over-reserve metadata when we
>> calculate worst-case, it complicates things for quota, since
>> we must reserve and then claim later, retry on EDQUOT, etc.
>> Quota is also a generally smaller pool than fs free blocks,
>> so this over-reservation hurts more, and more often.
> Yes, it's kind of nasty...
>
>> I'm of the opinion that it's not the worst thing to allow
>> metadata to push a user slightly over quota, if we can simplify
>> the code by doing so.
> Well, the code simplification isn't be such a big deal in this particular
> case at least for me - the false failures / warnings seem like a worse
> problem to me.
Well, good point. :)
>> In the longer run I'd like to even consider not charging
>> speculative metadata against the superblock counters, and use
>> a reserved space pool similar to what XFS does, but this change
>> could maybe stand on its own, too.
> This is going to be tough because you just cannot afford to fail
> an allocation as soon as you've accepted data to write. So you have to
> do some computations anyway to make sure you can always find space
> for metadata.
>
>> diff --git a/fs/ext4/balloc.c b/fs/ext4/balloc.c
>> index 22bc743..e11e5b0 100644
>> --- a/fs/ext4/balloc.c
>> +++ b/fs/ext4/balloc.c
>> @@ -604,13 +604,14 @@ ext4_fsblk_t ext4_new_meta_blocks(handle_t *handle, struct inode *inode,
>> ret = ext4_mb_new_blocks(handle, &ar, errp);
>> if (count)
>> *count = ar.len;
>> -
>> /*
>> - * Account for the allocated meta blocks
>> + * Account for the allocated meta blocks. We will never
>> + * fail EDQUOT for metdata, but we do account for it.
>> */
>> if (!(*errp) && EXT4_I(inode)->i_delalloc_reserved_flag) {
>> spin_lock(&EXT4_I(inode)->i_block_reservation_lock);
>> EXT4_I(inode)->i_allocated_meta_blocks += ar.len;
>> + vfs_dq_alloc_block(inode, ar.len);
> This won't work - vfs_dq_alloc_block just refuses to allocate blocks
> if you'd go over limit. You probably have to introduce new flag to
Oh, missed that. Bummer, ok.
> the function to don't do limit checks... BTW: Christoph Hellwig and Dmitry
> are doing some quota cleanups which change the interface so it'd be better
> if you based quota patches on top of my linux-fs-2.6 tree.
OK
> Also you shouldn't call vfs_dq_alloc_block inside a spinlock since it
> can sleep.
Oh, oops, I guess I knew that ...
> Otherwise the patch looks fine, although all the counter stuff looks messy
> so I'm not so sure I didn't miss some subtlety.
The counter stuff is still messy because the superblock needs it -
if that's the messiness you refer to ...
Thanks,
-Eric
> Honza
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
