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
