| Message ID | 1403716607-13535-12-git-send-email-ilya.dryomov@inktank.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 06/25/2014 12:16 PM, Ilya Dryomov wrote: > In the past, rbd_dev_header_watch_sync() used to handle both watch and > unwatch requests and was entangled and leaky. Commit b30a01f2a307 > ("rbd: fix osd_request memory leak in __rbd_dev_header_watch_sync()") > split it into two separate functions. This commit cleanly abstracts > the common bits, relying on the fixed rbd_obj_request_wait(). Adding this without calling it leads to an unused function warning in the build, I'm sure. You could probably squash this into the next patch. Reviewed-by: Alex Elder <elder@linaro.org> > Signed-off-by: Ilya Dryomov <ilya.dryomov@inktank.com> > --- > drivers/block/rbd.c | 53 +++++++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 53 insertions(+) > > diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c > index 20147aec86f3..02cf7aba7679 100644 > --- a/drivers/block/rbd.c > +++ b/drivers/block/rbd.c > @@ -2971,6 +2971,59 @@ static void rbd_watch_cb(u64 ver, u64 notify_id, u8 opcode, void *data) > } > > /* > + * Send a (un)watch request and wait for the ack. Return a request > + * with a ref held on success or error. > + */ > +static struct rbd_obj_request *rbd_obj_watch_request_helper( > + struct rbd_device *rbd_dev, > + bool watch) > +{ > + struct ceph_osd_client *osdc = &rbd_dev->rbd_client->client->osdc; > + struct rbd_obj_request *obj_request; > + int ret; > + > + obj_request = rbd_obj_request_create(rbd_dev->header_name, 0, 0, > + OBJ_REQUEST_NODATA); > + if (!obj_request) > + return ERR_PTR(-ENOMEM); > + > + obj_request->osd_req = rbd_osd_req_create(rbd_dev, true, 1, > + obj_request); > + if (!obj_request->osd_req) { > + ret = -ENOMEM; > + goto out; > + } > + > + osd_req_op_watch_init(obj_request->osd_req, 0, CEPH_OSD_OP_WATCH, > + rbd_dev->watch_event->cookie, 0, watch); > + rbd_osd_req_format_write(obj_request); > + > + if (watch) > + ceph_osdc_set_request_linger(osdc, obj_request->osd_req); > + > + ret = rbd_obj_request_submit(osdc, obj_request); > + if (ret) > + goto out; > + > + ret = rbd_obj_request_wait(obj_request); > + if (ret) > + goto out; > + > + ret = obj_request->result; > + if (ret) { > + if (watch) > + rbd_obj_request_end(obj_request); > + goto out; > + } > + > + return obj_request; > + > +out: > + rbd_obj_request_put(obj_request); > + return ERR_PTR(ret); > +} > + > +/* > * Initiate a watch request, synchronously. > */ > static int rbd_dev_header_watch_sync(struct rbd_device *rbd_dev) > -- To unsubscribe from this list: send the line "unsubscribe ceph-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Jul 8, 2014 at 2:36 AM, Alex Elder <elder@ieee.org> wrote: > On 06/25/2014 12:16 PM, Ilya Dryomov wrote: >> In the past, rbd_dev_header_watch_sync() used to handle both watch and >> unwatch requests and was entangled and leaky. Commit b30a01f2a307 >> ("rbd: fix osd_request memory leak in __rbd_dev_header_watch_sync()") >> split it into two separate functions. This commit cleanly abstracts >> the common bits, relying on the fixed rbd_obj_request_wait(). > > Adding this without calling it leads to an unused function > warning in the build, I'm sure. > > You could probably squash this into the next patch. It used to be a single patch in the previous version of this series, but it was too hard to review even for myself, so I had to split it. Thanks, Ilya -- To unsubscribe from this list: send the line "unsubscribe ceph-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c index 20147aec86f3..02cf7aba7679 100644 --- a/drivers/block/rbd.c +++ b/drivers/block/rbd.c @@ -2971,6 +2971,59 @@ static void rbd_watch_cb(u64 ver, u64 notify_id, u8 opcode, void *data) } /* + * Send a (un)watch request and wait for the ack. Return a request + * with a ref held on success or error. + */ +static struct rbd_obj_request *rbd_obj_watch_request_helper( + struct rbd_device *rbd_dev, + bool watch) +{ + struct ceph_osd_client *osdc = &rbd_dev->rbd_client->client->osdc; + struct rbd_obj_request *obj_request; + int ret; + + obj_request = rbd_obj_request_create(rbd_dev->header_name, 0, 0, + OBJ_REQUEST_NODATA); + if (!obj_request) + return ERR_PTR(-ENOMEM); + + obj_request->osd_req = rbd_osd_req_create(rbd_dev, true, 1, + obj_request); + if (!obj_request->osd_req) { + ret = -ENOMEM; + goto out; + } + + osd_req_op_watch_init(obj_request->osd_req, 0, CEPH_OSD_OP_WATCH, + rbd_dev->watch_event->cookie, 0, watch); + rbd_osd_req_format_write(obj_request); + + if (watch) + ceph_osdc_set_request_linger(osdc, obj_request->osd_req); + + ret = rbd_obj_request_submit(osdc, obj_request); + if (ret) + goto out; + + ret = rbd_obj_request_wait(obj_request); + if (ret) + goto out; + + ret = obj_request->result; + if (ret) { + if (watch) + rbd_obj_request_end(obj_request); + goto out; + } + + return obj_request; + +out: + rbd_obj_request_put(obj_request); + return ERR_PTR(ret); +} + +/* * Initiate a watch request, synchronously. */ static int rbd_dev_header_watch_sync(struct rbd_device *rbd_dev)
In the past, rbd_dev_header_watch_sync() used to handle both watch and unwatch requests and was entangled and leaky. Commit b30a01f2a307 ("rbd: fix osd_request memory leak in __rbd_dev_header_watch_sync()") split it into two separate functions. This commit cleanly abstracts the common bits, relying on the fixed rbd_obj_request_wait(). Signed-off-by: Ilya Dryomov <ilya.dryomov@inktank.com> --- drivers/block/rbd.c | 53 +++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 53 insertions(+)