On 07/30/2012 04:46 PM, Josh Durgin wrote:
> On 07/26/2012 12:11 PM, Alex Elder wrote:
>> When an rbd image is mapped, a watch request is issued so that the
>> client will get notified in the event the image header object
>> changes. This is done using rbd_init_watch_dev(), which calls
>> rbd_req_sync_watch().
>>
>> rbd_init_watch_dev() is organized as a loop, arranging to re-issue
>> the request after refreshing the header if -ERANGE ever gets
>> returned. But the only way rbd_req_sync_watch() will return -ERANGE
>> is if the CEPH_OSD_OP_WATCH operation returns that, which it will
>> not.
>>
>> As a result, the whole looping structure and in fact the whole
>> function becomes excessive. So get rid of rbd_init_watch_dev(),
>> and call rbd_req_sync_watch() directly in the one place it's used.
>
> Instead of getting rid of this loop, we should make it
> send a compound operation consisting of (CEPH_OSD_OP_ASSERT_VER,
> CEPH_OSD_OP_WATCH). Then we will get -ERANGE if our version
> of the header is out of data, so we don't lose header updates
> before the watch is established.
I don't have compound operations going yet, but now I see why it
may have been written this way. So I'll drop this patch, and
will make a note to re-visit this spot later.
-Alex
> Josh
>
>> Signed-off-by: Alex Elder <[email protected]>
>> ---
>> drivers/block/rbd.c | 18 +-----------------
>> 1 file changed, 1 insertion(+), 17 deletions(-)
>>
>> diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
>> index 94d0745..71e3f3b 100644
>> --- a/drivers/block/rbd.c
>> +++ b/drivers/block/rbd.c
>> @@ -2191,22 +2191,6 @@ static void rbd_bus_del_dev(struct rbd_device
>> *rbd_dev)
>> device_unregister(&rbd_dev->dev);
>> }
>>
>> -static int rbd_init_watch_dev(struct rbd_device *rbd_dev)
>> -{
>> - int ret, rc;
>> -
>> - do {
>> - ret = rbd_req_sync_watch(rbd_dev);
>> - if (ret == -ERANGE) {
>> - rc = rbd_refresh_header(rbd_dev, NULL);
>> - if (rc < 0)
>> - return rc;
>> - }
>> - } while (ret == -ERANGE);
>> -
>> - return ret;
>> -}
>> -
>> static atomic64_t rbd_id_max = ATOMIC64_INIT(0);
>>
>> /*
>> @@ -2510,7 +2494,7 @@ static ssize_t rbd_add(struct bus_type *bus,
>> if (rc)
>> goto err_out_bus;
>>
>> - rc = rbd_init_watch_dev(rbd_dev);
>> + rc = rbd_req_sync_watch(rbd_dev);
>> if (rc)
>> goto err_out_bus;
>>
>
>
>
--
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