Received-SPF: pass (google.com: domain of linux-ext4-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Subject: Re: [QUESTION] BUG_ON in ext4_mb_simple_scan_group
To:     linux-ext4@vger.kernel.org
Cc:     "zhangyi (F)" <yi.zhang@huawei.com>,
        yangerkun <yangerkun@huawei.com>, tytso@mit.edu, jack@suse.cz,
        dmonakhov@gmail.com, adilger@dilger.ca, bob.liu@oracle.com,
        wshilong@ddn.com
References: <9ba13e20-2946-897d-0b81-3ea7b21a4db6@huawei.com>
 <20200416183309.13914A404D@d06av23.portsmouth.uk.ibm.com>
 <39040d8c-9918-d976-a25a-0ec189f1e111@huawei.com>
 <20200417032644.75DF3A404D@d06av23.portsmouth.uk.ibm.com>
 <1740a49b-a10d-1bd3-a070-d76e9eb62fb2@huawei.com>
From:   Ritesh Harjani <riteshh@linux.ibm.com>
Date:   Fri, 1 May 2020 03:55:57 +0530
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101
 Thunderbird/60.7.2
MIME-Version: 1.0
In-Reply-To: <1740a49b-a10d-1bd3-a070-d76e9eb62fb2@huawei.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Message-Id: <20200430222558.805DEAE057@d06av26.portsmouth.uk.ibm.com>
Sender: linux-ext4-owner@vger.kernel.org
Precedence: bulk

>>
>>> If above is true, then may be we should not call
>>> "SetPageUptodate(page)", in case of an error reading block bitmap?
>>> Thoughts?
>>>
>> Yeah, it's better to set page uptodate only if all block bitmap buffers
>> are uptodate represent by this page.
> 
> 
> So I guess the *easier* thing to do here is to abort the loop which
> calls ext4_wait_block_bitmap() in ext4_mb_init_cache, similar to how
> the loop above it does (which calls for ext4_read_block_bitmap_nowait())
> 
> Since if any of the block bitmap buffer (which belongs to that page)
> could not be read properly, then we should not set the PageUptodate.
> (including for blocksize < pagesize where groups_per_page > 1) and
> no need of even setting up the in memory buddy and bitmap information
> (since we are anyway not going to use it).
> 
> Others can comment, if something else needs to be done?
> But I think over optimizing this logic for blocksize < pagesize
> may become an overkill? (since this mostly happens during an I/O error).
> 

Ok, so as I see this bug was possibly introduced to handle blocksize < 
pagesize case itself [1]. So it will make no sense to just optimize it
for blocksize == pagesize again. So I guess we should look into this
more closely rather then simply implementing above logic.

[1]: https://www.spinics.net/lists/linux-ext4/msg48111.html

So, I can get back to this some time later maybe.

-ritesh