openldap-bugs

openldap-bugs@openldap.org

1 participants
20959 discussions

(ITS#6437) sl_malloc issues
by h.b.furuseth＠usit.uio.no 01 Jan '10

01 Jan '10

Full_Name: Hallvard B Furuseth Version: HEAD OS: Linux x86_64 URL: Submission from: (NULL) (129.240.6.233) Submitted by: hallvard Here are a number of bugs and other issues with slapd/sl_malloc.c, and one for ber_memcalloc. Some for RE24, likely some for RE25, but I don't know where to cut off for RE24? (Waiting for someone to say...) * It claims to align on 2*sizeof(int) boundaries, but it looks like stack:realloc and pool:mem_create can un-align to (2n+1)*sizeof(int) boundaries on 32-bit hosts, where sizeof(ber_len_t) == sizeof(int). A slap_sl_realloc() fix for the stack implementation is to replace size += pad + sizeof( ber_len_t ); size &= ~pad; with size = ((size + pad) & ~pad) + sizeof( ber_len_t ); however see the optimizations and tweaks below. For the pool implementation, maybe slap_sl_mem_create() can just set so->so_ptr = (char *) sh->sh_base + sizeof(ber_len_t) % (pad+1); Unless that messes up its expectations of how much space is left, I haven't checked. Should mem_create size be increased accordingly? Unfortunately sl_malloc.c exports its internals, so I don't know what kind of fix would affect it: * Why is there a slap_sl_mem_detach() when there is no way to reattach it? As far as I can tell all the caller can do is to inspect/clean up the returned slab_heap itself. * slap_sl_mem_create() switching implementation stack<->pool on an old memctx slab can crash or leak, it runs the wrong cleanup code. Fix: Remove code duplication, rearrange. The pool cleanup duplicates slap_sl_mem_destroy(), except the final frees. Let destroy(key=NULL,) mean don't free and call that. Also the create/destroy pool branches duplicate the stack branches. * However, will the pool (i.e. non-stack) implementation ever be used? If not, a simpler fix to some of this is to delete it. OTOH if it someday may be used, hopefully some of these cleanups will be leaving it a bit more correct, easier to read, and faster. Though I haven't gone to the drastic step of studying it to see how it works. Or we could #ifdef it out by default. It's unclear to me why this is a run-time argument to the create function instead of instead of a -D<compiler flag>, why would the caller care? #define SLAP_SLAB_MODE <undef or 1:stack, 0:pool: -1:support both>. #define SLAP_SLAB_ISSTACK(stack) <stack if both, else SLAP_SLAB_MODE> * Regarding code duplication, the #ifdef SLAP_NO_SL_MALLOC code duplicates the !ctx code. Make that if(<enum No_sl_malloc> && !ctx) and the #ifdefs can be killed. * Also we can throw the #ifdef NO_THREADS/#else/#endif code out to two macros #define GET_MEMCTX()/SET_MEMCTX(). * The "enough room?" test (ptr + size < endptr) is invalid, can e.g. wrap around, when there is not enough room. Use (size < endptr-ptr). * Some failures assert(0)/exit() like ch_malloc.c, but without giving a Debug() message first. * The slap_sl_malloc() pool implementation can return NULL on failure at comment "FIXME: missing return; guessing...". For consistency it should exit on failure like the rest of sl_malloc.c and ch_malloc.c. * Realloc(last block) when the following free space is not big enough, does not move sh_last and reclaim free space properly. * slap_sl_calloc()/ber_memcalloc() do not catch <count>*<size> overflow: ber_len_t total = n * size; if (!n && total/n == size) <allocate...>; That can be optimized a bit, if anyone cares: #define LIM_SQRT(t) /* some value < sqrt(max value of unsigned type t) */ \ ((0UL|(t)-1) >>31>>31 > 1 ? ((t)1 <<32) - 1 : \ (0UL|(t)-1) >>31 ? 65535U : (0UL|(t)-1) >>15 ? 255U : 15U) ... /* sqrt test = slight optimization: often avoids the division */ if ((n | size) <= LIM_SQRT(ber_len_t) || !n || total/n == size) ... * slap_sl_<malloc/realloc> increase size before proceeding, they should reduce it before falling back to ch_malloc and for Debug messages. Space optimizations/cleanup: * To save space on 64-bit hosts: We can store store sizes (head, tail) in 4-byte typedef ber_uint_t slap_sl_size_t instead of a ber_len_t. After fixing the misalignment issue above, however. * Also the stack implementation does not need to allocate the tail. It can store the "block is freed" bit in the head of the _next_ block, and only store a tail in free blocks (i.e. if that bit is set). I think that does add a bit of code, though it is convenient for the "reclaim freed blocks at sh_last" code in slap_sl_free(). * Ignoring last point, the "reclaim" code can in any case be simplified: /* mark it free */ p[-1] = size |= 1; /* reclaim free space off tail */ if (sh->sh_last == p) { do { p = (slap_sllen_t*) ((char*) p - size + 1) - 1; size = p[-1]; } while (size & 1); sh->sh_last = p; } * Realloc can check if previous or next block is free before giving up and doing malloc/copy/free. That does multiply its code size though. Dunno if it's a good idea. Needs testing/timing, I guess. * Aligning with (size & ~pad) is wrong for non-two's complement signed pad, it should be (size & -(pad + 1)) aka (size & -Alignment). All right, so I'm nitpicking. I just dislike unportable code which isn't even simpler than the portable variant. * pad and order_start can be compile-time enums instead of computed at run time (if any compilers do not optimize it away already): enum { Align = 2*sizeof(int), Align_log2 = 1 + (Align>2) + (Align>4) + (Align>8) + (Align>16), pad = Align - 1, /* but drop that, it's just as easy to use Align */ order_start = Align_log2 /* which also can be dropped */ }; ... assert(Align == 1 << Align_log2); /* Adding head+tail preserves alignment */ assert(2*sizeof(ber_len_t) % Align == 0); * The pool code can be cleaned up by basing 'order' on Align-sized chunks instead of byte-sized chunks, i.e. subtract order_start from sh->sh_maxorder, order/i/j variables. Also subtract 1 from mem_create:order, so that 'order' means the same as 'order' elsewhere. (Straightforward replacement, no need to invoke the brain and understand the code:-) * The pool implementation sometimes does 1<<foo where it should do (ber_len_t)1<<foo or 1U<<foo.

1 0

Re: (ITS#4868) Binary Attribute Patch(es)
by hyc＠symas.com 30 Dec '09

30 Dec '09

vargok(a)yahoo.com wrote: > (I realize this is ridiculously late, but... well, better late than never, and this is the first I've gotten back to it.) > > Yes! It's working now in 2.4.x as expected, via the new userCertificate > stub data provided by example. FWIW, it appears to be working fine with the PostgreSQL example files as well. > > Unfortunately, it was never "back" ported to 2.3. We're currently using 2.3.43 as part of RHEL-5. Any chance it could get back-ported into the 2.3 branch? Development on 2.3 ended about a year ago. > Thanks, > Kevin > > > > ----- Original Message ---- >> From: Pierangelo Masarati<ando(a)sys-net.it> >> To: vargok(a)yahoo.com >> Cc: openldap-its(a)openldap.org >> Sent: Wed, April 11, 2007 7:06:57 PM >> Subject: Re: (ITS#4868) Binary Attribute Patch(es) >> >> ando(a)sys-net.it wrote: >> >>> Right. Time permitting, I'll check why back-sql behaves differently >>> and, in case, how to fix it. >> >> HEAD should be working as expected, now. I've also updated mysql and >> psql sql-test000 (the latter is untested, though). >> >> Please test and report. p. >> >> >> >> Ing. Pierangelo Masarati >> OpenLDAP Core Team >> >> SysNet s.r.l. >> via Dossi, 8 - 27100 Pavia - ITALIA >> http://www.sys-net.it >> --------------------------------------- >> Office: +39 02 23998309 >> Mobile: +39 333 4963172 >> Email: pierangelo.masarati(a)sys-net.it >> --------------------------------------- -- -- Howard Chu CTO, Symas Corp. http://www.symas.com Director, Highland Sun http://highlandsun.com/hyc/ Chief Architect, OpenLDAP http://www.openldap.org/project/

1 0

Re: (ITS#4868) Binary Attribute Patch(es)
by vargok＠yahoo.com 30 Dec '09

30 Dec '09

(I realize this is ridiculously late, but... well, better late than never, and this is the first I've gotten back to it.) Yes! It's working now in 2.4.x as expected, via the new userCertificate stub data provided by example. FWIW, it appears to be working fine with the PostgreSQL example files as well. Unfortunately, it was never "back" ported to 2.3. We're currently using 2.3.43 as part of RHEL-5. Any chance it could get back-ported into the 2.3 branch? Thanks, Kevin ----- Original Message ---- > From: Pierangelo Masarati <ando(a)sys-net.it> > To: vargok(a)yahoo.com > Cc: openldap-its(a)openldap.org > Sent: Wed, April 11, 2007 7:06:57 PM > Subject: Re: (ITS#4868) Binary Attribute Patch(es) > > ando(a)sys-net.it wrote: > > > Right. Time permitting, I'll check why back-sql behaves differently > > and, in case, how to fix it. > > HEAD should be working as expected, now. I've also updated mysql and > psql sql-test000 (the latter is untested, though). > > Please test and report. p. > > > > Ing. Pierangelo Masarati > OpenLDAP Core Team > > SysNet s.r.l. > via Dossi, 8 - 27100 Pavia - ITALIA > http://www.sys-net.it > --------------------------------------- > Office: +39 02 23998309 > Mobile: +39 333 4963172 > Email: pierangelo.masarati(a)sys-net.it > ---------------------------------------

1 0

Re: (ITS#6322) slapd suddenly stops working, and starts using 100% CPU
by helge＠monsternett.no 30 Dec '09

30 Dec '09

On Wed, Dec 30, 2009 at 09:04:12AM +0100, Helge Milde wrote: >I just enabled trace loglevel, perhaps that might give us a clue on what's going on. Slapd just stopped working again. Here's the (annotated) log for viewing and/or downloading: http://www.pastie.org/761198.txt Does this give you any clues? Tell me if I can do anything else to help. -- Helge Milde, 69701808 www.monsternett.no

1 0

Re: (ITS#6322) slapd suddenly stops working, and starts using 100% CPU
by helge＠monsternett.no 30 Dec '09

30 Dec '09

On Tue, Dec 29, 2009 at 11:00:25AM -0800, Howard Chu wrote: >Sorry, your issue was closed since we hadn't received any feedback >from you in a couple of months. > >The last trace you sent shows nothing unusual, just a bunch of >threads all waiting to write on their respective connections. If the core dump backtrace wasn't helpful, then please tell me so; I thought I had given you sufficient feedback, and that you were working on it. >Have you tried the .21 release yet? No.. I might do that next week. Is there anything else that I can do? I just enabled trace loglevel, perhaps that might give us a clue on what's going on. >-- > -- Howard Chu > CTO, Symas Corp. http://www.symas.com > Director, Highland Sun http://highlandsun.com/hyc/ > Chief Architect, OpenLDAP http://www.openldap.org/project/ -- Helge Milde, 69701808 www.monsternett.no

1 0

(ITS#6247)
by daniel＠pluta.biz 29 Dec '09

29 Dec '09

Hi, I did just some small cleanups, but added no new functionality. I've extracted the minimalistic slapd-api modifications in an extra file called "now_slapd.patch" also contained in the tgz. The code compiles and runs against todays version of "HEAD". The new tgz-file can be downloaded from here: ftp://ftp.openldap.org/incoming/daniel-pluta-now-091229-1.tgz Cheers Daniel

1 0

Re: (ITS#6322) slapd suddenly stops working, and starts using 100% CPU
by hyc＠symas.com 29 Dec '09

29 Dec '09

helge(a)monsternett.no wrote: > Would be nice to get some feedback on this one.. > Were using a core dump sufficient? If so, was the information helpful at all? > If there's anything more I could do, please say so. > Sorry, your issue was closed since we hadn't received any feedback from you in a couple of months. The last trace you sent shows nothing unusual, just a bunch of threads all waiting to write on their respective connections. Have you tried the .21 release yet? -- -- Howard Chu CTO, Symas Corp. http://www.symas.com Director, Highland Sun http://highlandsun.com/hyc/ Chief Architect, OpenLDAP http://www.openldap.org/project/

1 0

Re: (ITS#6322) slapd suddenly stops working, and starts using 100% CPU
by helge＠monsternett.no 29 Dec '09

29 Dec '09

Would be nice to get some feedback on this one.. Were using a core dump sufficient? If so, was the information helpful at all? If there's anything more I could do, please say so. -- Helge Milde, 69701808 www.monsternett.no

1 0

Re: (ITS#6194) Patch - Enhancement - provide LDIF support as libldif
by hyc＠symas.com 23 Dec '09

23 Dec '09

rmeggins(a)redhat.com wrote: > The ldif code does not currently have knowledge of any of the higher > level LDAP data types such as LDAPMod or LDAPControl - some of the > proposals would have the ldif API return these objects - Do we really > want to introduce this higher level knowledge into the relatively low > level libldif, along with the dependency on libldap? I think in order > to do this properly, we would have to introduce a new LDAP API e.g. > ldap_mods_from_ldif(const char *ldifstr, LDAPMod***, LDAPControl***) or > something like that - something that would parse the given ldif record > into those higher level data types. Should have mentioned this sooner; this ITS needs to be merged with ITS#4033. -- -- Howard Chu CTO, Symas Corp. http://www.symas.com Director, Highland Sun http://highlandsun.com/hyc/ Chief Architect, OpenLDAP http://www.openldap.org/project/

1 0

Re: Re : Re : ITS#6434
by masarati＠aero.polimi.it 22 Dec '09

22 Dec '09

KISTER RAPHAEL wrote: > I re-test with servers/slapd/overlays/rwm.c in version 1.125 and it works fine. I don't test with the next version. Fine. The other modifications are not strictly related to your issue, but rather to modifications to olcRwmMap using back-config. p.

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

2010

2009

2008

2007

2006

openldap-bugs