From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754047AbdFSMgT (ORCPT ); Mon, 19 Jun 2017 08:36:19 -0400 Received: from mx2.suse.de ([195.135.220.15]:46421 "EHLO mx1.suse.de" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1753922AbdFSMgR (ORCPT ); Mon, 19 Jun 2017 08:36:17 -0400 Date: Mon, 19 Jun 2017 14:36:14 +0200 From: Jan Kara To: Tahsin Erdogan Cc: Jan Kara , Jan Kara , "Theodore Ts'o" , Andreas Dilger , Dave Kleikamp , Alexander Viro , Mark Fasheh , Joel Becker , Jens Axboe , Deepa Dinamani , Mike Christie , Fabian Frederick , linux-ext4@vger.kernel.org, linux-kernel@vger.kernel.org, jfs-discussion@lists.sourceforge.net, linux-fsdevel@vger.kernel.org, ocfs2-devel@oss.oracle.com, reiserfs-devel@vger.kernel.org Subject: Re: [PATCH 28/28] quota: add extra inode count to dquot transfer functions Message-ID: <20170619123614.GA20405@quack2.suse.cz> References: <20170531081517.11438-1-tahsin@google.com> <20170531081517.11438-28-tahsin@google.com> <20170615075735.GB1764@quack2.suse.cz> <20170619090329.GE11837@quack2.suse.cz> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: 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 19-06-17 04:46:00, Tahsin Erdogan wrote: > >> I tried that approach by adding a "int get_inode_usage(struct inode > >> *inode, qsize_t *usage)" callback to dquot_operations. Unfortunately, > >> ext4 code that calculates the number of internal inodes > >> (ext4_xattr_inode_count()) is subject to failures so the callback has > >> to be able to report errors. And, that itself is problematic because > >> we can't afford to have errors in dquot_free_inode(). If you have > >> thoughts about how to address this please let me know. > > > > Well, you can just make dquot_free_inode() return error. Now most callers > > won't be able to do much with an error from dquot_free_inode() but that's > > the case also for other things during inode deletion - just handle it as > > other fatal failures during inode freeing. > > > I just checked dquot_free_inode() to see whether it calls anything > that could fail. It calls mark_all_dquot_dirty() and ignores the > return code from it. I would like to follow the same for the > get_inode_usage() as the only use case for get_inode_usage() (ext4) > should not fail at inode free time. > > Basically, I want to avoid changing return type from void to int > because it would create a new responsibility for the filesystem > implementations who do not know how to deal with it. Heh, this "pushing of responsibility" looks like a silly game. If an error can happen in a function, it is better to report it as far as easily possible (unless we can cleanly handle it which we cannot here). I'm guilty of making dquot_free_inode() ignore errors from mark_all_dquot_dirty() and in retrospect it would have been better if these were propagated to the caller as well. And eventually we can fix this if we decide we care enough. I'm completely fine with just returning an error from dquot_free_inode() and ignore it in all the callers except for ext4. Then filesystems which care enough can try to handle the error. That way we at least don't increase the design debt from the past. Honza -- Jan Kara SUSE Labs, CR From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jan Kara Date: Mon, 19 Jun 2017 14:36:14 +0200 Subject: [Ocfs2-devel] [PATCH 28/28] quota: add extra inode count to dquot transfer functions In-Reply-To: References: <20170531081517.11438-1-tahsin@google.com> <20170531081517.11438-28-tahsin@google.com> <20170615075735.GB1764@quack2.suse.cz> <20170619090329.GE11837@quack2.suse.cz> Message-ID: <20170619123614.GA20405@quack2.suse.cz> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: Tahsin Erdogan Cc: Jan Kara , Jan Kara , Theodore Ts'o , Andreas Dilger , Dave Kleikamp , Alexander Viro , Mark Fasheh , Joel Becker , Jens Axboe , Deepa Dinamani , Mike Christie , Fabian Frederick , linux-ext4@vger.kernel.org, linux-kernel@vger.kernel.org, jfs-discussion@lists.sourceforge.net, linux-fsdevel@vger.kernel.org, ocfs2-devel@oss.oracle.com, reiserfs-devel@vger.kernel.org On Mon 19-06-17 04:46:00, Tahsin Erdogan wrote: > >> I tried that approach by adding a "int get_inode_usage(struct inode > >> *inode, qsize_t *usage)" callback to dquot_operations. Unfortunately, > >> ext4 code that calculates the number of internal inodes > >> (ext4_xattr_inode_count()) is subject to failures so the callback has > >> to be able to report errors. And, that itself is problematic because > >> we can't afford to have errors in dquot_free_inode(). If you have > >> thoughts about how to address this please let me know. > > > > Well, you can just make dquot_free_inode() return error. Now most callers > > won't be able to do much with an error from dquot_free_inode() but that's > > the case also for other things during inode deletion - just handle it as > > other fatal failures during inode freeing. > > > I just checked dquot_free_inode() to see whether it calls anything > that could fail. It calls mark_all_dquot_dirty() and ignores the > return code from it. I would like to follow the same for the > get_inode_usage() as the only use case for get_inode_usage() (ext4) > should not fail at inode free time. > > Basically, I want to avoid changing return type from void to int > because it would create a new responsibility for the filesystem > implementations who do not know how to deal with it. Heh, this "pushing of responsibility" looks like a silly game. If an error can happen in a function, it is better to report it as far as easily possible (unless we can cleanly handle it which we cannot here). I'm guilty of making dquot_free_inode() ignore errors from mark_all_dquot_dirty() and in retrospect it would have been better if these were propagated to the caller as well. And eventually we can fix this if we decide we care enough. I'm completely fine with just returning an error from dquot_free_inode() and ignore it in all the callers except for ext4. Then filesystems which care enough can try to handle the error. That way we at least don't increase the design debt from the past. Honza -- Jan Kara SUSE Labs, CR