On Thu 13-10-16 01:20:10, Lorenzo Stoakes wrote: > This patch series adjusts functions in the get_user_pages* family such that > desired FOLL_* flags are passed as an argument rather than implied by flags. > > The purpose of this change is to make the use of FOLL_FORCE explicit so it is > easier to grep for and clearer to callers that this flag is being used. The use > of FOLL_FORCE is an issue as it overrides missing VM_READ/VM_WRITE flags for the > VMA whose pages we are reading from/writing to, which can result in surprising > behaviour. > > The patch series came out of the discussion around commit 38e0885, which > addressed a BUG_ON() being triggered when a page was faulted in with PROT_NONE > set but having been overridden by FOLL_FORCE. do_numa_page() was run on the > assumption the page _must_ be one marked for NUMA node migration as an actual > PROT_NONE page would have been dealt with prior to this code path, however > FOLL_FORCE introduced a situation where this assumption did not hold. > > See https://marc.info/?l=linux-mm&m=147585445805166 for the patch proposal. I like this cleanup. Tracking FOLL_FORCE users was always a nightmare and the flag behavior is really subtle so we should better be explicit about it. I haven't gone through each patch separately but rather applied the whole series and checked the resulting diff. This all seems OK to me and feel free to add Acked-by: Michal Hocko <mhocko@xxxxxxxx> I am wondering whether we can go further. E.g. it is not really clear to me whether we need an explicit FOLL_REMOTE when we can in fact check mm != current->mm and imply that. Maybe there are some contexts which wouldn't work, I haven't checked. Then I am also wondering about FOLL_TOUCH behavior. __get_user_pages_unlocked has only few callers which used to be get_user_pages_unlocked before 1e9877902dc7e ("mm/gup: Introduce get_user_pages_remote()"). To me a dropped FOLL_TOUCH seems unintentional. Now that get_user_pages_unlocked has gup_flags argument I guess we might want to get rid of the __g-u-p-u version altogether, no? __get_user_pages is quite low level and imho shouldn't be exported. It's only user - kvm - should rather pull those two functions to gup instead and export them. There is nothing really KVM specific in them. I also cannot say I would be entirely thrilled about get_user_pages_locked, we only have one user which can simply do lock g-u-p unlock AFAICS. I guess there is more work in that area and I do not want to impose all that work on you, but I couldn't resist once I saw you playing in that area ;) Definitely a good start! -- Michal Hocko SUSE Labs -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
