Re: [PATCH 2/3] rbd: add an option to set state_lock_timeout

[Dongsheng Yang <dongsheng.yang@xxxxxxxxxxxx>]

On 03/26/2018 04:03 PM, Ilya Dryomov wrote:
> On Tue, Mar 20, 2018 at 11:32 AM, Dongsheng Yang
> <dongsheng.yang@xxxxxxxxxxxx> wrote:
> > This patch allow user to set the state_lock_timeout in rbd mapping.
> This can be merged into the previous patch.
>
> > Signed-off-by: Dongsheng Yang <dongsheng.yang@xxxxxxxxxxxx>
> > ---
> >   drivers/block/rbd.c | 16 ++++++++++++++--
> >   1 file changed, 14 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
> > index 199819d..6bfbfe5 100644
> > --- a/drivers/block/rbd.c
> > +++ b/drivers/block/rbd.c
> > @@ -726,6 +726,7 @@ static struct rbd_client *rbd_client_find(struct ceph_options *ceph_opts)
> >    */
> >   enum {
> >          Opt_queue_depth,
> > +       Opt_state_lock_timeout,
> Opt_lock_timeout
>
> >          Opt_last_int,
> >          /* int args above */
> >          Opt_last_string,
> > @@ -739,6 +740,7 @@ enum {
> >
> >   static match_table_t rbd_opts_tokens = {
> >          {Opt_queue_depth, "queue_depth=%d"},
> > +       {Opt_state_lock_timeout, "state_lock_timeout=%d"},
> "lock_timeout=%d"
>
> >          /* int args above */
> >          /* string args above */
> >          {Opt_read_only, "read_only"},
> > @@ -752,6 +754,7 @@ enum {
> >
> >   struct rbd_options {
> >          int     queue_depth;
> > +       long    state_lock_timeout;
> unsigned long lock_timeout; /* jiffies */
>
> Look at how osd_request_timeout is handled in net/ceph/ceph_common.c.
> We store the value in jiffies and convert in ceph_parse_options().
>
> >          bool    read_only;
> >          bool    lock_on_read;
> >          bool    exclusive;
> > @@ -761,7 +764,7 @@ struct rbd_options {
> >   #define RBD_READ_ONLY_DEFAULT  false
> >   #define RBD_LOCK_ON_READ_DEFAULT false
> >   #define RBD_EXCLUSIVE_DEFAULT  false
> > -#define RBD_WAIT_LOCK_TIMEOUT_DEFAULT  MAX_SCHEDULE_TIMEOUT
> > +#define RBD_STATE_LOCK_TIMEOUT_DEFAULT (jiffies_to_msecs(MAX_SCHEDULE_TIMEOUT) / 1000)
> See my reply to the previous patch.
>
> >   static int parse_rbd_opts_token(char *c, void *private)
> >   {
> > @@ -791,6 +794,13 @@ static int parse_rbd_opts_token(char *c, void *private)
> >                  }
> >                  rbd_opts->queue_depth = intval;
> >                  break;
> > +       case Opt_state_lock_timeout:
> > +               if (intval < 0) {
> > +                       pr_err("state_lock_timeout out of range\n");
> > +                       return -EINVAL;
> > +               }
> > +               rbd_opts->state_lock_timeout = intval;
> > +               break;
> See case Opt_osdkeepalivetimeout in ceph_parse_options().
>
> >          case Opt_read_only:
> >                  rbd_opts->read_only = true;
> >                  break;
> > @@ -3497,7 +3507,8 @@ static int rbd_obj_method_sync(struct rbd_device *rbd_dev,
> >    */
> >   static int rbd_wait_state_locked(struct rbd_device *rbd_dev)
> >   {
> > -       long timeo = RBD_WAIT_LOCK_TIMEOUT_DEFAULT;
> > +       /* convert seconds to jiffies */
> > +       long timeo = msecs_to_jiffies(rbd_dev->opts->state_lock_timeout * 1000);
> schedule_timeout(ceph_timeout_jiffies(rbd_dev->opts->lock_timeout));
>
> No need for an extra variable assuming the conversion happens in
> ceph_parse_options().

But we need to get the return value of schedule_timeout(). so there is
still need for an variable.

Thanx
Yang
> Thanks,
>
>                  Ilya
> --
> To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at  http://vger.kernel.org/majordomo-info.html


--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html