slub checks for poison one byte by one, which is highly inefficient
and shows up frequently as a highest cpu-eater in perf top.
Joining reads gives nice speedup:
(Compiling some project with different options)
make -j12 make clean
slub_debug disabled: 1m 27s 1.2 s
slub_debug enabled: 1m 46s 7.6 s
slub_debug enabled + this patch: 1m 33s 3.2 s
check_bytes still shows up high, but not always at the top.
Signed-off-by: Marcin Slusarz <[email protected]>
Cc: Christoph Lameter <[email protected]>
Cc: Pekka Enberg <[email protected]>
Cc: Matt Mackall <[email protected]>
Cc: [email protected]
---
mm/slub.c | 36 ++++++++++++++++++++++++++++++++++--
1 files changed, 34 insertions(+), 2 deletions(-)
diff --git a/mm/slub.c b/mm/slub.c
index 35f351f..a40ef2d 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -557,10 +557,10 @@ static void init_object(struct kmem_cache *s, void *object, u8 val)
memset(p + s->objsize, val, s->inuse - s->objsize);
}
-static u8 *check_bytes(u8 *start, unsigned int value, unsigned int bytes)
+static u8 *check_bytes8(u8 *start, u8 value, unsigned int bytes)
{
while (bytes) {
- if (*start != (u8)value)
+ if (*start != value)
return start;
start++;
bytes--;
@@ -568,6 +568,38 @@ static u8 *check_bytes(u8 *start, unsigned int value, unsigned int bytes)
return NULL;
}
+static u8 *check_bytes(u8 *start, u8 value, unsigned int bytes)
+{
+ u64 value64;
+ unsigned int words, prefix;
+
+ if (bytes <= 16)
+ return check_bytes8(start, value, bytes);
+
+ value64 = value | value << 8 | value << 16 | value << 24;
+ value64 = value64 | value64 << 32;
+ prefix = 8 - ((unsigned long)start) % 8;
+
+ if (prefix) {
+ u8 *r = check_bytes8(start, value, prefix);
+ if (r)
+ return r;
+ start += prefix;
+ bytes -= prefix;
+ }
+
+ words = bytes / 8;
+
+ while (words) {
+ if (*(u64 *)start != value64)
+ return check_bytes8(start, value, 8);
+ start += 8;
+ words--;
+ }
+
+ return check_bytes8(start, value, bytes % 8);
+}
+
static void restore_bytes(struct kmem_cache *s, char *message, u8 data,
void *from, void *to)
{
--
1.7.5.3
On Sun, 26 Jun 2011, Marcin Slusarz wrote:
> slub checks for poison one byte by one, which is highly inefficient
> and shows up frequently as a highest cpu-eater in perf top.
Ummm.. Performance improvements for debugging modes? If you need
performance then switch off debuggin.
On 06/28/2011 12:32 PM, Christoph Lameter wrote:
> On Sun, 26 Jun 2011, Marcin Slusarz wrote:
>
>> slub checks for poison one byte by one, which is highly inefficient
>> and shows up frequently as a highest cpu-eater in perf top.
>
> Ummm.. Performance improvements for debugging modes? If you need
> performance then switch off debuggin.
There is no reason to make things gratuitously slow. I don't know about
the merits of this particular patch, but I must disagree with the
general sentiment.
We have high performance tracing, why not improve this as well.
Just last week I was trying to find the cause of memory corruption that
only occurred at very high network packet rates. Memory allocation
speed was definitely getting in the way of debugging. For me, faster
SLUB debugging would be welcome.
David Daney
On Tue, 28 Jun 2011, David Daney wrote:
> On 06/28/2011 12:32 PM, Christoph Lameter wrote:
> > On Sun, 26 Jun 2011, Marcin Slusarz wrote:
> >
> > > slub checks for poison one byte by one, which is highly inefficient
> > > and shows up frequently as a highest cpu-eater in perf top.
> >
> > Ummm.. Performance improvements for debugging modes? If you need
> > performance then switch off debuggin.
>
> There is no reason to make things gratuitously slow. I don't know about the
> merits of this particular patch, but I must disagree with the general
> sentiment.
>
> We have high performance tracing, why not improve this as well.
>
> Just last week I was trying to find the cause of memory corruption that only
> occurred at very high network packet rates. Memory allocation speed was
> definitely getting in the way of debugging. For me, faster SLUB debugging
> would be welcome.
>
SLUB debugging is useful only to diagnose issues or test new code, nobody
is going to be enabling it in production environment. We don't need 30
new lines of code that make one thing slightly faster, in fact we'd prefer
to have as simple and minimal code as possible for debugging features
unless you're adding more debugging coverage.
On 06/28/2011 01:58 PM, David Rientjes wrote:
> On Tue, 28 Jun 2011, David Daney wrote:
>
>> On 06/28/2011 12:32 PM, Christoph Lameter wrote:
>>> On Sun, 26 Jun 2011, Marcin Slusarz wrote:
>>>
>>>> slub checks for poison one byte by one, which is highly inefficient
>>>> and shows up frequently as a highest cpu-eater in perf top.
>>>
>>> Ummm.. Performance improvements for debugging modes? If you need
>>> performance then switch off debuggin.
>>
>> There is no reason to make things gratuitously slow. I don't know about the
>> merits of this particular patch, but I must disagree with the general
>> sentiment.
>>
>> We have high performance tracing, why not improve this as well.
>>
>> Just last week I was trying to find the cause of memory corruption that only
>> occurred at very high network packet rates. Memory allocation speed was
>> definitely getting in the way of debugging. For me, faster SLUB debugging
>> would be welcome.
>>
>
> SLUB debugging is useful only to diagnose issues or test new code, nobody
> is going to be enabling it in production environment. We don't need 30
> new lines of code that make one thing slightly faster, in fact we'd prefer
> to have as simple and minimal code as possible for debugging features
> unless you're adding more debugging coverage.
If your problem happens under load, then the overhead of slub could significantly
change the behaviour of the system. Anything that makes it more efficient without
unduly complicating code should be a win. That posted patch wasn't all that
complicated, and even if it has bugs, it could be fixed easily enough.
Thanks,
Ben
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
--
Ben Greear <[email protected]>
Candela Technologies Inc http://www.candelatech.com
On Tue, 28 Jun 2011, Ben Greear wrote:
> > SLUB debugging is useful only to diagnose issues or test new code, nobody
> > is going to be enabling it in production environment. We don't need 30
> > new lines of code that make one thing slightly faster, in fact we'd prefer
> > to have as simple and minimal code as possible for debugging features
> > unless you're adding more debugging coverage.
>
> If your problem happens under load, then the overhead of slub could
> significantly
> change the behaviour of the system.
You're talking about slub debugging and not slub in general, I assume.
> Anything that makes it more efficient
> without
> unduly complicating code should be a win. That posted patch wasn't all that
> complicated, and even if it has bugs, it could be fixed easily enough.
>
"Even if it has bugs"? Ask Pekka to merge this and be on the receiving
end of every other kernel development's flames when slub debugging no
longer finds their problems but instead has bugs of its own.
Again, we want simple and minimal slub debugging code unless you're adding
a new feature.
On Tue, Jun 28, 2011 at 01:58:06PM -0700, David Rientjes wrote:
> SLUB debugging is useful only to diagnose issues or test new code, nobody
> is going to be enabling it in production environment. We don't need 30
> new lines of code that make one thing slightly faster
During five of the six months of development, Fedora kernels are built with
slub debugging forced on. And it turns up problems every single release.
Having the impact of this be lower is very desirable. If this doesn't get
merged, I'd be tempted to carry it in Fedora regardless for this reason.
Dave