allocated_meta_data is already included in 'used' variable.
Signed-off-by: Dmitry Monakhov <[email protected]>
---
fs/ext4/inode.c | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index bec222c..bf989fb 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -1110,7 +1110,8 @@ void ext4_da_update_reserve_space(struct inode *inode,
*/
if (allocated_meta_blocks)
dquot_claim_block(inode, allocated_meta_blocks);
- dquot_release_reservation_block(inode, mdb_free + used);
+ dquot_release_reservation_block(inode, mdb_free + used -
+ allocated_meta_blocks);
}
/*
--
1.6.6.1
Dmitry Monakhov <[email protected]> writes:
> allocated_meta_data is already included in 'used' variable.
Since 2.6.33 is also affected the patch have to be pushed to
stable IMHO.
>
> Signed-off-by: Dmitry Monakhov <[email protected]>
> ---
> fs/ext4/inode.c | 3 ++-
> 1 files changed, 2 insertions(+), 1 deletions(-)
>
> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> index bec222c..bf989fb 100644
> --- a/fs/ext4/inode.c
> +++ b/fs/ext4/inode.c
> @@ -1110,7 +1110,8 @@ void ext4_da_update_reserve_space(struct inode *inode,
> */
> if (allocated_meta_blocks)
> dquot_claim_block(inode, allocated_meta_blocks);
> - dquot_release_reservation_block(inode, mdb_free + used);
> + dquot_release_reservation_block(inode, mdb_free + used -
> + allocated_meta_blocks);
> }
>
> /*
On Tue, 30 Mar 2010 18:24:35 +0400, Dmitry Monakhov <[email protected]> wrote:
> allocated_meta_data is already included in 'used' variable.
>
> Signed-off-by: Dmitry Monakhov <[email protected]>
> ---
> fs/ext4/inode.c | 3 ++-
> 1 files changed, 2 insertions(+), 1 deletions(-)
>
> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> index bec222c..bf989fb 100644
> --- a/fs/ext4/inode.c
> +++ b/fs/ext4/inode.c
> @@ -1110,7 +1110,8 @@ void ext4_da_update_reserve_space(struct inode *inode,
> */
> if (allocated_meta_blocks)
> dquot_claim_block(inode, allocated_meta_blocks);
> - dquot_release_reservation_block(inode, mdb_free + used);
> + dquot_release_reservation_block(inode, mdb_free + used -
> + allocated_meta_blocks);
> }
>
> /*
Do we really need to do this ? IIUC reservation count and actual
allocated count are two different. One block allocation we need to
remove all the blocks reserved from the reservation count and add
actually allocated blocks to the allocated count.
-aneesh
"Aneesh Kumar K. V" <[email protected]> writes:
> On Tue, 30 Mar 2010 18:24:35 +0400, Dmitry Monakhov <[email protected]> wrote:
>> allocated_meta_data is already included in 'used' variable.
>>
>> Signed-off-by: Dmitry Monakhov <[email protected]>
>> ---
>> fs/ext4/inode.c | 3 ++-
>> 1 files changed, 2 insertions(+), 1 deletions(-)
>>
>> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
>> index bec222c..bf989fb 100644
>> --- a/fs/ext4/inode.c
>> +++ b/fs/ext4/inode.c
>> @@ -1110,7 +1110,8 @@ void ext4_da_update_reserve_space(struct inode *inode,
>> */
>> if (allocated_meta_blocks)
>> dquot_claim_block(inode, allocated_meta_blocks);
>> - dquot_release_reservation_block(inode, mdb_free + used);
>> + dquot_release_reservation_block(inode, mdb_free + used -
>> + allocated_meta_blocks);
>> }
>>
>> /*
>
> Do we really need to do this ? IIUC reservation count and actual
> allocated count are two different. One block allocation we need to
> remove all the blocks reserved from the reservation count and add
Yes. remove all, but minus already allocated_metadata, which was
accounted in to metadata reservation.
> actually allocated blocks to the allocated count.
Just try an example:
reserve_space (inode, lblock := 1024 ) {
md_needed = ext4_calc_metadata_amount(inode, lblock) (let it be '2')
dquot_reserve_block(inode, md_needed + 1) /* '3' i.e blocks reserved*/
/* If this is first reservation for this inode then
dq_rsv = inode->i_reserved_data_blocks + inode->i_reserved_meta_block
*/
}
Later called from fallocate
update_rerved_space(inode, used:=1, claim :=0) {
/* Let i_allocatd_meta_data is '1' (as it so in most cases) */
ei->i_reserved_data_blocks -= used; /* 1 - 1 => 0 */
used += ei->i_allocated_meta_blocks; /* 1 + 1 => 2 */
ei->i_reserved_meta_blocks -= ei->i_allocated_meta_blocks /* 2 - 1 => 1 */
allocated_meta_blocks = ei->i_allocated_meta_blocks; /* 1 */
ei->i_allocated_meta_blocks = 0;
if (ei->i_reserved_data_blocks == 0) /* True in our case */
mdb_free = ei->i_reserved_meta_blocks; /* mbd_free == 1*/
if (allocated_meta_blocks)
dquot_claim_block(inode, allocated_meta_blocks); /* claim '1' block*/
dquot_release_reservation_block(inode, mdb_free + used); /* free (1 + 2) */
/* So we reserved:
dq_rsv = i_reserved_data_blocks + i_reserved_meta_block ( 3 blocks)
But during update we claim + free:
i_allocated_meta_data+(i_reserved_data_block+i_reserved_meta_data)
(4 blocks).
Which result in incorrect dquota reservation accounting(it
goes negative)
*/
Initially i've found the issue by executing fsstress with falloc support.
It takes enouth process to catch writepage/fallocate overlapping.
xfstests-dev/ltp/xfsfsstress -p100 -n99999999
On Tue, Mar 30, 2010 at 06:24:35PM +0400, Dmitry Monakhov wrote:
> allocated_meta_data is already included in 'used' variable.
>
> Signed-off-by: Dmitry Monakhov <[email protected]>
Thanks, added to the ext4 patch queue.
-- Ted