I like sticking this right next to the cursor update as it avoids any similar bugs. Looks good! Reviewed-by: Sage Weil <sage@xxxxxxxxxxx> On Fri, 29 Mar 2013, Alex Elder wrote: > (This patch is available in branch "review/wip-4450" of the > ceph-client git repository. It corrects a problem that arose > in the commit before it. It is not the topmost commit because > the one before and two after were three patches that had been > previously reviewed. Rather than change the reviewed patch, > I'm keeping this fix separate and have just rebased the two > that follow it.) > > > In write_partial_message_data() we aggregate the crc for the data > portion of the message as each new piece of the data item is > encountered. Because it was computed *before* sending the data, if > an attempt to send a new piece resulted in 0 bytes being sent, the > crc crc across that piece would erroneously get computed again and > added to the aggregate result. This would occasionally happen in > the evnet of a connection failure. > > The crc value isn't really needed until the complete value is known > after sending all data, so there's no need to compute it before > sending. > > So don't calculate the crc for a piece until *after* we know at > least one byte of it has been sent. That will avoid this problem. > > This resolves: > http://tracker.ceph.com/issues/4450 > > Signed-off-by: Alex Elder <elder@xxxxxxxxxxx> > --- > net/ceph/messenger.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/net/ceph/messenger.c b/net/ceph/messenger.c > index 02a37a5..3f5137c 100644 > --- a/net/ceph/messenger.c > +++ b/net/ceph/messenger.c > @@ -1467,8 +1467,6 @@ static int write_partial_message_data(struct > ceph_connection *con) > > page = ceph_msg_data_next(&msg->data, &page_offset, &length, > &last_piece); > - if (do_datacrc && cursor->need_crc) > - crc = ceph_crc32c_page(crc, page, page_offset, length); > ret = ceph_tcp_sendpage(con->sock, page, page_offset, > length, last_piece); > if (ret <= 0) { > @@ -1477,6 +1475,8 @@ static int write_partial_message_data(struct > ceph_connection *con) > > return ret; > } > + if (do_datacrc && cursor->need_crc) > + crc = ceph_crc32c_page(crc, page, page_offset, length); > out_msg_pos_next(con, page, length, (size_t) ret); > } > > -- > 1.7.9.5 > > -- > 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 > > -- 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
