* [PATCH v2] buffer: Avoid setting buffer bits that are already set
@ 2017-10-24 1:16 Kemi Wang
2017-11-03 2:29 ` kemi
0 siblings, 1 reply; 4+ messages in thread
From: Kemi Wang @ 2017-10-24 1:16 UTC (permalink / raw)
To: Jan Kara, Jens Axboe, Darrick J Wong, Kemi Wang, Eric Biggers,
Andreas Gruenbacher, Jeff Layton
Cc: Dave, Andi Kleen, Tim Chen, Ying Huang, Aaron Lu, Linux Kernel
It's expensive to set buffer flags that are already set, because that
causes a costly cache line transition.
A common case is setting the "verified" flag during ext4 writes.
This patch checks for the flag being set first.
With the AIM7/creat-clo benchmark testing on a 48G ramdisk based-on ext4
file system, we see 3.3%(15431->15936) improvement of aim7.jobs-per-min on
a 2-sockets broadwell platform.
What the benchmark does is: it forks 3000 processes, and each process do
the following:
a) open a new file
b) close the file
c) delete the file
until loop=100*1000 times.
The original patch is contributed by Andi Kleen.
Signed-off-by: Andi Kleen <ak@linux.intel.com>
Signed-off-by: Kemi Wang <kemi.wang@intel.com>
Tested-by: Kemi Wang <kemi.wang@intel.com>
Reviewed-by: Jens Axboe <axboe@kernel.dk>
---
include/linux/buffer_head.h | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/include/linux/buffer_head.h b/include/linux/buffer_head.h
index c8dae55..211d8f5 100644
--- a/include/linux/buffer_head.h
+++ b/include/linux/buffer_head.h
@@ -80,11 +80,14 @@ struct buffer_head {
/*
* macro tricks to expand the set_buffer_foo(), clear_buffer_foo()
* and buffer_foo() functions.
+ * To avoid reset buffer flags that are already set, because that causes
+ * a costly cache line transition, check the flag first.
*/
#define BUFFER_FNS(bit, name) \
static __always_inline void set_buffer_##name(struct buffer_head *bh) \
{ \
- set_bit(BH_##bit, &(bh)->b_state); \
+ if (!test_bit(BH_##bit, &(bh)->b_state)) \
+ set_bit(BH_##bit, &(bh)->b_state); \
} \
static __always_inline void clear_buffer_##name(struct buffer_head *bh) \
{ \
--
2.7.4
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH v2] buffer: Avoid setting buffer bits that are already set
2017-10-24 1:16 [PATCH v2] buffer: Avoid setting buffer bits that are already set Kemi Wang
@ 2017-11-03 2:29 ` kemi
2018-02-02 8:07 ` kemi
0 siblings, 1 reply; 4+ messages in thread
From: kemi @ 2017-11-03 2:29 UTC (permalink / raw)
To: Jan Kara, Jens Axboe, Darrick J Wong, Eric Biggers,
Andreas Gruenbacher, Jeff Layton
Cc: Dave, Andi Kleen, Tim Chen, Ying Huang, Aaron Lu, Linux Kernel
On 2017年10月24日 09:16, Kemi Wang wrote:
> It's expensive to set buffer flags that are already set, because that
> causes a costly cache line transition.
>
> A common case is setting the "verified" flag during ext4 writes.
> This patch checks for the flag being set first.
>
> With the AIM7/creat-clo benchmark testing on a 48G ramdisk based-on ext4
> file system, we see 3.3%(15431->15936) improvement of aim7.jobs-per-min on
> a 2-sockets broadwell platform.
>
> What the benchmark does is: it forks 3000 processes, and each process do
> the following:
> a) open a new file
> b) close the file
> c) delete the file
> until loop=100*1000 times.
>
> The original patch is contributed by Andi Kleen.
>
> Signed-off-by: Andi Kleen <ak@linux.intel.com>
> Signed-off-by: Kemi Wang <kemi.wang@intel.com>
> Tested-by: Kemi Wang <kemi.wang@intel.com>
> Reviewed-by: Jens Axboe <axboe@kernel.dk>
> ---
Seems that this patch is still not merged. Anything wrong with that? thanks
> include/linux/buffer_head.h | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/include/linux/buffer_head.h b/include/linux/buffer_head.h
> index c8dae55..211d8f5 100644
> --- a/include/linux/buffer_head.h
> +++ b/include/linux/buffer_head.h
> @@ -80,11 +80,14 @@ struct buffer_head {
> /*
> * macro tricks to expand the set_buffer_foo(), clear_buffer_foo()
> * and buffer_foo() functions.
> + * To avoid reset buffer flags that are already set, because that causes
> + * a costly cache line transition, check the flag first.
> */
> #define BUFFER_FNS(bit, name) \
> static __always_inline void set_buffer_##name(struct buffer_head *bh) \
> { \
> - set_bit(BH_##bit, &(bh)->b_state); \
> + if (!test_bit(BH_##bit, &(bh)->b_state)) \
> + set_bit(BH_##bit, &(bh)->b_state); \
> } \
> static __always_inline void clear_buffer_##name(struct buffer_head *bh) \
> { \
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2] buffer: Avoid setting buffer bits that are already set
2017-11-03 2:29 ` kemi
@ 2018-02-02 8:07 ` kemi
2018-02-02 14:50 ` Jens Axboe
0 siblings, 1 reply; 4+ messages in thread
From: kemi @ 2018-02-02 8:07 UTC (permalink / raw)
To: Jan Kara, Jens Axboe, Darrick J Wong, Eric Biggers,
Andreas Gruenbacher, Jeff Layton
Cc: Dave, Andi Kleen, Tim Chen, Ying Huang, linux-block, Linux Kernel
Hi, Jens
Could you help to merge this patch to your tree? Thanks
On 2017年11月03日 10:29, kemi wrote:
>
>
> On 2017年10月24日 09:16, Kemi Wang wrote:
>> It's expensive to set buffer flags that are already set, because that
>> causes a costly cache line transition.
>>
>> A common case is setting the "verified" flag during ext4 writes.
>> This patch checks for the flag being set first.
>>
>> With the AIM7/creat-clo benchmark testing on a 48G ramdisk based-on ext4
>> file system, we see 3.3%(15431->15936) improvement of aim7.jobs-per-min on
>> a 2-sockets broadwell platform.
>>
>> What the benchmark does is: it forks 3000 processes, and each process do
>> the following:
>> a) open a new file
>> b) close the file
>> c) delete the file
>> until loop=100*1000 times.
>>
>> The original patch is contributed by Andi Kleen.
>>
>> Signed-off-by: Andi Kleen <ak@linux.intel.com>
>> Signed-off-by: Kemi Wang <kemi.wang@intel.com>
>> Tested-by: Kemi Wang <kemi.wang@intel.com>
>> Reviewed-by: Jens Axboe <axboe@kernel.dk>
>> ---
>
> Seems that this patch is still not merged. Anything wrong with that? thanks
>
>> include/linux/buffer_head.h | 5 ++++-
>> 1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/include/linux/buffer_head.h b/include/linux/buffer_head.h
>> index c8dae55..211d8f5 100644
>> --- a/include/linux/buffer_head.h
>> +++ b/include/linux/buffer_head.h
>> @@ -80,11 +80,14 @@ struct buffer_head {
>> /*
>> * macro tricks to expand the set_buffer_foo(), clear_buffer_foo()
>> * and buffer_foo() functions.
>> + * To avoid reset buffer flags that are already set, because that causes
>> + * a costly cache line transition, check the flag first.
>> */
>> #define BUFFER_FNS(bit, name) \
>> static __always_inline void set_buffer_##name(struct buffer_head *bh) \
>> { \
>> - set_bit(BH_##bit, &(bh)->b_state); \
>> + if (!test_bit(BH_##bit, &(bh)->b_state)) \
>> + set_bit(BH_##bit, &(bh)->b_state); \
>> } \
>> static __always_inline void clear_buffer_##name(struct buffer_head *bh) \
>> { \
>>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2] buffer: Avoid setting buffer bits that are already set
2018-02-02 8:07 ` kemi
@ 2018-02-02 14:50 ` Jens Axboe
0 siblings, 0 replies; 4+ messages in thread
From: Jens Axboe @ 2018-02-02 14:50 UTC (permalink / raw)
To: kemi, Jan Kara, Darrick J Wong, Eric Biggers,
Andreas Gruenbacher, Jeff Layton
Cc: Dave, Andi Kleen, Tim Chen, Ying Huang, linux-block, Linux Kernel
On 2/2/18 1:07 AM, kemi wrote:
> Hi, Jens
> Could you help to merge this patch to your tree? Thanks
Yes, I'll queue it up, thanks.
--
Jens Axboe
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2018-02-02 14:50 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-10-24 1:16 [PATCH v2] buffer: Avoid setting buffer bits that are already set Kemi Wang
2017-11-03 2:29 ` kemi
2018-02-02 8:07 ` kemi
2018-02-02 14:50 ` Jens Axboe
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).