| Message ID | 20231010185944.3888849-4-davemarchevsky@fb.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | BPF |
| Headers | show |
| Series | Open-coded task_vma iter | expand |
On Tue, Oct 10, 2023 at 12:00 PM Dave Marchevsky <davemarchevsky@fb.com> wrote: > > This patch adds kfuncs bpf_iter_task_vma_{new,next,destroy} which allow > creation and manipulation of struct bpf_iter_task_vma in open-coded > iterator style. BPF programs can use these kfuncs directly or through > bpf_for_each macro for natural-looking iteration of all task vmas. > > The implementation borrows heavily from bpf_find_vma helper's locking - > differing only in that it holds the mmap_read lock for all iterations > while the helper only executes its provided callback on a maximum of 1 > vma. Aside from locking, struct vma_iterator and vma_next do all the > heavy lifting. > > A pointer to an inner data struct, struct bpf_iter_task_vma_data, is the > only field in struct bpf_iter_task_vma. This is because the inner data > struct contains a struct vma_iterator (not ptr), whose size is likely to > change under us. If bpf_iter_task_vma_kern contained vma_iterator directly > such a change would require change in opaque bpf_iter_task_vma struct's > size. So better to allocate vma_iterator using BPF allocator, and since > that alloc must already succeed, might as well allocate all iter fields, > thereby freezing struct bpf_iter_task_vma size. > > Signed-off-by: Dave Marchevsky <davemarchevsky@fb.com> > Cc: Nathan Slingerland <slinger@meta.com> > --- > kernel/bpf/helpers.c | 3 ++ > kernel/bpf/task_iter.c | 85 ++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 88 insertions(+) > LGTM. Acked-by: Andrii Nakryiko <andrii@kernel.org> > diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c > index d2840dd5b00d..62a53ebfedf9 100644 > --- a/kernel/bpf/helpers.c > +++ b/kernel/bpf/helpers.c [...]
Hi Dave, kernel test robot noticed the following build warnings: [auto build test WARNING on bpf-next/master] url: https://github.com/intel-lab-lkp/linux/commits/Dave-Marchevsky/bpf-Don-t-explicitly-emit-BTF-for-struct-btf_iter_num/20231011-030202 base: https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git master patch link: https://lore.kernel.org/r/20231010185944.3888849-4-davemarchevsky%40fb.com patch subject: [PATCH v6 bpf-next 3/4] bpf: Introduce task_vma open-coded iterator kfuncs config: x86_64-rhel-8.3-rust (https://download.01.org/0day-ci/archive/20231013/202310132300.wnnctWmF-lkp@intel.com/config) compiler: clang version 16.0.4 (https://github.com/llvm/llvm-project.git ae42196bc493ffe877a7e3dff8be32035dea4d07) reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231013/202310132300.wnnctWmF-lkp@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202310132300.wnnctWmF-lkp@intel.com/ All warnings (new ones prefixed by >>): >> kernel/bpf/task_iter.c:827:17: warning: no previous prototype for function 'bpf_iter_task_vma_new' [-Wmissing-prototypes] __bpf_kfunc int bpf_iter_task_vma_new(struct bpf_iter_task_vma *it, ^ kernel/bpf/task_iter.c:827:13: note: declare 'static' if the function is not intended to be used outside of this translation unit __bpf_kfunc int bpf_iter_task_vma_new(struct bpf_iter_task_vma *it, ^ static >> kernel/bpf/task_iter.c:871:36: warning: no previous prototype for function 'bpf_iter_task_vma_next' [-Wmissing-prototypes] __bpf_kfunc struct vm_area_struct *bpf_iter_task_vma_next(struct bpf_iter_task_vma *it) ^ kernel/bpf/task_iter.c:871:13: note: declare 'static' if the function is not intended to be used outside of this translation unit __bpf_kfunc struct vm_area_struct *bpf_iter_task_vma_next(struct bpf_iter_task_vma *it) ^ static >> kernel/bpf/task_iter.c:880:18: warning: no previous prototype for function 'bpf_iter_task_vma_destroy' [-Wmissing-prototypes] __bpf_kfunc void bpf_iter_task_vma_destroy(struct bpf_iter_task_vma *it) ^ kernel/bpf/task_iter.c:880:13: note: declare 'static' if the function is not intended to be used outside of this translation unit __bpf_kfunc void bpf_iter_task_vma_destroy(struct bpf_iter_task_vma *it) ^ static 3 warnings generated. vim +/bpf_iter_task_vma_new +827 kernel/bpf/task_iter.c 826 > 827 __bpf_kfunc int bpf_iter_task_vma_new(struct bpf_iter_task_vma *it, 828 struct task_struct *task, u64 addr) 829 { 830 struct bpf_iter_task_vma_kern *kit = (void *)it; 831 bool irq_work_busy = false; 832 int err; 833 834 BUILD_BUG_ON(sizeof(struct bpf_iter_task_vma_kern) != sizeof(struct bpf_iter_task_vma)); 835 BUILD_BUG_ON(__alignof__(struct bpf_iter_task_vma_kern) != __alignof__(struct bpf_iter_task_vma)); 836 837 /* is_iter_reg_valid_uninit guarantees that kit hasn't been initialized 838 * before, so non-NULL kit->data doesn't point to previously 839 * bpf_mem_alloc'd bpf_iter_task_vma_kern_data 840 */ 841 kit->data = bpf_mem_alloc(&bpf_global_ma, sizeof(struct bpf_iter_task_vma_kern_data)); 842 if (!kit->data) 843 return -ENOMEM; 844 845 kit->data->task = get_task_struct(task); 846 kit->data->mm = task->mm; 847 if (!kit->data->mm) { 848 err = -ENOENT; 849 goto err_cleanup_iter; 850 } 851 852 /* kit->data->work == NULL is valid after bpf_mmap_unlock_get_irq_work */ 853 irq_work_busy = bpf_mmap_unlock_get_irq_work(&kit->data->work); 854 if (irq_work_busy || !mmap_read_trylock(kit->data->mm)) { 855 err = -EBUSY; 856 goto err_cleanup_iter; 857 } 858 859 vma_iter_init(&kit->data->vmi, kit->data->mm, addr); 860 return 0; 861 862 err_cleanup_iter: 863 if (kit->data->task) 864 put_task_struct(kit->data->task); 865 bpf_mem_free(&bpf_global_ma, kit->data); 866 /* NULL kit->data signals failed bpf_iter_task_vma initialization */ 867 kit->data = NULL; 868 return err; 869 } 870 > 871 __bpf_kfunc struct vm_area_struct *bpf_iter_task_vma_next(struct bpf_iter_task_vma *it) 872 { 873 struct bpf_iter_task_vma_kern *kit = (void *)it; 874 875 if (!kit->data) /* bpf_iter_task_vma_new failed */ 876 return NULL; 877 return vma_next(&kit->data->vmi); 878 } 879 > 880 __bpf_kfunc void bpf_iter_task_vma_destroy(struct bpf_iter_task_vma *it) 881 { 882 struct bpf_iter_task_vma_kern *kit = (void *)it; 883 884 if (kit->data) { 885 bpf_mmap_unlock_mm(kit->data->work, kit->data->mm); 886 put_task_struct(kit->data->task); 887 bpf_mem_free(&bpf_global_ma, kit->data); 888 } 889 } 890
> On Oct 13, 2023, at 8:43 AM, kernel test robot <lkp@intel.com> wrote: > > Hi Dave, > > kernel test robot noticed the following build warnings: > > [auto build test WARNING on bpf-next/master] > > url: https://github.com/intel-lab-lkp/linux/commits/Dave-Marchevsky/bpf-Don-t-explicitly-emit-BTF-for-struct-btf_iter_num/20231011-030202 > base: https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git master > patch link: https://lore.kernel.org/r/20231010185944.3888849-4-davemarchevsky%40fb.com > patch subject: [PATCH v6 bpf-next 3/4] bpf: Introduce task_vma open-coded iterator kfuncs > config: x86_64-rhel-8.3-rust (https://download.01.org/0day-ci/archive/20231013/202310132300.wnnctWmF-lkp@intel.com/config ) > compiler: clang version 16.0.4 (https://github.com/llvm/llvm-project.git ae42196bc493ffe877a7e3dff8be32035dea4d07) > reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231013/202310132300.wnnctWmF-lkp@intel.com/reproduce ) > > If you fix the issue in a separate patch/commit (i.e. not just a new version of > the same patch/commit), kindly add following tags > | Reported-by: kernel test robot <lkp@intel.com> > | Closes: https://lore.kernel.org/oe-kbuild-all/202310132300.wnnctWmF-lkp@intel.com/ > > All warnings (new ones prefixed by >>): > >>> kernel/bpf/task_iter.c:827:17: warning: no previous prototype for function 'bpf_iter_task_vma_new' [-Wmissing-prototypes] > __bpf_kfunc int bpf_iter_task_vma_new(struct bpf_iter_task_vma *it, > ^ > kernel/bpf/task_iter.c:827:13: note: declare 'static' if the function is not intended to be used outside of this translation unit > __bpf_kfunc int bpf_iter_task_vma_new(struct bpf_iter_task_vma *it, > ^ > static >>> kernel/bpf/task_iter.c:871:36: warning: no previous prototype for function 'bpf_iter_task_vma_next' [-Wmissing-prototypes] > __bpf_kfunc struct vm_area_struct *bpf_iter_task_vma_next(struct bpf_iter_task_vma *it) > ^ > kernel/bpf/task_iter.c:871:13: note: declare 'static' if the function is not intended to be used outside of this translation unit > __bpf_kfunc struct vm_area_struct *bpf_iter_task_vma_next(struct bpf_iter_task_vma *it) > ^ > static >>> kernel/bpf/task_iter.c:880:18: warning: no previous prototype for function 'bpf_iter_task_vma_destroy' [-Wmissing-prototypes] > __bpf_kfunc void bpf_iter_task_vma_destroy(struct bpf_iter_task_vma *it) > ^ > kernel/bpf/task_iter.c:880:13: note: declare 'static' if the function is not intended to be used outside of this translation unit > __bpf_kfunc void bpf_iter_task_vma_destroy(struct bpf_iter_task_vma *it) > ^ > static > 3 warnings generated. We need the following to mute these: __diag_push(); __diag_ignore_all("-Wmissing-prototypes", "kfuncs which will be used in BPF programs"); __bpf_kfunc ... __diag_pop(); Thanks, Song > > > vim +/bpf_iter_task_vma_new +827 kernel/bpf/task_iter.c > > 826 >> 827 __bpf_kfunc int bpf_iter_task_vma_new(struct bpf_iter_task_vma *it, > 828 struct task_struct *task, u64 addr) > 829 { > 830 struct bpf_iter_task_vma_kern *kit = (void *)it; > 831 bool irq_work_busy = false; > 832 int err; > 833 > 834 BUILD_BUG_ON(sizeof(struct bpf_iter_task_vma_kern) != sizeof(struct bpf_iter_task_vma)); > 835 BUILD_BUG_ON(__alignof__(struct bpf_iter_task_vma_kern) != __alignof__(struct bpf_iter_task_vma)); > 836 > 837 /* is_iter_reg_valid_uninit guarantees that kit hasn't been initialized > 838 * before, so non-NULL kit->data doesn't point to previously > 839 * bpf_mem_alloc'd bpf_iter_task_vma_kern_data > 840 */ > 841 kit->data = bpf_mem_alloc(&bpf_global_ma, sizeof(struct bpf_iter_task_vma_kern_data)); > 842 if (!kit->data) > 843 return -ENOMEM; > 844 > 845 kit->data->task = get_task_struct(task); > 846 kit->data->mm = task->mm; > 847 if (!kit->data->mm) { > 848 err = -ENOENT; > 849 goto err_cleanup_iter; > 850 } > 851 > 852 /* kit->data->work == NULL is valid after bpf_mmap_unlock_get_irq_work */ > 853 irq_work_busy = bpf_mmap_unlock_get_irq_work(&kit->data->work); > 854 if (irq_work_busy || !mmap_read_trylock(kit->data->mm)) { > 855 err = -EBUSY; > 856 goto err_cleanup_iter; > 857 } > 858 > 859 vma_iter_init(&kit->data->vmi, kit->data->mm, addr); > 860 return 0; > 861 > 862 err_cleanup_iter: > 863 if (kit->data->task) > 864 put_task_struct(kit->data->task); > 865 bpf_mem_free(&bpf_global_ma, kit->data); > 866 /* NULL kit->data signals failed bpf_iter_task_vma initialization */ > 867 kit->data = NULL; > 868 return err; > 869 } > 870 >> 871 __bpf_kfunc struct vm_area_struct *bpf_iter_task_vma_next(struct bpf_iter_task_vma *it) > 872 { > 873 struct bpf_iter_task_vma_kern *kit = (void *)it; > 874 > 875 if (!kit->data) /* bpf_iter_task_vma_new failed */ > 876 return NULL; > 877 return vma_next(&kit->data->vmi); > 878 } > 879 >> 880 __bpf_kfunc void bpf_iter_task_vma_destroy(struct bpf_iter_task_vma *it) > 881 { > 882 struct bpf_iter_task_vma_kern *kit = (void *)it; > 883 > 884 if (kit->data) { > 885 bpf_mmap_unlock_mm(kit->data->work, kit->data->mm); > 886 put_task_struct(kit->data->task); > 887 bpf_mem_free(&bpf_global_ma, kit->data); > 888 } > 889 } > 890 > > -- > 0-DAY CI Kernel Test Service > https://github.com/intel/lkp-tests/wiki
diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c index d2840dd5b00d..62a53ebfedf9 100644 --- a/kernel/bpf/helpers.c +++ b/kernel/bpf/helpers.c @@ -2552,6 +2552,9 @@ BTF_ID_FLAGS(func, bpf_dynptr_slice_rdwr, KF_RET_NULL) BTF_ID_FLAGS(func, bpf_iter_num_new, KF_ITER_NEW) BTF_ID_FLAGS(func, bpf_iter_num_next, KF_ITER_NEXT | KF_RET_NULL) BTF_ID_FLAGS(func, bpf_iter_num_destroy, KF_ITER_DESTROY) +BTF_ID_FLAGS(func, bpf_iter_task_vma_new, KF_ITER_NEW | KF_RCU) +BTF_ID_FLAGS(func, bpf_iter_task_vma_next, KF_ITER_NEXT | KF_RET_NULL) +BTF_ID_FLAGS(func, bpf_iter_task_vma_destroy, KF_ITER_DESTROY) BTF_ID_FLAGS(func, bpf_dynptr_adjust) BTF_ID_FLAGS(func, bpf_dynptr_is_null) BTF_ID_FLAGS(func, bpf_dynptr_is_rdonly) diff --git a/kernel/bpf/task_iter.c b/kernel/bpf/task_iter.c index 7473068ed313..d6e29aca201a 100644 --- a/kernel/bpf/task_iter.c +++ b/kernel/bpf/task_iter.c @@ -7,7 +7,9 @@ #include <linux/fs.h> #include <linux/fdtable.h> #include <linux/filter.h> +#include <linux/bpf_mem_alloc.h> #include <linux/btf_ids.h> +#include <linux/mm_types.h> #include "mmap_unlock_work.h" static const char * const iter_task_type_names[] = { @@ -803,6 +805,89 @@ const struct bpf_func_proto bpf_find_vma_proto = { .arg5_type = ARG_ANYTHING, }; +struct bpf_iter_task_vma_kern_data { + struct task_struct *task; + struct mm_struct *mm; + struct mmap_unlock_irq_work *work; + struct vma_iterator vmi; +}; + +struct bpf_iter_task_vma { + /* opaque iterator state; having __u64 here allows to preserve correct + * alignment requirements in vmlinux.h, generated from BTF + */ + __u64 __opaque[1]; +} __attribute__((aligned(8))); + +/* Non-opaque version of bpf_iter_task_vma */ +struct bpf_iter_task_vma_kern { + struct bpf_iter_task_vma_kern_data *data; +} __attribute__((aligned(8))); + +__bpf_kfunc int bpf_iter_task_vma_new(struct bpf_iter_task_vma *it, + struct task_struct *task, u64 addr) +{ + struct bpf_iter_task_vma_kern *kit = (void *)it; + bool irq_work_busy = false; + int err; + + BUILD_BUG_ON(sizeof(struct bpf_iter_task_vma_kern) != sizeof(struct bpf_iter_task_vma)); + BUILD_BUG_ON(__alignof__(struct bpf_iter_task_vma_kern) != __alignof__(struct bpf_iter_task_vma)); + + /* is_iter_reg_valid_uninit guarantees that kit hasn't been initialized + * before, so non-NULL kit->data doesn't point to previously + * bpf_mem_alloc'd bpf_iter_task_vma_kern_data + */ + kit->data = bpf_mem_alloc(&bpf_global_ma, sizeof(struct bpf_iter_task_vma_kern_data)); + if (!kit->data) + return -ENOMEM; + + kit->data->task = get_task_struct(task); + kit->data->mm = task->mm; + if (!kit->data->mm) { + err = -ENOENT; + goto err_cleanup_iter; + } + + /* kit->data->work == NULL is valid after bpf_mmap_unlock_get_irq_work */ + irq_work_busy = bpf_mmap_unlock_get_irq_work(&kit->data->work); + if (irq_work_busy || !mmap_read_trylock(kit->data->mm)) { + err = -EBUSY; + goto err_cleanup_iter; + } + + vma_iter_init(&kit->data->vmi, kit->data->mm, addr); + return 0; + +err_cleanup_iter: + if (kit->data->task) + put_task_struct(kit->data->task); + bpf_mem_free(&bpf_global_ma, kit->data); + /* NULL kit->data signals failed bpf_iter_task_vma initialization */ + kit->data = NULL; + return err; +} + +__bpf_kfunc struct vm_area_struct *bpf_iter_task_vma_next(struct bpf_iter_task_vma *it) +{ + struct bpf_iter_task_vma_kern *kit = (void *)it; + + if (!kit->data) /* bpf_iter_task_vma_new failed */ + return NULL; + return vma_next(&kit->data->vmi); +} + +__bpf_kfunc void bpf_iter_task_vma_destroy(struct bpf_iter_task_vma *it) +{ + struct bpf_iter_task_vma_kern *kit = (void *)it; + + if (kit->data) { + bpf_mmap_unlock_mm(kit->data->work, kit->data->mm); + put_task_struct(kit->data->task); + bpf_mem_free(&bpf_global_ma, kit->data); + } +} + DEFINE_PER_CPU(struct mmap_unlock_irq_work, mmap_unlock_work); static void do_mmap_read_unlock(struct irq_work *entry)
This patch adds kfuncs bpf_iter_task_vma_{new,next,destroy} which allow creation and manipulation of struct bpf_iter_task_vma in open-coded iterator style. BPF programs can use these kfuncs directly or through bpf_for_each macro for natural-looking iteration of all task vmas. The implementation borrows heavily from bpf_find_vma helper's locking - differing only in that it holds the mmap_read lock for all iterations while the helper only executes its provided callback on a maximum of 1 vma. Aside from locking, struct vma_iterator and vma_next do all the heavy lifting. A pointer to an inner data struct, struct bpf_iter_task_vma_data, is the only field in struct bpf_iter_task_vma. This is because the inner data struct contains a struct vma_iterator (not ptr), whose size is likely to change under us. If bpf_iter_task_vma_kern contained vma_iterator directly such a change would require change in opaque bpf_iter_task_vma struct's size. So better to allocate vma_iterator using BPF allocator, and since that alloc must already succeed, might as well allocate all iter fields, thereby freezing struct bpf_iter_task_vma size. Signed-off-by: Dave Marchevsky <davemarchevsky@fb.com> Cc: Nathan Slingerland <slinger@meta.com> --- kernel/bpf/helpers.c | 3 ++ kernel/bpf/task_iter.c | 85 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 88 insertions(+)