From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx1.redhat.com ([209.132.183.28]:8249 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S934465AbeEIMr6 (ORCPT ); Wed, 9 May 2018 08:47:58 -0400 Date: Wed, 9 May 2018 08:47:56 -0400 From: Brian Foster Subject: Re: [PATCH v2 1/3] xfs: add bmapi nodiscard flag Message-ID: <20180509124756.GB65322@bfoster.bfoster> References: <20180508172231.53570-1-bfoster@redhat.com> <20180508172231.53570-2-bfoster@redhat.com> <20180509074629.GC19933@infradead.org> <20180509105853.GD64624@bfoster.bfoster> <20180509113942.GA17093@infradead.org> <20180509120150.GA65322@bfoster.bfoster> <20180509120738.GA1050@infradead.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20180509120738.GA1050@infradead.org> Sender: linux-xfs-owner@vger.kernel.org List-ID: List-Id: xfs To: Christoph Hellwig Cc: linux-xfs@vger.kernel.org, Dave Chinner On Wed, May 09, 2018 at 05:07:38AM -0700, Christoph Hellwig wrote: > On Wed, May 09, 2018 at 08:01:51AM -0400, Brian Foster wrote: > > > self-documenting, very much unlike a 'true' or 'false' argument. > > > > I'm fine with replacing the bool argument(s) with flags where applicable > > if we do eliminate the wrappers. I'm just hesitant to change it given > > the previous feedback to move away from something very close.. > > > > Dave, care to chime in here? As mentioned, I'll do a refactored v3 if > > there's some kind of consensus/agreement on a final approach. > > I've read the thread on the original patch now. While not my preference > I'm fine with doing an xfs_itruncate_extents_flags with a single > xfs_itruncate_extents wrapper and the same for bmapi, as long as we pass > flags instead of the bool, and don't add pointless wrappers for the > nodiscard case - those are just trickle down flags in general, so we > should keep things as simple as possible. Ok, do you mean to include xfs_free_extent() in that as well? E.g., xfs_free_extent_flags(..., XFS_EXTENT_BUSY_SKIP_DISCARD) vs. a single wrapper without _flags()? Note that that flag is still sourced from a boolean unless we also change the xfs_extent_free_item field, which I'm not sure makes sense. Alternatively, I could just kill the xfs_free_extent_nodiscard() wrapper and call the internal variant from the one place that wants to toggle discard behavior. Brian > -- > To unsubscribe from this list: send the line "unsubscribe linux-xfs" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html