From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755578AbbI1Gw7 (ORCPT ); Mon, 28 Sep 2015 02:52:59 -0400 Received: from mail-pa0-f49.google.com ([209.85.220.49]:32855 "EHLO mail-pa0-f49.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750843AbbI1Gw6 (ORCPT ); Mon, 28 Sep 2015 02:52:58 -0400 Content-Type: text/plain; charset=us-ascii Mime-Version: 1.0 (Mac OS X Mail 9.0 \(3094\)) Subject: Re: [PATCH] fs/buffer: simplify the code flow of LRU management algorithm From: yalin wang In-Reply-To: <20150928053639.GA10349@dhcp-128-25.nay.redhat.com> Date: Mon, 28 Sep 2015 14:52:51 +0800 Cc: Al Viro , linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org, Minfei Huang Content-Transfer-Encoding: 7bit Message-Id: <28690A54-7E8A-4170-9A86-CB077E4F112D@gmail.com> References: <1441872579-31595-1-git-send-email-mhuang@redhat.com> <20150928053639.GA10349@dhcp-128-25.nay.redhat.com> To: Minfei Huang X-Mailer: Apple Mail (2.3094) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org why not change like this: diff --git a/fs/buffer.c b/fs/buffer.c index 82283ab..d6769f1 100644 --- a/fs/buffer.c +++ b/fs/buffer.c @@ -1287,40 +1287,31 @@ static inline void check_irqs_on(void) */ static void bh_lru_install(struct buffer_head *bh) { - struct buffer_head *evictee = NULL; + struct buffer_head *old = NULL; check_irqs_on(); bh_lru_lock(); if (__this_cpu_read(bh_lrus.bhs[0]) != bh) { - struct buffer_head *bhs[BH_LRU_SIZE]; - int in; + struct buffer_head *temp; int out = 0; + old = __this_cpu_read(bh_lrus.bhs[0]); get_bh(bh); - bhs[out++] = bh; - for (in = 0; in < BH_LRU_SIZE; in++) { - struct buffer_head *bh2 = - __this_cpu_read(bh_lrus.bhs[in]); - - if (bh2 == bh) { - __brelse(bh2); + __this_cpu_write(bh_lrus.bhs[out++], bh); + for (; out < BH_LRU_SIZE; out++) { + if (old == bh || old == NULL) { + break; } else { - if (out >= BH_LRU_SIZE) { - BUG_ON(evictee != NULL); - evictee = bh2; - } else { - bhs[out++] = bh2; - } + temp = __this_cpu_read(bh_lrus.bhs[out]); + __this_cpu_write(bh_lrus.bhs[out], old); + old = temp; } } - while (out < BH_LRU_SIZE) - bhs[out++] = NULL; - memcpy(this_cpu_ptr(&bh_lrus.bhs), bhs, sizeof(bhs)); } bh_lru_unlock(); - if (evictee) - __brelse(evictee); + if (old) + __brelse(old); } /* more simple to understand and have better performance . am i understanding correctly ? > On Sep 28, 2015, at 13:36, Minfei Huang wrote: > > Ping, Could you someone help to review this patch? > > Thanks > Minfei > > On 09/10/15 at 04:09pm, Minfei Huang wrote: >> From: Minfei Huang >> >> There is a buffer_head lru list cache in local cpu to accelerate the >> speed. The LRU management algorithm is simple enough in >> bh_lru_install(). >> >> There are three situtaions we should deal with. >> 1) All/part of the lru cache is NULL. >> 2) The new buffer_head hitts the lru cache. >> 3) The new buffer_head does hit the lru cache. >> >> We put the new buffer_head at the head of lru cache, then copy the >> buffer_head from the original lru cache, and drop the spare. >> >> Signed-off-by: Minfei Huang >> --- >> fs/buffer.c | 32 ++++++++++++++++++++------------ >> 1 file changed, 20 insertions(+), 12 deletions(-) >> >> diff --git a/fs/buffer.c b/fs/buffer.c >> index 1cf7a53..2139574 100644 >> --- a/fs/buffer.c >> +++ b/fs/buffer.c >> @@ -1287,8 +1287,6 @@ static inline void check_irqs_on(void) >> */ >> static void bh_lru_install(struct buffer_head *bh) >> { >> - struct buffer_head *evictee = NULL; >> - >> check_irqs_on(); >> bh_lru_lock(); >> if (__this_cpu_read(bh_lrus.bhs[0]) != bh) { >> @@ -1302,25 +1300,35 @@ static void bh_lru_install(struct buffer_head *bh) >> struct buffer_head *bh2 = >> __this_cpu_read(bh_lrus.bhs[in]); >> >> - if (bh2 == bh) { >> + if (bh2 == NULL) { >> + /* Rest value in bh_lrus.bhs always is NULL */ >> + break; >> + } else if (bh2 == bh) { >> __brelse(bh2); >> } else { >> - if (out >= BH_LRU_SIZE) { >> - BUG_ON(evictee != NULL); >> - evictee = bh2; >> + if (out == BH_LRU_SIZE) { >> + /* >> + * this condition will be happened, >> + * only if none of entry in >> + * bh_lrus.bhs hits the new bh, >> + * so the last bh should be released. >> + */ >> + BUG_ON(in != BH_LRU_SIZE - 1); >> + __brelse(bh2); >> + break; >> } else { >> bhs[out++] = bh2; >> } >> } >> } >> - while (out < BH_LRU_SIZE) >> - bhs[out++] = NULL; >> - memcpy(this_cpu_ptr(&bh_lrus.bhs), bhs, sizeof(bhs)); >> + /* >> + * it is fine that the value out may be smaller than >> + * BH_LRU_SIZE. The rest of the value in bh_lrus.bhs is NULL. >> + */ >> + memcpy(this_cpu_ptr(&bh_lrus.bhs), bhs, >> + sizeof(struct buffer_head *) * out); >> } >> bh_lru_unlock(); >> - >> - if (evictee) >> - __brelse(evictee); >> } >> >> /* >> -- >> 2.1.0 >> >> -- >> 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/ > -- > 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/