Re: [snitzer:cel-nfsd-next-6.12-rc5] [nfsd] b4be3ccf1c: fsmark.app_overhead 92.0% regression

[Chuck Lever <chuck.lever@oracle.com>]

On 12/5/24 2:27 AM, Oliver Sang wrote:
> hi, Chuck Lever,
> 
> On Tue, Dec 03, 2024 at 02:54:47PM +0000, Chuck Lever III wrote:
>>
>>
>>> On Dec 3, 2024, at 12:01 AM, Oliver Sang <oliver.sang@intel.com> wrote:
>>>
>>> hi, Chuck Lever,
>>>
>>> On Mon, Dec 02, 2024 at 03:40:49PM +0000, Chuck Lever III wrote:
>>>>
>>>>
>>>>> On Nov 18, 2024, at 2:53 AM, Oliver Sang <oliver.sang@intel.com> wrote:
>>>>>
>>>>> hi, Chuck Lever,
>>>>>
>>>>> On Thu, Nov 14, 2024 at 09:37:30AM -0500, Chuck Lever wrote:
>>>>>> On Thu, Nov 07, 2024 at 12:55:02PM +0800, kernel test robot wrote:
>>>>>>>
>>>>>>> hi, Jeff Layton,
>>>>>>>
>>>>>>> in commit message, it is mentioned the change is expected to solve the
>>>>>>> "App Overhead" on the fs_mark test we reported in
>>>>>>> https://lore.kernel.org/oe-lkp/202409161645.d44bced5-oliver.sang@intel.com/
>>>>>>>
>>>>>>> however, in our tests, there is sill similar regression. at the same
>>>>>>> time, there is still no performance difference for fsmark.files_per_sec
>>>>>>>
>>>>>>>   2015880 ±  3%     +92.0%    3870164        fsmark.app_overhead
>>>>>>>     18.57            +0.0%      18.57        fsmark.files_per_sec
>>>>>>
>>>>>> Because the most recent version of this commit does not appear to
>>>>>> address the reported regression, Jeff suggested dropping it. I've
>>>>>> done that in the branch below, which is being prepared for v6.13.
>>>>>>
>>>>>>
>>>>>>> another thing is our bot bisect to this commit in repo/branch as below detail
>>>>>>> information. if there is a more porper repo/branch to test the patch, could
>>>>>>> you let us know? thanks a lot!
>>>>>>
>>>>>> The proper branch to test is:
>>>>>>
>>>>>> https://git.kernel.org/pub/scm/linux/kernel/git/cel/linux.git/log/?h=nfsd-next
>>>>>
>>>>> thanks for information! this repo is under bot's monitoring.
>>>>> unfortunately, our cluster run into problems these days and need some time
>>>>> to full back. so if there is still regression reports, maybe will delay to
>>>>> a later time.
>>>>
>>>> Hello Oliver -
>>>>
>>>> To avoid regressing v6.13, we postponed this series.
>>>>
>>>> The series is now in
>>>>
>>>>   https://git.kernel.org/pub/scm/linux/kernel/git/cel/linux.git/log/?h=nfsd-testing
>>>>
>>>> And I would like the robot to include that branch in regular
>>>> test runs going forward. I will try to push new patches there
>>>> first before pushing them to nfsd-next.
>>>
>>> got it. this nfsd-testing is already in our monitor.
>>>
>>> 07b2337dc158fb (cel/nfsd-testing) nfsd: add support for FATTR4_OPEN_ARGUMENTS
>>> 54b0e9c3727d2f nfs_common: make include/linux/nfs4.h include generated nfs4_1.h
>>> fcc756b92ce832 nfsd: fix handling of delegated change attr in CB_GETATTR
>>> a354743d35b49b nfsd: trace: remove redundant stateid even deleg_recall
>>> 40384c840ea194 (tag: v6.13-rc1,
>>
>> That's not yet a complete set of revised patches. We'll get
>> there in a few days.
> 
> got it. thanks for information.
> 
>>
>>
>>> at the same time, I triggered same tests for original report upon 07b2337dc158fb
>>> and v6.13-rc1, will update you results later.
> 
> if just comparing 07b2337dc158fb with v6.13-rc1, there is no big diff for
> fsmark.app_overhead.
> 
>         v6.13-rc1 07b2337dc158fb33e7af288b74c
> ---------------- ---------------------------
>           %stddev     %change         %stddev
>               \          |                \
>     3110649            -1.3%    3070867        fsmark.app_overhead
>       18.58            -0.3%      18.52        fsmark.files_per_sec
> 
> but as above, seems the 07b2337dc158fb is not the correct commit to test?

> we noticed it's now
> 
> b95256b0edb43 (cel/nfsd-testing) nfsd: prepare delegation code for handing out *_ATTRS_DELEG delegations
> 77152578a5c16 nfsd: rename NFS4_SHARE_WANT_* constants to OPEN4_SHARE_ACCESS_WANT_*
> 019cb8ea1e9d4 nfsd: switch to autogenerated definitions for open_delegation_type4
> 07b2337dc158f nfsd: add support for FATTR4_OPEN_ARGUMENTS
> 54b0e9c3727d2 nfs_common: make include/linux/nfs4.h include generated nfs4_1.h
> fcc756b92ce83 nfsd: fix handling of delegated change attr in CB_GETATTR
> a354743d35b49 nfsd: trace: remove redundant stateid even deleg_recall
> 40384c840ea19 (tag: v6.13-rc1,
> 
> if you need us test b95256b0edb43, please let us know. or you could request us
> when the branch is ready. thanks!

Earlier this week I thought I had the full set of patches to
test and could just apply them on v6.13-rc1, but it turns out
that I don't have them all yet. There are one or two that I
broke while trying to prepare for v6.12, and I've misplaced
Jeff's originals.

I will let you know when that branch is ready for a retest.
Sorry for the noise!


-- 
Chuck Lever