From: Dan Carpenter <dan.carpenter@oracle.com>
To: songliubraving@fb.com
Cc: linux-mm@kvack.org
Subject: [bug report] mm,thp: add read-only THP support for (non-shmem) FS
Date: Fri, 9 Aug 2019 15:34:41 +0300 [thread overview]
Message-ID: <20190809123441.GA9573@mwanda> (raw)
Hello Song Liu,
The patch 89e1c65c0db7: "mm,thp: add read-only THP support for
(non-shmem) FS" from Aug 7, 2019, leads to the following static
checker warning:
mm/khugepaged.c:1532 collapse_file()
error: double unlock 'irq:'
mm/khugepaged.c
1398 if (xa_is_value(page) || !PageUptodate(page)) {
1399 xas_unlock_irq(&xas);
^^^^^^^^^^^^^^^^^^^^
We enable IRQs.
1400 /* swap in or instantiate fallocated page */
1401 if (shmem_getpage(mapping->host, index, &page,
1402 SGP_NOHUGE)) {
1403 result = SCAN_FAIL;
1404 goto xa_unlocked;
1405 }
1406 } else if (trylock_page(page)) {
1407 get_page(page);
1408 xas_unlock_irq(&xas);
1409 } else {
1410 result = SCAN_PAGE_LOCK;
1411 goto xa_locked;
1412 }
1413 } else { /* !is_shmem */
1414 if (!page || xa_is_value(page)) {
1415 xas_unlock_irq(&xas);
1416 page_cache_sync_readahead(mapping, &file->f_ra,
1417 file, index,
1418 PAGE_SIZE);
1419 /* drain pagevecs to help isolate_lru_page() */
1420 lru_add_drain();
1421 page = find_lock_page(mapping, index);
1422 if (unlikely(page == NULL)) {
1423 result = SCAN_FAIL;
1424 goto xa_unlocked;
1425 }
1426 } else if (!PageUptodate(page)) {
1427 xas_unlock_irq(&xas);
1428 wait_on_page_locked(page);
1429 if (!trylock_page(page)) {
1430 result = SCAN_PAGE_LOCK;
1431 goto xa_unlocked;
1432 }
1433 get_page(page);
1434 } else if (PageDirty(page)) {
1435 result = SCAN_FAIL;
1436 goto xa_locked;
1437 } else if (trylock_page(page)) {
1438 get_page(page);
1439 xas_unlock_irq(&xas);
1440 } else {
1441 result = SCAN_PAGE_LOCK;
1442 goto xa_locked;
1443 }
1444 }
1445
1446 /*
1447 * The page must be locked, so we can drop the i_pages lock
1448 * without racing with truncate.
1449 */
1450 VM_BUG_ON_PAGE(!PageLocked(page), page);
1451 VM_BUG_ON_PAGE(!PageUptodate(page), page);
1452
1453 /*
1454 * If file was truncated then extended, or hole-punched, before
1455 * we locked the first page, then a THP might be there already.
1456 */
1457 if (PageTransCompound(page)) {
1458 result = SCAN_PAGE_COMPOUND;
1459 goto out_unlock;
1460 }
1461
1462 if (page_mapping(page) != mapping) {
1463 result = SCAN_TRUNCATED;
1464 goto out_unlock;
1465 }
1466
1467 if (isolate_lru_page(page)) {
1468 result = SCAN_DEL_PAGE_LRU;
1469 goto out_unlock;
1470 }
1471
1472 if (page_has_private(page) &&
1473 !try_to_release_page(page, GFP_KERNEL)) {
1474 result = SCAN_PAGE_HAS_PRIVATE;
1475 break;
The patch adds this break statement but IRQs are enabled at this point.
1476 }
1477
1478 if (page_mapped(page))
1479 unmap_mapping_pages(mapping, index, 1, false);
1480
1481 xas_lock_irq(&xas);
1482 xas_set(&xas, index);
1483
1484 VM_BUG_ON_PAGE(page != xas_load(&xas), page);
1485 VM_BUG_ON_PAGE(page_mapped(page), page);
1486
1487 /*
1488 * The page is expected to have page_count() == 3:
1489 * - we hold a pin on it;
1490 * - one reference from page cache;
1491 * - one from isolate_lru_page;
1492 */
1493 if (!page_ref_freeze(page, 3)) {
1494 result = SCAN_PAGE_COUNT;
1495 xas_unlock_irq(&xas);
1496 putback_lru_page(page);
1497 goto out_unlock;
1498 }
1499
1500 /*
1501 * Add the page to the list to be able to undo the collapse if
1502 * something go wrong.
1503 */
1504 list_add_tail(&page->lru, &pagelist);
1505
1506 /* Finally, replace with the new page. */
1507 xas_store(&xas, new_page);
1508 continue;
1509 out_unlock:
1510 unlock_page(page);
1511 put_page(page);
1512 goto xa_unlocked;
1513 }
1514
1515 if (is_shmem)
1516 __inc_node_page_state(new_page, NR_SHMEM_THPS);
1517 else {
1518 __inc_node_page_state(new_page, NR_FILE_THPS);
1519 filemap_nr_thps_inc(mapping);
1520 }
1521
1522 if (nr_none) {
1523 struct zone *zone = page_zone(new_page);
1524
1525 __mod_node_page_state(zone->zone_pgdat, NR_FILE_PAGES, nr_none);
1526 if (is_shmem)
1527 __mod_node_page_state(zone->zone_pgdat,
1528 NR_SHMEM, nr_none);
1529 }
1530
1531 xa_locked:
1532 xas_unlock_irq(&xas);
^^^^^^^^^^^^^^^^^^^^
Double unlock.
1533 xa_unlocked:
1534
1535 if (result == SCAN_SUCCEED) {
regards,
dan carpenter
next reply other threads:[~2019-08-09 12:34 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-08-09 12:34 Dan Carpenter [this message]
2019-08-09 14:24 ` [bug report] mm,thp: add read-only THP support for (non-shmem) FS Song Liu
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20190809123441.GA9573@mwanda \
--to=dan.carpenter@oracle.com \
--cc=linux-mm@kvack.org \
--cc=songliubraving@fb.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.