ext3: fix some wrong comments
A message that is not in the archive (sent off-list or not yet received): [email protected]
PATCH 1/1 ext3: fix some wrong comments
The comments in the code are wrong, because every generic_file_write() is replaced by generic_file_aio_write().
--- fs/ext3/inode.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
fs/ext3/inode.c +2 -2
@@ -1574,7 +1574,7 @@ static int buffer_unmapped(handle_t *handle, struct buffer_head *bh)* * Similar for: * -* ext3_file_write() -> generic_file_write() -> __alloc_pages() -> ... +* ext3_file_write() -> generic_file_aio_write() -> __alloc_pages() -> ... * * Same applies to ext3_get_block(). We will deadlock on various things like * lock_journal and i_truncate_mutex.
@@ -3212,7 +3212,7 @@ out_brelse:* * We are called from a few places: * -* - Within generic_file_write() for O_SYNC files. +* - Within generic_file_aio_write() for O_SYNC files. * Here, there will be no transaction running. We wait for any running * transaction to commit. *
Signature · 12 lines
1.8.1.4 . _______________________________________________ kernel.openeuler mailing list [email protected] http://rnd-openeuler.huawei.com/mailman/listinfo/kernel.openeuler
Quoted 13 lines · ZhangZhen
The comments in the code are wrong, because every
generic_file_write()is replaced bygeneric_file_aio_write().Signed-off-by Zhang Zhen <[email protected]>--- fs/ext3/inode.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)fs/ext3/inode.c +0
-0
fs/ext3/inode.c static int buffer_unmapped(handle_t *handle, struct buffer_head *bh) · line 1577
Quoted 5 lines
fs/ext3/inode.c +1
-1@@ -1574,7 +1574,7 @@ static int buffer_unmapped(handle_t *handle, struct buffer_head *bh)* * Similar for: * -* ext3_file_write() -> generic_file_write() -> __alloc_pages() -> ... +* ext3_file_write() -> generic_file_aio_write() -> __alloc_pages() -> ...
Well, but ext3_file_write() doesn't exist either so the comment obviously
needs further updates.
fs/ext3/inode.c out_brelse: · line 3218
Quoted 12 lines
fs/ext3/inode.c +1
-1* * Same applies to ext3_get_block(). We will deadlock on various things like * lock_journal and i_truncate_mutex. @@ -3212,7 +3212,7 @@ out_brelse:* * We are called from a few places: * -* - Within generic_file_write() for O_SYNC files. +* - Within generic_file_aio_write() for O_SYNC files. * Here, there will be no transaction running. We wait for any running * transaction to commit. *
This is correct but as I'm looking through the comment it needs much
larger update. So something like attached patches looks more appropriate. Thanks for pointing me to those outdated comments.
Honza
Signature · 2 lines
Jan Kara <[email protected]> SUSE Labs, CR
Quoted 1 line
From
13c8493bdd777508e6755454d4284c2497907c8eMon Sep 17 00:00:00 2001
From: Jan Kara <[email protected]>
Date: Fri, 28 Feb 2014 09:09:24 +0100
Subject: [PATCH 1/2] ext3: Update PF_MEMALLOC handling in ext3_write_inode()
The special handling of PF_MEMALLOC callers in ext3_write_inode() shouldn't be necessary as there shouldn't be any. Warn about it. Also update comment before the function as it seems somewhat outdated.
--- fs/ext3/inode.c | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-)
fs/ext3/inode.c +11 -12
@@ -3209,21 +3209,20 @@ out_brelse:* * We are called from a few places: * -* - Within generic_file_write() for O_SYNC files. +* - Within generic_file_aio_write() -> generic_write_sync() for O_SYNC files. * Here, there will be no transaction running. We wait for any running * transaction to commit. * -* - Within sys_sync(), kupdate and such. -* We wait on commit, if tol to. +* - Within flush work (for sys_sync(), kupdate and such). +* We wait on commit, if told to. * -* - Within prune_icache() (PF_MEMALLOC == true) -* Here we simply return. We can't afford to block kswapd on the -* journal commit. +* - Within iput_final() -> write_inode_now() +* We wait on commit, if told to. * * In all cases it is actually safe for us to return without doing anything, * because the inode has been copied into a raw inode buffer in -* ext3_mark_inode_dirty(). This is a correctness thing for O_SYNC and for -* knfsd. +* ext3_mark_inode_dirty(). This is a correctness thing for WB_SYNC_ALL +* writeback. * * Note that we are absolutely dependent upon all inode dirtiers doing the * right thing: they *must* call mark_inode_dirty() after dirtying info in
@@ -3235,13 +3234,13 @@ out_brelse:* stuff(); * inode->i_size = expr; * -* is in error because a kswapd-driven write_inode() could occur while -* `stuff()' is running, and the new i_size will be lost. Plus the inode -* will no longer be on the superblock's dirty inode list. +* is in error because write_inode() could occur while `stuff()' is running, +* and the new i_size will be lost. Plus the inode will no longer be on the +* superblock's dirty inode list. */ int ext3_write_inode (struct inode *inode,struct writeback_control *wbc) { -if (current->flags &PF_MEMALLOC ) +if (WARN_ON_ONCE (current->flags &PF_MEMALLOC ))return 0 ;if (ext3_journal_current_handle ()) {
Signature · 1 line
1.8.1.4
Quoted 1 line
From
ff75d02aa0fabc7379677b7ede181acb0204c77dMon Sep 17 00:00:00 2001
From: Jan Kara <[email protected]>
Date: Fri, 28 Feb 2014 09:31:10 +0100
Subject: [PATCH 2/2] ext3: Update outdated comment before
ext3_ordered_writepage()
The comment is heavily outdated. The recursion into the filesystem isn't possible because we use GFP_NOFS for our allocations, the issue about block_write_full_page() dirtying tail page is long resolved as well (that function doesn't dirty buffers at all), and finally we don't start a transaction if all blocks are already allocated and mapped.
--- fs/ext3/inode.c | 47 ++++------------------------------------------- 1 file changed, 4 insertions(+), 43 deletions(-)
fs/ext3/inode.c +4 -43
60 lines not on this page · raw message
Signature · 1 line
1.8.1.4
Quoted 38 lines · Jan Kara
Quoted 13 lines · ZhangZhen
The comments in the code are wrong, because every
generic_file_write()is replaced bygeneric_file_aio_write().Signed-off-by Zhang Zhen <[email protected]>--- fs/ext3/inode.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)fs/ext3/inode.c +0
-0fs/ext3/inode.c static int buffer_unmapped(handle_t *handle, struct buffer_head *bh) · line 1577
Quoted 5 lines
fs/ext3/inode.c +1
-1@@ -1574,7 +1574,7 @@ static int buffer_unmapped(handle_t *handle, struct buffer_head *bh)* * Similar for: * -* ext3_file_write() -> generic_file_write() -> __alloc_pages() -> ... +* ext3_file_write() -> generic_file_aio_write() -> __alloc_pages() -> ... Well, but
ext3_file_write()doesn't exist either so the comment obviously
needs further updates.fs/ext3/inode.c out_brelse: · line 3218
Quoted 12 lines
fs/ext3/inode.c +1
-1* * Same applies to ext3_get_block(). We will deadlock on various things like * lock_journal and i_truncate_mutex. @@ -3212,7 +3212,7 @@ out_brelse:* * We are called from a few places: * -* - Within generic_file_write() for O_SYNC files. +* - Within generic_file_aio_write() for O_SYNC files. * Here, there will be no transaction running. We wait for any running * transaction to commit. * This is correct but as I'm looking through the comment it needs much
larger update. So something like attached patches looks more appropriate. Thanks for pointing me to those outdated comments.
Quoted 2 lines
Honza
Hi Honza，
Your fix is more better than mine. It's a good chance for me to learn, thanks!
Zhen
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html