On Sun, May 28, 2023 at 04:49:52PM -0700, John Hubbard wrote: > On 5/26/23 20:18, Matthew Wilcox wrote: > > On Fri, May 26, 2023 at 07:11:05PM -0700, John Hubbard wrote: > > > > So any user with 1024 processes can fragment physical memory? :/ > > > > > > > > Sorry, I'd like to minimize the usage of folio_maybe_dma_pinned(). > > > > > > I was actually thinking that we should minimize any more cases of > > > fragile mapcount and refcount comparison, which then leads to > > > Matthew's approach here! > > > > I was wondering if we shouldn't make folio_maybe_dma_pinned() a little > > more accurate. eg: > > > > if (folio_test_large(folio)) > > return atomic_read(&folio->_pincount) > 0; > > return (unsigned)(folio_ref_count(folio) - folio_mapcount(folio)) >= > > GUP_PIN_COUNTING_BIAS; > > I'm trying to figure out what might be wrong with that, but it seems > OK. We must have talked about this earlier, but I recall vaguely that > there was not a lot of concern about the case of a page being mapped > > 1024 times. Because pinned or not, it's likely to be effectively > locked into memory due to LRU effects. As mentioned here, too. That was my point of view, but David convinced me that a hostile process can effectively lock its own memory into place. > Anyway, sure. > > A detail: > > The unsigned cast, I'm not sure that helps or solves anything, right? > That is, other than bugs, is it possible to get refcount < mapcount? > > And if it's only due to bugs, then the casting, again, isn't likely to > going to mitigate the fallout from whatever mess the bug caused. I wasn't thinking too hard about the cast. If the caller has the folio lock, I don't think it's possible for refcount < mapcount. This caller has a refcount, but doesn't hold the lock, so it is possible for them to read mapcount first, then have both mapcount and refcount decremented and see refcount < mapcount. I don't think it matters too much. We don't hold the folio lock, so it might transition from pinned to unpinned as much as a refcount might be decremented or a mapcount incremented. What's important is that a hostile process can't prevent memory from being moved indefinitely. David, have I missed something else?
