On Sun, Aug 11, 2024 at 10:38 AM Leon Romanovsky <leon@xxxxxxxxxx> wrote:
>
> On Fri, Aug 09, 2024 at 03:15:26PM +0200, Md Haris Iqbal wrote:
> > Do not allow opening RTRS server if it is already in use and print
> > proper error message.
>
> 1. How is it even possible? I see only one call to rtrs_srv_open() and
> it is happening when the driver is loaded.
rtrs_srv_open() is NOT called during RTRS driver load. It is called
during RNBD driver load, which is a client which uses RTRS.
RTRS server currently works with only a single client. Hence if, while
in use by RNBD, another driver wants to use RTRS and calls
rtrs_srv_open(), it should fail.
> 2. You already print an error message, why do you need to add another
> one?
This patch adds only a single error print, in function
rtrs_srv_open(). And it has not other print message. Am I missing
something?
>
> Thanks
>
> >
> > Signed-off-by: Md Haris Iqbal <haris.iqbal@xxxxxxxxx>
> > Signed-off-by: Jack Wang <jinpu.wang@xxxxxxxxx>
> > Signed-off-by: Grzegorz Prajsner <grzegorz.prajsner@xxxxxxxxx>
> > ---
> > drivers/infiniband/ulp/rtrs/rtrs-srv.c | 27 +++++++++++++++++++++++---
> > drivers/infiniband/ulp/rtrs/rtrs-srv.h | 1 +
> > 2 files changed, 25 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/infiniband/ulp/rtrs/rtrs-srv.c b/drivers/infiniband/ulp/rtrs/rtrs-srv.c
> > index 1d33efb8fb03..fb67b58a7f62 100644
> > --- a/drivers/infiniband/ulp/rtrs/rtrs-srv.c
> > +++ b/drivers/infiniband/ulp/rtrs/rtrs-srv.c
> > @@ -2174,9 +2174,18 @@ struct rtrs_srv_ctx *rtrs_srv_open(struct rtrs_srv_ops *ops, u16 port)
> > struct rtrs_srv_ctx *ctx;
> > int err;
> >
> > + mutex_lock(&ib_ctx.rtrs_srv_ib_mutex);
> > + if (ib_ctx.srv_ctx) {
> > + pr_err("%s: Already in use.\n", __func__);
> > + ctx = ERR_PTR(-EEXIST);
> > + goto out;
> > + }
> > +
> > ctx = alloc_srv_ctx(ops);
> > - if (!ctx)
> > - return ERR_PTR(-ENOMEM);
> > + if (!ctx) {
> > + ctx = ERR_PTR(-ENOMEM);
> > + goto out;
> > + }
> >
> > mutex_init(&ib_ctx.ib_dev_mutex);
> > ib_ctx.srv_ctx = ctx;
> > @@ -2185,9 +2194,11 @@ struct rtrs_srv_ctx *rtrs_srv_open(struct rtrs_srv_ops *ops, u16 port)
> > err = ib_register_client(&rtrs_srv_client);
> > if (err) {
> > free_srv_ctx(ctx);
> > - return ERR_PTR(err);
> > + ctx = ERR_PTR(err);
> > }
> >
> > +out:
> > + mutex_unlock(&ib_ctx.rtrs_srv_ib_mutex);
> > return ctx;
> > }
> > EXPORT_SYMBOL(rtrs_srv_open);
> > @@ -2221,10 +2232,16 @@ static void close_ctx(struct rtrs_srv_ctx *ctx)
> > */
> > void rtrs_srv_close(struct rtrs_srv_ctx *ctx)
> > {
> > + mutex_lock(&ib_ctx.rtrs_srv_ib_mutex);
> > + WARN_ON(ib_ctx.srv_ctx != ctx);
> > +
> > ib_unregister_client(&rtrs_srv_client);
> > mutex_destroy(&ib_ctx.ib_dev_mutex);
> > close_ctx(ctx);
> > free_srv_ctx(ctx);
> > +
> > + ib_ctx.srv_ctx = NULL;
> > + mutex_unlock(&ib_ctx.rtrs_srv_ib_mutex);
> > }
> > EXPORT_SYMBOL(rtrs_srv_close);
> >
> > @@ -2282,6 +2299,9 @@ static int __init rtrs_server_init(void)
> > goto out_dev_class;
> > }
> >
> > + mutex_init(&ib_ctx.rtrs_srv_ib_mutex);
> > + ib_ctx.srv_ctx = NULL;
> > +
> > return 0;
> >
> > out_dev_class:
> > @@ -2292,6 +2312,7 @@ static int __init rtrs_server_init(void)
> >
> > static void __exit rtrs_server_exit(void)
> > {
> > + mutex_destroy(&ib_ctx.rtrs_srv_ib_mutex);
> > destroy_workqueue(rtrs_wq);
> > class_unregister(&rtrs_dev_class);
> > rtrs_rdma_dev_pd_deinit(&dev_pd);
> > diff --git a/drivers/infiniband/ulp/rtrs/rtrs-srv.h b/drivers/infiniband/ulp/rtrs/rtrs-srv.h
> > index 5e325b82ff33..4924dde0a708 100644
> > --- a/drivers/infiniband/ulp/rtrs/rtrs-srv.h
> > +++ b/drivers/infiniband/ulp/rtrs/rtrs-srv.h
> > @@ -127,6 +127,7 @@ struct rtrs_srv_ib_ctx {
> > u16 port;
> > struct mutex ib_dev_mutex;
> > int ib_dev_count;
> > + struct mutex rtrs_srv_ib_mutex;
> > };
> >
> > extern const struct class rtrs_dev_class;
> > --
> > 2.25.1
> >
