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;
Date:   Tue, 11 Oct 2022 23:14:33 +0200
From:   Guillaume Nault <gnault@redhat.com>
To:     Trond Myklebust <trondmy@kernel.org>
Cc:     Christoph Hellwig <hch@infradead.org>,
        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>
Subject: Re: [PATCH v2] sunrpc: Use GFP_NOFS to prevent use of
 current->task_frag.
Message-ID: <20221011211433.GA13385@ibm-p9z-18-fsp.mgmt.pnr.lab.eng.rdu2.redhat.com>
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>
 <a0bf0d49a7a69d20cfe007d66586a2649557a30b.camel@kernel.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <a0bf0d49a7a69d20cfe007d66586a2649557a30b.camel@kernel.org>
Precedence: bulk

On Tue, Oct 11, 2022 at 11:57:53AM -0400, Trond Myklebust wrote:
> 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.

The problem is to find a hole in struct sock, in a cacheline that
wouldn't incur a cache miss.

So far the best option seems to add a special flag in sk_allocation,
which would serve the same purpose as GFP_NOFS in sk_page_frag(). But
that'd mean clearing this bit every time sk_allocation is used for
memory allocation. Not really something I'd propose for stable trees.

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