Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:18 as permitted sender) client-ip=2620:137:e000::1:18;
Message-ID: <4cbdd3e9-c6fe-d796-5560-cd09c9220868@fb.com>
Date:   Fri, 20 May 2022 17:58:47 -0700
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:91.0)
 Gecko/20100101 Thunderbird/91.9.0
Subject: Re: [PATCH bpf-next v1 3/5] bpf: Introduce cgroup iter
Content-Language: en-US
To:     Hao Luo <haoluo@google.com>, Tejun Heo <tj@kernel.org>
Cc:     Yosry Ahmed <yosryahmed@google.com>,
        Alexei Starovoitov <ast@kernel.org>,
        Daniel Borkmann <daniel@iogearbox.net>,
        Andrii Nakryiko <andrii@kernel.org>,
        Martin KaFai Lau <kafai@fb.com>,
        Song Liu <songliubraving@fb.com>,
        John Fastabend <john.fastabend@gmail.com>,
        KP Singh <kpsingh@kernel.org>,
        Zefan Li <lizefan.x@bytedance.com>,
        Johannes Weiner <hannes@cmpxchg.org>,
        Shuah Khan <shuah@kernel.org>,
        Roman Gushchin <roman.gushchin@linux.dev>,
        Michal Hocko <mhocko@kernel.org>,
        Stanislav Fomichev <sdf@google.com>,
        David Rientjes <rientjes@google.com>,
        Greg Thelen <gthelen@google.com>,
        Shakeel Butt <shakeelb@google.com>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        Networking <netdev@vger.kernel.org>, bpf <bpf@vger.kernel.org>,
        Cgroups <cgroups@vger.kernel.org>
References: <20220520012133.1217211-1-yosryahmed@google.com>
 <20220520012133.1217211-4-yosryahmed@google.com>
 <YodGI73xq8aIBrNM@slm.duckdns.org>
 <CAJD7tkbvMcMWESMcWi6TtdCKLr6keBNGgZTnqcHZvBrPa1qWPw@mail.gmail.com>
 <YodNLpxut+Zddnre@slm.duckdns.org>
 <73fd9853-5dab-8b59-24a0-74c0a6cae88e@fb.com>
 <YofFli6UCX4J5YnU@slm.duckdns.org>
 <CA+khW7gjWVKrwCgDD-4ZdCf5CMcA4-YL0bLm6aWM74+qNQ4c0A@mail.gmail.com>
 <CA+khW7iDDkO3h5WQixEA=nUL-tBmCTh7fMAf3iwNy98UfM-k9g@mail.gmail.com>
From:   Yonghong Song <yhs@fb.com>
In-Reply-To: <CA+khW7iDDkO3h5WQixEA=nUL-tBmCTh7fMAf3iwNy98UfM-k9g@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?aEtKZTk5SUNlNTE3cEV3WkxDL3Y1WTZsKzlsOTd1SHdoZmlJdlZZMEN3MG0x?=
 =?utf-8?B?SlRCUGFmOEUzOGwvYjAzVHZ1a1ZQVzVEeXN4MjB5dWRCc3RycUpkTmdpS2NE?=
 =?utf-8?B?NU4vcnNndkdxOTZCMmlSSDhWMkM2WG9CK3B0emNIc3VEa2QxWEFHWDFzL1FI?=
 =?utf-8?B?dmwvTjFjUlhRaU9uTDhXUTVEajViTzBxODQvWE5weGQ5aWYxZytQaVYyUkN6?=
 =?utf-8?B?c3lwNmtOcnVkOGZSelc5MHlhU3dSRm9rR3Nsanhvb2pHMU5zcEk4RE1RRVl0?=
 =?utf-8?B?bWVoNmZGbzlJQ3J6OSt2OG0zaVRMelVBUDJ5Zk1KNTJ5TEtuQmx4ZGt4S1ky?=
 =?utf-8?B?QXBiVzdJdUJDSG1Vc0hLcithYnZQYWphN2FyWjNFQTZKZTJHRjlqcHo5d3kv?=
 =?utf-8?B?VnpNMjMwSFNzSWhrYWx2SEg2QkpmaHh6dlNQVVYxeTlUemJpY0tIZ1FLSFZM?=
 =?utf-8?B?ajQ0NnhUenQ1UXp6Uk1Hdi96ZGgrT0cxTVl2UXkwS2VENzBLUlJzS0p3WlhE?=
 =?utf-8?B?Ykw3cUdVcUIrUkZDS3JueFRLK3ZDbmxvTktVMmhsbjN4bm1TZ1A3Nmo0U2gr?=
 =?utf-8?B?Uzh4bjlsRjNPVjE1RHhZazBPZ21Uck5ScnZYOUFUZndHbVVhTjNSY0toUGZ3?=
 =?utf-8?B?K1lybjVwaEJRKzJPYzdBR0ltbkVWakFpeGFldWZjNmhCNGJ4U204dGxSTEl5?=
 =?utf-8?B?YU1vU0xBNXhFOUsrc1V5TTBENkRmSHUyWFVDUWk2MWxaemQ4dlVRN2FLek1s?=
 =?utf-8?B?ek8vYTdtVFBFYWZxdzdxNmtLRy82eWpKM0tQNzlWb2dGcnpEekR4bDR6eE1P?=
 =?utf-8?B?YzVWSDRPa3BtR0k1aWZ6UC8vZlY2WUVYak52QldwVTV3UEVpOXhla1FzRjhF?=
 =?utf-8?B?by9aT1NYcjFZWm8xcWorb1ZpNFI2THBtUjZaY2JZalM3SWpHTk9jV0xRQ3BU?=
 =?utf-8?B?VTBHVkdnYlgrcWN5R0tmdkVTZDZmK3lxbGEzVG1WaE1TVGE5em10aWV3cG1Y?=
 =?utf-8?B?RXYrVEZ1MHZBdnlPeTY4LzFEblFqU0xLcndNS0NHY3V6SXl4bVhTRWU1Wk93?=
 =?utf-8?B?blZKM250TVhuNzZxSDhCeVNRRXpPczlmak01TkN2Z3I3NGxXanpiUHBUZUpJ?=
 =?utf-8?B?QkRpazg0R2JPWkZ3NzJCN1FadHg2TkdYWGwyQjgzS25MbEEzcHFFQS9YcnVU?=
 =?utf-8?B?ZkhUTURUYUV2S2NvenMvZGxXaFNCTmFPZDUwY1lVUzFTZVJDQUlYS1VONFhG?=
 =?utf-8?B?eC9uVWVVaTJRU3VSekIwVzdzU1ZXenE1SkFib1dVRU9MdE5UVE9FY1FnOGcr?=
 =?utf-8?B?WXByWlo0M0hDa0MrSGwzd2k0MGZwbEdGZWtWVnBxaVRIekl4bzc5Ykg5dkNo?=
 =?utf-8?B?VFJhZzlPanU5TnRKKy82SVpDYkc2N3pUMXNhclRDeHRaU1VEcFVLbW5WTXZJ?=
 =?utf-8?B?cGlxTWFMMDlpclhPNWRyc083VEY2YkoyNVFJUjhSYU5kNXB6REJFRUlvVi9W?=
 =?utf-8?B?UHpvUDBwK2k5TEpFNGJaOURvbEZ6dVVxaDRhU0VITXQxVCtzc2dsU0JVVFMz?=
 =?utf-8?B?WHdMZGdHdlBta1p2RjVyb0VsKzZzcSs4UHJQbnBLT09YaW1ycXpqb003M3JK?=
 =?utf-8?B?ZWQ2T0pDQ2Fra0RUY2xTOVJYNXNYV0UwMkNYL25mZHZDcjRoUE5sRG94TDJp?=
 =?utf-8?B?RThGZkhWbVBCMm9iRmNJSEVNOWpBK1F3Zkx0Z3JGVEVIVVN2bWhkSkNCM01z?=
 =?utf-8?B?N3ZPbC9RSkY4OFpoYUd3UlZPbG83a3duakphM05LZUJXYmR4cm95WWt2a1k1?=
 =?utf-8?B?UmRxcVVTRWp5bHRKUUFwcG5lVVAwMVp6LzFHWnpYcEZ2d3FDZWdGeWtTNCty?=
 =?utf-8?B?QWx6TVM3dWcvNmgzNEx0cFRic0RKN1VvdllnMlFCNjF4THFWVW14MThMU01q?=
 =?utf-8?B?UXp2Mm9XNzV5YmpoUzRkblRSUlU3UWFCRnJsMmx0bmdQNXltNFJZMFlxK3pM?=
 =?utf-8?B?dlkwTmJyZE5sMk5idGxZenJhRVpMRVRTMG8wUXVDUG1ocUdrWVhSTU1SLzBT?=
 =?utf-8?B?MUZ1SmVPNUJqMnhHNjBrSmpRcHZYUjlpUklQQ0Z0eU5MNnlxdEwzUEIraTdy?=
 =?utf-8?B?blpYWTM4WFI3bU5nV3U1eXczQ0pyazc5bHRYNDdPaGUxNWs5b2pmOU52bllo?=
 =?utf-8?B?M1pmcko2cGRUNDVXcDNiTjBkWXVLMDNUSHVoK28zek4vWXZPaXRBZXFJVTR4?=
 =?utf-8?B?MDFYNGNkNGF1U3ZUMFF2TzdmdmordTBYTnhHcjl3OTZzVXpuTm5lcVI4enNE?=
 =?utf-8?B?QmRldm91SjdRaTJIUTJHcG41bVU5MTZ4cHQwMXRJV2wvZDlLempIZWhZb29W?=
 =?utf-8?Q?sll7WQ0kU9MQEYlk=3D?=
X-OriginatorOrg: fb.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 4805a361-820a-485e-16ad-08da3ac5111b
X-MS-Exchange-CrossTenant-AuthSource: SN6PR1501MB2064.namprd15.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 21 May 2022 00:58:49.8021
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 8ae927fe-1255-47a7-a2af-5f3a069daaa2
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: qE/N17bxWsZhPpq1KWuSKyT16scCwXVV+DkNdbmfwoqwvLIP/YA1UE3bBi8mhcdW
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH0PR15MB5088
X-Proofpoint-GUID: hEr9imfLMsJU0VK8JtOiCmOYyFEOGOsS
X-Proofpoint-ORIG-GUID: hEr9imfLMsJU0VK8JtOiCmOYyFEOGOsS
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.205,Aquarius:18.0.874,Hydra:6.0.486,FMLib:17.11.64.514
 definitions=2022-05-20_08,2022-05-20_02,2022-02-23_01
X-Spam-Status: No, score=-3.7 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
	DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,
	MAILING_LIST_MULTI,NICE_REPLY_A,RDNS_NONE,SPF_HELO_NONE,
	T_SCC_BODY_TEXT_LINE autolearn=unavailable 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-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org


On 5/20/22 2:49 PM, Hao Luo wrote:
> Hi Tejun and Yonghong,
> 
> On Fri, May 20, 2022 at 12:42 PM Hao Luo <haoluo@google.com> wrote:
>>
>> Hi Tejun and Yonghong,
>>
>> On Fri, May 20, 2022 at 9:45 AM Tejun Heo <tj@kernel.org> wrote:
>>> On Fri, May 20, 2022 at 09:29:43AM -0700, Yonghong Song wrote:
>>>>     <various stats interested by the user>
>>>>
>>>> This way, user space can easily construct the cgroup hierarchy stat like
>>>>                             cpu   mem   cpu pressure   mem pressure ...
>>>>     cgroup1                 ...
>>>>        child1               ...
>>>>          grandchild1        ...
>>>>        child2               ...
>>>>     cgroup 2                ...
>>>>        child 3              ...
>>>>          ...                ...
>>>>
>>>> the bpf iterator can have additional parameter like
>>>> cgroup_id = ... to only call bpf program once with that
>>>> cgroup_id if specified.
>>
>> Yep, this should work. We just need to make the cgroup_id parameter
>> optional. If it is specified when creating bpf_iter_link, we print for
>> that cgroup only. If it is not specified, we iterate over all cgroups.
>> If I understand correctly, sounds doable.
>>
> 
> Yonghong, I realized that seek() which Tejun has been calling out, can
> be used to specify the target cgroup, rather than adding a new
> parameter. Maybe, we can pass cgroup_id to seek() on cgroup bpf_iter,
> which will instruct read() to return the corresponding cgroup's stats.
> On the other hand, reading without calling seek() beforehand will
> return all the cgroups.

Currently, seek is not supported for bpf_iter.

const struct file_operations bpf_iter_fops = {
         .open           = iter_open,
         .llseek         = no_llseek,
         .read           = bpf_seq_read,
         .release        = iter_release,
};

But if seek() works, I don't mind to remove this restriction.
But not sure what to seek. Do you mean to provide a cgroup_fd/cgroup_id
as the seek() syscall parameter? This may work.

But considering we have parameterized example (map_fd) and
in the future, we may have other parameterized bpf_iter
(e.g., for one task). Maybe parameter-based approach is better.

> 
> WDYT?