On Fri, Mar 20, 2020 at 6:17 AM John Hubbard <jhubbard@xxxxxxxxxx> wrote: > > On 3/17/20 4:47 AM, Pingfan Liu wrote: > > FOLL_LONGTERM is a special case of FOLL_PIN. It suggests a pin which is > > going to be given to hardware and can't move. It would truncate CMA > > permanently and should be excluded. > > > > In gup slow path, slow path, where > > > s/slow path, slow path/slow path/ Yeah. [...] > > > > /* > > + * Huge page's subpages have the same migrate type due to either > > + * allocation from a free_list[] or alloc_contig_range() with > > + * param MIGRATE_MOVABLE. So it is enough to check on a subpage. > > + */ > > Urggh, this comment is fine in the commit description, but at this location in the > code it is completely incomprehensible! Instead of an extremely far-removed tidbit about > interactions between CMA and huge pages, this comment should be explaining why we bail > out early in the specific case of FOLL_PIN + FOLL_LONGTERM. And we don't bail out for > FOLL_GET + FOLL_LONGTERM... > > > I'm expect it is something like: > > /* > * We can't do FOLL_LONGTERM + FOLL_PIN with CMA in the gup fast > * path, so fail and let the caller fall back to the slow path. > */ > > > ...approximately. Right? Yes, right. And I think it is better to drop "We". Thanks, Pingfan
