Pull AIO leak fixes from Ben LaHaise: "I've put these two patches plus Linus's change through a round of tests, and it passes millions of iterations of the aio numa migratepage test, as well as a number of repetitions of a few simple read and write tests. The first patch fixes the memory leak Kent introduced, while the second patch makes aio_migratepage() much more paranoid and robust" * git://git.kvack.org/~bcrl/aio-next: aio/migratepages: make aio migrate pages sane aio: fix kioctx leak introduced by "aio: Fix a trinity splat"
Unified · Split
| @@ -244,9 +244,14 @@ static void aio_free_ring(struct kioctx *ctx) · 243 unmodified lines | ||
| 244 | 244 | int i; |
| 245 | 245 | |
| 246 | 246 | for (i = 0; i < ctx->nr_pages; i++) { |
| 247 | + struct page *page; | |
| 247 | 248 | pr_debug("pid(%d) [%d] page->count=%d\n", current->pid, i, |
| 248 | 249 | page_count(ctx->ring_pages[i])); |
| 249 | − put_page(ctx->ring_pages[i]); | |
| 250 | + page = ctx->ring_pages[i]; | |
| 251 | + if (!page) | |
| 252 | + continue; | |
| 253 | + ctx->ring_pages[i] = NULL; | |
| 254 | + put_page(page); | |
| 250 | 255 | } |
| 251 | 256 | |
| 252 | 257 | put_aio_ring_file(ctx); |
| @@ -280,18 +285,38 @@ static int aio_migratepage(struct address_space *mapping, struct page *new, · 27 unmodified lines | ||
| 280 | 285 | unsigned long flags; |
| 281 | 286 | int rc; |
| 282 | 287 | |
| 288 | + rc = 0; | |
| 289 | + | |
| 290 | + /* Make sure the old page hasn't already been changed */ | |
| 291 | + spin_lock(&mapping->private_lock); | |
| 292 | + ctx = mapping->private_data; | |
| 293 | + if (ctx) { | |
| 294 | + pgoff_t idx; | |
| 295 | + spin_lock_irqsave(&ctx->completion_lock, flags); | |
| 296 | + idx = old->index; | |
| 297 | + if (idx < (pgoff_t)ctx->nr_pages) { | |
| 298 | + if (ctx->ring_pages[idx] != old) | |
| 299 | + rc = -EAGAIN; | |
| 300 | + } else | |
| 301 | + rc = -EINVAL; | |
| 302 | + spin_unlock_irqrestore(&ctx->completion_lock, flags); | |
| 303 | + } else | |
| 304 | + rc = -EINVAL; | |
| 305 | + spin_unlock(&mapping->private_lock); | |
| 306 | + | |
| 307 | + if (rc != 0) | |
| 308 | + return rc; | |
| 309 | + | |
| 283 | 310 | /* Writeback must be complete */ |
| 284 | 311 | BUG_ON(PageWriteback(old)); |
| 285 | − put_page(old); | |
| 312 | + get_page(new); | |
| 286 | 313 | |
| 287 | − rc = migrate_page_move_mapping(mapping, new, old, NULL, mode); | |
| 314 | + rc = migrate_page_move_mapping(mapping, new, old, NULL, mode, 1); | |
| 288 | 315 | if (rc != MIGRATEPAGE_SUCCESS) { |
| 289 | − get_page(old); | |
| 316 | + put_page(new); | |
| 290 | 317 | return rc; |
| 291 | 318 | } |
| 292 | 319 | |
| 293 | − get_page(new); | |
| 294 | − | |
| 295 | 320 | /* We can potentially race against kioctx teardown here. Use the |
| 296 | 321 | * address_space's private data lock to protect the mapping's |
| 297 | 322 | * private_data. |
| @@ -303,13 +328,24 @@ static int aio_migratepage(struct address_space *mapping, struct page *new, · 5 unmodified lines | ||
| 303 | 328 | spin_lock_irqsave(&ctx->completion_lock, flags); |
| 304 | 329 | migrate_page_copy(new, old); |
| 305 | 330 | idx = old->index; |
| 306 | − if (idx < (pgoff_t)ctx->nr_pages) | |
| 307 | − ctx->ring_pages[idx] = new; | |
| 331 | + if (idx < (pgoff_t)ctx->nr_pages) { | |
| 332 | + /* And only do the move if things haven't changed */ | |
| 333 | + if (ctx->ring_pages[idx] == old) | |
| 334 | + ctx->ring_pages[idx] = new; | |
| 335 | + else | |
| 336 | + rc = -EAGAIN; | |
| 337 | + } else | |
| 338 | + rc = -EINVAL; | |
| 308 | 339 | spin_unlock_irqrestore(&ctx->completion_lock, flags); |
| 309 | 340 | } else |
| 310 | 341 | rc = -EBUSY; |
| 311 | 342 | spin_unlock(&mapping->private_lock); |
| 312 | 343 | |
| 344 | + if (rc == MIGRATEPAGE_SUCCESS) | |
| 345 | + put_page(old); | |
| 346 | + else | |
| 347 | + put_page(new); | |
| 348 | + | |
| 313 | 349 | return rc; |
| 314 | 350 | } |
| 315 | 351 | #endif |
| @@ -640,7 +676,8 @@ static struct kioctx *ioctx_alloc(unsigned nr_events) · 324 unmodified lines | ||
| 640 | 676 | aio_nr += ctx->max_reqs; |
| 641 | 677 | spin_unlock(&aio_nr_lock); |
| 642 | 678 | |
| 643 | − percpu_ref_get(&ctx->users); /* io_setup() will drop this ref */ | |
| 679 | + percpu_ref_get(&ctx->users); /* io_setup() will drop this ref */ | |
| 680 | + percpu_ref_get(&ctx->reqs); /* free_ioctx_users() will drop this */ | |
| 644 | 681 | |
| 645 | 682 | err = ioctx_add_table(ctx, mm); |
| 646 | 683 | if (err) |
| @@ -55,7 +55,8 @@ extern int migrate_huge_page_move_mapping(struct address_space *mapping, · 54 unmodified lines | ||
| 55 | 55 | struct page *newpage, struct page *page); |
| 56 | 56 | extern int migrate_page_move_mapping(struct address_space *mapping, |
| 57 | 57 | struct page *newpage, struct page *page, |
| 58 | − struct buffer_head *head, enum migrate_mode mode); | |
| 58 | + struct buffer_head *head, enum migrate_mode mode, | |
| 59 | + int extra_count); | |
| 59 | 60 | #else |
| 60 | 61 | |
| 61 | 62 | static inline void putback_lru_pages(struct list_head *l) {} |
| @@ -317,14 +317,15 @@ static inline bool buffer_migrate_lock_buffers(struct buffer_head *head, · 316 unmodified lines | ||
| 317 | 317 | */ |
| 318 | 318 | int migrate_page_move_mapping(struct address_space *mapping, |
| 319 | 319 | struct page *newpage, struct page *page, |
| 320 | − struct buffer_head *head, enum migrate_mode mode) | |
| 320 | + struct buffer_head *head, enum migrate_mode mode, | |
| 321 | + int extra_count) | |
| 321 | 322 | { |
| 322 | − int expected_count = 0; | |
| 323 | + int expected_count = 1 + extra_count; | |
| 323 | 324 | void **pslot; |
| 324 | 325 | |
| 325 | 326 | if (!mapping) { |
| 326 | 327 | /* Anonymous page without mapping */ |
| 327 | − if (page_count(page) != 1) | |
| 328 | + if (page_count(page) != expected_count) | |
| 328 | 329 | return -EAGAIN; |
| 329 | 330 | return MIGRATEPAGE_SUCCESS; |
| 330 | 331 | } |
| @@ -334,7 +335,7 @@ int migrate_page_move_mapping(struct address_space *mapping, · 3 unmodified lines | ||
| 334 | 335 | pslot = radix_tree_lookup_slot(&mapping->page_tree, |
| 335 | 336 | page_index(page)); |
| 336 | 337 | |
| 337 | − expected_count = 2 + page_has_private(page); | |
| 338 | + expected_count += 1 + page_has_private(page); | |
| 338 | 339 | if (page_count(page) != expected_count || |
| 339 | 340 | radix_tree_deref_slot_protected(pslot, &mapping->tree_lock) != page) { |
| 340 | 341 | spin_unlock_irq(&mapping->tree_lock); |
| @@ -584,7 +585,7 @@ int migrate_page(struct address_space *mapping, · 243 unmodified lines | ||
| 584 | 585 | |
| 585 | 586 | BUG_ON(PageWriteback(page)); /* Writeback must be complete */ |
| 586 | 587 | |
| 587 | − rc = migrate_page_move_mapping(mapping, newpage, page, NULL, mode); | |
| 588 | + rc = migrate_page_move_mapping(mapping, newpage, page, NULL, mode, 0); | |
| 588 | 589 | |
| 589 | 590 | if (rc != MIGRATEPAGE_SUCCESS) |
| 590 | 591 | return rc; |
| @@ -611,7 +612,7 @@ int buffer_migrate_page(struct address_space *mapping, · 20 unmodified lines | ||
| 611 | 612 | |
| 612 | 613 | head = page_buffers(page); |
| 613 | 614 | |
| 614 | − rc = migrate_page_move_mapping(mapping, newpage, page, head, mode); | |
| 615 | + rc = migrate_page_move_mapping(mapping, newpage, page, head, mode, 0); | |
| 615 | 616 | |
| 616 | 617 | if (rc != MIGRATEPAGE_SUCCESS) |
| 617 | 618 | return rc; |