On 12/10/21 11:44, Hyeonggon Yoo wrote:
> On Wed, Dec 01, 2021 at 07:15:01PM +0100, Vlastimil Babka wrote:
>> From: "Matthew Wilcox (Oracle)" <willy@xxxxxxxxxxxxx>
>>
>> Use struct slab throughout the slob allocator.
>>
>> [ vbabka@xxxxxxx: don't introduce wrappers for PageSlobFree in mm/slab.h just
>> for the single callers being wrappers in mm/slob.c ]
>>
>> Signed-off-by: Matthew Wilcox (Oracle) <willy@xxxxxxxxxxxxx>
>> Signed-off-by: Vlastimil Babka <vbabka@xxxxxxx>
>> ---
>> mm/slob.c | 34 +++++++++++++++++-----------------
>> 1 file changed, 17 insertions(+), 17 deletions(-)
>>
>> diff --git a/mm/slob.c b/mm/slob.c
>> index d2d15e7f191c..d3512bcc3141 100644
>> --- a/mm/slob.c
>> +++ b/mm/slob.c
>
> ...
>
>> /* Enough room on this page? */
>> @@ -358,8 +358,8 @@ static void *slob_alloc(size_t size, gfp_t gfp, int align, int node,
>> b = slob_new_pages(gfp & ~__GFP_ZERO, 0, node);
>> if (!b)
>> return NULL;
>> - sp = virt_to_page(b);
>> - __SetPageSlab(sp);
>> + sp = virt_to_slab(b);
>> + __SetPageSlab(slab_page(sp));
>
> Hello Vlastimil.
>
> I've tested this patch on my machine and it causes NULL pointer dereference.
> that's because virt_to_slab returns NULL if folio_test_slab is false.
> and __SetPageSlab is called with sp = NULL.
Oops, thanks for catching this.
> diff below fixed bug.
Changed it to use folio and will push an updated branch
slab-struct_slab-v3r2
Thanks!
> diff --git a/mm/slob.c b/mm/slob.c
> index d3512bcc3141..cf669f03440f 100644
> --- a/mm/slob.c
> +++ b/mm/slob.c
> @@ -358,8 +358,8 @@ static void *slob_alloc(size_t size, gfp_t gfp, int a
> lign, int node,
> b = slob_new_pages(gfp & ~__GFP_ZERO, 0, node);
> if (!b)
> return NULL;
> + __SetPageSlab(virt_to_page(b));
> sp = virt_to_slab(b);
> - __SetPageSlab(slab_page(sp));
>
> spin_lock_irqsave(&slob_lock, flags);
> sp->units = SLOB_UNITS(PAGE_SIZE);
>
> Thanks,
> Hyeonggon.
>
>>
>> spin_lock_irqsave(&slob_lock, flags);
>> sp->units = SLOB_UNITS(PAGE_SIZE);
>> @@ -381,7 +381,7 @@ static void *slob_alloc(size_t size, gfp_t gfp, int align, int node,
>> */
>> static void slob_free(void *block, int size)
>> {
>> - struct page *sp;
>> + struct slab *sp;
>> slob_t *prev, *next, *b = (slob_t *)block;
>> slobidx_t units;
>> unsigned long flags;
>> @@ -391,7 +391,7 @@ static void slob_free(void *block, int size)
>> return;
>> BUG_ON(!size);
>>
>> - sp = virt_to_page(block);
>> + sp = virt_to_slab(block);
>> units = SLOB_UNITS(size);
>>
>> spin_lock_irqsave(&slob_lock, flags);
>> @@ -401,8 +401,8 @@ static void slob_free(void *block, int size)
>> if (slob_page_free(sp))
>> clear_slob_page_free(sp);
>> spin_unlock_irqrestore(&slob_lock, flags);
>> - __ClearPageSlab(sp);
>> - page_mapcount_reset(sp);
>> + __ClearPageSlab(slab_page(sp));
>> + page_mapcount_reset(slab_page(sp));
>> slob_free_pages(b, 0);
>> return;
>> }
>> --
>> 2.33.1
>>
>>
