Messages in this thread | | | Date | Wed, 11 May 2022 16:33:38 -0700 | Subject | Re: [PATCH v4] mm: fix is_pinnable_page against on cma page | From | John Hubbard <> |
| |
On 5/11/22 16:28, Minchan Kim wrote: > This is delta to confirm before posting next revision. > > Are you okay with this one?
Yes, just maybe delete the comment:
> > diff --git a/include/linux/mm.h b/include/linux/mm.h > index cbf79eb790e0..7b2df6780552 100644 > --- a/include/linux/mm.h > +++ b/include/linux/mm.h > @@ -1626,14 +1626,14 @@ static inline bool page_needs_cow_for_dma(struct vm_area_struct *vma, > static inline bool is_pinnable_page(struct page *page) > { > #ifdef CONFIG_CMA > + int mt = get_pageblock_migratetype(page); > + > /* > - * use volatile to use local variable mt instead of > - * refetching mt value. > + * "&" operation would prevent compiler split up > + * get_pageblock_migratetype two times for each > + * condition check: refetching mt value two times. > */
If you wanted a useful comment here, I think a description of what is running at the same time would be good. But otherwise, I'd just delete the entire comment, because a micro-comment about "&" is not really worth the vertical space here IMHO.
> - int __mt = get_pageblock_migratetype(page); > - int mt = __READ_ONCE(__mt); > - > - if (mt == MIGRATE_CMA || mt == MIGRATE_ISOLATE) > + if (mt & (MIGRATE_ISOLATE | MIGRATE_CMA)) > return false; > #endif >
Anyway, I'm comfortable with this now:
Reviewed-by: John Hubbard <jhubbard@nvidia.com>
thanks, -- John Hubbard NVIDIA
| |