From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from m16.mail.126.com (m16.mail.126.com [220.197.31.8]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 4B8FC18EB0 for ; Thu, 1 Jan 2026 11:33:48 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=220.197.31.8 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1767267232; cv=none; b=MEFl4p+oa4u2pmucXorN3GIzKSCFNW8HWn+2z7c/0Gw6UhFuE+GNwt2TI9tMbJLL5afJzC/zlSN4DMztRFl4HlLdwJFxI8nk2tG65IVvPIaZfbfcmLrXr/KHYqUX3NlY87CUk+Zp/jBNacg5WHuBoM3yDhMXiPX2sTo/6XIZZBQ= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1767267232; c=relaxed/simple; bh=4GaynoskhS56qRtY05UaXbZDTDKopSHNT5yIoGaH2VA=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=VFFHdAPSJqeQ2CRVrzeVjQ3OB+W49iHb95ZLUxKdP0sAkL+hx1gRYakZ+M6AHl+DAqo+/VVYzX0QoJZpwXeio/8w9zgJcSka2ureT1FZo/FGwLqGGlPydtF+VVyr26WixZwh3wPf9l++WMPX1raf3anxQTrTgWiQnlH9tLYdDto= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=126.com; spf=pass smtp.mailfrom=126.com; dkim=pass (1024-bit key) header.d=126.com header.i=@126.com header.b=hKpwsQv2; arc=none smtp.client-ip=220.197.31.8 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=126.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=126.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=126.com header.i=@126.com header.b="hKpwsQv2" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=126.com; s=s110527; h=Message-ID:Date:MIME-Version:Subject:To:From: Content-Type; bh=wRJ0M5Rhvpmu0SmgyWolmNnapBivis+1yU+V0txmr5I=; b=hKpwsQv2V8ur+2YOqg5yPalrkiSWLpdmgTx+w09ssbNlaK+DgKPsEeJx8Qgukl 8Pd32Y8wb0tI1d9kESkXgAsyky+LGcn/VI3O/EOwDS67UAZbvGexVa9KC0Qk2mbU wtqjJ0slifvOzPTnV7YuXf1J/9V7s5Yg92lynkVevz3SQ= Received: from [IPV6:2409:8a20:4df7:2871:62a7:9187:4185:993d] (unknown []) by gzga-smtp-mtada-g0-4 (Coremail) with SMTP id _____wDn9_BtWFZplu2qBA--.64963S2; Thu, 01 Jan 2026 19:20:13 +0800 (CST) Message-ID: Date: Thu, 1 Jan 2026 19:20:13 +0800 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [f2fs-dev] [PATCH 1/2 v4] f2fs: support large folio for immutable non-compressed case To: Jaegeuk Kim Cc: Chao Yu , linux-kernel@vger.kernel.org, linux-f2fs-devel@lists.sourceforge.net References: <20251120235446.1947532-1-jaegeuk@kernel.org> <0153ff69-789d-4fe1-a89c-0c607a9a7d6c@kernel.org> <0bf2eedd-eebb-440b-96f5-72ac3a68b608@kernel.org> Content-Language: en-US From: Nanzhe Zhao In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-CM-TRANSID:_____wDn9_BtWFZplu2qBA--.64963S2 X-Coremail-Antispam: 1Uf129KBjvJXoWxZw13GFWUXF1rWw43AF47Arb_yoWrCFyDpF WDKas3KrWDXr48ur1IyF1kXr1S934rWF45uaykGw13AF1aqrnakF1kGa4YvFW2yrWxAFsa qw4Ygrykua1DZaDanT9S1TB71UUUUU7qnTZGkaVYY2UrUUUUjbIjqfuFe4nvWSU5nxnvy2 9KBjDUYxBIdaVFxhVjvjDU0xZFpf9x07UG4EiUUUUU= X-CM-SenderInfo: xq22xtbr6rjloofrz/xtbBog4h8GlWWG4uzgAA3U Dear Kim: Happy New Year! > +static struct f2fs_folio_state * > +ffs_find_or_alloc(struct folio *folio) > +{ > + struct f2fs_folio_state *ffs = folio->private; > + > + if (ffs) > + return ffs; > + > + ffs = f2fs_kmem_cache_alloc(ffs_entry_slab, GFP_NOIO, true, NULL); > + > + spin_lock_init(&ffs->state_lock); > + folio_attach_private(folio, ffs); > + return ffs; > +} It looks like ffs_find_or_alloc() does not initialize read_pages_pending. When I debug locally, printing read_pages_pending shows an undefined random value. Also, when I run a basic read test with dd, tasks can hang (because read_pages_pending never reaches zero, so the folio is never unlocked and never marked uptodate). I know this function is modeled after iomap's ifs_alloc(): static struct iomap_folio_state *ifs_alloc(struct inode *inode, struct folio *folio, unsigned int flags) { struct iomap_folio_state *ifs = folio->private; unsigned int nr_blocks = i_blocks_per_folio(inode, folio); gfp_t gfp; if (ifs || nr_blocks <= 1) return ifs; /*...*/ /* * ifs->state tracks two sets of state flags when the * filesystem block size is smaller than the folio size. * The first state tracks per-block uptodate and the * second tracks per-block dirty state. */ ifs = kzalloc(struct_size(ifs, state, BITS_TO_LONGS(2 * nr_blocks)), gfp); if (!ifs) return ifs; spin_lock_init(&ifs->state_lock); if (folio_test_uptodate(folio)) bitmap_set(ifs->state, 0, nr_blocks); if (folio_test_dirty(folio)) bitmap_set(ifs->state, nr_blocks, nr_blocks); folio_attach_private(folio, ifs); return ifs; } Note ifs_alloc() uses kzalloc(), which zero-initializes the allocated memory by default while f2fs_kmem_cache_alloc() does not. We could fix this by explicitly setting read_pages_pending = 0, or by doing a memset() right after f2fs_kmem_cache_alloc() (the latter seems more extensible if the struct grows). What do you think? > /* > + * This page will go to BIO. Do we need to send this > + * BIO off first? > + */ > + if (bio && (!page_is_mergeable(F2FS_I_SB(inode), bio, > + last_block_in_bio, block_nr) || > + !f2fs_crypt_mergeable_bio(bio, inode, index, NULL))) { > +submit_and_realloc: > + f2fs_submit_read_bio(F2FS_I_SB(inode), bio, DATA); > + bio = NULL; > + } > + if (bio == NULL) > + bio = f2fs_grab_read_bio(inode, block_nr, > + max_nr_pages, > + f2fs_ra_op_flags(rac), > + index, false); > + > + /* > + * If the page is under writeback, we need to wait for > + * its completion to see the correct decrypted data. > + */ > + f2fs_wait_on_block_writeback(inode, block_nr); > + > + if (!bio_add_folio(bio, folio, F2FS_BLKSIZE, > + offset << PAGE_SHIFT)) > + goto submit_and_realloc; > + > + if (folio_test_large(folio)) { > + ffs = ffs_find_or_alloc(folio); > + > + /* set the bitmap to wait */ > + spin_lock_irq(&ffs->state_lock); > + ffs->read_pages_pending++; > + spin_unlock_irq(&ffs->state_lock); > + } In the current code, it looks like a subpage is added to the BIO (or a cached BIO is submitted) before read_pages_pending is incremented. This can cause the following behaviour: After one subpage of a folio is submitted, if the I/O completes very fast, the endio path may interrupt the read loop, run bio_endio, and eventually call f2fs_finish_read_bio(), which decrements read_pages_pending down to zero. That can make folio_finish_read() run too early, even though other parts of the same folio have not been added to a BIO yet. I managed to trigger this locally by creating a heavily fragmented file and temporarily injecting the following code right after BIO submission: f2fs_io_schedule_timeout(1); WARN_ON_ONCE(!folio_test_locked(folio)); I think the correct ordering is to increment read_pages_pending first, and then add the corresponding subpage to the BIO. In that ordering, the BIO side will either: 1) add a subpage after the increment (matching the new pending count), or 2) submit a BIO that corresponds to the pending increment from the ** previous iteration **, so read_pages_pending will not reach zero prematurely. This is exactly the order that iomap_readpage_iter() implements. If you need the script I used to reproduce the bug, please let me know. I will attach it in my next reply. Thanks! Best regards, Nanzhe Zhao