Re: [PATCH v4 15/16] btrfs: handle discarding fully-remapped block groups

[Mark Harmstone <mark@harmstone.com>]

On 31/10/2025 10.12 pm, Boris Burkov wrote:
> On Tue, Oct 28, 2025 at 12:04:11AM +0800, kernel test robot wrote:
>> Hi Mark,
>>
>> kernel test robot noticed the following build warnings:
>>
>> [auto build test WARNING on kdave/for-next]
>> [also build test WARNING on next-20251027]
>> [cannot apply to linus/master v6.18-rc3]
>> [If your patch is applied to the wrong git tree, kindly drop us a note.
>> And when submitting patch, we suggest to use '--base' as documented in
>> https://git-scm.com/docs/git-format-patch#_base_tree_information]
>>
>> url:    https://github.com/intel-lab-lkp/linux/commits/Mark-Harmstone/btrfs-add-definitions-and-constants-for-remap-tree/20251025-021910
>> base:   https://git.kernel.org/pub/scm/linux/kernel/git/kdave/linux.git for-next
>> patch link:    https://lore.kernel.org/r/20251024181227.32228-16-mark%40harmstone.com
>> patch subject: [PATCH v4 15/16] btrfs: handle discarding fully-remapped block groups
>> config: arm-randconfig-003-20251027 (https://download.01.org/0day-ci/archive/20251027/202510272322.N1S5rdDc-lkp@intel.com/config)
>> compiler: arm-linux-gnueabi-gcc (GCC) 8.5.0
>> reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20251027/202510272322.N1S5rdDc-lkp@intel.com/reproduce)
>>
>> If you fix the issue in a separate patch/commit (i.e. not just a new version of
>> the same patch/commit), kindly add following tags
>> | Reported-by: kernel test robot <lkp@intel.com>
>> | Closes: https://lore.kernel.org/oe-kbuild-all/202510272322.N1S5rdDc-lkp@intel.com/
>>
>> Note: it may well be a FALSE warning. FWIW you are at least aware of it now.
>> http://gcc.gnu.org/wiki/Better_Uninitialized_Warnings
>>
>> All warnings (new ones prefixed by >>):
>>
>>     fs/btrfs/discard.c: In function 'btrfs_discard_workfn':
>>>> fs/btrfs/discard.c:596:6: warning: 'discard_state' may be used uninitialized in this function [-Wmaybe-uninitialized]
>>        if (discard_state == BTRFS_DISCARD_BITMAPS ||
>>           ^
> 
> I think this gets set by peek_discard_list() so I don't think this is
> a valid warning.

You are correct. discard_state gets initialized if the return value of 
peek_discard_list() is not NULL, and if it is NULL we return before we 
use it.

This is an ancient version of GCC, the warning doesn't trigger on GCC 15 
- presumably it has better control flow analysis. I don't think the 
robot should be compiling with this warning turned on for old compiler 
versions, if it's prone to false positives.

>>
>>
>> vim +/discard_state +596 fs/btrfs/discard.c
>>
>>     513	
>>     514	/*
>>     515	 * Discard work queue callback
>>     516	 *
>>     517	 * @work: work
>>     518	 *
>>     519	 * Find the next block_group to start discarding and then discard a single
>>     520	 * region.  It does this in a two-pass fashion: first extents and second
>>     521	 * bitmaps.  Completely discarded block groups are sent to the unused_bgs path.
>>     522	 */
>>     523	static void btrfs_discard_workfn(struct work_struct *work)
>>     524	{
>>     525		struct btrfs_discard_ctl *discard_ctl;
>>     526		struct btrfs_block_group *block_group;
>>     527		enum btrfs_discard_state discard_state;
>>     528		int discard_index = 0;
>>     529		u64 trimmed = 0;
>>     530		u64 minlen = 0;
>>     531		u64 now = ktime_get_ns();
>>     532	
>>     533		discard_ctl = container_of(work, struct btrfs_discard_ctl, work.work);
>>     534	
>>     535		block_group = peek_discard_list(discard_ctl, &discard_state,
>>     536						&discard_index, now);
>>     537		if (!block_group)
>>     538			return;
>>     539		if (!btrfs_run_discard_work(discard_ctl)) {
>>     540			spin_lock(&discard_ctl->lock);
>>     541			btrfs_put_block_group(block_group);
>>     542			discard_ctl->block_group = NULL;
>>     543			spin_unlock(&discard_ctl->lock);
>>     544			return;
>>     545		}
>>     546		if (now < block_group->discard_eligible_time) {
>>     547			spin_lock(&discard_ctl->lock);
>>     548			btrfs_put_block_group(block_group);
>>     549			discard_ctl->block_group = NULL;
>>     550			spin_unlock(&discard_ctl->lock);
>>     551			btrfs_discard_schedule_work(discard_ctl, false);
>>     552			return;
>>     553		}
>>     554	
>>     555		/* Perform discarding */
>>     556		minlen = discard_minlen[discard_index];
>>     557	
>>     558		switch (discard_state) {
>>     559		case BTRFS_DISCARD_BITMAPS: {
>>     560			u64 maxlen = 0;
>>     561	
>>     562			/*
>>     563			 * Use the previous levels minimum discard length as the max
>>     564			 * length filter.  In the case something is added to make a
>>     565			 * region go beyond the max filter, the entire bitmap is set
>>     566			 * back to BTRFS_TRIM_STATE_UNTRIMMED.
>>     567			 */
>>     568			if (discard_index != BTRFS_DISCARD_INDEX_UNUSED)
>>     569				maxlen = discard_minlen[discard_index - 1];
>>     570	
>>     571			btrfs_trim_block_group_bitmaps(block_group, &trimmed,
>>     572					       block_group->discard_cursor,
>>     573					       btrfs_block_group_end(block_group),
>>     574					       minlen, maxlen, true);
>>     575			discard_ctl->discard_bitmap_bytes += trimmed;
>>     576	
>>     577			break;
>>     578		}
>>     579	
>>     580		case BTRFS_DISCARD_FULLY_REMAPPED:
>>     581			btrfs_trim_fully_remapped_block_group(block_group);
>>     582			break;
>>     583	
>>     584		default:
>>     585			btrfs_trim_block_group_extents(block_group, &trimmed,
>>     586					       block_group->discard_cursor,
>>     587					       btrfs_block_group_end(block_group),
>>     588					       minlen, true);
>>     589			discard_ctl->discard_extent_bytes += trimmed;
>>     590	
>>     591			break;
>>     592		}
>>     593	
>>     594		/* Determine next steps for a block_group */
>>     595		if (block_group->discard_cursor >= btrfs_block_group_end(block_group)) {
>>   > 596			if (discard_state == BTRFS_DISCARD_BITMAPS ||
>>     597			    discard_state == BTRFS_DISCARD_FULLY_REMAPPED) {
>>     598				btrfs_finish_discard_pass(discard_ctl, block_group);
>>     599			} else {
>>     600				block_group->discard_cursor = block_group->start;
>>     601				spin_lock(&discard_ctl->lock);
>>     602				if (block_group->discard_state !=
>>     603				    BTRFS_DISCARD_RESET_CURSOR)
>>     604					block_group->discard_state =
>>     605								BTRFS_DISCARD_BITMAPS;
>>     606				spin_unlock(&discard_ctl->lock);
>>     607			}
>>     608		}
>>     609	
>>     610		now = ktime_get_ns();
>>     611		spin_lock(&discard_ctl->lock);
>>     612		discard_ctl->prev_discard = trimmed;
>>     613		discard_ctl->prev_discard_time = now;
>>     614		btrfs_put_block_group(block_group);
>>     615		discard_ctl->block_group = NULL;
>>     616		__btrfs_discard_schedule_work(discard_ctl, now, false);
>>     617		spin_unlock(&discard_ctl->lock);
>>     618	}
>>     619	
>>
>> -- 
>> 0-DAY CI Kernel Test Service
>> https://github.com/intel/lkp-tests/wiki