* [PATCH] ext4: ext4_dx_add_entry should dirty directory metadata with the directory inode
@ 2011-08-11 21:15 Darrick J. Wong
2011-08-11 21:41 ` Andreas Dilger
0 siblings, 1 reply; 3+ messages in thread
From: Darrick J. Wong @ 2011-08-11 21:15 UTC (permalink / raw)
To: Theodore Ts'o; +Cc: linux-kernel, linux-ext4
ext4_dx_add_entry manipulates bh2 and frames[0].bh, which are two buffer_heads
that point to directory blocks assigned to the directory inode. However, the
function calls ext4_handle_dirty_metadata with the inode of the file that's
being added to the directory, not the directory inode itself. Therefore,
correct the code to dirty the directory buffers with the directory inode, not
the file inode.
Signed-off-by: Darrick J. Wong <djwong@us.ibm.com>
---
fs/ext4/namei.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c
index b754b77..79ddc43 100644
--- a/fs/ext4/namei.c
+++ b/fs/ext4/namei.c
@@ -1589,7 +1589,7 @@ static int ext4_dx_add_entry(handle_t *handle, struct dentry *dentry,
dxtrace(dx_show_index("node", frames[1].entries));
dxtrace(dx_show_index("node",
((struct dx_node *) bh2->b_data)->entries));
- err = ext4_handle_dirty_metadata(handle, inode, bh2);
+ err = ext4_handle_dirty_metadata(handle, dir, bh2);
if (err)
goto journal_error;
brelse (bh2);
@@ -1615,7 +1615,7 @@ static int ext4_dx_add_entry(handle_t *handle, struct dentry *dentry,
if (err)
goto journal_error;
}
- err = ext4_handle_dirty_metadata(handle, inode, frames[0].bh);
+ err = ext4_handle_dirty_metadata(handle, dir, frames[0].bh);
if (err) {
ext4_std_error(inode->i_sb, err);
goto cleanup;
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH] ext4: ext4_dx_add_entry should dirty directory metadata with the directory inode
2011-08-11 21:15 [PATCH] ext4: ext4_dx_add_entry should dirty directory metadata with the directory inode Darrick J. Wong
@ 2011-08-11 21:41 ` Andreas Dilger
2011-09-30 19:44 ` Darrick J. Wong
0 siblings, 1 reply; 3+ messages in thread
From: Andreas Dilger @ 2011-08-11 21:41 UTC (permalink / raw)
To: djwong; +Cc: Theodore Ts'o, linux-kernel, linux-ext4
On 2011-08-11, at 3:15 PM, Darrick J. Wong wrote:
> ext4_dx_add_entry manipulates bh2 and frames[0].bh, which are two buffer_heads
> that point to directory blocks assigned to the directory inode. However, the
> function calls ext4_handle_dirty_metadata with the inode of the file that's
> being added to the directory, not the directory inode itself. Therefore,
> correct the code to dirty the directory buffers with the directory inode, not
> the file inode.
Interesting. For journaled filesystems this is purely cosmetic, since
"handle" is valid and "inode" is unused in that case. For non-journal
filesystems it actually affects the correctness, since this buffer may
not be sync'd to disk even when the directory is marked "dirsync".
You can add my:
Reviewed-by: Andreas Dilger <adilger@dilger.ca>
> Signed-off-by: Darrick J. Wong <djwong@us.ibm.com>
> ---
>
> fs/ext4/namei.c | 4 ++--
> 1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c
> index b754b77..79ddc43 100644
> --- a/fs/ext4/namei.c
> +++ b/fs/ext4/namei.c
> @@ -1589,7 +1589,7 @@ static int ext4_dx_add_entry(handle_t *handle, struct dentry *dentry,
> dxtrace(dx_show_index("node", frames[1].entries));
> dxtrace(dx_show_index("node",
> ((struct dx_node *) bh2->b_data)->entries));
> - err = ext4_handle_dirty_metadata(handle, inode, bh2);
> + err = ext4_handle_dirty_metadata(handle, dir, bh2);
> if (err)
> goto journal_error;
> brelse (bh2);
> @@ -1615,7 +1615,7 @@ static int ext4_dx_add_entry(handle_t *handle, struct dentry *dentry,
> if (err)
> goto journal_error;
> }
> - err = ext4_handle_dirty_metadata(handle, inode, frames[0].bh);
> + err = ext4_handle_dirty_metadata(handle, dir, frames[0].bh);
> if (err) {
> ext4_std_error(inode->i_sb, err);
> goto cleanup;
> --
> To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
Cheers, Andreas
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] ext4: ext4_dx_add_entry should dirty directory metadata with the directory inode
2011-08-11 21:41 ` Andreas Dilger
@ 2011-09-30 19:44 ` Darrick J. Wong
0 siblings, 0 replies; 3+ messages in thread
From: Darrick J. Wong @ 2011-09-30 19:44 UTC (permalink / raw)
To: Andreas Dilger; +Cc: Theodore Ts'o, linux-kernel, linux-ext4
On Thu, Aug 11, 2011 at 03:41:59PM -0600, Andreas Dilger wrote:
> On 2011-08-11, at 3:15 PM, Darrick J. Wong wrote:
> > ext4_dx_add_entry manipulates bh2 and frames[0].bh, which are two buffer_heads
> > that point to directory blocks assigned to the directory inode. However, the
> > function calls ext4_handle_dirty_metadata with the inode of the file that's
> > being added to the directory, not the directory inode itself. Therefore,
> > correct the code to dirty the directory buffers with the directory inode, not
> > the file inode.
>
> Interesting. For journaled filesystems this is purely cosmetic, since
> "handle" is valid and "inode" is unused in that case. For non-journal
> filesystems it actually affects the correctness, since this buffer may
> not be sync'd to disk even when the directory is marked "dirsync".
>
> You can add my:
> Reviewed-by: Andreas Dilger <adilger@dilger.ca>
Hm.... any thoughts, Ted? Will this be picked up for 3.1/3.2?
--D
>
> > Signed-off-by: Darrick J. Wong <djwong@us.ibm.com>
> > ---
> >
> > fs/ext4/namei.c | 4 ++--
> > 1 files changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c
> > index b754b77..79ddc43 100644
> > --- a/fs/ext4/namei.c
> > +++ b/fs/ext4/namei.c
> > @@ -1589,7 +1589,7 @@ static int ext4_dx_add_entry(handle_t *handle, struct dentry *dentry,
> > dxtrace(dx_show_index("node", frames[1].entries));
> > dxtrace(dx_show_index("node",
> > ((struct dx_node *) bh2->b_data)->entries));
> > - err = ext4_handle_dirty_metadata(handle, inode, bh2);
> > + err = ext4_handle_dirty_metadata(handle, dir, bh2);
> > if (err)
> > goto journal_error;
> > brelse (bh2);
> > @@ -1615,7 +1615,7 @@ static int ext4_dx_add_entry(handle_t *handle, struct dentry *dentry,
> > if (err)
> > goto journal_error;
> > }
> > - err = ext4_handle_dirty_metadata(handle, inode, frames[0].bh);
> > + err = ext4_handle_dirty_metadata(handle, dir, frames[0].bh);
> > if (err) {
> > ext4_std_error(inode->i_sb, err);
> > goto cleanup;
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
>
>
> Cheers, Andreas
>
>
>
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2011-09-30 19:44 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-08-11 21:15 [PATCH] ext4: ext4_dx_add_entry should dirty directory metadata with the directory inode Darrick J. Wong
2011-08-11 21:41 ` Andreas Dilger
2011-09-30 19:44 ` Darrick J. Wong
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox