Re: [QUESTION] BUG_ON in ext4_mb_simple_scan_group

[Ritesh Harjani <riteshh@linux.ibm.com>]

Hello Kun,

On 4/16/20 7:49 PM, yangerkun wrote:
> Nowadays, we trigger the a bug that has been reported before[1](trigger 
> the bug with read block bitmap error before). After search the patch,
> I found some related patch which has not been included in our kernel.
> 
> eb5760863fc2 ext4: mark block bitmap corrupted when found instead of BUGON
> 736dedbb1a7d ext4: mark block bitmap corrupted when found
> 206f6d552d0c ext4: mark inode bitmap corrupted when found
> db79e6d1fb1f ext4: add new ext4_mark_group_bitmap_corrupted() helper
> 0db9fdeb347c ext4: fix wrong return value in ext4_read_inode_bitmap()

I see that you anyways have figured all these patches out.

> 
> Maybe this patch can fix the problem, but I am a little confused with
> the explain from Ted described in the mail:
> 
>  > What probably happened is that the page containing actual allocation
>  > bitmap was pushed out of memory due to memory pressure.  However, the
>  > buddy bitmap was still cached in memory.  That's actually quite
>  > possible since the buddy bitmap will often be referenced more
>  > frequently than the allocation bitmap (for example, while searching
>  > for free space of a specific size, and then having that block group
>  > skipped when it's not available).
> 
>  > Since there was an I/O error reading the allocation bitmap, the buffer
>  > is not valid.  So it's not surprising that the BUG_ON(k >= max) is
>  > getting triggered.

@Others, please correct me if I am wrong here.

So just as a small summary. Ext4 maintains an inode (we call it as
buddy cache inode which is sbi->s_buddy_cache) which stores the block
bitmap and buddy information for every block group. So we require 2
blocks for every block group to store both of this info in it.

So what generally happens is whenever there is a request to block
allocation, this(buddy and block bitmap information is loaded from the
disk into the page cache.

When someone does the block allocation these pages get loaded into the
page cache. And it will be there until these pages are getting heavily
used (that's coz of page eviction algo in mm).
But in case when the memory pressure is high, these pages may get
written out and eventually getting evicted from the page cache.
Now if any of this page is not present in the page cache we go and try
to read it from the disk. (I think that's the job of
ext4_mb_load_buddy_gfp()).

So let's say while reading this page from disk we get an I/O error,
so this means, as Ted explained, that the buffer which was not properly
read and hence it is not uptodate (and so we also don't set buffer
verified bit).
And in that case we should mark that block group corrupted. So that next
time, ext4_mb_good_group() does not allow us to do allocation from that
block group. I think some of the patches which you pointed add the logic
into the mballoc. So that we don't hit that bug_on().

{...
[Addition info - not related to your issue though]
So this could also be an e.g. where the grp->bb_free may not be uptodate
for a block group of which bitmap was not properly loaded.
...}


> 
> (Our machine: x86, 4K page size, 4K block size)
> 
> After check the related code, we found that once we get a IO error from 
> ext4_wait_block_bitmap, ext4_mb_init_cache will return directly with a 
> error number, so the latter ext4_mb_simple_scan_group may never been 
> called! So any other scene will trigger this BUG_ON?

Sorry that's not what I see in latest upstream kernel.
I am not sure which kernel version you are checking this on.
Check the latest upstream kernel and compare with it.


> 
> Thanks,
> Kun.
> 
> -----
> [1] https://www.spinics.net/lists/linux-ext4/msg60329.html
>