Re: [PATCH] cifs: Add mount parameter to control deferred close timeout

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

 



On 7/4/2022 9:52 AM, Shyam Prasad N 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?

Ronnie suggested /proc/fs/cifs/Debug which sounds appropriate to me.

Let me be clear - if the mount option approach is properly justified,
thoroughly documented and integrated into mount.cifs, I'd be more
inclined to agree with it. Your patch description doesn't give me
confidence it will actually fix anything though. Convince me/us of
that, too.

Tom.

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







[Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux