On Mon, 4 Jul 2022 at 23:52, Shyam Prasad N <nspmangalore@xxxxxxxxx> wrote: > > 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? I think /proc/fs/cifs/Debug would be an appropriate place since it would make it obvious that it is just a debugging option and not something normal users should use. Perhaps something like making it writeable by root and be able to do : echo "dclosetomeo:55" >> /proc/fs/cifs/Debug > > > > 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
