From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-1.1 required=3.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_PASS, URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 7BB53C43387 for ; Tue, 15 Jan 2019 19:25:24 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 4C8DF20657 for ; Tue, 15 Jan 2019 19:25:24 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=linux-foundation.org header.i=@linux-foundation.org header.b="fZ1TTgcz" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2389295AbfAOTZX (ORCPT ); Tue, 15 Jan 2019 14:25:23 -0500 Received: from mail-lf1-f65.google.com ([209.85.167.65]:33325 "EHLO mail-lf1-f65.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S2388953AbfAOTZW (ORCPT ); Tue, 15 Jan 2019 14:25:22 -0500 Received: by mail-lf1-f65.google.com with SMTP id i26so2986494lfc.0 for ; Tue, 15 Jan 2019 11:25:21 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux-foundation.org; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=dqROPm4NPUUMwcizVJYbl2sK2XhVYIs91sN+Y9Lt0qg=; b=fZ1TTgczycj/KktyZ4x0ojp/vqFNiw7HiqkUfNoNl47AdKIi7LN1ziSXU5j/wSRKT2 FgXiQoD+JO369pBG63qI9n4h2KNCQldX7v0MsckUxksZEgkLlB1NIb3gaUqh8sZkRDN3 1F0e7TOl5LT/8Dp5kmNrZbWX4Xw3GYMoRfpkI= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=dqROPm4NPUUMwcizVJYbl2sK2XhVYIs91sN+Y9Lt0qg=; b=IwRmE+XRT0sfChsdSlufBejsuv6OIS0TfE1jhZ3DXFgmwZPaA7DOSLybYUQ0NZ6Al7 txrTXsUHtX2PqY8t93FDanQLyo7QxlKjrWtiyUHSqqY2GlMwtw22GSs9kY+QJ24Mmp1w +gylPzRixxE7tq5KUagdBl2/dQxa6KwdH1jQ9gamgyrskR4cQwopxvHA00L8/H1lkr9Q f4vqR6oSiccrMduCdlZBKAh8D0N8k5GKEiHElYW8J9CU1MdROfs1WA+1U0HHl53B/Q4m 0qRGQ8NU7dn+noz+LX86M8YGW87sRzhMCg4qCh51BmhVUxHRJ34wjjirVa6nZm9OQRLS xAwA== X-Gm-Message-State: AJcUukeqZ1tU2Kx1RC6belPLiJWuZ3xQ+2huauS8Qp7ukIHJPbnkwCrs tPEjtfU40fSnjtC+2PPyC24EVBv6TEc= X-Google-Smtp-Source: ALg8bN5BgHIUeeoc78wMtk/EVgu/M12ZxN8ESDh9AlTBxiVy8OawabUFacyzso4aJAqNKbZ+9AU7nw== X-Received: by 2002:a19:c4cc:: with SMTP id u195mr3849902lff.141.1547580319464; Tue, 15 Jan 2019 11:25:19 -0800 (PST) Received: from mail-lj1-f177.google.com (mail-lj1-f177.google.com. [209.85.208.177]) by smtp.gmail.com with ESMTPSA id m21sm770042lfl.97.2019.01.15.11.25.18 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Tue, 15 Jan 2019 11:25:18 -0800 (PST) Received: by mail-lj1-f177.google.com with SMTP id l15-v6so3327471lja.9 for ; Tue, 15 Jan 2019 11:25:18 -0800 (PST) X-Received: by 2002:a2e:310a:: with SMTP id x10-v6mr4093784ljx.6.1547580317929; Tue, 15 Jan 2019 11:25:17 -0800 (PST) MIME-Version: 1.0 References: <20190102174736.GB29127@quack2.suse.cz> <20190108100500.GA15801@quack2.suse.cz> <20190115092414.GA4138@quack2.suse.cz> <20190115180156.GB6310@bombadil.infradead.org> In-Reply-To: <20190115180156.GB6310@bombadil.infradead.org> From: Linus Torvalds Date: Wed, 16 Jan 2019 07:25:01 +1200 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [GIT PULL] dtype handling cleanup for v4.21-rc1 To: Matthew Wilcox Cc: Jan Kara , linux-fsdevel , linux-ext4@vger.kernel.org, Al Viro Content-Type: text/plain; charset="UTF-8" Sender: linux-fsdevel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-fsdevel@vger.kernel.org On Wed, Jan 16, 2019 at 6:01 AM Matthew Wilcox wrote: > > The ext2/ext4 patches don't show much improvement. The other patches show > more: > > fs/nilfs2/dir.c | 52 ++++++++++-------------------- > include/uapi/linux/nilfs2_ondisk.h | 1 + > 2 files changed, 18 insertions(+), 35 deletions(-) > > (for example). > > UFS ends up benefiting the most. You can see the whole diffstat here: > > https://lore.kernel.org/lkml/20181023201952.GA15676@pathfinder/ Well, even with _all_ the filesystems converted, you actually have more lines added than removed by this "cleanup". Sharing code just isn't a win here. That said, it's not really the number of lines per se that make me question this, I think that's really more of a symptom than the root cause. The root cause for the newly adde lines is that this whole approach requires that all the numbers are in sync, but then they have different *names*. Honestly, my gut feel is that I should not pull this in this form. I have a suggestion: if people want to do this, and actually share the transformation, then the filesystems that use this common code should simply *NOT* have their own private names for the enumerations. They should actually use those standard names. So if the patch for ext2 (for example) were to entirely get rid of the whole EXT2_FT_DIR define entirely, and ext2 would just use the actual FT_DIR define, than I'd be ok with it. At that point you don't add a pointless and expensive abstraction. At that point you say "ext2 uses the standard values, so ext2 can just use the standard #defines directly". See my argument? I think it is completely disgsting to have stuff like this: + BUILD_BUG_ON(EXT2_FT_UNKNOWN != FT_UNKNOWN); + BUILD_BUG_ON(EXT2_FT_REG_FILE != FT_REG_FILE); + BUILD_BUG_ON(EXT2_FT_DIR != FT_DIR); + BUILD_BUG_ON(EXT2_FT_CHRDEV != FT_CHRDEV); + BUILD_BUG_ON(EXT2_FT_BLKDEV != FT_BLKDEV); + BUILD_BUG_ON(EXT2_FT_FIFO != FT_FIFO); + BUILD_BUG_ON(EXT2_FT_SOCK != FT_SOCK); + BUILD_BUG_ON(EXT2_FT_SYMLINK != FT_SYMLINK); + BUILD_BUG_ON(EXT2_FT_MAX != FT_MAX); the above is just *garbage*. If you fundamentally need the values to be the same, then you simply shouldn't have two different set of #defines. Get rid of the EXT2_FT_xyz enumeration entirely, and the whole craziness goes away. > We'd see a lot more improvement in line count if Philip weren't quite > so paranoid about checking FOOFS_FT_* == FT_* at build time; eg for btrfs: Exact same issue. So the more I look at this, the less I like it. But if people are actually willing to use *truly* shared code, instead of using their own values and then having the crazy "they need to match", then it would be a different issue. As it is, I think the patch series adds complexity rather than helping anything. More complexity and more lines of code? There is absolutely zero upside. Linus