* [bug report] mm,thp: add read-only THP support for (non-shmem) FS
@ 2019-08-09 12:34 Dan Carpenter
2019-08-09 14:24 ` Song Liu
0 siblings, 1 reply; 2+ messages in thread
From: Dan Carpenter @ 2019-08-09 12:34 UTC (permalink / raw)
To: songliubraving; +Cc: linux-mm
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
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [bug report] mm,thp: add read-only THP support for (non-shmem) FS
2019-08-09 12:34 [bug report] mm,thp: add read-only THP support for (non-shmem) FS Dan Carpenter
@ 2019-08-09 14:24 ` Song Liu
0 siblings, 0 replies; 2+ messages in thread
From: Song Liu @ 2019-08-09 14:24 UTC (permalink / raw)
To: Dan Carpenter; +Cc: Linux MM, Andrew Morton
+ Andrew
> On Aug 9, 2019, at 5:34 AM, Dan Carpenter <dan.carpenter@oracle.com> wrote:
>
> 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:'
>
[...]
Thanks Dan! The following patch fixes this issue.
Hi Andrew, could you please add this fix to the mm tree?
Thanks,
Song
================= 8< ======================
From 30fd3683bb62cb81bb144e945edb768aff7ca445 Mon Sep 17 00:00:00 2001
From: Song Liu <songliubraving@fb.com>
Date: Fri, 9 Aug 2019 07:15:17 -0700
Subject: [PATCH] khugepaged: fix double unlock in collapse_file()
In collapse_file, when try_to_release_page() fails, we need to goto
out_unlock, because we already called xas_unlock_irq().
Fixes: 89e1c65c0db7 ("mm,thp: add read-only THP support for (non-shmem) FS")
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Song Liu <songliubraving@fb.com>
---
mm/khugepaged.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/mm/khugepaged.c b/mm/khugepaged.c
index 40c25ddf29e4..f3b94a5a9c43 100644
--- a/mm/khugepaged.c
+++ b/mm/khugepaged.c
@@ -1472,7 +1472,7 @@ static void collapse_file(struct mm_struct *mm,
if (page_has_private(page) &&
!try_to_release_page(page, GFP_KERNEL)) {
result = SCAN_PAGE_HAS_PRIVATE;
- break;
+ goto out_unlock;
}
if (page_mapped(page))
--
2.17.1
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2019-08-09 14:24 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-09 12:34 [bug report] mm,thp: add read-only THP support for (non-shmem) FS Dan Carpenter
2019-08-09 14:24 ` Song Liu
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox