* [Ocfs2-devel] [PATCH 1/1] OCFS2: unhash all dentries on a inode.
@ 2008-09-22 9:48 wangang wang
2008-09-22 21:46 ` Sunil Mushran
0 siblings, 1 reply; 5+ messages in thread
From: wangang wang @ 2008-09-22 9:48 UTC (permalink / raw)
To: ocfs2-devel
In ocfs2_process_delete_request(), we should unhash all dentries on the inode.
--not only the ones with 0 referrence count. so that it's possible for dput()
to drop the stale inode.
the patch is against 1.2 svn.
Signed-off-by: Wengang wang <wen.gang.wang@oracle.com>
--
Index: fs/ocfs2/vote.c
===================================================================
--- fs/ocfs2/vote.c (revision 3101)
+++ fs/ocfs2/vote.c (working copy)
@@ -176,6 +176,7 @@
int deleting_node)
{
int response = OCFS2_RESPONSE_BUSY;
+ struct list_head *tmp, *head;
mlog(0, "DELETE vote on inode %lu, read lnk_cnt = %u, slot = %d\n",
inode->i_ino, inode->i_nlink, *orphaned_slot);
@@ -253,9 +254,19 @@
ocfs2_mark_inode_remotely_deleted(inode, deleting_node);
spin_unlock(&OCFS2_I(inode)->ip_lock);
- /* Not sure this is necessary anymore. */
- d_prune_aliases(inode);
+ /* unhash all dentries on this inode */
+ spin_lock(&dcache_lock);
+ head = &inode->i_dentry;
+ tmp = head;
+ while ((tmp = tmp->next) != head) {
+ struct dentry *dentry = list_entry(tmp, struct dentry, d_alias);
+ spin_lock(&dentry->d_lock);
+ __d_drop(dentry);
+ spin_unlock(&dentry->d_lock);
+ }
+ spin_unlock(&dcache_lock);
+
/* If we get here, then we're voting 'yes', so commit the
* delete on our side. */
response = OCFS2_RESPONSE_OK;
^ permalink raw reply [flat|nested] 5+ messages in thread* [Ocfs2-devel] [PATCH 1/1] OCFS2: unhash all dentries on a inode.
2008-09-22 9:48 [Ocfs2-devel] [PATCH 1/1] OCFS2: unhash all dentries on a inode wangang wang
@ 2008-09-22 21:46 ` Sunil Mushran
2008-09-23 1:51 ` wengang wang
0 siblings, 1 reply; 5+ messages in thread
From: Sunil Mushran @ 2008-09-22 21:46 UTC (permalink / raw)
To: ocfs2-devel
NAK.
I fail to understand your logic. The code you've added is pretty
much what d_prune_aliases() does (the call you have deleted).
wangang wang wrote:
> In ocfs2_process_delete_request(), we should unhash all dentries on the inode.
> --not only the ones with 0 referrence count. so that it's possible for dput()
> to drop the stale inode.
>
> the patch is against 1.2 svn.
>
> Signed-off-by: Wengang wang <wen.gang.wang@oracle.com>
> --
>
> Index: fs/ocfs2/vote.c
> ===================================================================
> --- fs/ocfs2/vote.c (revision 3101)
> +++ fs/ocfs2/vote.c (working copy)
> @@ -176,6 +176,7 @@
> int deleting_node)
> {
> int response = OCFS2_RESPONSE_BUSY;
> + struct list_head *tmp, *head;
>
> mlog(0, "DELETE vote on inode %lu, read lnk_cnt = %u, slot = %d\n",
> inode->i_ino, inode->i_nlink, *orphaned_slot);
> @@ -253,9 +254,19 @@
> ocfs2_mark_inode_remotely_deleted(inode, deleting_node);
> spin_unlock(&OCFS2_I(inode)->ip_lock);
>
> - /* Not sure this is necessary anymore. */
> - d_prune_aliases(inode);
> + /* unhash all dentries on this inode */
> + spin_lock(&dcache_lock);
> + head = &inode->i_dentry;
> + tmp = head;
>
> + while ((tmp = tmp->next) != head) {
> + struct dentry *dentry = list_entry(tmp, struct dentry, d_alias);
> + spin_lock(&dentry->d_lock);
> + __d_drop(dentry);
> + spin_unlock(&dentry->d_lock);
> + }
> + spin_unlock(&dcache_lock);
> +
> /* If we get here, then we're voting 'yes', so commit the
> * delete on our side. */
> response = OCFS2_RESPONSE_OK;
>
> _______________________________________________
> Ocfs2-devel mailing list
> Ocfs2-devel at oss.oracle.com
> http://oss.oracle.com/mailman/listinfo/ocfs2-devel
>
^ permalink raw reply [flat|nested] 5+ messages in thread* [Ocfs2-devel] [PATCH 1/1] OCFS2: unhash all dentries on a inode.
2008-09-22 21:46 ` Sunil Mushran
@ 2008-09-23 1:51 ` wengang wang
2008-09-23 4:47 ` Sunil Mushran
0 siblings, 1 reply; 5+ messages in thread
From: wengang wang @ 2008-09-23 1:51 UTC (permalink / raw)
To: ocfs2-devel
Sunil,
d_prune_aliases() unhashes dentries that has 0 reference count, dentries
with non-zero ref count won't be unhashed.
My code unhashes all no matter there reference counts are 0 or not.
regards,
wengang.
Sunil Mushran wrote:
> NAK.
>
> I fail to understand your logic. The code you've added is pretty
> much what d_prune_aliases() does (the call you have deleted).
>
> wangang wang wrote:
>> In ocfs2_process_delete_request(), we should unhash all dentries on
>> the inode.
>> --not only the ones with 0 referrence count. so that it's possible
>> for dput()
>> to drop the stale inode.
>>
>> the patch is against 1.2 svn.
>>
>> Signed-off-by: Wengang wang <wen.gang.wang@oracle.com>
>> --
>>
>> Index: fs/ocfs2/vote.c
>> ===================================================================
>> --- fs/ocfs2/vote.c (revision 3101)
>> +++ fs/ocfs2/vote.c (working copy)
>> @@ -176,6 +176,7 @@
>> int deleting_node)
>> {
>> int response = OCFS2_RESPONSE_BUSY;
>> + struct list_head *tmp, *head;
>>
>> mlog(0, "DELETE vote on inode %lu, read lnk_cnt = %u, slot = %d\n",
>> inode->i_ino, inode->i_nlink, *orphaned_slot);
>> @@ -253,9 +254,19 @@
>> ocfs2_mark_inode_remotely_deleted(inode, deleting_node);
>> spin_unlock(&OCFS2_I(inode)->ip_lock);
>>
>> - /* Not sure this is necessary anymore. */
>> - d_prune_aliases(inode);
>> + /* unhash all dentries on this inode */
>> + spin_lock(&dcache_lock);
>> + head = &inode->i_dentry;
>> + tmp = head;
>>
>> + while ((tmp = tmp->next) != head) {
>> + struct dentry *dentry = list_entry(tmp, struct dentry,
>> d_alias);
>> + spin_lock(&dentry->d_lock);
>> + __d_drop(dentry);
>> + spin_unlock(&dentry->d_lock);
>> + }
>> + spin_unlock(&dcache_lock);
>> +
>> /* If we get here, then we're voting 'yes', so commit the
>> * delete on our side. */
>> response = OCFS2_RESPONSE_OK;
>>
>> _______________________________________________
>> Ocfs2-devel mailing list
>> Ocfs2-devel at oss.oracle.com
>> http://oss.oracle.com/mailman/listinfo/ocfs2-devel
>>
>
--
Wengang Wang
Member of Technical Staff
Oracle Asia R&D Center
Open Source Technologies Development
Tel: +86 10 8278 6265
Mobile: +86 13381078925
^ permalink raw reply [flat|nested] 5+ messages in thread* [Ocfs2-devel] [PATCH 1/1] OCFS2: unhash all dentries on a inode.
2008-09-23 1:51 ` wengang wang
@ 2008-09-23 4:47 ` Sunil Mushran
2008-09-23 5:27 ` wengang wang
0 siblings, 1 reply; 5+ messages in thread
From: Sunil Mushran @ 2008-09-23 4:47 UTC (permalink / raw)
To: ocfs2-devel
If so, won't that be a bug?
wengang wang wrote:
> Sunil,
>
> d_prune_aliases() unhashes dentries that has 0 reference count,
> dentries with non-zero ref count won't be unhashed.
> My code unhashes all no matter there reference counts are 0 or not.
>
> regards,
> wengang.
>
> Sunil Mushran wrote:
>> NAK.
>>
>> I fail to understand your logic. The code you've added is pretty
>> much what d_prune_aliases() does (the call you have deleted).
>>
>> wangang wang wrote:
>>> In ocfs2_process_delete_request(), we should unhash all dentries on
>>> the inode.
>>> --not only the ones with 0 referrence count. so that it's possible
>>> for dput()
>>> to drop the stale inode.
>>>
>>> the patch is against 1.2 svn.
>>>
>>> Signed-off-by: Wengang wang <wen.gang.wang@oracle.com>
>>> --
>>>
>>> Index: fs/ocfs2/vote.c
>>> ===================================================================
>>> --- fs/ocfs2/vote.c (revision 3101)
>>> +++ fs/ocfs2/vote.c (working copy)
>>> @@ -176,6 +176,7 @@
>>> int deleting_node)
>>> {
>>> int response = OCFS2_RESPONSE_BUSY;
>>> + struct list_head *tmp, *head;
>>>
>>> mlog(0, "DELETE vote on inode %lu, read lnk_cnt = %u, slot =
>>> %d\n",
>>> inode->i_ino, inode->i_nlink, *orphaned_slot);
>>> @@ -253,9 +254,19 @@
>>> ocfs2_mark_inode_remotely_deleted(inode, deleting_node);
>>> spin_unlock(&OCFS2_I(inode)->ip_lock);
>>>
>>> - /* Not sure this is necessary anymore. */
>>> - d_prune_aliases(inode);
>>> + /* unhash all dentries on this inode */
>>> + spin_lock(&dcache_lock);
>>> + head = &inode->i_dentry;
>>> + tmp = head;
>>>
>>> + while ((tmp = tmp->next) != head) {
>>> + struct dentry *dentry = list_entry(tmp, struct dentry,
>>> d_alias);
>>> + spin_lock(&dentry->d_lock);
>>> + __d_drop(dentry);
>>> + spin_unlock(&dentry->d_lock);
>>> + }
>>> + spin_unlock(&dcache_lock);
>>> +
>>> /* If we get here, then we're voting 'yes', so commit the
>>> * delete on our side. */
>>> response = OCFS2_RESPONSE_OK;
>>>
>>> _______________________________________________
>>> Ocfs2-devel mailing list
>>> Ocfs2-devel at oss.oracle.com
>>> http://oss.oracle.com/mailman/listinfo/ocfs2-devel
>>>
>>
>
^ permalink raw reply [flat|nested] 5+ messages in thread* [Ocfs2-devel] [PATCH 1/1] OCFS2: unhash all dentries on a inode.
2008-09-23 4:47 ` Sunil Mushran
@ 2008-09-23 5:27 ` wengang wang
0 siblings, 0 replies; 5+ messages in thread
From: wengang wang @ 2008-09-23 5:27 UTC (permalink / raw)
To: ocfs2-devel
Sunil Mushran wrote:
> If so, won't that be a bug?
>
If you meant my patch is bug, why?
unhashing dentry from dentry cache only leads to a mis-match in that
cache when a new request comes.
when the dentry is not found in cache, before creating a new dentry, a
validation check should be done on the inode in inode cache or by
ocfs2_lookup().
so I think there is no problem.
without unhashing the dentries, dput() won't call dentry_iput(). thus
the stale inode will be kept in memory for ever. --this is a problem.
thanks,
wengang.
> wengang wang wrote:
>> Sunil,
>>
>> d_prune_aliases() unhashes dentries that has 0 reference count,
>> dentries with non-zero ref count won't be unhashed.
>> My code unhashes all no matter there reference counts are 0 or not.
>>
>> regards,
>> wengang.
>>
>> Sunil Mushran wrote:
>>> NAK.
>>>
>>> I fail to understand your logic. The code you've added is pretty
>>> much what d_prune_aliases() does (the call you have deleted).
>>>
>>> wangang wang wrote:
>>>> In ocfs2_process_delete_request(), we should unhash all dentries on
>>>> the inode.
>>>> --not only the ones with 0 referrence count. so that it's possible
>>>> for dput()
>>>> to drop the stale inode.
>>>>
>>>> the patch is against 1.2 svn.
>>>>
>>>> Signed-off-by: Wengang wang <wen.gang.wang@oracle.com>
>>>> --
>>>>
>>>> Index: fs/ocfs2/vote.c
>>>> ===================================================================
>>>> --- fs/ocfs2/vote.c (revision 3101)
>>>> +++ fs/ocfs2/vote.c (working copy)
>>>> @@ -176,6 +176,7 @@
>>>> int deleting_node)
>>>> {
>>>> int response = OCFS2_RESPONSE_BUSY;
>>>> + struct list_head *tmp, *head;
>>>>
>>>> mlog(0, "DELETE vote on inode %lu, read lnk_cnt = %u, slot =
>>>> %d\n",
>>>> inode->i_ino, inode->i_nlink, *orphaned_slot);
>>>> @@ -253,9 +254,19 @@
>>>> ocfs2_mark_inode_remotely_deleted(inode, deleting_node);
>>>> spin_unlock(&OCFS2_I(inode)->ip_lock);
>>>>
>>>> - /* Not sure this is necessary anymore. */
>>>> - d_prune_aliases(inode);
>>>> + /* unhash all dentries on this inode */
>>>> + spin_lock(&dcache_lock);
>>>> + head = &inode->i_dentry;
>>>> + tmp = head;
>>>>
>>>> + while ((tmp = tmp->next) != head) {
>>>> + struct dentry *dentry = list_entry(tmp, struct dentry,
>>>> d_alias);
>>>> + spin_lock(&dentry->d_lock);
>>>> + __d_drop(dentry);
>>>> + spin_unlock(&dentry->d_lock);
>>>> + }
>>>> + spin_unlock(&dcache_lock);
>>>> +
>>>> /* If we get here, then we're voting 'yes', so commit the
>>>> * delete on our side. */
>>>> response = OCFS2_RESPONSE_OK;
>>>>
>>>> _______________________________________________
>>>> Ocfs2-devel mailing list
>>>> Ocfs2-devel at oss.oracle.com
>>>> http://oss.oracle.com/mailman/listinfo/ocfs2-devel
>>>>
>>>
>>
>
--
Wengang Wang
Member of Technical Staff
Oracle Asia R&D Center
Open Source Technologies Development
Tel: +86 10 8278 6265
Mobile: +86 13381078925
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2008-09-23 5:27 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-09-22 9:48 [Ocfs2-devel] [PATCH 1/1] OCFS2: unhash all dentries on a inode wangang wang
2008-09-22 21:46 ` Sunil Mushran
2008-09-23 1:51 ` wengang wang
2008-09-23 4:47 ` Sunil Mushran
2008-09-23 5:27 ` wengang wang
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.