[CRIU] [PATCH v2 16/24] bpf/task_iter: In task_file_seq_get_next use task_lookup_next_fd_rcu

[Eric W. Biederman]

Yonghong Song <yhs at fb.com> writes:

> On 11/20/20 3:14 PM, Eric W. Biederman wrote:
>> When discussing[1] exec and posix file locks it was realized that none
>> of the callers of get_files_struct fundamentally needed to call
>> get_files_struct, and that by switching them to helper functions
>> instead it will both simplify their code and remove unnecessary
>> increments of files_struct.count.  Those unnecessary increments can
>> result in exec unnecessarily unsharing files_struct which breaking
>> posix locks, and it can result in fget_light having to fallback to
>> fget reducing system performance.
>>
>> Using task_lookup_next_fd_rcu simplifies task_file_seq_get_next, by
>> moving the checking for the maximum file descritor into the generic
>> code, and by remvoing the need for capturing and releasing a reference
>> on files_struct.  As the reference count of files_struct no longer
>> needs to be maintained bpf_iter_seq_task_file_info can have it's files
>> member removed and task_file_seq_get_next no longer needs it's fstruct
>> argument.
>>
>> The curr_fd local variable does need to become unsigned to be used
>> with fnext_task.  As curr_fd is assigned from and assigned a u32
>> making curr_fd an unsigned int won't cause problems and might prevent
>> them.
>>
>> [1] https://lkml.kernel.org/r/20180915160423.GA31461@redhat.com
>> Suggested-by: Oleg Nesterov <oleg at redhat.com>
>> v1: https://lkml.kernel.org/r/20200817220425.9389-11-ebiederm@xmission.com
>> Signed-off-by: "Eric W. Biederman" <ebiederm at xmission.com>
>> ---
>>   kernel/bpf/task_iter.c | 44 ++++++++++--------------------------------
>>   1 file changed, 10 insertions(+), 34 deletions(-)
>
>
> Just a heads-up. The following patch
>   bpf-next: 91b2db27d3ff ("bpf: Simplify task_file_seq_get_next()")
>
> https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git/commit/?id=91b2db27d3ff9ad29e8b3108dfbf1e2f49fe9bd3
> has been merged in bpf-next tree.
>
> It will have merge conflicts with this patch. The above patch
> is a refactoring for simplification with no functionality change.

Thanks for the heads up.

Eric