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=-10.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=unavailable 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 09B25C433DB for ; Wed, 10 Feb 2021 20:49:56 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id C372C64E30 for ; Wed, 10 Feb 2021 20:49:55 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232045AbhBJUti (ORCPT ); Wed, 10 Feb 2021 15:49:38 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:38750 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230005AbhBJUtg (ORCPT ); Wed, 10 Feb 2021 15:49:36 -0500 Received: from mail-lf1-x134.google.com (mail-lf1-x134.google.com [IPv6:2a00:1450:4864:20::134]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id DD50CC06174A for ; Wed, 10 Feb 2021 12:48:49 -0800 (PST) Received: by mail-lf1-x134.google.com with SMTP id v24so4915240lfr.7 for ; Wed, 10 Feb 2021 12:48:49 -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=bFCBVLmx4m3P8OJ7FexI4xTU+D+dqeOPB5L3zdP1j98=; b=OlXMEGE3+YXDW20u82up+ZZJxFuT5KQPVaOZp56fIviL/ca54YrjE5bQPiM0jLbtr5 JephDlBWc6ihBoJg5k8R1M7+hSbeoGkpREvy/8A8Bsg5/X4GhbjclPWfb060aUds7Xkf 2bFaMcZ2LOg9RoALDj7udW/xyJ4i9lrqIzZcg= 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=bFCBVLmx4m3P8OJ7FexI4xTU+D+dqeOPB5L3zdP1j98=; b=iBw2Ea2xV0yYXkywNa+8xmmcQpauM/1J+6L+4BvqiVIYEXPeFYlRvVUkWpSJ2lv83x 3CWTu2noTy6kNIfnFFQ+6zNaUcoKlC3Bzi3QwYHIqPvr7agjXytgmPwaEIVmWh48wGob 5daPyrEzQ0wMDM5zcOsel0tMRh7BhsboonP5q2pUSv/Z36mdw6+GwWsbnGD+s0lM9Nst BMs/udCMUpys+AQzYGKS96WF44w6t6vAu1YeObVMgua86dVBsgcWPgerVfvpjJJ6PA9b nluHoVWXsDVMBWhPKWfA7k9juzWC5IjXZ0o5s2DGEPJZYP1ToFPT/SHknaLurJU1+7OL dNhQ== X-Gm-Message-State: AOAM532yRVEMhUf1fLMSOmlcJcpKRpI8vMyhTPfNcODHiiaPuoFHWap1 p+mYc2vSlBaVtEt6vUOERbQOsEghtaprcA== X-Google-Smtp-Source: ABdhPJzi9KaCZI6G8CKfBuCl0VXpxDwJVD3f8UZsPbck5mYbKVj+fZzLreweT/9PGBfiELTL38NolA== X-Received: by 2002:a05:6512:2281:: with SMTP id f1mr2512725lfu.629.1612990127659; Wed, 10 Feb 2021 12:48:47 -0800 (PST) Received: from mail-lf1-f44.google.com (mail-lf1-f44.google.com. [209.85.167.44]) by smtp.gmail.com with ESMTPSA id h11sm444321lfd.243.2021.02.10.12.48.47 for (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 10 Feb 2021 12:48:47 -0800 (PST) Received: by mail-lf1-f44.google.com with SMTP id v30so4571967lfq.6 for ; Wed, 10 Feb 2021 12:48:47 -0800 (PST) X-Received: by 2002:a05:6512:a8c:: with SMTP id m12mr2551518lfu.253.1612989815438; Wed, 10 Feb 2021 12:43:35 -0800 (PST) MIME-Version: 1.0 References: <591237.1612886997@warthog.procyon.org.uk> <1330473.1612974547@warthog.procyon.org.uk> <1330751.1612974783@warthog.procyon.org.uk> In-Reply-To: <1330751.1612974783@warthog.procyon.org.uk> From: Linus Torvalds Date: Wed, 10 Feb 2021 12:43:19 -0800 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [GIT PULL] fscache: I/O API modernisation and netfs helper library To: David Howells Cc: Matthew Wilcox , Jeff Layton , David Wysochanski , Anna Schumaker , Trond Myklebust , Steve French , Dominique Martinet , Alexander Viro , ceph-devel@vger.kernel.org, linux-afs@lists.infradead.org, linux-cachefs@redhat.com, CIFS , linux-fsdevel , "open list:NFS, SUNRPC, AND..." , v9fs-developer@lists.sourceforge.net, Linux Kernel Mailing List Content-Type: text/plain; charset="UTF-8" Precedence: bulk List-ID: X-Mailing-List: linux-cifs@vger.kernel.org On Wed, Feb 10, 2021 at 8:33 AM David Howells wrote: > > Then I could follow it up with this patch here, moving towards dropping the > PG_fscache alias for the new API. So I don't mind the alias per se, but I did mind the odd mixing of names for the same thing. So I think your change to make it be named "wait_on_page_private_2()" fixed that mixing, but I also think that it's probably then a good idea to have aliases in place for filesystems that actually include the fscache.h header. Put another way: I think that it would be even better to simply just have a function like static inline void wait_on_page_fscache(struct page *page) { if (PagePrivate2(page)) wait_on_page_bit(page, PG_private_2); } and make that be *not* in , but simply be in under that big comment about how PG_private_2 is used for the fscache bit. You already have that comment, putting the above kind of helper function right there would very much explain why a "wait for fscache bit" function then uses the PagePrivate2 function to test the bit. Agreed? Alternatively, since that header file already has #define PageFsCache(page) PagePrivate2((page)) you could also just write the above as static inline void wait_on_page_fscache(struct page *page) { if (PageFsCache(page)) wait_on_page_bit(page, PG_fscache); } and now it is even more obvious. And there's no odd mixing of "fscache" and "private_2", it's all consistent. IOW, I'm not against "wait_on_page_fscache()" as a function, but I *am* against the odd _mixing_ of things without a big explanation, where the code itself looks very odd and questionable. And I think the "fscache" waiting functions should not be visible to any core VM or filesystem code - it should be limited explicitly to those filesystems that use fscache, and include that header file. Wouldn't that make sense? Also, honestly, I really *REALLY* want your commit messages to talk about who has been cc'd, who has been part of development, and point to the PUBLIC MAILING LISTS WHERE THAT DISCUSSION WAS TAKING PLACE, so that I can actually see that "yes, other people were involved" No, I don't require this in general, but exactly because of the history we have, I really really want to see that. I want to see a Link: https://lore.kernel.org/r/.... and the Cc's - or better yet, the Reviewed-by's etc - so that when I get a pull request, it really is very obvious to me when I look at it that others really have been involved. So if I continue to see just Signed-off-by: David Howells at the end of the commit messages, I will not pull. Yes, in this thread a couple of people have piped up and said that they were part of the discussion and that they are interested, but if I have to start asking around just to see that, then it's too little, too late. No more of this "it looks like David Howells did things in private". I want links I can follow to see the discussion, and I really want to see that others really have been involved. Ok? Linus