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: Josh Durgin <josh.durgin@inktank.com>
This commit is contained in:
committed by
Ilya Dryomov
parent
7dd440c9e0
commit
4e752f0ab0
@ -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(
|
|||||||
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(
|
|||||||
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(
|
|||||||
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 @@ out:
|
|||||||
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)
|
|||||||
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 @@ err_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);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
Reference in New Issue
Block a user