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(-)
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);
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
linux-stable-mirror@lists.linaro.org
-
Jan Kara -
Theodore Y. Ts'o