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:     Neil Brown <neilb@suse.de>
CC:     Chuck Lever <cel@kernel.org>,
        Linux NFS Mailing List <linux-nfs@vger.kernel.org>,
        "lorenzo@kernel.org" <lorenzo@kernel.org>,
        Jeff Layton <jlayton@redhat.com>,
        "david@fromorbit.com" <david@fromorbit.com>
Subject: Re: [PATCH v3 5/9] SUNRPC: Count pool threads that were awoken but
 found no work to do
Thread-Topic: [PATCH v3 5/9] SUNRPC: Count pool threads that were awoken but
 found no work to do
Thread-Index: AQHZs02GG6qEobU1aE6J2mdZLN/TsK+zlUyAgAAGRYA=
Date:   Mon, 10 Jul 2023 22:52:07 +0000
Message-ID: <D20E4286-30D2-461D-B856-41D3C53C756C@oracle.com>
References: <168900729243.7514.15141312295052254929.stgit@manet.1015granger.net>
 <168900734678.7514.887270657845753276.stgit@manet.1015granger.net>
 <168902815363.8939.4838920400288577480@noble.neil.brown.name>
In-Reply-To: <168902815363.8939.4838920400288577480@noble.neil.brown.name>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?cXvpUlHtlSNnMgmaDRRW+pMA+ExGR1d9XPHP7d7kwU2vQ8LBC/bm1j96ex6D?=
 =?us-ascii?Q?Provi4Ii0jwYOMtFFnmbYrp4iLSHJhDf8gngaZSjNXUCRgy8ghb0ANTe6mId?=
 =?us-ascii?Q?vj1XCUTNkEIze5LADZVTngaC+NLqc0Gw/gnfP9AOeYo44XhPOrETKctZbHVT?=
 =?us-ascii?Q?KVdqHs0uIgzXZFqcMhWqxALbCn7dtP3vxV/TGHcb/q1f8G3z/3LxT4iTKIGi?=
 =?us-ascii?Q?nq5cKgAL7rwvOnV/Gcl0BL5R++ic3vMR0rweyhPUYzmGYR7bOHjcB2mu8UcO?=
 =?us-ascii?Q?7C4JAib86gREAOWGsaxrFacL6qfguiUu3/doNkgTUROZ3iQrTpb9W8ukpwAf?=
 =?us-ascii?Q?SUB0MtOihtXngg/KMZmY7SPCrQXseAPWELUDI06VHCcP7M8ii1HMCh6ZXJhm?=
 =?us-ascii?Q?+kvGirkOO8t2tyr1+6TP5sP++vbdSN98NBIP1T9zE0Tda0oq9QyhvvIr6835?=
 =?us-ascii?Q?MhTrBnYEQkiER8MhGheTim0huX3ADMyKXNgkrPHODAJJp40J6DIMgZeIP34w?=
 =?us-ascii?Q?ILp+J60o70X+TCCnCWfh7fo9qpskWNivbw2GXJM6IxMGeHhNlWU5jC3plBGi?=
 =?us-ascii?Q?0Z9/lDJIj9WGrzOf0MUjsQFtQ0pq/dY+Y7hEOf+TF2VunzTDk40arMmzKC+c?=
 =?us-ascii?Q?c7EgXKfQqXFi1tLXSmsb94mw3tHHZY67aV70xedEDbu3tQo9s5Mwf3iVjo2c?=
 =?us-ascii?Q?ix3EiBJKfObCFqhx9cugd1gd4zZhVSmvW8mVKZUp6QHAqGaQfrXLMN1AvSW/?=
 =?us-ascii?Q?j8eZ2tjVelFmlQtVCyMkpcZinXhl686beXre6iwtCrir7juBzPDKW+E91OsW?=
 =?us-ascii?Q?6R6TSI659l6Tb+SAcGxbB9I6DHkgRRxXNYm+GFqvF9zXjxI3PHD4v3RKk9cK?=
 =?us-ascii?Q?yt1psQTpdbXBVEq8HW2WaNNoeMnZHjjTeuP2W0xXXjxBnWXEr+6tRMOqA9J2?=
 =?us-ascii?Q?oDLEHY60f6hi6dCGerd3wVEpfyW0mZSHQugYGajb6+C71KjZSIy2q8gFWcfi?=
 =?us-ascii?Q?fhPmwfjoWJEEoaTyjT1NFvrynvrkd0N4RvJHm9dMNKN6TQHBIRgMEdFWEioC?=
 =?us-ascii?Q?u/6pKKxxesJ3sKzEZXV/uwHk2Wbs/tlpPYjtPWOMzzMZAn4KgkXdD6FB+r9n?=
 =?us-ascii?Q?nzifpTFaJIyByo1c+kCMY8+ZBSWK+ME7Ec4wbdOg3ZtDLJFKiQc8Qtn95Y9f?=
 =?us-ascii?Q?f0Z43wMridSubgS+tVTwUUZkY5Lvis0E6OfeLKThq+P6c9GCWbDLZ5gdKEj/?=
 =?us-ascii?Q?YjwKokoygzlB+T5QyInh7bPXOc8cwewO2P51X/baQfhhFzrMydJyzxmOGiPs?=
 =?us-ascii?Q?uDq5PiIZXeY8moeb3v0CPGK3KEhC2VtQ6l9J0Zzc8qirvqDp1Y8AfnWQmKoZ?=
 =?us-ascii?Q?3k6ZuLs2Tz2KBb2d5ke328ugTj+0zdVC5jULuc20EKnCskLRbtOw3Vi0haQc?=
 =?us-ascii?Q?PCVii0B0C+2VEaqwFpWrAaOMtisII81KUYgY2IpHsQrlhavgLzP7OY7cmhEV?=
 =?us-ascii?Q?6TL86Hjo4NIDgCNZaD5nhdNKD0tfeQaBWqOImKy/ESAeOdmQ4oj4kRqqs27V?=
 =?us-ascii?Q?iR+eiGpcVrxSTb38uN3SzWmhIltNmtQhc5DvS/EBeA2EKyg92vyOdDNhT8Qz?=
 =?us-ascii?Q?nA=3D=3D?=
Content-Type: text/plain; charset="us-ascii"
Content-ID: <AD81D1A3DF715848A29CA96970B1F168@namprd10.prod.outlook.com>
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-0: mDV/hkQNiXhT2hXznCmkFotk3bOpSqEaRreVZS6pVLbhCL3M7wigpFJIvnzVFSdH1SHzw5gPUQ75gJQ+8cY+jzYlxCYDpuZyYf/fXwaDGAD+vM/ZqU2jAQF3TEE+FErpdvYdGVFxXUsRUg5VubYyyuoqk8Dvmec1ur5aGGdvwMuPdywtERuwiu0UMj3mfXIyltjPMYIchV5s+S8TT9NzEFFysBvJH8HeYmRaaACocF8CJx35Z53rblgVnCsrAUpckokyTj3WLKRMklTrshDDdCr9b6N+mekisDNolu5ghqBtZhu3q76GDqGv1Bq8YRNNGV9O2I+T88bI03OhzjVDzSP8HCUV3onoDx7z/VmATCJ3DA1EzMKiyLcrzH6y2eS6IOwKa/WhYnH8uyBs4i4swrIO1cjVuM+6SyImecZeCex0/FoxlzylKNA6yxWLLKEUvkQUJoQtIFt4Dop/3NG4H4Qs51qf44Jfep22qMZTCk4gINR4gy5T7Obrl5KL+tRlvpjNMLMQ5cfrUMpKPHRan4wQIz8pIbjMW7lDRLSR+BFbj4sszuusGdkbLUBPVkP7/s6mBM2UL7I6CfG2CACzxGu5wtGTLP1UpaI/a37Zn8ytaw+h/iqXkaPxiLI+WbDUDZgN3AuqEQ7aSZq3fVi/24C+EvtmtrOxUcYw/dCsQGvqFFH4Ut87pJOvPkXVbm7ELYogLScosw4952LXlrpv8ZcdxKyuv8GlA3xi6YdyCpwwEpf+VULPGNjs+b02VEcXJ6AT5tAnkXSamGXt/XNM7HOIaJwu/SPr4Qah0Ul7TN6Zh2O14RguK4rrEuM/801F6yYr7mn1f1ktmtioYaqmPsC5rAIvvotduWcvD2t6HmUt+90Q3EnKNgUrEF+P+Idm
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: 8dc0a491-391a-49e4-e408-08db81984a27
X-MS-Exchange-CrossTenant-originalarrivaltime: 10 Jul 2023 22:52:07.4949
 (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: pNwGay/zzKMHFXhFkDw60ssGwo856tm7gnG25rArmOmqgz3RnZSL8xH+5bZScItnCTsG9Odnwhaqli98LCUhMw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SJ0PR10MB5860
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.254,Aquarius:18.0.957,Hydra:6.0.591,FMLib:17.11.176.26
 definitions=2023-07-10_16,2023-07-06_02,2023-05-22_02
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 mlxscore=0 malwarescore=0 phishscore=0
 spamscore=0 mlxlogscore=999 suspectscore=0 adultscore=0 bulkscore=0
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2305260000
 definitions=main-2307100207
X-Proofpoint-GUID: xiycqjkGedlow4LbA1pr8aNKbDied7J-
X-Proofpoint-ORIG-GUID: xiycqjkGedlow4LbA1pr8aNKbDied7J-
X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_BLOCKED,
        RCVD_IN_MSPIKE_H5,RCVD_IN_MSPIKE_WL,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 Jul 10, 2023, at 6:29 PM, NeilBrown <neilb@suse.de> wrote:
>=20
> On Tue, 11 Jul 2023, Chuck Lever wrote:
>> From: Chuck Lever <chuck.lever@oracle.com>
>>=20
>> Measure a source of thread scheduling inefficiency -- count threads
>> that were awoken but found that the transport queue had already been
>> emptied.
>>=20
>> An empty transport queue is possible when threads that run between
>> the wake_up_process() call and the woken thread returning from the
>> scheduler have pulled all remaining work off the transport queue
>> using the first svc_xprt_dequeue() in svc_get_next_xprt().
>=20
> I'm in two minds about this.  The data being gathered here is
> potentially useful

It's actually pretty shocking: I've measured more than
15% of thread wake-ups find no work to do.


> - but who it is useful to?
> I think it is primarily useful for us - to understand the behaviour of
> the implementation so we can know what needs to be optimised.
> It isn't really of any use to a sysadmin who wants to understand how
> their system is performing.
>=20
> But then .. who are tracepoints for?  Developers or admins?
> I guess that fact that we feel free to modify them whenever we need
> means they are primarily for developers?  In which case this is a good
> patch, and maybe we'll revert the functionality one day if it turns out
> that we can change the implementation so that a thread is never woken
> when there is no work to do ....

A reasonable question to ask. The new "starved" metric
is similar: possibly useful while we are developing the
code, but not particularly valuable for system
administrators.

How are the pool_stats used by administrators?

(And, why are they in /proc/fs/nfsd/ rather than under
something RPC-related?)


>> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
>> ---
>> include/linux/sunrpc/svc.h |    1 +
>> net/sunrpc/svc.c           |    2 ++
>> net/sunrpc/svc_xprt.c      |    7 ++++---
>> 3 files changed, 7 insertions(+), 3 deletions(-)
>>=20
>> diff --git a/include/linux/sunrpc/svc.h b/include/linux/sunrpc/svc.h
>> index 74ea13270679..9dd3b16cc4c2 100644
>> --- a/include/linux/sunrpc/svc.h
>> +++ b/include/linux/sunrpc/svc.h
>> @@ -43,6 +43,7 @@ struct svc_pool {
>> struct percpu_counter sp_threads_woken;
>> struct percpu_counter sp_threads_timedout;
>> struct percpu_counter sp_threads_starved;
>> + struct percpu_counter sp_threads_no_work;
>>=20
>> #define SP_TASK_PENDING (0) /* still work to do even if no
>> * xprt is queued. */
>> diff --git a/net/sunrpc/svc.c b/net/sunrpc/svc.c
>> index 88b7b5fb6d75..b7a02309ecb1 100644
>> --- a/net/sunrpc/svc.c
>> +++ b/net/sunrpc/svc.c
>> @@ -518,6 +518,7 @@ __svc_create(struct svc_program *prog, unsigned int =
bufsize, int npools,
>> percpu_counter_init(&pool->sp_threads_woken, 0, GFP_KERNEL);
>> percpu_counter_init(&pool->sp_threads_timedout, 0, GFP_KERNEL);
>> percpu_counter_init(&pool->sp_threads_starved, 0, GFP_KERNEL);
>> + percpu_counter_init(&pool->sp_threads_no_work, 0, GFP_KERNEL);
>> }
>>=20
>> return serv;
>> @@ -595,6 +596,7 @@ svc_destroy(struct kref *ref)
>> percpu_counter_destroy(&pool->sp_threads_woken);
>> percpu_counter_destroy(&pool->sp_threads_timedout);
>> percpu_counter_destroy(&pool->sp_threads_starved);
>> + percpu_counter_destroy(&pool->sp_threads_no_work);
>> }
>> kfree(serv->sv_pools);
>> kfree(serv);
>> diff --git a/net/sunrpc/svc_xprt.c b/net/sunrpc/svc_xprt.c
>> index ecbccf0d89b9..6c2a702aa469 100644
>> --- a/net/sunrpc/svc_xprt.c
>> +++ b/net/sunrpc/svc_xprt.c
>> @@ -776,9 +776,9 @@ static struct svc_xprt *svc_get_next_xprt(struct svc=
_rqst *rqstp, long timeout)
>>=20
>> if (!time_left)
>> percpu_counter_inc(&pool->sp_threads_timedout);
>> -
>> if (signalled() || kthread_should_stop())
>> return ERR_PTR(-EINTR);
>> + percpu_counter_inc(&pool->sp_threads_no_work);
>> return ERR_PTR(-EAGAIN);
>> out_found:
>> /* Normally we will wait up to 5 seconds for any required
>> @@ -1445,13 +1445,14 @@ static int svc_pool_stats_show(struct seq_file *=
m, void *p)
>> return 0;
>> }
>>=20
>> - seq_printf(m, "%u %llu %llu %llu %llu %llu\n",
>> + seq_printf(m, "%u %llu %llu %llu %llu %llu %llu\n",
>> pool->sp_id,
>> percpu_counter_sum_positive(&pool->sp_messages_arrived),
>> percpu_counter_sum_positive(&pool->sp_sockets_queued),
>> percpu_counter_sum_positive(&pool->sp_threads_woken),
>> percpu_counter_sum_positive(&pool->sp_threads_timedout),
>> - percpu_counter_sum_positive(&pool->sp_threads_starved));
>> + percpu_counter_sum_positive(&pool->sp_threads_starved),
>> + percpu_counter_sum_positive(&pool->sp_threads_no_work));
>>=20
>> return 0;
>> }
>>=20
>>=20
>>=20
>=20

--
Chuck Lever