Re: [PATCH bpf-next 1/4] bpf: introduce task_vma bpf_iter

[Song Liu <songliubraving@fb.com>]

> On Dec 13, 2020, at 11:13 PM, Yonghong Song <yhs@fb.com> wrote:
> 
> 
> 
> On 12/11/20 6:48 PM, Song Liu wrote:
>> Introduce task_vma bpf_iter to print memory information of a process. It
>> can be used to print customized information similar to /proc/<pid>/maps.
>> task_vma iterator releases mmap_lock before calling the BPF program.
>> Therefore, we cannot pass vm_area_struct directly to the BPF program. A
>> new __vm_area_struct is introduced to keep key information of a vma. On
>> each iteration, task_vma gathers information in __vm_area_struct and
>> passes it to the BPF program.
>> If the vma maps to a file, task_vma also holds a reference to the file
>> while calling the BPF program.
>> Signed-off-by: Song Liu <songliubraving@fb.com>
>> ---
>>  include/linux/bpf.h      |   2 +-
>>  include/uapi/linux/bpf.h |   7 ++
>>  kernel/bpf/task_iter.c   | 193 ++++++++++++++++++++++++++++++++++++++-
>>  3 files changed, 200 insertions(+), 2 deletions(-)
>> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
>> index 07cb5d15e7439..49dd1e29c8118 100644
>> --- a/include/linux/bpf.h
>> +++ b/include/linux/bpf.h
>> @@ -1325,7 +1325,7 @@ enum bpf_iter_feature {
>>  	BPF_ITER_RESCHED	= BIT(0),
>>  };
>>  -#define BPF_ITER_CTX_ARG_MAX 2
>> +#define BPF_ITER_CTX_ARG_MAX 3
>>  struct bpf_iter_reg {
>>  	const char *target;
>>  	bpf_iter_attach_target_t attach_target;
>> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
>> index 30b477a264827..c2db8a1d0cbd2 100644
>> --- a/include/uapi/linux/bpf.h
>> +++ b/include/uapi/linux/bpf.h
>> @@ -5151,4 +5151,11 @@ enum {
>>  	BTF_F_ZERO	=	(1ULL << 3),
>>  };
>>  +struct __vm_area_struct {
>> +	__u64 start;
>> +	__u64 end;
>> +	__u64 flags;
>> +	__u64 pgoff;
>> +};
> 
> Probably we should not expose the above structure as uapi.
> All other bpf_iter ctx argument layouts are btf based
> and consider unstable. btf_iter itself is considered
> as an unstable interface to dump kernel internal
> data structures.

Yes, we can keep it in task_iter.c. I moved it to uapi to follow __sk_buff
pattern. It works fine in task_iter.c

Thanks,
Song



> 
>> +
>>  #endif /* _UAPI__LINUX_BPF_H__ */
>> diff --git a/kernel/bpf/task_iter.c b/kernel/bpf/task_iter.c
>> index 0458a40edf10a..30e5475d0831e 100644
>> --- a/kernel/bpf/task_iter.c
>> +++ b/kernel/bpf/task_iter.c
>> @@ -304,9 +304,171 @@ static const struct seq_operations task_file_seq_ops = {
>>  	.show	= task_file_seq_show,
>>  };
>>  +struct bpf_iter_seq_task_vma_info {
>> +	/* The first field must be struct bpf_iter_seq_task_common.
>> +	 * this is assumed by {init, fini}_seq_pidns() callback functions.
>> +	 */
>> +	struct bpf_iter_seq_task_common common;
>> +	struct task_struct *task;
>> +	struct __vm_area_struct vma;
>> +	struct file *file;
>> +	u32 tid;
>> +};
>> +
> [...]