[Drbd-dev] DRBD-8 (My) first problem with new requests handlingcode...

Graham, Simon Simon.Graham at stratus.com
Mon Sep 11 17:32:45 CEST 2006


> and, as the comment at the head of _req_mod says: we probably should
> split the case branches out into "STATIC" functions, and only have the
> _req_mod dispatch, or even remove the _req_mod switch statement again,
> and only have the branch functions...
> 
> otherwise, what do you thin/have found out about that monster patch?
> 

Well, since I haven't been able to boot my system yet I don't have any
real experience yet but I really like the idea of centralizing the
request handling -- if the code for the various cases could be moved
into separate routines in drbd_req.c, I think you COULD revert to an
inline top-level function containing just the case statement and the
calls to the various non-inline functions -- the compiler ought to be
able to nicely optimize this and only include the specific case(s) for
the type argument which would translate to direct calls to the per-case
routines...

I see you committed a change to the problem I reported and also to
remove the inline of the function for which I thank you!

BTW - there is another case of accessing a NULL bio pointer in the
read_completed_with_error case:

+	case read_completed_with_error:
+		drbd_set_out_of_sync(mdev,req->sector,req->size);
+		req->rq_state |= RQ_LOCAL_COMPLETED;
+		req->rq_state &= ~RQ_LOCAL_PENDING;
+
+		bio_put(req->private_bio);
+		req->private_bio = NULL;
+		dec_local(mdev);
+		if (bio_rw(req->master_bio) == READA)	<<<=== Bang!
+			/* it is legal to fail READA */
+			break;

That's the only other one I found so far (by inspection).

Also - one other question -- do you have an update on how much more you
plan to do before stabilizing and releasing the first official 8.0
version? I'd like to get 8.0 into my development community so it can get
wider testing but it's a bit hard at the moment...

Thanks for all the hard work,
Simon



More information about the drbd-dev mailing list