* [Ocfs2-devel] [PATCH] ocfs2/dlm: remove potential deadlock -V3
@ 2010-07-30 13:53 Wengang Wang
2010-07-30 14:45 ` Sunil Mushran
0 siblings, 1 reply; 6+ messages in thread
From: Wengang Wang @ 2010-07-30 13:53 UTC (permalink / raw)
To: ocfs2-devel
When we need to take both dlm_domain_lock and dlm->spinlock, we should take
them in order of: dlm_domain_lock then dlm->spinlock.
There is pathes disobey this order. That is calling dlm_lockres_put() with
dlm->spinlock held in dlm_run_purge_list. dlm_lockres_put() calls dlm_put() at
the ref and dlm_put() locks on dlm_domain_lock.
Fix:
Don't grab/put the dlm when the initialising/releasing lockres.
That grab is not required because we don't call dlm_unregister_domain()
based on refcount.
Signed-off-by: Wengang Wang <wen.gang.wang@oracle.com>
---
fs/ocfs2/dlm/dlmmaster.c | 3 ---
1 files changed, 0 insertions(+), 3 deletions(-)
diff --git a/fs/ocfs2/dlm/dlmmaster.c b/fs/ocfs2/dlm/dlmmaster.c
index 94b97fc..8408465 100644
--- a/fs/ocfs2/dlm/dlmmaster.c
+++ b/fs/ocfs2/dlm/dlmmaster.c
@@ -511,8 +511,6 @@ static void dlm_lockres_release(struct kref *kref)
atomic_dec(&dlm->res_cur_count);
- dlm_put(dlm);
-
if (!hlist_unhashed(&res->hash_node) ||
!list_empty(&res->granted) ||
!list_empty(&res->converting) ||
@@ -586,7 +584,6 @@ static void dlm_init_lockres(struct dlm_ctxt *dlm,
res->inflight_locks = 0;
/* put in dlm_lockres_release */
- dlm_grab(dlm);
res->dlm = dlm;
kref_init(&res->refs);
--
1.7.1.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [Ocfs2-devel] [PATCH] ocfs2/dlm: remove potential deadlock -V3
2010-07-30 13:53 [Ocfs2-devel] [PATCH] ocfs2/dlm: remove potential deadlock -V3 Wengang Wang
@ 2010-07-30 14:45 ` Sunil Mushran
2010-07-30 15:08 ` Wengang Wang
2010-07-30 15:18 ` Wengang Wang
0 siblings, 2 replies; 6+ messages in thread
From: Sunil Mushran @ 2010-07-30 14:45 UTC (permalink / raw)
To: ocfs2-devel
You can also remove the comment that talks about the put.
On Jul 30, 2010, at 6:53 AM, Wengang Wang <wen.gang.wang@oracle.com> wrote:
> When we need to take both dlm_domain_lock and dlm->spinlock, we should take
> them in order of: dlm_domain_lock then dlm->spinlock.
>
> There is pathes disobey this order. That is calling dlm_lockres_put() with
> dlm->spinlock held in dlm_run_purge_list. dlm_lockres_put() calls dlm_put() at
> the ref and dlm_put() locks on dlm_domain_lock.
>
> Fix:
> Don't grab/put the dlm when the initialising/releasing lockres.
> That grab is not required because we don't call dlm_unregister_domain()
> based on refcount.
>
> Signed-off-by: Wengang Wang <wen.gang.wang@oracle.com>
> ---
> fs/ocfs2/dlm/dlmmaster.c | 3 ---
> 1 files changed, 0 insertions(+), 3 deletions(-)
>
> diff --git a/fs/ocfs2/dlm/dlmmaster.c b/fs/ocfs2/dlm/dlmmaster.c
> index 94b97fc..8408465 100644
> --- a/fs/ocfs2/dlm/dlmmaster.c
> +++ b/fs/ocfs2/dlm/dlmmaster.c
> @@ -511,8 +511,6 @@ static void dlm_lockres_release(struct kref *kref)
>
> atomic_dec(&dlm->res_cur_count);
>
> - dlm_put(dlm);
> -
> if (!hlist_unhashed(&res->hash_node) ||
> !list_empty(&res->granted) ||
> !list_empty(&res->converting) ||
> @@ -586,7 +584,6 @@ static void dlm_init_lockres(struct dlm_ctxt *dlm,
> res->inflight_locks = 0;
>
> /* put in dlm_lockres_release */
> - dlm_grab(dlm);
> res->dlm = dlm;
>
> kref_init(&res->refs);
> --
> 1.7.1.1
>
>
> _______________________________________________
> Ocfs2-devel mailing list
> Ocfs2-devel at oss.oracle.com
> http://oss.oracle.com/mailman/listinfo/ocfs2-devel
^ permalink raw reply [flat|nested] 6+ messages in thread
* [Ocfs2-devel] [PATCH] ocfs2/dlm: remove potential deadlock -V3
2010-07-30 14:45 ` Sunil Mushran
@ 2010-07-30 15:08 ` Wengang Wang
2010-07-30 15:18 ` Wengang Wang
1 sibling, 0 replies; 6+ messages in thread
From: Wengang Wang @ 2010-07-30 15:08 UTC (permalink / raw)
To: ocfs2-devel
On 10-07-30 07:45, Sunil Mushran wrote:
> You can also remove the comment that talks about the put.
Yes, I should.
regards,
wengang.
>
> On Jul 30, 2010, at 6:53 AM, Wengang Wang <wen.gang.wang@oracle.com> wrote:
>
> > When we need to take both dlm_domain_lock and dlm->spinlock, we should take
> > them in order of: dlm_domain_lock then dlm->spinlock.
> >
> > There is pathes disobey this order. That is calling dlm_lockres_put() with
> > dlm->spinlock held in dlm_run_purge_list. dlm_lockres_put() calls dlm_put() at
> > the ref and dlm_put() locks on dlm_domain_lock.
> >
> > Fix:
> > Don't grab/put the dlm when the initialising/releasing lockres.
> > That grab is not required because we don't call dlm_unregister_domain()
> > based on refcount.
> >
> > Signed-off-by: Wengang Wang <wen.gang.wang@oracle.com>
> > ---
> > fs/ocfs2/dlm/dlmmaster.c | 3 ---
> > 1 files changed, 0 insertions(+), 3 deletions(-)
> >
> > diff --git a/fs/ocfs2/dlm/dlmmaster.c b/fs/ocfs2/dlm/dlmmaster.c
> > index 94b97fc..8408465 100644
> > --- a/fs/ocfs2/dlm/dlmmaster.c
> > +++ b/fs/ocfs2/dlm/dlmmaster.c
> > @@ -511,8 +511,6 @@ static void dlm_lockres_release(struct kref *kref)
> >
> > atomic_dec(&dlm->res_cur_count);
> >
> > - dlm_put(dlm);
> > -
> > if (!hlist_unhashed(&res->hash_node) ||
> > !list_empty(&res->granted) ||
> > !list_empty(&res->converting) ||
> > @@ -586,7 +584,6 @@ static void dlm_init_lockres(struct dlm_ctxt *dlm,
> > res->inflight_locks = 0;
> >
> > /* put in dlm_lockres_release */
> > - dlm_grab(dlm);
> > res->dlm = dlm;
> >
> > kref_init(&res->refs);
> > --
> > 1.7.1.1
> >
> >
> > _______________________________________________
> > Ocfs2-devel mailing list
> > Ocfs2-devel at oss.oracle.com
> > http://oss.oracle.com/mailman/listinfo/ocfs2-devel
^ permalink raw reply [flat|nested] 6+ messages in thread
* [Ocfs2-devel] [PATCH] ocfs2/dlm: remove potential deadlock -V3
2010-07-30 14:45 ` Sunil Mushran
2010-07-30 15:08 ` Wengang Wang
@ 2010-07-30 15:18 ` Wengang Wang
2010-07-30 17:06 ` Sunil Mushran
2010-08-07 18:40 ` Joel Becker
1 sibling, 2 replies; 6+ messages in thread
From: Wengang Wang @ 2010-07-30 15:18 UTC (permalink / raw)
To: ocfs2-devel
When we need to take both dlm_domain_lock and dlm->spinlock, we should take
them in order of: dlm_domain_lock then dlm->spinlock.
There is pathes disobey this order. That is calling dlm_lockres_put() with
dlm->spinlock held in dlm_run_purge_list. dlm_lockres_put() calls dlm_put() at
the ref and dlm_put() locks on dlm_domain_lock.
Fix:
Don't grab/put the dlm when the initialising/releasing lockres.
That grab is not required because we don't call dlm_unregister_domain()
based on refcount.
Signed-off-by: Wengang Wang <wen.gang.wang@oracle.com>
---
fs/ocfs2/dlm/dlmmaster.c | 4 ----
1 file changed, 4 deletions(-)
diff --git a/fs/ocfs2/dlm/dlmmaster.c b/fs/ocfs2/dlm/dlmmaster.c
index 94b97fc..0fe7c4d 100644
--- a/fs/ocfs2/dlm/dlmmaster.c
+++ b/fs/ocfs2/dlm/dlmmaster.c
@@ -511,8 +511,6 @@ static void dlm_lockres_release(struct kref *kref)
atomic_dec(&dlm->res_cur_count);
- dlm_put(dlm);
-
if (!hlist_unhashed(&res->hash_node) ||
!list_empty(&res->granted) ||
!list_empty(&res->converting) ||
@@ -585,8 +583,6 @@ static void dlm_init_lockres(struct dlm_ctxt *dlm,
res->migration_pending = 0;
res->inflight_locks = 0;
- /* put in dlm_lockres_release */
- dlm_grab(dlm);
res->dlm = dlm;
kref_init(&res->refs);
--
1.7.1.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [Ocfs2-devel] [PATCH] ocfs2/dlm: remove potential deadlock -V3
2010-07-30 15:18 ` Wengang Wang
@ 2010-07-30 17:06 ` Sunil Mushran
2010-08-07 18:40 ` Joel Becker
1 sibling, 0 replies; 6+ messages in thread
From: Sunil Mushran @ 2010-07-30 17:06 UTC (permalink / raw)
To: ocfs2-devel
Acked-by: Sunil Mushran <sunil.mushran@oracle.com>
On 07/30/2010 08:18 AM, Wengang Wang wrote:
> When we need to take both dlm_domain_lock and dlm->spinlock, we should take
> them in order of: dlm_domain_lock then dlm->spinlock.
>
> There is pathes disobey this order. That is calling dlm_lockres_put() with
> dlm->spinlock held in dlm_run_purge_list. dlm_lockres_put() calls dlm_put() at
> the ref and dlm_put() locks on dlm_domain_lock.
>
> Fix:
> Don't grab/put the dlm when the initialising/releasing lockres.
> That grab is not required because we don't call dlm_unregister_domain()
> based on refcount.
>
> Signed-off-by: Wengang Wang<wen.gang.wang@oracle.com>
> ---
> fs/ocfs2/dlm/dlmmaster.c | 4 ----
> 1 file changed, 4 deletions(-)
>
> diff --git a/fs/ocfs2/dlm/dlmmaster.c b/fs/ocfs2/dlm/dlmmaster.c
> index 94b97fc..0fe7c4d 100644
> --- a/fs/ocfs2/dlm/dlmmaster.c
> +++ b/fs/ocfs2/dlm/dlmmaster.c
> @@ -511,8 +511,6 @@ static void dlm_lockres_release(struct kref *kref)
>
> atomic_dec(&dlm->res_cur_count);
>
> - dlm_put(dlm);
> -
> if (!hlist_unhashed(&res->hash_node) ||
> !list_empty(&res->granted) ||
> !list_empty(&res->converting) ||
> @@ -585,8 +583,6 @@ static void dlm_init_lockres(struct dlm_ctxt *dlm,
> res->migration_pending = 0;
> res->inflight_locks = 0;
>
> - /* put in dlm_lockres_release */
> - dlm_grab(dlm);
> res->dlm = dlm;
>
> kref_init(&res->refs);
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* [Ocfs2-devel] [PATCH] ocfs2/dlm: remove potential deadlock -V3
2010-07-30 15:18 ` Wengang Wang
2010-07-30 17:06 ` Sunil Mushran
@ 2010-08-07 18:40 ` Joel Becker
1 sibling, 0 replies; 6+ messages in thread
From: Joel Becker @ 2010-08-07 18:40 UTC (permalink / raw)
To: ocfs2-devel
On Fri, Jul 30, 2010 at 11:18:00PM +0800, Wengang Wang wrote:
> When we need to take both dlm_domain_lock and dlm->spinlock, we should take
> them in order of: dlm_domain_lock then dlm->spinlock.
>
> There is pathes disobey this order. That is calling dlm_lockres_put() with
> dlm->spinlock held in dlm_run_purge_list. dlm_lockres_put() calls dlm_put() at
> the ref and dlm_put() locks on dlm_domain_lock.
>
> Fix:
> Don't grab/put the dlm when the initialising/releasing lockres.
> That grab is not required because we don't call dlm_unregister_domain()
> based on refcount.
>
> Signed-off-by: Wengang Wang <wen.gang.wang@oracle.com>
This patch is now in the fixes branch of ocfs2.git.
Joel
--
"If the human brain were so simple we could understand it, we would
be so simple that we could not."
- W. A. Clouston
Joel Becker
Consulting Software Developer
Oracle
E-mail: joel.becker at oracle.com
Phone: (650) 506-8127
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2010-08-07 18:40 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-07-30 13:53 [Ocfs2-devel] [PATCH] ocfs2/dlm: remove potential deadlock -V3 Wengang Wang
2010-07-30 14:45 ` Sunil Mushran
2010-07-30 15:08 ` Wengang Wang
2010-07-30 15:18 ` Wengang Wang
2010-07-30 17:06 ` Sunil Mushran
2010-08-07 18:40 ` Joel Becker
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).