On 08.11.22 05:45, Tomasz Figa wrote:
Hi David,
Hi Tomasz,
thanks for looking into this!
On Tue, Nov 8, 2022 at 1:19 AM David Hildenbrand <david@xxxxxxxxxx> wrote:
FOLL_FORCE is really only for debugger access. According to commit
707947247e95 ("media: videobuf2-vmalloc: get_userptr: buffers are always
writable"), the pinned pages are always writable.
As reference, the cover letter of the series:
https://lkml.kernel.org/r/20221107161740.144456-1-david@xxxxxxxxxx
Actually that patch is only a workaround to temporarily disable
support for read-only pages as they seemed to suffer from some
corruption issues in the retrieved user pages. We expect to support
read-only pages as hardware input after. That said, FOLL_FORCE doesn't
sound like the right thing even in that case, but I don't know the
background behind it being added here in the first place. +Hans
Verkuil +Marek Szyprowski do you happen to remember anything about it?
Maybe I mis-interpreted 707947247e95; re-reading it again, I am not
quite sure what the actual problem is and how it relates to GUP
FOLL_WRITE handling. FOLL_FORCE already was in place before 707947247e95
and should be removed.
What I understood is "Just always call vb2_create_framevec() with
FOLL_WRITE to always allow writing to the buffers.".
If the pinned page is never written to via the obtained GUP reference:
* FOLL_WRITE should not be set
* FOLL_FORCE should not be set
* We should not dirty the page when unpinning
If the pinned page may be written to via the obtained GUP reference:
* FOLL_WRITE should be set
* FOLL_FORCE should not be set
* We should dirty the page when unpinning
If the function is called for both, we should think about doing it
conditional based on a "write" variable, like pre-707947247e95 did.
@Hans, any insight?
--
Thanks,
David / dhildenb
