On Tue, Oct 19, 2010 at 9:26 PM, Sage Weil <sage@xxxxxxxxxxxx> wrote: > - SimpleMessenger::read_message() just needs to call > ms_deliver_get_data_buffer() if data_len > 0. If it returns true, use the > provided bufferptr. > > - The existing Objecter::read() method returns the tid, which librados > can then store in a private map of tid -> bufferptrs. When it's > ms_get_data_buffer() is called it can provide the bufferptr; no need to > modify the Objecter at all! Just make sure to verify that the entry > exists in the map before using the operator[] (use find()). If we want thread-local data, we should at least use pthread_setspecific / pthread_getspecific. No need for tids and STL maps. gettid() is a syscall and it will be slow. > The main corner case is this: if there is some osd cluster change, it's > theoretically possible we could have two threads reading read replies to > the same request off the wire at the same time. Even if we carefully > define which reply "wins", we need to make sure there isn't a stray thread > read/accessing the buffer after the read() call returns. I just don't see why we have to care about threads at all. If we need rados client-specific data, it should definitely be in class RadosClient. Otherwise, we should just get rid of class RadosClient, because it's useless. > I'm not sure what the solution to that one is yet. The kclient doesn't > have separate threads, and takes pains to yank the pages out from under > incoming message; that approach doesn't work well with the SimpleMessenger > threading model. :/ I think I can propose a patch which solves this problem without rados client-specific data, threads, tids, maps, thread-local data, or changes to SimpleMessenger. It will just involve modifying bufferlist so that it can use a buffer that's passed to it, rather than allocating its own. Then we need something like a freeze operation that will stop it from trying to reallocate or resize the buffer. This solution will have much more generality and probably will be useful elsewhere too. Colin -- To unsubscribe from this list: send the line "unsubscribe ceph-devel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
