* [Ocfs2-devel] [PATCH] ocfs2: make direntry invalid when deleting it
@ 2011-07-12 8:43 Wengang Wang
2011-07-12 15:31 ` Sunil Mushran
` (2 more replies)
0 siblings, 3 replies; 5+ messages in thread
From: Wengang Wang @ 2011-07-12 8:43 UTC (permalink / raw)
To: ocfs2-devel
When we deleting a direntry from a directory, if it's the first in a block we
invalid it by setting inode to 0; otherwise, we merge the deleted one to the
prior and contiguous direntry. And we don't truncate directories.
There is a problem for the later case since inode is not set to 0.
This problem happens when the caller passes a file position as parameter to
ocfs2_dir_foreach_blk(). If the position happens to point to a stale(not
the first, deleted in betweens of ocfs2_dir_foreach_blk()s) direntry, we are
not able to recognize its staleness. So that we treat it as a live one wrongly.
The fix is to set inode to 0 in both cases indicating the direntry is stale.
This won't introduce additional IOs.
Signed-off-by: Wengang Wang <wen.gang.wang@oracle.com>
---
fs/ocfs2/dir.c | 3 +--
1 files changed, 1 insertions(+), 2 deletions(-)
diff --git a/fs/ocfs2/dir.c b/fs/ocfs2/dir.c
index 8582e3f..3302088 100644
--- a/fs/ocfs2/dir.c
+++ b/fs/ocfs2/dir.c
@@ -1184,8 +1184,7 @@ static int __ocfs2_delete_entry(handle_t *handle, struct inode *dir,
if (pde)
le16_add_cpu(&pde->rec_len,
le16_to_cpu(de->rec_len));
- else
- de->inode = 0;
+ de->inode = 0;
dir->i_version++;
ocfs2_journal_dirty(handle, bh);
goto bail;
--
1.7.5.2
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [Ocfs2-devel] [PATCH] ocfs2: make direntry invalid when deleting it
2011-07-12 8:43 [Ocfs2-devel] [PATCH] ocfs2: make direntry invalid when deleting it Wengang Wang
@ 2011-07-12 15:31 ` Sunil Mushran
2011-07-13 2:04 ` Tao Ma
2011-08-22 4:29 ` Joel Becker
2 siblings, 0 replies; 5+ messages in thread
From: Sunil Mushran @ 2011-07-12 15:31 UTC (permalink / raw)
To: ocfs2-devel
Acked-by: Sunil Mushran <sunil.mushran@oracle.com>
On 07/12/2011 01:43 AM, Wengang Wang wrote:
> When we deleting a direntry from a directory, if it's the first in a block we
> invalid it by setting inode to 0; otherwise, we merge the deleted one to the
> prior and contiguous direntry. And we don't truncate directories.
>
> There is a problem for the later case since inode is not set to 0.
> This problem happens when the caller passes a file position as parameter to
> ocfs2_dir_foreach_blk(). If the position happens to point to a stale(not
> the first, deleted in betweens of ocfs2_dir_foreach_blk()s) direntry, we are
> not able to recognize its staleness. So that we treat it as a live one wrongly.
>
> The fix is to set inode to 0 in both cases indicating the direntry is stale.
> This won't introduce additional IOs.
>
> Signed-off-by: Wengang Wang<wen.gang.wang@oracle.com>
> ---
> fs/ocfs2/dir.c | 3 +--
> 1 files changed, 1 insertions(+), 2 deletions(-)
>
> diff --git a/fs/ocfs2/dir.c b/fs/ocfs2/dir.c
> index 8582e3f..3302088 100644
> --- a/fs/ocfs2/dir.c
> +++ b/fs/ocfs2/dir.c
> @@ -1184,8 +1184,7 @@ static int __ocfs2_delete_entry(handle_t *handle, struct inode *dir,
> if (pde)
> le16_add_cpu(&pde->rec_len,
> le16_to_cpu(de->rec_len));
> - else
> - de->inode = 0;
> + de->inode = 0;
> dir->i_version++;
> ocfs2_journal_dirty(handle, bh);
> goto bail;
^ permalink raw reply [flat|nested] 5+ messages in thread
* [Ocfs2-devel] [PATCH] ocfs2: make direntry invalid when deleting it
2011-07-12 8:43 [Ocfs2-devel] [PATCH] ocfs2: make direntry invalid when deleting it Wengang Wang
2011-07-12 15:31 ` Sunil Mushran
@ 2011-07-13 2:04 ` Tao Ma
2011-07-13 2:52 ` Wengang Wang
2011-08-22 4:29 ` Joel Becker
2 siblings, 1 reply; 5+ messages in thread
From: Tao Ma @ 2011-07-13 2:04 UTC (permalink / raw)
To: ocfs2-devel
Hi wengang,
On 07/12/2011 04:43 PM, Wengang Wang wrote:
> When we deleting a direntry from a directory, if it's the first in a block we
> invalid it by setting inode to 0; otherwise, we merge the deleted one to the
> prior and contiguous direntry. And we don't truncate directories.
>
> There is a problem for the later case since inode is not set to 0.
> This problem happens when the caller passes a file position as parameter to
> ocfs2_dir_foreach_blk(). If the position happens to point to a stale(not
> the first, deleted in betweens of ocfs2_dir_foreach_blk()s) direntry, we are
> not able to recognize its staleness. So that we treat it as a live one wrongly.
looks fine to me.
But do you have any test cases that expose this bug or is it just
inspired by code review?
Thnaks
Tao
>
> The fix is to set inode to 0 in both cases indicating the direntry is stale.
> This won't introduce additional IOs.
>
> Signed-off-by: Wengang Wang <wen.gang.wang@oracle.com>
> ---
> fs/ocfs2/dir.c | 3 +--
> 1 files changed, 1 insertions(+), 2 deletions(-)
>
> diff --git a/fs/ocfs2/dir.c b/fs/ocfs2/dir.c
> index 8582e3f..3302088 100644
> --- a/fs/ocfs2/dir.c
> +++ b/fs/ocfs2/dir.c
> @@ -1184,8 +1184,7 @@ static int __ocfs2_delete_entry(handle_t *handle, struct inode *dir,
> if (pde)
> le16_add_cpu(&pde->rec_len,
> le16_to_cpu(de->rec_len));
> - else
> - de->inode = 0;
> + de->inode = 0;
> dir->i_version++;
> ocfs2_journal_dirty(handle, bh);
> goto bail;
^ permalink raw reply [flat|nested] 5+ messages in thread
* [Ocfs2-devel] [PATCH] ocfs2: make direntry invalid when deleting it
2011-07-13 2:04 ` Tao Ma
@ 2011-07-13 2:52 ` Wengang Wang
0 siblings, 0 replies; 5+ messages in thread
From: Wengang Wang @ 2011-07-13 2:52 UTC (permalink / raw)
To: ocfs2-devel
On 11-07-13 10:04, Tao Ma wrote:
> Hi wengang,
> On 07/12/2011 04:43 PM, Wengang Wang wrote:
> > When we deleting a direntry from a directory, if it's the first in a block we
> > invalid it by setting inode to 0; otherwise, we merge the deleted one to the
> > prior and contiguous direntry. And we don't truncate directories.
> >
> > There is a problem for the later case since inode is not set to 0.
> > This problem happens when the caller passes a file position as parameter to
> > ocfs2_dir_foreach_blk(). If the position happens to point to a stale(not
> > the first, deleted in betweens of ocfs2_dir_foreach_blk()s) direntry, we are
> > not able to recognize its staleness. So that we treat it as a live one wrongly.
> looks fine to me.
> But do you have any test cases that expose this bug or is it just
> inspired by code review?
Hi Tao,
Actually I hit the problem while making patch for orphan scan(thougth not proved
to be this one exactly). It is the result of our(Srini and me) analysis.
I am going to make some test on this today.
For readdir path, I think it's not hard to reproduce :)
thanks,
wengang.
>
> Thnaks
> Tao
> >
> > The fix is to set inode to 0 in both cases indicating the direntry is stale.
> > This won't introduce additional IOs.
> >
> > Signed-off-by: Wengang Wang <wen.gang.wang@oracle.com>
> > ---
> > fs/ocfs2/dir.c | 3 +--
> > 1 files changed, 1 insertions(+), 2 deletions(-)
> >
> > diff --git a/fs/ocfs2/dir.c b/fs/ocfs2/dir.c
> > index 8582e3f..3302088 100644
> > --- a/fs/ocfs2/dir.c
> > +++ b/fs/ocfs2/dir.c
> > @@ -1184,8 +1184,7 @@ static int __ocfs2_delete_entry(handle_t *handle, struct inode *dir,
> > if (pde)
> > le16_add_cpu(&pde->rec_len,
> > le16_to_cpu(de->rec_len));
> > - else
> > - de->inode = 0;
> > + de->inode = 0;
> > dir->i_version++;
> > ocfs2_journal_dirty(handle, bh);
> > goto bail;
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* [Ocfs2-devel] [PATCH] ocfs2: make direntry invalid when deleting it
2011-07-12 8:43 [Ocfs2-devel] [PATCH] ocfs2: make direntry invalid when deleting it Wengang Wang
2011-07-12 15:31 ` Sunil Mushran
2011-07-13 2:04 ` Tao Ma
@ 2011-08-22 4:29 ` Joel Becker
2 siblings, 0 replies; 5+ messages in thread
From: Joel Becker @ 2011-08-22 4:29 UTC (permalink / raw)
To: ocfs2-devel
This patch is now in the fixes branch of ocfs2.git.
Joel
On Tue, Jul 12, 2011 at 04:43:14PM +0800, Wengang Wang wrote:
> When we deleting a direntry from a directory, if it's the first in a block we
> invalid it by setting inode to 0; otherwise, we merge the deleted one to the
> prior and contiguous direntry. And we don't truncate directories.
>
> There is a problem for the later case since inode is not set to 0.
> This problem happens when the caller passes a file position as parameter to
> ocfs2_dir_foreach_blk(). If the position happens to point to a stale(not
> the first, deleted in betweens of ocfs2_dir_foreach_blk()s) direntry, we are
> not able to recognize its staleness. So that we treat it as a live one wrongly.
>
> The fix is to set inode to 0 in both cases indicating the direntry is stale.
> This won't introduce additional IOs.
>
> Signed-off-by: Wengang Wang <wen.gang.wang@oracle.com>
> ---
> fs/ocfs2/dir.c | 3 +--
> 1 files changed, 1 insertions(+), 2 deletions(-)
>
> diff --git a/fs/ocfs2/dir.c b/fs/ocfs2/dir.c
> index 8582e3f..3302088 100644
> --- a/fs/ocfs2/dir.c
> +++ b/fs/ocfs2/dir.c
> @@ -1184,8 +1184,7 @@ static int __ocfs2_delete_entry(handle_t *handle, struct inode *dir,
> if (pde)
> le16_add_cpu(&pde->rec_len,
> le16_to_cpu(de->rec_len));
> - else
> - de->inode = 0;
> + de->inode = 0;
> dir->i_version++;
> ocfs2_journal_dirty(handle, bh);
> goto bail;
> --
> 1.7.5.2
>
>
> _______________________________________________
> Ocfs2-devel mailing list
> Ocfs2-devel at oss.oracle.com
> http://oss.oracle.com/mailman/listinfo/ocfs2-devel
--
"What no boss of a programmer can ever understand is that a programmer
is working when he's staring out of the window"
- With apologies to Burton Rascoe
http://www.jlbec.org/
jlbec at evilplan.org
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2011-08-22 4:29 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-07-12 8:43 [Ocfs2-devel] [PATCH] ocfs2: make direntry invalid when deleting it Wengang Wang
2011-07-12 15:31 ` Sunil Mushran
2011-07-13 2:04 ` Tao Ma
2011-07-13 2:52 ` Wengang Wang
2011-08-22 4:29 ` Joel Becker
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.