Skip to content

Commit

Permalink
libceph: use "do" in CRC-related Boolean variables
Browse files Browse the repository at this point in the history 
Change the name (and type) of a few CRC-related Boolean local
variables so they contain the word "do", to distingish their purpose
from variables used for holding an actual CRC value.

Note that in the process of doing this I identified a fairly serious
logic error in write_partial_msg_pages():  the value of "do_crc"
assigned appears to be the opposite of what it should be.  No
attempt to fix this is made here; this change preserves the
erroneous behavior.  The problem I found is documented here:
    http://tracker.newdream.net/issues/2064

Signed-off-by: Alex Elder <elder@dreamhost.com>
Signed-off-by: Sage Weil <sage@newdream.net>
  • Loading branch information
Alex Elder committed Mar 22, 2012
1 parent cffaba1 commit bca064d
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 21 deletions.
2 changes: 1 addition & 1 deletion include/linux/ceph/messenger.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ struct ceph_msg {
struct ceph_msg_pos {
int page, page_pos; /* which page; offset in page */
int data_pos; /* offset in data payload */
int did_page_crc; /* true if we've calculated crc for current page */
bool did_page_crc; /* true if we've calculated crc for current page */
};

/* ceph connection fault delay defaults, for exponential backoff */
Expand Down
40 changes: 20 additions & 20 deletions net/ceph/messenger.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -595,7 +595,7 @@ static void prepare_write_message(struct ceph_connection *con)
else
con->out_msg_pos.page_pos = 0;
con->out_msg_pos.data_pos = 0;
con->out_msg_pos.did_page_crc = 0;
con->out_msg_pos.did_page_crc = false;
con->out_more = 1; /* data + footer will follow */
} else {
/* no, queue up footer too and be done */
Expand Down Expand Up @@ -805,7 +805,7 @@ static int write_partial_msg_pages(struct ceph_connection *con)
struct ceph_msg *msg = con->out_msg;
unsigned data_len = le32_to_cpu(msg->hdr.data_len);
size_t len;
int crc = con->msgr->nocrc;
bool do_crc = con->msgr->nocrc;
int ret;
int total_max_write;
int in_trail = 0;
Expand Down Expand Up @@ -843,17 +843,17 @@ static int write_partial_msg_pages(struct ceph_connection *con)

page = list_first_entry(&msg->trail->head,
struct page, lru);
if (crc)
if (do_crc)
kaddr = kmap(page);
max_write = PAGE_SIZE;
} else if (msg->pages) {
page = msg->pages[con->out_msg_pos.page];
if (crc)
if (do_crc)
kaddr = kmap(page);
} else if (msg->pagelist) {
page = list_first_entry(&msg->pagelist->head,
struct page, lru);
if (crc)
if (do_crc)
kaddr = kmap(page);
#ifdef CONFIG_BLOCK
} else if (msg->bio) {
Expand All @@ -862,34 +862,34 @@ static int write_partial_msg_pages(struct ceph_connection *con)
bv = bio_iovec_idx(msg->bio_iter, msg->bio_seg);
page = bv->bv_page;
page_shift = bv->bv_offset;
if (crc)
if (do_crc)
kaddr = kmap(page) + page_shift;
max_write = bv->bv_len;
#endif
} else {
page = zero_page;
if (crc)
if (do_crc)
kaddr = zero_page_address;
}
len = min_t(int, max_write - con->out_msg_pos.page_pos,
total_max_write);

if (crc && !con->out_msg_pos.did_page_crc) {
if (do_crc && !con->out_msg_pos.did_page_crc) {
void *base = kaddr + con->out_msg_pos.page_pos;
u32 tmpcrc = le32_to_cpu(con->out_msg->footer.data_crc);

BUG_ON(kaddr == NULL);
con->out_msg->footer.data_crc =
cpu_to_le32(crc32c(tmpcrc, base, len));
con->out_msg_pos.did_page_crc = 1;
con->out_msg_pos.did_page_crc = true;
}
ret = kernel_sendpage(con->sock, page,
con->out_msg_pos.page_pos + page_shift,
len,
MSG_DONTWAIT | MSG_NOSIGNAL |
MSG_MORE);

if (crc &&
if (do_crc &&
(msg->pages || msg->pagelist || msg->bio || in_trail))
kunmap(page);

Expand All @@ -903,7 +903,7 @@ static int write_partial_msg_pages(struct ceph_connection *con)
if (ret == len) {
con->out_msg_pos.page_pos = 0;
con->out_msg_pos.page++;
con->out_msg_pos.did_page_crc = 0;
con->out_msg_pos.did_page_crc = false;
if (in_trail)
list_move_tail(&page->lru,
&msg->trail->head);
Expand All @@ -920,7 +920,7 @@ static int write_partial_msg_pages(struct ceph_connection *con)
dout("write_partial_msg_pages %p msg %p done\n", con, msg);

/* prepare and queue up footer, too */
if (!crc)
if (!do_crc)
con->out_msg->footer.flags |= CEPH_MSG_FOOTER_NOCRC;
ceph_con_out_kvec_reset(con);
prepare_write_message_footer(con);
Expand Down Expand Up @@ -1557,7 +1557,7 @@ static struct ceph_msg *ceph_alloc_msg(struct ceph_connection *con,

static int read_partial_message_pages(struct ceph_connection *con,
struct page **pages,
unsigned data_len, int datacrc)
unsigned data_len, bool do_datacrc)
{
void *p;
int ret;
Expand All @@ -1570,7 +1570,7 @@ static int read_partial_message_pages(struct ceph_connection *con,
p = kmap(pages[con->in_msg_pos.page]);
ret = ceph_tcp_recvmsg(con->sock, p + con->in_msg_pos.page_pos,
left);
if (ret > 0 && datacrc)
if (ret > 0 && do_datacrc)
con->in_data_crc =
crc32c(con->in_data_crc,
p + con->in_msg_pos.page_pos, ret);
Expand All @@ -1590,7 +1590,7 @@ static int read_partial_message_pages(struct ceph_connection *con,
#ifdef CONFIG_BLOCK
static int read_partial_message_bio(struct ceph_connection *con,
struct bio **bio_iter, int *bio_seg,
unsigned data_len, int datacrc)
unsigned data_len, bool do_datacrc)
{
struct bio_vec *bv = bio_iovec_idx(*bio_iter, *bio_seg);
void *p;
Expand All @@ -1606,7 +1606,7 @@ static int read_partial_message_bio(struct ceph_connection *con,

ret = ceph_tcp_recvmsg(con->sock, p + con->in_msg_pos.page_pos,
left);
if (ret > 0 && datacrc)
if (ret > 0 && do_datacrc)
con->in_data_crc =
crc32c(con->in_data_crc,
p + con->in_msg_pos.page_pos, ret);
Expand All @@ -1633,7 +1633,7 @@ static int read_partial_message(struct ceph_connection *con)
int ret;
int to, left;
unsigned front_len, middle_len, data_len;
int datacrc = con->msgr->nocrc;
bool do_datacrc = con->msgr->nocrc;
int skip;
u64 seq;

Expand Down Expand Up @@ -1744,15 +1744,15 @@ static int read_partial_message(struct ceph_connection *con)
while (con->in_msg_pos.data_pos < data_len) {
if (m->pages) {
ret = read_partial_message_pages(con, m->pages,
data_len, datacrc);
data_len, do_datacrc);
if (ret <= 0)
return ret;
#ifdef CONFIG_BLOCK
} else if (m->bio) {

ret = read_partial_message_bio(con,
&m->bio_iter, &m->bio_seg,
data_len, datacrc);
data_len, do_datacrc);
if (ret <= 0)
return ret;
#endif
Expand Down Expand Up @@ -1787,7 +1787,7 @@ static int read_partial_message(struct ceph_connection *con)
m, con->in_middle_crc, m->footer.middle_crc);
return -EBADMSG;
}
if (datacrc &&
if (do_datacrc &&
(m->footer.flags & CEPH_MSG_FOOTER_NOCRC) == 0 &&
con->in_data_crc != le32_to_cpu(m->footer.data_crc)) {
pr_err("read_partial_message %p data crc %u != exp. %u\n", m,
Expand Down

0 comments on commit bca064d

Please sign in to comment.