[5/9] rbd: don't acquire exclusive lock for read-only mappings
diff mbox series

Message ID 20191118133816.3963-6-idryomov@gmail.com
State New
Headers show
Series
  • wip-krbd-readonly
Related show

Commit Message

Ilya Dryomov Nov. 18, 2019, 1:38 p.m. UTC
A read-only mapping should be usable with read-only OSD caps, so
neither the header lock nor the object map lock can be acquired.
Unfortunately, this means that images mapped read-only lose the
advantage of the object map.

Snapshots, however, can take advantage of the object map without
any exclusionary locks, so if the object map is desired, snapshot
the image and map the snapshot instead of the image.

Signed-off-by: Ilya Dryomov <idryomov@gmail.com>
---
 drivers/block/rbd.c | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

Comments

Dongsheng Yang Nov. 19, 2019, 8:37 a.m. UTC | #1
On 11/18/2019 09:38 PM, Ilya Dryomov wrote:
> A read-only mapping should be usable with read-only OSD caps, so
> neither the header lock nor the object map lock can be acquired.
> Unfortunately, this means that images mapped read-only lose the
> advantage of the object map.
>
> Snapshots, however, can take advantage of the object map without
> any exclusionary locks, so if the object map is desired, snapshot
> the image and map the snapshot instead of the image.
>
> Signed-off-by: Ilya Dryomov <idryomov@gmail.com>

Reviewed-by: Dongsheng Yang <dongsheng.yang@easystack.cn>
> ---
>   drivers/block/rbd.c | 15 +++++++++++++--
>   1 file changed, 13 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
> index 979203cd934c..aaa359561356 100644
> --- a/drivers/block/rbd.c
> +++ b/drivers/block/rbd.c
> @@ -1833,6 +1833,17 @@ static u8 rbd_object_map_get(struct rbd_device *rbd_dev, u64 objno)
>   
>   static bool use_object_map(struct rbd_device *rbd_dev)
>   {
> +	/*
> +	 * An image mapped read-only can't use the object map -- it isn't
> +	 * loaded because the header lock isn't acquired.  Someone else can
> +	 * write to the image and update the object map behind our back.
> +	 *
> +	 * A snapshot can't be written to, so using the object map is always
> +	 * safe.
> +	 */
> +	if (!rbd_is_snap(rbd_dev) && rbd_is_ro(rbd_dev))
> +		return false;
> +
>   	return ((rbd_dev->header.features & RBD_FEATURE_OBJECT_MAP) &&
>   		!(rbd_dev->object_map_flags & RBD_FLAG_OBJECT_MAP_INVALID));
>   }
> @@ -3556,7 +3567,7 @@ static bool need_exclusive_lock(struct rbd_img_request *img_req)
>   	if (!(rbd_dev->header.features & RBD_FEATURE_EXCLUSIVE_LOCK))
>   		return false;
>   
> -	if (rbd_is_snap(rbd_dev))
> +	if (rbd_is_ro(rbd_dev))
>   		return false;
>   
>   	rbd_assert(!test_bit(IMG_REQ_CHILD, &img_req->flags));
> @@ -6677,7 +6688,7 @@ static int rbd_add_acquire_lock(struct rbd_device *rbd_dev)
>   		return -EINVAL;
>   	}
>   
> -	if (rbd_is_snap(rbd_dev))
> +	if (rbd_is_ro(rbd_dev))
>   		return 0;
>   
>   	rbd_assert(!rbd_is_lock_owner(rbd_dev));

Patch
diff mbox series

diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
index 979203cd934c..aaa359561356 100644
--- a/drivers/block/rbd.c
+++ b/drivers/block/rbd.c
@@ -1833,6 +1833,17 @@  static u8 rbd_object_map_get(struct rbd_device *rbd_dev, u64 objno)
 
 static bool use_object_map(struct rbd_device *rbd_dev)
 {
+	/*
+	 * An image mapped read-only can't use the object map -- it isn't
+	 * loaded because the header lock isn't acquired.  Someone else can
+	 * write to the image and update the object map behind our back.
+	 *
+	 * A snapshot can't be written to, so using the object map is always
+	 * safe.
+	 */
+	if (!rbd_is_snap(rbd_dev) && rbd_is_ro(rbd_dev))
+		return false;
+
 	return ((rbd_dev->header.features & RBD_FEATURE_OBJECT_MAP) &&
 		!(rbd_dev->object_map_flags & RBD_FLAG_OBJECT_MAP_INVALID));
 }
@@ -3556,7 +3567,7 @@  static bool need_exclusive_lock(struct rbd_img_request *img_req)
 	if (!(rbd_dev->header.features & RBD_FEATURE_EXCLUSIVE_LOCK))
 		return false;
 
-	if (rbd_is_snap(rbd_dev))
+	if (rbd_is_ro(rbd_dev))
 		return false;
 
 	rbd_assert(!test_bit(IMG_REQ_CHILD, &img_req->flags));
@@ -6677,7 +6688,7 @@  static int rbd_add_acquire_lock(struct rbd_device *rbd_dev)
 		return -EINVAL;
 	}
 
-	if (rbd_is_snap(rbd_dev))
+	if (rbd_is_ro(rbd_dev))
 		return 0;
 
 	rbd_assert(!rbd_is_lock_owner(rbd_dev));