| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Mon, 23 Jun 2008 12:11:38 -0700
From: Mingming <cmm@...ibm.com>
To: "Aneesh Kumar K.V" <aneesh.kumar@...ux.vnet.ibm.com>
Cc: linux-ext4@...r.kernel.org, Jan Kara <jack@...e.cz>
Subject: Re: [PATCH] delalloc: add quota handling
On Sat, 2008-06-21 at 16:18 +0530, Aneesh Kumar K.V wrote:
> On Fri, Jun 20, 2008 at 06:14:38PM -0700, Mingming wrote:
> > Correct quota handling in delayed allocation. With this patch,
> > the quota for blocks are counted at block reservation time when
> > the fs free blocks counter are updated, instead of at later
> > block allocation time.
> >
> > Signed-off-by: Mingming Cao <cmm@...ibm.com>
> > ---
> > fs/ext4/balloc.c | 8 +++++---
> > fs/ext4/inode.c | 5 +++++
> > fs/ext4/mballoc.c | 22 ++++++++++++----------
> > 3 files changed, 22 insertions(+), 13 deletions(-)
> >
> > Index: linux-2.6.26-rc6/fs/ext4/balloc.c
> > ===================================================================
> > --- linux-2.6.26-rc6.orig/fs/ext4/balloc.c 2008-06-20 18:06:02.000000000 -0700
> > +++ linux-2.6.26-rc6/fs/ext4/balloc.c 2008-06-20 18:06:05.000000000 -0700
> > @@ -1716,7 +1716,8 @@ ext4_fsblk_t ext4_old_new_blocks(handle_
> > /*
> > * Check quota for allocation of this block.
> > */
> > - if (DQUOT_ALLOC_BLOCK(inode, num)) {
> > + if (!EXT4_I(inode)->i_delalloc_reserved_flag &&
> > + DQUOT_ALLOC_BLOCK(inode, num)) {
> > *errp = -EDQUOT;
> > return 0;
> > }
> > @@ -1928,7 +1929,8 @@ allocated:
> >
> > *errp = 0;
> > brelse(bitmap_bh);
> > - DQUOT_FREE_BLOCK(inode, *count-num);
> > + if (!EXT4_I(inode)->i_delalloc_reserved_flag)
> > + DQUOT_FREE_BLOCK(inode, *count-num);
> > *count = num;
> > return ret_block;
> >
> > @@ -1942,7 +1944,7 @@ out:
> > /*
> > * Undo the block allocation
> > */
> > - if (!performed_allocation)
> > + if (!EXT4_I(inode)->i_delalloc_reserved_flag && !performed_allocation)
> > DQUOT_FREE_BLOCK(inode, *count);
> > brelse(bitmap_bh);
> > return 0;
> > Index: linux-2.6.26-rc6/fs/ext4/inode.c
> > ===================================================================
> > --- linux-2.6.26-rc6.orig/fs/ext4/inode.c 2008-06-20 18:06:02.000000000 -0700
> > +++ linux-2.6.26-rc6/fs/ext4/inode.c 2008-06-20 18:06:05.000000000 -0700
> > @@ -1450,6 +1450,9 @@ static int ext4_da_reserve_space(struct
> > return -ENOSPC;
> > }
> >
> > + if (DQUOT_ALLOC_BLOCK(inode, total))
> > + return -EDQUOT;
> > +
>
> We should not be counting meta-data blocks for quota. I guess this
> should be
>
> if (DQUOT_ALLOC_BLOCK(inode, nrblocks))
> return -EDQUOT;
>
Metadata blocks are also accounted for quota today with mballoc and non
mballoc allocation.
> Also i think we should be doing quota check first.
Okay, I will move the quota check/update before checking fs free blocks
> In mballoc we request
> for less number of blocks if quota is limited. In case of
> ext4_da_reserve_space even though we get called only with one block
> reservation request to make the code correct i guess we should do the
> mballoc equivalent.
>
> while (ar->len && DQUOT_ALLOC_BLOCK(ar->inode, ar->len)) {
> ar->flags |= EXT4_MB_HINT_NOPREALLOC;
> ar->len--;
> }
>
Hmm.. not sure if the requested allocation length can be reduced since
this is all needed for allocating one block. Reduce the length may
result in later ENOSPC. BTW ar is a temprory variable that only alive
in the path doing block allocation, we need some addition field in inode
to pass the hint flag from write_begin() time down to the writepages().
>
> Also in mballoc we should do some test for free quota blocks and
> should set the EXT4_MB_HINT_NOPREALLOC even for delalloc.
>
With this patch, since the quota has already accounted at write_begin()
time with delayed allocation., Should we check the free quota again
here (at the block allocation time)?
I think doing the quota reservation at write_begin() time for delalloc
is probably the right way, as you suggested on IRC.
Mingming
> > /* reduce fs free blocks counter */
> > percpu_counter_sub(&sbi->s_freeblocks_counter, total);
> >
> > @@ -1479,6 +1482,8 @@ void ext4_da_release_space(struct inode
> >
> > release = to_free + mdb_free;
> >
> > + DQUOT_FREE_BLOCK(inode, release);
>
>
> This should be
> DQUOT_FREE_BLOCK(inode, to_free);
>
> > +
> > /* update fs free blocks counter for truncate case */
> > percpu_counter_add(&sbi->s_freeblocks_counter, release);
> >
> > Index: linux-2.6.26-rc6/fs/ext4/mballoc.c
> > ===================================================================
> > --- linux-2.6.26-rc6.orig/fs/ext4/mballoc.c 2008-06-20 18:06:01.000000000 -0700
> > +++ linux-2.6.26-rc6/fs/ext4/mballoc.c 2008-06-20 18:06:05.000000000 -0700
> > @@ -4037,7 +4037,7 @@ ext4_fsblk_t ext4_mb_new_blocks(handle_t
> > struct super_block *sb;
> > ext4_fsblk_t block = 0;
> > int freed;
> > - int inquota;
> > + int inquota = 0;
> >
> > sb = ar->inode->i_sb;
> > sbi = EXT4_SB(sb);
> > @@ -4059,15 +4059,17 @@ ext4_fsblk_t ext4_mb_new_blocks(handle_t
> > return 0;
> > }
> >
> > - while (ar->len && DQUOT_ALLOC_BLOCK(ar->inode, ar->len)) {
> > - ar->flags |= EXT4_MB_HINT_NOPREALLOC;
> > - ar->len--;
> > - }
> > - if (ar->len == 0) {
> > - *errp = -EDQUOT;
> > - return 0;
> > + if (!EXT4_I(ar->inode)->i_delalloc_reserved_flag) {
> > + while (ar->len && DQUOT_ALLOC_BLOCK(ar->inode, ar->len)) {
> > + ar->flags |= EXT4_MB_HINT_NOPREALLOC;
> > + ar->len--;
> > + }
>
>
> We need to set ar->flags even for delalloc. Otherwise we will try to
> normalize the request in mballoc.
>
>
> > + if (ar->len == 0) {
> > + *errp = -EDQUOT;
> > + return 0;
> > + }
> > + inquota = ar->len;
> > }
> > - inquota = ar->len;
> >
> > if (EXT4_I(ar->inode)->i_delalloc_reserved_flag)
> > ar->flags |= EXT4_MB_DELALLOC_RESERVED;
> > @@ -4134,7 +4136,7 @@ repeat:
> > out2:
> > kmem_cache_free(ext4_ac_cachep, ac);
> > out1:
> > - if (ar->len < inquota)
> > + if (!EXT4_I(ar->inode)->i_delalloc_reserved_flag && ar->len < inquota)
> > DQUOT_FREE_BLOCK(ar->inode, inquota - ar->len);
> >
> > return block;
> >
> >
>
> -aneesh
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists