Messages in this thread | | | Subject | Re: [PATCH] ext4: deaccount delayed allocations at freeing inode in ext4_evict_inode() | From | Konstantin Khlebnikov <> | Date | Fri, 8 Nov 2019 11:30:56 +0300 |
| |
On 08/11/2019 05.08, Ritesh Harjani wrote: > > > On 10/29/19 12:47 PM, Konstantin Khlebnikov wrote: >> If inode->i_blocks is zero then ext4_evict_inode() skips ext4_truncate(). >> Delayed allocation extents are freed later in ext4_clear_inode() but this >> happens when quota reference is already dropped. This leads to leak of >> reserved space in quota block, which disappears after umount-mount. >> >> This seems broken for a long time but worked somehow until recent changes >> in delayed allocation. > > Sorry, I may have missed it, but could you please help understand > what recent changes in delayed allocation make this break or worse?
I don't see problem for 4.19. Haven't bisected yet. Most likely this is around 'reserved cluster accounting'.
I suspect before these changes something always triggered da before unlink and space usage committed and then truncated at eviction.
> > > A silly query, since I couldn't figure it out. Maybe the code has been > there ever since like this:-
> So why can't we just move drop_dquot later after the ext4_es_remove_extent() (in function ext4_clear_inode)? Any known > problems around that?
Clear_inode is called also when inode evicts from cache while it has nlinks and stays at disk. I'm not sure how this must interact with reserves.
> > -ritesh > > >> >> Signed-off-by: Konstantin Khlebnikov <khlebnikov@yandex-team.ru> >> --- >> fs/ext4/inode.c | 9 +++++++++ >> 1 file changed, 9 insertions(+) >> >> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c >> index 516faa280ced..580898145e8f 100644 >> --- a/fs/ext4/inode.c >> +++ b/fs/ext4/inode.c >> @@ -293,6 +293,15 @@ void ext4_evict_inode(struct inode *inode) >> inode->i_ino, err); >> goto stop_handle; >> } >> + } else if (EXT4_I(inode)->i_reserved_data_blocks) { >> + /* Deaccount reserve if inode has only delayed allocations. */ >> + err = ext4_es_remove_extent(inode, 0, EXT_MAX_BLOCKS); >> + if (err) { >> + ext4_warning(inode->i_sb, >> + "couldn't remove extents %lu (err %d)", >> + inode->i_ino, err); >> + goto stop_handle; >> + } >> } >> >> /* Remove xattr references. */ >> >
| |