On Tuesday, March 5, 2013 at 7:33 AM, Alex Elder wrote: > (This patch is available as the top commit in branch > "review/wip-4324" in the ceph-client git repository.) > > In ceph_con_in_msg_alloc() it is possible for a connection's > alloc_msg method to indicate an incoming message should be skipped. > By default, read_partial_message() initializes the skip variable > to 0 before it gets provided to ceph_con_in_msg_alloc(). > > The osd client, mon client, and mds client each supply an alloc_msg > method. The mds client always assigns skip to be 0. > > The other two leave the skip value of as-is, or assigns it to zero, > except: > - if no (osd or mon) request having the given tid is found, in > which case skip is set to 1 and NULL is returned; or > - in the osd client, if the data of the reply message is not > adequate to hold the message to be read, it assigns skip > value 1 and returns NULL. > So the returned message pointer will always be NULL if skip is ever > non-zero. > > Clean up the logic a bit in ceph_con_in_msg_alloc() to make this > state of affairs more obvious. Add a comment explaining how a null > message pointer can mean either a message that should be skipped or > a problem allocating a message. > > This resolves: > http://tracker.ceph.com/issues/4324 > > Reported-by: Greg Farnum <greg@xxxxxxxxxxx (mailto:greg@xxxxxxxxxxx)> > Signed-off-by: Alex Elder <elder@xxxxxxxxxxx (mailto:elder@xxxxxxxxxxx)> > --- > net/ceph/messenger.c | 23 +++++++++++++---------- > 1 file changed, 13 insertions(+), 10 deletions(-) > > diff --git a/net/ceph/messenger.c b/net/ceph/messenger.c > index 5bf1bb5..644cb6c 100644 > --- a/net/ceph/messenger.c > +++ b/net/ceph/messenger.c > @@ -2860,18 +2860,21 @@ static int ceph_con_in_msg_alloc(struct > ceph_connection *con, int *skip) > ceph_msg_put(msg); > return -EAGAIN; > } > - con->in_msg = msg; > - if (con->in_msg) { > + if (msg) { > + BUG_ON(*skip); > + con->in_msg = msg; > con->in_msg->con = con->ops->get(con); > BUG_ON(con->in_msg->con == NULL); > - } > - if (*skip) { > - con->in_msg = NULL; > - return 0; > - } > - if (!con->in_msg) { > - con->error_msg = > - "error allocating memory for incoming message"; > + } else { > + /* > + * Null message pointer means either we should skip > + * this message or we couldn't allocate memory. The > + * former is not an error. > + */ > + if (*skip) > + return 0; > + con->error_msg = "error allocating memory for incoming message"; > + > return -ENOMEM; > } > memcpy(&con->in_msg->hdr, &con->in_hdr, sizeof(con->in_hdr)); > -- > 1.7.9.5 Reviewed-by: Greg Farnum <greg@xxxxxxxxxxx> -- 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
