> Jan Kara wrote:
> ...
>
> >> + /* try a sync to flush delalloc space & free resvd metadata */
> >> + if (!ext4_has_free_blocks(EXT4_SB(sb), 1) && dirtyblocks) {
> >> + if (!ext4_journal_current_handle()) {
> >> + down_read(&sb->s_umount);
> >> + sync_inodes_sb(sb);
> >> + up_read(&sb->s_umount);
> > ext4_should_retry_alloc() is called quite deep from the filesystem. In
> > particular we can hold i_mutex of some inodes etc. So I'd almost bet
> > that taking s_umount sem here violates lock ranking in some code paths
> > (an easy check would be to enable lockdep and stress the filesystem a
> > bit).
> > Also calling sync_inodes_sb() with i_mutex held just seems as a bad
> > thing to do although I don't see where it could deadlock and so it's
> > probably just a matter of taste...
>
> Well, to be honest I agree with you ;) It does still feel like a hack.
>
> > If we start writeback from ext4_nonda_switch as you do below, I think
> > that we should get decent results even without synchronous writeback in
> > the allocation path (maybe we'd need to tweak a bit the logic in
> > ext4_nonda_switch to provide more time for writeback thread to catchup).
>
> I think starting writeback helps a lot, but it seems that in the end we
> still need a synchronous attempt when we hit a real enocpc... after I
> finish dealing with this corruption thing I'll come back and look at this.
Without the synchronous attempt, it will never be perfect, that is
correct. But it could be quite close to perfect...
> Maybe we should put the writeback in for now, and worry about the
> synchronous sync-up later?
Yes, I'd do that for now.
Honza
--
Jan Kara <jack@xxxxxxx>
SuSE CR Labs
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
