| Message ID | 28690A54-7E8A-4170-9A86-CB077E4F112D@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 09/28/15 at 02:52pm, yalin wang wrote: > 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; If we should copy the successive struct buffer_head, it is appropriate to use memcpy to copy a bunch of struct buffer_head. Thanks Minfei > } > } > - 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 <mhuang@redhat.com> 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 <mnfhuang@gmail.com> > >> > >> 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 <mnfhuang@gmail.com> > >> --- > >> 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/ > -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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); } /*