On 01/30/2013 06:33 PM, Josh Durgin wrote:
> It'd be nice to have a log message when libceph_compatible fails.
> It could be a future patch though.
>
> Reviewed-by: Josh Durgin <[email protected]>
You're right, and I had thought of doing that but forgot.
I will add it before I commit. Thanks a lot.
-Alex
> On 01/30/2013 12:49 PM, Alex Elder wrote:
>> Currently, if the OSD client finds an osd request has had a bio list
>> attached to it, it drops a reference to it (or rather, to the first
>> entry on that list) when the request is released.
>>
>> The code that added that reference (i.e., the rbd client) is
>> therefore required to take an extra reference to that first bio
>> structure.
>>
>> The osd client doesn't really do anything with the bio pointer other
>> than transfer it from the osd request structure to outgoing (for
>> writes) and ingoing (for reads) messages. So it really isn't the
>> right place to be taking or dropping references.
>>
>> Furthermore, the rbd client already holds references to all bio
>> structures it passes to the osd client, and holds them until the
>> request is completed. So there's no need for this extra reference
>> whatsoever.
>>
>> So remove the bio_put() call in ceph_osdc_release_request(), as
>> well as its matching bio_get() call in rbd_osd_req_create().
>>
>> This change could lead to a crash if old libceph.ko was used with
>> new rbd.ko. Add a compatibility check at rbd initialization time to
>> avoid this possibilty.
>>
>> This resolves:
>> http://tracker.ceph.com/issues/3798 and
>> http://tracker.ceph.com/issues/3799
>>
>> Signed-off-by: Alex Elder <[email protected]>
>> ---
>> drivers/block/rbd.c | 3 ++-
>> net/ceph/ceph_common.c | 2 +-
>> net/ceph/osd_client.c | 4 ----
>> 3 files changed, 3 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
>> index a9ce58c..6586800 100644
>> --- a/drivers/block/rbd.c
>> +++ b/drivers/block/rbd.c
>> @@ -1342,7 +1342,6 @@ static struct ceph_osd_request *rbd_osd_req_create(
>> case OBJ_REQUEST_BIO:
>> rbd_assert(obj_request->bio_list != NULL);
>> osd_req->r_bio = obj_request->bio_list;
>> - bio_get(osd_req->r_bio);
>> /* osd client requires "num pages" even for bio */
>> osd_req->r_num_pages = calc_pages_for(offset, length);
>> break;
>> @@ -4150,6 +4149,8 @@ int __init rbd_init(void)
>> {
>> int rc;
>>
>> + if (!libceph_compatible(NULL))
>> + return -EINVAL;
>> rc = rbd_sysfs_init();
>> if (rc)
>> return rc;
>> diff --git a/net/ceph/ceph_common.c b/net/ceph/ceph_common.c
>> index a98c03f..c236c235 100644
>> --- a/net/ceph/ceph_common.c
>> +++ b/net/ceph/ceph_common.c
>> @@ -39,7 +39,7 @@
>> */
>> bool libceph_compatible(void *data)
>> {
>> - return false;
>> + return true;
>> }
>> EXPORT_SYMBOL(libceph_compatible);
>>
>> diff --git a/net/ceph/osd_client.c b/net/ceph/osd_client.c
>> index 500ae8b..ba03648 100644
>> --- a/net/ceph/osd_client.c
>> +++ b/net/ceph/osd_client.c
>> @@ -147,10 +147,6 @@ void ceph_osdc_release_request(struct kref *kref)
>> if (req->r_own_pages)
>> ceph_release_page_vector(req->r_pages,
>> req->r_num_pages);
>> -#ifdef CONFIG_BLOCK
>> - if (req->r_bio)
>> - bio_put(req->r_bio);
>> -#endif
>> ceph_put_snap_context(req->r_snapc);
>> ceph_pagelist_release(&req->r_trail);
>> if (req->r_mempool)
>>
>
--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
