Received-SPF: pass (google.com: domain of linux-nfs-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <a0bf0d49a7a69d20cfe007d66586a2649557a30b.camel@kernel.org>
Subject: Re: [PATCH v2] sunrpc: Use GFP_NOFS to prevent use of
 current->task_frag.
From:   Trond Myklebust <trondmy@kernel.org>
To:     Guillaume Nault <gnault@redhat.com>,
        Christoph Hellwig <hch@infradead.org>
Cc:     Trond Myklebust <trond.myklebust@hammerspace.com>,
        Anna Schumaker <anna@kernel.org>,
        Chuck Lever <chuck.lever@oracle.com>,
        Jeff Layton <jlayton@kernel.org>, linux-nfs@vger.kernel.org,
        Paolo Abeni <pabeni@redhat.com>,
        Benjamin Coddington <bcodding@redhat.com>
Date:   Tue, 11 Oct 2022 11:57:53 -0400
In-Reply-To: <20221011150057.GB3606@localhost.localdomain>
References: <de6d99321d1dcaa2ad456b92b3680aa77c07a747.1665401788.git.gnault@redhat.com>
         <Y0QyYV1Wyo4vof70@infradead.org>
         <20221010165650.GA3456@ibm-p9z-18-fsp.mgmt.pnr.lab.eng.rdu2.redhat.com>
         <Y0UKq62ByUGNQpuY@infradead.org>
         <20221011150057.GB3606@localhost.localdomain>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 8BIT
User-Agent: Evolution 3.44.4 (3.44.4-2.fc36) 
MIME-Version: 1.0
Precedence: bulk

On Tue, 2022-10-11 at 17:00 +0200, Guillaume Nault wrote:
> On Mon, Oct 10, 2022 at 11:18:19PM -0700, Christoph Hellwig wrote:
> > On Mon, Oct 10, 2022 at 06:56:50PM +0200, Guillaume Nault wrote:
> > > That's what my RFC patch did. It was rejected because reading
> > > current->flags may incur a cache miss thus slowing down TCP fast
> > > path.
> > > See the discussion in the Link tag:
> > > https://lore.kernel.org/netdev/b4d8cb09c913d3e34f853736f3f5628abfd7f4b6.1656699567.git.gnault@redhat.com/
> > 
> > As GFP_NOFS/NOIO are on their way out the networking people will
> > have to
> > do this anyway.
> 
> We can always think of a nicer solution in the future. But right now
> we
> have a real bug to fix.
> 
> Commit a1231fda7e94 ("SUNRPC: Set memalloc_nofs_save() on all
> rpciod/xprtiod jobs") introduces a bug that crashes the kernel. I
> can't
> see anything wrong with a partial revert.
> 

How about instead just adding a dedicated flag to the socket that
switches between the two page_frag modes?

That would remain future proofed, and it would give kernel users a
lever with which to do the right thing without unnecessarily
constraining the allocation modes.

-- 
Trond Myklebust
Linux NFS client maintainer, Hammerspace
trond.myklebust@hammerspace.com