On 06/06/2012 12:22 AM, Sage Weil wrote:
>> diff --git a/net/ceph/messenger.c b/net/ceph/messenger.c
>> > index 79f3032..a036141 100644
>> > --- a/net/ceph/messenger.c
>> > +++ b/net/ceph/messenger.c
>> > @@ -2456,17 +2456,24 @@ void ceph_msg_revoke(struct ceph_msg *msg)
>> > /*
>> > * Revoke a message that we may be reading data into
>> > */
>> > -void ceph_con_revoke_message(struct ceph_connection *con, struct
>> > ceph_msg *msg)
>> > +void ceph_msg_revoke_incoming(struct ceph_msg *msg)
>> > {
>> > + struct ceph_connection *con;
>> > +
>> > + BUG_ON(msg == NULL);
>> > + if (!msg->con)
>> > + return; /* Message not in our posession */
> This case weirds me out. I think it can happen, but maybe we should at
> least get a debug msg, like the no-op one below.
>
> Reviewed-by: Sage Weil <sage@xxxxxxxxxxx>
>
I have not been inserting debug messages, but probably should be.
I'm just not accustomed to doing that.
I am not actually convinced this can happen, but did it this way to
match what was happening in the ceph_msg_revoke() case, where
the caller would blindly revoke a message when closing, in case
it was currently queued.
However given how this is used now (only called if an osd_client request
thinks it is waiting for the connection to fill the reply) I see
why it weirds you out--it's conceivable the osd client's notion of
which connection holds the message is out of synch with reality.
I truly want to do away with that osd client's req->r_con_filling_msg
field, which now is kept only for the benefit of reference counting the
ceph_osd...
I will put in a dout() call in this case.
-Alex
--
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
