| Message ID | 20191003220613.10791-4-jack@suse.cz |
|---|---|
| State | Superseded |
| Headers | show |
| Series | ext4: Fix transaction overflow due to revoke descriptors | expand |
On Fri, Oct 04, 2019 at 12:05:50AM +0200, Jan Kara wrote: > Estimate for the number of credits needed for final freeing of inode in > ext4_evict_inode() was to small. We may modify 4 blocks (inode & sb for > orphan deletion, bitmap & group descriptor for inode freeing) and not > just 3. The modification for the inode should already be included in the calculation for ext4_blocks_for_truncate(), no? So we only need 3 extra blocks (sb, inode bitmap, and bg descriptor for the inode). - Ted
On Sun 20-10-19 21:07:23, Theodore Y. Ts'o wrote: > On Fri, Oct 04, 2019 at 12:05:50AM +0200, Jan Kara wrote: > > Estimate for the number of credits needed for final freeing of inode in > > ext4_evict_inode() was to small. We may modify 4 blocks (inode & sb for > > orphan deletion, bitmap & group descriptor for inode freeing) and not > > just 3. > > The modification for the inode should already be included in the > calculation for ext4_blocks_for_truncate(), no? So we only need 3 > extra blocks (sb, inode bitmap, and bg descriptor for the inode). Yes, but 'extra_credits' is also passed to ext4_xattr_delete_inode() and if that needs to restart a transaction, it needs to reserve enough for inode modification in that new transaction. This patch is actually a result of assertion checks I was getting with more accurate transaction restart handling implemented later in this series... I agree we can actually subtract 3 from ext4_blocks_for_truncate(inode)+extra_credits when starting the initial transaction as inode changes get double-accounted there. I can do that and I'll also update the changelog to explain this better. Honza
diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c index 516faa280ced..e6b631d50c26 100644 --- a/fs/ext4/inode.c +++ b/fs/ext4/inode.c @@ -196,7 +196,12 @@ void ext4_evict_inode(struct inode *inode) { handle_t *handle; int err; - int extra_credits = 3; + /* + * Credits for final inode cleanup and freeing: + * sb + inode (ext4_orphan_del()), block bitmap, group descriptor + * (xattr block freeind), bitmap, group descriptor (inode freeing) + */ + int extra_credits = 6; struct ext4_xattr_inode_array *ea_inode_array = NULL; trace_ext4_evict_inode(inode);
Estimate for the number of credits needed for final freeing of inode in ext4_evict_inode() was to small. We may modify 4 blocks (inode & sb for orphan deletion, bitmap & group descriptor for inode freeing) and not just 3. Fixes: e50e5129f384 ("ext4: xattr-in-inode support") CC: stable@vger.kernel.org Signed-off-by: Jan Kara <jack@suse.cz> --- fs/ext4/inode.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)