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@fb.com>
To:     Luis Chamberlain <mcgrof@kernel.org>
CC:     Peter Zijlstra <peterz@infradead.org>,
        Steven Rostedt <rostedt@goodmis.org>,
        Thomas Gleixner <tglx@linutronix.de>,
        Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>,
        Masami Hiramatsu <mhiramat@kernel.org>,
        "Naveen N. Rao" <naveen.n.rao@linux.ibm.com>,
        "David S. Miller" <davem@davemloft.net>,
        Anil S Keshavamurthy <anil.s.keshavamurthy@intel.com>,
        Kees Cook <keescook@chromium.org>, Song Liu <song@kernel.org>,
        bpf <bpf@vger.kernel.org>, Christoph Hellwig <hch@infradead.org>,
        Davidlohr Bueso <dave@stgolabs.net>,
        lkml <linux-kernel@vger.kernel.org>,
        Linux-MM <linux-mm@kvack.org>,
        Daniel Borkmann <daniel@iogearbox.net>,
        Kernel Team <Kernel-team@fb.com>,
        "x86@kernel.org" <x86@kernel.org>,
        "dave.hansen@linux.intel.com" <dave.hansen@linux.intel.com>,
        "rick.p.edgecombe@intel.com" <rick.p.edgecombe@intel.com>,
        "linux-modules@vger.kernel.org" <linux-modules@vger.kernel.org>
Subject: Re: [PATCH v6 bpf-next 0/5] bpf_prog_pack followup
Thread-Topic: [PATCH v6 bpf-next 0/5] bpf_prog_pack followup
Thread-Index: AQHYklIC37qezjyxh0moueFgoO/pNa1zhdgAgAAO4YCAABD0AIAAC/MAgADwxACAAEM5AIAAKMMAgAAvboCABOqLgIAAGVQAgADeG4CAAEVBgIAACGmAgAAV1QA=
Date:   Wed, 13 Jul 2022 01:00:37 +0000
Message-ID: <728C4142-0070-4B85-9085-260EFC959681@fb.com>
References: <YseAEsjE49AZDp8c@bombadil.infradead.org>
 <C96F5607-6FFE-4B45-9A9D-B89E3F67A79A@fb.com>
 <YshUEEQ0lk1ON7H6@bombadil.infradead.org>
 <863A2D5B-976D-4724-AEB1-B2A494AD2BDB@fb.com>
 <YsiupnNJ8WANZiIc@bombadil.infradead.org>
 <6214B9C9-557B-4DC0-BFDE-77EAC425E577@fb.com>
 <Ysz2LX3q2OsaO4gM@bombadil.infradead.org>
 <E23B6EB1-AFFA-4B65-963E-B44BA0F2142D@fb.com>
 <Ys3FvYnASr2v9iPc@bombadil.infradead.org>
 <6CB56563-29E2-4CE0-BF7B-360979E42429@fb.com>
 <Ys4G4/dG6SGYV/iz@bombadil.infradead.org>
In-Reply-To: <Ys4G4/dG6SGYV/iz@bombadil.infradead.org>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?zJM/HR/ST/MsQMid6wGvSYhdx4+zjYmy5L2bu3TbVyhdbdMZiFGmBP+bpcGs?=
 =?us-ascii?Q?oypPMxtAb7pegOK6G+1axMByKd4dmH8J8pJssAtoLbdp5zkd57BQ9T66F2Z0?=
 =?us-ascii?Q?Y2f5QPotR6Ls1WIcUKN8I0C47g3UboTrtYdfwaMigjn+2uB0+y/ndloLwyl0?=
 =?us-ascii?Q?kjkSEORRyRUMyIYdlUkbHxc5RLYVLMqxw1AjsMW8UfvxlroIU1nO470iT63z?=
 =?us-ascii?Q?qmyrjcB9i6xLCF40quU7Erq/8r0cr1dddB5AoY9FNRT2+6+6whvW1thQ+l8l?=
 =?us-ascii?Q?F+najP8/5xpYgt26Le1x3NhbzHa34tGxpYaEVuqrRpfLgxAyzdsiFMfDV+Zn?=
 =?us-ascii?Q?FFDU2sA/2hZG6iSe+Nq4uRC87Ymwx27JiYGeFKvrvVtuQwhnTFcKf1ljk7+u?=
 =?us-ascii?Q?8ZFugXnzcZBM0DlbqZ8NBXyqVWf+7kuZUB6Rci/Laj2KPOLMVUmNhV1W2j+h?=
 =?us-ascii?Q?wrDgpZPi/yKbp/Et3qjVBBFhqqqeLecl5NKeCO/S8Bu36Yc8rGtYTWXQ/S9f?=
 =?us-ascii?Q?tzuSS0yCPYk/bZSD/ibnvHJWd6wRPk1dKQLEvF5y6hufnQqRO3fkwd/EO4ma?=
 =?us-ascii?Q?urMCl56eZTOpn3sSWsMQOsQlk+2GU+vZ3QUJ3/Die7jCmudcKWsK3O8c0yeP?=
 =?us-ascii?Q?S0X/hoffk6IV7nV3x/ZwGIg9tG/AxzN0V6dTganb7e43Whf/plKwjpgX5Jpv?=
 =?us-ascii?Q?ciwBQsqWxp6W12zB48wZ8VaLqYKSdX8oq9NffaKLM6+UPo/VmBrvOgtmUu67?=
 =?us-ascii?Q?Nn9HCFI6/RFVyCO4YGqyij/5z4EI2JC81s6Nm9x4liWPOUTf+rM6FQbNUdGG?=
 =?us-ascii?Q?WAEPC4SkINtIIFoJ43UMNyhF3gcUq+HrrVTqFtAKg59ens6OeHIGNW0iZyNu?=
 =?us-ascii?Q?aesAGC1EfpuZEOCfC4whmA3+UQaFsCLXSgxghq46psmjWxmJx5Bagk9qYZ4u?=
 =?us-ascii?Q?GsFBqUBwxKN+pwmUYWerkpNAHQXzFYd1PPyyHI4j+Mtr/d30EAjLYqwFjFrm?=
 =?us-ascii?Q?t9KeUIjjfB5VelZG2CJzUwwU1zjgO0+JfB1RtQ5/PGWX7riR1n+/4X0H08wn?=
 =?us-ascii?Q?9r49SNKs2KZwGi2UFBtHDWMZaRqmCYRRR2tjsJZf2eHuzFib8JGG9WsEmnvS?=
 =?us-ascii?Q?eWm0P722aswE68L52D2kxFcIKpfd0VSdkZ/JYNTasVCFQVoh0B+zl3M8wJhA?=
 =?us-ascii?Q?yIQCapgF4uJMt/FnBjKCVVIIam+egbTb6a4zFkRQnG4h8Ojhu+tCKD30Cdeb?=
 =?us-ascii?Q?k1mS5PKihSwLDmc972Kgs9wC9o953ixyO9yY96QvPYaOPCUScvbbgSOsAVi4?=
 =?us-ascii?Q?a0jbzJoyIMycxOqfulqdY+K785ML1LKLT7QPUSVFfvVeHi1LTa9IoL6rLwCU?=
 =?us-ascii?Q?Tbj/uGAdHTzGiJJsgReFGhNFRQGwqEbD+jNI5Nsrdd+AJiBk1RlH325nsXfR?=
 =?us-ascii?Q?s5yygJKkHMTarjyEfrlSUhVLtrTtsQk6zU+sXmYxUKh3b91O8c4dzcMLi7bW?=
 =?us-ascii?Q?2qRWpveR0N4TEkoknKsGLgHmz798UDafZncVGeWsNXE06YZ3dBjYonEf7htV?=
 =?us-ascii?Q?yCXmKMIdPz8R+BSzOTflIbiAJZo/vLw8ISrU8AoudaO7S1CWkRVxvGGjMKHR?=
 =?us-ascii?Q?/oScHCX33l5SR3xQGLtvXFY=3D?=
Content-Type: text/plain; charset="us-ascii"
Content-ID: <29F3282AA9BBE54E93CE7512CE4F72FA@namprd15.prod.outlook.com>
MIME-Version: 1.0
X-OriginatorOrg: fb.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: SA1PR15MB5109.namprd15.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 7ae795c5-22b8-4475-f63c-08da646b1962
X-MS-Exchange-CrossTenant-originalarrivaltime: 13 Jul 2022 01:00:37.6383
 (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: pd/iiYBWmAYHhOTNNMkVg2mOpSAO6kOzUugy9ttzu9fxQduSzz0pTvWWGxh1PjnA/g8Sj5SrUdMwaQbPPSRScA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MN2PR15MB2813
X-Proofpoint-ORIG-GUID: izKhQGTm_14PMeCmgKs3m-f7mGXDNq6n
X-Proofpoint-GUID: izKhQGTm_14PMeCmgKs3m-f7mGXDNq6n
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.205,Aquarius:18.0.883,Hydra:6.0.517,FMLib:17.11.122.1
 definitions=2022-07-12_14,2022-07-12_01,2022-06-22_01
X-Spam-Status: No, score=-2.7 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_NONE,
        RCVD_IN_MSPIKE_H3,RCVD_IN_MSPIKE_WL,SPF_HELO_NONE,SPF_NONE,
        T_SCC_BODY_TEXT_LINE 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 Jul 12, 2022, at 4:42 PM, Luis Chamberlain <mcgrof@kernel.org> wrote:
> 
> On Tue, Jul 12, 2022 at 11:12:22PM +0000, Song Liu wrote:
>> 
>> 
>>> On Jul 12, 2022, at 12:04 PM, Luis Chamberlain <mcgrof@kernel.org> wrote:
>>> 
>>> On Tue, Jul 12, 2022 at 05:49:32AM +0000, Song Liu wrote:
>>>>> On Jul 11, 2022, at 9:18 PM, Luis Chamberlain <mcgrof@kernel.org> wrote:
>>>> 
>>>>> I believe you are mentioning requiring text_poke() because the way
>>>>> eBPF code uses the module_alloc() is different. Correct me if I'm
>>>>> wrong, but from what I gather is you use the text_poke_copy() as the data
>>>>> is already RO+X, contrary module_alloc() use cases. You do this since your
>>>>> bpf_prog_pack_alloc() calls set_memory_ro() and set_memory_x() after
>>>>> module_alloc() and before you can use this memory. This is a different type
>>>>> of allocator. And, again please correct me if I'm wrong but now you want to
>>>>> share *one* 2 MiB huge-page for multiple BPF programs to help with the
>>>>> impact of TLB misses.
>>>> 
>>>> Yes, sharing 1x 2MiB huge page is the main reason to require text_poke. 
>>>> OTOH, 2MiB huge pages without sharing is not really useful. Both kprobe
>>>> and ftrace only uses a fraction of a 4kB page. Most BPF programs and 
>>>> modules cannot use 2MiB either. Therefore, vmalloc_rw_exec() doesn't add
>>>> much value on top of current module_alloc(). 
>>> 
>>> Thanks for the clarification.
>>> 
>>>>> A vmalloc_ro_exec() by definition would imply a text_poke().
>>>>> 
>>>>> Can kprobes, ftrace and modules use it too? It would be nice
>>>>> so to not have to deal with the loose semantics on the user to
>>>>> have to use set_vm_flush_reset_perms() on ro+x later, but
>>>>> I think this can be addressed separately on a case by case basis.
>>>> 
>>>> I am pretty confident that kprobe and ftrace can share huge pages with 
>>>> BPF programs.
>>> 
>>> Then wonderful, we know where to go in terms of a new API then as it
>>> can be shared in the future for sure and there are gains.
>>> 
>>>> I haven't looked into all the details with modules, but 
>>>> given CONFIG_ARCH_WANTS_MODULES_DATA_IN_VMALLOC, I think it is also 
>>>> possible.
>>> 
>>> Sure.
>>> 
>>>> Once this is done, a regular system (without huge BPF program or huge
>>>> modules) will just use 1x 2MB page for text from module, ftrace, kprobe, 
>>>> and bpf programs. 
>>> 
>>> That would be nice, if possible, however modules will require likely its
>>> own thing, on my system I see about 57 MiB used on coresize alone.
>>> 
>>> lsmod | grep -v Module | cut -f1 -d ' ' | \
>>> 	xargs sudo modinfo | grep filename | \
>>> 	grep -o '/.*' | xargs stat -c "%s - %n" | \
>>> 	awk 'BEGIN {sum=0} {sum+=$1} END {print sum}'
>>> 60001272
>>> 
>>> And so perhaps we need such a pool size to be configurable.
>>> 
>>>>> But a vmalloc_ro_exec() with a respective free can remove the
>>>>> requirement to do set_vm_flush_reset_perms().
>>>> 
>>>> Removing the requirement to set_vm_flush_reset_perms() is the other
>>>> reason to go directly to vmalloc_ro_exec(). 
>>> 
>>> Yes fantastic.
>>> 
>>>> My current version looks like this:
>>>> 
>>>> void *vmalloc_exec(unsigned long size);
>>>> void vfree_exec(void *ptr, unsigned int size);
>>>> 
>>>> ro is eliminated as there is no rw version of the API. 
>>> 
>>> Alright.
>>> 
>>> I am not sure if 2 MiB will suffice given what I mentioned above, and
>>> what to do to ensure this grows at a reasonable pace. Then, at least for
>>> usage for all architectures since not all will support text_poke() we
>>> will want to consider a way to make it easy to users to use non huge
>>> page fallbacks, but that would be up to those users, so we can wait for
>>> that.
>> 
>> We are not limited to 2MiB total. The logic is like: 
>> 
>> 1. Anything bigger than 2MiB gets its own allocation.
> 
> And does that allocation get split up into a few huge 2 MiB pages?
> When freed does that go into the pool of available list of 2 MiB pages
> to use?

This would have some 2MiB pages and some 4kB pages. For example, if we 
need 4MiB + 5kB, it will allocate 2x 2MiB pages, and 2x 4kB pages (round
up to 8kB). 

On free, the will not go to the pool. Instead, it will be vfree()'ed. 

> 
>> 2. We maintain a list of 2MiB pages, and bitmaps showing which parts of 
>>   these pages are in use. 
> 
> How many 2 MiB huge pages are allocated initially? Do we have a cap?

Current logic just allocates 1 huge page at a time. No cap. 

> 
>> 3. For objects smaller than 2MiB, we will try to fit it in one of these
>>   pages. 
>>   3. a) If there isn't a page with big enough continuous free space, we
>>        will allocate a new 2MiB page. 
>> 
>> (For system with n NUMA nodes, multiple 2MiB above by n). 
>> 
>> So, if we have 100 kernel modules using 1MiB each, they will share 50x
>> 2MiB pages. 
> 
> lsmod | grep -v Module | cut -f1 -d ' ' | \
> 	xargs sudo modinfo | grep filename |\
> 	grep -o '/.*' | xargs stat -c "%s - %n" | \
> 	awk 'BEGIN {sum=0} {sum+=$1} END {print sum/NR/1024}' 
> 271.273
> 
> On average my system's modules are 271 KiB.
> 
> Then I only have 6 out of 216 modules which are use more than 2 MiB or
> memory for coresize. So roughly 97% of my modules would be covered
> with this. Not bad.

Are these all the modules we have in tree? ;)

Thanks,
Song

> 
> The monsters:
> 
> lsmod | grep -v Module | cut -f1 -d ' ' | xargs sudo modinfo \
> 	| grep filename |grep -o '/.*' | xargs stat -c "%s %n" | \
> 	sort -n -k 1 -r | head -10 | \
> 	awk '{print $1/1024/1024" "$2}'
> 6.50775 /lib/modules/5.17.0-1-amd64/kernel/drivers/gpu/drm/i915/i915.ko
> 3.6847 /lib/modules/5.17.0-1-amd64/kernel/fs/xfs/xfs.ko
> 3.34252 /lib/modules/5.17.0-1-amd64/kernel/fs/btrfs/btrfs.ko
> 2.37677 /lib/modules/5.17.0-1-amd64/kernel/net/mac80211/mac80211.ko
> 2.2972 /lib/modules/5.17.0-1-amd64/kernel/net/wireless/cfg80211.ko
> 2.05754 /lib/modules/5.17.0-1-amd64/kernel/arch/x86/kvm/kvm.ko
> 1.96126 /lib/modules/5.17.0-1-amd64/kernel/net/bluetooth/bluetooth.ko
> 1.83429 /lib/modules/5.17.0-1-amd64/kernel/fs/ext4/ext4.ko
> 1.7724 /lib/modules/5.17.0-1-amd64/kernel/fs/nfsd/nfsd.ko
> 1.60539 /lib/modules/5.17.0-1-amd64/kernel/net/sunrpc/sunrpc.ko
> 
> On a big iron server I have 149 modules and the situation is better
> there:
> 
> 3.69791 /lib/modules/5.16.0-6-amd64/kernel/fs/xfs/xfs.ko
> 3.35575 /lib/modules/5.16.0-6-amd64/kernel/fs/btrfs/btrfs.ko
> 3.21056 /lib/modules/5.16.0-6-amd64/kernel/drivers/net/ethernet/mellanox/mlx5/core/mlx5_core.ko
> 2.02773 /lib/modules/5.16.0-6-amd64/kernel/arch/x86/kvm/kvm.ko
> 1.82574 /lib/modules/5.16.0-6-amd64/kernel/fs/ext4/ext4.ko
> 1.36571 /lib/modules/5.16.0-6-amd64/kernel/net/sunrpc/sunrpc.ko
> 1.32686 /lib/modules/5.16.0-6-amd64/kernel/fs/nfsd/nfsd.ko
> 1.12648 /lib/modules/5.16.0-6-amd64/kernel/drivers/gpu/drm/drm.ko
> 0.898623 /lib/modules/5.16.0-6-amd64/kernel/drivers/infiniband/hw/mlx5/mlx5_ib.ko
> 0.86922 /lib/modules/5.16.0-6-amd64/kernel/drivers/infiniband/core/ib_core.ko
> 
> So this may just work nicely.
> 
>  Luis