IRC.

2 files changed, 267 insertions, 0 deletions

diff --git a/open_issues/libpthread_pthread_key_create_reuse.mdwn b/open_issues/libpthread_pthread_key_create_reuse.mdwn
new file mode 100644
index 00000000..11c9a821
--- /dev/null
+++ b/open_issues/libpthread_pthread_key_create_reuse.mdwn
@@ -0,0 +1,47 @@
+[[!meta copyright="Copyright © 2011 Free Software Foundation, Inc."]]
+
+[[!meta license="""[[!toggle id="license" text="GFDL 1.2+"]][[!toggleable
+id="license" text="Permission is granted to copy, distribute and/or modify this
+document under the terms of the GNU Free Documentation License, Version 1.2 or
+any later version published by the Free Software Foundation; with no Invariant
+Sections, no Front-Cover Texts, and no Back-Cover Texts.  A copy of the license
+is included in the section entitled [[GNU Free Documentation
+License|/fdl]]."]]"""]]
+
+[[!meta title="libpthread: pthread_key_create, reuse"]]
+
+IRC, FreeNode, #hurd, 2011-07-02
+
+[[!tag open_issue_libpthread]]
+
+    < pinotree> hm, maybe i found a libpthread bug
+    * pinotree tries a testcase
+    < pinotree> yesssss, found the bug :)
+    < pinotree> youpi: it's a problem of the key reuse in pthread_key_create()
+    < youpi> it doesn't reset it?
+    < youpi> were you looking at the licq issue?
+    < pinotree> no, gtest
+    < youpi> k
+    < youpi> licq has a failing threadspecific issue
+    < youpi> [  FAILED  ] ThreadSpecificData.dataDeletedWhenThreadExits
+    < pinotree> basically, pthread_key_delete() does not delete the key values
+      from the "thread_specifics" ihash
+    < pinotree> but those were new keys, so i'm not sure it is allowed to
+      return values of previous keys?
+    < pinotree> after all, the actual key value is an implementation detail,
+      applications shouldn't care about it being reused
+    < pinotree> (imho)
+    < youpi> Upon key creation, the value NULL shall be associated with the new
+      key in all active threads.
+    < youpi> ok, so we have to clear it in all threads
+    < youpi> that's a pity
+    < pinotree> or just remove the entry from the hash on key removal
+    < youpi> pinotree: from all the hashes, you mean?
+    < pinotree> youpi: from how i see it, adding a snippet like
+      http://paste.debian.net/121690/ in pthread_key_delete() should do the job
+    < youpi> that only drops from the current thread
+    < pinotree> ah hm, other threads
+    < youpi> we need to drop from all  threads
+    < youpi> that's the pity part
+    < pinotree> youpi: the licq case could look like a similar issue, at a
+      veeery quick glance
diff --git a/open_issues/rework_gnumach_ipc_spaces.mdwn b/open_issues/rework_gnumach_ipc_spaces.mdwn
index b7cda227..f5d40abd 100644
--- a/open_issues/rework_gnumach_ipc_spaces.mdwn
+++ b/open_issues/rework_gnumach_ipc_spaces.mdwn
@@ -318,3 +318,223 @@ IRC, freenode, #hurd, 2011-04-23
     <antrik> braunr: idr?
     <braunr> antrik: a data structure used to map integers to pointers
     <braunr> http://fxr.watson.org/fxr/source/lib/idr.c?v=linux-2.6
+
+2011-06-18
+
+    < braunr> hmm, i'm having a problem with integrating my radix tree code in
+      gnumach
+    < braunr> inserting into such a tree can trigger memory allocation
+    < braunr> so commonly, the tree i loaded with nodes before insertion,
+      usually if it requires strong locking
+    < braunr> ipc spaces are locked using "simple locks" (which are spin locks)
+    < braunr> but spin locks are noops on UP, and gnumach is always UP ..
+    < braunr> so, should i still include preloading code, even if it'll end up
+      dead code ?
+    < antrik> hm... I think we discussed this before; but isn't gnumach
+      supposed to be SMP-capable, minus bugs?...
+    < braunr> it is
+    < braunr> but ofc, if i choose not to include preloading, i'll write
+      #errors so that the day gnumach is built for SMP again, such support will
+      be included
+    < antrik> oh, sorry, I think I misread. what is UP?
+    < braunr> uniprocessor
+    < antrik> well, if it's just bugs forcing the current UP state, I think
+      saying that gnumach is always UP is a stretch...
+    < braunr> sure, but it's a practical consideration
+    < antrik> does the locking complicate stuff? or is it just performance
+      considerations?
+    < braunr> no it's about correctness and completeness
+    < braunr> if you don't preload a tree before locking
+    < braunr> and memory allocation occurs while you're holding a simple lock
+    < braunr> and memory allocation requires the kernel to sleep
+    < braunr> you're screwed
+    < braunr> but i hate the idea of including code that won't be used and
+      which won't be easy to test
+    < braunr> so i'm wondering if it's ok for now to just put this in a TODO
+      comment and write it when the time is right
+    < braunr> or if i should spens the week adding this and tweaking the
+      userspace implementation to "emulate" spin locks
+    < antrik> well, it's tricky situation. on one hand, it seems stupid to
+      include handling for something that presently isn't used, and it's not
+      clear when it will. on the other hand, I'd rather not see additional
+      problems introduced that will make fixing SMP even harder...
+    < braunr> that's why i'm asking here
+    < antrik> of course, you could resolve this question by fixing SMP
+      first... ;-)
+    < braunr> ew
+    < antrik> well, I guess it would be best first to make the code work... and
+      we can still decide about the locking thing before it goes mainline I'd
+      say?
+    < braunr> "make the code work" ?
+    < antrik> I mean make gnumach work with your radix tree code
+    < braunr> without preloading then
+    < antrik> yeah... as a first step... I guess adding it later won't be any
+      harder than adding it right now?
+    < braunr> not much
+    < braunr> testing is what requires time really
+
+2011-06-27
+
+    < braunr> ok, here is the radix tree code:
+      http://git.sceen.net/rbraun/libbraunr.git/
+    < braunr> the preloading stuff will be added in the kernel only, as it's
+      really pointless and not easily doable in userspace
+    < youpi> preloading?
+    < braunr> youpi: yes, preloading
+    < braunr> radix trees allocate external nodes
+    < youpi> well, providing a url at some random time of some random day is
+      not a great way  to get eyes on it :)
+    < braunr> and ipc spaces are locked when inserting/allocating names
+    < braunr> we normally don't need preloading in gnumach
+    < braunr> since there is no preemption nor SMP
+    < braunr> but in case someone changes that, i'd like the code to be mostly
+      ready
+    < braunr> and correctly handle those ugly simple locks
+    < braunr> youpi: is what i say clear enough or do you need more background
+      on what is done ?
+    < youpi> about preloading?
+    < braunr> yes
+    < youpi> I guess it means allocating nodes in advance?
+    < braunr> yes
+    < youpi> k
+    < braunr> before locking the ipc spaces
+
+2011-06-28
+
+    < braunr> antrik: i think i won't write the code for the preloading stuff
+      actually
+    < braunr> antrik: it's not very difficult, but i really hate the idea of
+      not being able to reliably test it
+    < braunr> antrik: and i'd rather concentrate on integrating the radix tree
+      code in gnu mach now
+    < braunr> (i've already removed much code, even some files which weren't
+      actually used before my changes !)
+    < braunr> hmm, i won't be able not to write the preloading code after all
+    < antrik> braunr: not able not to write? how's that?
+    < braunr> antrik: it's actually required
+    < braunr> there are three functions, ipc_entry_get, ipc_entry_alloc, and
+      ipc_entry_grow_table
+    < braunr> ipc_entry_get cannot allocate memory
+    < braunr> if it fails, ipc_entry_grow_table is called, which will allocate
+      memory
+    < braunr> ipc_entry_alloc calls both of them depending on the result of
+      ipc_entry_get
+    < braunr> this is the equivalent of the preloading thing i had in mind
+    < braunr> not a bad thing after all
+    < braunr> the only thing i'm afraid of are the "optimized" version of those
+      ipc functions in te so-called fast paths
+    < braunr> i'm afraid if i don't deal right with those, the kernel may end
+      up using mostly slow paths
+    < braunr> but considering the purpose of those fast paths was merely to
+      avoid the overhead of function calls and some locking functions, it
+      shouldn't be that bad
+    < braunr> this is such a mess eh
+    < antrik> hurray microoptimisations ;-)
+    < braunr> there, the preload functions are done, easy :)
+    < antrik> braunr: seems you spent less time implementing it than pondering
+      whether you should implement it ;-)
+    < braunr> well, i couldn't implement it correctly before knowing what
+      should have been done exactly
+    < braunr> and there are still other problems :/
+    < braunr> and the other problems make me reconsider if this was useful at
+      all eh
+    < braunr> youpi: i'm unable to find where ipc tree entries are released
+      except in ipc_entry_alloc_name(), which could mean they're leaked ...
+    < braunr> youpi: would you have time to take a look ?
+    < youpi> they aren't in ipc_entry_dealloc() ?
+    < braunr> no .....
+    < youpi> it's not so unprobable that they're only freed when the task quits
+    < braunr> i don't see that either
+    < braunr> i only see them released in ipc_entry_alloc_name()
+    < braunr> so maybe they're reused
+    < braunr> but i'm not sure about that when reading the code
+    < braunr> oh wait, yes, they are :/
+    < braunr> my bad
+    < youpi> in the ipc_splay_tree_* fucntions I guess?
+    < braunr> yes
+    < braunr> it's just surprsing to see them allocated outside the tree code
+      only
+    < braunr> but released in both the entry and the splay tree code ...
+
+2011-06-29
+
+    < braunr> hmm i missed an important thing :/
+    < braunr> and it's scary
+    < braunr> it looks like the splay tree is mainly used when names are
+      provided
+    < braunr> whereas the entry table is used when names are allocated
+    < braunr> which means the table is the main ipc data structure, even for
+      tasks with lots of rights
+    < braunr> i can make my root ext2fs have more than 10k rights, and i see
+      the ipc table table grow along that number ...
+    < braunr> now thetable has 15k+ entries
+    < braunr> IOW there is no point to put the radix tree code in gnumach :(
+    < antrik> braunr: what do you mean by "provided" and "allocated"?
+    < antrik> and what is that table you are talking about?
+    < braunr> antrik: provided means the user space tasks gives the name of the
+      new right
+    < braunr> antrik: allocated means the kernel generates it
+    < braunr> antrik: the table i'm talking about is is_table in struct
+      ipc_space
+    < braunr>  55  *      Every space has a non-NULL is_table with
+      is_table_size entries.
+    < braunr>  56  *      A space may have a NULL is_tree.  is_tree_small
+      records the
+    < braunr>  57  *      number of entries in the tree that, if the table were
+      to grow
+    < braunr>  58  *      to the next larger size, would move from the tree to
+      the table.
+    < braunr> here is the description which mislead me (in addition of the
+      obscure code)
+    < braunr>  50  *      Spaces hold capabilities for ipc_object_t's (ports
+      and port sets).
+    < braunr>  51  *      Each ipc_entry_t records a capability.  Most
+      capabilities have
+    < braunr>  52  *      small names, and the entries are elements of a table.
+    < braunr>  53  *      Capabilities can have large names, and a splay tree
+      holds
+    < braunr>  54  *      those entries.  The cutoff point between the table
+      and the tree
+    < braunr>  55  *      is adjusted dynamically to minimize memory
+      consumption.
+    < antrik> ah, so the rights with a low name are in a linear table, and only
+      those with "random" high names are stored in the splay tree instead?
+    < antrik> seems a rather complex design... I guess though there isn't much
+      room for further optimisation there :-(
+    < antrik> (well, except for code size optimisation -- which could in fact
+      make a considerable difference...)
+    < braunr> well there are problems with this approach, but most don't
+      concern performance
+    < braunr> when the table gets big (close to the page size or more), it gets
+      remapped when reallocated
+    < braunr> which will incur some penalty because of the tlb
+    < braunr> but it's annoying even for small tables
+    < braunr> the initial table size is 4 entries, and from what i can see,
+      most tables are 128 entries wide when tasks are destroyed
+    < braunr> an obvious simple optimization is to set a larger default size
+    < braunr> the same applies for the dead name tables
+    < braunr> those reallocations are a pain, and they're due to this design
+    < braunr> they can also fail because of fragmentation
+    < braunr> there would be a point to radix trees if they would replace all
+      that, and not just the splay tree
+    < braunr> but this would cause a lot of changes in a lot of places, and in
+      particular the "optimized" fast paths i mentioned yesterday
+    < braunr> we'll see how they perform in x15 :>
+    < braunr> there is a slight noticeable improvement when increasing the
+      initial size of the entry table
+    < antrik> braunr: well, if you use them in a completely different
+      implementation, there will be no way of telling whether they make a
+      difference
+    < antrik> how did you test the improvement?
+    < braunr> antrik: no actually it's completely negligeable
+    < braunr> hm
+    < braunr> is that a valid word ? :)
+    < braunr> negligible
+    < braunr> youpi: did you see my comments about the ipc stuff this earlier
+      today ?
+    < braunr> youpi: well to make things short, when port names are allocated,
+      the right they refer to is allocated from the ipc table
+    < braunr> youpi: the splay tree is only used for user provided names
+    < braunr> youpi: i had tables as large as the number of rights in a space
+      (i could easily reach 20k)
+    < braunr> youpi: whereas the splay trees had at most ~40 entries ..