On Fri, Sep 09, 2022 at 06:08:32AM -0600, Alex Williamson wrote: > On Fri, 9 Sep 2022 07:53:17 -0400 > Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx> wrote: > > > On Fri, Sep 9, 2022 at 6:52 AM Alex Williamson > > <alex.williamson@xxxxxxxxxx> wrote: > > > > > > VFIO fix for v6.0-rc5 > > > > > > - Fix zero page refcount leak (Alex Williamson) > > > > Ugh. This is disgusting. > > > > Don't get me wrong - I've pulled this, but I think there's some deeper > > problem that made this patch required. The basic issue is that VFIO is using the reserved page as an indication that pin_user_pages_remote() didn't process the page and instead VFIO's bad follow_pfn() path was used. It really shouldn't be working like this, which path was used to obtain the PFN should be recorded independently. VFIO just doesn't have a datastructure to accommodate it. Aside from that, most longterm users don't ever even see the zero page because that typically creates that GUP incoherence we've talked about alot. The ugly FOLL_FORCE prevents it from happening. VFIO doesn't do this either so it suffers the incoherence bug along with exposure to the zero pages :( David will fix the FOLL_FORCE issue and this will almost go away. We had a long thread about this and discussed fixing it by simply setting FOLL_FORCE as other pin_user_page users do. This reached an interesting point that I'm curious on your view about ABI stability. It is related to mlock accounting. When vfio pins pages it accounts for them in mlock. Alex says there are users that set a mlock limit. VFIO has this historical bug where it doesn't account for every page it is asked to pin in mlock, eg it ignores the zero page. So, if we change how GUP or VFIO does page pinning in a way that increases its mlock charge is this an ABI break? On one hand users with an very strict limit may find their environment requires a limit increase after a kernel upgrade. So it fails the 'everything keeps working after kernel upgrade' test. On the other hand, treating these sorts of limits as ABI means that a lot of internal stuff is suddenly ABI - eg with a memory cgroup is increasing the charge on GFP_KERNEL_ACCOUNT also ABI breaking? That seems unworkable. Thanks, Jason
