> On Jun 28, 2017, at 5:56 AM, Leon Romanovsky <leon@xxxxxxxxxx> wrote: > > On Tue, Jun 27, 2017 at 12:24:20PM +0300, Sagi Grimberg wrote: >> >>>> It will be "marked" as UAPI and we won't be able to change it, >>>> which IMHO bad for the debug. It also invites users to set it >>>> and in perfect world, we should test our HCAs with this flag >>>> on and off, because curious user can set it. >>> >>> We have loads of legacy debugging interfaces. >>> >>> And yes, ULP developers should be aware of it and test with >>> both settings. >>> >>> I'm not convinced any of this is a problem for this particular >>> setting. However, I guess the thing for you to do is propose a >>> patch. >> >> Are you guys seriously still on this? Its simply a trivial >> debug flag, whats all the fuss really about? > > I hope that we are done. > https://patchwork.kernel.org/patch/9808615/ Somehow this got by me. The patch description says "exposed to regular users." The force_mr option is not exposed to regular users. Module parameters are visible only to system administrators. Also, just looking at the patch, it's not obvious why "The force_mr module parameter wasn't exposed ... from the beginning". IMO an explanation of what has prevented exposure is needed. Commit a060b5629ab0 got the fourth parameter of module_param_named wrong. Maybe a Fixes: tag is needed too. But frankly there is no visible change from this patch. Before the patch, force_mr is not visible, and after the patch it is not visible. Why then is this change necessary? It would be helpful to cite a policy about why removal is preferred over fixing the incorrect parameter. Therefore IMO the patch description is not adequate. -- 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
