From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx2.suse.de ([195.135.220.15]:48971 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752902AbeADPv4 (ORCPT ); Thu, 4 Jan 2018 10:51:56 -0500 Date: Thu, 4 Jan 2018 16:51:52 +0100 From: Jan Kara To: Jiang Biao Cc: viro@zeniv.linux.org.uk, linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org, zhong.weidong@zte.com.cn Subject: Re: [PATCH] fs/mbcache: make count_objects more robust. Message-ID: <20180104155152.GC2170@quack2.suse.cz> References: <1511753419-52328-1-git-send-email-jiang.biao2@zte.com.cn> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1511753419-52328-1-git-send-email-jiang.biao2@zte.com.cn> Sender: linux-fsdevel-owner@vger.kernel.org List-ID: On Mon 27-11-17 11:30:19, Jiang Biao wrote: > When running ltp stress test for 7*24 hours, the vmscan occasionally > complains the following warning continuously, > > mb_cache_scan+0x0/0x3f0 negative objects to delete > nr=-9232265467809300450 > ... > > The tracing result shows the freeable(mb_cache_count returns) > is -1, which causes the continuous accumulation and overflow of > total_scan. > > This patch make sure the mb_cache_count not return negative value, > which make the mbcache shrinker more robust. > > Signed-off-by: Jiang Biao Going through some old email... a) c_entry_count is unsigned so your patch is a nop as Coverity properly noticed. b) c_entry_count being outside 0..2*cache->c_max_entries is a plain bug. I went through the logic and cannot find out how that could happen though. But in either case your patch just does not make sense. Honza > --- > fs/mbcache.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/fs/mbcache.c b/fs/mbcache.c > index d818fd2..b8b8b9c 100644 > --- a/fs/mbcache.c > +++ b/fs/mbcache.c > @@ -269,6 +269,9 @@ static unsigned long mb_cache_count(struct shrinker *shrink, > struct mb_cache *cache = container_of(shrink, struct mb_cache, > c_shrink); > > + /* Unlikely, but not impossible */ > + if (unlikely(cache->c_entry_count < 0)) > + return 0; > return cache->c_entry_count; > } > > -- > 2.7.4 > -- Jan Kara SUSE Labs, CR