linux-kernel - Re: [PATCH 2/2] writeback: Rework wb_[dec|inc]

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 20 Jun 2017 13:33:46 -0400
From:   Tejun Heo <tj@...nel.org>
To:     Nikolay Borisov <nborisov@...e.com>
Cc:     jbacik@...com, jack@...e.cz, linux-kernel@...r.kernel.org,
        hannes@...xchg.org, mgorman@...hsingularity.net
Subject: Re: [PATCH 2/2] writeback: Rework wb_[dec|inc]_stat family of
 functions

Hello,

On Tue, Jun 20, 2017 at 02:36:30PM +0300, Nikolay Borisov wrote:
> Currently the writeback statistics code uses a percpu counters to hold
> various statistics. As such we have 2 families of functions - those which
> disable local irq and those which doesn't and whose names are begin with
> double underscore. However, they both end up calling __add_wb_stats which in
> turn end up calling percpu_counter_add_batch which is already SMP-safe.

There's a difference between being SMP-safe and being preemption / irq
save.  Even on UP machine, rmw cycles can go wrong due to different
contexts operating on the same memory area, but you're right, all
percpu counter ops are irq safe, so there's no reason for wb stat
operations to have different variants.

> Exploiting this fact allows to eliminated the __wb_* functions since they do
> in fact cal SMP-safe primitive. Furthermore, refactor the wb_* function
> to call __add_wb_stat directly without the irq-disabling dance. This will
> likely result in better runtime of code which deals with modifying the stat
> counters.
> 
> Signed-off-by: Nikolay Borisov <nborisov@...e.com>
> ---
> Hello Tejun, 
> 
> This patch resulted from me reading your feedback on Josef's memory 
> throttling prep patch https://patchwork.kernel.org/patch/9395219/ . If these
> changes are merged then his series can eliminated irq clustering and use 
> straight __add_wb_stat call. I'd like to see his series merged sooner rather
> than later hence why sending this cleanup. 

Sure, but can you please update the patch description?

Thanks.

-- 
tejun