Received-SPF: pass (google.com: domain of linux-nfs-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   Chuck Lever III <chuck.lever@oracle.com>
To:     Trond Myklebust <trondmy@hammerspace.com>
CC:     "jlayton@kernel.org" <jlayton@kernel.org>,
        "linux@stwm.de" <linux@stwm.de>,
        "gregkh@linuxfoundation.org" <gregkh@linuxfoundation.org>,
        "stable@vger.kernel.org" <stable@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        Linux NFS Mailing List <linux-nfs@vger.kernel.org>
Subject: Re: 5.4.188 and later: massive performance regression with nfsd
Thread-Topic: 5.4.188 and later: massive performance regression with nfsd
Thread-Index: AQHYZR5cM0piVXjf4kWw2Fv1L4O8aq0Zna6AgAAbcgCAAAISgIAAA5uAgA41xoCAABHMgIAAVxMAgAAJFACAABXugIABJ+MAgAANoACAABEJAA==
Date:   Sat, 21 May 2022 19:11:49 +0000
Message-ID: <7831BCDD-00A9-42E7-A533-4D6AFA488AEF@oracle.com>
References: <f8d9b9112607df4807fba8948ac6e145@stwm.de>
 <YnuuLZe6h80KCNhd@kroah.com>
 <6A15DEE1-CAC9-4C64-8643-AD28EA423046@oracle.com>
 <YnvG71v4Ay560D+X@kroah.com>
 <0F5EA15D-08A0-4D3E-B5A9-D62C49BF2B56@oracle.com>
 <59118799-0148-41F3-BA03-BB69151B74EE@oracle.com>
 <1792eb49d2a9820bbcf7b240f53158170041da6d.camel@hammerspace.com>
 <3FE1F779-A2EC-4E23-BBCC-28C5E8AFCBB1@oracle.com>
 <1a38a01debc727a1e11243fd6207d915ac90c251.camel@hammerspace.com>
 <FB8CBEF6-07B4-416C-B9B4-56E03C7E6628@oracle.com>
 <40D1CC9F-1DA7-4134-9DBD-0C0DFA16A361@oracle.com>
 <78739abdf5fe7aa0aab9b7f3c9d660010868b0f4.camel@hammerspace.com>
In-Reply-To: <78739abdf5fe7aa0aab9b7f3c9d660010868b0f4.camel@hammerspace.com>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?wV4Ufuhf77vIyhlo7PtleI9BVc49f/bTOLwdXfdEmvts4a9KcwZIRhh1ztX7?=
 =?us-ascii?Q?fUSH0mQ6HdmFQONMxrLaRaTEy+glh2/GTmAo7vbXquQ0XYurhZklp5VDkU6X?=
 =?us-ascii?Q?FMwV/8KP215xpqz4nJT31EGjeCBugGwNaFPu/mWFix5+Y1M3kkja15bsmcyT?=
 =?us-ascii?Q?nbgKtBvvlJvmN2l3BHF/9XvUPflzXTpLQqMidPCevt1dwtoRaB760xFGTHCq?=
 =?us-ascii?Q?TiiIJ8u3P8jn1mNvcU0ZcdHRo+/IlBg0LIv7G+187cah8w+nCGW+K5nyMhaD?=
 =?us-ascii?Q?sHtSLqKRXBfxbsUPuogHCqnih9An4ls8gjbc+4JTg/mk1swXTE42CZ5JAfai?=
 =?us-ascii?Q?euNlupVTTg3ccKBXdPVn5CNsk8K20WMUUmUtsJVSiChTVqF8+iJeEwpqQ7Bk?=
 =?us-ascii?Q?ZnyEGIqRS32BBM6EGGA1OBuKyxIwL7U+Wrp4xJrzua7h3CK23DXB6T/OHKBp?=
 =?us-ascii?Q?D8MQVqzgmpQ6wmegZovezVnhfcuJADdBFgwRumZAbKCWAU+TsndDmDkYhUDo?=
 =?us-ascii?Q?h0FcfIV6Ll1DO5+mYjV8HOVSHJKd0vXSEqVyUcphoQsofExPwueEJpG9nZIn?=
 =?us-ascii?Q?aqLcQngCMINNNz7LQgv60tk5/y3R2qNSD7EENqSyf5L1EZqplsgjI+NgibQL?=
 =?us-ascii?Q?xOHUAFSqCTBiFCAx+jPIJEzPa71yDigHeAtDhXOxDGTXfsgFxbGuqC/+Mte4?=
 =?us-ascii?Q?WXIP4SLG04w132PIp5NvuFPbKo0OiSlpN/lOb1MDr/kbhOOlXFnvMauZUQnD?=
 =?us-ascii?Q?Pt75Tc9sGOoxToO9hzRaZDAprGJEkfhl6ucAr0Qut+OnqxWAZ04v07Efh6Ql?=
 =?us-ascii?Q?PQU+khnWRWcQwgcsw4Ytt0rgCOEtQcfio7LoYBI/vNkZUOXJJIF7YWjtSBPk?=
 =?us-ascii?Q?wK3Y/Leo1kIhgptKsURtkYObfDp+z3VrdW+kdehpLlFHSEdBSMJD8Lae78jr?=
 =?us-ascii?Q?ySogdKjjDr7VmxXNQsg/NcvcMIN+0GtRwMBImGkcfNEKkgElQAyeCLbp45nY?=
 =?us-ascii?Q?wpqEBjNInD4R1CIJGmmGwSMmPrxE9bMC0HWxsJgRYGeuvq6u0oviRESf9HmT?=
 =?us-ascii?Q?59wbgB+K07uBcLae1MhgXFpWwkax1zNREkduIdl7Uqg+ikBf5Vg+kC6DUPaW?=
 =?us-ascii?Q?tL9ghHSJN825BE0ol8Mq/StDtdJ4cOTBVpXIpWTdHjX1MNCnzs+cyaTJwhy9?=
 =?us-ascii?Q?9vxTAeFjokyixdmtne22kfGKSTyG/A8v2Xysp+GkGSQYpZgjp7WX/QRSYyMb?=
 =?us-ascii?Q?PMka/WsxmNrlJNJ4JM5RgTYOUzMstr6bNCJ8acgqMk+EWqI5ttijcYGtCAPc?=
 =?us-ascii?Q?jZ+igTTE1VmLa4ZPYoNPqhfo2HLaOpsPTO5i3jBcg4VUW/yr3knv5bp9ROYJ?=
 =?us-ascii?Q?rXkyxIyWswpaZb3OWCCsq5fbXaNsGCrlEJdNeekQ/jmFfXia1Jaiah4ThwkK?=
 =?us-ascii?Q?L552PWzgByQKRB5n0GilknhX8ajqCg2DKvThCXcxRaZehuF4pAuzGynzO9T/?=
 =?us-ascii?Q?ZO+CH/hHV0YOgjwC+7W+A5yg0N8r63Sh9d9QbsHCBMqE071ga+Q9r1s920bV?=
 =?us-ascii?Q?hdyB96XUXkKj+udmTGm8ZeVstKw6Dp08omu5DVYU5XUAqcXeCZ1L/SAFgMsP?=
 =?us-ascii?Q?twioxdpaBL8IvUzgEqk7grthIvkLQ/a2+RqsTEWpyr0UInAa9RDYV8TsMbM+?=
 =?us-ascii?Q?IGiZRsEPraXkoAdzXHGz3gR5muKuzP9sMqwhsMDDKQZb1Um0mAaKPezE03V8?=
 =?us-ascii?Q?OtOuuOmY9v3EtT7lLRMaYX37jzxsCeE=3D?=
Content-Type: text/plain; charset="us-ascii"
Content-ID: <DB25B1A1F9AE7D4AA83CC2729BE3313C@namprd10.prod.outlook.com>
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: BN0PR10MB5128.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 5520099f-0e83-4ece-e6f5-08da3b5dc1ae
X-MS-Exchange-CrossTenant-originalarrivaltime: 21 May 2022 19:11:49.3340
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 4e2c6054-71cb-48f1-bd6c-3a9705aca71b
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: 88MpwRQM+UUmQzSmauQ02kOk7SUXjcezVzqm6VQzI3D47bIDW1nn84plpJ+QlP8ZZaOVxAJg+SstS5A2FDLLvQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM5PR10MB1242
X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:6.0.486,18.0.874
 definitions=2022-05-21_06:2022-05-20,2022-05-21 signatures=0
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 suspectscore=0 adultscore=0 bulkscore=0
 mlxlogscore=999 mlxscore=0 malwarescore=0 phishscore=0 spamscore=0
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2202240000
 definitions=main-2205210123
X-Proofpoint-ORIG-GUID: yjnfrspt2AoYoHpSlgmCdsefLRpylLgI
X-Proofpoint-GUID: yjnfrspt2AoYoHpSlgmCdsefLRpylLgI
X-Spam-Status: No, score=-2.8 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_LOW,
        RCVD_IN_MSPIKE_H2,SPF_HELO_NONE,SPF_NONE,T_SCC_BODY_TEXT_LINE
        autolearn=ham autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-nfs.vger.kernel.org>
X-Mailing-List: linux-nfs@vger.kernel.org


> On May 21, 2022, at 2:10 PM, Trond Myklebust <trondmy@hammerspace.com> wr=
ote:
>=20
> On Sat, 2022-05-21 at 17:22 +0000, Chuck Lever III wrote:
>>=20
>>=20
>>> On May 20, 2022, at 7:43 PM, Chuck Lever III
>>> <chuck.lever@oracle.com> wrote:
>>>=20
>>>=20
>>>=20
>>>> On May 20, 2022, at 6:24 PM, Trond Myklebust
>>>> <trondmy@hammerspace.com> wrote:
>>>>=20
>>>> On Fri, 2022-05-20 at 21:52 +0000, Chuck Lever III wrote:
>>>>>=20
>>>>>=20
>>>>>> On May 20, 2022, at 12:40 PM, Trond Myklebust
>>>>>> <trondmy@hammerspace.com> wrote:
>>>>>>=20
>>>>>> On Fri, 2022-05-20 at 15:36 +0000, Chuck Lever III wrote:
>>>>>>>=20
>>>>>>>=20
>>>>>>>> On May 11, 2022, at 10:36 AM, Chuck Lever III
>>>>>>>> <chuck.lever@oracle.com> wrote:
>>>>>>>>=20
>>>>>>>>=20
>>>>>>>>=20
>>>>>>>>> On May 11, 2022, at 10:23 AM, Greg KH
>>>>>>>>> <gregkh@linuxfoundation.org> wrote:
>>>>>>>>>=20
>>>>>>>>> On Wed, May 11, 2022 at 02:16:19PM +0000, Chuck Lever
>>>>>>>>> III
>>>>>>>>> wrote:
>>>>>>>>>>=20
>>>>>>>>>>=20
>>>>>>>>>>> On May 11, 2022, at 8:38 AM, Greg KH
>>>>>>>>>>> <gregkh@linuxfoundation.org> wrote:
>>>>>>>>>>>=20
>>>>>>>>>>> On Wed, May 11, 2022 at 12:03:13PM +0200, Wolfgang
>>>>>>>>>>> Walter
>>>>>>>>>>> wrote:
>>>>>>>>>>>> Hi,
>>>>>>>>>>>>=20
>>>>>>>>>>>> starting with 5.4.188 wie see a massive
>>>>>>>>>>>> performance
>>>>>>>>>>>> regression on our
>>>>>>>>>>>> nfs-server. It basically is serving requests very
>>>>>>>>>>>> very
>>>>>>>>>>>> slowly with cpu
>>>>>>>>>>>> utilization of 100% (with 5.4.187 and earlier it
>>>>>>>>>>>> is
>>>>>>>>>>>> 10%) so
>>>>>>>>>>>> that it is
>>>>>>>>>>>> unusable as a fileserver.
>>>>>>>>>>>>=20
>>>>>>>>>>>> The culprit are commits (or one of it):
>>>>>>>>>>>>=20
>>>>>>>>>>>> c32f1041382a88b17da5736886da4a492353a1bb "nfsd:
>>>>>>>>>>>> cleanup
>>>>>>>>>>>> nfsd_file_lru_dispose()"
>>>>>>>>>>>> 628adfa21815f74c04724abc85847f24b5dd1645 "nfsd:
>>>>>>>>>>>> Containerise filecache
>>>>>>>>>>>> laundrette"
>>>>>>>>>>>>=20
>>>>>>>>>>>> (upstream
>>>>>>>>>>>> 36ebbdb96b694dd9c6b25ad98f2bbd263d022b63 and
>>>>>>>>>>>> 9542e6a643fc69d528dfb3303f145719c61d3050)
>>>>>>>>>>>>=20
>>>>>>>>>>>> If I revert them in v5.4.192 the kernel works as
>>>>>>>>>>>> before
>>>>>>>>>>>> and
>>>>>>>>>>>> performance is
>>>>>>>>>>>> ok again.
>>>>>>>>>>>>=20
>>>>>>>>>>>> I did not try to revert them one by one as any
>>>>>>>>>>>> disruption
>>>>>>>>>>>> of our nfs-server
>>>>>>>>>>>> is a severe problem for us and I'm not sure if
>>>>>>>>>>>> they are
>>>>>>>>>>>> related.
>>>>>>>>>>>>=20
>>>>>>>>>>>> 5.10 and 5.15 both always performed very badly on
>>>>>>>>>>>> our
>>>>>>>>>>>> nfs-
>>>>>>>>>>>> server in a
>>>>>>>>>>>> similar way so we were stuck with 5.4.
>>>>>>>>>>>>=20
>>>>>>>>>>>> I now think this is because of
>>>>>>>>>>>> 36ebbdb96b694dd9c6b25ad98f2bbd263d022b63
>>>>>>>>>>>> and/or 9542e6a643fc69d528dfb3303f145719c61d3050
>>>>>>>>>>>> though
>>>>>>>>>>>> I
>>>>>>>>>>>> didn't tried to
>>>>>>>>>>>> revert them in 5.15 yet.
>>>>>>>>>>>=20
>>>>>>>>>>> Odds are 5.18-rc6 is also a problem?
>>>>>>>>>>=20
>>>>>>>>>> We believe that
>>>>>>>>>>=20
>>>>>>>>>> 6b8a94332ee4 ("nfsd: Fix a write performance
>>>>>>>>>> regression")
>>>>>>>>>>=20
>>>>>>>>>> addresses the performance regression. It was merged
>>>>>>>>>> into
>>>>>>>>>> 5.18-
>>>>>>>>>> rc.
>>>>>>>>>=20
>>>>>>>>> And into 5.17.4 if someone wants to try that release.
>>>>>>>>=20
>>>>>>>> I don't have a lot of time to backport this one myself,
>>>>>>>> so
>>>>>>>> I welcome anyone who wants to apply that commit to their
>>>>>>>> favorite LTS kernel and test it for us.
>>>>>>>>=20
>>>>>>>>=20
>>>>>>>>>>> If so, I'll just wait for the fix to get into
>>>>>>>>>>> Linus's
>>>>>>>>>>> tree as
>>>>>>>>>>> this does
>>>>>>>>>>> not seem to be a stable-tree-only issue.
>>>>>>>>>>=20
>>>>>>>>>> Unfortunately I've received a recent report that the
>>>>>>>>>> fix
>>>>>>>>>> introduces
>>>>>>>>>> a "sleep while spinlock is held" for NFSv4.0 in rare
>>>>>>>>>> cases.
>>>>>>>>>=20
>>>>>>>>> Ick, not good, any potential fixes for that?
>>>>>>>>=20
>>>>>>>> Not yet. I was at LSF last week, so I've just started
>>>>>>>> digging
>>>>>>>> into this one. I've confirmed that the report is a real
>>>>>>>> bug,
>>>>>>>> but we still don't know how hard it is to hit it with
>>>>>>>> real
>>>>>>>> workloads.
>>>>>>>=20
>>>>>>> We believe the following, which should be part of the first
>>>>>>> NFSD pull request for 5.19, will properly address the
>>>>>>> splat.
>>>>>>>=20
>>>>>>> https://git.kernel.org/pub/scm/linux/kernel/git/cel/linux.git/commi=
t/?h=3Dfor-next&id=3D556082f5e5d7ecfd0ee45c3641e2b364bff9ee44
>>>>>>>=20
>>>>>>>=20
>>>>>> Uh... What happens if you have 2 simultaneous calls to
>>>>>> nfsd4_release_lockowner() for the same file? i.e. 2 separate
>>>>>> processes
>>>>>> owned by the same user, both locking the same file.
>>>>>>=20
>>>>>> Can't that cause the 'putlist' to get corrupted when both
>>>>>> callers
>>>>>> add
>>>>>> the same nf->nf_putfile to two separate lists?
>>>>>=20
>>>>> IIUC, cl_lock serializes the two RELEASE_LOCKOWNER calls.
>>>>>=20
>>>>> The first call finds the lockowner in cl_ownerstr_hashtbl and
>>>>> unhashes it before releasing cl_lock.
>>>>>=20
>>>>> Then the second cannot find that lockowner, thus it can't
>>>>> requeue it for bulk_put.
>>>>>=20
>>>>> Am I missing something?
>>>>=20
>>>> In the example I quoted, there are 2 separate processes running
>>>> on the
>>>> client. Those processes could share the same open owner + open
>>>> stateid,
>>>> and hence the same struct nfs4_file, since that depends only on
>>>> the
>>>> process credentials matching. However they will not normally
>>>> share a
>>>> lock owner, since POSIX does not expect different processes to
>>>> share
>>>> locks.
>>>>=20
>>>> IOW: The point is that one can relatively easily create 2
>>>> different
>>>> lock owners with different lock stateids that share the same
>>>> underlying
>>>> struct nfs4_file.
>>>=20
>>> Is there a similar exposure if two different clients are locking
>>> the same file? If so, then we can't use a per-nfs4_client semaphore
>>> to serialize access to the nf_putfile field.
>>=20
>> I had a thought about an alternate approach.
>>=20
>> Create a second nfsd_file_put API that is not allowed to sleep.
>> Let's call it "nfsd_file_put_async()". Teach check_for_locked()
>> to use that instead of nfsd_file_put().
>>=20
>> Here's where I'm a little fuzzy: nfsd_file_put_async() could do
>> something like:
>>=20
>> void nfsd_file_put_async(struct nfsd_file *nf)
>> {
>>         if (refcount_dec_and_test(&nf->nf_ref))
>>                 nfsd_file_close_inode(nf->nf_inode);
>> }
>>=20
>>=20
>=20
> That approach moves the sync to the garbage collector, which was
> exactly what we're trying to avoid in the first place.

Totally understood.

My thought was that "put" for RELEASE_LOCKOWNER/FREE_STATEID
would be unlikely to have any data to sync -- callers that
actually have data to flush are elsewhere, and those would
continue to use the synchronous nfsd_file_put() API.

Do you have a workload where we can test this assumption?


> Why not just do this "check_for_locks()" thing differently?
>=20
> It really shouldn't be too hard to add something to
> nfsd4_lm_get_owner()/nfsd4_lm_put_owner() that bumps a counter in the
> lockowner in order to tell you whether or not locks are still held
> instead of doing this bone headed walk through the list of locks.

I thought of that a couple weeks ago. That doesn't work
because you can lock or unlock by range. That means the
symmetry of LOCK and LOCKU is not guaranteed, and I don't
believe these calls are used that way anyway. So I
abandoned the idea of using get_owner / put_owner.

But maybe we can provide some other mechanism to record
whether a lockowner is associated with file locks.


--
Chuck Lever