Message ID | 20200609120533.25867-1-joro@8bytes.org (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | mm: Move p?d_alloc_track to separate header file | expand |
On Tue, Jun 09, 2020 at 02:05:33PM +0200, Joerg Roedel wrote: > From: Joerg Roedel <jroedel@suse.de> > > The functions are only used in two source files, so there is no need > for them to be in the global <linux/mm.h> header. Move them to the new > <linux/pgalloc-track.h> header and include it only where needed. > > Signed-off-by: Joerg Roedel <jroedel@suse.de> Acked-by: Mike Rapoport <rppt@linux.ibm.com> > --- > include/linux/mm.h | 45 ------------------------------- > include/linux/pgalloc-track.h | 51 +++++++++++++++++++++++++++++++++++ > lib/ioremap.c | 1 + > mm/vmalloc.c | 1 + > 4 files changed, 53 insertions(+), 45 deletions(-) > create mode 100644 include/linux/pgalloc-track.h > > diff --git a/include/linux/mm.h b/include/linux/mm.h > index 9d6042178ca7..22d8b2a2c9bc 100644 > --- a/include/linux/mm.h > +++ b/include/linux/mm.h > @@ -2092,51 +2092,11 @@ static inline pud_t *pud_alloc(struct mm_struct *mm, p4d_t *p4d, > NULL : pud_offset(p4d, address); > } > > -static inline p4d_t *p4d_alloc_track(struct mm_struct *mm, pgd_t *pgd, > - unsigned long address, > - pgtbl_mod_mask *mod_mask) > - > -{ > - if (unlikely(pgd_none(*pgd))) { > - if (__p4d_alloc(mm, pgd, address)) > - return NULL; > - *mod_mask |= PGTBL_PGD_MODIFIED; > - } > - > - return p4d_offset(pgd, address); > -} > - > -static inline pud_t *pud_alloc_track(struct mm_struct *mm, p4d_t *p4d, > - unsigned long address, > - pgtbl_mod_mask *mod_mask) > -{ > - if (unlikely(p4d_none(*p4d))) { > - if (__pud_alloc(mm, p4d, address)) > - return NULL; > - *mod_mask |= PGTBL_P4D_MODIFIED; > - } > - > - return pud_offset(p4d, address); > -} > - > static inline pmd_t *pmd_alloc(struct mm_struct *mm, pud_t *pud, unsigned long address) > { > return (unlikely(pud_none(*pud)) && __pmd_alloc(mm, pud, address))? > NULL: pmd_offset(pud, address); > } > - > -static inline pmd_t *pmd_alloc_track(struct mm_struct *mm, pud_t *pud, > - unsigned long address, > - pgtbl_mod_mask *mod_mask) > -{ > - if (unlikely(pud_none(*pud))) { > - if (__pmd_alloc(mm, pud, address)) > - return NULL; > - *mod_mask |= PGTBL_PUD_MODIFIED; > - } > - > - return pmd_offset(pud, address); > -} > #endif /* CONFIG_MMU */ > > #if USE_SPLIT_PTE_PTLOCKS > @@ -2252,11 +2212,6 @@ static inline void pgtable_pte_page_dtor(struct page *page) > ((unlikely(pmd_none(*(pmd))) && __pte_alloc_kernel(pmd))? \ > NULL: pte_offset_kernel(pmd, address)) > > -#define pte_alloc_kernel_track(pmd, address, mask) \ > - ((unlikely(pmd_none(*(pmd))) && \ > - (__pte_alloc_kernel(pmd) || ({*(mask)|=PGTBL_PMD_MODIFIED;0;})))?\ > - NULL: pte_offset_kernel(pmd, address)) > - > #if USE_SPLIT_PMD_PTLOCKS > > static struct page *pmd_to_page(pmd_t *pmd) > diff --git a/include/linux/pgalloc-track.h b/include/linux/pgalloc-track.h > new file mode 100644 > index 000000000000..1dcc865029a2 > --- /dev/null > +++ b/include/linux/pgalloc-track.h > @@ -0,0 +1,51 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > +#ifndef _LINUX_PGALLLC_TRACK_H > +#define _LINUX_PGALLLC_TRACK_H > + > +#if defined(CONFIG_MMU) > +static inline p4d_t *p4d_alloc_track(struct mm_struct *mm, pgd_t *pgd, > + unsigned long address, > + pgtbl_mod_mask *mod_mask) > +{ > + if (unlikely(pgd_none(*pgd))) { > + if (__p4d_alloc(mm, pgd, address)) > + return NULL; > + *mod_mask |= PGTBL_PGD_MODIFIED; > + } > + > + return p4d_offset(pgd, address); > +} > + > +static inline pud_t *pud_alloc_track(struct mm_struct *mm, p4d_t *p4d, > + unsigned long address, > + pgtbl_mod_mask *mod_mask) > +{ > + if (unlikely(p4d_none(*p4d))) { > + if (__pud_alloc(mm, p4d, address)) > + return NULL; > + *mod_mask |= PGTBL_P4D_MODIFIED; > + } > + > + return pud_offset(p4d, address); > +} > + > +static inline pmd_t *pmd_alloc_track(struct mm_struct *mm, pud_t *pud, > + unsigned long address, > + pgtbl_mod_mask *mod_mask) > +{ > + if (unlikely(pud_none(*pud))) { > + if (__pmd_alloc(mm, pud, address)) > + return NULL; > + *mod_mask |= PGTBL_PUD_MODIFIED; > + } > + > + return pmd_offset(pud, address); > +} > +#endif /* CONFIG_MMU */ > + > +#define pte_alloc_kernel_track(pmd, address, mask) \ > + ((unlikely(pmd_none(*(pmd))) && \ > + (__pte_alloc_kernel(pmd) || ({*(mask)|=PGTBL_PMD_MODIFIED;0;})))?\ > + NULL: pte_offset_kernel(pmd, address)) > + > +#endif /* _LINUX_PGALLLC_TRACK_H */ > diff --git a/lib/ioremap.c b/lib/ioremap.c > index ad485f08173b..608fcccd21c8 100644 > --- a/lib/ioremap.c > +++ b/lib/ioremap.c > @@ -11,6 +11,7 @@ > #include <linux/sched.h> > #include <linux/io.h> > #include <linux/export.h> > +#include <linux/pgalloc-track.h> > #include <asm/cacheflush.h> > #include <asm/pgtable.h> > > diff --git a/mm/vmalloc.c b/mm/vmalloc.c > index 3091c2ca60df..edc43f003165 100644 > --- a/mm/vmalloc.c > +++ b/mm/vmalloc.c > @@ -35,6 +35,7 @@ > #include <linux/bitops.h> > #include <linux/rbtree_augmented.h> > #include <linux/overflow.h> > +#include <linux/pgalloc-track.h> > > #include <linux/uaccess.h> > #include <asm/tlbflush.h> > -- > 2.26.2 >
Le 09/06/2020 à 14:05, Joerg Roedel a écrit : > From: Joerg Roedel <jroedel@suse.de> > > The functions are only used in two source files, so there is no need > for them to be in the global <linux/mm.h> header. Move them to the new > <linux/pgalloc-track.h> header and include it only where needed. Do you mean we will now create a new header file for any new couple on functions based on where they are used ? Can you explain why this change is needed or is a plus ? Christophe > > Signed-off-by: Joerg Roedel <jroedel@suse.de> > --- > include/linux/mm.h | 45 ------------------------------- > include/linux/pgalloc-track.h | 51 +++++++++++++++++++++++++++++++++++ > lib/ioremap.c | 1 + > mm/vmalloc.c | 1 + > 4 files changed, 53 insertions(+), 45 deletions(-) > create mode 100644 include/linux/pgalloc-track.h > > diff --git a/include/linux/mm.h b/include/linux/mm.h > index 9d6042178ca7..22d8b2a2c9bc 100644 > --- a/include/linux/mm.h > +++ b/include/linux/mm.h > @@ -2092,51 +2092,11 @@ static inline pud_t *pud_alloc(struct mm_struct *mm, p4d_t *p4d, > NULL : pud_offset(p4d, address); > } > > -static inline p4d_t *p4d_alloc_track(struct mm_struct *mm, pgd_t *pgd, > - unsigned long address, > - pgtbl_mod_mask *mod_mask) > - > -{ > - if (unlikely(pgd_none(*pgd))) { > - if (__p4d_alloc(mm, pgd, address)) > - return NULL; > - *mod_mask |= PGTBL_PGD_MODIFIED; > - } > - > - return p4d_offset(pgd, address); > -} > - > -static inline pud_t *pud_alloc_track(struct mm_struct *mm, p4d_t *p4d, > - unsigned long address, > - pgtbl_mod_mask *mod_mask) > -{ > - if (unlikely(p4d_none(*p4d))) { > - if (__pud_alloc(mm, p4d, address)) > - return NULL; > - *mod_mask |= PGTBL_P4D_MODIFIED; > - } > - > - return pud_offset(p4d, address); > -} > - > static inline pmd_t *pmd_alloc(struct mm_struct *mm, pud_t *pud, unsigned long address) > { > return (unlikely(pud_none(*pud)) && __pmd_alloc(mm, pud, address))? > NULL: pmd_offset(pud, address); > } > - > -static inline pmd_t *pmd_alloc_track(struct mm_struct *mm, pud_t *pud, > - unsigned long address, > - pgtbl_mod_mask *mod_mask) > -{ > - if (unlikely(pud_none(*pud))) { > - if (__pmd_alloc(mm, pud, address)) > - return NULL; > - *mod_mask |= PGTBL_PUD_MODIFIED; > - } > - > - return pmd_offset(pud, address); > -} > #endif /* CONFIG_MMU */ > > #if USE_SPLIT_PTE_PTLOCKS > @@ -2252,11 +2212,6 @@ static inline void pgtable_pte_page_dtor(struct page *page) > ((unlikely(pmd_none(*(pmd))) && __pte_alloc_kernel(pmd))? \ > NULL: pte_offset_kernel(pmd, address)) > > -#define pte_alloc_kernel_track(pmd, address, mask) \ > - ((unlikely(pmd_none(*(pmd))) && \ > - (__pte_alloc_kernel(pmd) || ({*(mask)|=PGTBL_PMD_MODIFIED;0;})))?\ > - NULL: pte_offset_kernel(pmd, address)) > - > #if USE_SPLIT_PMD_PTLOCKS > > static struct page *pmd_to_page(pmd_t *pmd) > diff --git a/include/linux/pgalloc-track.h b/include/linux/pgalloc-track.h > new file mode 100644 > index 000000000000..1dcc865029a2 > --- /dev/null > +++ b/include/linux/pgalloc-track.h > @@ -0,0 +1,51 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > +#ifndef _LINUX_PGALLLC_TRACK_H > +#define _LINUX_PGALLLC_TRACK_H > + > +#if defined(CONFIG_MMU) > +static inline p4d_t *p4d_alloc_track(struct mm_struct *mm, pgd_t *pgd, > + unsigned long address, > + pgtbl_mod_mask *mod_mask) > +{ > + if (unlikely(pgd_none(*pgd))) { > + if (__p4d_alloc(mm, pgd, address)) > + return NULL; > + *mod_mask |= PGTBL_PGD_MODIFIED; > + } > + > + return p4d_offset(pgd, address); > +} > + > +static inline pud_t *pud_alloc_track(struct mm_struct *mm, p4d_t *p4d, > + unsigned long address, > + pgtbl_mod_mask *mod_mask) > +{ > + if (unlikely(p4d_none(*p4d))) { > + if (__pud_alloc(mm, p4d, address)) > + return NULL; > + *mod_mask |= PGTBL_P4D_MODIFIED; > + } > + > + return pud_offset(p4d, address); > +} > + > +static inline pmd_t *pmd_alloc_track(struct mm_struct *mm, pud_t *pud, > + unsigned long address, > + pgtbl_mod_mask *mod_mask) > +{ > + if (unlikely(pud_none(*pud))) { > + if (__pmd_alloc(mm, pud, address)) > + return NULL; > + *mod_mask |= PGTBL_PUD_MODIFIED; > + } > + > + return pmd_offset(pud, address); > +} > +#endif /* CONFIG_MMU */ > + > +#define pte_alloc_kernel_track(pmd, address, mask) \ > + ((unlikely(pmd_none(*(pmd))) && \ > + (__pte_alloc_kernel(pmd) || ({*(mask)|=PGTBL_PMD_MODIFIED;0;})))?\ > + NULL: pte_offset_kernel(pmd, address)) > + > +#endif /* _LINUX_PGALLLC_TRACK_H */ > diff --git a/lib/ioremap.c b/lib/ioremap.c > index ad485f08173b..608fcccd21c8 100644 > --- a/lib/ioremap.c > +++ b/lib/ioremap.c > @@ -11,6 +11,7 @@ > #include <linux/sched.h> > #include <linux/io.h> > #include <linux/export.h> > +#include <linux/pgalloc-track.h> > #include <asm/cacheflush.h> > #include <asm/pgtable.h> > > diff --git a/mm/vmalloc.c b/mm/vmalloc.c > index 3091c2ca60df..edc43f003165 100644 > --- a/mm/vmalloc.c > +++ b/mm/vmalloc.c > @@ -35,6 +35,7 @@ > #include <linux/bitops.h> > #include <linux/rbtree_augmented.h> > #include <linux/overflow.h> > +#include <linux/pgalloc-track.h> > > #include <linux/uaccess.h> > #include <asm/tlbflush.h> >
Hi Christophe, On Tue, 9 Jun 2020 17:24:14 +0200 Christophe Leroy <christophe.leroy@csgroup.eu> wrote: > > Le 09/06/2020 à 14:05, Joerg Roedel a écrit : > > From: Joerg Roedel <jroedel@suse.de> > > > > The functions are only used in two source files, so there is no need > > for them to be in the global <linux/mm.h> header. Move them to the new > > <linux/pgalloc-track.h> header and include it only where needed. > > Do you mean we will now create a new header file for any new couple on > functions based on where they are used ? > > Can you explain why this change is needed or is a plus ? Well at a minimum, it means 45 lines less to be parsed every time the linux/mm is included (in at last count, 1996 places some of which are include files included by other files). And, as someone who does a lot of builds every day, I am in favour of that :-)
On Tue, 9 Jun 2020 14:05:33 +0200 Joerg Roedel <joro@8bytes.org> wrote: > From: Joerg Roedel <jroedel@suse.de> > > The functions are only used in two source files, so there is no need > for them to be in the global <linux/mm.h> header. Move them to the new > <linux/pgalloc-track.h> header and include it only where needed. > > ... > > new file mode 100644 > index 000000000000..1dcc865029a2 > --- /dev/null > +++ b/include/linux/pgalloc-track.h > @@ -0,0 +1,51 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > +#ifndef _LINUX_PGALLLC_TRACK_H > +#define _LINUX_PGALLLC_TRACK_H hm, no #includes. I guess this is OK, given the limited use. But it does make one wonder whether ioremap.c should be moved from lib/ to mm/ and this file should be moved from include/linux/ to mm/. Oh well.
Hi Joerg, Sorry for the late reply. On Tue, 9 Jun 2020 14:05:33 +0200 Joerg Roedel <joro@8bytes.org> wrote: > > diff --git a/include/linux/pgalloc-track.h b/include/linux/pgalloc-track.h > new file mode 100644 > index 000000000000..1dcc865029a2 > --- /dev/null > +++ b/include/linux/pgalloc-track.h > @@ -0,0 +1,51 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > +#ifndef _LINUX_PGALLLC_TRACK_H > +#define _LINUX_PGALLLC_TRACK_H > + Maybe this could have a comment that it should always be included after mm.h or we could add enough to make it standalone (even just an include of mm.h would probably be enough).
On Wed, Jun 17, 2020 at 06:12:26PM -0700, Andrew Morton wrote: > On Tue, 9 Jun 2020 14:05:33 +0200 Joerg Roedel <joro@8bytes.org> wrote: > > > From: Joerg Roedel <jroedel@suse.de> > > > > The functions are only used in two source files, so there is no need > > for them to be in the global <linux/mm.h> header. Move them to the new > > <linux/pgalloc-track.h> header and include it only where needed. > > > > ... > > > > new file mode 100644 > > index 000000000000..1dcc865029a2 > > --- /dev/null > > +++ b/include/linux/pgalloc-track.h > > @@ -0,0 +1,51 @@ > > +/* SPDX-License-Identifier: GPL-2.0 */ > > +#ifndef _LINUX_PGALLLC_TRACK_H > > +#define _LINUX_PGALLLC_TRACK_H > > hm, no #includes. I guess this is OK, given the limited use. > > But it does make one wonder whether ioremap.c should be moved from lib/ > to mm/ and this file should be moved from include/linux/ to mm/. It makes sense, but I am anyway planning consolidation of pgalloc.h, so most probably pgalloc-track will not survive until 5.9-rc1 :) If you think that it worth moving ioremap.c to mm/ regardless of chrun, I can send a patch for that. > Oh well.
diff --git a/include/linux/mm.h b/include/linux/mm.h index 9d6042178ca7..22d8b2a2c9bc 100644 --- a/include/linux/mm.h +++ b/include/linux/mm.h @@ -2092,51 +2092,11 @@ static inline pud_t *pud_alloc(struct mm_struct *mm, p4d_t *p4d, NULL : pud_offset(p4d, address); } -static inline p4d_t *p4d_alloc_track(struct mm_struct *mm, pgd_t *pgd, - unsigned long address, - pgtbl_mod_mask *mod_mask) - -{ - if (unlikely(pgd_none(*pgd))) { - if (__p4d_alloc(mm, pgd, address)) - return NULL; - *mod_mask |= PGTBL_PGD_MODIFIED; - } - - return p4d_offset(pgd, address); -} - -static inline pud_t *pud_alloc_track(struct mm_struct *mm, p4d_t *p4d, - unsigned long address, - pgtbl_mod_mask *mod_mask) -{ - if (unlikely(p4d_none(*p4d))) { - if (__pud_alloc(mm, p4d, address)) - return NULL; - *mod_mask |= PGTBL_P4D_MODIFIED; - } - - return pud_offset(p4d, address); -} - static inline pmd_t *pmd_alloc(struct mm_struct *mm, pud_t *pud, unsigned long address) { return (unlikely(pud_none(*pud)) && __pmd_alloc(mm, pud, address))? NULL: pmd_offset(pud, address); } - -static inline pmd_t *pmd_alloc_track(struct mm_struct *mm, pud_t *pud, - unsigned long address, - pgtbl_mod_mask *mod_mask) -{ - if (unlikely(pud_none(*pud))) { - if (__pmd_alloc(mm, pud, address)) - return NULL; - *mod_mask |= PGTBL_PUD_MODIFIED; - } - - return pmd_offset(pud, address); -} #endif /* CONFIG_MMU */ #if USE_SPLIT_PTE_PTLOCKS @@ -2252,11 +2212,6 @@ static inline void pgtable_pte_page_dtor(struct page *page) ((unlikely(pmd_none(*(pmd))) && __pte_alloc_kernel(pmd))? \ NULL: pte_offset_kernel(pmd, address)) -#define pte_alloc_kernel_track(pmd, address, mask) \ - ((unlikely(pmd_none(*(pmd))) && \ - (__pte_alloc_kernel(pmd) || ({*(mask)|=PGTBL_PMD_MODIFIED;0;})))?\ - NULL: pte_offset_kernel(pmd, address)) - #if USE_SPLIT_PMD_PTLOCKS static struct page *pmd_to_page(pmd_t *pmd) diff --git a/include/linux/pgalloc-track.h b/include/linux/pgalloc-track.h new file mode 100644 index 000000000000..1dcc865029a2 --- /dev/null +++ b/include/linux/pgalloc-track.h @@ -0,0 +1,51 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +#ifndef _LINUX_PGALLLC_TRACK_H +#define _LINUX_PGALLLC_TRACK_H + +#if defined(CONFIG_MMU) +static inline p4d_t *p4d_alloc_track(struct mm_struct *mm, pgd_t *pgd, + unsigned long address, + pgtbl_mod_mask *mod_mask) +{ + if (unlikely(pgd_none(*pgd))) { + if (__p4d_alloc(mm, pgd, address)) + return NULL; + *mod_mask |= PGTBL_PGD_MODIFIED; + } + + return p4d_offset(pgd, address); +} + +static inline pud_t *pud_alloc_track(struct mm_struct *mm, p4d_t *p4d, + unsigned long address, + pgtbl_mod_mask *mod_mask) +{ + if (unlikely(p4d_none(*p4d))) { + if (__pud_alloc(mm, p4d, address)) + return NULL; + *mod_mask |= PGTBL_P4D_MODIFIED; + } + + return pud_offset(p4d, address); +} + +static inline pmd_t *pmd_alloc_track(struct mm_struct *mm, pud_t *pud, + unsigned long address, + pgtbl_mod_mask *mod_mask) +{ + if (unlikely(pud_none(*pud))) { + if (__pmd_alloc(mm, pud, address)) + return NULL; + *mod_mask |= PGTBL_PUD_MODIFIED; + } + + return pmd_offset(pud, address); +} +#endif /* CONFIG_MMU */ + +#define pte_alloc_kernel_track(pmd, address, mask) \ + ((unlikely(pmd_none(*(pmd))) && \ + (__pte_alloc_kernel(pmd) || ({*(mask)|=PGTBL_PMD_MODIFIED;0;})))?\ + NULL: pte_offset_kernel(pmd, address)) + +#endif /* _LINUX_PGALLLC_TRACK_H */ diff --git a/lib/ioremap.c b/lib/ioremap.c index ad485f08173b..608fcccd21c8 100644 --- a/lib/ioremap.c +++ b/lib/ioremap.c @@ -11,6 +11,7 @@ #include <linux/sched.h> #include <linux/io.h> #include <linux/export.h> +#include <linux/pgalloc-track.h> #include <asm/cacheflush.h> #include <asm/pgtable.h> diff --git a/mm/vmalloc.c b/mm/vmalloc.c index 3091c2ca60df..edc43f003165 100644 --- a/mm/vmalloc.c +++ b/mm/vmalloc.c @@ -35,6 +35,7 @@ #include <linux/bitops.h> #include <linux/rbtree_augmented.h> #include <linux/overflow.h> +#include <linux/pgalloc-track.h> #include <linux/uaccess.h> #include <asm/tlbflush.h>