[chao-linux:erofs-dev 24/33] fs/erofs/unzip_vle.c:651:1-7: preceding lock on line 509
Gao Xiang
gaoxiang25 at huawei.com
Tue Jul 3 15:43:25 AEST 2018
Hi Julia,
On 2018/7/3 13:27, Julia Lawall wrote:
> Hello,
>
> There is not actually a bug here, but I wonder if the backwards goto int
> another if branch (use_global_pagemap) is really the best way to do this?
> I found it rather confusing to think about. It could be better to just
> duplicate the assignment to pages, or make a flag that would be tested
> after the if-else.
>
I wrote this code just because of the designed pagemap allocation policy:
If the pagemap is too large enough to alloc onstack, then
I try to use the global pagemap with "trylock" if the global pagemap is not occupied.
or if the global pagemap is occupied,
alloc a pagemap if the corresponding memory allocation is ok,
or if the memory allocation is failed, fall back to use the global pagemap with sleeping "lock" again.
If the code style makes automatic testing confused, I could use alternative way to replace it in the next version.
Thanks for your caring and report. :)
Thanks,
Gao Xiang
> julia
>
> On Tue, 3 Jul 2018, kbuild test robot wrote:
>
>>
>> tree: https://git.kernel.org/pub/scm/linux/kernel/git/chao/linux.git erofs-dev
>> head: 17811ace07ef1cec14913201833ea69ac3243d3a
>> commit: 477609336cc282eac2aff56f5071be42be208867 [24/33] erofs: introduce VLE decompression subsystem
>> :::::: branch date: 13 hours ago
>> :::::: commit date: 13 hours ago
>>
>>>> fs/erofs/unzip_vle.c:651:1-7: preceding lock on line 509
>>
>> # https://git.kernel.org/pub/scm/linux/kernel/git/chao/linux.git/commit/?id=477609336cc282eac2aff56f5071be42be208867
>> git remote add chao-linux https://git.kernel.org/pub/scm/linux/kernel/git/chao/linux.git
>> git remote update chao-linux
>> git checkout 477609336cc282eac2aff56f5071be42be208867
>> vim +651 fs/erofs/unzip_vle.c
>>
>> 47760933 Gao Xiang 2018-07-02 474
>> 47760933 Gao Xiang 2018-07-02 475 static int z_erofs_vle_unzip(struct super_block *sb,
>> 47760933 Gao Xiang 2018-07-02 476 struct z_erofs_vle_work *work,
>> 47760933 Gao Xiang 2018-07-02 477 bool cached, struct list_head *page_pool)
>> 47760933 Gao Xiang 2018-07-02 478 {
>> 47760933 Gao Xiang 2018-07-02 479 unsigned clusterpages = erofs_clusterpages(EROFS_SB(sb));
>> 47760933 Gao Xiang 2018-07-02 480 struct z_erofs_pagevec_ctor ctor;
>> 47760933 Gao Xiang 2018-07-02 481 unsigned nr_pages;
>> 47760933 Gao Xiang 2018-07-02 482 struct page *pages_onstack[Z_EROFS_VLE_VMAP_ONSTACK_PAGES];
>> 47760933 Gao Xiang 2018-07-02 483 struct page **pages, **compressed_pages, *page;
>> 47760933 Gao Xiang 2018-07-02 484 unsigned i, llen;
>> 47760933 Gao Xiang 2018-07-02 485
>> 47760933 Gao Xiang 2018-07-02 486 enum z_erofs_page_type page_type;
>> 47760933 Gao Xiang 2018-07-02 487 bool overlapped;
>> 47760933 Gao Xiang 2018-07-02 488 struct z_erofs_vle_workgroup *grp;
>> 47760933 Gao Xiang 2018-07-02 489 void *vout;
>> 47760933 Gao Xiang 2018-07-02 490 int err;
>> 47760933 Gao Xiang 2018-07-02 491
>> 47760933 Gao Xiang 2018-07-02 492 BUG_ON(!READ_ONCE(work->nr_pages));
>> 47760933 Gao Xiang 2018-07-02 493 might_sleep();
>> 47760933 Gao Xiang 2018-07-02 494
>> 47760933 Gao Xiang 2018-07-02 495 mutex_lock(&work->lock);
>> 47760933 Gao Xiang 2018-07-02 496 nr_pages = work->nr_pages;
>> 47760933 Gao Xiang 2018-07-02 497
>> 47760933 Gao Xiang 2018-07-02 498 if (likely(nr_pages <= Z_EROFS_VLE_VMAP_ONSTACK_PAGES))
>> 47760933 Gao Xiang 2018-07-02 499 pages = pages_onstack;
>> 47760933 Gao Xiang 2018-07-02 500 else if (nr_pages <= Z_EROFS_VLE_VMAP_GLOBAL_PAGES &&
>> 47760933 Gao Xiang 2018-07-02 501 mutex_trylock(&z_pagemap_global_lock))
>> 47760933 Gao Xiang 2018-07-02 502 use_global_pagemap:
>> 47760933 Gao Xiang 2018-07-02 503 pages = z_pagemap_global;
>> 47760933 Gao Xiang 2018-07-02 504 else {
>> 47760933 Gao Xiang 2018-07-02 505 pages = kvmalloc(nr_pages, GFP_KERNEL | __GFP_NOFAIL);
>> 47760933 Gao Xiang 2018-07-02 506
>> 47760933 Gao Xiang 2018-07-02 507 /* fallback to global pagemap for the lowmem scenario */
>> 47760933 Gao Xiang 2018-07-02 508 if (unlikely(pages == NULL)) {
>> 47760933 Gao Xiang 2018-07-02 @509 mutex_lock(&z_pagemap_global_lock);
>> 47760933 Gao Xiang 2018-07-02 510 goto use_global_pagemap;
>> 47760933 Gao Xiang 2018-07-02 511 }
>> 47760933 Gao Xiang 2018-07-02 512 }
>> 47760933 Gao Xiang 2018-07-02 513
>> 47760933 Gao Xiang 2018-07-02 514 for (i = 0; i < nr_pages; ++i)
>> 47760933 Gao Xiang 2018-07-02 515 pages[i] = NULL;
>> 47760933 Gao Xiang 2018-07-02 516
>> 47760933 Gao Xiang 2018-07-02 517 z_erofs_pagevec_ctor_init(&ctor,
>> 47760933 Gao Xiang 2018-07-02 518 Z_EROFS_VLE_INLINE_PAGEVECS, work->pagevec, 0);
>> 47760933 Gao Xiang 2018-07-02 519
>> 47760933 Gao Xiang 2018-07-02 520 for (i = 0; i < work->vcnt; ++i) {
>> 47760933 Gao Xiang 2018-07-02 521 unsigned pagenr;
>> 47760933 Gao Xiang 2018-07-02 522
>> 47760933 Gao Xiang 2018-07-02 523 page = z_erofs_pagevec_ctor_dequeue(&ctor, &page_type);
>> 47760933 Gao Xiang 2018-07-02 524 BUG_ON(!page);
>> 47760933 Gao Xiang 2018-07-02 525
>> 47760933 Gao Xiang 2018-07-02 526 if (page->mapping == NULL) {
>> 47760933 Gao Xiang 2018-07-02 527 list_add(&page->lru, page_pool);
>> 47760933 Gao Xiang 2018-07-02 528 continue;
>> 47760933 Gao Xiang 2018-07-02 529 }
>> 47760933 Gao Xiang 2018-07-02 530
>> 47760933 Gao Xiang 2018-07-02 531 if (page_type == Z_EROFS_VLE_PAGE_TYPE_HEAD)
>> 47760933 Gao Xiang 2018-07-02 532 pagenr = 0;
>> 47760933 Gao Xiang 2018-07-02 533 else
>> 47760933 Gao Xiang 2018-07-02 534 pagenr = z_erofs_onlinepage_index(page);
>> 47760933 Gao Xiang 2018-07-02 535
>> 47760933 Gao Xiang 2018-07-02 536 BUG_ON(pagenr >= nr_pages);
>> 47760933 Gao Xiang 2018-07-02 537
>> 47760933 Gao Xiang 2018-07-02 538 #ifndef CONFIG_EROFS_FS_ZIP_MULTIREF
>> 47760933 Gao Xiang 2018-07-02 539 BUG_ON(pages[pagenr] != NULL);
>> 47760933 Gao Xiang 2018-07-02 540 #endif
>> 47760933 Gao Xiang 2018-07-02 541 pages[pagenr] = page;
>> 47760933 Gao Xiang 2018-07-02 542 }
>> 47760933 Gao Xiang 2018-07-02 543
>> 47760933 Gao Xiang 2018-07-02 544 z_erofs_pagevec_ctor_exit(&ctor, true);
>> 47760933 Gao Xiang 2018-07-02 545
>> 47760933 Gao Xiang 2018-07-02 546 overlapped = false;
>> 47760933 Gao Xiang 2018-07-02 547 if (cached) {
>> 47760933 Gao Xiang 2018-07-02 548 grp = z_erofs_vle_work_workgroup(work);
>> 47760933 Gao Xiang 2018-07-02 549 compressed_pages = z_erofs_vle_cached_managed(grp);
>> 47760933 Gao Xiang 2018-07-02 550 } else {
>> 47760933 Gao Xiang 2018-07-02 551 grp = z_erofs_vle_work_workgroup(work);
>> 47760933 Gao Xiang 2018-07-02 552 compressed_pages = z_erofs_vle_work_uncached_mux(work);
>> 47760933 Gao Xiang 2018-07-02 553
>> 47760933 Gao Xiang 2018-07-02 554 for(i = 0; i < clusterpages; ++i) {
>> 47760933 Gao Xiang 2018-07-02 555 unsigned pagenr;
>> 47760933 Gao Xiang 2018-07-02 556
>> 47760933 Gao Xiang 2018-07-02 557 BUG_ON(compressed_pages[i] == NULL);
>> 47760933 Gao Xiang 2018-07-02 558 page = compressed_pages[i];
>> 47760933 Gao Xiang 2018-07-02 559
>> 47760933 Gao Xiang 2018-07-02 560 if (page->mapping == NULL)
>> 47760933 Gao Xiang 2018-07-02 561 continue;
>> 47760933 Gao Xiang 2018-07-02 562
>> 47760933 Gao Xiang 2018-07-02 563 pagenr = z_erofs_onlinepage_index(page);
>> 47760933 Gao Xiang 2018-07-02 564
>> 47760933 Gao Xiang 2018-07-02 565 BUG_ON(pagenr >= nr_pages);
>> 47760933 Gao Xiang 2018-07-02 566 #ifndef CONFIG_EROFS_FS_ZIP_MULTIREF
>> 47760933 Gao Xiang 2018-07-02 567 BUG_ON(pages[pagenr] != NULL);
>> 47760933 Gao Xiang 2018-07-02 568 #endif
>> 47760933 Gao Xiang 2018-07-02 569 pages[pagenr] = page;
>> 47760933 Gao Xiang 2018-07-02 570
>> 47760933 Gao Xiang 2018-07-02 571 overlapped = true;
>> 47760933 Gao Xiang 2018-07-02 572 }
>> 47760933 Gao Xiang 2018-07-02 573 }
>> 47760933 Gao Xiang 2018-07-02 574
>> 47760933 Gao Xiang 2018-07-02 575 llen = (nr_pages << PAGE_SHIFT) - work->pageofs;
>> 47760933 Gao Xiang 2018-07-02 576
>> 47760933 Gao Xiang 2018-07-02 577 if (z_erofs_vle_workgroup_fmt(grp) == Z_EROFS_WORK_FORMAT_PLAIN) {
>> 47760933 Gao Xiang 2018-07-02 578 BUG_ON(grp->llen != llen);
>> 47760933 Gao Xiang 2018-07-02 579
>> 47760933 Gao Xiang 2018-07-02 580 err = z_erofs_vle_plain_copy(compressed_pages, clusterpages,
>> 47760933 Gao Xiang 2018-07-02 581 pages, nr_pages, work->pageofs);
>> 47760933 Gao Xiang 2018-07-02 582 goto out;
>> 47760933 Gao Xiang 2018-07-02 583 }
>> 47760933 Gao Xiang 2018-07-02 584
>> 47760933 Gao Xiang 2018-07-02 585 if (llen > grp->llen)
>> 47760933 Gao Xiang 2018-07-02 586 llen = grp->llen;
>> 47760933 Gao Xiang 2018-07-02 587
>> 47760933 Gao Xiang 2018-07-02 588 err = z_erofs_vle_unzip_fast_percpu(compressed_pages,
>> 47760933 Gao Xiang 2018-07-02 589 clusterpages, pages, llen, work->pageofs);
>> 47760933 Gao Xiang 2018-07-02 590 if (err != -ENOTSUPP)
>> 47760933 Gao Xiang 2018-07-02 591 goto out;
>> 47760933 Gao Xiang 2018-07-02 592
>> 47760933 Gao Xiang 2018-07-02 593 #ifndef CONFIG_EROFS_FS_ZIP_MULTIREF
>> 47760933 Gao Xiang 2018-07-02 594 if (work->vcnt == nr_pages)
>> 47760933 Gao Xiang 2018-07-02 595 goto skip_allocpage;
>> 47760933 Gao Xiang 2018-07-02 596 #endif
>> 47760933 Gao Xiang 2018-07-02 597
>> 47760933 Gao Xiang 2018-07-02 598 for (i = 0; i < nr_pages; ++i) {
>> 47760933 Gao Xiang 2018-07-02 599 if (pages[i] != NULL)
>> 47760933 Gao Xiang 2018-07-02 600 continue;
>> 47760933 Gao Xiang 2018-07-02 601 pages[i] = erofs_allocpage(page_pool, GFP_KERNEL);
>> 47760933 Gao Xiang 2018-07-02 602 }
>> 47760933 Gao Xiang 2018-07-02 603
>> 47760933 Gao Xiang 2018-07-02 604 #ifndef CONFIG_EROFS_FS_ZIP_MULTIREF
>> 47760933 Gao Xiang 2018-07-02 605 skip_allocpage:
>> 47760933 Gao Xiang 2018-07-02 606 #endif
>> 47760933 Gao Xiang 2018-07-02 607 vout = erofs_vmap(pages, nr_pages);
>> 47760933 Gao Xiang 2018-07-02 608
>> 47760933 Gao Xiang 2018-07-02 609 err = z_erofs_vle_unzip_vmap(compressed_pages,
>> 47760933 Gao Xiang 2018-07-02 610 clusterpages, vout, llen, work->pageofs, overlapped);
>> 47760933 Gao Xiang 2018-07-02 611
>> 47760933 Gao Xiang 2018-07-02 612 erofs_vunmap(vout, nr_pages);
>> 47760933 Gao Xiang 2018-07-02 613
>> 47760933 Gao Xiang 2018-07-02 614 out:
>> 47760933 Gao Xiang 2018-07-02 615 for (i = 0; i < nr_pages; ++i) {
>> 47760933 Gao Xiang 2018-07-02 616 page = pages[i];
>> 47760933 Gao Xiang 2018-07-02 617
>> 47760933 Gao Xiang 2018-07-02 618 /* recycle all individual pages */
>> 47760933 Gao Xiang 2018-07-02 619 if (page->mapping == NULL) {
>> 47760933 Gao Xiang 2018-07-02 620 list_add(&page->lru, page_pool);
>> 47760933 Gao Xiang 2018-07-02 621 continue;
>> 47760933 Gao Xiang 2018-07-02 622 }
>> 47760933 Gao Xiang 2018-07-02 623
>> 47760933 Gao Xiang 2018-07-02 624 if (unlikely(err < 0))
>> 47760933 Gao Xiang 2018-07-02 625 SetPageError(page);
>> 47760933 Gao Xiang 2018-07-02 626
>> 47760933 Gao Xiang 2018-07-02 627 z_erofs_onlinepage_endio(page);
>> 47760933 Gao Xiang 2018-07-02 628 }
>> 47760933 Gao Xiang 2018-07-02 629
>> 47760933 Gao Xiang 2018-07-02 630 for (i = 0; i < clusterpages; ++i) {
>> 47760933 Gao Xiang 2018-07-02 631 page = compressed_pages[i];
>> 47760933 Gao Xiang 2018-07-02 632
>> 47760933 Gao Xiang 2018-07-02 633 /* recycle all individual pages */
>> 47760933 Gao Xiang 2018-07-02 634 if (page->mapping == NULL)
>> 47760933 Gao Xiang 2018-07-02 635 list_add(&page->lru, page_pool);
>> 47760933 Gao Xiang 2018-07-02 636
>> 47760933 Gao Xiang 2018-07-02 637 if (!cached)
>> 47760933 Gao Xiang 2018-07-02 638 WRITE_ONCE(compressed_pages[i], NULL);
>> 47760933 Gao Xiang 2018-07-02 639 }
>> 47760933 Gao Xiang 2018-07-02 640
>> 47760933 Gao Xiang 2018-07-02 641 if (pages == z_pagemap_global)
>> 47760933 Gao Xiang 2018-07-02 642 mutex_unlock(&z_pagemap_global_lock);
>> 47760933 Gao Xiang 2018-07-02 643 else if (unlikely(pages != pages_onstack))
>> 47760933 Gao Xiang 2018-07-02 644 kvfree(pages);
>> 47760933 Gao Xiang 2018-07-02 645
>> 47760933 Gao Xiang 2018-07-02 646 work->nr_pages = 0;
>> 47760933 Gao Xiang 2018-07-02 647 work->vcnt = 0;
>> 47760933 Gao Xiang 2018-07-02 648
>> 47760933 Gao Xiang 2018-07-02 649 WRITE_ONCE(work->next, Z_EROFS_WORK_TPTR_NIL);
>> 47760933 Gao Xiang 2018-07-02 650 mutex_unlock(&work->lock);
>> 47760933 Gao Xiang 2018-07-02 @651 return err;
>> 47760933 Gao Xiang 2018-07-02 652 }
>> 47760933 Gao Xiang 2018-07-02 653
>>
>> ---
>> 0-DAY kernel test infrastructure Open Source Technology Center
>> https://lists.01.org/pipermail/kbuild-all Intel Corporation
>>
More information about the Linux-erofs
mailing list