From: Simon Glass <simon.glass@canonical.com> Several buffer cache issues cause problems when remounting: 1. bh_cache_insert() only checks block number, but the same block can be read with different sizes (e.g. superblock at 1K vs 4K). Check both block number and size when determining if already cached. 2. bh_cache_clear() leaves stale buffer references, causing memory leaks. Force the reference count to 1 before releasing since ext4 code won't access these buffers after unmount. 3. brelse() frees buffer heads when the reference count reaches zero, but cached buffer heads should only be freed by bh_cache_clear() during unmount. This causes a double-free. Add a BH_Cached flag to distinguish cached buffers from temporary ones: set BH_Cached in bh_cache_insert() when adding to cache, and in brelse() only free non-cached buffers when count reaches zero. Also fix a bit conflict: BH_OwnsData was using BH_JBDPrivateStart which conflicts with BH_BITMAP_UPTODATE in ext4.h. Move U-Boot private bits to start at BH_JBDPrivateStart + 1. Co-developed-by: Claude <noreply@anthropic.com> Signed-off-by: Simon Glass <simon.glass@canonical.com> --- (no changes since v1) fs/ext4l/ext4_uboot.h | 15 ++++++++++++--- fs/ext4l/support.c | 28 ++++++++++++++++++++++++---- 2 files changed, 36 insertions(+), 7 deletions(-) diff --git a/fs/ext4l/ext4_uboot.h b/fs/ext4l/ext4_uboot.h index 78d44faa0db..588670a8b62 100644 --- a/fs/ext4l/ext4_uboot.h +++ b/fs/ext4l/ext4_uboot.h @@ -521,12 +521,21 @@ struct sb_writers { #include <linux/jbd2.h> /* - * U-Boot: marks buffer owns b_data and should free it. - * Use BH_JBDPrivateStart to avoid conflicts with JBD2 state bits. + * U-Boot buffer head private bits. + * + * Start at BH_JBDPrivateStart + 1 because ext4.h uses BH_JBDPrivateStart + * for BH_BITMAP_UPTODATE. */ -#define BH_OwnsData BH_JBDPrivateStart +#define BH_OwnsData (BH_JBDPrivateStart + 1) BUFFER_FNS(OwnsData, ownsdata) +/* + * U-Boot: marks buffer is in the buffer cache. + * Cached buffers are freed by bh_cache_clear(), not brelse(). + */ +#define BH_Cached (BH_JBDPrivateStart + 2) +BUFFER_FNS(Cached, cached) + /* Forward declare for get_block_t */ struct inode; struct buffer_head; diff --git a/fs/ext4l/support.c b/fs/ext4l/support.c index 7f2bea4ca06..0f68746d99a 100644 --- a/fs/ext4l/support.c +++ b/fs/ext4l/support.c @@ -258,9 +258,10 @@ static void bh_cache_insert(struct buffer_head *bh) unsigned int hash = bh_cache_hash(bh->b_blocknr); struct bh_cache_entry *entry; - /* Check if already in cache */ + /* Check if already in cache - must match block AND size */ for (entry = bh_cache[hash]; entry; entry = entry->next) { - if (entry->bh && entry->bh->b_blocknr == bh->b_blocknr) + if (entry->bh && entry->bh->b_blocknr == bh->b_blocknr && + entry->bh->b_size == bh->b_size) return; /* Already cached */ } @@ -272,6 +273,9 @@ static void bh_cache_insert(struct buffer_head *bh) entry->next = bh_cache[hash]; bh_cache[hash] = entry; + /* Mark as cached so brelse() knows not to free it */ + set_buffer_cached(bh); + /* Add a reference to keep the buffer alive in cache */ atomic_inc(&bh->b_count); } @@ -316,7 +320,12 @@ void bh_cache_clear(void) struct buffer_head *bh = entry->bh; bh_clear_stale_jbd(bh); - /* Release the cache's reference */ + /* + * Force count to 1 so the buffer will be freed. + * On unmount, ext4 code won't access these + * buffers again, so extra references are stale. + */ + atomic_set(&bh->b_count, 1); if (atomic_dec_and_test(&bh->b_count)) free_buffer_head(bh); } @@ -629,13 +638,24 @@ struct buffer_head *sb_bread(struct super_block *sb, sector_t block) /** * brelse() - Release a buffer_head * @bh: Buffer head to release + * + * Decrements the reference count on the buffer. Cached buffer heads are + * freed by bh_cache_clear() on unmount, so this just decrements the count. + * Non-cached buffers are freed when the count reaches zero. */ void brelse(struct buffer_head *bh) { if (!bh) return; - if (atomic_dec_and_test(&bh->b_count)) + /* + * If buffer has JBD attached, don't let ref count go to zero. + * The journal owns a reference and will clean up properly. + */ + if (buffer_jbd(bh) && atomic_read(&bh->b_count) <= 1) + return; + + if (atomic_dec_and_test(&bh->b_count) && !buffer_cached(bh)) free_buffer_head(bh); } -- 2.43.0