From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from bombadil.infradead.org ([198.137.202.9]:50661 "EHLO bombadil.infradead.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750707AbcHAHfi (ORCPT ); Mon, 1 Aug 2016 03:35:38 -0400 Date: Mon, 1 Aug 2016 00:08:40 -0700 From: Christoph Hellwig To: "Darrick J. Wong" Cc: david@fromorbit.com, linux-fsdevel@vger.kernel.org, vishal.l.verma@intel.com, bfoster@redhat.com, xfs@oss.sgi.com Subject: Re: [PATCH 11/47] xfs: move deferred operations into a separate file Message-ID: <20160801070840.GL15590@infradead.org> References: <146907695530.25461.3225785294902719773.stgit@birch.djwong.org> <146907703710.25461.16650495404061662831.stgit@birch.djwong.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <146907703710.25461.16650495404061662831.stgit@birch.djwong.org> Sender: linux-fsdevel-owner@vger.kernel.org List-ID: > + * NOTE: To avoid exceeding the transaction reservation, we limit the > + * number of items that we attach to a given xfs_defer_pending. This seems like a new feature compared to the old bmap_free code, can you explain it it in a little more detail? As I'm trying to compare this code to the existing one there are a few things that confuse me, most importantly the purpose of the xfs_defer_pending structure, which seems like a new indirection. I don't really like that indirection at all, as it introduces another memory allocation deep down in the commit path where we can't recover from ENOMEM. Also there seems to be a 1:1 relationship between it and the xfs_bmap_free_item structure that got renamed to xfs_extent_free_item. > +static const struct xfs_defer_op_type *defer_op_types[XFS_DEFER_OPS_TYPE_MAX]; We shouldn't really need this global array and the xfs_defer_ops_type enum, just pass the xfs_defer_op_type pointer into xfs_defer_add.