On Fri 03-09-21 08:48:38, Theodore Ts'o wrote: > On Fri, Sep 03, 2021 at 12:05:38PM +0300, Dan Carpenter wrote: > > No one expects error logging functions to sleep so sometimes they are > > called with spinlocks held. In this case the problematic call tree is: > > > > ext2_statfs() <- disables preempt > > -> ext2_count_free_inodes() > > -> ext2_get_group_desc() > > -> ext2_error() > > > > Signed-off-by: Dan Carpenter <dan.carpenter@xxxxxxxxxx> > > --- > > This is just from static analysis. NOT TESTED! > > > > Probably a safer fix would be to just call pr_err() instead of > > ext2_error() in ext2_get_group_desc(). I can send that fix instead if > > people want. > > Looking at both of the ext2_error() calls in ext2_get_group_desc(), > those are really more in the way of assertions rather than warning of > an on-disk corruption issue. The second "group descriptor not loaded" > should never happen, and the "block_group >= groups_count" should have > been caught via an invalid block number or check by the caller (or an > outright code bug in say ext2_statfs(). > > So I suspect both of those would be more usefule as a WARN() rather > than a call to ext2_error(), since stack trace would actually provide > more useful data to root causing the issue. Jan, what do you think? Yes, I agree. Definitely better than not flushing error on other ext2_error() calls. BTW, Dan, I don't see a patch with WARN() in my inbox. Did it get lost somewhere? Honza -- Jan Kara <jack@xxxxxxxx> SUSE Labs, CR
