Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753899AbbHGOtS (ORCPT ); Fri, 7 Aug 2015 10:49:18 -0400 Received: from mail-pa0-f51.google.com ([209.85.220.51]:34931 "EHLO mail-pa0-f51.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753615AbbHGOtQ (ORCPT ); Fri, 7 Aug 2015 10:49:16 -0400 Date: Fri, 7 Aug 2015 23:49:04 +0900 From: Minchan Kim To: Joonsoo Kim Cc: Andrew Morton , Nitin Gupta , Sergey Senozhatsky , linux-kernel@vger.kernel.org, Joonsoo Kim Subject: Re: [PATCH] zram: fix possible race when checking idle_strm Message-ID: <20150807144904.GA32614@blaptop> References: <1438934609-16924-1-git-send-email-iamjoonsoo.kim@lge.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1438934609-16924-1-git-send-email-iamjoonsoo.kim@lge.com> User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2657 Lines: 81 Hi Joonsoo, On Fri, Aug 07, 2015 at 05:03:29PM +0900, Joonsoo Kim wrote: > Currently, when we enter the wait state due to lack of idle stream, > we check idle_strm list without holding the lock in expanding of > wait_event define. In this case, some one can see stale value and > process could fall into wait state without any upcoming wakeup process. > Although I didn't see any error related to this race, it should be fixed. Long time ago, I wondered about lost wake-up problem and found a article. http://www.linuxjournal.com/article/8144 >From then, I have thought such issue shouldn't happen if something is wrong since then and I believe it's same issue. Could you point out exact code sequence about the problem you mentioned? Thanks. > > To fix it, we should check idle_strm with holding the lock and > this patch implements it. > > Signed-off-by: Joonsoo Kim > --- > drivers/block/zram/zcomp.c | 15 +++++++++++++-- > 1 file changed, 13 insertions(+), 2 deletions(-) > > diff --git a/drivers/block/zram/zcomp.c b/drivers/block/zram/zcomp.c > index 80a62e7..837e9c3 100644 > --- a/drivers/block/zram/zcomp.c > +++ b/drivers/block/zram/zcomp.c > @@ -76,6 +76,17 @@ static struct zcomp_strm *zcomp_strm_alloc(struct zcomp *comp) > return zstrm; > } > > +static bool avail_idle_strm(struct zcomp_strm_multi *zs) > +{ > + int avail; > + > + spin_lock(&zs->strm_lock); > + avail = !list_empty(&zs->idle_strm); > + spin_unlock(&zs->strm_lock); > + > + return avail; > +} > + > /* > * get idle zcomp_strm or wait until other process release > * (zcomp_strm_release()) one for us > @@ -97,7 +108,7 @@ static struct zcomp_strm *zcomp_strm_multi_find(struct zcomp *comp) > /* zstrm streams limit reached, wait for idle stream */ > if (zs->avail_strm >= zs->max_strm) { > spin_unlock(&zs->strm_lock); > - wait_event(zs->strm_wait, !list_empty(&zs->idle_strm)); > + wait_event(zs->strm_wait, avail_idle_strm(zs)); > continue; > } > /* allocate new zstrm stream */ > @@ -109,7 +120,7 @@ static struct zcomp_strm *zcomp_strm_multi_find(struct zcomp *comp) > spin_lock(&zs->strm_lock); > zs->avail_strm--; > spin_unlock(&zs->strm_lock); > - wait_event(zs->strm_wait, !list_empty(&zs->idle_strm)); > + wait_event(zs->strm_wait, avail_idle_strm(zs)); > continue; > } > break; > -- > 1.9.1 > -- Kind regards, Minchan Kim -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/