Bug 15827

Summary:	ext4_get_blocks may be called while ext4_truncate() is in progress
Product:	File System	Reporter:	Dmitry Monakhov (dmonakhov)
Component:	ext4	Assignee:	fs_ext4 (fs_ext4)
Status:	RESOLVED CODE_FIX
Severity:	high	CC:	dmonakhov, jack
Priority:	P1
Hardware:	All
OS:	Linux
Kernel Version:	v2.6.29-rc5	Subsystem:
Regression:	No	Bisected commit-id:
Attachments:	dmesg output debug patch against ext4.git/next + patches from bug #15792

Description Dmitry Monakhov 2010-04-21 15:36:07 UTC

Created attachment 26081 [details]
dmesg output

During truncate we may need to restart new transaction, to avoid deadlock on i_data_sem it was dropped 
commit 487caeef9fc08c0565e082c40a8aaf58dad92bbb
Author: Jan Kara <jack@suse.cz>
Date:   Mon Aug 17 22:17:20 2009 -0400

Jan given a good explanation why this approach would work, I have better explanation why this can't work work.
Yes we have blocked all writers beyond i_size, but writers(flush, page_mkwrite)
before i_size still may change node blocks, so 'path' which was lookup by truncate is not longer valid. So we are in big big troubles. 

I've add created inode's history tracer patch which spotted the issue.
See attachments.

Comment 1 Dmitry Monakhov 2010-04-21 15:41:13 UTC

Created attachment 26082 [details]
debug patch against ext4.git/next + patches from bug #15792

The debug patch is rather ugly, but still useful.

Comment 2 Dmitry Monakhov 2010-04-21 15:55:15 UTC

in given example both alloc and truncate tasks try to modify same path
path:  218:[1]33:108506
So ext4_ext_rm_leaf goes crazy because leaf node was modified.

Possible solutions:
1) Truncate_task: Drop path collected if ext4_ext_truncate_extend_restart result in journal_restart, ant retry lookup.
2) Save current truncate path some where in inode, so get_block may check
   about conflicts and to let truncate task to know that path is no longer valid.
   If so truncate task must goes back to (1).

Comment 3 Dmitry Monakhov 2010-04-22 04:36:10 UTC

Proposed patch
http://patchwork.ozlabs.org/patch/50687/

BTW IMHO it is also reasonable to replace simple
printk("strange request..") to something more scary
like EXT4_ERROR_INODE(inode, "strange request..")