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.1 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 C1380C433E0 for ; Wed, 27 Jan 2021 07:29:26 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 779A42073C for ; Wed, 27 Jan 2021 07:29:26 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S316439AbhA0AdF (ORCPT ); Tue, 26 Jan 2021 19:33:05 -0500 Received: from us-smtp-delivery-124.mimecast.com ([63.128.21.124]:27621 "EHLO us-smtp-delivery-124.mimecast.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S2389430AbhAZS2U (ORCPT ); Tue, 26 Jan 2021 13:28:20 -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-wKLYRedIOGe6b4Z1tbvLkw-1; Tue, 26 Jan 2021 13:26:37 -0500 X-MC-Unique: wKLYRedIOGe6b4Z1tbvLkw-1 Received: by mail-ed1-f71.google.com with SMTP id a24so9776757eda.14 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=CHHVrte7fpjT8DCLFY738BfVwp6heQWvNQ0TR7ZnQP7EoUBYmJC4CtVsU5TbcnPqJL Ihs/OHqaXSkr71CkNi27XScOj4x0t/ZTEu3n64cbva7OwIRcHgmo/RPG1iLISWEpf4Ij qR06mgYsiABK5tusoC3GMDeHRT0KFPYljLWrBwAJQLZYeYUReQI2QYGIyzg0uroOY8ZD S0p5K743tiA9nFbmZuqueVfajOOF0RkeYVxkwvUaCpq4U35xjM/x+Kpibu59lpe+uOaw 4W1zgiU4BQn8du8fcb1bL5S48tPElLtIacWACUS2XDZrsgUifDEgKURGUYcW1YSnBTsZ R/ug== X-Gm-Message-State: AOAM533PmO/jnCX4jvQGoBzBLLIAseAXwY/8fgJK5dkr9KcFcrYM6R1d ZTkiHmQRN2JnZWIpLjxFNmT9JVTLBrChDgvCMmZYpA5dNp7pxkaZvXI8nw8S/6BUbB9zzndB37m lL7AdmOdEAs2CBCX9D1boHtLMhJc1AogmtWBj8Q== X-Received: by 2002:a17:906:409:: with SMTP id d9mr4137080eja.70.1611685595822; 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-cifs@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.