Bug 207457 (64bitcounters) - rhds 7.1 - server stats use 32-bit integers - entrycachehitratio 1503%
Summary: rhds 7.1 - server stats use 32-bit integers - entrycachehitratio 1503%
Keywords:
Status: CLOSED CURRENTRELEASE
Alias: 64bitcounters
Product: Red Hat Directory Server
Classification: Red Hat
Component: Performance
Version: 7.1
Hardware: All
OS: Linux
high
medium
Target Milestone: DS8.1
: ---
Assignee: Nathan Kinder
QA Contact: Chandrasekar Kannan
URL:
Whiteboard:
Depends On:
Blocks: 249650 FDS1.2.0
TreeView+ depends on / blocked
 
Reported: 2006-09-21 07:44 UTC by Issue Tracker
Modified: 2018-10-20 00:47 UTC (History)
8 users (show)

Fixed In Version: 8.1
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2009-04-29 22:59:15 UTC
Target Upstream Version:
Embargoed:


Attachments (Terms of Use)
Slapi_Counter implementation (9.58 KB, text/plain)
2008-10-17 20:43 UTC, Nathan Kinder
no flags Details
Solaris atomic operation assembly code (5.82 KB, text/plain)
2008-10-17 20:44 UTC, Nathan Kinder
no flags Details
Diffs (first phase) (126.59 KB, patch)
2008-10-17 20:49 UTC, Nathan Kinder
no flags Details | Diff
cvs diffs ldap/servers/slapd/{slapi_counter.c,slapi_counter_sunos_sparcv9.il} (4.10 KB, patch)
2008-10-18 23:58 UTC, Noriko Hosoi
no flags Details | Diff
Solaris: failed to let automake process *.s/*.S (5.45 KB, text/plain)
2008-10-21 05:51 UTC, Noriko Hosoi
no flags Details
cvs diff Makefile.am configure.ac ldap/servers/slapd/slapi_counter.c (3.34 KB, patch)
2008-10-22 19:13 UTC, Noriko Hosoi
no flags Details | Diff
New file: ldap/servers/slapd/slapi_counter_sunos_sparcv9.S (5.90 KB, text/plain)
2008-10-22 19:15 UTC, Noriko Hosoi
no flags Details
svn diff pkgs/redhat-ds-base/redhat-ds-base.spec (494 bytes, patch)
2008-10-22 19:20 UTC, Noriko Hosoi
no flags Details | Diff
cvs commit messages (comment#29,30,31) (3.58 KB, text/plain)
2008-10-22 20:35 UTC, Noriko Hosoi
no flags Details
Diffs (final phase) (136.97 KB, patch)
2008-10-24 21:45 UTC, Nathan Kinder
no flags Details | Diff
Revised Diffs (final phase) (133.96 KB, text/plain)
2008-10-24 22:33 UTC, Nathan Kinder
no flags Details
cvs diff ldap/servers/slapd/back-ldbm/proto-back-ldbm.h (944 bytes, patch)
2008-10-24 23:31 UTC, Noriko Hosoi
no flags Details | Diff
cvs diff ldap/servers/slapd/snmp_collator.c (753 bytes, patch)
2008-10-27 18:32 UTC, Noriko Hosoi
no flags Details | Diff
cvs commit message (snmp_collator.c) (514 bytes, text/plain)
2008-10-28 00:20 UTC, Noriko Hosoi
no flags Details
Atomic functions for platforms without builtins (11.71 KB, patch)
2008-10-29 16:53 UTC, Nathan Kinder
no flags Details | Diff
Additional diff to fix 32-bit platforms with builtins (1.76 KB, text/plain)
2008-10-30 18:40 UTC, Nathan Kinder
no flags Details
cvs diff cache.c (2.16 KB, patch)
2008-11-11 21:32 UTC, Noriko Hosoi
no flags Details | Diff
Assembly offset two-liner patch (1.29 KB, patch)
2008-11-12 00:12 UTC, Nathan Kinder
no flags Details | Diff

Description Issue Tracker 2006-09-21 07:44:50 UTC
Escalated to Bugzilla from IssueTracker

Comment 3 Rich Megginson 2006-09-21 14:35:23 UTC
We will investigate this for DS 7.2.  It may be fairly simple to use 64 bit
integers for the counters.

Comment 5 Rich Megginson 2006-09-21 15:05:19 UTC
Yes, that's another possible implementation.  I think we may currently use
atomic increment operations on 32 bit integers, and AFAIK there are no atomic
operations on 64 bit integers, so we may have two counters, 1 to keep track of
wraps.

I would consider this a bug, so yes, it will definitely be considered for the
upcoming release or the next one.

Comment 15 Ulf Weltman 2007-10-03 20:49:24 UTC
On HP-UX Itanium you can increment 32-bit and 64-bit integers with inline
assembly if you #include <machine/sys/inline.h>:
  long long ll=1, ll2;
  ll2 = _Asm_fetchadd(_FASZ_D, _SEM_ACQ, &ll, +1, _LDHINT_NONE);
This would leave ll set to 2 and ll2 to 1 (the value before incrementing).
The first argument is the operand size and can be either _FASZ_W for word or
_FASZ_D for double word.  The fourth argument is increment value, it can be -16,
-8, -4, -1, 1, 4, 8 or 16.
Reference:
http://h21007.www2.hp.com/portal/site/dspp/PAGE.template/page.document?ciid=4308e2f5bde02110e2f5bde02110275d6e10RCRD

On HP-UX PA-RISC we only have one atomic operation, ldcws (load and clear word).
 This can be used to implement a spinlock around the increment code.
Reference:
http://h21007.www2.hp.com/portal/download/files/unprot/itanium/spinlocks.pdf

However, for PA-RISC I would suggest using the same implementation for 64-bit
that NSPR uses for 32-bit increment operations (PR_AtomicIncrement()), which is
pthread locks.
Reference: http://lxr.mozilla.org/seamonkey/source/nsprpub/pr/src/misc/pratom.c#165

Comment 16 Ulf Weltman 2007-10-04 00:51:59 UTC
I was looking at an older release of NSPR, it seems that the tip at least
already has 32-bit atomic increment implemented with the fetchadd assembly
instruction on IPF:
http://lxr.mozilla.org/seamonkey/source/nsprpub/pr/src/md/unix/os_HPUX_ia64.s#41

Maybe you can persuade NSPR to add 64-bit versions of the PR_AtomicXXX() functions.

Comment 20 Rich Megginson 2008-02-13 19:11:51 UTC
This feature is planned for RHDS 8.1 and Fedora DS 1.2 - here is the design
document - comments are welcome -
http://directory.fedoraproject.org/wiki/64_bit_counters_design

Comment 21 Nathan Kinder 2008-10-17 20:43:21 UTC
Created attachment 320720 [details]
Slapi_Counter implementation

New slapi_counter.c source file.

Comment 22 Nathan Kinder 2008-10-17 20:44:46 UTC
Created attachment 320721 [details]
Solaris atomic operation assembly code

New slapi_counter_sunos_sparcv9.il inline assembly source code file.

Comment 23 Nathan Kinder 2008-10-17 20:49:07 UTC
Created attachment 320722 [details]
Diffs (first phase)

This first phase of diffs exposes the new Slapi_Counter API via SLAPI.  The totalconnections counter was converted to use the new Slapi_Counter type for the purpose of testing out the new code.  This required the connection ID to be changed throughout the source tree, which affects logging mostly.

While testing the new code on HP-UX, it was found that we were using atoll() in our code, which is not available on HP-UX.  This was switched to strtoll(), which is available on all of our supported platforms.

Comment 24 Noriko Hosoi 2008-10-17 21:37:18 UTC
(In reply to comment #23)
> Created an attachment (id=320722) [details]
> Diffs (first phase)
> 
Your diffs look good to me.   I've noticed we still have "AM_PROG_AS" in configure.ac, which seems not needed since we don't have an assembly lang source file any more. I've rebuilt the tree w/o the macro and it compiles just fine.

Comment 25 Nathan Kinder 2008-10-17 22:10:39 UTC
(In reply to (In reply to comment #24)
> Your diffs look good to me.   I've noticed we still have "AM_PROG_AS" in
> configure.ac, which seems not needed since we don't have an assembly lang
> source file any more. I've rebuilt the tree w/o the macro and it compiles just
> fine.

I have removed the AC_PROG_AS macro from configure.ac and re-ran autogen.sh.  Good catch!  I also reviewed the portions of the code that you wrote (Solaris imnplementation), and they look good.

Comment 26 Nathan Kinder 2008-10-17 22:15:41 UTC
Checking in Makefile.am;
/cvs/dirsec/ldapserver/Makefile.am,v  <--  Makefile.am
new revision: 1.74; previous revision: 1.73
done
Checking in Makefile.in;
/cvs/dirsec/ldapserver/Makefile.in,v  <--  Makefile.in
new revision: 1.97; previous revision: 1.96
done
Checking in aclocal.m4;
/cvs/dirsec/ldapserver/aclocal.m4,v  <--  aclocal.m4
new revision: 1.75; previous revision: 1.74
done
Checking in config.guess;
/cvs/dirsec/ldapserver/config.guess,v  <--  config.guess
new revision: 1.56; previous revision: 1.55
done
Checking in config.h.in;
/cvs/dirsec/ldapserver/config.h.in,v  <--  config.h.in
new revision: 1.21; previous revision: 1.20
done
Checking in config.sub;
/cvs/dirsec/ldapserver/config.sub,v  <--  config.sub
new revision: 1.56; previous revision: 1.55
done
Checking in configure;
/cvs/dirsec/ldapserver/configure,v  <--  configure
new revision: 1.92; previous revision: 1.91
done
Checking in depcomp;
/cvs/dirsec/ldapserver/depcomp,v  <--  depcomp
new revision: 1.57; previous revision: 1.56
done
Checking in install-sh;
/cvs/dirsec/ldapserver/install-sh,v  <--  install-sh
new revision: 1.57; previous revision: 1.56
done
Checking in ltmain.sh;
/cvs/dirsec/ldapserver/ltmain.sh,v  <--  ltmain.sh
new revision: 1.28; previous revision: 1.27
done
Checking in missing;
/cvs/dirsec/ldapserver/missing,v  <--  missing
new revision: 1.57; previous revision: 1.56
done
Checking in ldap/servers/plugins/acl/acl.c;
/cvs/dirsec/ldapserver/ldap/servers/plugins/acl/acl.c,v  <--  acl.c
new revision: 1.12; previous revision: 1.11
done
Checking in ldap/servers/plugins/acl/acl.h;
/cvs/dirsec/ldapserver/ldap/servers/plugins/acl/acl.h,v  <--  acl.h
new revision: 1.6; previous revision: 1.5
done
Checking in ldap/servers/plugins/acl/acl_ext.c;
/cvs/dirsec/ldapserver/ldap/servers/plugins/acl/acl_ext.c,v  <--  acl_ext.c
new revision: 1.9; previous revision: 1.8
done
Checking in ldap/servers/plugins/acl/aclanom.c;
/cvs/dirsec/ldapserver/ldap/servers/plugins/acl/aclanom.c,v  <--  aclanom.c
new revision: 1.8; previous revision: 1.7
done
Checking in ldap/servers/plugins/dna/dna.c;
/cvs/dirsec/ldapserver/ldap/servers/plugins/dna/dna.c,v  <--  dna.c
new revision: 1.13; previous revision: 1.12
done
Checking in ldap/servers/plugins/replication/repl.h;
/cvs/dirsec/ldapserver/ldap/servers/plugins/replication/repl.h,v  <--  repl.h
new revision: 1.8; previous revision: 1.7
done
Checking in ldap/servers/plugins/replication/repl5.h;
/cvs/dirsec/ldapserver/ldap/servers/plugins/replication/repl5.h,v  <--  repl5.h
new revision: 1.12; previous revision: 1.11
done
Checking in ldap/servers/plugins/replication/repl5_init.c;
/cvs/dirsec/ldapserver/ldap/servers/plugins/replication/repl5_init.c,v  <--  repl5_init.c
new revision: 1.8; previous revision: 1.7
done
Checking in ldap/servers/plugins/replication/repl5_replica.c;
/cvs/dirsec/ldapserver/ldap/servers/plugins/replication/repl5_replica.c,v  <--  repl5_replica.c
new revision: 1.19; previous revision: 1.18
done
Checking in ldap/servers/plugins/replication/repl5_total.c;
/cvs/dirsec/ldapserver/ldap/servers/plugins/replication/repl5_total.c,v  <--  repl5_total.c
new revision: 1.12; previous revision: 1.11
done
Checking in ldap/servers/plugins/replication/repl_connext.c;
/cvs/dirsec/ldapserver/ldap/servers/plugins/replication/repl_connext.c,v  <--  repl_connext.c
new revision: 1.7; previous revision: 1.6
done
Checking in ldap/servers/plugins/replication/repl_extop.c;
/cvs/dirsec/ldapserver/ldap/servers/plugins/replication/repl_extop.c,v  <--  repl_extop.c
new revision: 1.14; previous revision: 1.13
done
Checking in ldap/servers/plugins/replication/replutil.c;
/cvs/dirsec/ldapserver/ldap/servers/plugins/replication/replutil.c,v  <--  replutil.c
new revision: 1.13; previous revision: 1.12
done
Checking in ldap/servers/slapd/abandon.c;
/cvs/dirsec/ldapserver/ldap/servers/slapd/abandon.c,v  <--  abandon.c
new revision: 1.8; previous revision: 1.7
done
Checking in ldap/servers/slapd/add.c;
/cvs/dirsec/ldapserver/ldap/servers/slapd/add.c,v  <--  add.c
new revision: 1.14; previous revision: 1.13
done
Checking in ldap/servers/slapd/auth.c;
/cvs/dirsec/ldapserver/ldap/servers/slapd/auth.c,v  <--  auth.c
new revision: 1.12; previous revision: 1.11
done
Checking in ldap/servers/slapd/bind.c;
/cvs/dirsec/ldapserver/ldap/servers/slapd/bind.c,v  <--  bind.c
new revision: 1.16; previous revision: 1.15
done
Checking in ldap/servers/slapd/compare.c;
/cvs/dirsec/ldapserver/ldap/servers/slapd/compare.c,v  <--  compare.c
new revision: 1.8; previous revision: 1.7
done
Checking in ldap/servers/slapd/connection.c;
/cvs/dirsec/ldapserver/ldap/servers/slapd/connection.c,v  <--  connection.c
new revision: 1.21; previous revision: 1.20
done
Checking in ldap/servers/slapd/conntable.c;
/cvs/dirsec/ldapserver/ldap/servers/slapd/conntable.c,v  <--  conntable.c
new revision: 1.11; previous revision: 1.10
done
Checking in ldap/servers/slapd/daemon.c;
/cvs/dirsec/ldapserver/ldap/servers/slapd/daemon.c,v  <--  daemon.c
new revision: 1.21; previous revision: 1.20
done
Checking in ldap/servers/slapd/delete.c;
/cvs/dirsec/ldapserver/ldap/servers/slapd/delete.c,v  <--  delete.c
new revision: 1.9; previous revision: 1.8
done
Checking in ldap/servers/slapd/extendop.c;
/cvs/dirsec/ldapserver/ldap/servers/slapd/extendop.c,v  <--  extendop.c
new revision: 1.9; previous revision: 1.8
done
Checking in ldap/servers/slapd/fe.h;
/cvs/dirsec/ldapserver/ldap/servers/slapd/fe.h,v  <--  fe.h
new revision: 1.9; previous revision: 1.8
done
Checking in ldap/servers/slapd/globals.c;
/cvs/dirsec/ldapserver/ldap/servers/slapd/globals.c,v  <--  globals.c
new revision: 1.8; previous revision: 1.7
done
Checking in ldap/servers/slapd/init.c;
/cvs/dirsec/ldapserver/ldap/servers/slapd/init.c,v  <--  init.c
new revision: 1.7; previous revision: 1.6
done
Checking in ldap/servers/slapd/modify.c;
/cvs/dirsec/ldapserver/ldap/servers/slapd/modify.c,v  <--  modify.c
new revision: 1.17; previous revision: 1.16
done
Checking in ldap/servers/slapd/modrdn.c;
/cvs/dirsec/ldapserver/ldap/servers/slapd/modrdn.c,v  <--  modrdn.c
new revision: 1.10; previous revision: 1.9
done
Checking in ldap/servers/slapd/opshared.c;
/cvs/dirsec/ldapserver/ldap/servers/slapd/opshared.c,v  <--  opshared.c
new revision: 1.13; previous revision: 1.12
done
Checking in ldap/servers/slapd/pblock.c;
/cvs/dirsec/ldapserver/ldap/servers/slapd/pblock.c,v  <--  pblock.c
new revision: 1.18; previous revision: 1.17
done
Checking in ldap/servers/slapd/plugin_internal_op.c;
/cvs/dirsec/ldapserver/ldap/servers/slapd/plugin_internal_op.c,v  <--  plugin_internal_op.c
new revision: 1.11; previous revision: 1.10
done
Checking in ldap/servers/slapd/proto-slap.h;
/cvs/dirsec/ldapserver/ldap/servers/slapd/proto-slap.h,v  <--  proto-slap.h
new revision: 1.38; previous revision: 1.37
done
Checking in ldap/servers/slapd/psearch.c;
/cvs/dirsec/ldapserver/ldap/servers/slapd/psearch.c,v  <--  psearch.c
new revision: 1.10; previous revision: 1.9
done
Checking in ldap/servers/slapd/result.c;
/cvs/dirsec/ldapserver/ldap/servers/slapd/result.c,v  <--  result.c
new revision: 1.14; previous revision: 1.13
done
Checking in ldap/servers/slapd/sasl_io.c;
/cvs/dirsec/ldapserver/ldap/servers/slapd/sasl_io.c,v  <--  sasl_io.c
new revision: 1.15; previous revision: 1.14
done
Checking in ldap/servers/slapd/search.c;
/cvs/dirsec/ldapserver/ldap/servers/slapd/search.c,v  <--  search.c
new revision: 1.10; previous revision: 1.9
done
Checking in ldap/servers/slapd/slap.h;
/cvs/dirsec/ldapserver/ldap/servers/slapd/slap.h,v  <--  slap.h
new revision: 1.37; previous revision: 1.36
done
Checking in ldap/servers/slapd/slapi-plugin.h;
/cvs/dirsec/ldapserver/ldap/servers/slapd/slapi-plugin.h,v  <--  slapi-plugin.h
new revision: 1.32; previous revision: 1.31
done
Checking in ldap/servers/slapd/snmp_collator.c;
/cvs/dirsec/ldapserver/ldap/servers/slapd/snmp_collator.c,v  <--  snmp_collator.c
new revision: 1.15; previous revision: 1.14
done
Checking in ldap/servers/slapd/stubrepl.c;
/cvs/dirsec/ldapserver/ldap/servers/slapd/stubrepl.c,v  <--  stubrepl.c
new revision: 1.7; previous revision: 1.6
done
Checking in ldap/servers/slapd/unbind.c;
/cvs/dirsec/ldapserver/ldap/servers/slapd/unbind.c,v  <--  unbind.c
new revision: 1.6; previous revision: 1.5
done
Checking in ldap/servers/slapd/value.c;
/cvs/dirsec/ldapserver/ldap/servers/slapd/value.c,v  <--  value.c
new revision: 1.8; previous revision: 1.7
done
Checking in ldap/servers/slapd/back-ldbm/back-ldbm.h;
/cvs/dirsec/ldapserver/ldap/servers/slapd/back-ldbm/back-ldbm.h,v  <--  back-ldbm.h
new revision: 1.18; previous revision: 1.17
done
Checking in ldap/servers/slapd/back-ldbm/ldbm_delete.c;
/cvs/dirsec/ldapserver/ldap/servers/slapd/back-ldbm/ldbm_delete.c,v  <--  ldbm_delete.c
new revision: 1.9; previous revision: 1.8
done
Checking in ldap/servers/slapd/back-ldbm/ldbm_modrdn.c;
/cvs/dirsec/ldapserver/ldap/servers/slapd/back-ldbm/ldbm_modrdn.c,v  <--  ldbm_modrdn.c
new revision: 1.9; previous revision: 1.8
done
Checking in ldap/servers/slapd/back-ldbm/misc.c;
/cvs/dirsec/ldapserver/ldap/servers/slapd/back-ldbm/misc.c,v  <--  misc.c
new revision: 1.7; previous revision: 1.6
done
Checking in ldap/servers/slapd/slapi_counter.c;
/cvs/dirsec/ldapserver/ldap/servers/slapd/slapi_counter.c,v  <--  slapi_counter.c
initial revision: 1.1
done
RCS file: /cvs/dirsec/ldapserver/ldap/servers/slapd/slapi_counter_sunos_sparcv9.il,v
done
Checking in ldap/servers/slapd/slapi_counter_sunos_sparcv9.il;
/cvs/dirsec/ldapserver/ldap/servers/slapd/slapi_counter_sunos_sparcv9.il,v  <--  slapi_counter_sunos_sparcv9.il
initial revision: 1.1
done

Comment 27 Noriko Hosoi 2008-10-18 23:58:04 UTC
Created attachment 320774 [details]
cvs diffs ldap/servers/slapd/{slapi_counter.c,slapi_counter_sunos_sparcv9.il}

It turned out the Solaris specific code had some problems.
slapi_counter.c: 
1) the inline function cannot be called from 2 or more places since the inline code contains a tag which is allowed only once in a file. Replaced _sparcv9_AtomicSet_il with slapi_counter_set_value.
2) added inline function declarations.
slapi_counter_sunos_sparcv9.il:
1) added membar to make sure previous load and store are finished.
2) replaced 32-bit ld with ldx
3) removed retl which is not needed for the inline function

Comment 28 Noriko Hosoi 2008-10-21 05:51:08 UTC
Created attachment 320975 [details]
Solaris: failed to let automake process *.s/*.S

Comment 29 Noriko Hosoi 2008-10-22 19:13:17 UTC
Created attachment 321198 [details]
cvs diff Makefile.am configure.ac ldap/servers/slapd/slapi_counter.c

Change description:
1) Makefile.am: instead of the inline assembly langauge file .il, include an independent .S file to the libslapd_la_SOURCES list.
2) add AM_PROG_AS to configure.ac to accept CCAS and CCASFLAGS.
3) slapi_counter.c: adjusted to slapi_counter_sunos_sparcv9.S.

Comment 30 Noriko Hosoi 2008-10-22 19:15:47 UTC
Created attachment 321199 [details]
New file: ldap/servers/slapd/slapi_counter_sunos_sparcv9.S

Comment 31 Noriko Hosoi 2008-10-22 19:20:48 UTC
Created attachment 321201 [details]
svn diff pkgs/redhat-ds-base/redhat-ds-base.spec

Change description: added CCAS=cc CCASFLAGS="-D_ASM -D__STDC__=0 $arg64" to let configure know which compiler (asssembler) and its flags need to be used to process the assembly language code.

Comment 32 Noriko Hosoi 2008-10-22 20:35:12 UTC
Created attachment 321208 [details]
cvs commit messages (comment#29,30,31)

Reviewed by Rich and Nathan (Thank you!!!)

Checked in into CVS HEAD as well as SVN trunk.

Comment 33 Nathan Kinder 2008-10-24 21:45:41 UTC
Created attachment 321467 [details]
Diffs (final phase)

These diffs convert the rest of the performance counters over to use the new Slapi_Counter API.  In addition, Noriko also added a configuration setting that allows the non-critical counters to be disabled for improved performance.  The build has also enabled the new 64-bit atomic increment code.  The SNMP sub-agent was updated to use new Counter64 types as well.

Comment 34 Nathan Kinder 2008-10-24 22:33:28 UTC
Created attachment 321475 [details]
Revised Diffs (final phase)

Noriko pointed out that the check if counters are enabled in snmp_collator_init() is redundant since we never end up calling snmp_collator_init() when counters are disabled.  This revised set of diffs simply cleans this up.

Comment 35 Nathan Kinder 2008-10-24 22:38:45 UTC
Checked into ldapserver (HEAD).  Thanks to Noriko for the review (and code)!

Checking in config.h.in;
/cvs/dirsec/ldapserver/config.h.in,v  <--  config.h.in
new revision: 1.24; previous revision: 1.23
done
Checking in configure;
/cvs/dirsec/ldapserver/configure,v  <--  configure
new revision: 1.96; previous revision: 1.95
done
Checking in configure.ac;
/cvs/dirsec/ldapserver/configure.ac,v  <--  configure.ac
new revision: 1.55; previous revision: 1.54
done
Checking in ldap/servers/slapd/add.c;
/cvs/dirsec/ldapserver/ldap/servers/slapd/add.c,v  <--  add.c
new revision: 1.15; previous revision: 1.14
done
Checking in ldap/servers/slapd/agtmmap.c;
/cvs/dirsec/ldapserver/ldap/servers/slapd/agtmmap.c,v  <--  agtmmap.c
new revision: 1.12; previous revision: 1.11
done
Checking in ldap/servers/slapd/agtmmap.h;
/cvs/dirsec/ldapserver/ldap/servers/slapd/agtmmap.h,v  <--  agtmmap.h
new revision: 1.12; previous revision: 1.11
done
Checking in ldap/servers/slapd/bind.c;
/cvs/dirsec/ldapserver/ldap/servers/slapd/bind.c,v  <--  bind.c
new revision: 1.17; previous revision: 1.16
done
Checking in ldap/servers/slapd/compare.c;
/cvs/dirsec/ldapserver/ldap/servers/slapd/compare.c,v  <--  compare.c
new revision: 1.9; previous revision: 1.8
done
Checking in ldap/servers/slapd/connection.c;
/cvs/dirsec/ldapserver/ldap/servers/slapd/connection.c,v  <--  connection.c
new revision: 1.22; previous revision: 1.21
done
Checking in ldap/servers/slapd/defbackend.c;
/cvs/dirsec/ldapserver/ldap/servers/slapd/defbackend.c,v  <--  defbackend.c
new revision: 1.7; previous revision: 1.6
done
Checking in ldap/servers/slapd/delete.c;
/cvs/dirsec/ldapserver/ldap/servers/slapd/delete.c,v  <--  delete.c
new revision: 1.10; previous revision: 1.9
done
Checking in ldap/servers/slapd/fe.h;
/cvs/dirsec/ldapserver/ldap/servers/slapd/fe.h,v  <--  fe.h
new revision: 1.10; previous revision: 1.9
done
Checking in ldap/servers/slapd/globals.c;
/cvs/dirsec/ldapserver/ldap/servers/slapd/globals.c,v  <--  globals.c
new revision: 1.9; previous revision: 1.8
done
Checking in ldap/servers/slapd/init.c;
/cvs/dirsec/ldapserver/ldap/servers/slapd/init.c,v  <--  init.c
new revision: 1.8; previous revision: 1.7
done
Checking in ldap/servers/slapd/libglobs.c;
/cvs/dirsec/ldapserver/ldap/servers/slapd/libglobs.c,v  <--  libglobs.c
new revision: 1.28; previous revision: 1.27
done
Checking in ldap/servers/slapd/libslapd.def;
/cvs/dirsec/ldapserver/ldap/servers/slapd/libslapd.def,v  <--  libslapd.def
new revision: 1.19; previous revision: 1.18
done
Checking in ldap/servers/slapd/main.c;
/cvs/dirsec/ldapserver/ldap/servers/slapd/main.c,v  <--  main.c
new revision: 1.27; previous revision: 1.26
done
Checking in ldap/servers/slapd/modify.c;
/cvs/dirsec/ldapserver/ldap/servers/slapd/modify.c,v  <--  modify.c
new revision: 1.18; previous revision: 1.17
done
Checking in ldap/servers/slapd/modrdn.c;
/cvs/dirsec/ldapserver/ldap/servers/slapd/modrdn.c,v  <--  modrdn.c
new revision: 1.11; previous revision: 1.10
done
Checking in ldap/servers/slapd/monitor.c;
/cvs/dirsec/ldapserver/ldap/servers/slapd/monitor.c,v  <--  monitor.c
new revision: 1.8; previous revision: 1.7
done
Checking in ldap/servers/slapd/proto-slap.h;
/cvs/dirsec/ldapserver/ldap/servers/slapd/proto-slap.h,v  <--  proto-slap.h
new revision: 1.40; previous revision: 1.39
done
Checking in ldap/servers/slapd/result.c;
/cvs/dirsec/ldapserver/ldap/servers/slapd/result.c,v  <--  result.c
new revision: 1.15; previous revision: 1.14
done
Checking in ldap/servers/slapd/search.c;
/cvs/dirsec/ldapserver/ldap/servers/slapd/search.c,v  <--  search.c
new revision: 1.11; previous revision: 1.10
done
Checking in ldap/servers/slapd/slap.h;
/cvs/dirsec/ldapserver/ldap/servers/slapd/slap.h,v  <--  slap.h
new revision: 1.38; previous revision: 1.37
done
Checking in ldap/servers/slapd/slapi-private.h;
/cvs/dirsec/ldapserver/ldap/servers/slapd/slapi-private.h,v  <--  slapi-private.h
new revision: 1.28; previous revision: 1.27
done
Checking in ldap/servers/slapd/snmp_collator.c;
/cvs/dirsec/ldapserver/ldap/servers/slapd/snmp_collator.c,v  <--  snmp_collator.c
new revision: 1.16; previous revision: 1.15
done
Checking in ldap/servers/slapd/back-ldbm/back-ldbm.h;
/cvs/dirsec/ldapserver/ldap/servers/slapd/back-ldbm/back-ldbm.h,v  <--  back-ldbm.h
new revision: 1.19; previous revision: 1.18
done
Checking in ldap/servers/slapd/back-ldbm/cache.c;
/cvs/dirsec/ldapserver/ldap/servers/slapd/back-ldbm/cache.c,v  <--  cache.c
new revision: 1.7; previous revision: 1.6
done
Checking in ldap/servers/snmp/ldap-agent.c;
/cvs/dirsec/ldapserver/ldap/servers/snmp/ldap-agent.c,v  <--  ldap-agent.c
new revision: 1.13; previous revision: 1.12
done
Checking in ldap/servers/snmp/ldap-agent.h;
/cvs/dirsec/ldapserver/ldap/servers/snmp/ldap-agent.h,v  <--  ldap-agent.h
new revision: 1.11; previous revision: 1.10
done
Checking in ldap/servers/snmp/main.c;
/cvs/dirsec/ldapserver/ldap/servers/snmp/main.c,v  <--  main.c
new revision: 1.16; previous revision: 1.15
done
Checking in ldap/servers/snmp/redhat-directory.mib;
/cvs/dirsec/ldapserver/ldap/servers/snmp/redhat-directory.mib,v  <--  redhat-directory.mib
new revision: 1.4; previous revision: 1.3
done

Comment 36 Noriko Hosoi 2008-10-24 23:31:28 UTC
Created attachment 321479 [details]
cvs diff ldap/servers/slapd/back-ldbm/proto-back-ldbm.h

Checking in the header file for the function proto type (cache_get_stats).

Resolves: 207457
Summary: Convert counters to 64-bit capable Slapi_Counter type.
CVS: ----------------------------------------------------------------------
CVS: Modified Files:
CVS:    proto-back-ldbm.h
CVS: ----------------------------------------------------------------------
Checking in proto-back-ldbm.h;
/cvs/dirsec/ldapserver/ldap/servers/slapd/back-ldbm/proto-back-ldbm.h,v  <--  proto-back-ldbm.h
new revision: 1.16; previous revision: 1.15
done

Comment 37 Noriko Hosoi 2008-10-27 18:32:36 UTC
Created attachment 321640 [details]
cvs diff ldap/servers/slapd/snmp_collator.c

Description: added '/' at the head of the semaphore name.

man sem_open (HP-UX)
           [EINVAL]       The name argument does not begin with "/" or
                          contains "." or ".." as a pathname component.
man sem_open (Solaris)
     The first character of  name must be a
     slash   (/)  character and the remaining characters of  name
     cannot include any slash characters.

Comment 38 Noriko Hosoi 2008-10-28 00:20:52 UTC
Created attachment 321670 [details]
cvs commit message (snmp_collator.c)

Reviewed by Nathan (Thanks!)
Checked in into CVS HEAD.

Comment 39 Nathan Kinder 2008-10-29 16:53:40 UTC
Created attachment 321821 [details]
Atomic functions for platforms without builtins

The previous patches did not work properly for all platforms.  On our supported Linux platforms, we have the following situations to deal with:

  - Have gcc builtins with 64-bit atomic implementations (RHEL5 x86-64)
  - Have gcc builtins, but without 64-bit atomic implementations (RHEL5 x86)
  - Don't have gcc builtins (RHEL4 x86 and x86-64)

To deal with all of these situations, we need to determine if we're on an x86 or x86-64 system at configure time.  We also need to determine if the gcc atomic builtins are available.

If the builtins are available and we're on x86-64 hardware, we can use the builtins (as done in the previous patches).  If we have the builtins, but we're on x86 hardware, we need to implement our own atomic functions for 64-bit values.  The builtins will call our functions automatically.  If we don't have the builtins available at all, we have to implement our own atomic functions and call them directly ourselves.

When using our own atomic functions, there is a minor difference in the inline assembly code that we need to use between x86 and x86-64.  On x86, the %ebx register is used as the PIC register, so we're not allowed to clobber it.  Unfortunately, the cmpxchg8b instruction requires us to use %ebx for storing half of the new value.  To get around this, we push %ebx onto the stack at the beginning of our atomic functions and pop it back when we're done.  We don't have to do this on x86-64 hardware since PIC uses a different register.

Comment 40 Nathan Kinder 2008-10-29 19:22:00 UTC
Checked diffs from comment#39 into ldapserver (HEAD).  Thanks to Rich and Noriko for their reviews!

Checking in config.h.in;
/cvs/dirsec/ldapserver/config.h.in,v  <--  config.h.in
new revision: 1.25; previous revision: 1.24
done
Checking in configure;
/cvs/dirsec/ldapserver/configure,v  <--  configure
new revision: 1.97; previous revision: 1.96
done
Checking in configure.ac;
/cvs/dirsec/ldapserver/configure.ac,v  <--  configure.ac
new revision: 1.56; previous revision: 1.55
done
Checking in ldap/servers/slapd/result.c;
/cvs/dirsec/ldapserver/ldap/servers/slapd/result.c,v  <--  result.c
new revision: 1.16; previous revision: 1.15
done
Checking in ldap/servers/slapd/slapi_counter.c;
/cvs/dirsec/ldapserver/ldap/servers/slapd/slapi_counter.c,v  <--  slapi_counter.c
new revision: 1.4; previous revision: 1.3
done
Checking in ldap/servers/snmp/ldap-agent.h;
/cvs/dirsec/ldapserver/ldap/servers/snmp/ldap-agent.h,v  <--  ldap-agent.h
new revision: 1.12; previous revision: 1.11
done

Comment 41 Nathan Kinder 2008-10-30 18:40:44 UTC
Created attachment 321972 [details]
Additional diff to fix 32-bit platforms with builtins

It turns out that the __sync_*_8 functions can not be declared as static functions, otherwise we run into built-time problems on x86 platforms that have knowledge of the built-ins.  What is odd is that it works fine with debug builds, but optimized builds will fail.  Simply removing the "static" specifier fixes the issue.

Comment 42 Nathan Kinder 2008-10-30 19:15:46 UTC
Checked patch from comment#41 into ldapserver (HEAD).  Thanks to Rich for his review!

Checking in ldap/servers/slapd/slapi_counter.c;
/cvs/dirsec/ldapserver/ldap/servers/slapd/slapi_counter.c,v  <--  slapi_counter.c
new revision: 1.5; previous revision: 1.4
done

Comment 43 Noriko Hosoi 2008-11-11 21:32:06 UTC
Created attachment 323261 [details]
cvs diff cache.c

Description: e->ep_refcnt should have been protected by cache->c_mutex, otherwise it breaks the lru list under the stress.

(1 line fix) * 3

Resolves: #207457
Summary: (64bitcounters) rhds 7.1 - server stats use 32-bit integers - entrycachehitratio 1503%
Description: additional fix for #207457; e->ep_refcnt should have been 
protected by cache->c_mutex, otherwise it breaks the lru list under the stress.
CVS: ----------------------------------------------------------------------
CVS: Enter Log.  Lines beginning with `CVS:' are removed automatically
CVS: Modified Files:
CVS:    cache.c
CVS: ----------------------------------------------------------------------
Checking in cache.c;
/cvs/dirsec/ldapserver/ldap/servers/slapd/back-ldbm/cache.c,v  <--  cache.c
new revision: 1.8; previous revision: 1.7
done

Comment 44 Nathan Kinder 2008-11-12 00:12:57 UTC
Created attachment 323277 [details]
Assembly offset two-liner patch

I found on my RHEL4 x86 system that the atomic increment was actually making the value jump by random amounts instead of by 1, despite a test program with the same inline assembly code behaving correctly.  It turns out that when building with gcc3, the offset from %ebp for the passed in "addval" or "subval" parameter is positive, where it is negative when building with gcc4.  This results in the upper half of "addval" or "subval" being grabbed from the wrong location in memory, which results in the value being wrong.  Here's an example of what was found:

  Inline ASM:
    movl %2, %%ebx;
    movl 4%2, %%ecx;

  Generated ASM code:
    movl 12(%ebp), %ebx;
    movl 412(%ebp), %ecx

The compiler knows that the passed in parameter is at an offset of 12 from the base pointer, so it just inserts the "12" where needed, but this concatenates with our explicit offset of "4", resulting in an incorrect offset of "412".  With gcc4, the offset is negative resulting in a calculated offset that works.

The solution is to put a "+" after our explicit offset of "4", resulting in the offset being calculated regardless of the gcc added offset being positive or negative.  It should be noted that a warning would be generated if there was not another offset after the "+", but we will always have an offset from the base pointer since "addval" and "subval" are parameters that were passed into our atomic functions.  The other lines of inline assembly where we use an explicit offset of "4" are for variables that were not passed into the function directly (local or struct members), so they will have no offset added by gcc.

Comment 45 Nathan Kinder 2008-11-12 16:58:40 UTC
Checked in patc from comment#44 to ldapserver (HEAD).  Thanks to Noriko for her review!

Checking in ldap/servers/slapd/slapi_counter.c;
/cvs/dirsec/ldapserver/ldap/servers/slapd/slapi_counter.c,v  <--  slapi_counter.c
new revision: 1.6; previous revision: 1.5
done

Comment 46 Nathan Kinder 2008-11-21 17:06:18 UTC
One change was missed in the patch from comment#44.  We need to specify the offset the same way in the assembly code in the slapi_counter_set_value() function for the passed in newvalue parameter.

Index: ldap/servers/slapd/slapi_counter.c
===================================================================
RCS file: /cvs/dirsec/ldapserver/ldap/servers/slapd/slapi_counter.c,v
retrieving revision 1.6
diff -u -5 -t -r1.6 slapi_counter.c
--- ldap/servers/slapd/slapi_counter.c  12 Nov 2008 16:58:06 -0000      1.6
+++ ldap/servers/slapd/slapi_counter.c  21 Nov 2008 17:00:13 -0000
@@ -304,11 +304,11 @@
         /* Put value of counter->value in EDX:EAX */
         "retryset: movl %0, %%eax;"
         " movl 4%0, %%edx;"
         /* Put newval in ECX:EBX */
         " movl %1, %%ebx;"
-        " movl 4%1, %%ecx;"
+        " movl 4+%1, %%ecx;"
         /* If EDX:EAX and counter-> are the same,
          * replace *ptr with ECX:EBX */
         " lock; cmpxchg8b %0;"
         " jnz retryset;"
 #ifdef CPU_x86

Checked into ldapserver (HEAD) under the one-line fix rule.

Checking in ldap/servers/slapd/slapi_counter.c;
/cvs/dirsec/ldapserver/ldap/servers/slapd/slapi_counter.c,v  <--  slapi_counter.c
new revision: 1.7; previous revision: 1.6
done

Comment 47 Jenny Severance 2009-04-01 20:41:15 UTC
closing bug - 64 bit counters now implemented.

Comment 48 Chandrasekar Kannan 2009-04-29 22:59:15 UTC
An advisory has been issued which should help the problem
described in this bug report. This report is therefore being
closed with a resolution of ERRATA. For more information
on therefore solution and/or where to find the updated files,
please follow the link below. You may reopen this bug report
if the solution does not work for you.

http://rhn.redhat.com/errata/RHEA-2009-0455.html


Note You need to log in before you can comment on or make changes to this bug.