From 5766651971e81298732466c9aa462ff47898ba37 Mon Sep 17 00:00:00 2001 From: Alex Elder Date: Mon, 23 Jan 2012 15:49:27 -0600 Subject: ceph: use a shared zero page rather than one per messenger Each messenger allocates a page to be used when writing zeroes out in the event of error or other abnormal condition. Instead, use the kernel ZERO_PAGE() for that purpose. Signed-off-by: Alex Elder Signed-off-by: Sage Weil --- include/linux/ceph/messenger.h | 1 - 1 file changed, 1 deletion(-) (limited to 'include/linux/ceph') diff --git a/include/linux/ceph/messenger.h b/include/linux/ceph/messenger.h index ffbeb2c217b4..6b5af5f976d1 100644 --- a/include/linux/ceph/messenger.h +++ b/include/linux/ceph/messenger.h @@ -54,7 +54,6 @@ struct ceph_connection_operations { struct ceph_messenger { struct ceph_entity_inst inst; /* my name+address */ struct ceph_entity_addr my_enc_addr; - struct page *zero_page; /* used in certain error cases */ bool nocrc; -- cgit v1.2.3 From ee57741c5209154b8ef124bcaa2496da1b69a988 Mon Sep 17 00:00:00 2001 From: Alex Elder Date: Tue, 24 Jan 2012 10:08:36 -0600 Subject: rbd: make ceph_parse_options() return a pointer ceph_parse_options() takes the address of a pointer as an argument and uses it to return the address of an allocated structure if successful. With this interface is not evident at call sites that the pointer is always initialized. Change the interface to return the address instead (or a pointer-coded error code) to make the validity of the returned pointer obvious. Signed-off-by: Alex Elder Signed-off-by: Sage Weil --- drivers/block/rbd.c | 6 ++++-- fs/ceph/super.c | 6 ++++-- include/linux/ceph/libceph.h | 2 +- net/ceph/ceph_common.c | 16 ++++++++-------- 4 files changed, 17 insertions(+), 13 deletions(-) (limited to 'include/linux/ceph') diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c index b9371f0b9532..ed6711e35323 100644 --- a/drivers/block/rbd.c +++ b/drivers/block/rbd.c @@ -371,11 +371,13 @@ static int rbd_get_client(struct rbd_device *rbd_dev, const char *mon_addr, rbd_opts->notify_timeout = RBD_NOTIFY_TIMEOUT_DEFAULT; - ret = ceph_parse_options(&opt, options, mon_addr, + opt = ceph_parse_options(options, mon_addr, mon_addr + strlen(mon_addr), parse_rbd_opts_token, rbd_opts); - if (ret < 0) + if (IS_ERR(opt)) { + ret = PTR_ERR(opt); goto done_err; + } spin_lock(&node_lock); rbdc = __rbd_client_find(opt); diff --git a/fs/ceph/super.c b/fs/ceph/super.c index c3da3b32bdde..4fab1fdcfa6a 100644 --- a/fs/ceph/super.c +++ b/fs/ceph/super.c @@ -334,10 +334,12 @@ static int parse_mount_options(struct ceph_mount_options **pfsopt, *path += 2; dout("server path '%s'\n", *path); - err = ceph_parse_options(popt, options, dev_name, dev_name_end, + *popt = ceph_parse_options(options, dev_name, dev_name_end, parse_fsopt_token, (void *)fsopt); - if (err) + if (IS_ERR(*popt)) { + err = PTR_ERR(*popt); goto out; + } /* success */ *pfsopt = fsopt; diff --git a/include/linux/ceph/libceph.h b/include/linux/ceph/libceph.h index 95bd8502e715..92eef7c3d3c5 100644 --- a/include/linux/ceph/libceph.h +++ b/include/linux/ceph/libceph.h @@ -207,7 +207,7 @@ extern struct kmem_cache *ceph_cap_cachep; extern struct kmem_cache *ceph_dentry_cachep; extern struct kmem_cache *ceph_file_cachep; -extern int ceph_parse_options(struct ceph_options **popt, char *options, +extern struct ceph_options *ceph_parse_options(char *options, const char *dev_name, const char *dev_name_end, int (*parse_extra_token)(char *c, void *private), void *private); diff --git a/net/ceph/ceph_common.c b/net/ceph/ceph_common.c index 761ad9d6cc3b..621c3221b393 100644 --- a/net/ceph/ceph_common.c +++ b/net/ceph/ceph_common.c @@ -277,10 +277,11 @@ out: return err; } -int ceph_parse_options(struct ceph_options **popt, char *options, - const char *dev_name, const char *dev_name_end, - int (*parse_extra_token)(char *c, void *private), - void *private) +struct ceph_options * +ceph_parse_options(char *options, const char *dev_name, + const char *dev_name_end, + int (*parse_extra_token)(char *c, void *private), + void *private) { struct ceph_options *opt; const char *c; @@ -289,7 +290,7 @@ int ceph_parse_options(struct ceph_options **popt, char *options, opt = kzalloc(sizeof(*opt), GFP_KERNEL); if (!opt) - return err; + return ERR_PTR(-ENOMEM); opt->mon_addr = kcalloc(CEPH_MAX_MON, sizeof(*opt->mon_addr), GFP_KERNEL); if (!opt->mon_addr) @@ -412,12 +413,11 @@ int ceph_parse_options(struct ceph_options **popt, char *options, } /* success */ - *popt = opt; - return 0; + return opt; out: ceph_destroy_options(opt); - return err; + return ERR_PTR(err); } EXPORT_SYMBOL(ceph_parse_options); -- cgit v1.2.3 From e0f43c9419c1900e5b50de4261e9686a45a0a2b8 Mon Sep 17 00:00:00 2001 From: Alex Elder Date: Tue, 14 Feb 2012 14:05:33 -0600 Subject: libceph: make ceph_msgr_wq private The messenger workqueue has no need to be public. So give it static scope. Signed-off-by: Alex Elder Signed-off-by: Sage Weil --- include/linux/ceph/messenger.h | 2 -- net/ceph/messenger.c | 2 +- 2 files changed, 1 insertion(+), 3 deletions(-) (limited to 'include/linux/ceph') diff --git a/include/linux/ceph/messenger.h b/include/linux/ceph/messenger.h index 6b5af5f976d1..5ca0f8244203 100644 --- a/include/linux/ceph/messenger.h +++ b/include/linux/ceph/messenger.h @@ -14,8 +14,6 @@ struct ceph_msg; struct ceph_connection; -extern struct workqueue_struct *ceph_msgr_wq; /* receive work queue */ - /* * Ceph defines these callbacks for handling connection events. */ diff --git a/net/ceph/messenger.c b/net/ceph/messenger.c index 04d2b975ab0c..31f59ac03d8a 100644 --- a/net/ceph/messenger.c +++ b/net/ceph/messenger.c @@ -97,7 +97,7 @@ static void encode_my_addr(struct ceph_messenger *msgr) /* * work queue for all reading and writing to/from the socket. */ -struct workqueue_struct *ceph_msgr_wq; +static struct workqueue_struct *ceph_msgr_wq; int ceph_msgr_init(void) { -- cgit v1.2.3 From bca064d236a2e3162a07c758855221bcbe3c475b Mon Sep 17 00:00:00 2001 From: Alex Elder Date: Wed, 15 Feb 2012 07:43:54 -0600 Subject: libceph: use "do" in CRC-related Boolean variables 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 Signed-off-by: Sage Weil --- include/linux/ceph/messenger.h | 2 +- net/ceph/messenger.c | 40 ++++++++++++++++++++-------------------- 2 files changed, 21 insertions(+), 21 deletions(-) (limited to 'include/linux/ceph') diff --git a/include/linux/ceph/messenger.h b/include/linux/ceph/messenger.h index 5ca0f8244203..3bff047f6b0f 100644 --- a/include/linux/ceph/messenger.h +++ b/include/linux/ceph/messenger.h @@ -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 */ diff --git a/net/ceph/messenger.c b/net/ceph/messenger.c index 44d8c77cabdd..204e229e6628 100644 --- a/net/ceph/messenger.c +++ b/net/ceph/messenger.c @@ -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 */ @@ -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; @@ -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) { @@ -862,26 +862,26 @@ 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, @@ -889,7 +889,7 @@ static int write_partial_msg_pages(struct ceph_connection *con) MSG_DONTWAIT | MSG_NOSIGNAL | MSG_MORE); - if (crc && + if (do_crc && (msg->pages || msg->pagelist || msg->bio || in_trail)) kunmap(page); @@ -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); @@ -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); @@ -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; @@ -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); @@ -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; @@ -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); @@ -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; @@ -1744,7 +1744,7 @@ 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 @@ -1752,7 +1752,7 @@ static int read_partial_message(struct ceph_connection *con) 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 @@ -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, -- cgit v1.2.3