On Sun 22-01-17 17:25:27, Ted Tso wrote: > > > @@ -1497,12 +1493,11 @@ int ext4_expand_extra_isize_ea(struct inode *inode, int new_extra_isize, > > > int error = 0, tried_min_extra_isize = 0; > > > int s_min_extra_isize = le16_to_cpu(EXT4_SB(inode->i_sb)->s_es->s_min_extra_isize); > > > int isize_diff; /* How much do we need to grow i_extra_isize */ > > > + int no_expand; > > > + > > > + if (ext4_write_trylock_xattr(inode, &no_expand) == 0) > > > + return 0; > > > > Why do you play tricks with trylock here? ext4_mark_inode_dirty() checks > > EXT4_STATE_NO_EXPAND and thus we should not ever get here if we already > > hold xattr_sem... > > The problem is still a lock inversion in the truncate code path. The > simplest way of dealing with it to simply avoiding doing the > expand_isize operation on truncates. In the case where this is > happening on the deletion of an inode, doing the expansion is > pointless anyway. I see, thanks for explanation. Well seeing all these problems with ext4_expand_extra_isize() wouldn't we be better off by not calling it from ext4_mark_inode_dirty() but rather explicitely from several well-defined places? Because this implicit calling looks like it causes us too much trouble. Honza -- Jan Kara <jack@xxxxxxxx> SUSE Labs, CR
