On Mon, Jun 22, 2020 at 8:01 AM Matt Pallissard <matt@xxxxxxxxxxxxxx> wrote:
>
>
>
> On 2020-06-21T08:44:28 -0700, Matt Pallissard wrote:
> >
> >
> > On 2020-06-20T20:29:43 -0700, Andrii Nakryiko wrote:
> > > On Sat, Jun 20, 2020 at 1:07 PM Matt Pallissard <matt@xxxxxxxxxxxxxx> wrote:
> > > >
> > > >
> > > >
> > > >
> > > > On 2020-06-20T11:11:55 -0700, Yonghong Song wrote:
> > > > >
> > > > >
> > > > > On 6/20/20 9:22 AM, Matt Pallissard wrote:
> > > > > > New to bpf here.
> > > > > >
> > > > > > I'm trying to read values out of of mm_struct. I have code like this;
> > > > > >
> > > > > > unsigned long i[10] = {};
> > > > > > struct task_struct *t;
> > > > > > struct mm_rss_stat *rss;
> > > > > >
> > > > > > t = (struct task_struct *)bpf_get_current_task();
> > > > > > BPF_CORE_READ_INTO(&rss, t, mm, rss_stat);
> > > > > > BPF_CORE_READ_INTO(i, rss, count);
> > > > > >
> > > > > > However, all values in `i` appear to be 0 (i[MM_FILEPAGES], etc), as if no data gets copied. I'm about 100% confident that this is caused by a glaring oversight on my part.
> > > > >
> > > > > Maybe you want to check the return value of BPF_CORE_READ_INTO.
> > > > > Underlying it is using bpf_probe_read and bpf_probe_read may fail e.g., due
> > > > > to major fault.
> > > >
> > > > Doh, I should have known to check the return codes! Yes, it was failing. I knew I was overlooking something trivial.
> > > >
> > >
> > > I wrote exactly such piece of code a while ago. Here's part of it for
> > > reference, I think it will be helpful:
> > >
> > > struct task_struct *task = (struct task_struct *)bpf_get_current_task();
> > > const struct mm_struct *mm = BPF_CORE_READ(task, mm);
> > >
> > > if (mm) {
> > > u64 hiwater_rss = BPF_CORE_READ(mm, hiwater_rss);
> > > u64 file_pages = BPF_CORE_READ(mm, rss_stat.count[MM_FILEPAGES].counter);
> > > u64 anon_pages = BPF_CORE_READ(mm, rss_stat.count[MM_ANONPAGES].counter);
> > > u64 shmem_pages = BPF_CORE_READ(mm,
> > > rss_stat.count[MM_SHMEMPAGES].counter);
> > > u64 active_rss = file_pages + anon_pages + shmem_pages;
> > > /* ... */
> >
> > Thank you,
> >
> > After realizing that I was referencing the struct incorrectly, I wound up with a similar block of code. However, as I started testing it against /proc/pid/smaps[,_rollup] I noticed that my numbers didn't match up. Always smaller.
> >
> > I took a quick glance at fs/proc/task_mmu.c. I think I'll have to walk some sort of accounting structure.
>
>
> I started to take a hard look at fs/proc/task_mmu.c. With all the locking, globals, and compile-time constants, I'm not sure that it's even possible to correctly walk `vm_area_struct` in bpf.
Yes, you can't take all those locks from BPF. But reading atomic
counters from BPF should be no problem. You might get a slightly out
of sync readings, but whatever you are doing shouldn't expect to have
100% correct values anyways, because they might change so fast after
you read them.
>
> If anyone has suggestions for getting memory numbers from an entire process, not just a task/thread, I'd love to hear them. If not, I'll pursue this on my own.
For this, you'd need to iterate across many tasks and aggregate their
results based on tasks's tgid. Check iter/task programs in selftests
(progs/bpf_iter_task.c, I think).
>
>
> Matt Pallissard
