In z3fold_destroy_pool() we call destroy_workqueue(&pool->compact_wq). However, we have no guarantee that migration isn't happening in the background at that time. Migration directly calls queue_work_on(pool->compact_wq), if destruction wins that race we are using a destroyed workqueue.
Unified · Split
| @@ -41,6 +41,7 @@ · 40 unmodified lines | |||
| 41 | #include <linux/workqueue.h> | 41 | #include <linux/workqueue.h> |
| 42 | #include <linux/slab.h> | 42 | #include <linux/slab.h> |
| 43 | #include <linux/spinlock.h> | 43 | #include <linux/spinlock.h> |
| 44 | #include <linux/wait.h> | ||
| 44 | #include <linux/zpool.h> | 45 | #include <linux/zpool.h> |
| 45 | #include <linux/magic.h> | 46 | #include <linux/magic.h> |
| 46 | 47 | ||
| @@ -145,6 +146,8 @@ struct z3fold_header { · 98 unmodified lines | |||
| 145 | * @release_wq: workqueue for safe page release | 146 | * @release_wq: workqueue for safe page release |
| 146 | * @work: work_struct for safe page release | 147 | * @work: work_struct for safe page release |
| 147 | * @inode: inode for z3fold pseudo filesystem | 148 | * @inode: inode for z3fold pseudo filesystem |
| 149 | * @destroying: bool to stop migration once we start destruction | ||
| 150 | * @isolated: int to count the number of pages currently in isolation | ||
| 148 | * | 151 | * |
| 149 | * This structure is allocated at pool creation time and maintains metadata | 152 | * This structure is allocated at pool creation time and maintains metadata |
| 150 | * pertaining to a particular z3fold pool. | 153 | * pertaining to a particular z3fold pool. |
| @@ -163,8 +166,11 @@ struct z3fold_pool { · 12 unmodified lines | |||
| 163 | const struct zpool_ops *zpool_ops; | 166 | const struct zpool_ops *zpool_ops; |
| 164 | struct workqueue_struct *compact_wq; | 167 | struct workqueue_struct *compact_wq; |
| 165 | struct workqueue_struct *release_wq; | 168 | struct workqueue_struct *release_wq; |
| 169 | struct wait_queue_head isolate_wait; | ||
| 166 | struct work_struct work; | 170 | struct work_struct work; |
| 167 | struct inode *inode; | 171 | struct inode *inode; |
| 172 | bool destroying; | ||
| 173 | int isolated; | ||
| 168 | }; | 174 | }; |
| 169 | 175 | ||
| 170 | /* | 176 | /* |
| @@ -769,6 +775,7 @@ static struct z3fold_pool *z3fold_create_pool(const char *name, gfp_t gfp, · 598 unmodified lines | |||
| 769 | goto out_c; | 775 | goto out_c; |
| 770 | spin_lock_init(&pool->lock); | 776 | spin_lock_init(&pool->lock); |
| 771 | spin_lock_init(&pool->stale_lock); | 777 | spin_lock_init(&pool->stale_lock); |
| 778 | init_waitqueue_head(&pool->isolate_wait); | ||
| 772 | pool->unbuddied = __alloc_percpu(sizeof(struct list_head)*NCHUNKS, 2); | 779 | pool->unbuddied = __alloc_percpu(sizeof(struct list_head)*NCHUNKS, 2); |
| 773 | if (!pool->unbuddied) | 780 | if (!pool->unbuddied) |
| 774 | goto out_pool; | 781 | goto out_pool; |
| @@ -808,6 +815,15 @@ out: · 33 unmodified lines | |||
| 808 | return NULL; | 815 | return NULL; |
| 809 | } | 816 | } |
| 810 | 817 | ||
| 818 | static bool pool_isolated_are_drained(struct z3fold_pool *pool) | ||
| 819 | { | ||
| 820 | bool ret; | ||
| 821 | |||
| 822 | spin_lock(&pool->lock); | ||
| 823 | ret = pool->isolated == 0; | ||
| 824 | spin_unlock(&pool->lock); | ||
| 825 | return ret; | ||
| 826 | } | ||
| 811 | /** | 827 | /** |
| 812 | * z3fold_destroy_pool() - destroys an existing z3fold pool | 828 | * z3fold_destroy_pool() - destroys an existing z3fold pool |
| 813 | * @pool: the z3fold pool to be destroyed | 829 | * @pool: the z3fold pool to be destroyed |
| @@ -817,6 +833,22 @@ out: · 3 unmodified lines | |||
| 817 | static void z3fold_destroy_pool(struct z3fold_pool *pool) | 833 | static void z3fold_destroy_pool(struct z3fold_pool *pool) |
| 818 | { | 834 | { |
| 819 | kmem_cache_destroy(pool->c_handle); | 835 | kmem_cache_destroy(pool->c_handle); |
| 836 | /* | ||
| 837 | * We set pool-> destroying under lock to ensure that | ||
| 838 | * z3fold_page_isolate() sees any changes to destroying. This way we | ||
| 839 | * avoid the need for any memory barriers. | ||
| 840 | */ | ||
| 841 | |||
| 842 | spin_lock(&pool->lock); | ||
| 843 | pool->destroying = true; | ||
| 844 | spin_unlock(&pool->lock); | ||
| 845 | |||
| 846 | /* | ||
| 847 | * We need to ensure that no pages are being migrated while we destroy | ||
| 848 | * these workqueues, as migration can queue work on either of the | ||
| 849 | * workqueues. | ||
| 850 | */ | ||
| 851 | wait_event(pool->isolate_wait, !pool_isolated_are_drained(pool)); | ||
| 820 | 852 | ||
| 821 | /* | 853 | /* |
| 822 | * We need to destroy pool->compact_wq before pool->release_wq, | 854 | * We need to destroy pool->compact_wq before pool->release_wq, |
| @@ -1307,6 +1339,28 @@ static u64 z3fold_get_pool_size(struct z3fold_pool *pool) · 484 unmodified lines | |||
| 1307 | return atomic64_read(&pool->pages_nr); | 1339 | return atomic64_read(&pool->pages_nr); |
| 1308 | } | 1340 | } |
| 1309 | 1341 | ||
| 1342 | /* | ||
| 1343 | * z3fold_dec_isolated() expects to be called while pool->lock is held. | ||
| 1344 | */ | ||
| 1345 | static void z3fold_dec_isolated(struct z3fold_pool *pool) | ||
| 1346 | { | ||
| 1347 | assert_spin_locked(&pool->lock); | ||
| 1348 | VM_BUG_ON(pool->isolated <= 0); | ||
| 1349 | pool->isolated--; | ||
| 1350 | |||
| 1351 | /* | ||
| 1352 | * If we have no more isolated pages, we have to see if | ||
| 1353 | * z3fold_destroy_pool() is waiting for a signal. | ||
| 1354 | */ | ||
| 1355 | if (pool->isolated == 0 && waitqueue_active(&pool->isolate_wait)) | ||
| 1356 | wake_up_all(&pool->isolate_wait); | ||
| 1357 | } | ||
| 1358 | |||
| 1359 | static void z3fold_inc_isolated(struct z3fold_pool *pool) | ||
| 1360 | { | ||
| 1361 | pool->isolated++; | ||
| 1362 | } | ||
| 1363 | |||
| 1310 | static bool z3fold_page_isolate(struct page *page, isolate_mode_t mode) | 1364 | static bool z3fold_page_isolate(struct page *page, isolate_mode_t mode) |
| 1311 | { | 1365 | { |
| 1312 | struct z3fold_header *zhdr; | 1366 | struct z3fold_header *zhdr; |
| @@ -1333,6 +1387,33 @@ static bool z3fold_page_isolate(struct page *page, isolate_mode_t mode) · 20 unmodified lines | |||
| 1333 | spin_lock(&pool->lock); | 1387 | spin_lock(&pool->lock); |
| 1334 | if (!list_empty(&page->lru)) | 1388 | if (!list_empty(&page->lru)) |
| 1335 | list_del(&page->lru); | 1389 | list_del(&page->lru); |
| 1390 | /* | ||
| 1391 | * We need to check for destruction while holding pool->lock, as | ||
| 1392 | * otherwise destruction could see 0 isolated pages, and | ||
| 1393 | * proceed. | ||
| 1394 | */ | ||
| 1395 | if (unlikely(pool->destroying)) { | ||
| 1396 | spin_unlock(&pool->lock); | ||
| 1397 | /* | ||
| 1398 | * If this page isn't stale, somebody else holds a | ||
| 1399 | * reference to it. Let't drop our refcount so that they | ||
| 1400 | * can call the release logic. | ||
| 1401 | */ | ||
| 1402 | if (unlikely(kref_put(&zhdr->refcount, | ||
| 1403 | release_z3fold_page_locked))) { | ||
| 1404 | /* | ||
| 1405 | * If we get here we have kref problems, so we | ||
| 1406 | * should freak out. | ||
| 1407 | */ | ||
| 1408 | WARN(1, "Z3fold is experiencing kref problems\n"); | ||
| 1409 | return false; | ||
| 1410 | } | ||
| 1411 | z3fold_page_unlock(zhdr); | ||
| 1412 | return false; | ||
| 1413 | } | ||
| 1414 | |||
| 1415 | |||
| 1416 | z3fold_inc_isolated(pool); | ||
| 1336 | spin_unlock(&pool->lock); | 1417 | spin_unlock(&pool->lock); |
| 1337 | z3fold_page_unlock(zhdr); | 1418 | z3fold_page_unlock(zhdr); |
| 1338 | return true; | 1419 | return true; |
| @@ -1401,6 +1482,10 @@ static int z3fold_page_migrate(struct address_space *mapping, struct page *newpa · 62 unmodified lines | |||
| 1401 | 1482 | ||
| 1402 | queue_work_on(new_zhdr->cpu, pool->compact_wq, &new_zhdr->work); | 1483 | queue_work_on(new_zhdr->cpu, pool->compact_wq, &new_zhdr->work); |
| 1403 | 1484 | ||
| 1485 | spin_lock(&pool->lock); | ||
| 1486 | z3fold_dec_isolated(pool); | ||
| 1487 | spin_unlock(&pool->lock); | ||
| 1488 | |||
| 1404 | page_mapcount_reset(page); | 1489 | page_mapcount_reset(page); |
| 1405 | put_page(page); | 1490 | put_page(page); |
| 1406 | return 0; | 1491 | return 0; |
| @@ -1420,10 +1505,14 @@ static void z3fold_page_putback(struct page *page) · 13 unmodified lines | |||
| 1420 | INIT_LIST_HEAD(&page->lru); | 1505 | INIT_LIST_HEAD(&page->lru); |
| 1421 | if (kref_put(&zhdr->refcount, release_z3fold_page_locked)) { | 1506 | if (kref_put(&zhdr->refcount, release_z3fold_page_locked)) { |
| 1422 | atomic64_dec(&pool->pages_nr); | 1507 | atomic64_dec(&pool->pages_nr); |
| 1508 | spin_lock(&pool->lock); | ||
| 1509 | z3fold_dec_isolated(pool); | ||
| 1510 | spin_unlock(&pool->lock); | ||
| 1423 | return; | 1511 | return; |
| 1424 | } | 1512 | } |
| 1425 | spin_lock(&pool->lock); | 1513 | spin_lock(&pool->lock); |
| 1426 | list_add(&page->lru, &pool->lru); | 1514 | list_add(&page->lru, &pool->lru); |
| 1515 | z3fold_dec_isolated(pool); | ||
| 1427 | spin_unlock(&pool->lock); | 1516 | spin_unlock(&pool->lock); |
| 1428 | z3fold_page_unlock(zhdr); | 1517 | z3fold_page_unlock(zhdr); |
| 1429 | } | 1518 | } |