From:   Trond Myklebust <trondmy@hammerspace.com>
To:     David Howells <dhowells@redhat.com>
CC:     Matthew Wilcox <willy@infradead.org>,
        "David S. Miller" <davem@davemloft.net>,
        Eric Dumazet <edumazet@google.com>,
        Jakub Kicinski <kuba@kernel.org>,
        Paolo Abeni <pabeni@redhat.com>,
        Alexander Viro <viro@zeniv.linux.org.uk>,
        Christoph Hellwig <hch@infradead.org>,
        Jens Axboe <axboe@kernel.dk>,
        Jeffrey Layton <jlayton@kernel.org>,
        Christian Brauner <brauner@kernel.org>,
        Linus Torvalds <torvalds@linux-foundation.org>,
        "netdev@vger.kernel.org" <netdev@vger.kernel.org>,
        "linux-fsdevel@vger.kernel.org" <linux-fsdevel@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "linux-mm@kvack.org" <linux-mm@kvack.org>,
        Anna Schumaker <anna@kernel.org>,
        Charles Edward Lever <chuck.lever@oracle.com>,
        "linux-nfs@vger.kernel.org" <linux-nfs@vger.kernel.org>
Subject: Re: [RFC PATCH 27/28] sunrpc: Use sendmsg(MSG_SPLICE_PAGES) rather
 then sendpage
Thread-Topic: [RFC PATCH 27/28] sunrpc: Use sendmsg(MSG_SPLICE_PAGES) rather
 then sendpage
Thread-Index: AQHZWBvarYtr7Kf76ku/+p4ZPGQrUa79lXMAgAAB/4CAABBbgIAADCQAgAAPPwA=
Date:   Thu, 16 Mar 2023 19:01:35 +0000
Message-ID: <9C741BDB-31B0-460C-8FE7-F1C9B49002D5@hammerspace.com>
References: <8F8B62FD-0F16-4383-BB34-97E850DAA7AF@hammerspace.com>
 <3DFBF27C-A62B-4AFE-87FD-3DF53FC39E8E@hammerspace.com>
 <20230316152618.711970-1-dhowells@redhat.com>
 <20230316152618.711970-28-dhowells@redhat.com>
 <754534.1678983891@warthog.procyon.org.uk>
 <809995.1678990010@warthog.procyon.org.uk>
In-Reply-To: <809995.1678990010@warthog.procyon.org.uk>
Accept-Language: en-US, en-GB
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?QnwhHwuM6Y58ycuAVxw8g7mebdsTMBZiU2cWVFc9Ma3j9585knofXGpAjhUS?=
 =?us-ascii?Q?ysUKiVnQPY9JRuwyezAN8ZoqybUPuBMsaHJDRfjDvVU3C+ctf6NKeiSHr1T0?=
 =?us-ascii?Q?/ZtagyDn7jrKZHSFJ792c80ndRVvd1K3ny7muMYg8yeXsN0plahuarGZmux5?=
 =?us-ascii?Q?cflv86z4z800851pvfTfc4xnkJWaYWocUK7s0BYAzicYENjX9czCPqex2XXd?=
 =?us-ascii?Q?0Ya3DRFdD7ODhW1upbWtEOMsXp0+D52xO3nu9BYO8zlQCg+AbCi3KdgIpfMA?=
 =?us-ascii?Q?n6I4KXuWi2KMUJthv244E6r7EwC5o30bar0/ktrcHvc2WvwiC/ZxypeC+J4h?=
 =?us-ascii?Q?hdw3oqiI4ZKsbVCO3Cm3zTceExe1p10kKaAK1PSDjSMQwCK2gq5olazugae3?=
 =?us-ascii?Q?mQbF9GYFKR+D/0vfyMY6LXH3iD7EVJAJFbGIBYpdcHaBAm1P9JCpep97Spf/?=
 =?us-ascii?Q?SrtX00EAS9vqakM2NJujCJoG7aBc7oBjFB4HbOXSksrTswlwfcNH82Hezysn?=
 =?us-ascii?Q?NDvHK0bQ7Tq4O+xFdP60TKXbkJ23Q9XMKM8TrJOmRaUN/tFZzVfbJNsHANJz?=
 =?us-ascii?Q?/ofVDdcmtz8zTRlKVCVvm5NeATop312b2abR5kmLVWng7PJ0wEl/ls2MkTFP?=
 =?us-ascii?Q?Eo9R9rHGHqDQWos0JbmIRMDUOXpyU6V1Rj77NuLQhaJhMZlwD/7Dtpox1NbG?=
 =?us-ascii?Q?mhd99+tD2GhFTFZei6TzOLYGqDXzM0Y5EeOsK5pwxbyrB7HfzivvwbvhHxkU?=
 =?us-ascii?Q?+mvnFwH/n7FfKeZ+g41K+Yatv+vHnjKWJt+nZ9mx1BdDbeimu12BVTGZTYcR?=
 =?us-ascii?Q?MkGq2kOI47gUhjafdqFcL6n81SBZtK3HUpx0zoPRcRR28JrV7h6JeK7LPldP?=
 =?us-ascii?Q?mO9orqwmWuHKbfVqVJB+oqhtDCQiCAPHe2AdyfT8BfQGpKKtzNM5OjEZno0l?=
 =?us-ascii?Q?bUdNg4lmj5qUSrI+lPsiqfjab4NnrrCjyoP5msQX2H9L/N2djJNhoAr79PXp?=
 =?us-ascii?Q?KlPv5UfAOHAz3nkyL8aqhFhv0imD/Dix8YysbHqoqtaP3+q7FPuDW/VVlNz4?=
 =?us-ascii?Q?2du/B4UHNISPjS/SYjbuXLKy0QggLjSMMf5ji5D1LcrojRbactiXOIZp4LC2?=
 =?us-ascii?Q?xTcRWqmIl3QahXiJGGNAoQDIItA+wzxmktbkCN3v36GcfsnE5zO5H+uDxxsT?=
 =?us-ascii?Q?zJb66d1BdVU5i7q4Fr27a0gsQYG3D9ptWaTGmOi6ICGmVXiA4LmjY1JpZcfe?=
 =?us-ascii?Q?pDwyW7vDQIfYiHbwnp8HiN8lDNaLwZH3/HEmcT1gCEQH2A9Juwb4NpWZw4Q5?=
 =?us-ascii?Q?ddKpMhd6KN6Cpk42TfK6NysMtZq5aWrnejoPR9LgqRaws4g45lRg7kGkdtk/?=
 =?us-ascii?Q?piyYGXr/K6t27IqUl7zGb6U/VMUWXN5CVp79UacNFY4Z2TXPTUorhy4Me/rR?=
 =?us-ascii?Q?ehW8dj/XhITyldAGoriO5ol33A3qvTiMmInTz2iNejqIaX336AXu0LpJIzt1?=
 =?us-ascii?Q?bIqST+8cHzL90wnF2mvGznDWBBWpC9jUrPEQsDdx4eEyvhZknwcgsjK9MRDX?=
 =?us-ascii?Q?Aw5VLJeP5TMxm2P3N/8gf6rXgIb4Ksw8bW7z95D02nP4dPHHyMujW2Lfe0NQ?=
 =?us-ascii?Q?JLXkn/rjOlLsfvYozI3LpE1sKNMS34zzBwuStaDQm5lrHlLfMr8zruXv9ks4?=
 =?us-ascii?Q?sWboDQ=3D=3D?=
Content-Type: text/plain; charset="us-ascii"
Content-ID: <62428EC330CCF346865612A43F1E98D4@namprd13.prod.outlook.com>
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-OriginatorOrg: hammerspace.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: CH0PR13MB5084.namprd13.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 7fe19cf9-46f2-46b0-91b9-08db2650dd0d
X-MS-Exchange-CrossTenant-originalarrivaltime: 16 Mar 2023 19:01:35.0325
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 0d4fed5c-3a70-46fe-9430-ece41741f59e
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: TncB0RjoJc8QitQl6CkfFU2oBmKUFoe/C/rDKYwjfnmweOEi4RTrkoz+96t/U3LdqjAQ5sCJMfRaGbadPchppQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MW4PR13MB5410
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org


> On Mar 16, 2023, at 14:06, David Howells <dhowells@redhat.com> wrote:
>=20
> Trond Myklebust <trondmy@hammerspace.com> wrote:
>=20
>> 1) This is code that is common to the client and the server. Why are we
>> adding unused 3 bvec slots to every client RPC call?
>=20
> Fair point, but I'm trying to avoid making four+ sendmsg calls in nfsd ra=
ther
> than one.

Add an enum iter_type for ITER_ITER ? :-)

Otherwise, please just split these functions into one for knfsd and a separ=
ate one for the client.

>=20
>> 2) It obfuscates the existence of these bvec slots.
>=20
> True, it'd be nice to find a better way to do it.  Question is, can the c=
lient
> make use of MSG_SPLICE_PAGES also?

The requirement for O_DIRECT support means we get the stable write issues w=
ith added extra spicy sauce.

>=20
>> 3) knfsd may use splice_direct_to_actor() in order to avoid copying the =
page
>> cache data into private buffers (it just takes a reference to the
>> pages). Using MSG_SPLICE_PAGES will presumably require it to protect tho=
se
>> pages against further writes while the socket is referencing them.
>=20
> Upstream sunrpc is using sendpage with TCP.  It already has that issue.
> MSG_SPLICE_PAGES is a way of doing sendpage through sendmsg.

Fair enough. I do seem to remember a schism with the knfsd developers over =
that issue.

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