linux-kernel - Re: [PATCH] SLUB: use have_arch

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: <20070827202501.GC5653@Krystal>
Date:	Mon, 27 Aug 2007 16:25:02 -0400
From:	Mathieu Desnoyers <mathieu.desnoyers@...ymtl.ca>
To:	Christoph Lameter <clameter@....com>
Cc:	Pekka Enberg <penberg@...helsinki.fi>, akpm@...ux-foundation.org,
	linux-kernel@...r.kernel.org, mingo@...hat.com
Subject: Re: [PATCH] SLUB: use have_arch_cmpxchg()

* Christoph Lameter (clameter@....com) wrote:
> On Mon, 27 Aug 2007, Mathieu Desnoyers wrote:
> 
> > Hrm, actually, I don't think such have_arch_cmpxchg() macro will be
> > required at all because of the small performance hit disabling
> > preemption will have on the slow and fast paths. Let's compare, for each
> > of the slow path and fast path, what locking looks like on architecture
> > with and without local cmpxchg:
> > 
> > What we actually do here is:
> > 
> > fast path:
> > with local_cmpxchg:
> >   preempt_disable()
> >   preempt_enable()
> > without local_cmpxchg:
> >   preempt_disable()
> >   local_irq_save
> >   local_irq_restore
> >   preempt_enable()
> >     (we therefore disable preemption _and_ disable interrupts for
> >     nothing)
> 
> Hmmm..... This is a performance issue for preempt kernels.
> 
> > slow path:
> > both with and without local_cmpxchg():
> >   preempt_disable()
> >   preempt_enable()
> 
> Here we potentially loose our per cpu structure since the process may be 
> rescheduled.
> 

Yes, what you do here is more precisely:

  preempt_disable()
  local_irq_save()
  preempt_enable_no_resched()
  ...
  local_irq_restore()
  preempt_check_resched()


> >   local_irq_save()
> >   local_irq_restore()
> > 
> > 
> > Therefore, we would add preempt disable/enable to the fast path of
> > architectures where local_cmpxchg is emulated with irqs off. But since
> > preempt disable/enable is just a check counter increment/decrement with
> > barrier() and thread flag check, I doubt it would hurt performances
> > enough to justify the added complexity of disabling interrupts for the
> > whole fast path in this case.
> 
> One additional cacheline referenced in the hot path. Ok the counter may be 
> hot as well if this is a preemptible kernel. Nevertheless a cause for 
> concern.
> 

If kernel is non preemptible, the macros preempt_*() are defined to
nothing. And as you say, on preemptible kernels, these variables (thread
flags and preempt count) are very likely to be in cache, since they are
in the thread info struct, but it should still be kept in mind.

Mathieu

-- 
Mathieu Desnoyers
Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68
-
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/