Hi Khandual,
> > long freed);
> > bool isolate_huge_page(struct page *page, struct list_head *list);
> > void putback_active_hugepage(struct page *page);
> > -void free_huge_page(struct page *page);
> > +void huge_page_dtor(struct page *page);
> > void hugetlb_fix_reserve_counts(struct inode *inode);
> > extern struct mutex *hugetlb_fault_mutex_table;
> > u32 hugetlb_fault_mutex_hash(struct hstate *h, struct mm_struct *mm,
> > diff --git a/include/linux/mm.h b/include/linux/mm.h
> > index 065d99d..adfa906 100644
> > --- a/include/linux/mm.h
> > +++ b/include/linux/mm.h
> > @@ -616,7 +616,7 @@ void split_page(struct page *page, unsigned int order);
> > * prototype for that function and accessor functions.
> > * These are _only_ valid on the head of a compound page.
> > */
> > -typedef void compound_page_dtor(struct page *);
> > +typedef void compound_page_dtor_t(struct page *);
>
> Why changing this ? I understand _t kind of specifies it more
> like a type def but this patch is just to rename the compound
> page destructor functions. Not sure we should change datatype
> here as well in this patch.
>
It is because of name conflict. I think you already get it per below comments.
I will describe it in commit message.
> >
> > /* Keep the enum in sync with compound_page_dtors array in mm/page_alloc.c */
> > enum compound_dtor_id {
> > @@ -630,7 +630,7 @@ enum compound_dtor_id {
> > #endif
> > NR_COMPOUND_DTORS,
> > };
> > -extern compound_page_dtor * const compound_page_dtors[];
> > +extern compound_page_dtor_t * const compound_page_dtors[];
> >
> > static inline void set_compound_page_dtor(struct page *page,
> > enum compound_dtor_id compound_dtor)
> > @@ -639,7 +639,7 @@ static inline void set_compound_page_dtor(struct page *page,
> > page[1].compound_dtor = compound_dtor;
> > }
> >
> > -static inline compound_page_dtor *get_compound_page_dtor(struct page *page)
> > +static inline compound_page_dtor_t *get_compound_page_dtor(struct page *page)
>
> Which is adding these kind of changes to the patch without
> having a corresponding description in the commit message.
>
> > {
> > VM_BUG_ON_PAGE(page[1].compound_dtor >= NR_COMPOUND_DTORS, page);
> > return compound_page_dtors[page[1].compound_dtor];
> > @@ -657,7 +657,7 @@ static inline void set_compound_order(struct page *page, unsigned int order)
> > page[1].compound_order = order;
> > }
> >
> > -void free_compound_page(struct page *page);
> > +void compound_page_dtor(struct page *page);
> >
> > #ifdef CONFIG_MMU
> > /*
> > diff --git a/mm/huge_memory.c b/mm/huge_memory.c
> > index e267488..a01125b 100644
> > --- a/mm/huge_memory.c
> > +++ b/mm/huge_memory.c
> > @@ -2717,7 +2717,7 @@ fail: if (mapping)
> > return ret;
> > }
> >
> > -void free_transhuge_page(struct page *page)
> > +void transhuge_page_dtor(struct page *page)
> > {
> > struct pglist_data *pgdata = NODE_DATA(page_to_nid(page));
> > unsigned long flags;
> > @@ -2728,7 +2728,7 @@ void free_transhuge_page(struct page *page)
> > list_del(page_deferred_list(page));
> > }
> > spin_unlock_irqrestore(&pgdata->split_queue_lock, flags);
> > - free_compound_page(page);
> > + compound_page_dtor(page);
> > }
> >
> > void deferred_split_huge_page(struct page *page)
> > diff --git a/mm/hugetlb.c b/mm/hugetlb.c
> > index 424b0ef..1af2c4e7 100644
> > --- a/mm/hugetlb.c
> > +++ b/mm/hugetlb.c
> > @@ -1250,7 +1250,7 @@ static void clear_page_huge_active(struct page *page)
> > ClearPagePrivate(&page[1]);
> > }
> >
> > -void free_huge_page(struct page *page)
> > +void huge_page_dtor(struct page *page)
> > {
> > /*
> > * Can't pass hstate in here because it is called from the
> > @@ -1363,7 +1363,7 @@ int PageHeadHuge(struct page *page_head)
> > if (!PageHead(page_head))
> > return 0;
> >
> > - return get_compound_page_dtor(page_head) == free_huge_page;
> > + return get_compound_page_dtor(page_head) == huge_page_dtor;
> > }
> >
> > pgoff_t __basepage_index(struct page *page)
> > @@ -1932,11 +1932,11 @@ static long vma_add_reservation(struct hstate *h,
> > * specific error paths, a huge page was allocated (via alloc_huge_page)
> > * and is about to be freed. If a reservation for the page existed,
> > * alloc_huge_page would have consumed the reservation and set PagePrivate
> > - * in the newly allocated page. When the page is freed via free_huge_page,
> > + * in the newly allocated page. When the page is freed via huge_page_dtor,
> > * the global reservation count will be incremented if PagePrivate is set.
> > - * However, free_huge_page can not adjust the reserve map. Adjust the
> > + * However, huge_page_dtor can not adjust the reserve map. Adjust the
> > * reserve map here to be consistent with global reserve count adjustments
> > - * to be made by free_huge_page.
> > + * to be made by huge_page_dtor.
> > */
> > static void restore_reserve_on_error(struct hstate *h,
> > struct vm_area_struct *vma, unsigned long address,
> > @@ -1950,7 +1950,7 @@ static void restore_reserve_on_error(struct hstate *h,
> > * Rare out of memory condition in reserve map
> > * manipulation. Clear PagePrivate so that
> > * global reserve count will not be incremented
> > - * by free_huge_page. This will make it appear
> > + * by huge_page_dtor. This will make it appear
> > * as though the reservation for this page was
> > * consumed. This may prevent the task from
> > * faulting in the page at a later time. This
> > @@ -2304,7 +2304,7 @@ static unsigned long set_max_huge_pages(struct hstate *h, unsigned long count,
> > while (count > persistent_huge_pages(h)) {
> > /*
> > * If this allocation races such that we no longer need the
> > - * page, free_huge_page will handle it by freeing the page
> > + * page, huge_page_dtor will handle it by freeing the page
> > * and reducing the surplus.
> > */
> > spin_unlock(&hugetlb_lock);
> > diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> > index 77e4d3c..b31205c 100644
> > --- a/mm/page_alloc.c
> > +++ b/mm/page_alloc.c
> > @@ -248,14 +248,14 @@ char * const migratetype_names[MIGRATE_TYPES] = {
> > #endif
> > };
> >
> > -compound_page_dtor * const compound_page_dtors[] = {
> > +compound_page_dtor_t * const compound_page_dtors[] = {
>
> Adding this chunk as well.
>
Sure.
> > NULL,
> > - free_compound_page,
> > + compound_page_dtor,
> > #ifdef CONFIG_HUGETLB_PAGE
> > - free_huge_page,
> > + huge_page_dtor,
> > #endif
> > #ifdef CONFIG_TRANSPARENT_HUGEPAGE
> > - free_transhuge_page,
> > + transhuge_page_dtor,
> > #endif
> > };
>
> Having *dtor* in the destructor functions for the huge pages
> (all of them) actually makes sense. It wont be confused with
> a lot other free_* functions and some of them dealing with
> THP/HugeTLB as well.
>
echo!
> >
> > @@ -586,7 +586,7 @@ static void bad_page(struct page *page, const char *reason,
> > * This usage means that zero-order pages may not be compound.
> > */
> >
> > -void free_compound_page(struct page *page)
> > +void compound_page_dtor(struct page *page)
> > {
> > __free_pages_ok(page, compound_order(page));
> > }
> > diff --git a/mm/swap.c b/mm/swap.c
> > index a77d68f..8f98caf 100644
> > --- a/mm/swap.c
> > +++ b/mm/swap.c
> > @@ -81,7 +81,7 @@ static void __put_single_page(struct page *page)
> >
> > static void __put_compound_page(struct page *page)
> > {
> > - compound_page_dtor *dtor;
> > + compound_page_dtor_t *dtor;
>
> If the typedef change needs to be retained then the commit message
> must include a line.
>
will do it. Thanks.
> --
> To unsubscribe, send a message with 'unsubscribe linux-mm' in
> the body to majordomo@xxxxxxxxx. For more info on Linux MM,
> see: http://www.linux-mm.org/ .
> Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>
--
Thanks,
Changbin Du
Attachment:
signature.asc
Description: PGP signature
