On 8/10/21 2:29 AM, Oscar Salvador wrote: > On 2021-08-09 20:48, Mike Kravetz wrote: >> Code in prep_compound_gigantic_page waits for a rcu grace period if it >> notices a temporarily inflated ref count on a tail page. This was due >> to the identified potential race with speculative page cache references >> which could only last for a rcu grace period. This is overly complicated >> as this situation is VERY unlikely to ever happen. Instead, just quickly >> return an error. >> Also, only print a warning in prep_compound_gigantic_page instead of >> multiple callers. > > The above makes sense to me. Thanks for taking a look! > My only question would be: gather_bootmem_prealloc() is an __init function. > Can we have speculative refcounts due to e.g: pagecache at that early stage? > I think we cannot, but I am not really sure. I had the same thought when adding that code. We cannot have a speculative refcount this early after boot. However, further thinking below... > > We might be able to remove that else() in case we cannot have such scenarios. > Instead of removing the else, I think we should put a BUG_ON() just to make sure the condition never happens in the future. Otherwise, we would just abandon the gigantic page and leave memory (1GB or so) unavailable. Even if it were possible to have speculative references this early in the boot process, the likelihood of it happening here is still VERY small. So, I would not expect a BUG_ON() to ever be hit in development or testing. Rather, with our luck it would show up in some production environment. Since handling the race in this routine is so simple, I chose to just add the code to handle it. -- Mike Kravetz
