> On May 18, 2023, at 2:01 PM, Dai Ngo <dai.ngo@xxxxxxxxxx> wrote: > > > On 5/18/23 10:16 AM, Chuck Lever III wrote: >> >>> On May 18, 2023, at 1:11 PM, Dai Ngo <dai.ngo@xxxxxxxxxx> wrote: >>> >>> >>> On 5/18/23 6:23 AM, Chuck Lever III wrote: >>>>> On May 17, 2023, at 7:38 PM, Dai Ngo <dai.ngo@xxxxxxxxxx> wrote: >>>>> >>>>> This patch grants write delegation for OPEN with NFS4_SHARE_ACCESS_WRITE >>>>> if there is no conflict with other OPENs. >>>>> >>>>> Write delegation conflict with another OPEN, REMOVE, RENAME and SETATTR >>>>> are handled the same as read delegation using notify_change, >>>>> try_break_deleg. >>>> Very clean. A couple of suggestions, one is down below, and here is >>>> the other: >>>> >>>> I was thinking we should add one or two counters in fs/nfsd/stats.c >>>> to track how often read and write delegations are offered, and >>>> perhaps one to count the number of DELEGRETURN operations. What do >>>> you think makes sense? >>> I'm not sure what these counters will tell us, currently we already >>> has a counter for number of delegations handed out. >> I haven't found that, where is it? Certainly, if NFSD already >> has one, then no need to add more. > > num_delegations in nfs4state.c > >> >> It would be nice one day, perhaps, to have a metric of how many >> delegations a client holds. That's not for this series. > > okay. > >> >> >>> I think a counter >>> on how often nfsd has to recall the write delegation due to GETATTR can >>> be useful to know whether we should implement CB_GETATTR. >> I hesitated to mention that because I wonder if that's something >> that would be interesting only for defending a design choice, >> not for site-to-site tuning. In other words, after we plumb it >> into NFSD, it will never actually be used after CB_GETATTR >> support is added. >> >> Do you believe it's something that administrators can use to >> help balance or tune their workloads? > > You're right. That is just for ourselves to determine if CB_GETATTR > is needed. To be clear, such a counter, I agree, would be useful /to us/. I'm just not sure how we could add something that would not become part of the kernel/userspace API. Anyone have thoughts about that? -- Chuck Lever
