Re: [PATCH] ib_core: Enable and expose force_mr module parameter

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



> On Jun 20, 2017, at 2:03 PM, Leon Romanovsky <leon@xxxxxxxxxx> wrote:
> 
> On Tue, Jun 20, 2017 at 11:43:56AM -0400, Chuck Lever wrote:
>> Hi Leon-
>> 
>> 
>>> On Jun 20, 2017, at 3:32 AM, Leon Romanovsky <leon@xxxxxxxxxx> wrote:
>>> 
>>> On Mon, Jun 19, 2017 at 11:26:40AM -0400, Chuck Lever wrote:
>>>> The fourth parameter of the module_param_named macro is a set of
>>>> file permissions. Passing 0 there means that module parameter is
>>>> not created and that adding "options ib_core force_mr=1" to a
>>>> modprobe.conf file has no effect.
>>>> 
>>>> The default setting of rdma_rw_force_mr continues to be 0, or false.
>>>> 
>>>> Fixes: a060b5629ab0 ("IB/core: generic RDMA READ/WRITE API")
>>>> Signed-off-by: Chuck Lever <chuck.lever@xxxxxxxxxx>
>>>> ---
>>>> Hi Doug-
>>>> 
>>>> This doesn't seem appropriate to go through Bruce's tree for 4.13.
>>>> 
>>>> Last discussion didn't seem to conclude with full consensus.
>>>> Probably people don't care enough one way or another. But I'd like
>>>> to see this get fixed if there aren't strong objections. Would you
>>>> take it for 4.13?
>>> 
>>> I care and believe that it should be removed from exposure to users.
>>> The variable force_mr should be leaved in the code for the debug,
>>> but module_param should be removed.
>> 
>> I don't understand the technical grounds of your objection.
>> Why is
>> 
>> options mlx4_core internal_err_reset=0
>> 
>> acceptable, but
>> 
>> options ib_core force_mr=1
>> 
>> not?
> 
> That mlx4 variable was exposed before I started to follow after
> Mellanox's submissions.
> 
> The force_mr parameter is for debug of new ULP and/or conversion of
> existing ULP to RW API. This is not very common situation and it is
> for the developers and not for the users.

No disagreement on that, but that's still not an argument
not to expose it. What is the risk, as you see it?


> Thanks
> 
>> 
>> 
>>> Thanks
>>> 
>>> 
>>>> 
>>>> 
>>>> drivers/infiniband/core/rw.c |    2 +-
>>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>> 
>>>> diff --git a/drivers/infiniband/core/rw.c b/drivers/infiniband/core/rw.c
>>>> index dbfd854..1cc8f07 100644
>>>> --- a/drivers/infiniband/core/rw.c
>>>> +++ b/drivers/infiniband/core/rw.c
>>>> @@ -23,7 +23,7 @@ enum {
>>>> };
>>>> 
>>>> static bool rdma_rw_force_mr;
>>>> -module_param_named(force_mr, rdma_rw_force_mr, bool, 0);
>>>> +module_param_named(force_mr, rdma_rw_force_mr, bool, 0644);
>>>> MODULE_PARM_DESC(force_mr, "Force usage of MRs for RDMA READ/WRITE operations");
>>>> 
>>>> /*
>>>> 
>>>> --
>>>> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
>>>> the body of a message to majordomo@xxxxxxxxxxxxxxx
>>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>> 
>> --
>> Chuck Lever

--
Chuck Lever



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



[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Photo]     [Yosemite News]     [Yosemite Photos]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux