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 Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 74305C3DA4A for ; Tue, 20 Aug 2024 13:07:41 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id B78F46B0083; Tue, 20 Aug 2024 09:07:40 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id B280A6B0085; Tue, 20 Aug 2024 09:07:40 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 9C8BD6B0088; Tue, 20 Aug 2024 09:07:40 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0012.hostedemail.com [216.40.44.12]) by kanga.kvack.org (Postfix) with ESMTP id 7C14E6B0083 for ; Tue, 20 Aug 2024 09:07:40 -0400 (EDT) Received: from smtpin17.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay09.hostedemail.com (Postfix) with ESMTP id 0306381A1A for ; Tue, 20 Aug 2024 13:07:39 +0000 (UTC) X-FDA: 82472650680.17.9101A47 Received: from szxga01-in.huawei.com (szxga01-in.huawei.com [45.249.212.187]) by imf17.hostedemail.com (Postfix) with ESMTP id 6C4FA40018 for ; Tue, 20 Aug 2024 13:07:35 +0000 (UTC) Authentication-Results: imf17.hostedemail.com; dkim=none; spf=pass (imf17.hostedemail.com: domain of linyunsheng@huawei.com designates 45.249.212.187 as permitted sender) smtp.mailfrom=linyunsheng@huawei.com; dmarc=pass (policy=quarantine) header.from=huawei.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1724159242; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=yDKc/6xoYjtFNkM4oka/cWTFsDfETxi1OyIqvH0kI1g=; b=l+Ig82lnC6KtHUQW5H4qp6klHxxlwh1gVqig9Ylhzn6w/bgvZdDXbFSN1XjThGkhk+AylZ FphpuZ3p289SFiT/KFXHdn8ATSoQWaq08OWjqBrEuAdlKcNsZIRyFq0JGPhycTRpTOD/Oa y8smWmvZM0ccmjQZFLgQOwGhbIXT68o= ARC-Authentication-Results: i=1; imf17.hostedemail.com; dkim=none; spf=pass (imf17.hostedemail.com: domain of linyunsheng@huawei.com designates 45.249.212.187 as permitted sender) smtp.mailfrom=linyunsheng@huawei.com; dmarc=pass (policy=quarantine) header.from=huawei.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1724159242; a=rsa-sha256; cv=none; b=wg8g7+CS7FKdcQrPGcWlDMeVxEcZ81e3z3OFdv/l/yanhUEMv7oalXYyTg449nJdL4cqZD Iic4aNRk0Cz4gJ6xy0KPrp0kKzbCXkY0MenBG9X5X9FLWuT+mGS3tqj5k10zpN+0r2IXqC 1P0JpNw7AnRONatNoSA7OMNmCYKcuK0= Received: from mail.maildlp.com (unknown [172.19.88.105]) by szxga01-in.huawei.com (SkyGuard) with ESMTP id 4Wp8rD3W5wzyR2p; Tue, 20 Aug 2024 21:07:08 +0800 (CST) Received: from dggpemf200006.china.huawei.com (unknown [7.185.36.61]) by mail.maildlp.com (Postfix) with ESMTPS id 1EF6B140138; Tue, 20 Aug 2024 21:07:30 +0800 (CST) Received: from [10.67.120.129] (10.67.120.129) by dggpemf200006.china.huawei.com (7.185.36.61) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.2.1544.11; Tue, 20 Aug 2024 21:07:29 +0800 Message-ID: <98ceade3-8d60-45bf-a419-ff3982a96101@huawei.com> Date: Tue, 20 Aug 2024 21:07:29 +0800 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH net-next v13 04/14] mm: page_frag: add '_va' suffix to page_frag API To: Alexander Duyck CC: , , , , , Subbaraya Sundeep , Chuck Lever , Sagi Grimberg , Jeroen de Borst , Praveen Kaligineedi , Shailend Chand , Eric Dumazet , Tony Nguyen , Przemek Kitszel , Sunil Goutham , Geetha sowjanya , hariprasad , Felix Fietkau , Sean Wang , Mark Lee , Lorenzo Bianconi , Matthias Brugger , AngeloGioacchino Del Regno , Keith Busch , Jens Axboe , Christoph Hellwig , Chaitanya Kulkarni , "Michael S. Tsirkin" , Jason Wang , =?UTF-8?Q?Eugenio_P=C3=A9rez?= , Andrew Morton , Alexei Starovoitov , Daniel Borkmann , Jesper Dangaard Brouer , John Fastabend , Andrii Nakryiko , Martin KaFai Lau , Eduard Zingerman , Song Liu , Yonghong Song , KP Singh , Stanislav Fomichev , Hao Luo , Jiri Olsa , David Howells , Marc Dionne , Jeff Layton , Neil Brown , Olga Kornievskaia , Dai Ngo , Tom Talpey , Trond Myklebust , Anna Schumaker , Shuah Khan , , , , , , , , , , , References: <20240808123714.462740-1-linyunsheng@huawei.com> <20240808123714.462740-5-linyunsheng@huawei.com> <676a2a15-d390-48a7-a8d7-6e491c89e200@huawei.com> <3e069c81-a728-4d72-a5bb-3be00d182107@huawei.com> Content-Language: en-US From: Yunsheng Lin In-Reply-To: Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 7bit X-Originating-IP: [10.67.120.129] X-ClientProxiedBy: dggems705-chm.china.huawei.com (10.3.19.182) To dggpemf200006.china.huawei.com (7.185.36.61) X-Rspam-User: X-Stat-Signature: bqxiignp1ddas1emo8qq61m1uubzzp7x X-Rspamd-Queue-Id: 6C4FA40018 X-Rspamd-Server: rspam11 X-HE-Tag: 1724159255-891831 X-HE-Meta: U2FsdGVkX19+abwLGzSDlsPEy40eKdoOWcpJgfuvqgo6cDwFKTX4j3aNRtMB1maRsl4J+aUyQcj9hQkZbjY9Qmw/0Fk4ETvZ4J7vj2GfEbyEMMQRfX89V/xzxHWeUc+4I6LbZAsrCVoXDhJyEXWFrF9pmXBBZoBGd4EqAHs0QJad8hVO/VqsZfZwmMGN0SlddJ72+nGrzEdgQl1bEWJ+I679GjqdbHdWSda+jxhJTkjBPtwJBdG/ycAyzl466pOuSSaN7emm2srCVEFNK3bzU127rMZ3XrC+pZAcxGRr+IVc8sZJOgpdurXq2Y98a30VCyn+xQRVGMd0AcNEAVu0CEZU+Qyipm94zRWYO1Igf783nWlBXCqyE8P4zMBIgxVgenpAt9SDGsF5HpcQvJXPCqmFwK7n/nAlVps9T9PsJXbbFI3FyzQFGhQaCpcKeIY9XnFUKTap0Fh2EDppx+Tu5Iv5T7uOuhkHhLsl90tThlwsiX5ce2T3aPt+OjLAJtAjbV1JXnQn0XPHVpe35tzDvRhvs8CkGlgpFDVNTgMqfJZBY6bMTdCA7YkUcVVGlnsRjIZd051QU+zaT5PVGdnnvDko1ObJiXMo2lYavCKzrZJPVCOGJEhPgKspEzCbmHEKNMkDAxuSOpa/se7/ZUL3cBJUBl+ahktPQ26+UoiIAvPYRoyscLBUQGUW21gG9zZHs7kSLrEOwsDkDmev/EjYIU05nPvv8g5pEIGDpITvmbwi/o1H6jlMreDdW6n7/DqXNVsTjQU9NcjlMS9ir4SKN8vzLGsfxMcEgth4CvhLPG5xecEQFjTR5e9oSd/KR4rR93bRFgIKNyejJ3oWJZRO9nk0+BZZGc9NUTzgE8ko0fuyf7KHNby/Wiu27sk9uuT1Wzzz+QpMKkH7MXHxI+pRCMi2ppdBAXibCMsBkowFuFd/SBZ1FYaPpwoeUFLg6PphDDNmv3PC6G6ax1COLPm A71bOkr6 fozxi8UtWJBOSCIIR28S7ox5NWRfqTW2ASrmYXT3N80Er6mxfAwHZXEXPl1zVQc178JwXCpZStNf8BPMVEROD5Y6vPLvmxZ3SLWJHdLyKtt+7smWoKs8VFceDz9eLNOvBp62F4EhfUc4oc4ZZT3LkzxN1g2G3NWRO0WePyjMoiN28hy0KHOyXIMdBLaF48vy6zI5utH/iIoQTemrFNrMbwnyo3m3T1Evc4jXiTLMHc1Hz4iOSRWRYl69jVNVQWiLdD2T72XViap7zl8RtvTXdS8KnlcZJ17nWHeIG X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: List-Subscribe: List-Unsubscribe: On 2024/8/19 23:54, Alexander Duyck wrote: ... >>>> >>>> "There are three types of API as proposed in this patchset instead of >>>> two types of API: >>>> 1. page_frag_alloc_va() returns [va]. >>>> 2. page_frag_alloc_pg() returns [page, offset]. >>>> 3. page_frag_alloc() returns [va] & [page, offset]. >>>> >>>> You seemed to miss that we need a third naming for the type 3 API. >>>> Do you see type 3 API as a valid API? if yes, what naming are you >>>> suggesting for it? if no, why it is not a valid API?" >>> >>> I didn't. I just don't see the point in pushing out the existing API >>> to support that. In reality 2 and 3 are redundant. You probably only >>> need 3. Like I mentioned earlier you can essentially just pass a >> >> If the caller just expect [page, offset], do you expect the caller also >> type 3 API, which return both [va] and [page, offset]? >> >> I am not sure if I understand why you think 2 and 3 are redundant here? >> If you think 2 and 3 are redundant here, aren't 1 and 3 also redundant >> as the similar agrument? > > The big difference is the need to return page and offset. Basically to > support returning page and offset you need to pass at least one value > as a pointer so you can store the return there. > > The reason why 3 is just a redundant form of 2 is that you will > normally just be converting from a va to a page and offset so the va > should already be easily accessible. I am assuming that by 'easily accessible', you meant the 'va' can be calculated as below, right? va = encoded_page_address(encoded_va) + (page_frag_cache_page_size(encoded_va) - remaining); I guess it is easily accessible, but it is not without some overhead to calculate the 'va' here. > >>> page_frag via pointer to the function. With that you could also look >>> at just returning a virtual address as well if you insist on having >>> something that returns all of the above. No point in having 2 and 3 be >>> seperate functions. >> >> Let's be more specific about what are your suggestion here: which way >> is the prefer way to return the virtual address. It seems there are two >> options: >> >> 1. Return the virtual address by function returning as below: >> void *page_frag_alloc_bio(struct page_frag_cache *nc, struct bio_vec *bio); >> >> 2. Return the virtual address by double pointer as below: >> int page_frag_alloc_bio(struct page_frag_cache *nc, struct bio_vec *bio, >> void **va); > > I was thinking more of option 1. Basically this is a superset of > page_frag_alloc_va that is also returning the page and offset via a > page frag. However instead of bio_vec I would be good with "struct > page_frag *" being the value passed to the function to play the role > of container. Basically the big difference between 1 and 2/3 if I am > not mistaken is the fact that for 1 you pass the size, whereas with > 2/3 you are peeling off the page frag from the larger page frag cache Let's be clear here: The callers just expecting [page, offset] also need to call type 3 API, which return both [va] and [page, offset]? and it is ok to ignore the overhead of calculating the 'va' for those kinds of callers just because we don't want to do the renaming for a existing API and can't come up with good naming for that? > after the fact via a commit type action. Just be clear here, there is no commit type action for some subtype of type 2/3 API. For example, for type 2 API in this patchset, it has below subtypes: subtype 1: it does not need a commit type action, it just return [page, offset] instead of page_frag_alloc_va() returning [va], and it does not return the allocated fragsz back to the caller as page_frag_alloc_va() does not too: struct page *page_frag_alloc_pg(struct page_frag_cache *nc, unsigned int *offset, unsigned int fragsz, gfp_t gfp) subtype 2: it does need a commit type action, and @fragsz is returned to the caller and caller used that to commit how much fragsz to commit. struct page *page_frag_alloc_pg_prepare(struct page_frag_cache *nc, unsigned int *offset, unsigned int *fragsz, gfp_t gfp) Do you see subtype 1 as valid API? If no, why? If yes, do you also expect the caller to use "struct page_frag *" as the container? If yes, what is the caller expected to do with the size field in "struct page_frag *" from API perspective? Just ignore it?