* [PATCH] zcache: fix deadlock condition
@ 2012-01-25 22:32 Dan Magenheimer
2012-01-27 20:39 ` Konrad Rzeszutek Wilk
0 siblings, 1 reply; 3+ messages in thread
From: Dan Magenheimer @ 2012-01-25 22:32 UTC (permalink / raw)
To: Konrad Wilk, linux-kernel, devel, Greg Kroah-Hartman
Cc: Seth Jennings, Nitin Gupta
I discovered this deadlock condition awhile ago working on RAMster
but it affects zcache as well. The list spinlock must be
locked prior to the page spinlock and released after. As
a result, the page copy must also be done while the locks are held.
Applies to 3.2. Konrad, please push (via GregKH?)...
this is definitely a bug fix so need not be pushed during
a -rc0 window.
Signed-off-by: Dan Magenheimer <dan.magenheimer@oracle.com>
diff --git a/drivers/staging/zcache/zcache-main.c b/drivers/staging/zcache/zcache-main.c
index 56c1f9c..5b9f74e 100644
--- a/drivers/staging/zcache/zcache-main.c
+++ b/drivers/staging/zcache/zcache-main.c
@@ -358,8 +358,8 @@ static struct zbud_hdr *zbud_create(uint16_t client_id, uint16_t pool_id,
if (unlikely(zbpg == NULL))
goto out;
/* ok, have a page, now compress the data before taking locks */
- spin_lock(&zbpg->lock);
spin_lock(&zbud_budlists_spinlock);
+ spin_lock(&zbpg->lock);
list_add_tail(&zbpg->bud_list, &zbud_unbuddied[nchunks].list);
zbud_unbuddied[nchunks].count++;
zh = &zbpg->buddy[0];
@@ -389,12 +389,11 @@ init_zh:
zh->oid = *oid;
zh->pool_id = pool_id;
zh->client_id = client_id;
- /* can wait to copy the data until the list locks are dropped */
- spin_unlock(&zbud_budlists_spinlock);
-
to = zbud_data(zh, size);
memcpy(to, cdata, size);
spin_unlock(&zbpg->lock);
+ spin_unlock(&zbud_budlists_spinlock);
+
zbud_cumul_chunk_counts[nchunks]++;
atomic_inc(&zcache_zbud_curr_zpages);
zcache_zbud_cumul_zpages++;
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] zcache: fix deadlock condition
2012-01-25 22:32 [PATCH] zcache: fix deadlock condition Dan Magenheimer
@ 2012-01-27 20:39 ` Konrad Rzeszutek Wilk
2012-01-27 22:18 ` Greg KH
0 siblings, 1 reply; 3+ messages in thread
From: Konrad Rzeszutek Wilk @ 2012-01-27 20:39 UTC (permalink / raw)
To: Dan Magenheimer, gregkh
Cc: linux-kernel, devel, Greg Kroah-Hartman, Seth Jennings, Nitin Gupta
On Wed, Jan 25, 2012 at 02:32:51PM -0800, Dan Magenheimer wrote:
> I discovered this deadlock condition awhile ago working on RAMster
> but it affects zcache as well. The list spinlock must be
> locked prior to the page spinlock and released after. As
> a result, the page copy must also be done while the locks are held.
>
> Applies to 3.2. Konrad, please push (via GregKH?)...
> this is definitely a bug fix so need not be pushed during
> a -rc0 window.
Looks good to me. Greg, could you apply the patch to your tree
please? [Or would you rather I sent an GIT pull?]
Thanks!
>
> Signed-off-by: Dan Magenheimer <dan.magenheimer@oracle.com>
>
> diff --git a/drivers/staging/zcache/zcache-main.c b/drivers/staging/zcache/zcache-main.c
> index 56c1f9c..5b9f74e 100644
> --- a/drivers/staging/zcache/zcache-main.c
> +++ b/drivers/staging/zcache/zcache-main.c
> @@ -358,8 +358,8 @@ static struct zbud_hdr *zbud_create(uint16_t client_id, uint16_t pool_id,
> if (unlikely(zbpg == NULL))
> goto out;
> /* ok, have a page, now compress the data before taking locks */
> - spin_lock(&zbpg->lock);
> spin_lock(&zbud_budlists_spinlock);
> + spin_lock(&zbpg->lock);
> list_add_tail(&zbpg->bud_list, &zbud_unbuddied[nchunks].list);
> zbud_unbuddied[nchunks].count++;
> zh = &zbpg->buddy[0];
> @@ -389,12 +389,11 @@ init_zh:
> zh->oid = *oid;
> zh->pool_id = pool_id;
> zh->client_id = client_id;
> - /* can wait to copy the data until the list locks are dropped */
> - spin_unlock(&zbud_budlists_spinlock);
> -
> to = zbud_data(zh, size);
> memcpy(to, cdata, size);
> spin_unlock(&zbpg->lock);
> + spin_unlock(&zbud_budlists_spinlock);
> +
> zbud_cumul_chunk_counts[nchunks]++;
> atomic_inc(&zcache_zbud_curr_zpages);
> zcache_zbud_cumul_zpages++;
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] zcache: fix deadlock condition
2012-01-27 20:39 ` Konrad Rzeszutek Wilk
@ 2012-01-27 22:18 ` Greg KH
0 siblings, 0 replies; 3+ messages in thread
From: Greg KH @ 2012-01-27 22:18 UTC (permalink / raw)
To: Konrad Rzeszutek Wilk
Cc: Dan Magenheimer, linux-kernel, devel, Seth Jennings, Nitin Gupta
On Fri, Jan 27, 2012 at 03:39:28PM -0500, Konrad Rzeszutek Wilk wrote:
> On Wed, Jan 25, 2012 at 02:32:51PM -0800, Dan Magenheimer wrote:
> > I discovered this deadlock condition awhile ago working on RAMster
> > but it affects zcache as well. The list spinlock must be
> > locked prior to the page spinlock and released after. As
> > a result, the page copy must also be done while the locks are held.
> >
> > Applies to 3.2. Konrad, please push (via GregKH?)...
> > this is definitely a bug fix so need not be pushed during
> > a -rc0 window.
>
> Looks good to me. Greg, could you apply the patch to your tree
> please? [Or would you rather I sent an GIT pull?]
I will queue it up, thanks.
greg k-h
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2012-01-27 22:25 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-01-25 22:32 [PATCH] zcache: fix deadlock condition Dan Magenheimer
2012-01-27 20:39 ` Konrad Rzeszutek Wilk
2012-01-27 22:18 ` Greg KH
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).