All of lore.kernel.org
 help / color / mirror / Atom feed
From: Daniel Vetter <daniel@ffwll.ch>
To: "Michel Dänzer" <michel@daenzer.net>
Cc: christian.koenig@amd.com, Michal Hocko <mhocko@kernel.org>,
	dri-devel@lists.freedesktop.org, Roman Gushchin <guro@fb.com>,
	linux-kernel@vger.kernel.org, linux-mm@kvack.org,
	amd-gfx@lists.freedesktop.org
Subject: Re: [RFC] Per file OOM badness
Date: Tue, 30 Jan 2018 11:42:16 +0100	[thread overview]
Message-ID: <20180130104216.GR25930@phenom.ffwll.local> (raw)
In-Reply-To: <3db43c1a-59b8-af86-2b87-c783c629f512@daenzer.net>

On Tue, Jan 30, 2018 at 10:43:10AM +0100, Michel Dänzer wrote:
> On 2018-01-30 10:31 AM, Daniel Vetter wrote:
> > On Wed, Jan 24, 2018 at 01:11:09PM +0100, Christian König wrote:
> >> Am 24.01.2018 um 12:50 schrieb Michal Hocko:
> >>> On Wed 24-01-18 12:23:10, Michel Dänzer wrote:
> >>>> On 2018-01-24 12:01 PM, Michal Hocko wrote:
> >>>>> On Wed 24-01-18 11:27:15, Michel Dänzer wrote:
> >>> [...]
> >>>>>> 2. If the OOM killer kills a process which is sharing BOs with another
> >>>>>> process, this should result in the other process dropping its references
> >>>>>> to the BOs as well, at which point the memory is released.
> >>>>> OK. How exactly are those BOs mapped to the userspace?
> >>>> I'm not sure what you're asking. Userspace mostly uses a GEM handle to
> >>>> refer to a BO. There can also be userspace CPU mappings of the BO's
> >>>> memory, but userspace doesn't need CPU mappings for all BOs and only
> >>>> creates them as needed.
> >>> OK, I guess you have to bear with me some more. This whole stack is a
> >>> complete uknonwn. I am mostly after finding a boundary where you can
> >>> charge the allocated memory to the process so that the oom killer can
> >>> consider it. Is there anything like that? Except for the proposed file
> >>> handle hack?
> >>
> >> Not that I knew of.
> >>
> >> As I said before we need some kind of callback that a process now starts to
> >> use a file descriptor, but without anything from that file descriptor mapped
> >> into the address space.
> > 
> > For more context: With DRI3 and wayland the compositor opens the DRM fd
> > and then passes it to the client, which then starts allocating stuff. That
> > makes book-keeping rather annoying.
> 
> Actually, what you're describing is only true for the buffers shared by
> an X server with an X11 compositor. For the actual applications, the
> buffers are created on the client side and then shared with the X server
> / Wayland compositor.
> 
> Anyway, it doesn't really matter. In all cases, the buffers are actually
> used by all parties that are sharing them, so charging the memory to all
> of them is perfectly appropriate.
> 
> 
> > I guess a good first order approximation would be if we simply charge any
> > newly allocated buffers to the process that created them, but that means
> > hanging onto lots of mm_struct pointers since we want to make sure we then
> > release those pages to the right mm again (since the process that drops
> > the last ref might be a totally different one, depending upon how the
> > buffers or DRM fd have been shared).
> > 
> > Would it be ok to hang onto potentially arbitrary mmget references
> > essentially forever? If that's ok I think we can do your process based
> > account (minus a few minor inaccuracies for shared stuff perhaps, but no
> > one cares about that).
> 
> Honestly, I think you and Christian are overthinking this. Let's try
> charging the memory to every process which shares a buffer, and go from
> there.

I'm not concerned about wrongly accounting shared buffers (they don't
matter), but imbalanced accounting. I.e. allocate a buffer in the client,
share it, but then the compositor drops the last reference.

If we store the mm_struct pointer in drm_gem_object, we don't need any
callback from the vfs when fds are shared or anything like that. We can
simply account any newly allocated buffers to the current->mm, and then
store that later for dropping the account for when the gem obj is
released. This would entirely ignore any complications with shared
buffers, which I think we can do because even when we pass the DRM fd to a
different process, the actual buffer allocations are not passed around
like that for private buffers. And private buffers are the only ones that
really matter.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

WARNING: multiple messages have this Message-ID (diff)
From: Daniel Vetter <daniel@ffwll.ch>
To: "Michel Dänzer" <michel@daenzer.net>
Cc: christian.koenig@amd.com, Michal Hocko <mhocko@kernel.org>,
	dri-devel@lists.freedesktop.org, Roman Gushchin <guro@fb.com>,
	linux-kernel@vger.kernel.org, linux-mm@kvack.org,
	amd-gfx@lists.freedesktop.org
Subject: Re: [RFC] Per file OOM badness
Date: Tue, 30 Jan 2018 11:42:16 +0100	[thread overview]
Message-ID: <20180130104216.GR25930@phenom.ffwll.local> (raw)
In-Reply-To: <3db43c1a-59b8-af86-2b87-c783c629f512@daenzer.net>

On Tue, Jan 30, 2018 at 10:43:10AM +0100, Michel Danzer wrote:
> On 2018-01-30 10:31 AM, Daniel Vetter wrote:
> > On Wed, Jan 24, 2018 at 01:11:09PM +0100, Christian Konig wrote:
> >> Am 24.01.2018 um 12:50 schrieb Michal Hocko:
> >>> On Wed 24-01-18 12:23:10, Michel Danzer wrote:
> >>>> On 2018-01-24 12:01 PM, Michal Hocko wrote:
> >>>>> On Wed 24-01-18 11:27:15, Michel Danzer wrote:
> >>> [...]
> >>>>>> 2. If the OOM killer kills a process which is sharing BOs with another
> >>>>>> process, this should result in the other process dropping its references
> >>>>>> to the BOs as well, at which point the memory is released.
> >>>>> OK. How exactly are those BOs mapped to the userspace?
> >>>> I'm not sure what you're asking. Userspace mostly uses a GEM handle to
> >>>> refer to a BO. There can also be userspace CPU mappings of the BO's
> >>>> memory, but userspace doesn't need CPU mappings for all BOs and only
> >>>> creates them as needed.
> >>> OK, I guess you have to bear with me some more. This whole stack is a
> >>> complete uknonwn. I am mostly after finding a boundary where you can
> >>> charge the allocated memory to the process so that the oom killer can
> >>> consider it. Is there anything like that? Except for the proposed file
> >>> handle hack?
> >>
> >> Not that I knew of.
> >>
> >> As I said before we need some kind of callback that a process now starts to
> >> use a file descriptor, but without anything from that file descriptor mapped
> >> into the address space.
> > 
> > For more context: With DRI3 and wayland the compositor opens the DRM fd
> > and then passes it to the client, which then starts allocating stuff. That
> > makes book-keeping rather annoying.
> 
> Actually, what you're describing is only true for the buffers shared by
> an X server with an X11 compositor. For the actual applications, the
> buffers are created on the client side and then shared with the X server
> / Wayland compositor.
> 
> Anyway, it doesn't really matter. In all cases, the buffers are actually
> used by all parties that are sharing them, so charging the memory to all
> of them is perfectly appropriate.
> 
> 
> > I guess a good first order approximation would be if we simply charge any
> > newly allocated buffers to the process that created them, but that means
> > hanging onto lots of mm_struct pointers since we want to make sure we then
> > release those pages to the right mm again (since the process that drops
> > the last ref might be a totally different one, depending upon how the
> > buffers or DRM fd have been shared).
> > 
> > Would it be ok to hang onto potentially arbitrary mmget references
> > essentially forever? If that's ok I think we can do your process based
> > account (minus a few minor inaccuracies for shared stuff perhaps, but no
> > one cares about that).
> 
> Honestly, I think you and Christian are overthinking this. Let's try
> charging the memory to every process which shares a buffer, and go from
> there.

I'm not concerned about wrongly accounting shared buffers (they don't
matter), but imbalanced accounting. I.e. allocate a buffer in the client,
share it, but then the compositor drops the last reference.

If we store the mm_struct pointer in drm_gem_object, we don't need any
callback from the vfs when fds are shared or anything like that. We can
simply account any newly allocated buffers to the current->mm, and then
store that later for dropping the account for when the gem obj is
released. This would entirely ignore any complications with shared
buffers, which I think we can do because even when we pass the DRM fd to a
different process, the actual buffer allocations are not passed around
like that for private buffers. And private buffers are the only ones that
really matter.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

WARNING: multiple messages have this Message-ID (diff)
From: Daniel Vetter <daniel@ffwll.ch>
To: "Michel Dänzer" <michel@daenzer.net>
Cc: linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org,
	christian.koenig@amd.com, linux-mm@kvack.org,
	amd-gfx@lists.freedesktop.org, Michal Hocko <mhocko@kernel.org>,
	Roman Gushchin <guro@fb.com>
Subject: Re: [RFC] Per file OOM badness
Date: Tue, 30 Jan 2018 11:42:16 +0100	[thread overview]
Message-ID: <20180130104216.GR25930@phenom.ffwll.local> (raw)
In-Reply-To: <3db43c1a-59b8-af86-2b87-c783c629f512@daenzer.net>

On Tue, Jan 30, 2018 at 10:43:10AM +0100, Michel Dänzer wrote:
> On 2018-01-30 10:31 AM, Daniel Vetter wrote:
> > On Wed, Jan 24, 2018 at 01:11:09PM +0100, Christian König wrote:
> >> Am 24.01.2018 um 12:50 schrieb Michal Hocko:
> >>> On Wed 24-01-18 12:23:10, Michel Dänzer wrote:
> >>>> On 2018-01-24 12:01 PM, Michal Hocko wrote:
> >>>>> On Wed 24-01-18 11:27:15, Michel Dänzer wrote:
> >>> [...]
> >>>>>> 2. If the OOM killer kills a process which is sharing BOs with another
> >>>>>> process, this should result in the other process dropping its references
> >>>>>> to the BOs as well, at which point the memory is released.
> >>>>> OK. How exactly are those BOs mapped to the userspace?
> >>>> I'm not sure what you're asking. Userspace mostly uses a GEM handle to
> >>>> refer to a BO. There can also be userspace CPU mappings of the BO's
> >>>> memory, but userspace doesn't need CPU mappings for all BOs and only
> >>>> creates them as needed.
> >>> OK, I guess you have to bear with me some more. This whole stack is a
> >>> complete uknonwn. I am mostly after finding a boundary where you can
> >>> charge the allocated memory to the process so that the oom killer can
> >>> consider it. Is there anything like that? Except for the proposed file
> >>> handle hack?
> >>
> >> Not that I knew of.
> >>
> >> As I said before we need some kind of callback that a process now starts to
> >> use a file descriptor, but without anything from that file descriptor mapped
> >> into the address space.
> > 
> > For more context: With DRI3 and wayland the compositor opens the DRM fd
> > and then passes it to the client, which then starts allocating stuff. That
> > makes book-keeping rather annoying.
> 
> Actually, what you're describing is only true for the buffers shared by
> an X server with an X11 compositor. For the actual applications, the
> buffers are created on the client side and then shared with the X server
> / Wayland compositor.
> 
> Anyway, it doesn't really matter. In all cases, the buffers are actually
> used by all parties that are sharing them, so charging the memory to all
> of them is perfectly appropriate.
> 
> 
> > I guess a good first order approximation would be if we simply charge any
> > newly allocated buffers to the process that created them, but that means
> > hanging onto lots of mm_struct pointers since we want to make sure we then
> > release those pages to the right mm again (since the process that drops
> > the last ref might be a totally different one, depending upon how the
> > buffers or DRM fd have been shared).
> > 
> > Would it be ok to hang onto potentially arbitrary mmget references
> > essentially forever? If that's ok I think we can do your process based
> > account (minus a few minor inaccuracies for shared stuff perhaps, but no
> > one cares about that).
> 
> Honestly, I think you and Christian are overthinking this. Let's try
> charging the memory to every process which shares a buffer, and go from
> there.

I'm not concerned about wrongly accounting shared buffers (they don't
matter), but imbalanced accounting. I.e. allocate a buffer in the client,
share it, but then the compositor drops the last reference.

If we store the mm_struct pointer in drm_gem_object, we don't need any
callback from the vfs when fds are shared or anything like that. We can
simply account any newly allocated buffers to the current->mm, and then
store that later for dropping the account for when the gem obj is
released. This would entirely ignore any complications with shared
buffers, which I think we can do because even when we pass the DRM fd to a
different process, the actual buffer allocations are not passed around
like that for private buffers. And private buffers are the only ones that
really matter.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

  parent reply	other threads:[~2018-01-30 10:42 UTC|newest]

Thread overview: 170+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-01-18 16:47 [RFC] Per file OOM badness Andrey Grodzovsky
2018-01-18 16:47 ` Andrey Grodzovsky
2018-01-18 16:47 ` Andrey Grodzovsky
2018-01-18 16:47 ` [PATCH 1/4] fs: add OOM badness callback in file_operatrations struct Andrey Grodzovsky
2018-01-18 16:47   ` Andrey Grodzovsky
2018-01-18 16:47   ` Andrey Grodzovsky
2018-01-18 16:47 ` [PATCH 2/4] oom: take per file badness into account Andrey Grodzovsky
2018-01-18 16:47   ` Andrey Grodzovsky
2018-01-18 16:47   ` Andrey Grodzovsky
2018-01-18 16:47 ` [PATCH 3/4] drm/gem: adjust per file OOM badness on handling buffers Andrey Grodzovsky
2018-01-18 16:47   ` Andrey Grodzovsky
2018-01-18 16:47   ` Andrey Grodzovsky
2018-01-19  6:01   ` Chunming Zhou
2018-01-19  6:01     ` Chunming Zhou
2018-01-19  6:01     ` Chunming Zhou
2018-01-18 16:47 ` [PATCH 4/4] drm/amdgpu: Use drm_oom_badness for amdgpu Andrey Grodzovsky
2018-01-18 16:47   ` Andrey Grodzovsky
2018-01-18 16:47   ` Andrey Grodzovsky
2018-01-30  9:24   ` Daniel Vetter
2018-01-30  9:24     ` Daniel Vetter
2018-01-30 12:42     ` Andrey Grodzovsky
2018-01-30 12:42       ` Andrey Grodzovsky
2018-01-30 12:42       ` Andrey Grodzovsky
2018-01-18 17:00 ` [RFC] Per file OOM badness Michal Hocko
2018-01-18 17:00   ` Michal Hocko
2018-01-18 17:00   ` Michal Hocko
2018-01-18 17:13   ` Michal Hocko
2018-01-18 17:13     ` Michal Hocko
2018-01-18 20:01     ` Eric Anholt
2018-01-19  8:20       ` Michal Hocko
2018-01-19  8:20         ` Michal Hocko
2018-01-19  8:20         ` Michal Hocko
2018-01-19  8:39         ` Christian König
2018-01-19  8:39           ` Christian König
2018-01-19  8:39           ` Christian König
2018-01-19  9:32           ` Michel Dänzer
2018-01-19  9:32             ` Michel Dänzer
2018-01-19  9:32             ` Michel Dänzer
2018-01-19  9:58             ` Christian König
2018-01-19  9:58               ` Christian König
2018-01-19  9:58               ` Christian König
2018-01-19 10:02               ` Michel Dänzer
2018-01-19 10:02                 ` Michel Dänzer
2018-01-19 10:02                 ` Michel Dänzer
2018-01-19 15:07                 ` Michel Dänzer
2018-01-19 15:07                   ` Michel Dänzer
2018-01-21  6:50                   ` Eric Anholt
2018-01-19 10:40           ` Michal Hocko
2018-01-19 10:40             ` Michal Hocko
2018-01-19 10:40             ` Michal Hocko
2018-01-19 11:37             ` Christian König
2018-01-19 11:37               ` Christian König
2018-01-19 11:37               ` Christian König
2018-01-19 12:13               ` Michal Hocko
2018-01-19 12:13                 ` Michal Hocko
2018-01-19 12:13                 ` Michal Hocko
2018-01-19 12:20                 ` Michal Hocko
2018-01-19 12:20                   ` Michal Hocko
2018-01-19 12:20                   ` Michal Hocko
2018-01-19 16:54                   ` Christian König
2018-01-19 16:54                     ` Christian König
2018-01-19 16:54                     ` Christian König
2018-01-23 11:39                     ` Michal Hocko
2018-01-23 11:39                       ` Michal Hocko
2018-01-23 11:39                       ` Michal Hocko
2018-01-19 16:48               ` Michel Dänzer
2018-01-19 16:48                 ` Michel Dänzer
2018-01-19 16:48                 ` Michel Dänzer
2018-01-19  8:35       ` Christian König
2018-01-19  8:35         ` Christian König
2018-01-19  6:01     ` He, Roger
2018-01-19  8:25       ` Michal Hocko
2018-01-19  8:25         ` Michal Hocko
2018-01-19 10:02         ` roger
2018-01-19 10:02           ` roger
2018-01-23 15:27   ` Roman Gushchin
2018-01-23 15:27     ` Roman Gushchin
2018-01-23 15:27     ` Roman Gushchin
2018-01-23 15:36     ` Michal Hocko
2018-01-23 15:36       ` Michal Hocko
2018-01-23 15:36       ` Michal Hocko
2018-01-23 16:39       ` Michel Dänzer
2018-01-23 16:39         ` Michel Dänzer
2018-01-23 16:39         ` Michel Dänzer
2018-01-24  9:28         ` Michal Hocko
2018-01-24  9:28           ` Michal Hocko
2018-01-24  9:28           ` Michal Hocko
2018-01-24 10:27           ` Michel Dänzer
2018-01-24 10:27             ` Michel Dänzer
2018-01-24 10:27             ` Michel Dänzer
2018-01-24 11:01             ` Michal Hocko
2018-01-24 11:01               ` Michal Hocko
2018-01-24 11:01               ` Michal Hocko
2018-01-24 11:23               ` Michel Dänzer
2018-01-24 11:23                 ` Michel Dänzer
2018-01-24 11:23                 ` Michel Dänzer
2018-01-24 11:50                 ` Michal Hocko
2018-01-24 11:50                   ` Michal Hocko
2018-01-24 11:50                   ` Michal Hocko
2018-01-24 12:11                   ` Christian König
2018-01-24 12:11                     ` Christian König
2018-01-30  9:31                     ` Daniel Vetter
2018-01-30  9:31                       ` Daniel Vetter
2018-01-30  9:31                       ` Daniel Vetter
2018-01-30  9:43                       ` Michel Dänzer
2018-01-30  9:43                         ` Michel Dänzer
2018-01-30  9:43                         ` Michel Dänzer
2018-01-30 10:40                         ` Christian König
2018-01-30 10:40                           ` Christian König
2018-01-30 10:40                           ` Christian König
2018-01-30 11:02                           ` Michel Dänzer
2018-01-30 11:02                             ` Michel Dänzer
2018-01-30 11:02                             ` Michel Dänzer
2018-01-30 11:28                             ` Christian König
2018-01-30 11:28                               ` Christian König
2018-01-30 11:34                               ` Michel Dänzer
2018-01-30 11:34                                 ` Michel Dänzer
2018-01-30 11:34                                 ` Michel Dänzer
2018-01-30 11:36                                 ` Nicolai Hähnle
2018-01-30 11:36                                   ` Nicolai Hähnle
2018-01-30 11:36                                   ` Nicolai Hähnle
2018-01-30 11:42                                   ` Michel Dänzer
2018-01-30 11:42                                     ` Michel Dänzer
2018-01-30 11:42                                     ` Michel Dänzer
2018-01-30 11:56                                     ` Christian König
2018-01-30 11:56                                       ` Christian König
2018-01-30 11:56                                       ` Christian König
2018-01-30 15:52                                       ` Michel Dänzer
2018-01-30 15:52                                         ` Michel Dänzer
2018-01-30 15:52                                         ` Michel Dänzer
2018-01-30 10:42                         ` Daniel Vetter [this message]
2018-01-30 10:42                           ` Daniel Vetter
2018-01-30 10:42                           ` Daniel Vetter
2018-01-30 10:48                           ` Michel Dänzer
2018-01-30 10:48                             ` Michel Dänzer
2018-01-30 10:48                             ` Michel Dänzer
2018-01-30 11:35                             ` Nicolai Hähnle
2018-01-30 11:35                               ` Nicolai Hähnle
2018-01-30 11:35                               ` Nicolai Hähnle
2018-01-24 14:31                   ` Michel Dänzer
2018-01-24 14:31                     ` Michel Dänzer
2018-01-24 14:31                     ` Michel Dänzer
2018-01-30  9:29                   ` Michel Dänzer
2018-01-30  9:29                     ` Michel Dänzer
2018-01-30  9:29                     ` Michel Dänzer
2018-01-30 10:28                     ` Michal Hocko
2018-01-30 10:28                       ` Michal Hocko
2018-01-30 10:28                       ` Michal Hocko
2018-03-26 14:36                       ` Lucas Stach
2018-03-26 14:36                         ` Lucas Stach
2018-03-26 14:36                         ` Lucas Stach
2018-04-04  9:09                         ` Michel Dänzer
2018-04-04  9:09                           ` Michel Dänzer
2018-04-04  9:09                           ` Michel Dänzer
2018-04-04  9:36                           ` Lucas Stach
2018-04-04  9:36                             ` Lucas Stach
2018-04-04  9:36                             ` Lucas Stach
2018-04-04  9:46                             ` Michel Dänzer
2018-04-04  9:46                               ` Michel Dänzer
2018-04-04  9:46                               ` Michel Dänzer
2018-01-19  5:39 ` He, Roger
2018-01-19  8:17   ` Christian König
2018-01-19  8:17     ` Christian König
2018-01-19  8:17     ` Christian König
2018-01-22 23:23 ` Andrew Morton
2018-01-22 23:23   ` Andrew Morton
2018-01-22 23:23   ` Andrew Morton
2018-01-23  1:59   ` Andrey Grodzovsky
2018-01-23  1:59     ` Andrey Grodzovsky
  -- strict thread matches above, loose matches on Subject: below --
2015-09-04 12:53 Christian König

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=20180130104216.GR25930@phenom.ffwll.local \
    --to=daniel@ffwll.ch \
    --cc=amd-gfx@lists.freedesktop.org \
    --cc=christian.koenig@amd.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=guro@fb.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mhocko@kernel.org \
    --cc=michel@daenzer.net \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.