From mboxrd@z Thu Jan 1 00:00:00 1970 Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S965471AbeAMEGe (ORCPT + 1 other); Fri, 12 Jan 2018 23:06:34 -0500 Received: from server.coly.li ([162.144.45.48]:35592 "EHLO server.coly.li" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S965403AbeAMEGd (ORCPT ); Fri, 12 Jan 2018 23:06:33 -0500 Subject: Re: [PATCH] bcache: btree.c: Fix GC thread exit in case of cache device failure and unregister To: Pavel Vazharov , mlyle@lyle.org, kent.overstreet@gmail.com Cc: linux-bcache@vger.kernel.org, linux-kernel@vger.kernel.org References: <1515770690-18562-1-git-send-email-freakpv@gmail.com> From: Coly Li Message-ID: <8bf2eafd-651e-ce0b-3a4c-aa10e292ce2f@coly.li> Date: Sat, 13 Jan 2018 12:06:26 +0800 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.13; rv:52.0) Gecko/20100101 Thunderbird/52.5.2 MIME-Version: 1.0 In-Reply-To: <1515770690-18562-1-git-send-email-freakpv@gmail.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 8bit X-AntiAbuse: This header was added to track abuse, please include it with any abuse report X-AntiAbuse: Primary Hostname - server.coly.li X-AntiAbuse: Original Domain - vger.kernel.org X-AntiAbuse: Originator/Caller UID/GID - [47 12] / [47 12] X-AntiAbuse: Sender Address Domain - coly.li X-Get-Message-Sender-Via: server.coly.li: authenticated_id: i@coly.li X-Authenticated-Sender: server.coly.li: i@coly.li X-Source: X-Source-Args: X-Source-Dir: Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Return-Path: On 12/01/2018 11:24 PM, Pavel Vazharov wrote: > There was a possibility for infinite do-while loop inside the GC thread > function in case of total failure of the caching device. I was able to > reproduce it 3 times simulating disappearing of the caching device via > 'echo 1 > /sys/block//device/delete'. In that case the btree_root > starts to return non zero and non -EAGAIN result, 'gc failed' message > start to fill the kernel log and the do-while becomes infinite loop > occupying single CPU core at 100%. > There is already a logic which unregisters the cache_set (or panics) in > case of io errors and thus we exit the loop here if the unregistering > procedure has already started. > > Signed-off-by: Pavel Vazharov > --- > drivers/md/bcache/btree.c | 8 ++++++-- > 1 file changed, 6 insertions(+), 2 deletions(-) > > diff --git a/drivers/md/bcache/btree.c b/drivers/md/bcache/btree.c > index 81e8dc3..a672081 100644 > --- a/drivers/md/bcache/btree.c > +++ b/drivers/md/bcache/btree.c > @@ -1748,8 +1748,12 @@ static void bch_btree_gc(struct cache_set *c) > closure_sync(&writes); > cond_resched(); > > - if (ret && ret != -EAGAIN) > - pr_warn("gc failed!"); > + if (ret && ret != -EAGAIN) { > + if (test_bit(CACHE_SET_UNREGISTERING, &c->flags)) > + break; > + else > + pr_warn("gc failed!"); > + } > } while (ret); > > bch_btree_gc_finish(c); > Hi Pavel, I see the point here. But there are 2 code paths to call cache_set_flush(), one is from bch_cache_set_error(), one is from sysfs interface (echo 1 > /sys/fs/bcache//stop). CACHE_SET_UNREGISTERING is set in the first code path, the another code path from sysfs does not set CACHE_SET_UNREGISTERING. In this case maybe the above while-loop can not be stopped. In my device failure cache set, I add an io_disable (in v2 it is CACHE_SET_IO_DISABLE flag) to disable all cache set I/O, maybe it can be used to check the condition and break the while-loop. Thanks for the hint, I will also try to fix it in my patch set. If you don't mind, I am glad to have your "Reviewed-by:" after I post the v2 patch set. Thanks. -- Coly Li