linux-kernel - Re: [PATCH] make cancel_rearming_delayed

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]

Message-ID: <464DAB3D.4020000@gmail.com>
Date:	Fri, 18 May 2007 15:33:49 +0200
From:	Tejun Heo <htejun@...il.com>
To:	Jarek Poplawski <jarkao2@...pl>
CC:	Oleg Nesterov <oleg@...sign.ru>,
	Andrew Morton <akpm@...ux-foundation.org>,
	David Chinner <dgc@....com>,
	David Howells <dhowells@...hat.com>,
	Gautham Shenoy <ego@...ibm.com>, Ingo Molnar <mingo@...e.hu>,
	Srivatsa Vaddagiri <vatsa@...ibm.com>,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] make cancel_rearming_delayed_work() reliable

Hello,

Jarek Poplawski wrote:
> 2. IMHO the current solution with smp barriers is very good:
> these barriers are really needed and they should be enough.
> Oleg repeats all the time he hates barriers, but I think
> it's wrong approach - they should be seen as something
> natural for programming modern processors and trying to
> change the code only to avoid them is wrong (unless it
> could be done with some minor changes).
> 
> 3. The alternative solution without barriers, based on the
> idea of Tejun Heo and presented in the patch proposal from
> 2007-05-13, could be probably a little faster (for some
> processors), after some changes:
> 
> - I think, instead of separate __set_bit and __clear_bit,
> WORK_STRUCT_QUEUED bit should be changed during set_wq_data
> (additional parameter or new variant eg: set_wq_data_queued),
> and similarly something like  work_clear_pending_queued -
> so, without additional operations instead of these barriers.
> 
> - changing this if condition in try_to_grab_pending.
> 
> But there is a question - is this worth of one (last) bit
> occupied only for optimisation (but not always)?

I wasn't really aiming for performance optimization.  I agree that we
have to live with barriers but it's also true that they and other
synchronization constructs can be difficult to understand and thus to
verify, so IMHO, when it comes to synchronization constructs, it's best
stick to or make things look similar to more established ways, so that
people can categorize the usage and understand it more easily.

Locked enter/leave model, where each cpu has its own lock and whatever
entity enters and leaves a cpu while holding the respective per-cpu
lock, guarantees that while holding a per-cpu lock, no one enters or
leaves the cpu.  It's conceptually simple and basically the semantic
that we're all trying to achieve here.

I think things can be made more similar to the locked enter/leave model
using the extra bit by putting manipulation and testing of the bit into
accesor functions, so readability was my primary concern not
performance.  But, then again, we can probably make the barrier() model
readable enough too with proper accessor functions and plenty of
comments above them.

-- 
tejun
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/