#2657: Implement XSI shared memory <sys/shm.h> ---------------------------+------------------------------ Reporter: kaliber | Owner: emitrax Type: enhancement | Status: new Priority: normal | Milestone: R1 Component: System/POSIX | Version: R1/Development Resolution: | Keywords: Blocked By: | Has a Patch: 1 Platform: All | Blocking: ---------------------------+------------------------------ Comment (by bonefish): Replying to [comment:17 0xffea]: > i tried to correct the style issues and not to use areas in kernel space. The coding style looks much better, now. A few issues remain: * Inconsistent parameter naming in prototypes/implementations of various related functions: - `*shmctl()`: `mds`, `memoryDs`, `_buffer`, `buffer`. Unless you come up with a better fitting name, `buffer` is the best one so far. - `*shmat()`: `memoryPointer` -> `address`, `rAddress` -> `_returnAddress` - `*shmdt()`: `memoryPointer` -> `address` * Inconsistent pointer style in the `_kern_*()` prototypes. Since both styles are used in the file, either would be OK, but mixing in one block or even one function is not so nice. Preferred is `Foo* foo`. * In `syscalls.h`: `struct shmid_ds` is in wrong line. * In `xsi_shared_memory.cpp`: The `<kernel/...>` includes block is not sorted and the `kernel/` part should be omitted. The block should be joined with the `<vm/...>` and `<util/...>` includes blocks (both are kernel private includes). * `MAX_XSI_SHARED_MEMORY_SIZE` definition: Missing spaces between the operators and operands. Also, the whole expression should be put in parentheses to prevent potential operator precedence issues where the macro is expanded. * In both `*HashTableDefinition` classes: - Incorrect space after `HashKey`. - Superfluous `const` of parameter type in `HashKey()` and `Compare()`. - Superfluous parenthesis in `HashKey()` implementation. * There are still a few instances of implicit `(foo & bar)` to `bool` conversions. Also, `!(flags & IPC_CREAT)` should be `(flags & IPC_CREAT) == 0`. * Superfluous parenthesis in `if ((ipcKey->Size() < size) && (size != 0))` (`_user_xsi_shmget()`), besides that, due to `size_t` being unsigned, the second check is superfluous. * Incorrect `(void*)` cast spacing in `_user_xsi_shmat()` (2x). * Inconsistent pointer style in `xsi_shm.cpp`. > * create a cache with VMCacheFactory::CreateAnonymousCache > * use VMAddressSpace->CreateArea and VMAddressSpace->InsertArea to create the area in user space > > hope this is the right way to do it. not sure about the proper locking. Unfortunately things are quite a bit more complicated: * A `VMAddressSpace` object is reference counted and has a read/write lock. Holding the write lock is required for any manipulation operation (creating, deleting, resizing areas) and also for manipulating most members of the `VMArea` objects contained in the address space. Holding the read lock guarantees that the respective address space and area members don't change. * A `VMCache` object is reference counted and has a simple mutex lock. The lock must be held for almost all kinds of accesses to the object. * Locking order is: address space -> area cache -> source cache -> .... If multiple address spaces need to be locked, a `MultiAddressSpaceLocker` must be used. There are more locking helper classes in `src/system/kernel/vm/VMAddressSpaceLocking.h`. * To map a `VMCache` into an address space, creating an area, the function of choice is `map_backing_store()` in `vm/vm.cpp`. It gets the job done with all the bells and whistles (including address and alignment constraints, unmapping of previously mapped stuff, etc.). Only problem, it's currently local to that source file. A simple kernel-private wrapper (e.g. `vm_map_cache()`) could be introduced (prototype in `<vm/vm.h>`). * When creating an area, it must be marked `B_SHARED_AREA` or it will have copy-on-write behavior on `fork()`. Generally I would recommend having a look at `vm/vm.cpp`. There are quite a few functions that implement functionality similar to what you need. E.g. `vm_create_anonymous_area()` creates a cache and maps it. I haven't looked closely at the features the POSIX API requires, but it might be possible to avoid playing with `VMCache`s altogether by using memory-mapped files. That would allow for using a higher-lever API, probably simplifying things quite a bit. Regarding the tests: Actually using the shared memory is missing and is definitely something that should be tested, also with multiple processes. I believe the OpenPosixTestsuite in `src/tests/system/libroot/posix/posixtestsuite` also has XSI shared memory tests. Unfortunately some tests of the testing framework use functionality Haiku is lacking or doesn't implement correctly yet (`clock_*()`, signals), so the respective tests might not pass anyway, but it certainly doesn't harm to have a look. -- Ticket URL: <http://dev.haiku-os.org/ticket/2657#comment:18> Haiku <http://dev.haiku-os.org> Haiku - the operating system.