On 05/23/2012 11:08 PM, Christoph Hellwig wrote:
> Oh ok - this patch changes things enough that we can't do what I
> proposed in the previous mail.
>
>> + switch (req->rp.result) {
>> + case SD_RES_OLD_NODE_VER:
>> + /* retry as gateway. */
>> + req->rq.epoch = sys->epoch;
>> + put_vnode_info(req->vnodes);
>> + req->vnodes = get_vnode_info();
>> + setup_access_to_local_objects(req);
>
> Can you add a helper to share this code with gateway_op_done? Having
> this in a properly named helper also helps with self-documenting the
> code.
I didn't say gateway_op_done in your code, do you mean to split gateway_op_done
from io_op_done ?
>
>> + case SD_RES_NEW_NODE_VER:
>> + list_del(&req->request_list);
>> + list_add_tail(&req->request_list, &sys->request_queue);
>> + process_request_event_queues();
>> + break;
>> + default:
>
> We're probably still better off calling process_request_event_queues
> once after the loop instead of possibly starting to queue and complete
> I/O from inside the loop. resume_pending_requests already does that
> variant, although I think it has a bug because it only checks for a
> non-emptry request_queue but not the event queue.
Calling process_request_event_queue outside the loop seems good to me,
but I don't think it's necessary to call resume_pending_requests instead of
process_request_event_queue, resume_pending_requests move the requests in
req_wait_for_obj_list into the sys->request_queue, I don't think it's always
necessary.
thanks,
levin
|
|