Re: [PATCH] ext4: fix quota charging for shared xattr blocks

From: Jan Kara
Date: Tue May 23 2017 - 04:11:14 EST


On Mon 22-05-17 20:01:48, Tahsin Erdogan wrote:
> ext4_xattr_block_set() calls dquot_alloc_block() to charge for an xattr
> block when new references are made. However if dquot_initialize() hasn't
> been called on an inode, request for charging is effectively ignored
> because ext4_inode_info->i_dquot is not initialized yet.
>
> Add dquot_initialize() call to ext4_xattr_set_handle().

Thanks for finding the bug! However this is a wrong place where to insert
dquot_initialize(). Generally we try really hard to do dquot_initalize()
calls outside of a transaction (as they may have pretty big requirements on
the amount of transaction credits - and for your patch to be correct you'd
have to update credit estimates for each possible transaction involved). So
you should rather place these calls into ext4_xattr_set(), ext4_set_acl(),
ext4_set_context() (only in the case when transaction needs to be started
before starting it). You can then add a warning in ext4_xattr_set_handle()
checking whether quota is indeed initialized as expected.

Honza

>
> Signed-off-by: Tahsin Erdogan <tahsin@xxxxxxxxxx>
> ---
> fs/ext4/xattr.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/fs/ext4/xattr.c b/fs/ext4/xattr.c
> index 8fb7ce14e6eb..e94575448550 100644
> --- a/fs/ext4/xattr.c
> +++ b/fs/ext4/xattr.c
> @@ -1166,6 +1166,11 @@ ext4_xattr_set_handle(handle_t *handle, struct inode *inode, int name_index,
> return -EINVAL;
> if (strlen(name) > 255)
> return -ERANGE;
> +
> + error = dquot_initialize(inode);
> + if (error)
> + return error;
> +
> ext4_write_lock_xattr(inode, &no_expand);
>
> error = ext4_reserve_inode_write(handle, inode, &is.iloc);
> --
> 2.13.0.219.gdb65acc882-goog
>
--
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR