From patchwork Fri Nov 3 12:19:15 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Amir Goldstein X-Patchwork-Id: 10039937 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 12C0F6032D for ; Fri, 3 Nov 2017 12:19:19 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id EA9C1295B6 for ; Fri, 3 Nov 2017 12:19:18 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id DED90295BE; Fri, 3 Nov 2017 12:19:18 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.3 required=2.0 tests=BAYES_00, DKIM_ADSP_CUSTOM_MED, DKIM_SIGNED, FREEMAIL_FROM, RCVD_IN_DNSWL_HI, RCVD_IN_SORBS_SPAM, T_DKIM_INVALID autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 737B0295B6 for ; Fri, 3 Nov 2017 12:19:18 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756195AbdKCMTS (ORCPT ); Fri, 3 Nov 2017 08:19:18 -0400 Received: from mail-yw0-f193.google.com ([209.85.161.193]:45330 "EHLO mail-yw0-f193.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756141AbdKCMTR (ORCPT ); Fri, 3 Nov 2017 08:19:17 -0400 Received: by mail-yw0-f193.google.com with SMTP id j4so2225228ywb.2 for ; Fri, 03 Nov 2017 05:19:16 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=CpGYfQuYMXb12hPQoUCwCoRjNgTdhXXOfZ7jD7SdUnE=; b=B2Ro+XevP2y+ijW2nU3O2Jbo7ML4qF2ESP0BNYI5acCu03lMYJcM6E2byRlHyPtIrn ppihKyZ6B3IIZY45I06RyHCDkwCq/5VY44BmfkaxJWtcfSTl6BXLkmEr4mbn2g0/CJyM 0H3M1Ar+gDMHeVlPG5P0H5PqCNABJQ/9v6wGqJU7sM5e00BicyYMFt3XU6sLWUNFjfP6 nSS6w5tZXhyJUF1OMs8twbbu9BYftB0i1nGUseABED/CNI1ZBoV6dT9yh7AAA2HoPlNa 5zoAZI8Nk59uiMZfNTDpm69lXwCE8CcSjYaycE3xhTORRRwvSuCA3exRfrsbcwf7xmbi 8Bcw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=CpGYfQuYMXb12hPQoUCwCoRjNgTdhXXOfZ7jD7SdUnE=; b=ZVahi8bZEoD06UeNT5pwkgQ5h9dvI7Od6vora+jKCRGX36HJbGzGCfk/34YT5ePGbC uNeXamNNAIbBpTxv8s4QhJfiWDvgsrktQ/WT5wdSOiQBiT2dAtKIQuUnwSwFMX+g78mK eZcKOzsBPwdgPfwHofurmuvTPOx8ue+dZANYXqdvBoXvS0yaN74MYBVPqh4o5bHq+nbx xpYPH/XYovgdRUku/10jqus9C2Xm/WaJSqfMicJccCJW6DWlHeXlc+Xgcb8T9mnUt3XU cusfr5AuD6DXFg8NB6X/FGgAhiRt+7q9akOaObkL0ZC4INE7W9jR2JT+sF6TsyywOS9g 4z5Q== X-Gm-Message-State: AMCzsaXa1cGRZD0bkS0dlkxjW1+Tzhu0UdUMuGYwxceicMKlXJ380KAK t/QiCaFqdHzl1CNaAA0ju6VgLnaDxaxhE1x5+7s= X-Google-Smtp-Source: ABhQp+Q5YRGjbPF9jKPEpwawLNZ6/+ETmQvwF5TY+q69lCIiT3JxnZ1izg7dQLu6tTdx26qkAi9tGNIobBmzVYFkpfY= X-Received: by 10.129.166.74 with SMTP id d71mr4528425ywh.241.1509711556416; Fri, 03 Nov 2017 05:19:16 -0700 (PDT) MIME-Version: 1.0 Received: by 10.129.115.212 with HTTP; Fri, 3 Nov 2017 05:19:15 -0700 (PDT) In-Reply-To: <20171103112626.GA19974@bfoster.bfoster> References: <20171026083322.20428-1-david@fromorbit.com> <20171030133117.GA3535@bfoster.bfoster> <20171030210941.GA5858@dastard> <20171031112432.GA7093@bfoster.bfoster> <20171101004513.GK5858@dastard> <20171101141720.GB11709@bfoster.bfoster> <20171101235300.GT5858@dastard> <20171102112533.GA15748@bfoster.bfoster> <20171102233017.GX5858@dastard> <20171103112626.GA19974@bfoster.bfoster> From: Amir Goldstein Date: Fri, 3 Nov 2017 14:19:15 +0200 Message-ID: Subject: Re: [RFC PATCH 0/14] xfs: Towards thin provisioning aware filesystems To: Brian Foster Cc: Dave Chinner , linux-xfs Sender: linux-xfs-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-xfs@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On Fri, Nov 3, 2017 at 1:26 PM, Brian Foster wrote: > On Fri, Nov 03, 2017 at 10:30:17AM +1100, Dave Chinner wrote: >> On Thu, Nov 02, 2017 at 07:25:33AM -0400, Brian Foster wrote: ... >> >> As such, trying to determine a future proof growfs interface change >> right now is simply a waste of time because we're not going to get >> it right. >> > > I'm not sure where the flags proposal thing came from, but regardless... > I'm not trying to future proof/design a shrink interface. Rather, just > reviewing the (use of the) interface as it already exists. I suppose I > am kind of assuming that the current interface would enable some form of > functionality in that hypothetical future world where physical shrink > exists. Perhaps that is not so realistic, however, as you suggest. > Guys, Can we PLEASE stop talking about physical shrink? I get it, it's unlikely to happen, I sorry I brought up this example in the first place. Whether or not we implement physical shirk is not the main issue. The main issue is implicitly changing the meaning of an existing API. What can go wrong? I don't know and I rather not give examples, because then people address the examples and not the conceptual flaw. This is how I propose to smooth in the new API with as little pain as possible for existing deployments, yet making sure that "usable block" is only modified by new programs that intend to modify it: ---------------------- ---------------- When xfs_grow WANTS to change size of fs known to be thin, it should set in->imaxpct |= 1 << 8; Dave, Is that the complication of implementation you were talking about? Really? Don't you see that this is the right thing to do w.r.t. API design? If it were you on the other side of review, I bet you would argued the same as Brian and myself and use the argument that "The syscall and ioctl APIs are littered with examples and we should pay heed to the lessons those mistakes teach us." to oppose implicit API change. Seriously, my opinion does not carry enough weight in the xfs community to out weight your opinion and if it weren't for Brian who stepped in to argue in favor of my claims, I would have given up trying to convince you. Sorry if this is coming off as too harsh of a response. The sole motivation behind this argument is to prevent pain in the future. And you are right, we can never predict the future correctly, except for one thing - that we WILL encounter use cases that none of us can see right now. Cheers, Amir --- 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 diff --git a/fs/xfs/xfs_fsops.c b/fs/xfs/xfs_fsops.c index 8f22fc579dbb..922798ebf3e8 100644 --- a/fs/xfs/xfs_fsops.c +++ b/fs/xfs/xfs_fsops.c @@ -171,10 +171,25 @@ xfs_growfs_data_private( xfs_rfsblock_t nfree; xfs_agnumber_t oagcount; int pct; + unsigned ver; xfs_trans_t *tp; nb = in->newblocks; - pct = in->imaxpct; + pct = in->imaxpct & 0xff; + ver = in->imaxpct >> 8; + if (ver > 1) + return -EINVAL; + + /* + * V0 API is only allowed to grow sb_usable_dblocks along with + * sb_dblocks. Any other change to sb_usable_dblocks requires + * V1 API to prove that userspace is aware of usable_dblocks. + */ + if (ver == 0 && xfs_sb_version_hasthinspace(mp->m_sb) && + (mp->m_sb.sb_usable_dblocks != mp->m_sb.sb_dblocks || + nb < mp->m_sb.sb_dblocks)) + return -EINVAL; + if (nb < mp->m_sb.sb_dblocks || pct < 0 || pct > 100) return -EINVAL;