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=-6.0 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=no 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 104ABC43331 for ; Tue, 26 Jan 2021 22:37:15 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id E59C020679 for ; Tue, 26 Jan 2021 22:37:14 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728864AbhAZWfo (ORCPT ); Tue, 26 Jan 2021 17:35:44 -0500 Received: from us-smtp-delivery-124.mimecast.com ([63.128.21.124]:24120 "EHLO us-smtp-delivery-124.mimecast.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S2389402AbhAZS2T (ORCPT ); Tue, 26 Jan 2021 13:28:19 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1611685598; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=2fOOzn8pPhlTKm9KikLbj00xwMG7fV3rZ/r/LGxiqKA=; b=KZLRu9pGfOh+PGgCdJZzF+WnkvDgUlUFbTjG9b/0mOeDr6lY9RtmreTeOnRoPoLhRucWzf QuKvt75FDQvAPwIVL9hmrDZRLIAIaq7o51caSehCrc+c1mLrpTiLpnTeqTZOxPlmfzBs47 vxzmU41PUkpQhRcZPdDZObCYjiZrzNc= Received: from mail-ed1-f71.google.com (mail-ed1-f71.google.com [209.85.208.71]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-60-LkoLAifuPg6Z66fHZGC7qQ-1; Tue, 26 Jan 2021 13:26:37 -0500 X-MC-Unique: LkoLAifuPg6Z66fHZGC7qQ-1 Received: by mail-ed1-f71.google.com with SMTP id n8so9795019edo.19 for ; Tue, 26 Jan 2021 10:26:36 -0800 (PST) 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=2fOOzn8pPhlTKm9KikLbj00xwMG7fV3rZ/r/LGxiqKA=; b=MRX/4ELvhMmEGH5PceL3/+OrpkNNhLryk3ancmvoOZXFw49Qt+AsquyerNM6I98oSP NFm9vLrnxnIK9+lyHLynVWb/DXV1yGvYVXcXe3HrgZEymkH6YcL24fRZiDWIhMsAmjYF epfbYqNcriQ+7ZROv4Ki3/PayDLkkD02a5tFihSuUJwwxFb02jEeWaMTlAOd6QiVGDVl ZOQJDzkdG6xplA6VOhkLrDH1TpO2RdsTL+YVlg20hY5bZPMHhGiuhqqczLnSd73FPcKz co0jV70deTFukkAAUHDgdV4qTJH9Z64TL/Fjqele6FXB+rAKEcvni1dNInvfbcJi1XJv ZOIQ== X-Gm-Message-State: AOAM531CWYlHAXKuYJ7ePLsxxb6QiJdVq1Mh83UpHeMuP5OanUX2SDRI i4qC42LPR2nDI8frwulwcw0YYCfzdQ/DktsL2bzmESn7X3c6GM7KnlPTu1uVk9Ls+dLEEWkG8k2 1x/KKk3w92pE6cDlq3YrcYzQ/mW5lhadeNESrYVlTDQ== X-Received: by 2002:a17:906:409:: with SMTP id d9mr4137076eja.70.1611685595820; Tue, 26 Jan 2021 10:26:35 -0800 (PST) X-Google-Smtp-Source: ABdhPJyEFq3YOlGW6iMDijn0AhVKCr8FbAfIU0rqEW4JxM7phQSOhY8lxoG5RgplzfjnQ6XssK0VTMbS4BNxu0SuS8k= X-Received: by 2002:a17:906:409:: with SMTP id d9mr4137063eja.70.1611685595666; Tue, 26 Jan 2021 10:26:35 -0800 (PST) MIME-Version: 1.0 References: <161161025063.2537118.2009249444682241405.stgit@warthog.procyon.org.uk> <161161064956.2537118.3354798147866150631.stgit@warthog.procyon.org.uk> <20210126013611.GI308988@casper.infradead.org> In-Reply-To: <20210126013611.GI308988@casper.infradead.org> From: David Wysochanski Date: Tue, 26 Jan 2021 13:25:59 -0500 Message-ID: Subject: Re: [PATCH 32/32] NFS: Convert readpage to readahead and use netfs_readahead for fscache To: Matthew Wilcox Cc: David Howells , Trond Myklebust , Anna Schumaker , Steve French , Dominique Martinet , Jeff Layton , Alexander Viro , linux-cachefs@redhat.com, linux-afs@lists.infradead.org, linux-nfs , linux-cifs , ceph-devel@vger.kernel.org, v9fs-developer@lists.sourceforge.net, linux-fsdevel , Linux Kernel Mailing List Content-Type: text/plain; charset="UTF-8" Precedence: bulk List-ID: X-Mailing-List: linux-fsdevel@vger.kernel.org On Mon, Jan 25, 2021 at 8:37 PM Matthew Wilcox wrote: > > > For Subject: s/readpage/readpages/ > Fixed > On Mon, Jan 25, 2021 at 09:37:29PM +0000, David Howells wrote: > > +int __nfs_readahead_from_fscache(struct nfs_readdesc *desc, > > + struct readahead_control *rac) > > I thought you wanted it called ractl instead of rac? That's what I've > been using in new code. > Fixed > > - dfprintk(FSCACHE, "NFS: nfs_getpages_from_fscache (0x%p/%u/0x%p)\n", > > - nfs_i_fscache(inode), npages, inode); > > + dfprintk(FSCACHE, "NFS: nfs_readahead_from_fscache (0x%p/0x%p)\n", > > + nfs_i_fscache(inode), inode); > > We do have readahead_count() if this is useful information to be logging. > Right, I used it elsewhere so I'll add here as well. > > +static inline int nfs_readahead_from_fscache(struct nfs_readdesc *desc, > > + struct readahead_control *rac) > > { > > - if (NFS_I(inode)->fscache) > > - return __nfs_readpages_from_fscache(ctx, inode, mapping, pages, > > - nr_pages); > > + if (NFS_I(rac->mapping->host)->fscache) > > + return __nfs_readahead_from_fscache(desc, rac); > > return -ENOBUFS; > > } > > Not entirely sure that it's worth having the two functions separated any more. > Yeah it's questionable so I'll collapse. I'll also do that with nfs_readpage_from_fscache(). > > /* attempt to read as many of the pages as possible from the cache > > * - this returns -ENOBUFS immediately if the cookie is negative > > */ > > - ret = nfs_readpages_from_fscache(desc.ctx, inode, mapping, > > - pages, &nr_pages); > > + ret = nfs_readahead_from_fscache(&desc, rac); > > if (ret == 0) > > goto read_complete; /* all pages were read */ > > > > nfs_pageio_init_read(&desc.pgio, inode, false, > > &nfs_async_read_completion_ops); > > > > - ret = read_cache_pages(mapping, pages, readpage_async_filler, &desc); > > + while ((page = readahead_page(rac))) { > > + ret = readpage_async_filler(&desc, page); > > + put_page(page); > > + } > > I thought with the new API we didn't need to do this kind of thing > any more? ie no matter whether fscache is configured in or not, it'll > submit the I/Os. > We don't. This patchset was only intended as a stepping stone to get the netfs API accepted with minimal invasiveness in NFS. I have another patch which will unconditionally call netfs API but I didn't post it. Since I'm not an NFS maintainer, and maintainer's didn't weigh in on the approach, I opted to go with the least invasive approach. There's an NFS "remote bakeathon" coming up at the end of Feb. That would probably be a good time to get further testing on NFS unconditionally calling the netfs API, and we should be able to cover things like any performance concerns, etc.