[sheepdog] [PATCH] use queue_request when resume waiting requests

Christoph Hellwig hch at infradead.org
Wed May 30 10:45:48 CEST 2012


Give me a bit more time for a real review and testing, but here's
a simple comment from a quick look over the code:

>  		list_del(&req->request_list);
> +		put_vnode_info(req->vnodes);
> +		queue_request(req);

This pattern happens so often that at least a requeue_request() helper
bounding these three calls would help.

> @@ -313,10 +312,14 @@ void resume_wait_epoch_requests(void)
>  			put_vnode_info(req->vnodes);
>  			req->vnodes = get_vnode_info();
>  			setup_access_to_local_objects(req);
> +			list_del(&req->request_list);
> +			process_io_request(req);

This also seems prettry equivalent to the (re)queue_request sequence.

>  			request_list) {
> +		if (req->local_oid != oid)
> +			continue;
> +
>  		/* the object requested by a pending request has been
>  		 * recovered, notify the pending request. */
> -		if (req->local_oid == oid) {
> -			dprintf("retry %" PRIx64 "\n", req->local_oid);
> -			list_del(&req->request_list);
> -			process_io_request(req);
> -		}
> +		dprintf("retry %" PRIx64 "\n", req->local_oid);
> +		list_del(&req->request_list);
> +		put_vnode_info(req->vnodes);
> +		queue_request(req);
>  	}

I think all these need the splite to a local list, then process pattern.
Without that we could add the request to the list currently walked and
create a busy loop.




More information about the sheepdog mailing list