Re: svn commit: r242274 - head/sys/sys

2012-10-29 Thread Attilio Rao 
On 10/29/12, Gleb Smirnoff gleb...@freebsd.org wrote:
 On Mon, Oct 29, 2012 at 01:35:17AM +, Attilio Rao wrote:
 A Author: attilio
 A Date: Mon Oct 29 01:35:17 2012
 A New Revision: 242274
 A URL: http://svn.freebsd.org/changeset/base/242274
 A
 A Log:
 A   Compiler have a precise knowledge of the content of sched_pin() and
 A   sched_unpin() as they are functions static and inline.  This way it
 A   can do two dangerous things:
 A   - Reorder instructions around both of them, taking out from the safe
 A path operations that are supposed to be (ie. per-cpu accesses)
 A   - Cache the value of td_pinned in CPU registers not making visible
 A in kernel context to the scheduler once it is scanning the runqueue,
 A as td_pinned is not marked volatile.
 A
 A   In order to avoid both possible bugs explicitly, protect the safe path
 A   with compiler memory barriers. This will prevent reordering and
 caching
 A   by the compiler about td_pinned operations.
 A
 A   Generally this could lead to suboptimal code traversing the pinnings
 A   but this is not the case as can be easilly verified:
 A
 http://lists.freebsd.org/pipermail/svn-src-projects/2012-October/005797.html

 Now __compiler_membar() can be removed from kern_rmlock.c:360

No, they are there to protect td_critnest which has nothing to do with
sched_pin().

Attilio


-- 
Peace can only be achieved by understanding - A. Einstein
___
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org

svn commit: r242274 - head/sys/sys

2012-10-28 Thread Attilio Rao 
Author: attilio
Date: Mon Oct 29 01:35:17 2012
New Revision: 242274
URL: http://svn.freebsd.org/changeset/base/242274

Log:
  Compiler have a precise knowledge of the content of sched_pin() and
  sched_unpin() as they are functions static and inline.  This way it
  can do two dangerous things:
  - Reorder instructions around both of them, taking out from the safe
path operations that are supposed to be (ie. per-cpu accesses)
  - Cache the value of td_pinned in CPU registers not making visible
in kernel context to the scheduler once it is scanning the runqueue,
as td_pinned is not marked volatile.
  
  In order to avoid both possible bugs explicitly, protect the safe path
  with compiler memory barriers. This will prevent reordering and caching
  by the compiler about td_pinned operations.
  
  Generally this could lead to suboptimal code traversing the pinnings
  but this is not the case as can be easilly verified:
  http://lists.freebsd.org/pipermail/svn-src-projects/2012-October/005797.html
  
  Discussed with:   jeff, jhb
  MFC after:2 weeks

Modified:
  head/sys/sys/sched.h

Modified: head/sys/sys/sched.h
==
--- head/sys/sys/sched.hMon Oct 29 00:51:53 2012(r242273)
+++ head/sys/sys/sched.hMon Oct 29 01:35:17 2012(r242274)
@@ -151,11 +151,13 @@ static __inline void
 sched_pin(void)
 {
curthread-td_pinned++;
+   __compiler_membar();
 }
 
 static __inline void
 sched_unpin(void)
 {
+   __compiler_membar();
curthread-td_pinned--;
 }
 
___
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org

Re: svn commit: r242274 - head/sys/sys

2012-10-28 Thread Gleb Smirnoff 
On Mon, Oct 29, 2012 at 01:35:17AM +, Attilio Rao wrote:
A Author: attilio
A Date: Mon Oct 29 01:35:17 2012
A New Revision: 242274
A URL: http://svn.freebsd.org/changeset/base/242274
A 
A Log:
A   Compiler have a precise knowledge of the content of sched_pin() and
A   sched_unpin() as they are functions static and inline.  This way it
A   can do two dangerous things:
A   - Reorder instructions around both of them, taking out from the safe
A path operations that are supposed to be (ie. per-cpu accesses)
A   - Cache the value of td_pinned in CPU registers not making visible
A in kernel context to the scheduler once it is scanning the runqueue,
A as td_pinned is not marked volatile.
A   
A   In order to avoid both possible bugs explicitly, protect the safe path
A   with compiler memory barriers. This will prevent reordering and caching
A   by the compiler about td_pinned operations.
A   
A   Generally this could lead to suboptimal code traversing the pinnings
A   but this is not the case as can be easilly verified:
A   
http://lists.freebsd.org/pipermail/svn-src-projects/2012-October/005797.html

Now __compiler_membar() can be removed from kern_rmlock.c:360

-- 
Totus tuus, Glebius.
___
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org