On 9/21/20 2:55 PM, Jann Horn wrote:
On Mon, Sep 21, 2020 at 11:20 PM Peter Xu <peterx@xxxxxxxxxx> wrote:
...
I dislike the whole pin_user_pages() concept because (as far as I understand) it fundamentally tries to fix a problem in the subset of cases that are more likely to occur in practice (long-term pins overlapping with things like writeback), and ignores the rarer cases ("short-term" GUP).
Well, no, that's not really fair. pin_user_pages() provides a key prerequisite to fixing *all* of the bugs in that area, not just a subset. The 5 cases in Documentation/core-api/pin_user_pages.rst cover this pretty well. Or if they don't, let me know and I'll have another pass at it. The case for a "pin count" that is (logically) separate from a page->_refcount is real, and it fixes real problems. An elevated refcount can be caused by a lot of things, but it can normally be waited for and/or retried. The FOLL_PIN pages cannot. Of course, a valid remaining criticism of the situation is, "why not just *always* mark any of these pages as "dma-pinned"? In other words, why even have a separate gup/pup API? And in fact, perhaps eventually we'll just get rid of the get_user_pages*() side of it. But the pin count will need to remain, in order to discern between DMA pins and temporary refcount boosts. thanks, -- John Hubbard NVIDIA