| Message ID | 5AAFA682.2080901@hitachi.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Yang, > I am not sure is this the best way for this case, what about adding an option in "rbd map -o thick rbd/test"? I will add such option to the rbd map command to manipulate image settings. So, the end-user do not change the settings directly via sysfs file. > @@ -4011,6 +4012,15 @@ static void rbd_queue_workfn(struct work_struct *work) > goto err; > } > > + /* Ignore/skip discard requests for thick-provision image */ > > Just ignore? or return -EOPNOTSUPP? Thanks, I think -EOPNOTSUPP is better because user programs cannot know the result of requested operation when the kernel rbd driver ignores discard request. The result of requested operation when the kernel rbd driver ignores discard requests, which probably misleads the user programs. > In addition, we should not ignore the REQ_OP_WRITE_ZEROES. Relating to the above, the return code of REQ_OP_WRITE_ZEROS request is also -EOPNOTSUPP instead of ignoring. I think the result of -EOPNOTSUPP is also better for this request because the kernel rbd driver can expect that user programs write zero data by itself. > + spin_lock_irq(&rbd_dev->lock); > + if (!strncmp(buf, "1", size)) > + set_bit(RBD_DEV_FLAG_THICK, &rbd_dev->flags); > + else if (!strncmp(buf, "0", size)) > + clear_bit(RBD_DEV_FLAG_THICK, &rbd_dev->flags); > else ? I will add the warning message in else case. > static DEVICE_ATTR(snap_id, S_IRUGO, rbd_snap_id_show, NULL); > static DEVICE_ATTR(parent, S_IRUGO, rbd_parent_show, NULL); > +static DEVICE_ATTR(thick, 0644, rbd_thick_show, rbd_thick_store); > maybe S_IRUGO | S_IWUGO? I will change the code by using the flags. Regards,
On Thu, Mar 22, 2018 at 12:57 PM, 亀井仁志 / KAMEI，HITOSHI <hitoshi.kamei.xm@hitachi.com> wrote: > Hi Yang, > >> I am not sure is this the best way for this case, what about adding an option in "rbd map -o thick rbd/test"? > > I will add such option to the rbd map command to manipulate image settings. So, the end-user > do not change the settings directly via sysfs file. > >> @@ -4011,6 +4012,15 @@ static void rbd_queue_workfn(struct work_struct *work) >> goto err; >> } >> >> + /* Ignore/skip discard requests for thick-provision image */ >> >> Just ignore? or return -EOPNOTSUPP? > > Thanks, I think -EOPNOTSUPP is better because user programs cannot know > the result of requested operation when the kernel rbd driver ignores > discard request. The result of requested operation when the kernel rbd driver > ignores discard requests, which probably misleads the user programs. > >> In addition, we should not ignore the REQ_OP_WRITE_ZEROES. > > Relating to the above, the return code of REQ_OP_WRITE_ZEROS request > is also -EOPNOTSUPP instead of ignoring. I think the result of > -EOPNOTSUPP is also better for this request because the kernel > rbd driver can expect that user programs write zero data by itself. REQ_OP_WRITE_ZEROES should continue to work, we just need to make sure it never issues truncates or deletes and instead writes zeroes explicitly. I think we should be explicit about the fact that discard is not supported instead of accepting the discard request and failing it in rbd_queue_workfn(). Attached patch is what I have in mind. Thanks, Ilya -- To unsubscribe from this list: send the line "unsubscribe ceph-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c index 8e40da093766..dba60ef43a47 100644 --- a/drivers/block/rbd.c +++ b/drivers/block/rbd.c @@ -427,6 +427,7 @@ enum rbd_dev_flags { RBD_DEV_FLAG_EXISTS, /* mapped snapshot has not been deleted */ RBD_DEV_FLAG_REMOVING, /* this mapping is being removed */ RBD_DEV_FLAG_BLACKLISTED, /* our ceph_client is blacklisted */ + RBD_DEV_FLAG_THICK, /* image is thick-provisioned */ }; static DEFINE_MUTEX(client_mutex); /* Serialize client creation */ @@ -4011,6 +4012,15 @@ static void rbd_queue_workfn(struct work_struct *work) goto err; } + /* Ignore/skip discard requests for thick-provision image */ + + if (op_type == OBJ_OP_DISCARD && + test_bit(RBD_DEV_FLAG_THICK, &rbd_dev->flags)) { + dout("%s: Ignored a discard request\n", __func__); + result = 0; + goto err_rq; + } + /* Ignore/skip any zero-length requests */ if (!length) { @@ -4600,6 +4610,32 @@ static ssize_t rbd_image_refresh(struct device *dev, return size; } +static ssize_t rbd_thick_store(struct device *dev, + struct device_attribute *attr, + const char *buf, + size_t size) +{ + struct rbd_device *rbd_dev = dev_to_rbd_dev(dev); + + spin_lock_irq(&rbd_dev->lock); + if (!strncmp(buf, "1", size)) + set_bit(RBD_DEV_FLAG_THICK, &rbd_dev->flags); + else if (!strncmp(buf, "0", size)) + clear_bit(RBD_DEV_FLAG_THICK, &rbd_dev->flags); + spin_unlock_irq(&rbd_dev->lock); + + return size; +} + +static ssize_t rbd_thick_show(struct device *dev, + struct device_attribute *attr, char *buf) +{ + struct rbd_device *rbd_dev = dev_to_rbd_dev(dev); + int is_thick = test_bit(RBD_DEV_FLAG_THICK, &rbd_dev->flags) ? 1 : 0; + + return sprintf(buf, "%d\n", is_thick); +} + static DEVICE_ATTR(size, S_IRUGO, rbd_size_show, NULL); static DEVICE_ATTR(features, S_IRUGO, rbd_features_show, NULL); static DEVICE_ATTR(major, S_IRUGO, rbd_major_show, NULL); @@ -4616,6 +4652,7 @@ static DEVICE_ATTR(refresh, S_IWUSR, NULL, rbd_image_refresh); static DEVICE_ATTR(current_snap, S_IRUGO, rbd_snap_show, NULL); static DEVICE_ATTR(snap_id, S_IRUGO, rbd_snap_id_show, NULL); static DEVICE_ATTR(parent, S_IRUGO, rbd_parent_show, NULL); +static DEVICE_ATTR(thick, 0644, rbd_thick_show, rbd_thick_store); static struct attribute *rbd_attrs[] = { &dev_attr_size.attr, @@ -4634,6 +4671,7 @@ static struct attribute *rbd_attrs[] = { &dev_attr_snap_id.attr, &dev_attr_parent.attr, &dev_attr_refresh.attr, + &dev_attr_thick.attr, NULL };
This patch adds a user interface to prevent from issuing discard requests onto the specified rbd device. This is needed for thick-provisioned images. To avoid discarding allocated blocks on thick-provision image, users can specify this flag bit via sysfs (/sys/bus/rbd/devices/<dev-id>/thick). When users write "1" to the file, rbd doesn't issue discard operation; meanwhile, if users write "0" to the file then rbd issues discard operation. Signed-off-by: Hitoshi Kamei <hitoshi.kamei.xm@hitachi.com> Cc: Mitsuo Hayasaka <mitsuo.hayasaka.hu@hitachi.com> --- drivers/block/rbd.c | 38 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+)