On Tue 26-07-11 19:52:20, Al Viro wrote: > On Tue, Jul 26, 2011 at 11:36:29AM -0700, Linus Torvalds wrote: > > > So I *really* want people to take a look at that ext3_sync_file() > > function. Please? > > While we are at it, could somebody please explain what the hell is ext4 > doing in > static int ext4_sync_parent(struct inode *inode) > { > struct writeback_control wbc; > struct dentry *dentry = NULL; > int ret = 0; > > while (inode && ext4_test_inode_state(inode, EXT4_STATE_NEWENTRY)) { > ext4_clear_inode_state(inode, EXT4_STATE_NEWENTRY); > dentry = list_entry(inode->i_dentry.next, > struct dentry, d_alias); > if (!dentry || !dentry->d_parent || !dentry->d_parent->d_inode) > break; > inode = dentry->d_parent->d_inode; > ret = sync_mapping_buffers(inode->i_mapping); > ... > Note that dentry obviously can't be NULL there. dentry->d_parent is never > NULL. And dentry->d_parent would better not be negative, for crying out > loud! What's worse, there's no guarantees that dentry->d_parent will > remain our parent over that sync_mapping_buffers() *and* that inode won't > just be freed under us (after rename() and memory pressure leading to > eviction of what used to be our dentry->d_parent). Moreover, even if > inode survives in icache, there is no promise that it will have an alias > in dcache by the time we get to the next iteration of the loop, so this > list_entry() next time around can bloody well happen to &inode->i_dentry, > dentry being a garbage address somewhere inside that struct inode (or a > bit above it - I hadn't compared offsets). > > What the hell is going on there? It appeared more than a year ago in commit > 14ece1028b3ed53ffec1b1213ffc6acaf79ad77c > Author: Frank Mayhar <fmayhar@xxxxxxxxxx> > Date: Mon May 17 08:00:00 2010 -0400 > ext4: Make fsync sync new parent directories in no-journal mode > > and it had remained broken ever after... It really looks broken. Added Ted to CC... Honza -- Jan Kara <jack@xxxxxxx> SUSE Labs, CR -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html