Date:   Thu, 13 Jan 2022 16:08:57 -0500
From:   Daniel Jordan <daniel.m.jordan@oracle.com>
To:     Tejun Heo <tj@kernel.org>
Cc:     Peter Zijlstra <peterz@infradead.org>,
        Alexander Duyck <alexanderduyck@fb.com>,
        Alex Williamson <alex.williamson@redhat.com>,
        Andrew Morton <akpm@linux-foundation.org>,
        Ben Segall <bsegall@google.com>,
        Cornelia Huck <cohuck@redhat.com>,
        Dan Williams <dan.j.williams@intel.com>,
        Dave Hansen <dave.hansen@linux.intel.com>,
        Dietmar Eggemann <dietmar.eggemann@arm.com>,
        Herbert Xu <herbert@gondor.apana.org.au>,
        Ingo Molnar <mingo@redhat.com>,
        Jason Gunthorpe <jgg@nvidia.com>,
        Johannes Weiner <hannes@cmpxchg.org>,
        Josh Triplett <josh@joshtriplett.org>,
        Michal Hocko <mhocko@suse.com>, Nico Pache <npache@redhat.com>,
        Pasha Tatashin <pasha.tatashin@soleen.com>,
        Steffen Klassert <steffen.klassert@secunet.com>,
        Steve Sistare <steven.sistare@oracle.com>,
        Tim Chen <tim.c.chen@linux.intel.com>,
        Vincent Guittot <vincent.guittot@linaro.org>,
        linux-mm@kvack.org, kvm@vger.kernel.org,
        linux-kernel@vger.kernel.org, linux-crypto@vger.kernel.org
Subject: Re: [RFC 15/16] sched/fair: Account kthread runtime debt for CFS
 bandwidth
Message-ID: <20220113210857.d3xkupgmpdeqknhn@oracle.com>
References: <20220106004656.126790-1-daniel.m.jordan@oracle.com>
 <20220106004656.126790-16-daniel.m.jordan@oracle.com>
 <Yd1w/TxTcGk5Ht53@hirez.programming.kicks-ass.net>
 <20220111162950.jk3edkm3nh5apviq@oracle.com>
 <Yd83iDzoUOWPB6yH@slm.duckdns.org>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <Yd83iDzoUOWPB6yH@slm.duckdns.org>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?do8R8cXjDqklGqS/IdjSb0jwwSMmDzPYXODWzH0NMTv7Q08kqkHPdlOtcoa6?=
 =?us-ascii?Q?g82fPd6e/bIcMRVh14Sh9pvBjC5BKchpTewR2GVAqeOItKm5GvOPbIREtLQZ?=
 =?us-ascii?Q?MmfSPkpzu+al1jjpVcuYHIGqpwbPovg1eeDtXvwYMUAZj3QcJa+uVW8aMt7B?=
 =?us-ascii?Q?eCtD7NrpWyzvGaAK1DS8aTHiUUwa9OSZXws/yunJ8EqxmDBsPcg8mxZQa+ws?=
 =?us-ascii?Q?xmyWei+j40jS7e+fkqzdeq94K7gUWfAUG81BCVky1K28gHy6GzLOJKeF7iP1?=
 =?us-ascii?Q?YqxxR5erRi3yfwp/Ly/2Hv0zjC7+AkXm5pXLcMaOGR27EIFNnrTffF8UO0l8?=
 =?us-ascii?Q?Quy9i5h1JQn231zSJGZ8o98o2pnbJUOx6Hj8z+xhub4Vz3RdVh9tSf4UORSy?=
 =?us-ascii?Q?QtKOwI0KoVDhQ3898QPgSflPANYVsl/g4rpUsnyajLZsTgyK+lehSqqG56BM?=
 =?us-ascii?Q?mR8IPWMQI9lHaTHvNhaO0bosAKZRSpHpQX+Y7wK0rdsBZkBBUgm+z4ReVJm4?=
 =?us-ascii?Q?2FTqyEhOz/sUl5PDLGHTrhPbvHyjJYqZB4LRBC+KJ2E/D9t4yiw2vMdEKTAp?=
 =?us-ascii?Q?US8VYIjIeOzLD6geQdw0vTPfWRdOL2oDm05Pn624at7CX7GbuRuyle5zrGtO?=
 =?us-ascii?Q?ZYMJBOmQKbAL1azh3s7CsuvdXoOfbkkCJXqz2qVzxUjecBE5mzq7m4812INI?=
 =?us-ascii?Q?ygsX3ubitH+j1FQu+TjkhF9JJt4onaso0EzR48VARI0JvcrxWEIkpo47bkP5?=
 =?us-ascii?Q?8J9f8eDMQI0qwc0GOuFtak+vFHPyPdsz9FV5r5J2mgzxOo+yQHWQzigmlYE/?=
 =?us-ascii?Q?Dda29KvYBqEyLRtckS6WmP7DUi3sq/cxTWf4GDHEC9zKJJslq8LR6oudwy+Q?=
 =?us-ascii?Q?8jCK4NjrZcphMKFLccV1daEsUAZyr7pqxWEvGPF/8OhQJDPNDNu5bJVTkJkZ?=
 =?us-ascii?Q?7YFLy/cuT/G8cYywDcXBi2C+aSdwgvDIXannjShL2WMmTtTCUT8gtPVsIsqT?=
 =?us-ascii?Q?cL6wp9niRdFbwtEOiq+HBTnPfwakJGUleVV2QA/QnkeeIrA4N3/D6AO8Mvg6?=
 =?us-ascii?Q?g1PlkW9DDaMpwNjgPIj4L+sJxht7DZe4UsXzFcgF+4iia/+R9+FtJqTIADF2?=
 =?us-ascii?Q?Tn5/umbpWacqSDKNcgGg/IM76cT+7Y0ZiwbecF4f/Z+ESmp00e61s50QUO0a?=
 =?us-ascii?Q?W7AeRroQK8WA0O/X+SoXaNHuyiFyBaQKOy0gOxJAKnN8oUQcb8/1KidLyIlQ?=
 =?us-ascii?Q?5hhfvOdIq2sQnP6f4ETH1QqqKXWupkbLq1G5fxDAqKCKdZpa9NyU7kQNi1Z+?=
 =?us-ascii?Q?ce7zDLsMSEof9CxKldLfML+PKqc5AH/ygH0XUtAbHHvSRZmpMLNLpgybI9nZ?=
 =?us-ascii?Q?hldhZ5ZeCsn7kzzkhkehdMwOyhxg1nhTiHWgeTgklFAIREW55eNefxK8xAo9?=
 =?us-ascii?Q?CfgXD0cMZVHUfZIJaI6MG+Sdv42ueGgTVRZAK8l1IDemKBU7k6E0THoRv71j?=
 =?us-ascii?Q?o4YThHqw3e5YEtn+rr+Z+pP3rSzR8DJR9VIm382CtnpLZFj7xLE+JcMk7iLP?=
 =?us-ascii?Q?UX47iIDf9e14XDAKLpVQ/CmY5CitKeS7npB8lAZEEAjUT7PZqjS1TUxBWkkW?=
 =?us-ascii?Q?CqAEG7WnvSWF5pS3KqbC7DbhbfGiqamiUQhYBGktyqL9zsolorRZGRh9/2Ia?=
 =?us-ascii?Q?OSef3g=3D=3D?=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 3bbe163e-4ed8-468f-0557-08d9d6d8ebfb
X-MS-Exchange-CrossTenant-AuthSource: SA1PR10MB5711.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 13 Jan 2022 21:09:01.2824
 (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: 5UExDNpJgDsGPyTeuRL7PYBQW6ZYme8Vo4kCgYMXhqw6OWnfI0DSKjMDdnoOEhOKhxFaX99+lc0WSNO6kkj0h2gdHd06PVJCL4gY9egOuCA=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BYAPR10MB3447
X-Proofpoint-Virus-Version: vendor=nai engine=6300 definitions=10226 signatures=668683
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 mlxscore=0 suspectscore=0 spamscore=0
 mlxlogscore=999 adultscore=0 phishscore=0 malwarescore=0 bulkscore=0
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2110150000
 definitions=main-2201130129
X-Proofpoint-ORIG-GUID: uBnESALOBZDdWVtEeO8LZwfGqghLSHbM
X-Proofpoint-GUID: uBnESALOBZDdWVtEeO8LZwfGqghLSHbM
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Wed, Jan 12, 2022 at 10:18:16AM -1000, Tejun Heo wrote:
> Hello,

Hi, Tejun.

> On Tue, Jan 11, 2022 at 11:29:50AM -0500, Daniel Jordan wrote:
> ...
> > This problem arises with multithreaded jobs, but is also an issue in other
> > places.  CPU activity from async memory reclaim (kswapd, cswapd?[5]) should be
> > accounted to the cgroup that the memory belongs to, and similarly CPU activity
> > from net rx should be accounted to the task groups that correspond to the
> > packets being received.  There are also vague complaints from Android[6].
> 
> These are pretty big holes in CPU cycle accounting right now and I think
> spend-first-and-backcharge is the right solution for most of them given
> experiences from other controllers. That said,
> 
> > Each use case has its own requirements[7].  In padata and reclaim, the task
> > group to account to is known ahead of time, but net rx has to spend cycles
> > processing a packet before its destination task group is known, so any solution
> > should be able to work without knowing the task group in advance.  Furthermore,
> > the CPU controller shouldn't throttle reclaim or net rx in real time since both
> > are doing high priority work.  These make approaches that run kthreads directly
> > in a task group, like cgroup-aware workqueues[8] or a kernel path for
> > CLONE_INTO_CGROUP, infeasible.  Running kthreads directly in cgroups also has a
> > downside for padata because helpers' MAX_NICE priority is "shadowed" by the
> > priority of the group entities they're running under.
> > 
> > The proposed solution of remote charging can accrue debt to a task group to be
> > paid off or forgiven later, addressing all these issues.  A kthread calls the
> > interface
> > 
> >     void cpu_cgroup_remote_begin(struct task_struct *p,
> >                                  struct cgroup_subsys_state *css);
> > 
> > to begin remote charging to @css, causing @p's current sum_exec_runtime to be
> > updated and saved.  The @css arg isn't required and can be removed later to
> > facilitate the unknown cgroup case mentioned above.  Then the kthread calls
> > another interface
> > 
> >     void cpu_cgroup_remote_charge(struct task_struct *p,
> >                                   struct cgroup_subsys_state *css);
> > 
> > to account the sum_exec_runtime that @p has used since the first call.
> > Internally, a new field cfs_bandwidth::debt is added to keep track of unpaid
> > debt that's only used when the debt exceeds the quota in the current period.
> > 
> > Weight-based control isn't implemented for now since padata helpers run at
> > MAX_NICE and so always yield to anything higher priority, meaning they would
> > rarely compete with other task groups.
> 
> If we're gonna do this, let's please do it right and make weight based
> control work too. Otherwise, its usefulness is pretty limited.

Ok, understood.

Doing it as presented is an incremental step and all that's required for
this.  I figured weight could be added later with the first user that
actually needs it.

I did prototype weight too, though, just to see if it was all gonna work
together, so given how the discussion elsewhere in the thread is going,
I might respin the scheduler part of this with another use case and
weight-based control included.

I got this far, do the interface and CFS skeleton seem sane?  Both are
basically unchanged with weight-based control included, the weight parts
are just more code on top.

Thanks for looking.