[Sheepdog] [PATCH 1/2] sheep: cleanup request list handling

Christoph Hellwig hch at infradead.org
Tue May 15 08:55:13 CEST 2012


On Tue, May 15, 2012 at 02:39:45PM +0800, Liu Yuan wrote:
> On 05/11/2012 09:19 PM, Christoph Hellwig wrote:
> 
> > static void queue_request(struct request *req)
> >  {
> > -	struct event_struct *cevent = &req->cev;
> >  	struct sd_req *hdr = (struct sd_req *)&req->rq;
> >  	struct sd_rsp *rsp = (struct sd_rsp *)&req->rp;
> >  
> > +	assert(list_empty(&req->request_list));
> > +
> 
> 
> request is allocated in client_rx_handler() just before calling
> queue_request(), so I think this assertion is useless. We don't need
> this safe-guard because there is nothing to guard. This path is quit
> hot, so we'd better off only doing guard when necessary.

The old code removed it from a list that it was never added to, so
I placed it in here to verify the assumption that it never should be.

I'm fine to remove it, but we probably want more asserts like this at
least for builds with debugging enabled.  What do people think about
adding an ASSERT macro that is compiled away for non-debug builds?

I'd also like to port over the Linux kernel linked list debugging
(CONFIG_DEBUG_LIST) to the sheepdog copy of list.h for debug builds,
as that would catch errors like that, too.




More information about the sheepdog mailing list