Re: [PATCH] ext4: correct best extent lstart adjustment logic

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 2024/2/1 19:08, Ojaswin Mujoo wrote:

Hi Ojaswin, Jan

Hi Baokun, Jan

Thanks for the CC, I somehow missed this patch.

As described in the discussion Jan linked [1] , there is a known bug in the
normalize code (which i should probably get back to now ) where we sometimes
end up with a goal range which doesn't completely cover the original extent and
this was causing issues when we tried to cover the complete original request in
the PA window adjustment logic. That and to minimize fragmentation, we ended up
going with the logic we have right now.

In short, I agree that in the example Baokun pointed out, it is not optimal to
have to make an allocation request twice when we can get it in one go.

I also think Baokun is correct that if keeping the best extent at the end doesn't
cover the original start, then any other case should not lead to it overflowing out
of goal extent, including the case where original extent is overflowing goal extent.

So, as mentioned, it boils down to a trade off between multiple allocations and slightly
increased fragmentation. iiuc preallocations are anyways dropped when the file closes
so I think it shouldn't hurt too much fragmentation wise to prioritize less
allocations. What are your thoughts on this Jan, Baokun?

Coming to the code, the only thing I think might cause an issue is the following line:

-		BUG_ON(ac->ac_o_ex.fe_len > ac->ac_b_ex.fe_len);
+		BUG_ON(o_ex_end > extent_logical_end(sbi, &ex));

So as discussed towards the end here [1] we could have ac_o_ex that
overflows the goal and hence would be beyond the best length. I'll try
to look into the normalize logic to fix this however till then, I think
we should not have this BUG_ON since it would crash the kernel if this
happens.

Rest of it looks good to me.

Regards,
Ojaswin

[1]
https://lore.kernel.org/all/Y+UzQJRIJEiAr4Z4@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx/
I will remove the problematic BUG_ON and add some comments in
the next version.

Thanks to Ojaswin and Jan for the review!
--
With Best Regards,
Baokun Li
.




[Index of Archives]     [Reiser Filesystem Development]     [Ceph FS]     [Kernel Newbies]     [Security]     [Netfilter]     [Bugtraq]     [Linux FS]     [Yosemite National Park]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Device Mapper]     [Linux Media]

  Powered by Linux