mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: "Christian König" <christian.koenig@amd.com>
To: John Stultz <john.stultz@linaro.org>
Cc: lkml <linux-kernel@vger.kernel.org>,
	"Daniel Vetter" <daniel@ffwll.ch>,
	"Sumit Semwal" <sumit.semwal@linaro.org>,
	"Liam Mark" <lmark@codeaurora.org>,
	"Chris Goldsworthy" <cgoldswo@codeaurora.org>,
	"Laura Abbott" <labbott@kernel.org>,
	"Brian Starkey" <Brian.Starkey@arm.com>,
	"Hridya Valsaraju" <hridya@google.com>,
	"Suren Baghdasaryan" <surenb@google.com>,
	"Sandeep Patil" <sspatil@google.com>,
	"Daniel Mentz" <danielmentz@google.com>,
	"Ørjan Eide" <orjan.eide@arm.com>,
	"Robin Murphy" <robin.murphy@arm.com>,
	"Ezequiel Garcia" <ezequiel@collabora.com>,
	"Simon Ser" <contact@emersion.fr>,
	"James Jones" <jajones@nvidia.com>,
	linux-media <linux-media@vger.kernel.org>,
	dri-devel <dri-devel@lists.freedesktop.org>
Subject: Re: [RFC][PATCH v6 1/7] drm: Add a sharable drm page-pool implementation
Date: Tue, 9 Feb 2021 13:57:17 +0100	[thread overview]
Message-ID: <a9dd7f8a-ef30-9eb4-4834-37801d43b96f@amd.com> (raw)
In-Reply-To: <48225879-2fe1-22ac-daae-c61d52465aea@amd.com>

Am 09.02.21 um 13:11 schrieb Christian König:
> [SNIP]
>>>> +void drm_page_pool_add(struct drm_page_pool *pool, struct page *page)
>>>> +{
>>>> +     spin_lock(&pool->lock);
>>>> +     list_add_tail(&page->lru, &pool->items);
>>>> +     pool->count++;
>>>> +     atomic_long_add(1 << pool->order, &total_pages);
>>>> +     spin_unlock(&pool->lock);
>>>> +
>>>> +     mod_node_page_state(page_pgdat(page), 
>>>> NR_KERNEL_MISC_RECLAIMABLE,
>>>> +                         1 << pool->order);
>>> Hui what? What should that be good for?
>> This is a carryover from the ION page pool implementation:
>> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit.kernel.org%2Fpub%2Fscm%2Flinux%2Fkernel%2Fgit%2Ftorvalds%2Flinux.git%2Ftree%2Fdrivers%2Fstaging%2Fandroid%2Fion%2Fion_page_pool.c%3Fh%3Dv5.10%23n28&amp;data=04%7C01%7Cchristian.koenig%40amd.com%7Cc4eadb0a9cf6491d99ba08d8ca173457%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637481548325174885%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=FUjZK5NSDMUYfU7vGeE4fDU2HCF%2FYyNBwc30aoLLPQ4%3D&amp;reserved=0 
>>
>>
>> My sense is it helps with the vmstat/meminfo accounting so folks can
>> see the cached pages are shrinkable/freeable. This maybe falls under
>> other dmabuf accounting/stats discussions, so I'm happy to remove it
>> for now, or let the drivers using the shared page pool logic handle
>> the accounting themselves?

Intentionally separated the discussion for that here.

As far as I can see this is just bluntly incorrect.

Either the page is reclaimable or it is part of our pool and freeable 
through the shrinker, but never ever both.

In the best case this just messes up the accounting, in the worst case 
it can cause memory corruption.

Christian.

  reply	other threads:[~2021-02-09 12:58 UTC|newest]

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-02-05  8:06 [RFC][PATCH v6 0/7] Generic page pool & deferred freeing for system dmabuf heap John Stultz
2021-02-05  8:06 ` [RFC][PATCH v6 1/7] drm: Add a sharable drm page-pool implementation John Stultz
2021-02-05  8:46   ` Christian König
2021-02-05 20:46     ` John Stultz
2021-02-05 22:38       ` Suren Baghdasaryan
2021-02-09 12:11       ` Christian König
2021-02-09 12:57         ` Christian König [this message]
2021-02-09 17:33           ` Suren Baghdasaryan
2021-02-09 17:46             ` Christian König
2021-02-09 17:52               ` Suren Baghdasaryan
2021-02-09 20:03               ` Daniel Vetter
2021-02-09 20:16                 ` Suren Baghdasaryan
2021-02-10 13:06                   ` Daniel Vetter
2021-02-10 16:39                     ` Suren Baghdasaryan
2021-02-10 17:21                       ` Daniel Vetter
2021-02-10 17:28                         ` Suren Baghdasaryan
2021-02-10 18:32                       ` Christian König
2021-02-10 19:12                         ` Suren Baghdasaryan
2021-02-10 19:23                           ` Christian König
2021-02-09 17:51         ` John Stultz
2021-02-05  8:06 ` [RFC][PATCH v6 2/7] drm: ttm_pool: Rename the ttm_pool_dma structure to ttm_pool_page_dat John Stultz
2021-02-05  8:06 ` [RFC][PATCH v6 3/7] drm: ttm_pool: Rework ttm_pool_free_page to allow us to use it as a function pointer John Stultz
2021-02-05  8:28   ` Christian König
2021-02-05 19:47     ` John Stultz
2021-02-09 12:13       ` Christian König
2021-02-09 17:39         ` John Stultz
2021-02-05  8:06 ` [RFC][PATCH v6 4/7] drm: ttm_pool: Rework ttm_pool to use drm_page_pool John Stultz
2021-02-05  8:50   ` Christian König
2021-02-05  8:06 ` [RFC][PATCH v6 5/7] dma-buf: heaps: Add deferred-free-helper library code John Stultz
2021-02-05  8:06 ` [RFC][PATCH v6 6/7] dma-buf: system_heap: Add drm pagepool support to system heap John Stultz
2021-02-05  8:06 ` [RFC][PATCH v6 7/7] dma-buf: system_heap: Add deferred freeing to the " John Stultz
2021-02-05 10:36 ` [RFC][PATCH v6 0/7] Generic page pool & deferred freeing for system dmabuf heap Christian König
2021-02-05 20:57   ` John Stultz

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=a9dd7f8a-ef30-9eb4-4834-37801d43b96f@amd.com \
    --to=christian.koenig@amd.com \
    --cc=Brian.Starkey@arm.com \
    --cc=cgoldswo@codeaurora.org \
    --cc=contact@emersion.fr \
    --cc=daniel@ffwll.ch \
    --cc=danielmentz@google.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=ezequiel@collabora.com \
    --cc=hridya@google.com \
    --cc=jajones@nvidia.com \
    --cc=john.stultz@linaro.org \
    --cc=labbott@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-media@vger.kernel.org \
    --cc=lmark@codeaurora.org \
    --cc=orjan.eide@arm.com \
    --cc=robin.murphy@arm.com \
    --cc=sspatil@google.com \
    --cc=sumit.semwal@linaro.org \
    --cc=surenb@google.com \
    /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

all inboxes | Powered by JetHome®