Commit 4e752f0a authored by Josh Durgin's avatar Josh Durgin Committed by Ilya Dryomov

rbd: access snapshot context and mapping size safely

These fields may both change while the image is mapped if a snapshot
is created or deleted or the image is resized.  They are guarded by
rbd_dev->header_rwsem, so hold that while reading them, and store a
local copy to refer to outside of the critical section. The local copy
will stay consistent since the snapshot context is reference counted,
and the mapping size is just a u64. This prevents torn loads from
giving us inconsistent values.

Move reading header.snapc into the caller of rbd_img_request_create()
so that we only need to take the semaphore once. The read-only caller,
rbd_parent_request_create() can just pass NULL for snapc, since the
snapshot context is only relevant for writes.
Signed-off-by: default avatarJosh Durgin <josh.durgin@inktank.com>
parent 7dd440c9
...@@ -2057,7 +2057,8 @@ static bool rbd_dev_parent_get(struct rbd_device *rbd_dev) ...@@ -2057,7 +2057,8 @@ static bool rbd_dev_parent_get(struct rbd_device *rbd_dev)
static struct rbd_img_request *rbd_img_request_create( static struct rbd_img_request *rbd_img_request_create(
struct rbd_device *rbd_dev, struct rbd_device *rbd_dev,
u64 offset, u64 length, u64 offset, u64 length,
bool write_request) bool write_request,
struct ceph_snap_context *snapc)
{ {
struct rbd_img_request *img_request; struct rbd_img_request *img_request;
...@@ -2065,12 +2066,6 @@ static struct rbd_img_request *rbd_img_request_create( ...@@ -2065,12 +2066,6 @@ static struct rbd_img_request *rbd_img_request_create(
if (!img_request) if (!img_request)
return NULL; return NULL;
if (write_request) {
down_read(&rbd_dev->header_rwsem);
ceph_get_snap_context(rbd_dev->header.snapc);
up_read(&rbd_dev->header_rwsem);
}
img_request->rq = NULL; img_request->rq = NULL;
img_request->rbd_dev = rbd_dev; img_request->rbd_dev = rbd_dev;
img_request->offset = offset; img_request->offset = offset;
...@@ -2078,7 +2073,7 @@ static struct rbd_img_request *rbd_img_request_create( ...@@ -2078,7 +2073,7 @@ static struct rbd_img_request *rbd_img_request_create(
img_request->flags = 0; img_request->flags = 0;
if (write_request) { if (write_request) {
img_request_write_set(img_request); img_request_write_set(img_request);
img_request->snapc = rbd_dev->header.snapc; img_request->snapc = snapc;
} else { } else {
img_request->snap_id = rbd_dev->spec->snap_id; img_request->snap_id = rbd_dev->spec->snap_id;
} }
...@@ -2134,8 +2129,8 @@ static struct rbd_img_request *rbd_parent_request_create( ...@@ -2134,8 +2129,8 @@ static struct rbd_img_request *rbd_parent_request_create(
rbd_assert(obj_request->img_request); rbd_assert(obj_request->img_request);
rbd_dev = obj_request->img_request->rbd_dev; rbd_dev = obj_request->img_request->rbd_dev;
parent_request = rbd_img_request_create(rbd_dev->parent, parent_request = rbd_img_request_create(rbd_dev->parent, img_offset,
img_offset, length, false); length, false, NULL);
if (!parent_request) if (!parent_request)
return NULL; return NULL;
...@@ -3183,9 +3178,11 @@ static int rbd_obj_method_sync(struct rbd_device *rbd_dev, ...@@ -3183,9 +3178,11 @@ static int rbd_obj_method_sync(struct rbd_device *rbd_dev,
static void rbd_handle_request(struct rbd_device *rbd_dev, struct request *rq) static void rbd_handle_request(struct rbd_device *rbd_dev, struct request *rq)
{ {
struct rbd_img_request *img_request; struct rbd_img_request *img_request;
struct ceph_snap_context *snapc = NULL;
u64 offset = (u64)blk_rq_pos(rq) << SECTOR_SHIFT; u64 offset = (u64)blk_rq_pos(rq) << SECTOR_SHIFT;
u64 length = blk_rq_bytes(rq); u64 length = blk_rq_bytes(rq);
bool wr = rq_data_dir(rq) == WRITE; bool wr = rq_data_dir(rq) == WRITE;
u64 mapping_size;
int result; int result;
/* Ignore/skip any zero-length requests */ /* Ignore/skip any zero-length requests */
...@@ -3226,14 +3223,23 @@ static void rbd_handle_request(struct rbd_device *rbd_dev, struct request *rq) ...@@ -3226,14 +3223,23 @@ static void rbd_handle_request(struct rbd_device *rbd_dev, struct request *rq)
goto err_rq; /* Shouldn't happen */ goto err_rq; /* Shouldn't happen */
} }
if (offset + length > rbd_dev->mapping.size) { down_read(&rbd_dev->header_rwsem);
mapping_size = rbd_dev->mapping.size;
if (wr) {
snapc = rbd_dev->header.snapc;
ceph_get_snap_context(snapc);
}
up_read(&rbd_dev->header_rwsem);
if (offset + length > mapping_size) {
rbd_warn(rbd_dev, "beyond EOD (%llu~%llu > %llu)", offset, rbd_warn(rbd_dev, "beyond EOD (%llu~%llu > %llu)", offset,
length, rbd_dev->mapping.size); length, mapping_size);
result = -EIO; result = -EIO;
goto err_rq; goto err_rq;
} }
img_request = rbd_img_request_create(rbd_dev, offset, length, wr); img_request = rbd_img_request_create(rbd_dev, offset, length, wr,
snapc);
if (!img_request) { if (!img_request) {
result = -ENOMEM; result = -ENOMEM;
goto err_rq; goto err_rq;
...@@ -3256,6 +3262,8 @@ static void rbd_handle_request(struct rbd_device *rbd_dev, struct request *rq) ...@@ -3256,6 +3262,8 @@ static void rbd_handle_request(struct rbd_device *rbd_dev, struct request *rq)
if (result) if (result)
rbd_warn(rbd_dev, "%s %llx at %llx result %d", rbd_warn(rbd_dev, "%s %llx at %llx result %d",
wr ? "write" : "read", length, offset, result); wr ? "write" : "read", length, offset, result);
if (snapc)
ceph_put_snap_context(snapc);
blk_end_request_all(rq, result); blk_end_request_all(rq, result);
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment