Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751773AbbHGJ5n (ORCPT ); Fri, 7 Aug 2015 05:57:43 -0400 Received: from mail-pd0-f181.google.com ([209.85.192.181]:34443 "EHLO mail-pd0-f181.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750889AbbHGJ5l (ORCPT ); Fri, 7 Aug 2015 05:57:41 -0400 Date: Fri, 7 Aug 2015 18:58:16 +0900 From: Sergey Senozhatsky To: Sergey Senozhatsky Cc: Joonsoo Kim , Andrew Morton , Minchan Kim , Nitin Gupta , linux-kernel@vger.kernel.org, Joonsoo Kim Subject: Re: [PATCH] zram: fix possible race when checking idle_strm Message-ID: <20150807095816.GP1891@swordfish> References: <1438934609-16924-1-git-send-email-iamjoonsoo.kim@lge.com> <20150807091457.GL1891@swordfish> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20150807091457.GL1891@swordfish> User-Agent: Mutt/1.5.23+102 (2ca89bed6448) (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1906 Lines: 56 On (08/07/15 18:14), Sergey Senozhatsky wrote: > hm... I need to think about it more. > > we do wake_up every time we put stream back to the list > > zcomp_strm_multi_release(): > > spin_lock(&zs->strm_lock); > if (zs->avail_strm <= zs->max_strm) { > list_add(&zstrm->list, &zs->idle_strm); > spin_unlock(&zs->strm_lock); > wake_up(&zs->strm_wait); > return; > } > > > but I can probably see what you mean... in some very extreme case, > though. I can't even formulate it... eh... we use a multi stream > backend with ->max_strm == 1 and there are two processes, one > just falsely passed the wait_event() `if (condition)' check, the > other one just put stream back to ->idle_strm and called wake_up(), > but the first process hasn't yet executed prepare_to_wait_event() > so it might miss a wakeup. and there should be no other process > doing read or write operation. otherwise, there will be wakeup > eventually. > > is this the case you were thinking of?... then yes, this spinlock > may help. > on the other hand... it's actually wait_event() is if (condition) break; prepare_to_wait_event(&wq, &__wait, state) if (condition) break; schedule(); if first condition check was false and we missed a wakeup call between first condition and prepare_to_wait_event(), then second condition check should do the trick I think (or you expect that second condition check may be wrongly pre-fetched or something). if wakeup arrives after prepare_to_wait_event(), then we are fine by defintion. so, I'm puzzled a bit. do we have a problem or we are ok. -ss -- 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/