| Message ID | 20201202052330.474592-2-pasha.tatashin@soleen.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | prohibit pinning pages in ZONE_MOVABLE | expand |
On Wed, Dec 02, 2020 at 12:23:25AM -0500, Pavel Tatashin wrote: > There is no need to check_dax_vmas() and run through the npage loop of > pinned pages if FS_DAX is not enabled. > > Add a stub check_dax_vmas() function for no-FS_DAX case. This looks like a good idea. > > Signed-off-by: Pavel Tatashin <pasha.tatashin@soleen.com> > --- > mm/gup.c | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/mm/gup.c b/mm/gup.c > index 98eb8e6d2609..cdb8b9eeb016 100644 > --- a/mm/gup.c > +++ b/mm/gup.c > @@ -1568,6 +1568,7 @@ struct page *get_dump_page(unsigned long addr) > #endif /* CONFIG_ELF_CORE */ > > #if defined(CONFIG_FS_DAX) || defined (CONFIG_CMA) In addition, I think it would make a lot of sense to clean up this config as well like this: 08:20:10 > git di diff --git a/mm/gup.c b/mm/gup.c index 102877ed77a4..92cfda220aeb 100644 --- a/mm/gup.c +++ b/mm/gup.c @@ -1567,7 +1567,7 @@ struct page *get_dump_page(unsigned long addr) } #endif /* CONFIG_ELF_CORE */ -#if defined(CONFIG_FS_DAX) || defined (CONFIG_CMA) +#ifdef CONFIG_FS_DAX static bool check_dax_vmas(struct vm_area_struct **vmas, long nr_pages) { long i; @@ -1586,6 +1586,12 @@ static bool check_dax_vmas(struct vm_area_struct **vmas, long nr_pages) } return false; } +#else +static bool check_dax_vmas(struct vm_area_struct **vmas, long nr_pages) +{ + return false; +} +#endif /* CONFIG_FS_DAX */ #ifdef CONFIG_CMA static long check_and_migrate_cma_pages(struct mm_struct *mm, @@ -1691,6 +1697,7 @@ static long check_and_migrate_cma_pages(struct mm_struct *mm, } #endif /* CONFIG_CMA */ +#if defined(CONFIG_FS_DAX) || defined (CONFIG_CMA) /* * __gup_longterm_locked() is a wrapper for __get_user_pages_locked which * allows us to process the FOLL_LONGTERM flag. That makes it more clear what is going on with __gup_longterm_locked() and places both CMA and FS_DAX code within their own blocks. Ira > +#ifdef CONFIG_FS_DAX > static bool check_dax_vmas(struct vm_area_struct **vmas, long nr_pages) > { > long i; > @@ -1586,6 +1587,12 @@ static bool check_dax_vmas(struct vm_area_struct **vmas, long nr_pages) > } > return false; > } > +#else > +static bool check_dax_vmas(struct vm_area_struct **vmas, long nr_pages) > +{ > + return false; > +} > +#endif > > #ifdef CONFIG_CMA > static long check_and_migrate_cma_pages(struct mm_struct *mm, > -- > 2.25.1 > >
On Wed, Dec 02, 2020 at 12:23:25AM -0500, Pavel Tatashin wrote: > There is no need to check_dax_vmas() and run through the npage loop of > pinned pages if FS_DAX is not enabled. > > Add a stub check_dax_vmas() function for no-FS_DAX case. > > Signed-off-by: Pavel Tatashin <pasha.tatashin@soleen.com> > --- > mm/gup.c | 7 +++++++ > 1 file changed, 7 insertions(+) I have a patch to delete check_dax_vmas that is just waiting on me to figure out how to test with dax. That makes all this ifdefery much simpler Jason
On Wed, Dec 2, 2020 at 11:22 AM Ira Weiny <ira.weiny@intel.com> wrote: > > On Wed, Dec 02, 2020 at 12:23:25AM -0500, Pavel Tatashin wrote: > > There is no need to check_dax_vmas() and run through the npage loop of > > pinned pages if FS_DAX is not enabled. > > > > Add a stub check_dax_vmas() function for no-FS_DAX case. > > This looks like a good idea. > > > > > Signed-off-by: Pavel Tatashin <pasha.tatashin@soleen.com> > > --- > > mm/gup.c | 7 +++++++ > > 1 file changed, 7 insertions(+) > > > > diff --git a/mm/gup.c b/mm/gup.c > > index 98eb8e6d2609..cdb8b9eeb016 100644 > > --- a/mm/gup.c > > +++ b/mm/gup.c > > @@ -1568,6 +1568,7 @@ struct page *get_dump_page(unsigned long addr) > > #endif /* CONFIG_ELF_CORE */ > > > > #if defined(CONFIG_FS_DAX) || defined (CONFIG_CMA) > > In addition, I think it would make a lot of sense to clean up this config as > well like this: > > 08:20:10 > git di > diff --git a/mm/gup.c b/mm/gup.c > index 102877ed77a4..92cfda220aeb 100644 > --- a/mm/gup.c > +++ b/mm/gup.c > @@ -1567,7 +1567,7 @@ struct page *get_dump_page(unsigned long addr) > } > #endif /* CONFIG_ELF_CORE */ > > -#if defined(CONFIG_FS_DAX) || defined (CONFIG_CMA) > +#ifdef CONFIG_FS_DAX > static bool check_dax_vmas(struct vm_area_struct **vmas, long nr_pages) > { > long i; > @@ -1586,6 +1586,12 @@ static bool check_dax_vmas(struct vm_area_struct **vmas, long nr_pages) > } > return false; > } > +#else > +static bool check_dax_vmas(struct vm_area_struct **vmas, long nr_pages) > +{ > + return false; > +} > +#endif /* CONFIG_FS_DAX */ > > #ifdef CONFIG_CMA > static long check_and_migrate_cma_pages(struct mm_struct *mm, > @@ -1691,6 +1697,7 @@ static long check_and_migrate_cma_pages(struct mm_struct *mm, > } > #endif /* CONFIG_CMA */ > > +#if defined(CONFIG_FS_DAX) || defined (CONFIG_CMA) > /* > * __gup_longterm_locked() is a wrapper for __get_user_pages_locked which > * allows us to process the FOLL_LONGTERM flag. > > > That makes it more clear what is going on with __gup_longterm_locked() and > places both CMA and FS_DAX code within their own blocks. Hi Ira, Thank you for your comments. I think the end result of what you are suggesting is the same for this series when __gup_longterm_locked is made common. " #if defined(CONFIG_FS_DAX) || defined (CONFIG_CMA)" mess is removed in that patch. Pasha
On Wed, Dec 2, 2020 at 11:30 AM Jason Gunthorpe <jgg@ziepe.ca> wrote: > > On Wed, Dec 02, 2020 at 12:23:25AM -0500, Pavel Tatashin wrote: > > There is no need to check_dax_vmas() and run through the npage loop of > > pinned pages if FS_DAX is not enabled. > > > > Add a stub check_dax_vmas() function for no-FS_DAX case. > > > > Signed-off-by: Pavel Tatashin <pasha.tatashin@soleen.com> > > --- > > mm/gup.c | 7 +++++++ > > 1 file changed, 7 insertions(+) > > I have a patch to delete check_dax_vmas that is just waiting on me to > figure out how to test with dax. That makes all this ifdefery much > simpler Hi Jason, Yeap, that would be nice. I made this change as a preparation for moving __gup_longterm_locked into common code, so when you send your patch it can remove both versions of check_dax_vmas. Thank you, Pasha > > Jason
On 12/1/20 9:23 PM, Pavel Tatashin wrote: > There is no need to check_dax_vmas() and run through the npage loop of > pinned pages if FS_DAX is not enabled. > > Add a stub check_dax_vmas() function for no-FS_DAX case. > > Signed-off-by: Pavel Tatashin <pasha.tatashin@soleen.com> > --- > mm/gup.c | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/mm/gup.c b/mm/gup.c > index 98eb8e6d2609..cdb8b9eeb016 100644 > --- a/mm/gup.c > +++ b/mm/gup.c > @@ -1568,6 +1568,7 @@ struct page *get_dump_page(unsigned long addr) > #endif /* CONFIG_ELF_CORE */ > > #if defined(CONFIG_FS_DAX) || defined (CONFIG_CMA) > +#ifdef CONFIG_FS_DAX > static bool check_dax_vmas(struct vm_area_struct **vmas, long nr_pages) > { > long i; > @@ -1586,6 +1587,12 @@ static bool check_dax_vmas(struct vm_area_struct **vmas, long nr_pages) > } > return false; > } > +#else > +static bool check_dax_vmas(struct vm_area_struct **vmas, long nr_pages) > +{ > + return false; > +} > +#endif > > #ifdef CONFIG_CMA > static long check_and_migrate_cma_pages(struct mm_struct *mm, > Looks obviously correct, and the follow-up simplication is very nice. Reviewed-by: John Hubbard <jhubbard@nvidia.com> thanks,
> Looks obviously correct, and the follow-up simplication is very nice. > > Reviewed-by: John Hubbard <jhubbard@nvidia.com> Thank you, Pasha
diff --git a/mm/gup.c b/mm/gup.c index 98eb8e6d2609..cdb8b9eeb016 100644 --- a/mm/gup.c +++ b/mm/gup.c @@ -1568,6 +1568,7 @@ struct page *get_dump_page(unsigned long addr) #endif /* CONFIG_ELF_CORE */ #if defined(CONFIG_FS_DAX) || defined (CONFIG_CMA) +#ifdef CONFIG_FS_DAX static bool check_dax_vmas(struct vm_area_struct **vmas, long nr_pages) { long i; @@ -1586,6 +1587,12 @@ static bool check_dax_vmas(struct vm_area_struct **vmas, long nr_pages) } return false; } +#else +static bool check_dax_vmas(struct vm_area_struct **vmas, long nr_pages) +{ + return false; +} +#endif #ifdef CONFIG_CMA static long check_and_migrate_cma_pages(struct mm_struct *mm,
There is no need to check_dax_vmas() and run through the npage loop of pinned pages if FS_DAX is not enabled. Add a stub check_dax_vmas() function for no-FS_DAX case. Signed-off-by: Pavel Tatashin <pasha.tatashin@soleen.com> --- mm/gup.c | 7 +++++++ 1 file changed, 7 insertions(+)