[f2fs-dev] [bug report] f2fs: Use folios in do_garbage_collect()

[Dan Carpenter <dan.carpenter@linaro.org>]

Hello Matthew Wilcox (Oracle),

Commit 5d895f7beae9 ("f2fs: Use folios in do_garbage_collect()") from
Mar 31, 2025 (linux-next), leads to the following Smatch static
checker warning:

	fs/f2fs/gc.c:1799 do_garbage_collect()
	error: 'sum_folio' dereferencing possible ERR_PTR()

fs/f2fs/gc.c
    1716 static int do_garbage_collect(struct f2fs_sb_info *sbi,
    1717                                 unsigned int start_segno,
    1718                                 struct gc_inode_list *gc_list, int gc_type,
    1719                                 bool force_migrate, bool one_time)
    1720 {
    1721         struct blk_plug plug;
    1722         unsigned int segno = start_segno;
    1723         unsigned int end_segno = start_segno + SEGS_PER_SEC(sbi);
    1724         unsigned int sec_end_segno;
    1725         int seg_freed = 0, migrated = 0;
    1726         unsigned char type = IS_DATASEG(get_seg_entry(sbi, segno)->type) ?
    1727                                                 SUM_TYPE_DATA : SUM_TYPE_NODE;
    1728         unsigned char data_type = (type == SUM_TYPE_DATA) ? DATA : NODE;
    1729         int submitted = 0;
    1730 
    1731         if (__is_large_section(sbi)) {
    1732                 sec_end_segno = rounddown(end_segno, SEGS_PER_SEC(sbi));
    1733 
    1734                 /*
    1735                  * zone-capacity can be less than zone-size in zoned devices,
    1736                  * resulting in less than expected usable segments in the zone,
    1737                  * calculate the end segno in the zone which can be garbage
    1738                  * collected
    1739                  */
    1740                 if (f2fs_sb_has_blkzoned(sbi))
    1741                         sec_end_segno -= SEGS_PER_SEC(sbi) -
    1742                                         f2fs_usable_segs_in_sec(sbi);
    1743 
    1744                 if (gc_type == BG_GC || one_time) {
    1745                         unsigned int window_granularity =
    1746                                 sbi->migration_window_granularity;
    1747 
    1748                         if (f2fs_sb_has_blkzoned(sbi) &&
    1749                                         !has_enough_free_blocks(sbi,
    1750                                         sbi->gc_thread->boost_zoned_gc_percent))
    1751                                 window_granularity *=
    1752                                         BOOST_GC_MULTIPLE;
    1753 
    1754                         end_segno = start_segno + window_granularity;
    1755                 }
    1756 
    1757                 if (end_segno > sec_end_segno)
    1758                         end_segno = sec_end_segno;
    1759         }
    1760 
    1761         sanity_check_seg_type(sbi, get_seg_entry(sbi, segno)->type);
    1762 
    1763         /* readahead multi ssa blocks those have contiguous address */
    1764         if (__is_large_section(sbi))
    1765                 f2fs_ra_meta_pages(sbi, GET_SUM_BLOCK(sbi, segno),
    1766                                         end_segno - segno, META_SSA, true);
    1767 
    1768         /* reference all summary page */
    1769         while (segno < end_segno) {
    1770                 struct folio *sum_folio = f2fs_get_sum_folio(sbi, segno++);
    1771                 if (IS_ERR(sum_folio)) {
    1772                         int err = PTR_ERR(sum_folio);
    1773 
    1774                         end_segno = segno - 1;
    1775                         for (segno = start_segno; segno < end_segno; segno++) {
    1776                                 sum_folio = filemap_get_folio(META_MAPPING(sbi),
    1777                                                 GET_SUM_BLOCK(sbi, segno));
    1778                                 folio_put_refs(sum_folio, 2);
    1779                         }
    1780                         return err;
    1781                 }
    1782                 folio_unlock(sum_folio);
    1783         }
    1784 
    1785         blk_start_plug(&plug);
    1786 
    1787         for (segno = start_segno; segno < end_segno; segno++) {
    1788                 struct f2fs_summary_block *sum;
    1789 
    1790                 /* find segment summary of victim */
    1791                 struct folio *sum_folio = filemap_get_folio(META_MAPPING(sbi),
    1792                                         GET_SUM_BLOCK(sbi, segno));

Smatch gets a bit confused here and thinks filemap_get_folio() can return
a lot of different error pointers, but really filemap_get_folio() can only
return ERR_PTR(-ENOENT).  And possibly in this context, it can't even
return that?

One time email warning etc.  I could also mark filemap_get_folio() as
a no fail function to prevent false positives.

    1793 
    1794                 if (get_valid_blocks(sbi, segno, false) == 0)
    1795                         goto freed;
    1796                 if (gc_type == BG_GC && __is_large_section(sbi) &&
    1797                                 migrated >= sbi->migration_granularity)
    1798                         goto skip;
--> 1799                 if (!folio_test_uptodate(sum_folio) ||
                                                  ^^^^^^^^^
Dereferenced here.

    1800                     unlikely(f2fs_cp_error(sbi)))
    1801                         goto skip;
    1802 

regards,
dan carpenter


_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel