On 17 September 2013 10:36, Martin Decky <[email protected]> wrote: > sleeper.h is used to implement the pipe queue. It's been a while since I >> implemented it, but I think the reason was that it's impossible (or at >> least very difficult) to implement the same structure using stuff from >> fibril_synch.h >> > > Could you please elaborate on this? The pipe queue is essentially a > producer/consumer (maybe with some twists, but still probably nothing > unheard of) and you are saying that it is very difficult to implement it > using standard synchronization primitives? How come > lib/c/generic/adt/prodcons.c is some 40 lines of code? > > Some of the reasons: In pipe, both readers and writers must wait for its counterpart. Additionally, write must restart if corresponding read fails, and in such case it should be returned directly to the head of the writers queue (not possible with condvar), otherwise data would get out of order. In short, pipe does not work as prodcons.
> As for atomics, I use them in places where lock-based synchronization >> would be too awkward for my taste. >> > > For my taste, your sleep and wakeup are very low-level operations, > suitable for some reusable general-purpose data structure or as building > blocks for high-level synchronization primitives. But probably not so great > for the direct use in target code. It is used only for implementation of pipe, which can be viewed as a high-level synchronization primitive. > > > Off topic: >> I am fairly certain you could elegantly implement all of >> fibril_synch.h (or any other synchronization mechanism) just by using >> futex and sleeper, without directly touching internal details of fibril >> and async framework. >> > > Of course, using sleep and wakeup to implement synchronization primitives > is one of the several possible ways to do it. However, it is hard to to > judge in general whether it is the best way to do it or not. That depends > on the actual implementation. > > If you accept the challenge, feel free to reimplement fibril_synch using > sleep and wakeup. But having custom synchronization means in a single > server is probably not the best thing to do. > I certainly accept. > The relative complexity of the current fibril_synch is mostly due to > optimizations, support for timeouts, etc. > > It's not so much the raw algorithmic complexity, as the use of fibril/async internals, that is bothering me. I think if we bother to separate high-level synchronization from the rest of the fibril library, it should be separated properly, without depending on how fibrils are implemented. > > Because list_t feels too obscure to me. To me it's much easier to write >> linked list manipulation in-line, and I think the resulting code is >> clearer as well. >> > > I have to strongly back Jakub here. Reimplementing the same pattern over > and over despite having a perfectly reusable generic implementation at hand > really provides no benefits. > > As I answered to Jakub. I will provide a prototype of subjectively better list library as well. In any case, I should be able to differentiate failure to allocate >> resources and failure of operation. async functions return ENOENT in case >> of NULL exchange, which in case of VFS is very inappropriate. >> > > The ENOENT return value from async functions is nothing set into stone, a > dedicated error code is more suitable here. Thanks for drawing my attention > to it. > > But the general point is that async_exchange_begin() is a function with a > no-fail semantics (similar to mutex_lock(), etc.). Meaning that you don't > want to have a dedicated error path in the target code every time you use > it. Since techically async_exchange_begin() can fail, it's up to the other > async_* functions to handle it gracefully. > > Yes, I can see how it simplifies stuff. The documentation of async_exchange_begin() should be changed to reflect this. I.e. instead of saying "returns NULL on error", it should say "NULL is considered a valid exchange and the rest of the framework handles this value properly". -- Jirka Z.
_______________________________________________ HelenOS-devel mailing list [email protected] http://lists.modry.cz/listinfo/helenos-devel
