On Wed, Jun 28, 2017 at 10:49:00AM -0400, Chuck Lever wrote: > > > 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. I'll update it. Thanks > > > -- > Chuck Lever > > >
Attachment:
signature.asc
Description: PGP signature
