lkml.org 
[lkml]   [2020]   [Nov]   [28]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    SubjectRe: scheduling while atomic in z3fold
    On Sat, Nov 28, 2020 at 03:09:24PM +0100, Oleksandr Natalenko wrote:
    > > While running v5.10-rc5-rt11 I bumped into the following:
    > >
    > > ```
    > > BUG: scheduling while atomic: git/18695/0x00000002
    > > Preemption disabled at:
    > > [<ffffffffbb93fcb3>] z3fold_zpool_malloc+0x463/0x6e0
    > > …
    > > Call Trace:
    > > dump_stack+0x6d/0x88
    > > __schedule_bug.cold+0x88/0x96
    > > __schedule+0x69e/0x8c0
    > > preempt_schedule_lock+0x51/0x150
    > > rt_spin_lock_slowlock_locked+0x117/0x2c0
    > > rt_spin_lock_slowlock+0x58/0x80
    > > rt_spin_lock+0x2a/0x40
    > > z3fold_zpool_malloc+0x4c1/0x6e0
    > > zswap_frontswap_store+0x39c/0x980
    > > __frontswap_store+0x6e/0xf0
    > > swap_writepage+0x39/0x70
    > > shmem_writepage+0x31b/0x490
    > > pageout+0xf4/0x350
    > > shrink_page_list+0xa28/0xcc0
    > > shrink_inactive_list+0x300/0x690
    > > shrink_lruvec+0x59a/0x770
    > > shrink_node+0x2d6/0x8d0
    > > do_try_to_free_pages+0xda/0x530
    > > try_to_free_pages+0xff/0x260
    > > __alloc_pages_slowpath.constprop.0+0x3d5/0x1230
    > > __alloc_pages_nodemask+0x2f6/0x350
    > > allocate_slab+0x3da/0x660
    > > ___slab_alloc+0x4ff/0x760
    > > __slab_alloc.constprop.0+0x7a/0x100
    > > kmem_cache_alloc+0x27b/0x2c0
    > > __d_alloc+0x22/0x230
    > > d_alloc_parallel+0x67/0x5e0
    > > __lookup_slow+0x5c/0x150
    > > path_lookupat+0x2ea/0x4d0
    > > filename_lookup+0xbf/0x210
    > > vfs_statx.constprop.0+0x4d/0x110
    > > __do_sys_newlstat+0x3d/0x80
    > > do_syscall_64+0x33/0x40
    > > entry_SYSCALL_64_after_hwframe+0x44/0xa9
    > > ```
    > >
    > > The preemption seems to be disabled here:
    > >
    > > ```
    > > $ scripts/faddr2line mm/z3fold.o z3fold_zpool_malloc+0x463
    > > z3fold_zpool_malloc+0x463/0x6e0:
    > > add_to_unbuddied at mm/z3fold.c:645
    > > (inlined by) z3fold_alloc at mm/z3fold.c:1195
    > > (inlined by) z3fold_zpool_malloc at mm/z3fold.c:1737
    > > ```
    > >
    > > The call to the rt_spin_lock() seems to be here:
    > >
    > > ```
    > > $ scripts/faddr2line mm/z3fold.o z3fold_zpool_malloc+0x4c1
    > > z3fold_zpool_malloc+0x4c1/0x6e0:
    > > add_to_unbuddied at mm/z3fold.c:649
    > > (inlined by) z3fold_alloc at mm/z3fold.c:1195
    > > (inlined by) z3fold_zpool_malloc at mm/z3fold.c:1737
    > > ```
    > >
    > > Or, in source code:
    > >
    > > ```
    > > 639 /* Add to the appropriate unbuddied list */
    > > 640 static inline void add_to_unbuddied(struct z3fold_pool *pool,
    > > 641 struct z3fold_header *zhdr)
    > > 642 {
    > > 643 if (zhdr->first_chunks == 0 || zhdr->last_chunks == 0 ||
    > > 644 zhdr->middle_chunks == 0) {
    > > 645 struct list_head *unbuddied = get_cpu_ptr(pool->unbuddied);
    > > 646
    > > 647 int freechunks = num_free_chunks(zhdr);
    > > 648 spin_lock(&pool->lock);
    > > 649 list_add(&zhdr->buddy, &unbuddied[freechunks]);
    > > 650 spin_unlock(&pool->lock);
    > > 651 zhdr->cpu = smp_processor_id();
    > > 652 put_cpu_ptr(pool->unbuddied);
    > > 653 }
    > > 654 }
    > > ```
    > >
    > > Shouldn't the list manipulation be protected with
    > > local_lock+this_cpu_ptr instead of get_cpu_ptr+spin_lock?

    Totally untested:

    ```
    diff --git a/mm/z3fold.c b/mm/z3fold.c
    index 18feaa0bc537..53fcb80c6167 100644
    --- a/mm/z3fold.c
    +++ b/mm/z3fold.c
    @@ -41,6 +41,7 @@
    #include <linux/workqueue.h>
    #include <linux/slab.h>
    #include <linux/spinlock.h>
    +#include <linux/local_lock.h>
    #include <linux/zpool.h>
    #include <linux/magic.h>
    #include <linux/kmemleak.h>
    @@ -156,6 +157,7 @@ struct z3fold_pool {
    const char *name;
    spinlock_t lock;
    spinlock_t stale_lock;
    + local_lock_t llock;
    struct list_head *unbuddied;
    struct list_head lru;
    struct list_head stale;
    @@ -642,14 +644,17 @@ static inline void add_to_unbuddied(struct z3fold_pool *pool,
    {
    if (zhdr->first_chunks == 0 || zhdr->last_chunks == 0 ||
    zhdr->middle_chunks == 0) {
    - struct list_head *unbuddied = get_cpu_ptr(pool->unbuddied);
    + struct list_head *unbuddied;
    + int freechunks;
    + local_lock(&pool->llock);
    + unbuddied = *this_cpu_ptr(&pool->unbuddied);

    - int freechunks = num_free_chunks(zhdr);
    + freechunks = num_free_chunks(zhdr);
    spin_lock(&pool->lock);
    list_add(&zhdr->buddy, &unbuddied[freechunks]);
    spin_unlock(&pool->lock);
    zhdr->cpu = smp_processor_id();
    - put_cpu_ptr(pool->unbuddied);
    + local_unlock(&pool->llock);
    }
    }

    @@ -887,7 +892,8 @@ static inline struct z3fold_header *__z3fold_alloc(struct z3fold_pool *pool,

    lookup:
    /* First, try to find an unbuddied z3fold page. */
    - unbuddied = get_cpu_ptr(pool->unbuddied);
    + local_lock(&pool->llock);
    + unbuddied = *this_cpu_ptr(&pool->unbuddied);
    for_each_unbuddied_list(i, chunks) {
    struct list_head *l = &unbuddied[i];

    @@ -905,7 +911,7 @@ static inline struct z3fold_header *__z3fold_alloc(struct z3fold_pool *pool,
    !z3fold_page_trylock(zhdr)) {
    spin_unlock(&pool->lock);
    zhdr = NULL;
    - put_cpu_ptr(pool->unbuddied);
    + local_unlock(&pool->llock);
    if (can_sleep)
    cond_resched();
    goto lookup;
    @@ -919,7 +925,7 @@ static inline struct z3fold_header *__z3fold_alloc(struct z3fold_pool *pool,
    test_bit(PAGE_CLAIMED, &page->private)) {
    z3fold_page_unlock(zhdr);
    zhdr = NULL;
    - put_cpu_ptr(pool->unbuddied);
    + local_unlock(&pool->llock);
    if (can_sleep)
    cond_resched();
    goto lookup;
    @@ -934,7 +940,7 @@ static inline struct z3fold_header *__z3fold_alloc(struct z3fold_pool *pool,
    kref_get(&zhdr->refcount);
    break;
    }
    - put_cpu_ptr(pool->unbuddied);
    + local_unlock(&pool->llock);

    if (!zhdr) {
    int cpu;
    @@ -1005,6 +1011,7 @@ static struct z3fold_pool *z3fold_create_pool(const char *name, gfp_t gfp,
    goto out_c;
    spin_lock_init(&pool->lock);
    spin_lock_init(&pool->stale_lock);
    + local_lock_init(&pool->llock);
    pool->unbuddied = __alloc_percpu(sizeof(struct list_head)*NCHUNKS, 2);
    if (!pool->unbuddied)
    goto out_pool;
    ```

    --
    Oleksandr Natalenko (post-factum)

    \
     
     \ /
      Last update: 2020-11-28 23:39    [W:4.367 / U:0.356 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site