All of lore.kernel.org
 help / color / mirror / Atom feed
From: John Stultz <john.stultz@linaro.org>
To: guangming.cao@mediatek.com
Cc: "Sumit Semwal" <sumit.semwal@linaro.org>,
	"Benjamin Gaignard" <benjamin.gaignard@linaro.org>,
	"Liam Mark" <lmark@codeaurora.org>,
	"Laura Abbott" <labbott@redhat.com>,
	"Brian Starkey" <Brian.Starkey@arm.com>,
	"Christian König" <christian.koenig@amd.com>,
	"Matthias Brugger" <matthias.bgg@gmail.com>,
	"open list:DMA-BUF HEAPS FRAMEWORK" <linux-media@vger.kernel.org>,
	"open list:DMA-BUF HEAPS FRAMEWORK"
	<dri-devel@lists.freedesktop.org>,
	"moderated list:DMA-BUF HEAPS FRAMEWORK"
	<linaro-mm-sig@lists.linaro.org>,
	"open list" <linux-kernel@vger.kernel.org>,
	"moderated list:ARM/Mediatek SoC support"
	<linux-arm-kernel@lists.infradead.org>,
	"moderated list:ARM/Mediatek SoC support"
	<linux-mediatek@lists.infradead.org>,
	"Bo Song" <bo.song@mediatek.com>,
	"Libo Kang" <libo.kang@mediatek.com>,
	"jianjiao zeng" <jianjiao.zeng@mediatek.com>,
	"mingyuan ma" <mingyuan.ma@mediatek.com>,
	"Yunfei Wang" <yf.wang@mediatek.com>,
	wsd_upstream@mediatek.com
Subject: Re: [PATCH v2] dma-buf: dma-heap: Add a size check for allocation
Date: Mon, 3 Jan 2022 10:57:30 -0800	[thread overview]
Message-ID: <CALAqxLVA4jUk-2o28RZobrPDUnuXfV1xN77Pt8Pu1o27V3aUQQ@mail.gmail.com> (raw)
In-Reply-To: <20211227095102.6054-1-guangming.cao@mediatek.com>

On Mon, Dec 27, 2021 at 1:52 AM <guangming.cao@mediatek.com> wrote:
>
> From: Guangming <Guangming.Cao@mediatek.com>
>

Thanks for submitting this!

> Add a size check for allcation since the allocation size is

nit: "allocation" above.

> always less than the total DRAM size.

In general, it might be good to add more context to the commit message
to better answer *why* this change is needed rather than what the
change is doing.  ie: What negative thing happens without this change?
And so how does this change avoid or improve things?


> Signed-off-by: Guangming <Guangming.Cao@mediatek.com>
> Signed-off-by: jianjiao zeng <jianjiao.zeng@mediatek.com>
> ---
> v2: 1. update size limitation as total_dram page size.
>     2. update commit message
> ---
>  drivers/dma-buf/dma-heap.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/drivers/dma-buf/dma-heap.c b/drivers/dma-buf/dma-heap.c
> index 56bf5ad01ad5..e39d2be98d69 100644
> --- a/drivers/dma-buf/dma-heap.c
> +++ b/drivers/dma-buf/dma-heap.c
> @@ -55,6 +55,8 @@ static int dma_heap_buffer_alloc(struct dma_heap *heap, size_t len,
>         struct dma_buf *dmabuf;
>         int fd;
>
> +       if (len / PAGE_SIZE > totalram_pages())
> +               return -EINVAL;

This seems sane. I know ION used to have some 1/2 of memory cap to
avoid unnecessary memory pressure on crazy allocations.

Could you send again with an improved commit message?

thanks
-john

WARNING: multiple messages have this Message-ID (diff)
From: John Stultz <john.stultz@linaro.org>
To: guangming.cao@mediatek.com
Cc: wsd_upstream@mediatek.com, "Libo Kang" <libo.kang@mediatek.com>,
	"open list" <linux-kernel@vger.kernel.org>,
	"Liam Mark" <lmark@codeaurora.org>,
	"moderated list:DMA-BUF HEAPS FRAMEWORK"
	<linaro-mm-sig@lists.linaro.org>,
	"jianjiao zeng" <jianjiao.zeng@mediatek.com>,
	"Yunfei Wang" <yf.wang@mediatek.com>,
	"moderated list:ARM/Mediatek SoC support"
	<linux-mediatek@lists.infradead.org>,
	"open list:DMA-BUF HEAPS FRAMEWORK"
	<dri-devel@lists.freedesktop.org>,
	"Benjamin Gaignard" <benjamin.gaignard@linaro.org>,
	"Bo Song" <bo.song@mediatek.com>,
	"Matthias Brugger" <matthias.bgg@gmail.com>,
	"mingyuan ma" <mingyuan.ma@mediatek.com>,
	"Laura Abbott" <labbott@redhat.com>,
	"Christian König" <christian.koenig@amd.com>,
	"moderated list:ARM/Mediatek SoC support"
	<linux-arm-kernel@lists.infradead.org>,
	"open list:DMA-BUF HEAPS FRAMEWORK" <linux-media@vger.kernel.org>
Subject: Re: [PATCH v2] dma-buf: dma-heap: Add a size check for allocation
Date: Mon, 3 Jan 2022 10:57:30 -0800	[thread overview]
Message-ID: <CALAqxLVA4jUk-2o28RZobrPDUnuXfV1xN77Pt8Pu1o27V3aUQQ@mail.gmail.com> (raw)
In-Reply-To: <20211227095102.6054-1-guangming.cao@mediatek.com>

On Mon, Dec 27, 2021 at 1:52 AM <guangming.cao@mediatek.com> wrote:
>
> From: Guangming <Guangming.Cao@mediatek.com>
>

Thanks for submitting this!

> Add a size check for allcation since the allocation size is

nit: "allocation" above.

> always less than the total DRAM size.

In general, it might be good to add more context to the commit message
to better answer *why* this change is needed rather than what the
change is doing.  ie: What negative thing happens without this change?
And so how does this change avoid or improve things?


> Signed-off-by: Guangming <Guangming.Cao@mediatek.com>
> Signed-off-by: jianjiao zeng <jianjiao.zeng@mediatek.com>
> ---
> v2: 1. update size limitation as total_dram page size.
>     2. update commit message
> ---
>  drivers/dma-buf/dma-heap.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/drivers/dma-buf/dma-heap.c b/drivers/dma-buf/dma-heap.c
> index 56bf5ad01ad5..e39d2be98d69 100644
> --- a/drivers/dma-buf/dma-heap.c
> +++ b/drivers/dma-buf/dma-heap.c
> @@ -55,6 +55,8 @@ static int dma_heap_buffer_alloc(struct dma_heap *heap, size_t len,
>         struct dma_buf *dmabuf;
>         int fd;
>
> +       if (len / PAGE_SIZE > totalram_pages())
> +               return -EINVAL;

This seems sane. I know ION used to have some 1/2 of memory cap to
avoid unnecessary memory pressure on crazy allocations.

Could you send again with an improved commit message?

thanks
-john

WARNING: multiple messages have this Message-ID (diff)
From: John Stultz <john.stultz@linaro.org>
To: guangming.cao@mediatek.com
Cc: "Sumit Semwal" <sumit.semwal@linaro.org>,
	"Benjamin Gaignard" <benjamin.gaignard@linaro.org>,
	"Liam Mark" <lmark@codeaurora.org>,
	"Laura Abbott" <labbott@redhat.com>,
	"Brian Starkey" <Brian.Starkey@arm.com>,
	"Christian König" <christian.koenig@amd.com>,
	"Matthias Brugger" <matthias.bgg@gmail.com>,
	"open list:DMA-BUF HEAPS FRAMEWORK" <linux-media@vger.kernel.org>,
	"open list:DMA-BUF HEAPS FRAMEWORK"
	<dri-devel@lists.freedesktop.org>,
	"moderated list:DMA-BUF HEAPS FRAMEWORK"
	<linaro-mm-sig@lists.linaro.org>,
	"open list" <linux-kernel@vger.kernel.org>,
	"moderated list:ARM/Mediatek SoC support"
	<linux-arm-kernel@lists.infradead.org>,
	"moderated list:ARM/Mediatek SoC support"
	<linux-mediatek@lists.infradead.org>,
	"Bo Song" <bo.song@mediatek.com>,
	"Libo Kang" <libo.kang@mediatek.com>,
	"jianjiao zeng" <jianjiao.zeng@mediatek.com>,
	"mingyuan ma" <mingyuan.ma@mediatek.com>,
	"Yunfei Wang" <yf.wang@mediatek.com>,
	wsd_upstream@mediatek.com
Subject: Re: [PATCH v2] dma-buf: dma-heap: Add a size check for allocation
Date: Mon, 3 Jan 2022 10:57:30 -0800	[thread overview]
Message-ID: <CALAqxLVA4jUk-2o28RZobrPDUnuXfV1xN77Pt8Pu1o27V3aUQQ@mail.gmail.com> (raw)
In-Reply-To: <20211227095102.6054-1-guangming.cao@mediatek.com>

On Mon, Dec 27, 2021 at 1:52 AM <guangming.cao@mediatek.com> wrote:
>
> From: Guangming <Guangming.Cao@mediatek.com>
>

Thanks for submitting this!

> Add a size check for allcation since the allocation size is

nit: "allocation" above.

> always less than the total DRAM size.

In general, it might be good to add more context to the commit message
to better answer *why* this change is needed rather than what the
change is doing.  ie: What negative thing happens without this change?
And so how does this change avoid or improve things?


> Signed-off-by: Guangming <Guangming.Cao@mediatek.com>
> Signed-off-by: jianjiao zeng <jianjiao.zeng@mediatek.com>
> ---
> v2: 1. update size limitation as total_dram page size.
>     2. update commit message
> ---
>  drivers/dma-buf/dma-heap.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/drivers/dma-buf/dma-heap.c b/drivers/dma-buf/dma-heap.c
> index 56bf5ad01ad5..e39d2be98d69 100644
> --- a/drivers/dma-buf/dma-heap.c
> +++ b/drivers/dma-buf/dma-heap.c
> @@ -55,6 +55,8 @@ static int dma_heap_buffer_alloc(struct dma_heap *heap, size_t len,
>         struct dma_buf *dmabuf;
>         int fd;
>
> +       if (len / PAGE_SIZE > totalram_pages())
> +               return -EINVAL;

This seems sane. I know ION used to have some 1/2 of memory cap to
avoid unnecessary memory pressure on crazy allocations.

Could you send again with an improved commit message?

thanks
-john

_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

WARNING: multiple messages have this Message-ID (diff)
From: John Stultz <john.stultz@linaro.org>
To: guangming.cao@mediatek.com
Cc: "Sumit Semwal" <sumit.semwal@linaro.org>,
	"Benjamin Gaignard" <benjamin.gaignard@linaro.org>,
	"Liam Mark" <lmark@codeaurora.org>,
	"Laura Abbott" <labbott@redhat.com>,
	"Brian Starkey" <Brian.Starkey@arm.com>,
	"Christian König" <christian.koenig@amd.com>,
	"Matthias Brugger" <matthias.bgg@gmail.com>,
	"open list:DMA-BUF HEAPS FRAMEWORK" <linux-media@vger.kernel.org>,
	"open list:DMA-BUF HEAPS FRAMEWORK"
	<dri-devel@lists.freedesktop.org>,
	"moderated list:DMA-BUF HEAPS FRAMEWORK"
	<linaro-mm-sig@lists.linaro.org>,
	"open list" <linux-kernel@vger.kernel.org>,
	"moderated list:ARM/Mediatek SoC support"
	<linux-arm-kernel@lists.infradead.org>,
	"moderated list:ARM/Mediatek SoC support"
	<linux-mediatek@lists.infradead.org>,
	"Bo Song" <bo.song@mediatek.com>,
	"Libo Kang" <libo.kang@mediatek.com>,
	"jianjiao zeng" <jianjiao.zeng@mediatek.com>,
	"mingyuan ma" <mingyuan.ma@mediatek.com>,
	"Yunfei Wang" <yf.wang@mediatek.com>,
	wsd_upstream@mediatek.com
Subject: Re: [PATCH v2] dma-buf: dma-heap: Add a size check for allocation
Date: Mon, 3 Jan 2022 10:57:30 -0800	[thread overview]
Message-ID: <CALAqxLVA4jUk-2o28RZobrPDUnuXfV1xN77Pt8Pu1o27V3aUQQ@mail.gmail.com> (raw)
In-Reply-To: <20211227095102.6054-1-guangming.cao@mediatek.com>

On Mon, Dec 27, 2021 at 1:52 AM <guangming.cao@mediatek.com> wrote:
>
> From: Guangming <Guangming.Cao@mediatek.com>
>

Thanks for submitting this!

> Add a size check for allcation since the allocation size is

nit: "allocation" above.

> always less than the total DRAM size.

In general, it might be good to add more context to the commit message
to better answer *why* this change is needed rather than what the
change is doing.  ie: What negative thing happens without this change?
And so how does this change avoid or improve things?


> Signed-off-by: Guangming <Guangming.Cao@mediatek.com>
> Signed-off-by: jianjiao zeng <jianjiao.zeng@mediatek.com>
> ---
> v2: 1. update size limitation as total_dram page size.
>     2. update commit message
> ---
>  drivers/dma-buf/dma-heap.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/drivers/dma-buf/dma-heap.c b/drivers/dma-buf/dma-heap.c
> index 56bf5ad01ad5..e39d2be98d69 100644
> --- a/drivers/dma-buf/dma-heap.c
> +++ b/drivers/dma-buf/dma-heap.c
> @@ -55,6 +55,8 @@ static int dma_heap_buffer_alloc(struct dma_heap *heap, size_t len,
>         struct dma_buf *dmabuf;
>         int fd;
>
> +       if (len / PAGE_SIZE > totalram_pages())
> +               return -EINVAL;

This seems sane. I know ION used to have some 1/2 of memory cap to
avoid unnecessary memory pressure on crazy allocations.

Could you send again with an improved commit message?

thanks
-john

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

  reply	other threads:[~2022-01-03 18:57 UTC|newest]

Thread overview: 97+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-12-17  9:41 [PATCH] dma-buf: dma-heap: Add a size limitation for allocation guangming.cao
2021-12-17  9:41 ` guangming.cao
2021-12-17  9:41 ` guangming.cao
2021-12-17  9:41 ` guangming.cao
2021-12-27  9:51 ` [PATCH v2] dma-buf: dma-heap: Add a size check " guangming.cao
2021-12-27  9:51   ` guangming.cao
2021-12-27  9:51   ` guangming.cao
2021-12-27  9:51   ` guangming.cao
2022-01-03 18:57   ` John Stultz [this message]
2022-01-03 18:57     ` John Stultz
2022-01-03 18:57     ` John Stultz
2022-01-03 18:57     ` John Stultz
2022-01-04  7:47     ` Christian König
2022-01-04  7:47       ` Christian König
2022-01-04  7:47       ` Christian König
2022-01-04  7:47       ` Christian König
2022-01-04  8:44       ` Guangming.Cao
2022-01-04  8:44         ` Guangming.Cao
2022-01-04  8:44         ` Guangming.Cao
2022-01-05  6:36       ` guangming.cao
2022-01-05  6:36         ` guangming.cao
2022-01-05  6:36         ` guangming.cao
2022-01-05  6:36         ` guangming.cao
2022-01-13 10:50         ` Sumit Semwal
2022-01-13 10:50           ` Sumit Semwal
2022-01-13 10:50           ` Sumit Semwal
2022-01-13 10:50           ` Sumit Semwal
2022-01-13 12:34           ` [PATCH v3] " guangming.cao
2022-01-13 12:34             ` guangming.cao
2022-01-13 12:34             ` guangming.cao
2022-01-13 12:34             ` guangming.cao
2022-01-13 12:57             ` Ruhl, Michael J
2022-01-13 12:57               ` Ruhl, Michael J
2022-01-13 12:57               ` Ruhl, Michael J
2022-01-13 12:57               ` Ruhl, Michael J
2022-01-13 13:00               ` Ruhl, Michael J
2022-01-13 13:00                 ` Ruhl, Michael J
2022-01-13 13:00                 ` Ruhl, Michael J
2022-01-13 13:00                 ` Ruhl, Michael J
2022-01-13 13:05                 ` Christian König
2022-01-13 13:05                   ` Christian König
2022-01-13 13:05                   ` Christian König
2022-01-13 13:05                   ` Christian König
2022-01-13 23:26                   ` John Stultz
2022-01-13 23:26                     ` John Stultz
2022-01-13 23:26                     ` John Stultz
2022-01-13 23:26                     ` John Stultz
2022-01-14  7:16                     ` Christian König
2022-01-14  7:16                       ` Christian König
2022-01-14  7:16                       ` Christian König
2022-01-14  7:16                       ` Christian König
2022-01-14 12:05                       ` Guangming.Cao
2022-01-14 12:05                         ` Guangming.Cao
2022-01-14 12:05                         ` Guangming.Cao
2022-01-15  1:17                         ` John Stultz
2022-01-15  1:17                           ` John Stultz
2022-01-15  1:17                           ` John Stultz
2022-01-15  1:17                           ` John Stultz
2022-01-19  9:59                           ` Guangming.Cao
2022-01-19  9:59                             ` Guangming.Cao
2022-01-19  9:59                             ` Guangming.Cao
2022-01-19 20:37                             ` John Stultz
2022-01-19 20:37                               ` John Stultz
2022-01-19 20:37                               ` John Stultz
2022-01-19 20:37                               ` John Stultz
2022-01-20  3:34                               ` [PATCH v4] dma-buf: system_heap: " guangming.cao
2022-01-20  3:34                                 ` guangming.cao
2022-01-20  3:34                                 ` guangming.cao
2022-01-20  3:34                                 ` guangming.cao
2022-01-20  3:48                                 ` John Stultz
2022-01-20  3:48                                   ` John Stultz
2022-01-20  3:48                                   ` John Stultz
2022-01-20  3:48                                   ` John Stultz
2022-01-20  7:08                                   ` [PATCH v5] " guangming.cao
2022-01-20  7:08                                     ` guangming.cao
2022-01-20  7:08                                     ` guangming.cao
2022-01-20  7:08                                     ` guangming.cao
2022-01-20  8:27                                     ` Christian König
2022-01-20  8:27                                       ` Christian König
2022-01-20  8:27                                       ` Christian König
2022-01-20  8:27                                       ` Christian König
2022-01-20  8:52                                       ` [PATCH v6] " guangming.cao
2022-01-20  8:52                                         ` guangming.cao
2022-01-20  8:52                                         ` guangming.cao
2022-01-20  8:52                                         ` guangming.cao
2022-01-20 10:00                                         ` [PATCH v6 RESEND] " guangming.cao
2022-01-20 10:00                                           ` guangming.cao
2022-01-20 10:00                                           ` guangming.cao
2022-01-20 10:00                                           ` guangming.cao
2022-01-20 10:22                                           ` Christian König
2022-01-20 10:22                                             ` Christian König
2022-01-20 10:22                                             ` Christian König
2022-01-20 10:22                                             ` Christian König
2021-12-27  9:47 [PATCH v2] dma-buf: dma-heap: " guangming.cao
2021-12-27  9:47 ` guangming.cao
2021-12-27  9:47 ` guangming.cao
2021-12-27  9:47 ` guangming.cao

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=CALAqxLVA4jUk-2o28RZobrPDUnuXfV1xN77Pt8Pu1o27V3aUQQ@mail.gmail.com \
    --to=john.stultz@linaro.org \
    --cc=Brian.Starkey@arm.com \
    --cc=benjamin.gaignard@linaro.org \
    --cc=bo.song@mediatek.com \
    --cc=christian.koenig@amd.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=guangming.cao@mediatek.com \
    --cc=jianjiao.zeng@mediatek.com \
    --cc=labbott@redhat.com \
    --cc=libo.kang@mediatek.com \
    --cc=linaro-mm-sig@lists.linaro.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-media@vger.kernel.org \
    --cc=linux-mediatek@lists.infradead.org \
    --cc=lmark@codeaurora.org \
    --cc=matthias.bgg@gmail.com \
    --cc=mingyuan.ma@mediatek.com \
    --cc=sumit.semwal@linaro.org \
    --cc=wsd_upstream@mediatek.com \
    --cc=yf.wang@mediatek.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 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.