On Tue, 28 Mar 2023 at 13:55, Marco Elver <elver@xxxxxxxxxx> wrote:
>
> On Tue, 28 Mar 2023 at 11:58, Muchun Song <songmuchun@xxxxxxxxxxxxx> wrote:
> >
> > There are three similar loops to initialize kfence pool, we could merge
> > all of them into one loop to simplify the code and make code more
> > efficient.
> >
> > Signed-off-by: Muchun Song <songmuchun@xxxxxxxxxxxxx>
>
> Reviewed-by: Marco Elver <elver@xxxxxxxxxx>
>
> > ---
> > mm/kfence/core.c | 47 ++++++-----------------------------------------
> > 1 file changed, 6 insertions(+), 41 deletions(-)
> >
> > diff --git a/mm/kfence/core.c b/mm/kfence/core.c
> > index 7d01a2c76e80..de62a84d4830 100644
> > --- a/mm/kfence/core.c
> > +++ b/mm/kfence/core.c
> > @@ -539,35 +539,10 @@ static void rcu_guarded_free(struct rcu_head *h)
> > static unsigned long kfence_init_pool(void)
> > {
> > unsigned long addr = (unsigned long)__kfence_pool;
> > - struct page *pages;
> > int i;
> >
> > if (!arch_kfence_init_pool())
> > return addr;
> > -
> > - pages = virt_to_page(__kfence_pool);
> > -
> > - /*
> > - * Set up object pages: they must have PG_slab set, to avoid freeing
> > - * these as real pages.
> > - *
> > - * We also want to avoid inserting kfence_free() in the kfree()
> > - * fast-path in SLUB, and therefore need to ensure kfree() correctly
> > - * enters __slab_free() slow-path.
> > - */
Actually: can you retain this comment somewhere?
> > - for (i = 0; i < KFENCE_POOL_SIZE / PAGE_SIZE; i++) {
> > - struct slab *slab = page_slab(nth_page(pages, i));
> > -
> > - if (!i || (i % 2))
> > - continue;
> > -
> > - __folio_set_slab(slab_folio(slab));
> > -#ifdef CONFIG_MEMCG
> > - slab->memcg_data = (unsigned long)&kfence_metadata[i / 2 - 1].objcg |
> > - MEMCG_DATA_OBJCGS;
> > -#endif
> > - }
> > -
> > /*
> > * Protect the first 2 pages. The first page is mostly unnecessary, and
> > * merely serves as an extended guard page. However, adding one
> > @@ -581,8 +556,9 @@ static unsigned long kfence_init_pool(void)
> > addr += PAGE_SIZE;
> > }
> >
> > - for (i = 0; i < CONFIG_KFENCE_NUM_OBJECTS; i++) {
> > + for (i = 0; i < CONFIG_KFENCE_NUM_OBJECTS; i++, addr += 2 * PAGE_SIZE) {
> > struct kfence_metadata *meta = &kfence_metadata[i];
> > + struct slab *slab = page_slab(virt_to_page(addr));
> >
> > /* Initialize metadata. */
> > INIT_LIST_HEAD(&meta->list);
> > @@ -593,26 +569,15 @@ static unsigned long kfence_init_pool(void)
> >
> > /* Protect the right redzone. */
> > if (unlikely(!kfence_protect(addr + PAGE_SIZE)))
> > - goto reset_slab;
> > -
> > - addr += 2 * PAGE_SIZE;
> > - }
> > -
> > - return 0;
> > -
> > -reset_slab:
> > - for (i = 0; i < KFENCE_POOL_SIZE / PAGE_SIZE; i++) {
> > - struct slab *slab = page_slab(nth_page(pages, i));
> > + return addr;
> >
> > - if (!i || (i % 2))
> > - continue;
> > + __folio_set_slab(slab_folio(slab));
> > #ifdef CONFIG_MEMCG
> > - slab->memcg_data = 0;
> > + slab->memcg_data = (unsigned long)&meta->objcg | MEMCG_DATA_OBJCGS;
> > #endif
> > - __folio_clear_slab(slab_folio(slab));
> > }
> >
> > - return addr;
> > + return 0;
> > }
> >
> > static bool __init kfence_init_pool_early(void)
> > --
> > 2.11.0
> >
