From: Jan Kara Subject: Re: [PATCH, RFC] ext4: don't use quota reservation for speculative metadata blocks Date: Wed, 24 Feb 2010 14:29:47 +0100 Message-ID: <20100224132947.GA18470@atrey.karlin.mff.cuni.cz> References: <4B846241.7050205@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: ext4 development To: Eric Sandeen Return-path: Received: from ksp.mff.cuni.cz ([195.113.26.206]:38781 "EHLO atrey.karlin.mff.cuni.cz" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1757029Ab0BXN3t (ORCPT ); Wed, 24 Feb 2010 08:29:49 -0500 Content-Disposition: inline In-Reply-To: <4B846241.7050205@redhat.com> Sender: linux-ext4-owner@vger.kernel.org List-ID: > 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. > 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 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. Also you shouldn't call vfs_dq_alloc_block inside a spinlock since it can sleep. Otherwise the patch looks fine, although all the counter stuff looks messy so I'm not so sure I didn't miss some subtlety. Honza -- Jan Kara SuSE CR Labs