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: <c66b13a6-b9d7-381a-4fb8-cc5e8b833ac5@nvidia.com>
Date:   Wed, 31 Aug 2022 11:02:45 -0700
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.2.0
Subject: Re: [PATCH 5/6] NFS: direct-io: convert to FOLL_PIN pages
Content-Language: en-US
To:     Jan Kara <jack@suse.cz>
Cc:     Al Viro <viro@zeniv.linux.org.uk>,
        Andrew Morton <akpm@linux-foundation.org>,
        Jens Axboe <axboe@kernel.dk>,
        Miklos Szeredi <miklos@szeredi.hu>,
        Christoph Hellwig <hch@infradead.org>,
        "Darrick J . Wong" <djwong@kernel.org>,
        Trond Myklebust <trond.myklebust@hammerspace.com>,
        Anna Schumaker <anna@kernel.org>,
        Logan Gunthorpe <logang@deltatee.com>,
        linux-block@vger.kernel.org, linux-fsdevel@vger.kernel.org,
        linux-xfs@vger.kernel.org, linux-nfs@vger.kernel.org,
        linux-mm@kvack.org, LKML <linux-kernel@vger.kernel.org>
References: <20220827083607.2345453-1-jhubbard@nvidia.com>
 <20220827083607.2345453-6-jhubbard@nvidia.com> <YwqfWoAE2Awp4YvT@ZenIV>
 <353f18ac-0792-2cb7-6675-868d0bd41d3d@nvidia.com> <Ywq5ILRNxsbWvFQe@ZenIV>
 <Ywq5VrSrY341UVpL@ZenIV> <217b4a17-1355-06c5-291e-7980c0d3cea6@nvidia.com>
 <20220829160808.rwkkiuelipr3huxk@quack3>
 <a53b2d14-687a-16c9-2f63-4f94876f8b3c@nvidia.com>
 <20220831094349.boln4jjajkdtykx3@quack3>
From:   John Hubbard <jhubbard@nvidia.com>
In-Reply-To: <20220831094349.boln4jjajkdtykx3@quack3>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?eXVoMC9VU29IVTV1QmtWWkFZWTlOd0xLdDV0L2VHTFZ4REhLWkZpTGRYcmxp?=
 =?utf-8?B?Y1VEMWdSUVYzMGN1TnBHTmUrS1hIZ2lCcTEwcjdveHppL0J0bnQwWmVGU2NI?=
 =?utf-8?B?c3F3RTVWSldqOGk5bEFZeXQxVC8xbHFaS01SSVB4T3ltWjBJWGUwSllMYlhu?=
 =?utf-8?B?Zm9aOEtTRjd5SlZyYXAwZzV1d1JoWCs0VGpoRnpnOEVKZ3RUSlNzYk4zZjBx?=
 =?utf-8?B?dDhHcEorcnFqSThVYWQ2Q2I3QVhBekFORllkK3VPclhkTURjT2ltZElKQXRl?=
 =?utf-8?B?elBIbmpxVVk2aUlOdi8yK1lGS1hJbUlobzBEdm1aeTUydXM5Z1hBbFM2bWhQ?=
 =?utf-8?B?bEtGT1J2ZUJsMmdtRzJPOFhVTU8rRlMzanlEdmVHWllCem5aNHVhUndJejlr?=
 =?utf-8?B?ZmlKUHJLa2R2Vi91dEtGdTNocHN5RTg5OEtEL0N1OXhDNTA2QkNqcmlBNitH?=
 =?utf-8?B?MncrbFoxNStRQy84K1N3UTJEMkpnQjBjaXFvVlNSTFUvUGkzclUxcnRNM3cv?=
 =?utf-8?B?K3ZyZUNZVE1IaFVsRVBQVmoxYlNGQTdkcjJodzdkRk1aR1lnSUE5YVFLcFk3?=
 =?utf-8?B?U2grL1lCaWVvNmZMOFdqbmYyZCt3R09FZGVOeDN2WXZ5b3ZsRGp5eEJraWlk?=
 =?utf-8?B?NW8yZnNmdjdwazJuYi9INWxEU0MycmNrSVFuaVJ2ODZxS1orZ2JOazV6bDNu?=
 =?utf-8?B?dHU4Vnd2ZEdWRjY4VnJ1ekEvNkVDOG95VmR0V0lySjNKWDdpSU9uWEh1N2h2?=
 =?utf-8?B?TmEyQzdPNEwrSVFnbFE5ZGQ3eVFYRzhydys2MTlHdjRwNnhmVlRsN3ppQWdu?=
 =?utf-8?B?MVRQRFpnbjdNU3JXZm10L2NHTU5MKzk0bzV1TkZDS3owTHIzdTBJeWVMeEkv?=
 =?utf-8?B?K2ZZSWFGbVFrTmRPc0x6WksrZ1ptM1JUNWRZVGVOdk5sVlhoVnltVlljd0Na?=
 =?utf-8?B?THkyaW8vMWd5ejVCM0VpeGJGWldlY09seGFJc1VqY250R1RWUm5qTkdmT2o5?=
 =?utf-8?B?TERQUE56SlFsRGpWY3BEOGRIak5ybzBUWng5TDlmbGc0TUpoSXZLN2FSZjNE?=
 =?utf-8?B?azFxWXkydTIvVUFkMkRlVEptTW5Qam0yU0M1L0lyejBuck1SY29RK2xtUjU3?=
 =?utf-8?B?NlBsT3IrN0crWk43Sm5tRVQ1UHVFNmVKdFpWeXVXRHRWWmtGTXd0YW5HS2Nk?=
 =?utf-8?B?N2dqcGtROEdvSnFHQ0hnNWVrelRkRFBBWmZSMGh3eDgzT0VOVkIyeVJKK1kx?=
 =?utf-8?B?WGF0aUZ4aE8xa0J0YTFrNXpHZ0tlOUdJOXd4ZENpd3hVeDNYY2gwRzY3OSs3?=
 =?utf-8?B?bG13YVZDT0gzY1JMNFRXM1JlWkI3RTExUThPd1VVVmNwcjA4N1hZMUpzTy95?=
 =?utf-8?B?VkJLY2J5ay9sWHhtMjgvOStKcXF6NGcrd29EOXNWT1dpam91dHRzSzdzTFE5?=
 =?utf-8?B?TVB6c2E4dTBFZmNENWxkWWs0WEIzcUtaRDhZdDJTKy9PeFZJeDkvc1kzWFF5?=
 =?utf-8?B?c0xCVmUrZ2d4dkNoMUxLTjBwVDFSY3F6ODJZWXZ1UzlzYk44VVovOHBJNkRp?=
 =?utf-8?B?RTN0MFlpMXA2TFpsVG52YlgyVExpclkyQ2FhdGdGb2hmY2Y4akUzNkFVMEsw?=
 =?utf-8?B?ck5mL2ZadmppNE9SZzBITi94WjB1TXJDenI2Sm1UVnRZZFpjdVlXbW0zRnUy?=
 =?utf-8?B?VGxjRGFvTnlieldxS25lckJhVzRqOGxFOFpyQ29JWUw0VEFaVGdDU1lUcUJD?=
 =?utf-8?B?YlZ4WXhORHpOMjBVLzl0d1JoZ0t1RFBiSDQ1Qnl2SHdyY2JJQkNQZGhtSmVY?=
 =?utf-8?B?S0c3WTEzaXVXNnVVRDF2dW1DbjU3VUNpejFLeHBRcDVmTEIwS3VhN1dVTkNr?=
 =?utf-8?B?ZFQxRWEvVm1LTUc3THhPUVJlbHFxbHYrS0tnL1oyNHdPS0ZSODREcEZ4NlZh?=
 =?utf-8?B?QkFNSlc0Qmw1ZTN6Q2l1MVQ5aWlySnNaWVNiZk4rbHpaSU5tOE1KQnNZVTEy?=
 =?utf-8?B?QmlLNzFpejlQRm5ZdkN4N2huYThIZGNPNFdyL0VrZHl1djNmMU9hOVRtL1Ft?=
 =?utf-8?B?S1Z1STlJMDVmTTVvN2V0NVdqa0c3N3dmRElEUVFiQ1lac1VxaFh3SmpOa2tF?=
 =?utf-8?Q?GyHA7ufh5iGbw1P5eUfqzbe/H?=
X-OriginatorOrg: Nvidia.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 952b5246-39bc-4c85-d96a-08da8b7b03a4
X-MS-Exchange-CrossTenant-AuthSource: BY5PR12MB4130.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 31 Aug 2022 18:02:48.6438
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 43083d15-7273-40c1-b7db-39efd9ccc17a
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: aC564EZzlLoGCRfQPGAqDr6OJJvIJ7Wj4jwnFOEHJQlCb4gfKSyfxlNPUz3ldVGwcu/gBZTDkV0uhuT735H/Vw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH7PR12MB5880
X-Spam-Status: No, score=-1.1 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FORGED_SPF_HELO,
        NICE_REPLY_A,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H2,SPF_HELO_PASS,
        SPF_NONE,T_SCC_BODY_TEXT_LINE autolearn=no autolearn_force=no
        version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-nfs.vger.kernel.org>
X-Mailing-List: linux-nfs@vger.kernel.org

On 8/31/22 02:43, Jan Kara wrote:
>> OK, thanks, that looks viable. So, that approach assumes that the
>> remaining two cases in __iov_iter_get_pages_alloc() will never end up
>> being released via bio_release_pages():
>>
>>     iov_iter_is_pipe(i)
>>     iov_iter_is_xarray(i)
>>
>> I'm actually a little worried about ITER_XARRAY, which is a recent addition.
>> It seems to be used in ways that are similar to ITER_BVEC, and cephfs is
>> using it. It's probably OK for now, for this series, which doesn't yet
>> convert cephfs.
> 
> So after looking into that a bit more, I think a clean approach would be to
> provide iov_iter_pin_pages2() and iov_iter_pages_alloc2(), under the hood
> in __iov_iter_get_pages_alloc() make sure we use pin_user_page() instead of
> get_page() in all the cases (using this in pipe_get_pages() and
> iter_xarray_get_pages() is easy) and then make all bio handling use the
> pinning variants for iters. I think at least iov_iter_is_pipe() case needs
> to be handled as well because as I wrote above, pipe pages can enter direct
> IO code e.g. for splice(2).
> 

OK, yes.

> Also I think that all iov_iter_get_pages2() (or the _alloc2 variant) users
> actually do want the "pin page" semantics in the end (they are accessing
> page contents) so eventually we should convert them all to
> iov_iter_pin_pages2() and remove iov_iter_get_pages2() altogether. But this
> will take some more conversion work with networking etc. so I'd start with
> converting bios only.
> 
> 								Honza


I'll give this approach a spin, thanks very much for the guidance!


thanks,

-- 
John Hubbard
NVIDIA