tech-kern archive
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]
Re: Support for "pshared" POSIX semaphores
> On Feb 2, 2019, at 10:33 AM, Mindaugas Rasiukevicius <rmind%netbsd.org@localhost> wrote:
>
> Jason Thorpe <thorpej%me.com@localhost> wrote:
>> Patch is here:
>> https://patch-diff.githubusercontent.com/raw/thorpej/netbsd-src/pull/5.diff
>
> Thanks for working on this.
>
> - Why not just add a new syscall, instead of the KSEM_PSHARED stuff?
Mainly because a new syscall is more intrusive, and harder to back-port. It’s a shame the existing system call didn’t have a way to pass flags up.
> - If you add the 'fd' parameter to ksem_release(), then you can move
> fd_putfile() there too (with fd != -1 case) and simplify a little bit.
I’ll take a look. There are a few spots where I think the pattern still needs to “leak” out of ksem_release().
> - ksem_alloc_pshared_id: can ~KSEM_MARKER_MASK range be exhausted as an
> attach vector?
You’d have to allocate a ton of pshared semaphores … it’s a 23-bit range, and that definitely exceeds the system max semaphores limit. I assign it randomly only to make it harder to guess that the in-use semaphore IDs are (mainly as a way to avoid bad behavior caused by buggy programs).
(As an aside, the POSIX semaphore API is … annoyingly underspecified — there’s a lot of room for implementation-defined behavior that the spec doesn’t even bother to call out as “implementation-defined”).
> - Can you eliminate ksem_insert_pshared_locked(), ksem_remove_pshared_locked()
> and ksem_remove_pshared() wrappers? They serve no purpose.
I initially did that just to hide the “what kind of data structure contains the objects” away from the main logic. I’ll go ahead and simplify, though.
> - ksem_lookup_pshared_locked: it makes sense to use a hash table here.
> Using hashinit() is clumsy nowadays.. personally, I would replace most
> of the subr_hash.c use cases with something like rhashmap [1] and much
> more convenient get/put/del semantics. Anyway, that is off-topic, so
> up to you if you want to bother.
It does make sense to use something else .. I thought about using an r-b tree. Given the default system limits, we’re talking about a small number of objects, but yah, I’ll use something better there.
-- thorpej
Home |
Main Index |
Thread Index |
Old Index