On Fri, Jul 1, 2022 at 9:00 PM Tom Talpey <tom@xxxxxxxxxx> wrote: > > On 7/1/2022 7:00 AM, ronnie sahlberg wrote: > > On Fri, 1 Jul 2022 at 18:44, Shyam Prasad N <nspmangalore@xxxxxxxxx> wrote: > >> > >> On Fri, Jul 1, 2022 at 12:07 PM ronnie sahlberg > >> <ronniesahlberg@xxxxxxxxx> wrote: > >>> > >>> On Fri, 1 Jul 2022 at 16:03, Shyam Prasad N <nspmangalore@xxxxxxxxx> wrote: > >>>> > >>>> On Fri, Jul 1, 2022 at 3:23 AM Tom Talpey <tom@xxxxxxxxxx> wrote: > >>>>> > >>>>> Is there a justification for why this is necessary? > >>>>> > >>>>> When and how are admins expected to use it, and with what values? > >>>>> > >>>> > >>>> Hi Tom, > >>>> > >>>> This came up specifically when a customer reported an issue with lease break. > >>>> We wanted to rule out (or confirm) if deferred close is playing any > >>>> part in this by disabling it. > >>>> However, to disable it today, we will need to set acregmax to 0, which > >>>> will also disable attribute caching. > >>>> > >>>> So Bharath now submitted a patch for this to be able to tune this > >>>> parameter separately. > >>> > >>> Ok, will the option be removed later once the investigation is done? > >>> We shouldn't add options that are difficult/impossible to use > >>> correctly by normal users. > >> We didn't intend to. We thought that this could be a useful tunable > >> parameter that the basic users need not even worry about, but advanced > >> users / developers could suggest changing it to tune / troubleshoot > >> specific scenarios. > > > > If it is just for developers needing it to debug specific issues it > > should absolutely not be a mount option in upstream. > > Maybe have it as a /proc/fs/cifs/Debug thing or just provide custom > > builds for the customer when debugging specific issues. > > Absolutely agree. Upstream isn't a developer sandbox. > > I'll point out that this patch: > 1) Doesn't change any behavior as-is > 2) Doesn't give any guidance on values > 3) Doesn't update the userspace mount command > 4) Doesn't ever go away because it changes the ABI > 5) Increases the total number of cifs.ko mount options to 102. > > I'm not sure about that last one. It might be 103. > I understand that the concern is mainly around this being a mount option. So let's discuss what's the correct place for tuning such configurations? It could be a module parameter. It would make it global. But I guess that's okay. Thoughts? > > Once they become mount options they need to be documented, what they > > do and why you would use them and exactly how to determine what to set > > them to. > > > > > > > >>> > >>> > >>>> > >>>>> On 6/29/2022 4:26 AM, Bharath SM wrote: > >>>>>> Adding a new mount parameter to specifically control timeout for deferred close. > >>>> > >>>> > >>>> > >>>> -- > >>>> Regards, > >>>> Shyam > >> > >> > >> > >> -- > >> Regards, > >> Shyam > > -- Regards, Shyam
