Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   Song Liu <songliubraving@meta.com>
To:     "Edgecombe, Rick P" <rick.p.edgecombe@intel.com>
CC:     Song Liu <songliubraving@meta.com>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "peterz@infradead.org" <peterz@infradead.org>,
        Kernel Team <Kernel-team@fb.com>,
        "linux-mm@kvack.org" <linux-mm@kvack.org>,
        "song@kernel.org" <song@kernel.org>, "hch@lst.de" <hch@lst.de>,
        "x86@kernel.org" <x86@kernel.org>,
        "akpm@linux-foundation.org" <akpm@linux-foundation.org>,
        "Hansen, Dave" <dave.hansen@intel.com>,
        "urezki@gmail.com" <urezki@gmail.com>
Subject: Re: [RFC v2 4/4] vmalloc_exec: share a huge page with kernel text
Thread-Topic: [RFC v2 4/4] vmalloc_exec: share a huge page with kernel text
Thread-Index: AQHY2qb4/DleVypvfEGw7i865KJ5wq4H9+4AgAAKPACAABD6AIABU84AgABHRYCAAJX7gA==
Date:   Wed, 12 Oct 2022 05:37:43 +0000
Message-ID: <0D4668C5-28C1-4846-9698-C5C05BC23F0B@fb.com>
References: <20221007234315.2877365-1-song@kernel.org>
 <20221007234315.2877365-5-song@kernel.org>
 <af5c148fb3b9c56d1eb7e92b05db7c6c04bcfc3c.camel@intel.com>
 <B849A9FF-1054-4D5C-AEBD-72AED8B8F277@fb.com>
 <3842f1e7cfdde4f848e164872f62c0c1da654fec.camel@intel.com>
 <2B66E2E7-7D32-418C-9DFD-1E17180300B4@fb.com>
 <99201f0c3509e1ea3d08a462beaaea9d60382cff.camel@intel.com>
In-Reply-To: <99201f0c3509e1ea3d08a462beaaea9d60382cff.camel@intel.com>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?iWVj+166mfCScUvQWkLmgmqapHuVGikWFFaM/vgC80uYVDqi5Yph2wXT2a67?=
 =?us-ascii?Q?AJuiUnjCZI+b1GTlb06bp3TZZa9J1cb0UEM302+bdjMGKASRE8Zl3eU/8bIS?=
 =?us-ascii?Q?2Exg5UYexfjO4cG4Jt4ZCR20VOEjyHH1/p3VcTLyItdvjk/FLxqlBPQjqhMV?=
 =?us-ascii?Q?hIqZHUhP91aUM3M1InBoeMtDCZ+RbipQtiBcRfjGtOH+eJA0HtU5enwKtP4e?=
 =?us-ascii?Q?RdJK1YVWOTQD7Io/3t+bKgJlB+il+JdttCA6br6AlMCKY70MAvWUTRnjmiwv?=
 =?us-ascii?Q?SI9DmikwSDCQplfxRx/P31dUvAHeXv/jyUfSax6Cl1RXgMrAIghur/TS5j8v?=
 =?us-ascii?Q?bAJasRmpjVb2aUStiQ64iu+A71ALi4kpQlV4hWjp+BUgYf8/91g8UmuuVM+L?=
 =?us-ascii?Q?y9yEZObLXvrFLrVwgBYYxENM80KBFEShNuMyU30fG4TpRogeDFaOppyUBVDV?=
 =?us-ascii?Q?KJmULZXD5YpQBtESx7dP0J6lmkXqrN4vABGeoE6bgXKFwXoRh84eiCvA69/f?=
 =?us-ascii?Q?tZGrL0ZsoGWR7tdcSjxrgpVQUJalx07XZPpPwmPfRF65TGZxSkHeT9C5kX3v?=
 =?us-ascii?Q?oYhRb1kE/c2tig6WWP8Vjr1P0ypX5U81tz0Ir7tBMnxIWqs094f+WETX16S3?=
 =?us-ascii?Q?HY32YvY7CIjgMAvO0u+vPAU3Lkp0wmPnKoUpWuqnIDMf5z/nFChpeHlsx7Th?=
 =?us-ascii?Q?v+f8f2D1NFO3cO+81htjOCmf27+zJ0wtwrCcmC2+yYjnlV9No3bdvw0vCU9o?=
 =?us-ascii?Q?uu6qYLoNLFVH17hnUnsvRBEEO0GY11p1FSxm+X2Ki9L5rrqHWplX+zu65bd4?=
 =?us-ascii?Q?Qt5YO8YzTzP49GOimDYRcrl5d24I5vKnxWjI6N5EhGnojbeQ9yhzr9A5jLjC?=
 =?us-ascii?Q?QhkiwkK8PdOn5q5It6rav4lH1jSR9RDIMlE0EtwAfN1+09Zd+tRrN6zo2lLE?=
 =?us-ascii?Q?L746PM9E7AgLHnJgGAhWqAho4aLir8sJze/5UCDS6AFJ1zLXf5oMURYk3B+t?=
 =?us-ascii?Q?dsM4xGRiT4ExhDRqh0Tvzj7D9qT+gznyNiSpsca21OW6WPHfpHiGmRVluao9?=
 =?us-ascii?Q?U15fZnNtOalQV8ov17VvdlvyMF9L4nYX07glgNO/Vt+VYDsWCGwu0+DMcsjb?=
 =?us-ascii?Q?3LUbLkvWF7Z5wveiFG+uORKZ44nZqQtjhE/qa/1LqilW/KVM7W3IDgEQOS2s?=
 =?us-ascii?Q?YYRpfOv6EKEWDzUeppslbWd/FeAe8qAa90wQf2abuTy2P6GkP74QGyVx5dYU?=
 =?us-ascii?Q?jg3Q6ByhfSmPH6kmz1x0/FQcXivs6f3MOC8UJzILnWpx7xEazHbthdMSg7I0?=
 =?us-ascii?Q?O2b1qVp+tTPpYZqz/y4NYYbhBFxG+ejPhTG6ga113gFpglaKqO4bg5RXWF7N?=
 =?us-ascii?Q?Slm1uee78cfPH6UinwjWgeRS+ifybaAZEOZqMbZEr3MBXMbvsXEqaZ9ONcYC?=
 =?us-ascii?Q?N2iY8zsZVhQlhOejuwkp7yL0kZOv3njiXXYMAlDiEoejZoDFSH8LKxazVruB?=
 =?us-ascii?Q?bElyFpCXvCl+tFnjl1mFH7lzRppR9GyBZLeykix+EhP7jthjtaaG/irpPQID?=
 =?us-ascii?Q?8nD4A5FRLeC7mVawMpaTDNPZZ2R6eDgchjPciixk3CpNylDd91HZXIc2Fbdb?=
 =?us-ascii?Q?TxN4fVI+kBwNdRIaoAD3auCAq8FO9EZ8NyXa4Sh5v4nF?=
Content-Type: text/plain; charset="us-ascii"
Content-ID: <F9B4057D2B861E4C94FB081223F22CBD@namprd15.prod.outlook.com>
MIME-Version: 1.0
X-OriginatorOrg: meta.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: SA1PR15MB5109.namprd15.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 4b1de4f2-35df-4579-c9be-08daac13e313
X-MS-Exchange-CrossTenant-originalarrivaltime: 12 Oct 2022 05:37:44.0115
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 8ae927fe-1255-47a7-a2af-5f3a069daaa2
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: d+Qqiy6SerHkEjqQKDzM89+VHAwnjguqL8z+sSMlRriv3qOOdKV93zb8hUe8NrdAaqpYbwzwJPyl1hrr1e2MjQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SJ0PR15MB4711
X-Proofpoint-GUID: Jt8rQkrPoP2r4XwOIkqwXlz39CHzcmjf
X-Proofpoint-ORIG-GUID: Jt8rQkrPoP2r4XwOIkqwXlz39CHzcmjf
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.205,Aquarius:18.0.895,Hydra:6.0.545,FMLib:17.11.122.1
 definitions=2022-10-12_03,2022-10-11_02,2022-06-22_01
X-Spam-Status: No, score=-2.8 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_LOW,
        RCVD_IN_MSPIKE_H3,RCVD_IN_MSPIKE_WL,SPF_HELO_NONE,SPF_NONE
        autolearn=ham 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-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org


> On Oct 11, 2022, at 1:40 PM, Edgecombe, Rick P <rick.p.edgecombe@intel.com> wrote:
> 
> On Tue, 2022-10-11 at 16:25 +0000, Song Liu wrote:
>>> Maybe this is just me missing some vmalloc understanding, but this
>>> pointer to an all zero vm_struct seems weird too. Are there other
>>> vmap
>>> allocations like this? Which vmap APIs work with this and which
>>> don't?
>> 
>> There are two vmap trees at the moment: free_area_ tree and 
>> vmap_area_ tree. free_area_ tree uses vmap->subtree_max_size, while 
>> vmap_area_ tree contains vmap backed by vm_struct, and thus uses 
>> vmap->vm. 
>> 
>> This set add a new tree, free_text_area_. This tree is different to 
>> the other two, as it uses subtree_max_size, and it is also backed 
>> by vm_struct. To handle this requirement without growing vmap_struct,
>> we introduced all_text_vm to store the vm_struct for free_text_area_
>> tree. 
>> 
>> free_text_area_ tree is different to vmap_area_ tree. Each vmap in
>> vmap_area_ tree has its own vm_struct (1 to 1 mapping), while 
>> multiple vmap in free_text_area_ tree map to a single vm_struct.
>> 
>> Also, free_text_area_ handles granularity < PAGE_SIZE; while the
>> other two trees only work with PAGE_SIZE aligned memory. 
>> 
>> Does this answer your questions? 
> 
> I mean from the perspective of someone trying to use this without
> diving into the entire implementation.
> 
> The function is called vmalloc_exec() and is freed with vfree_exec().
> Makes sense. But with the other vmallocs_foo's (including previous
> vmalloc_exec() implementations) you can call find_vm_area(), etc on
> them. They show in "vmallocinfo" and generally behave similarly. That
> isn't true for these new allocations, right?

That's right. These operations are not supported (at least for now). 

> 
> Then you have code that operates on module text like:
> if (is_vmalloc_or_module_addr(addr))
> 	pfn = vmalloc_to_pfn(addr);
> 
> It looks like it would work (on x86 at least). Should it be expected
> to?
> 
> Especially after this patch, where there is memory that isn't even
> tracked by the original vmap_area trees, it is pretty much a separate
> allocator. So I think it might be nice to spell out which other vmalloc
> APIs work with these new functions since they are named "vmalloc".
> Maybe just say none of them do.

I guess it is fair to call this a separate allocator. Maybe 
vmalloc_exec is not the right name? I do think this is the best 
way to build an allocator with vmap tree logic. 

> 
> 
> Separate from that, I guess you are planning to make this limited to
> certain architectures? It might be better to put logic with assumptions
> about x86 boot time page table details inside arch/x86 somewhere.

Yes, the architecture need some text_poke mechanism to use this. 
On BPF side, x86_64 calls this directly from arch code (jit engine), 
so it is mostly covered. For modules, we need to handle this better. 

Thanks,
Song