Re: [PATCH RFC 16/19] mm/frame-vector: remove FOLL_FORCE usage

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux