On Tue, Jun 2, 2020 at 4:34 AM Dongsheng Yang
<dongsheng.yang@xxxxxxxxxxxx> wrote:
>
> Hi Ilya,
>
> 在 6/2/2020 3:58 AM, Ilya Dryomov 写道:
> > Allow hinting to bluestore if the data should/should not be compressed.
> > The default is to not hint (compression_hint=none).
> >
> > Signed-off-by: Ilya Dryomov <idryomov@xxxxxxxxx>
> > ---
> > drivers/block/rbd.c | 43 ++++++++++++++++++++++++++++++++++++++++++-
> > 1 file changed, 42 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
> > index b1cd41e671d1..e02089d550a4 100644
> > --- a/drivers/block/rbd.c
> > +++ b/drivers/block/rbd.c
> > @@ -836,6 +836,7 @@ enum {
> > Opt_lock_timeout,
> > /* int args above */
> > Opt_pool_ns,
> > + Opt_compression_hint,
> > /* string args above */
> > Opt_read_only,
> > Opt_read_write,
> > @@ -844,8 +845,23 @@ enum {
> > Opt_notrim,
> > };
> >
> > +enum {
> > + Opt_compression_hint_none,
> > + Opt_compression_hint_compressible,
> > + Opt_compression_hint_incompressible,
> > +};
> > +
> > +static const struct constant_table rbd_param_compression_hint[] = {
> > + {"none", Opt_compression_hint_none},
> > + {"compressible", Opt_compression_hint_compressible},
> > + {"incompressible", Opt_compression_hint_incompressible},
> > + {}
> > +};
> > +
> > static const struct fs_parameter_spec rbd_parameters[] = {
> > fsparam_u32 ("alloc_size", Opt_alloc_size),
> > + fsparam_enum ("compression_hint", Opt_compression_hint,
> > + rbd_param_compression_hint),
> > fsparam_flag ("exclusive", Opt_exclusive),
> > fsparam_flag ("lock_on_read", Opt_lock_on_read),
> > fsparam_u32 ("lock_timeout", Opt_lock_timeout),
> > @@ -867,6 +883,8 @@ struct rbd_options {
> > bool lock_on_read;
> > bool exclusive;
> > bool trim;
> > +
> > + u32 alloc_hint_flags; /* CEPH_OSD_OP_ALLOC_HINT_FLAG_* */
> > };
> >
> > #define RBD_QUEUE_DEPTH_DEFAULT BLKDEV_MAX_RQ
> > @@ -2254,7 +2272,7 @@ static void __rbd_osd_setup_write_ops(struct ceph_osd_request *osd_req,
> > osd_req_op_alloc_hint_init(osd_req, which++,
> > rbd_dev->layout.object_size,
> > rbd_dev->layout.object_size,
> > - 0);
> > + rbd_dev->opts->alloc_hint_flags);
> > }
> >
> > if (rbd_obj_is_entire(obj_req))
> > @@ -6332,6 +6350,29 @@ static int rbd_parse_param(struct fs_parameter *param,
> > pctx->spec->pool_ns = param->string;
> > param->string = NULL;
> > break;
> > + case Opt_compression_hint:
> > + switch (result.uint_32) {
> > + case Opt_compression_hint_none:
> > + opt->alloc_hint_flags &=
> > + ~(CEPH_OSD_ALLOC_HINT_FLAG_COMPRESSIBLE |
> > + CEPH_OSD_ALLOC_HINT_FLAG_INCOMPRESSIBLE);
> > + break;
> > + case Opt_compression_hint_compressible:
> > + opt->alloc_hint_flags |=
> > + CEPH_OSD_ALLOC_HINT_FLAG_COMPRESSIBLE;
> > + opt->alloc_hint_flags &=
> > + ~CEPH_OSD_ALLOC_HINT_FLAG_INCOMPRESSIBLE;
> > + break;
> > + case Opt_compression_hint_incompressible:
> > + opt->alloc_hint_flags |=
> > + CEPH_OSD_ALLOC_HINT_FLAG_INCOMPRESSIBLE;
> > + opt->alloc_hint_flags &=
> > + ~CEPH_OSD_ALLOC_HINT_FLAG_COMPRESSIBLE;
> > + break;
>
>
> Just one little question here,
>
> (1) none opt means clear compressible related bits in hint flags, then
> lets the compressor in bluestore to decide compress or not.
>
> (2) compressible opt means set compressible bit and clear incompressible bit
>
> (3) incompressible opt means set incompressible bit and clear
> compressible bit
>
>
> Is there any scenario that alloc_hint_flags is not zero filled before
> rbd_parse_param(), then we have to clear the unexpected bit?
Hi Dongsheng,
This is to handle the case when the map option string has multiple
compression_hint options:
name=admin,...,compression_hint=compressible,...,compression_hint=none
The last one wins and we always end up with either zero or just one
of the flags set, not both.
Thanks,
Ilya
