Re: [PATCH] procfs/dmabuf: Add /proc/<pid>/task/<tid>/dmabuf_fds

[Cc linux-api as this is a new user interface]

On Tue 26-01-21 22:51:28, Kalesh Singh wrote:

In order to measure how much memory a process actually consumes, it is
necessary to include the DMA buffer sizes for that process in the memory
accounting. Since the handle to DMA buffers are raw FDs, it is important
to be able to identify which processes have FD references to a DMA buffer.

Currently, DMA buffer FDs can be accounted using /proc/<pid>/fd/* and
/proc/<pid>/fdinfo -- both of which are only root readable, as follows:
  1. Do a readlink on each FD.
  2. If the target path begins with "/dmabuf", then the FD is a dmabuf FD.
  3. stat the file to get the dmabuf inode number.
  4. Read/ proc/<pid>/fdinfo/<fd>, to get the DMA buffer size.

Android captures per-process system memory state when certain low memory
events (e.g a foreground app kill) occur, to identify potential memory
hoggers. To include a process’s dmabuf usage as part of its memory state,
the data collection needs to be fast enough to reflect the memory state at
the time of such events.

Since reading /proc/<pid>/fd/ and /proc/<pid>/fdinfo/ requires root
privileges, this approach is not suitable for production builds. Granting
root privileges even to a system process increases the attack surface and
is highly undesirable. Additionally this is slow as it requires many
context switches for searching and getting the dma-buf info.

With the addition of per-buffer dmabuf stats in sysfs [1], the DMA buffer
details can be queried using their unique inode numbers.

This patch proposes adding a /proc/<pid>/task/<tid>/dmabuf_fds interface.

/proc/<pid>/task/<tid>/dmabuf_fds contains a list of inode numbers for
every DMA buffer FD that the task has. Entries with the same inode
number can appear more than once, indicating the total FD references
for the associated DMA buffer.

If a thread shares the same files as the group leader then its
dmabuf_fds file will be empty, as these dmabufs are reported by the
group leader.

The interface requires PTRACE_MODE_READ_FSCRED (same as /proc/<pid>/maps)
and allows the efficient accounting of per-process DMA buffer usage without

328 more quoted lines

-- 
Michal Hocko
SUSE Labs

Am 27.01.21 um 10:05 schrieb Michal Hocko:

[Cc linux-api as this is a new user interface]

On Tue 26-01-21 22:51:28, Kalesh Singh wrote:

quoted

In order to measure how much memory a process actually consumes, it is
necessary to include the DMA buffer sizes for that process in the memory
accounting. Since the handle to DMA buffers are raw FDs, it is important
to be able to identify which processes have FD references to a DMA buffer.

Currently, DMA buffer FDs can be accounted using /proc/<pid>/fd/* and
/proc/<pid>/fdinfo -- both of which are only root readable, as follows:
   1. Do a readlink on each FD.
   2. If the target path begins with "/dmabuf", then the FD is a dmabuf FD.
   3. stat the file to get the dmabuf inode number.
   4. Read/ proc/<pid>/fdinfo/<fd>, to get the DMA buffer size.

Android captures per-process system memory state when certain low memory
events (e.g a foreground app kill) occur, to identify potential memory
hoggers. To include a process’s dmabuf usage as part of its memory state,
the data collection needs to be fast enough to reflect the memory state at
the time of such events.

Since reading /proc/<pid>/fd/ and /proc/<pid>/fdinfo/ requires root
privileges, this approach is not suitable for production builds. Granting
root privileges even to a system process increases the attack surface and
is highly undesirable. Additionally this is slow as it requires many
context switches for searching and getting the dma-buf info.

With the addition of per-buffer dmabuf stats in sysfs [1], the DMA buffer
details can be queried using their unique inode numbers.

While this looks technically clean I have to agree with Daniel that this approach doesn't sounds like the right thing to do. The fundamental problem goes deeper than what's proposed here.

In general processes are currently not held accountable for memory they reference through their file descriptors. DMA-buf is just one special case.

In other words you can currently do something like this

fd = memfd_create("test", 0);
while (1)
    write(fd, buf, 1024);

and the OOM killer will terminate random processes, but never the one holding the memfd reference.

It even becomes worse with GPU and acceleration drivers where easily all of the system memory is allocated for for games or scientific applications without being able to see that in proc or sysfs.

Some years ago I've proposed a way to at least improve the OOM killer decision which process to terminate, but that never went something.

Regards,
Christian.

quoted

This patch proposes adding a /proc/<pid>/task/<tid>/dmabuf_fds interface.

/proc/<pid>/task/<tid>/dmabuf_fds contains a list of inode numbers for
every DMA buffer FD that the task has. Entries with the same inode
number can appear more than once, indicating the total FD references
for the associated DMA buffer.

If a thread shares the same files as the group leader then its
dmabuf_fds file will be empty, as these dmabufs are reported by the
group leader.

The interface requires PTRACE_MODE_READ_FSCRED (same as /proc/<pid>/maps)
and allows the efficient accounting of per-process DMA buffer usage without
requiring root privileges. (See data below)

Performance Comparison:
-----------------------

The following data compares the time to capture the sizes of all DMA
buffers referenced by FDs for all processes on an arm64 android device.

-------------------------------------------------------
                    |  Core 0 (Little)  |  Core 7 (Big) |
-------------------------------------------------------

quoted

From <pid>/fdinfo  |      318 ms       |     145 ms    |

-------------------------------------------------------
Inodes from        |      114 ms       |      27 ms    |
dmabuf_fds;        |    (2.8x  ^)      |   (5.4x  ^)   |
data from sysfs    |                   |               |
-------------------------------------------------------

It can be inferred that in the worst case there is a 2.8x speedup for
determining per-process DMA buffer FD sizes, when using the proposed
interfaces.

[1] https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.kernel.org%2Fdri-devel%2F20210119225723.388883-1-hridya%40google.com%2F&data=04%7C01%7Cchristian.koenig%40amd.com%7Cfafaecb186a8408c307208d8c2a2b264%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637473351428416475%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=NIhGLE6ysENKIZPMKari23pczegYl5xNwbz0gzK8sj4%3D&reserved=0

Signed-off-by: Kalesh Singh <redacted>
---

302 more quoted lines

On Wed 27-01-21 11:53:55, Christian König wrote:
[...]

In general processes are currently not held accountable for memory they
reference through their file descriptors. DMA-buf is just one special case.

True

In other words you can currently do something like this

fd = memfd_create("test", 0);
while (1)
    write(fd, buf, 1024);

and the OOM killer will terminate random processes, but never the one
holding the memfd reference.

memfd is just shmem under cover, no? And that means that the memory gets
accounted to MM_SHMEMPAGES. But you are right that this in its own
doesn't help much if the fd is shared and the memory stays behind a
killed victim.

But I do agree with you that there are resources which are bound to a
process life time but the oom killer has no idea about those as they are
not accounted on a per process level and/or oom_badness doesn't take
them into consideration.
-- 
Michal Hocko
SUSE Labs

Am 27.01.21 um 12:02 schrieb Michal Hocko:

On Wed 27-01-21 11:53:55, Christian König wrote:
[...]

quoted

In general processes are currently not held accountable for memory they
reference through their file descriptors. DMA-buf is just one special case.

True

quoted

In other words you can currently do something like this

fd = memfd_create("test", 0);
while (1)
     write(fd, buf, 1024);

and the OOM killer will terminate random processes, but never the one
holding the memfd reference.

memfd is just shmem under cover, no? And that means that the memory gets
accounted to MM_SHMEMPAGES. But you are right that this in its own
doesn't help much if the fd is shared and the memory stays behind a
killed victim.

I think so, yes. But I just tested this and it doesn't seem to work correctly.

When I run the few lines above the OOM killer starts to terminate processes, but since my small test program uses very very little memory basically everything else gets terminated (including X, desktop, sshd etc..) before it is terminated as well.

Regards,
Christian.

But I do agree with you that there are resources which are bound to a
process life time but the oom killer has no idea about those as they are
not accounted on a per process level and/or oom_badness doesn't take
them into consideration.

On Wed 27-01-21 12:08:50, Christian König wrote:

Am 27.01.21 um 12:02 schrieb Michal Hocko:

quoted

On Wed 27-01-21 11:53:55, Christian König wrote:
[...]

quoted

In general processes are currently not held accountable for memory they
reference through their file descriptors. DMA-buf is just one special case.

True

quoted

In other words you can currently do something like this

fd = memfd_create("test", 0);
while (1)
     write(fd, buf, 1024);

and the OOM killer will terminate random processes, but never the one
holding the memfd reference.

memfd is just shmem under cover, no? And that means that the memory gets
accounted to MM_SHMEMPAGES. But you are right that this in its own
doesn't help much if the fd is shared and the memory stays behind a
killed victim.

I think so, yes. But I just tested this and it doesn't seem to work
correctly.

When I run the few lines above the OOM killer starts to terminate processes,
but since my small test program uses very very little memory basically
everything else gets terminated (including X, desktop, sshd etc..) before it
is terminated as well.

Something worth looking into. Maybe those pages are not really accounted
properly after all. Can you send a separate email about details with oom
reports please?
-- 
Michal Hocko
SUSE Labs