[bug report] mm,thp: add read-only THP support for (non-shmem) FS

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Bugtraq]     [Linux OMAP]     [Linux MIPS]     [eCos]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux