From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1762284AbdEWILM (ORCPT ); Tue, 23 May 2017 04:11:12 -0400 Received: from mx2.suse.de ([195.135.220.15]:38817 "EHLO mx1.suse.de" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1752129AbdEWILI (ORCPT ); Tue, 23 May 2017 04:11:08 -0400 Date: Tue, 23 May 2017 10:10:55 +0200 From: Jan Kara To: Tahsin Erdogan Cc: "Theodore Ts'o" , Andreas Dilger , linux-ext4@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH] ext4: fix quota charging for shared xattr blocks Message-ID: <20170523081055.GB1230@quack2.suse.cz> References: <20170523030148.24361-1-tahsin@google.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20170523030148.24361-1-tahsin@google.com> User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 > --- > 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 SUSE Labs, CR