diff mbox series

[2/2] rbd: compression_hint option

Message ID 20200601195826.17159-3-idryomov@gmail.com
State New, archived
Headers show
Series rbd: compression_hint option | expand

Commit Message

Ilya Dryomov June 1, 2020, 7:58 p.m. UTC
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@gmail.com>
---
 drivers/block/rbd.c | 43 ++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 42 insertions(+), 1 deletion(-)

Comments

Dongsheng Yang June 2, 2020, 2:34 a.m. UTC | #1
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@gmail.com>
> ---
>   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?


Thanx

Yang

> +		default:
> +			BUG();
> +		}
> +		break;
>   	case Opt_read_only:
>   		opt->read_only = true;
>   		break;
Ilya Dryomov June 2, 2020, 8:31 a.m. UTC | #2
On Tue, Jun 2, 2020 at 4:34 AM Dongsheng Yang
<dongsheng.yang@easystack.cn> 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@gmail.com>
> > ---
> >   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
Dongsheng Yang June 2, 2020, 9:06 a.m. UTC | #3
在 6/2/2020 4:31 PM, Ilya Dryomov 写道:
> On Tue, Jun 2, 2020 at 4:34 AM Dongsheng Yang
> <dongsheng.yang@easystack.cn> 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@gmail.com>
>>> ---
>>>    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.


Hi Ilya,

      Considering this case, should we make this kind of useage invalid? 
Maybe we

can do it in another patch to solve all rbd parameters conflicting problem.


Thanx

> Thanks,
>
>                  Ilya
Ilya Dryomov June 2, 2020, 9:59 a.m. UTC | #4
On Tue, Jun 2, 2020 at 11:06 AM Dongsheng Yang
<dongsheng.yang@easystack.cn> wrote:
>
>
> 在 6/2/2020 4:31 PM, Ilya Dryomov 写道:
> > On Tue, Jun 2, 2020 at 4:34 AM Dongsheng Yang
> > <dongsheng.yang@easystack.cn> 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@gmail.com>
> >>> ---
> >>>    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.
>
>
> Hi Ilya,
>
>       Considering this case, should we make this kind of useage invalid?
> Maybe we
>
> can do it in another patch to solve all rbd parameters conflicting problem.

No.  On the contrary, the intention is to support overriding in
the form of "the last one wins" going forward to be consistent with
filesystems, where this behaviour is expected for the use case of
overriding /etc/fstab options on the command line.

Thanks,

                Ilya
Dongsheng Yang June 2, 2020, 10:52 a.m. UTC | #5
在 6/2/2020 5:59 PM, Ilya Dryomov 写道:
> On Tue, Jun 2, 2020 at 11:06 AM Dongsheng Yang
> <dongsheng.yang@easystack.cn> wrote:
>>
>> 在 6/2/2020 4:31 PM, Ilya Dryomov 写道:
>>> On Tue, Jun 2, 2020 at 4:34 AM Dongsheng Yang
>>> <dongsheng.yang@easystack.cn> 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@gmail.com>
>>>>> ---
>>>>>     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.
>>
>> Hi Ilya,
>>
>>        Considering this case, should we make this kind of useage invalid?
>> Maybe we
>>
>> can do it in another patch to solve all rbd parameters conflicting problem.
> No.  On the contrary, the intention is to support overriding in
> the form of "the last one wins" going forward to be consistent with
> filesystems, where this behaviour is expected for the use case of
> overriding /etc/fstab options on the command line.


Okey makes sense.


Thanx

>
> Thanks,
>
>                  Ilya
diff mbox series

Patch

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;
+		default:
+			BUG();
+		}
+		break;
 	case Opt_read_only:
 		opt->read_only = true;
 		break;