From: Daniel Vetter <daniel@ffwll.ch>
To: Gerd Hoffmann <kraxel@redhat.com>
Cc: dri-devel <dri-devel@lists.freedesktop.org>,
David Airlie <airlied@linux.ie>,
Tomeu Vizoso <tomeu.vizoso@collabora.com>,
Sumit Semwal <sumit.semwal@linaro.org>,
Shuah Khan <shuah@kernel.org>,
open list <linux-kernel@vger.kernel.org>,
"open list:DMA BUFFER SHARING FRAMEWORK"
<linux-media@vger.kernel.org>,
"moderated list:DMA BUFFER SHARING FRAMEWORK"
<linaro-mm-sig@lists.linaro.org>,
"open list:KERNEL SELFTEST FRAMEWORK"
<linux-kselftest@vger.kernel.org>
Subject: Re: [PATCH v3] Add udmabuf misc device
Date: Tue, 29 May 2018 14:30:55 +0200 [thread overview]
Message-ID: <CAKMK7uGnxNwOUoY=kqG4hW4AZd8Fouh9GRq8Na-fRQUUvF9fZw@mail.gmail.com> (raw)
In-Reply-To: <20180529084845.2al2dmpvjpz6eexp@sirius.home.kraxel.org>
On Tue, May 29, 2018 at 10:48 AM, Gerd Hoffmann <kraxel@redhat.com> wrote:
> Hi,
>
>> > +static void *kmap_atomic_udmabuf(struct dma_buf *buf, unsigned long page_num)
>> > +{
>> > + struct udmabuf *ubuf = buf->priv;
>> > + struct page *page = ubuf->pages[page_num];
>> > +
>> > + return kmap_atomic(page);
>> > +}
>> > +
>> > +static void *kmap_udmabuf(struct dma_buf *buf, unsigned long page_num)
>> > +{
>> > + struct udmabuf *ubuf = buf->priv;
>> > + struct page *page = ubuf->pages[page_num];
>> > +
>> > + return kmap(page);
>> > +}
>>
>> The above leaks like mad since no kunamp?
>
> /me checks code. Oops. Yes.
>
> The docs say map() is required and unmap() is not (for both atomic and
> non-atomic cases), so I assumed there is a default implementation just
> doing kunmap(page). Which is not the case. /me looks a bit surprised.
>
> I'll fix it for v4.
>
>> Also I think we have 0 users of the kmap atomic interfaces ... so not sure
>> whether it's worth it to implement those.
>
> Well, the docs are correct. kmap_atomic() is required, dma-buf.c calls
> the function pointer without checking it exists beforehand ...
Frankly with the pletoria of dummy kmap functions that just return
NULL; it might be better to move that into core dma-buf code and make
it optional for real. Since it's indeed very surprising.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
WARNING: multiple messages have this Message-ID (diff)
From: daniel at ffwll.ch (Daniel Vetter)
Subject: [PATCH v3] Add udmabuf misc device
Date: Tue, 29 May 2018 14:30:55 +0200 [thread overview]
Message-ID: <CAKMK7uGnxNwOUoY=kqG4hW4AZd8Fouh9GRq8Na-fRQUUvF9fZw@mail.gmail.com> (raw)
In-Reply-To: <20180529084845.2al2dmpvjpz6eexp@sirius.home.kraxel.org>
On Tue, May 29, 2018 at 10:48 AM, Gerd Hoffmann <kraxel at redhat.com> wrote:
> Hi,
>
>> > +static void *kmap_atomic_udmabuf(struct dma_buf *buf, unsigned long page_num)
>> > +{
>> > + struct udmabuf *ubuf = buf->priv;
>> > + struct page *page = ubuf->pages[page_num];
>> > +
>> > + return kmap_atomic(page);
>> > +}
>> > +
>> > +static void *kmap_udmabuf(struct dma_buf *buf, unsigned long page_num)
>> > +{
>> > + struct udmabuf *ubuf = buf->priv;
>> > + struct page *page = ubuf->pages[page_num];
>> > +
>> > + return kmap(page);
>> > +}
>>
>> The above leaks like mad since no kunamp?
>
> /me checks code. Oops. Yes.
>
> The docs say map() is required and unmap() is not (for both atomic and
> non-atomic cases), so I assumed there is a default implementation just
> doing kunmap(page). Which is not the case. /me looks a bit surprised.
>
> I'll fix it for v4.
>
>> Also I think we have 0 users of the kmap atomic interfaces ... so not sure
>> whether it's worth it to implement those.
>
> Well, the docs are correct. kmap_atomic() is required, dma-buf.c calls
> the function pointer without checking it exists beforehand ...
Frankly with the pletoria of dummy kmap functions that just return
NULL; it might be better to move that into core dma-buf code and make
it optional for real. Since it's indeed very surprising.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
--
To unsubscribe from this list: send the line "unsubscribe linux-kselftest" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
WARNING: multiple messages have this Message-ID (diff)
From: daniel@ffwll.ch (Daniel Vetter)
Subject: [PATCH v3] Add udmabuf misc device
Date: Tue, 29 May 2018 14:30:55 +0200 [thread overview]
Message-ID: <CAKMK7uGnxNwOUoY=kqG4hW4AZd8Fouh9GRq8Na-fRQUUvF9fZw@mail.gmail.com> (raw)
Message-ID: <20180529123055.4Pr2qpSgnLdqfPRuWpUUZu8kg2B05EcU-z01uqqPPjI@z> (raw)
In-Reply-To: <20180529084845.2al2dmpvjpz6eexp@sirius.home.kraxel.org>
On Tue, May 29, 2018@10:48 AM, Gerd Hoffmann <kraxel@redhat.com> wrote:
> Hi,
>
>> > +static void *kmap_atomic_udmabuf(struct dma_buf *buf, unsigned long page_num)
>> > +{
>> > + struct udmabuf *ubuf = buf->priv;
>> > + struct page *page = ubuf->pages[page_num];
>> > +
>> > + return kmap_atomic(page);
>> > +}
>> > +
>> > +static void *kmap_udmabuf(struct dma_buf *buf, unsigned long page_num)
>> > +{
>> > + struct udmabuf *ubuf = buf->priv;
>> > + struct page *page = ubuf->pages[page_num];
>> > +
>> > + return kmap(page);
>> > +}
>>
>> The above leaks like mad since no kunamp?
>
> /me checks code. Oops. Yes.
>
> The docs say map() is required and unmap() is not (for both atomic and
> non-atomic cases), so I assumed there is a default implementation just
> doing kunmap(page). Which is not the case. /me looks a bit surprised.
>
> I'll fix it for v4.
>
>> Also I think we have 0 users of the kmap atomic interfaces ... so not sure
>> whether it's worth it to implement those.
>
> Well, the docs are correct. kmap_atomic() is required, dma-buf.c calls
> the function pointer without checking it exists beforehand ...
Frankly with the pletoria of dummy kmap functions that just return
NULL; it might be better to move that into core dma-buf code and make
it optional for real. Since it's indeed very surprising.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
--
To unsubscribe from this list: send the line "unsubscribe linux-kselftest" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
WARNING: multiple messages have this Message-ID (diff)
From: Daniel Vetter <daniel@ffwll.ch>
To: Gerd Hoffmann <kraxel@redhat.com>
Cc: Tomeu Vizoso <tomeu.vizoso@collabora.com>,
David Airlie <airlied@linux.ie>,
open list <linux-kernel@vger.kernel.org>,
dri-devel <dri-devel@lists.freedesktop.org>,
"moderated list:DMA BUFFER SHARING FRAMEWORK"
<linaro-mm-sig@lists.linaro.org>,
"open list:KERNEL SELFTEST FRAMEWORK"
<linux-kselftest@vger.kernel.org>, Shuah Khan <shuah@kernel.org>,
"open list:DMA BUFFER SHARING FRAMEWORK"
<linux-media@vger.kernel.org>
Subject: Re: [PATCH v3] Add udmabuf misc device
Date: Tue, 29 May 2018 14:30:55 +0200 [thread overview]
Message-ID: <CAKMK7uGnxNwOUoY=kqG4hW4AZd8Fouh9GRq8Na-fRQUUvF9fZw@mail.gmail.com> (raw)
In-Reply-To: <20180529084845.2al2dmpvjpz6eexp@sirius.home.kraxel.org>
On Tue, May 29, 2018 at 10:48 AM, Gerd Hoffmann <kraxel@redhat.com> wrote:
> Hi,
>
>> > +static void *kmap_atomic_udmabuf(struct dma_buf *buf, unsigned long page_num)
>> > +{
>> > + struct udmabuf *ubuf = buf->priv;
>> > + struct page *page = ubuf->pages[page_num];
>> > +
>> > + return kmap_atomic(page);
>> > +}
>> > +
>> > +static void *kmap_udmabuf(struct dma_buf *buf, unsigned long page_num)
>> > +{
>> > + struct udmabuf *ubuf = buf->priv;
>> > + struct page *page = ubuf->pages[page_num];
>> > +
>> > + return kmap(page);
>> > +}
>>
>> The above leaks like mad since no kunamp?
>
> /me checks code. Oops. Yes.
>
> The docs say map() is required and unmap() is not (for both atomic and
> non-atomic cases), so I assumed there is a default implementation just
> doing kunmap(page). Which is not the case. /me looks a bit surprised.
>
> I'll fix it for v4.
>
>> Also I think we have 0 users of the kmap atomic interfaces ... so not sure
>> whether it's worth it to implement those.
>
> Well, the docs are correct. kmap_atomic() is required, dma-buf.c calls
> the function pointer without checking it exists beforehand ...
Frankly with the pletoria of dummy kmap functions that just return
NULL; it might be better to move that into core dma-buf code and make
it optional for real. Since it's indeed very surprising.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
next prev parent reply other threads:[~2018-05-29 12:31 UTC|newest]
Thread overview: 40+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-05-25 14:08 [PATCH v3] Add udmabuf misc device Gerd Hoffmann
2018-05-25 14:08 ` Gerd Hoffmann
2018-05-25 14:08 ` Gerd Hoffmann
2018-05-25 14:08 ` kraxel
2018-05-29 8:23 ` Daniel Vetter
2018-05-29 8:23 ` Daniel Vetter
2018-05-29 8:23 ` Daniel Vetter
2018-05-29 8:23 ` daniel
2018-05-29 8:44 ` Daniel Vetter
2018-05-29 8:44 ` Daniel Vetter
2018-05-29 8:44 ` Daniel Vetter
2018-05-29 8:44 ` daniel
2018-05-29 10:48 ` Gerd Hoffmann
2018-05-29 10:48 ` Gerd Hoffmann
2018-05-29 10:48 ` Gerd Hoffmann
2018-05-29 10:48 ` kraxel
2018-05-29 12:29 ` [Linaro-mm-sig] " Daniel Vetter
2018-05-29 12:29 ` Daniel Vetter
2018-05-29 12:29 ` Daniel Vetter
2018-05-29 12:29 ` daniel.vetter
2018-05-29 8:48 ` Gerd Hoffmann
2018-05-29 8:48 ` Gerd Hoffmann
2018-05-29 8:48 ` Gerd Hoffmann
2018-05-29 8:48 ` kraxel
2018-05-29 12:30 ` Daniel Vetter [this message]
2018-05-29 12:30 ` Daniel Vetter
2018-05-29 12:30 ` Daniel Vetter
2018-05-29 12:30 ` daniel
2018-05-29 9:16 ` Oleksandr Andrushchenko
2018-05-29 9:16 ` Oleksandr Andrushchenko
2018-05-29 9:16 ` Oleksandr Andrushchenko
2018-05-29 9:16 ` andr2000
2018-05-29 10:50 ` Gerd Hoffmann
2018-05-29 10:50 ` Gerd Hoffmann
2018-05-29 10:50 ` Gerd Hoffmann
2018-05-29 10:50 ` kraxel
2018-05-29 11:07 ` Oleksandr Andrushchenko
2018-05-29 11:07 ` Oleksandr Andrushchenko
2018-05-29 11:07 ` Oleksandr Andrushchenko
2018-05-29 11:07 ` andr2000
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='CAKMK7uGnxNwOUoY=kqG4hW4AZd8Fouh9GRq8Na-fRQUUvF9fZw@mail.gmail.com' \
--to=daniel@ffwll.ch \
--cc=airlied@linux.ie \
--cc=dri-devel@lists.freedesktop.org \
--cc=kraxel@redhat.com \
--cc=linaro-mm-sig@lists.linaro.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-kselftest@vger.kernel.org \
--cc=linux-media@vger.kernel.org \
--cc=shuah@kernel.org \
--cc=sumit.semwal@linaro.org \
--cc=tomeu.vizoso@collabora.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.