linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Al Viro <viro@zeniv.linux.org.uk>
To: Kalesh Singh <kaleshsingh@google.com>
Cc: Suren Baghdasaryan <surenb@google.com>,
	Matthew Wilcox <willy@infradead.org>,
	Jonathan Corbet <corbet@lwn.net>,
	Sumit Semwal <sumit.semwal@linaro.org>,
	Steven Rostedt <rostedt@goodmis.org>,
	Ingo Molnar <mingo@redhat.com>,
	linux-doc@vger.kernel.org, LKML <linux-kernel@vger.kernel.org>,
	linux-media@vger.kernel.org,
	DRI mailing list <dri-devel@lists.freedesktop.org>,
	linaro-mm-sig@lists.linaro.org, linux-fsdevel@vger.kernel.org,
	Hridya Valsaraju <hridya@google.com>,
	Ioannis Ilkos <ilkos@google.com>,
	John Stultz <john.stultz@linaro.org>,
	kernel-team <kernel-team@android.com>
Subject: Re: [PATCH 2/2] dmabuf/tracing: Add dma-buf trace events
Date: Tue, 4 Aug 2020 19:27:24 +0100	[thread overview]
Message-ID: <20200804182724.GK1236603@ZenIV.linux.org.uk> (raw)
In-Reply-To: <20200804154451.GA948167@google.com>

On Tue, Aug 04, 2020 at 03:44:51PM +0000, Kalesh Singh wrote:

> Hi Al. Thank you for the comments. Ultimately what we need is to identify processes
> that hold a file reference to the dma-buf. Unfortunately we can't use only
> explicit dma_buf_get/dma_buf_put to track them because when an FD is being shared
> between processes the file references are taken implicitly.
> 
> For example, on the sender side:
>    unix_dgram_sendmsg -> send_scm -> __send_scm -> scm_fp_copy -> fget_raw
> and on the receiver side:
>    unix_dgram_recvmsg -> scm_recv -> scm_detach_fds -> __scm_install_fd -> get_file
> 
> I understand now that fd_install is not an appropriate abstraction level to track these.
> Is there a more appropriate alternative where we could use to track these implicit file
> references?

There is no single lock that would stabilize the descriptor tables of all
processes.  And there's not going to be one, ever - it would be a contention
point from hell, since that would've been a system-wide lock that would have
to be taken by *ALL* syscalls modifying any descriptor table.  Not going to
happen, for obvious reasons.  Moreover, you would have to have fork(2) take
the same lock, since it does copy descriptor table.  And clone(2) either does
the same, or has the child share the descriptor table of parent.

What's more, a reference to struct file can bloody well survive without
a single descriptor refering to that file.  In the example you've mentioned
above, sender has ever right to close all descriptors it has sent.   Files
will stay opened as long as the references are held in the datagram; when
that datagram is received, the references will be inserted into recepient's
descriptor table.  At that point you again have descriptors refering to
that file, can do any IO on it, etc.

So "the set of processes that hold a file reference to the dma-buf" is
	* inherently unstable, unless you are willing to freeze every
process in the system except for the one trying to find that set.
	* can remain empty for any amount of time (hours, weeks, whatever),
only to get non-empty later, with syscalls affecting the object in question
done afterwards.

So... what were you going to do with that set if you could calculate it?
If it's really "how do we debug a leak?", it's one thing; in that case
I would suggest keeping track of creation/destruction of objects (not
gaining/dropping references - actual constructors and destructors) to
see what gets stuck around for too long and use fuser(1) to try and locate
the culprits if you see that something *was* living for too long.  "Try"
since the only reference might indeed have been stashed into an SCM_RIGHTS
datagram sitting in a queue of some AF_UNIX socket.  Note that "fuser
needs elevated priveleges" is not a strong argument - the ability to
do that sort of tracking does imply elevated priveleges anyway, and
having a root process taking requests along the lines of "gimme the
list of PIDs that have such-and-such dma_buf in their descriptor table"
is not much of an attack surface.

If you want to use it for something else, you'll need to describe that
intended use; there might be sane ways to do that, but it's hard to
come up with one without knowing what's being attempted...

  reply	other threads:[~2020-08-04 18:27 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-08-03 14:47 [PATCH 0/2] Per process tracking of dma buffers Kalesh Singh
2020-08-03 14:47 ` [PATCH 1/2] fs: Add fd_install file operation Kalesh Singh
2020-08-03 16:34   ` Christoph Hellwig
2020-08-03 18:26     ` Kalesh Singh
2020-08-03 22:18   ` Al Viro
2020-08-04  0:30   ` Joel Fernandes
2020-08-04 13:54     ` Kalesh Singh
2020-08-03 14:47 ` [PATCH 2/2] dmabuf/tracing: Add dma-buf trace events Kalesh Singh
2020-08-03 15:32   ` Steven Rostedt
2020-08-03 16:33     ` Kalesh Singh
2020-08-03 15:41   ` Matthew Wilcox
2020-08-03 16:00     ` Suren Baghdasaryan
2020-08-03 16:12       ` Matthew Wilcox
2020-08-03 16:22         ` Suren Baghdasaryan
2020-08-03 22:28           ` Al Viro
2020-08-04  1:09             ` Al Viro
2020-08-04  2:10               ` Suren Baghdasaryan
2020-08-04 15:44               ` Kalesh Singh
2020-08-04 18:27                 ` Al Viro [this message]
2020-08-04 20:42                   ` Kalesh Singh
2020-08-04 20:26             ` Daniel Vetter

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200804182724.GK1236603@ZenIV.linux.org.uk \
    --to=viro@zeniv.linux.org.uk \
    --cc=corbet@lwn.net \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=hridya@google.com \
    --cc=ilkos@google.com \
    --cc=john.stultz@linaro.org \
    --cc=kaleshsingh@google.com \
    --cc=kernel-team@android.com \
    --cc=linaro-mm-sig@lists.linaro.org \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-media@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=rostedt@goodmis.org \
    --cc=sumit.semwal@linaro.org \
    --cc=surenb@google.com \
    --cc=willy@infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).