* [PATCH] mm: Unsigned 'nr_pages' always larger than zero @ 2019-09-04 10:26 zhong jiang 2019-09-04 11:24 ` Vlastimil Babka 0 siblings, 1 reply; 26+ messages in thread From: zhong jiang @ 2019-09-04 10:26 UTC (permalink / raw) To: akpm, mhocko Cc: anshuman.khandual, vbabka, zhongjiang, linux-mm, linux-kernel With the help of unsigned_lesser_than_zero.cocci. Unsigned 'nr_pages"' compare with zero. And __get_user_pages_locked will return an long value. Hence, Convert the long to compare with zero is feasible. Signed-off-by: zhong jiang <zhongjiang@huawei.com> --- mm/gup.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mm/gup.c b/mm/gup.c index 23a9f9c..956d5a1 100644 --- a/mm/gup.c +++ b/mm/gup.c @@ -1508,7 +1508,7 @@ static long check_and_migrate_cma_pages(struct task_struct *tsk, pages, vmas, NULL, gup_flags); - if ((nr_pages > 0) && migrate_allow) { + if (((long)nr_pages > 0) && migrate_allow) { drain_allow = true; goto check_again; } -- 1.7.12.4 ^ permalink raw reply related [flat|nested] 26+ messages in thread
* Re: [PATCH] mm: Unsigned 'nr_pages' always larger than zero 2019-09-04 10:26 [PATCH] mm: Unsigned 'nr_pages' always larger than zero zhong jiang @ 2019-09-04 11:24 ` Vlastimil Babka 2019-09-04 18:25 ` Weiny, Ira ` (3 more replies) 0 siblings, 4 replies; 26+ messages in thread From: Vlastimil Babka @ 2019-09-04 11:24 UTC (permalink / raw) To: zhong jiang, akpm, mhocko Cc: anshuman.khandual, linux-mm, linux-kernel, Ira Weiny, Aneesh Kumar K.V On 9/4/19 12:26 PM, zhong jiang wrote: > With the help of unsigned_lesser_than_zero.cocci. Unsigned 'nr_pages"' > compare with zero. And __get_user_pages_locked will return an long value. > Hence, Convert the long to compare with zero is feasible. It would be nicer if the parameter nr_pages was long again instead of unsigned long (note there are two variants of the function, so both should be changed). > Signed-off-by: zhong jiang <zhongjiang@huawei.com> Fixes: 932f4a630a69 ("mm/gup: replace get_user_pages_longterm() with FOLL_LONGTERM") (which changed long to unsigned long) AFAICS... stable shouldn't be needed as the only "risk" is that we goto check_again even when we fail, which should be harmless. Vlastimil > --- > mm/gup.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/mm/gup.c b/mm/gup.c > index 23a9f9c..956d5a1 100644 > --- a/mm/gup.c > +++ b/mm/gup.c > @@ -1508,7 +1508,7 @@ static long check_and_migrate_cma_pages(struct task_struct *tsk, > pages, vmas, NULL, > gup_flags); > > - if ((nr_pages > 0) && migrate_allow) { > + if (((long)nr_pages > 0) && migrate_allow) { > drain_allow = true; > goto check_again; > } > ^ permalink raw reply [flat|nested] 26+ messages in thread
* RE: [PATCH] mm: Unsigned 'nr_pages' always larger than zero 2019-09-04 11:24 ` Vlastimil Babka @ 2019-09-04 18:25 ` Weiny, Ira 2019-09-04 18:39 ` Matthew Wilcox 2019-09-04 18:48 ` Andrew Morton ` (2 subsequent siblings) 3 siblings, 1 reply; 26+ messages in thread From: Weiny, Ira @ 2019-09-04 18:25 UTC (permalink / raw) To: Vlastimil Babka, zhong jiang, akpm, mhocko Cc: anshuman.khandual, linux-mm, linux-kernel, Aneesh Kumar K.V > On 9/4/19 12:26 PM, zhong jiang wrote: > > With the help of unsigned_lesser_than_zero.cocci. Unsigned 'nr_pages"' > > compare with zero. And __get_user_pages_locked will return an long > value. > > Hence, Convert the long to compare with zero is feasible. > > It would be nicer if the parameter nr_pages was long again instead of > unsigned long (note there are two variants of the function, so both should be > changed). Why? What does it mean for nr_pages to be negative? The check below seems valid. Unsigned can be 0 so the check can fail. IOW Checking unsigned > 0 seems ok. What am I missing? Ira > > > Signed-off-by: zhong jiang <zhongjiang@huawei.com> > > Fixes: 932f4a630a69 ("mm/gup: replace get_user_pages_longterm() with > FOLL_LONGTERM") > > (which changed long to unsigned long) > > AFAICS... stable shouldn't be needed as the only "risk" is that we goto > check_again even when we fail, which should be harmless. > > Vlastimil > > > --- > > mm/gup.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/mm/gup.c b/mm/gup.c > > index 23a9f9c..956d5a1 100644 > > --- a/mm/gup.c > > +++ b/mm/gup.c > > @@ -1508,7 +1508,7 @@ static long check_and_migrate_cma_pages(struct > task_struct *tsk, > > pages, vmas, NULL, > > gup_flags); > > > > - if ((nr_pages > 0) && migrate_allow) { > > + if (((long)nr_pages > 0) && migrate_allow) { > > drain_allow = true; > > goto check_again; > > } > > ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] mm: Unsigned 'nr_pages' always larger than zero 2019-09-04 18:25 ` Weiny, Ira @ 2019-09-04 18:39 ` Matthew Wilcox 0 siblings, 0 replies; 26+ messages in thread From: Matthew Wilcox @ 2019-09-04 18:39 UTC (permalink / raw) To: Weiny, Ira Cc: Vlastimil Babka, zhong jiang, akpm, mhocko, anshuman.khandual, linux-mm, linux-kernel, Aneesh Kumar K.V On Wed, Sep 04, 2019 at 06:25:19PM +0000, Weiny, Ira wrote: > > On 9/4/19 12:26 PM, zhong jiang wrote: > > > With the help of unsigned_lesser_than_zero.cocci. Unsigned 'nr_pages"' > > > compare with zero. And __get_user_pages_locked will return an long > > value. > > > Hence, Convert the long to compare with zero is feasible. > > > > It would be nicer if the parameter nr_pages was long again instead of > > unsigned long (note there are two variants of the function, so both should be > > changed). > > Why? What does it mean for nr_pages to be negative? The check below seems valid. Unsigned can be 0 so the check can fail. IOW Checking unsigned > 0 seems ok. > > What am I missing? __get_user_pages can return a negative errno. ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] mm: Unsigned 'nr_pages' always larger than zero 2019-09-04 11:24 ` Vlastimil Babka 2019-09-04 18:25 ` Weiny, Ira @ 2019-09-04 18:48 ` Andrew Morton 2019-09-04 20:20 ` Weiny, Ira ` (3 more replies) 2019-09-04 19:01 ` Andrew Morton 2019-09-05 2:05 ` zhong jiang 3 siblings, 4 replies; 26+ messages in thread From: Andrew Morton @ 2019-09-04 18:48 UTC (permalink / raw) To: Vlastimil Babka Cc: zhong jiang, mhocko, anshuman.khandual, linux-mm, linux-kernel, Ira Weiny, Aneesh Kumar K.V On Wed, 4 Sep 2019 13:24:58 +0200 Vlastimil Babka <vbabka@suse.cz> wrote: > On 9/4/19 12:26 PM, zhong jiang wrote: > > With the help of unsigned_lesser_than_zero.cocci. Unsigned 'nr_pages"' > > compare with zero. And __get_user_pages_locked will return an long value. > > Hence, Convert the long to compare with zero is feasible. > > It would be nicer if the parameter nr_pages was long again instead of unsigned > long (note there are two variants of the function, so both should be changed). nr_pages should be unsigned - it's a count of pages! The bug is that __get_user_pages_locked() returns a signed long which can be a -ve errno. I think it's best if __get_user_pages_locked() is to get itself a new local with the same type as its return value. Something like: --- a/mm/gup.c~a +++ a/mm/gup.c @@ -1450,6 +1450,7 @@ static long check_and_migrate_cma_pages( bool drain_allow = true; bool migrate_allow = true; LIST_HEAD(cma_page_list); + long ret; check_again: for (i = 0; i < nr_pages;) { @@ -1511,17 +1512,18 @@ check_again: * again migrating any new CMA pages which we failed to isolate * earlier. */ - nr_pages = __get_user_pages_locked(tsk, mm, start, nr_pages, + ret = __get_user_pages_locked(tsk, mm, start, nr_pages, pages, vmas, NULL, gup_flags); - if ((nr_pages > 0) && migrate_allow) { + nr_pages = ret; + if (ret > 0 && migrate_allow) { drain_allow = true; goto check_again; } } - return nr_pages; + return ret; } #else static long check_and_migrate_cma_pages(struct task_struct *tsk, ^ permalink raw reply [flat|nested] 26+ messages in thread
* RE: [PATCH] mm: Unsigned 'nr_pages' always larger than zero 2019-09-04 18:48 ` Andrew Morton @ 2019-09-04 20:20 ` Weiny, Ira 2019-09-04 20:40 ` Vlastimil Babka ` (2 subsequent siblings) 3 siblings, 0 replies; 26+ messages in thread From: Weiny, Ira @ 2019-09-04 20:20 UTC (permalink / raw) To: Andrew Morton, Vlastimil Babka Cc: zhong jiang, mhocko, anshuman.khandual, linux-mm, linux-kernel, Aneesh Kumar K.V > Subject: Re: [PATCH] mm: Unsigned 'nr_pages' always larger than zero > > On Wed, 4 Sep 2019 13:24:58 +0200 Vlastimil Babka <vbabka@suse.cz> > wrote: > > > On 9/4/19 12:26 PM, zhong jiang wrote: > > > With the help of unsigned_lesser_than_zero.cocci. Unsigned 'nr_pages"' > > > compare with zero. And __get_user_pages_locked will return an long > value. > > > Hence, Convert the long to compare with zero is feasible. > > > > It would be nicer if the parameter nr_pages was long again instead of > > unsigned long (note there are two variants of the function, so both should > be changed). > > nr_pages should be unsigned - it's a count of pages! > > The bug is that __get_user_pages_locked() returns a signed long which can > be a -ve errno. Ok... This is my bad... I think this is the correct fix though. Not changing the type of nr_pages. Sorry, Ira > > I think it's best if __get_user_pages_locked() is to get itself a new local with > the same type as its return value. Something like: > > --- a/mm/gup.c~a > +++ a/mm/gup.c > @@ -1450,6 +1450,7 @@ static long check_and_migrate_cma_pages( > bool drain_allow = true; > bool migrate_allow = true; > LIST_HEAD(cma_page_list); > + long ret; > > check_again: > for (i = 0; i < nr_pages;) { > @@ -1511,17 +1512,18 @@ check_again: > * again migrating any new CMA pages which we failed to > isolate > * earlier. > */ > - nr_pages = __get_user_pages_locked(tsk, mm, start, > nr_pages, > + ret = __get_user_pages_locked(tsk, mm, start, nr_pages, > pages, vmas, NULL, > gup_flags); > > - if ((nr_pages > 0) && migrate_allow) { > + nr_pages = ret; > + if (ret > 0 && migrate_allow) { > drain_allow = true; > goto check_again; > } > } > > - return nr_pages; > + return ret; > } > #else > static long check_and_migrate_cma_pages(struct task_struct *tsk, > > ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] mm: Unsigned 'nr_pages' always larger than zero 2019-09-04 18:48 ` Andrew Morton 2019-09-04 20:20 ` Weiny, Ira @ 2019-09-04 20:40 ` Vlastimil Babka 2019-09-05 6:18 ` zhong jiang 2019-09-05 6:19 ` John Hubbard 3 siblings, 0 replies; 26+ messages in thread From: Vlastimil Babka @ 2019-09-04 20:40 UTC (permalink / raw) To: Andrew Morton Cc: zhong jiang, mhocko, anshuman.khandual, linux-mm, linux-kernel, Ira Weiny, Aneesh Kumar K.V On 9/4/19 8:48 PM, Andrew Morton wrote: > On Wed, 4 Sep 2019 13:24:58 +0200 Vlastimil Babka <vbabka@suse.cz> wrote: > >> On 9/4/19 12:26 PM, zhong jiang wrote: >>> With the help of unsigned_lesser_than_zero.cocci. Unsigned 'nr_pages"' >>> compare with zero. And __get_user_pages_locked will return an long value. >>> Hence, Convert the long to compare with zero is feasible. >> >> It would be nicer if the parameter nr_pages was long again instead of unsigned >> long (note there are two variants of the function, so both should be changed). > > nr_pages should be unsigned - it's a count of pages! Hm right, I thought check_and_migrate_cma_pages() could be already called with negative nr_pages from __gup_longterm_locked(), but I missed there's a if (rc < 0) goto out before that. > The bug is that __get_user_pages_locked() returns a signed long which > can be a -ve errno. > > I think it's best if __get_user_pages_locked() is to get itself a new You mean check_and_migrate_cma_pages() > local with the same type as its return value. Something like: Agreed. > --- a/mm/gup.c~a > +++ a/mm/gup.c > @@ -1450,6 +1450,7 @@ static long check_and_migrate_cma_pages( > bool drain_allow = true; > bool migrate_allow = true; > LIST_HEAD(cma_page_list); > + long ret; Should be initialized to nr_pages in case we don't go via "if (!list_empty(&cma_page_list))" at all. > > check_again: > for (i = 0; i < nr_pages;) { > @@ -1511,17 +1512,18 @@ check_again: > * again migrating any new CMA pages which we failed to isolate > * earlier. > */ > - nr_pages = __get_user_pages_locked(tsk, mm, start, nr_pages, > + ret = __get_user_pages_locked(tsk, mm, start, nr_pages, > pages, vmas, NULL, > gup_flags); > > - if ((nr_pages > 0) && migrate_allow) { > + nr_pages = ret; > + if (ret > 0 && migrate_allow) { > drain_allow = true; > goto check_again; > } > } > > - return nr_pages; > + return ret; > } > #else > static long check_and_migrate_cma_pages(struct task_struct *tsk, > > ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] mm: Unsigned 'nr_pages' always larger than zero 2019-09-04 18:48 ` Andrew Morton 2019-09-04 20:20 ` Weiny, Ira 2019-09-04 20:40 ` Vlastimil Babka @ 2019-09-05 6:18 ` zhong jiang 2019-09-05 7:19 ` Vlastimil Babka 2019-09-05 6:19 ` John Hubbard 3 siblings, 1 reply; 26+ messages in thread From: zhong jiang @ 2019-09-05 6:18 UTC (permalink / raw) To: Andrew Morton Cc: Vlastimil Babka, mhocko, anshuman.khandual, linux-mm, linux-kernel, Ira Weiny, Aneesh Kumar K.V On 2019/9/5 2:48, Andrew Morton wrote: > On Wed, 4 Sep 2019 13:24:58 +0200 Vlastimil Babka <vbabka@suse.cz> wrote: > >> On 9/4/19 12:26 PM, zhong jiang wrote: >>> With the help of unsigned_lesser_than_zero.cocci. Unsigned 'nr_pages"' >>> compare with zero. And __get_user_pages_locked will return an long value. >>> Hence, Convert the long to compare with zero is feasible. >> It would be nicer if the parameter nr_pages was long again instead of unsigned >> long (note there are two variants of the function, so both should be changed). > nr_pages should be unsigned - it's a count of pages! > > The bug is that __get_user_pages_locked() returns a signed long which > can be a -ve errno. > > I think it's best if __get_user_pages_locked() is to get itself a new > local with the same type as its return value. Something like: > > --- a/mm/gup.c~a > +++ a/mm/gup.c > @@ -1450,6 +1450,7 @@ static long check_and_migrate_cma_pages( > bool drain_allow = true; > bool migrate_allow = true; > LIST_HEAD(cma_page_list); > + long ret; > > check_again: > for (i = 0; i < nr_pages;) { > @@ -1511,17 +1512,18 @@ check_again: > * again migrating any new CMA pages which we failed to isolate > * earlier. > */ > - nr_pages = __get_user_pages_locked(tsk, mm, start, nr_pages, > + ret = __get_user_pages_locked(tsk, mm, start, nr_pages, > pages, vmas, NULL, > gup_flags); > > - if ((nr_pages > 0) && migrate_allow) { > + nr_pages = ret; > + if (ret > 0 && migrate_allow) { > drain_allow = true; > goto check_again; > } > } > > - return nr_pages; > + return ret; > } > #else > static long check_and_migrate_cma_pages(struct task_struct *tsk, Firstly, I consider the some modified method as you has writen down above. It seems to work well. According to Vlastimil's feedback, I repost the patch in v2, changing the parameter to long to fix the issue. which one do you prefer? Thanks, zhong jiang ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] mm: Unsigned 'nr_pages' always larger than zero 2019-09-05 6:18 ` zhong jiang @ 2019-09-05 7:19 ` Vlastimil Babka 0 siblings, 0 replies; 26+ messages in thread From: Vlastimil Babka @ 2019-09-05 7:19 UTC (permalink / raw) To: zhong jiang, Andrew Morton Cc: mhocko, anshuman.khandual, linux-mm, linux-kernel, Ira Weiny, Aneesh Kumar K.V On 9/5/19 8:18 AM, zhong jiang wrote: > On 2019/9/5 2:48, Andrew Morton wrote: > Firstly, I consider the some modified method as you has writen down above. It seems to work well. > According to Vlastimil's feedback, I repost the patch in v2, changing the parameter to long to fix > the issue. which one do you prefer? Please forget about my suggestion to change parameter to long, it was wrong. New variable is better. > Thanks, > zhong jiang > ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] mm: Unsigned 'nr_pages' always larger than zero 2019-09-04 18:48 ` Andrew Morton ` (2 preceding siblings ...) 2019-09-05 6:18 ` zhong jiang @ 2019-09-05 6:19 ` John Hubbard 2019-10-16 9:07 ` zhong jiang 3 siblings, 1 reply; 26+ messages in thread From: John Hubbard @ 2019-09-05 6:19 UTC (permalink / raw) To: Andrew Morton, Vlastimil Babka Cc: zhong jiang, mhocko, anshuman.khandual, linux-mm, linux-kernel, Ira Weiny, Aneesh Kumar K.V On 9/4/19 11:48 AM, Andrew Morton wrote: > On Wed, 4 Sep 2019 13:24:58 +0200 Vlastimil Babka <vbabka@suse.cz> wrote: > >> On 9/4/19 12:26 PM, zhong jiang wrote: >>> With the help of unsigned_lesser_than_zero.cocci. Unsigned 'nr_pages"' >>> compare with zero. And __get_user_pages_locked will return an long value. >>> Hence, Convert the long to compare with zero is feasible. >> >> It would be nicer if the parameter nr_pages was long again instead of unsigned >> long (note there are two variants of the function, so both should be changed). > > nr_pages should be unsigned - it's a count of pages! > Yes! > The bug is that __get_user_pages_locked() returns a signed long which > can be a -ve errno. > > I think it's best if __get_user_pages_locked() is to get itself a new > local with the same type as its return value. Something like: > > --- a/mm/gup.c~a > +++ a/mm/gup.c > @@ -1450,6 +1450,7 @@ static long check_and_migrate_cma_pages( > bool drain_allow = true; > bool migrate_allow = true; > LIST_HEAD(cma_page_list); > + long ret; > > check_again: > for (i = 0; i < nr_pages;) { > @@ -1511,17 +1512,18 @@ check_again: > * again migrating any new CMA pages which we failed to isolate > * earlier. > */ > - nr_pages = __get_user_pages_locked(tsk, mm, start, nr_pages, > + ret = __get_user_pages_locked(tsk, mm, start, nr_pages, > pages, vmas, NULL, > gup_flags); > > - if ((nr_pages > 0) && migrate_allow) { > + nr_pages = ret; > + if (ret > 0 && migrate_allow) { > drain_allow = true; > goto check_again; > } > } > > - return nr_pages; > + return ret; > } > #else > static long check_and_migrate_cma_pages(struct task_struct *tsk, > +1 for this approach, please. thanks, -- John Hubbard NVIDIA ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] mm: Unsigned 'nr_pages' always larger than zero 2019-09-05 6:19 ` John Hubbard @ 2019-10-16 9:07 ` zhong jiang 2019-10-17 0:49 ` Andrew Morton 0 siblings, 1 reply; 26+ messages in thread From: zhong jiang @ 2019-10-16 9:07 UTC (permalink / raw) To: John Hubbard Cc: Andrew Morton, Vlastimil Babka, mhocko, anshuman.khandual, linux-mm, linux-kernel, Ira Weiny, Aneesh Kumar K.V On 2019/9/5 14:19, John Hubbard wrote: > On 9/4/19 11:48 AM, Andrew Morton wrote: >> On Wed, 4 Sep 2019 13:24:58 +0200 Vlastimil Babka <vbabka@suse.cz> wrote: >> >>> On 9/4/19 12:26 PM, zhong jiang wrote: >>>> With the help of unsigned_lesser_than_zero.cocci. Unsigned 'nr_pages"' >>>> compare with zero. And __get_user_pages_locked will return an long value. >>>> Hence, Convert the long to compare with zero is feasible. >>> >>> It would be nicer if the parameter nr_pages was long again instead of unsigned >>> long (note there are two variants of the function, so both should be changed). >> >> nr_pages should be unsigned - it's a count of pages! >> > > Yes! > >> The bug is that __get_user_pages_locked() returns a signed long which >> can be a -ve errno. >> >> I think it's best if __get_user_pages_locked() is to get itself a new >> local with the same type as its return value. Something like: >> >> --- a/mm/gup.c~a >> +++ a/mm/gup.c >> @@ -1450,6 +1450,7 @@ static long check_and_migrate_cma_pages( >> bool drain_allow = true; >> bool migrate_allow = true; >> LIST_HEAD(cma_page_list); >> + long ret; >> check_again: >> for (i = 0; i < nr_pages;) { >> @@ -1511,17 +1512,18 @@ check_again: >> * again migrating any new CMA pages which we failed to isolate >> * earlier. >> */ >> - nr_pages = __get_user_pages_locked(tsk, mm, start, nr_pages, >> + ret = __get_user_pages_locked(tsk, mm, start, nr_pages, >> pages, vmas, NULL, >> gup_flags); >> - if ((nr_pages > 0) && migrate_allow) { >> + nr_pages = ret; >> + if (ret > 0 && migrate_allow) { >> drain_allow = true; >> goto check_again; >> } >> } >> - return nr_pages; >> + return ret; >> } >> #else >> static long check_and_migrate_cma_pages(struct task_struct *tsk, >> > > +1 for this approach, please. > > > thanks, Hi, Andrew I didn't see the fix for the issue in the upstream. Your proposal should be appiled to upstream. Could you appiled the patch or repost by me ? Thanks, zhogn jiang ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] mm: Unsigned 'nr_pages' always larger than zero 2019-10-16 9:07 ` zhong jiang @ 2019-10-17 0:49 ` Andrew Morton 2019-10-17 1:38 ` zhong jiang 0 siblings, 1 reply; 26+ messages in thread From: Andrew Morton @ 2019-10-17 0:49 UTC (permalink / raw) To: zhong jiang Cc: John Hubbard, Vlastimil Babka, mhocko, anshuman.khandual, linux-mm, linux-kernel, Ira Weiny, Aneesh Kumar K.V On Wed, 16 Oct 2019 17:07:44 +0800 zhong jiang <zhongjiang@huawei.com> wrote: > >> --- a/mm/gup.c~a > >> +++ a/mm/gup.c > >> @@ -1450,6 +1450,7 @@ static long check_and_migrate_cma_pages( > >> bool drain_allow = true; > >> bool migrate_allow = true; > >> LIST_HEAD(cma_page_list); > >> + long ret; > >> check_again: > >> for (i = 0; i < nr_pages;) { > >> @@ -1511,17 +1512,18 @@ check_again: > >> * again migrating any new CMA pages which we failed to isolate > >> * earlier. > >> */ > >> - nr_pages = __get_user_pages_locked(tsk, mm, start, nr_pages, > >> + ret = __get_user_pages_locked(tsk, mm, start, nr_pages, > >> pages, vmas, NULL, > >> gup_flags); > >> - if ((nr_pages > 0) && migrate_allow) { > >> + nr_pages = ret; > >> + if (ret > 0 && migrate_allow) { > >> drain_allow = true; > >> goto check_again; > >> } > >> } > >> - return nr_pages; > >> + return ret; > >> } > >> #else > >> static long check_and_migrate_cma_pages(struct task_struct *tsk, > >> > > > > +1 for this approach, please. > > > > > > thanks, > Hi, Andrew > > I didn't see the fix for the issue in the upstream. Your proposal should be > appiled to upstream. Could you appiled the patch or repost by me ? Forgotten about it ;) Please send a patch sometime? ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] mm: Unsigned 'nr_pages' always larger than zero 2019-10-17 0:49 ` Andrew Morton @ 2019-10-17 1:38 ` zhong jiang 0 siblings, 0 replies; 26+ messages in thread From: zhong jiang @ 2019-10-17 1:38 UTC (permalink / raw) To: Andrew Morton Cc: John Hubbard, Vlastimil Babka, mhocko, anshuman.khandual, linux-mm, linux-kernel, Ira Weiny, Aneesh Kumar K.V On 2019/10/17 8:49, Andrew Morton wrote: > On Wed, 16 Oct 2019 17:07:44 +0800 zhong jiang <zhongjiang@huawei.com> wrote: > >>>> --- a/mm/gup.c~a >>>> +++ a/mm/gup.c >>>> @@ -1450,6 +1450,7 @@ static long check_and_migrate_cma_pages( >>>> bool drain_allow = true; >>>> bool migrate_allow = true; >>>> LIST_HEAD(cma_page_list); >>>> + long ret; >>>> check_again: >>>> for (i = 0; i < nr_pages;) { >>>> @@ -1511,17 +1512,18 @@ check_again: >>>> * again migrating any new CMA pages which we failed to isolate >>>> * earlier. >>>> */ >>>> - nr_pages = __get_user_pages_locked(tsk, mm, start, nr_pages, >>>> + ret = __get_user_pages_locked(tsk, mm, start, nr_pages, >>>> pages, vmas, NULL, >>>> gup_flags); >>>> - if ((nr_pages > 0) && migrate_allow) { >>>> + nr_pages = ret; >>>> + if (ret > 0 && migrate_allow) { >>>> drain_allow = true; >>>> goto check_again; >>>> } >>>> } >>>> - return nr_pages; >>>> + return ret; >>>> } >>>> #else >>>> static long check_and_migrate_cma_pages(struct task_struct *tsk, >>>> >>> +1 for this approach, please. >>> >>> >>> thanks, >> Hi, Andrew >> >> I didn't see the fix for the issue in the upstream. Your proposal should be >> appiled to upstream. Could you appiled the patch or repost by me ? > Forgotten about it ;) Please send a patch sometime? > > . > I will repost the patch as your suggestion. Thanks, Sincerely, zhong jiang ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] mm: Unsigned 'nr_pages' always larger than zero 2019-09-04 11:24 ` Vlastimil Babka 2019-09-04 18:25 ` Weiny, Ira 2019-09-04 18:48 ` Andrew Morton @ 2019-09-04 19:01 ` Andrew Morton 2019-09-04 20:44 ` Vlastimil Babka 2019-09-05 2:05 ` zhong jiang 3 siblings, 1 reply; 26+ messages in thread From: Andrew Morton @ 2019-09-04 19:01 UTC (permalink / raw) To: Vlastimil Babka Cc: zhong jiang, mhocko, anshuman.khandual, linux-mm, linux-kernel, Ira Weiny, Aneesh Kumar K.V On Wed, 4 Sep 2019 13:24:58 +0200 Vlastimil Babka <vbabka@suse.cz> wrote: > On 9/4/19 12:26 PM, zhong jiang wrote: > > With the help of unsigned_lesser_than_zero.cocci. Unsigned 'nr_pages"' > > compare with zero. And __get_user_pages_locked will return an long value. > > Hence, Convert the long to compare with zero is feasible. > > It would be nicer if the parameter nr_pages was long again instead of unsigned > long (note there are two variants of the function, so both should be changed). > > > Signed-off-by: zhong jiang <zhongjiang@huawei.com> > > Fixes: 932f4a630a69 ("mm/gup: replace get_user_pages_longterm() with FOLL_LONGTERM") > > (which changed long to unsigned long) > > AFAICS... stable shouldn't be needed as the only "risk" is that we goto > check_again even when we fail, which should be harmless. > Really? If nr_pages gets a value of -EFAULT from the __get_user_pages_locked() call, check_and_migrate_cma_pages() will go berzerk? And does __get_user_pages_locked() correctly handle a -ve errno returned by __get_user_pages()? It's hard to see how... ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] mm: Unsigned 'nr_pages' always larger than zero 2019-09-04 19:01 ` Andrew Morton @ 2019-09-04 20:44 ` Vlastimil Babka 0 siblings, 0 replies; 26+ messages in thread From: Vlastimil Babka @ 2019-09-04 20:44 UTC (permalink / raw) To: Andrew Morton Cc: zhong jiang, mhocko, anshuman.khandual, linux-mm, linux-kernel, Ira Weiny, Aneesh Kumar K.V On 9/4/19 9:01 PM, Andrew Morton wrote: > On Wed, 4 Sep 2019 13:24:58 +0200 Vlastimil Babka <vbabka@suse.cz> wrote: > >> On 9/4/19 12:26 PM, zhong jiang wrote: >>> With the help of unsigned_lesser_than_zero.cocci. Unsigned 'nr_pages"' >>> compare with zero. And __get_user_pages_locked will return an long value. >>> Hence, Convert the long to compare with zero is feasible. >> >> It would be nicer if the parameter nr_pages was long again instead of unsigned >> long (note there are two variants of the function, so both should be changed). >> >>> Signed-off-by: zhong jiang <zhongjiang@huawei.com> >> >> Fixes: 932f4a630a69 ("mm/gup: replace get_user_pages_longterm() with FOLL_LONGTERM") >> >> (which changed long to unsigned long) >> >> AFAICS... stable shouldn't be needed as the only "risk" is that we goto >> check_again even when we fail, which should be harmless. >> > > Really? If nr_pages gets a value of -EFAULT from the > __get_user_pages_locked() call, check_and_migrate_cma_pages() will go > berzerk? Hmm it should only reach that goto when it migrated something, which means it won't have to be migrated again, so eventually it should terminate. But it's very subtle, so I might be wrong. > And does __get_user_pages_locked() correctly handle a -ve errno > returned by __get_user_pages()? It's hard to see how... I think it does, but agree. ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] mm: Unsigned 'nr_pages' always larger than zero 2019-09-04 11:24 ` Vlastimil Babka ` (2 preceding siblings ...) 2019-09-04 19:01 ` Andrew Morton @ 2019-09-05 2:05 ` zhong jiang 3 siblings, 0 replies; 26+ messages in thread From: zhong jiang @ 2019-09-05 2:05 UTC (permalink / raw) To: Vlastimil Babka Cc: akpm, mhocko, anshuman.khandual, linux-mm, linux-kernel, Ira Weiny, Aneesh Kumar K.V On 2019/9/4 19:24, Vlastimil Babka wrote: > On 9/4/19 12:26 PM, zhong jiang wrote: >> With the help of unsigned_lesser_than_zero.cocci. Unsigned 'nr_pages"' >> compare with zero. And __get_user_pages_locked will return an long value. >> Hence, Convert the long to compare with zero is feasible. > It would be nicer if the parameter nr_pages was long again instead of unsigned > long (note there are two variants of the function, so both should be changed). Yep, the parameter 'nr_pages' was changed to long. and the variants ‘i、step’ should be changed accordingly. >> Signed-off-by: zhong jiang <zhongjiang@huawei.com> > Fixes: 932f4a630a69 ("mm/gup: replace get_user_pages_longterm() with FOLL_LONGTERM") > > (which changed long to unsigned long) > > AFAICS... stable shouldn't be needed as the only "risk" is that we goto > check_again even when we fail, which should be harmless. Agreed, Thanks. Sincerely, zhong jiang > Vlastimil > >> --- >> mm/gup.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/mm/gup.c b/mm/gup.c >> index 23a9f9c..956d5a1 100644 >> --- a/mm/gup.c >> +++ b/mm/gup.c >> @@ -1508,7 +1508,7 @@ static long check_and_migrate_cma_pages(struct task_struct *tsk, >> pages, vmas, NULL, >> gup_flags); >> >> - if ((nr_pages > 0) && migrate_allow) { >> + if (((long)nr_pages > 0) && migrate_allow) { >> drain_allow = true; >> goto check_again; >> } >> > > . > ^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH] mm: Unsigned 'nr_pages' always larger than zero @ 2019-10-17 3:19 zhong jiang 2019-10-17 18:01 ` Ira Weiny 2019-10-17 20:42 ` John Hubbard 0 siblings, 2 replies; 26+ messages in thread From: zhong jiang @ 2019-10-17 3:19 UTC (permalink / raw) To: akpm; +Cc: vbabka, jhubbard, linux-mm, zhongjiang With the help of unsigned_lesser_than_zero.cocci. Unsigned 'nr_pages"' compare with zero. And __get_user_pages_locked will return an long value. The patch use a new local variable to store the return value of __get_user_pages_locked(). Then use it to compare with zero. Suggested-by: Andrew Morton <akpm@linux-foundation.org> Signed-off-by: zhong jiang <zhongjiang@huawei.com> --- mm/gup.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/mm/gup.c b/mm/gup.c index 8f236a3..1fe0ceb 100644 --- a/mm/gup.c +++ b/mm/gup.c @@ -1443,6 +1443,7 @@ static long check_and_migrate_cma_pages(struct task_struct *tsk, bool drain_allow = true; bool migrate_allow = true; LIST_HEAD(cma_page_list); + long ret; check_again: for (i = 0; i < nr_pages;) { @@ -1504,17 +1505,18 @@ static long check_and_migrate_cma_pages(struct task_struct *tsk, * again migrating any new CMA pages which we failed to isolate * earlier. */ - nr_pages = __get_user_pages_locked(tsk, mm, start, nr_pages, + ret = __get_user_pages_locked(tsk, mm, start, nr_pages, pages, vmas, NULL, gup_flags); - if ((nr_pages > 0) && migrate_allow) { + nr_pages = ret; + if ((ret > 0) && migrate_allow) { drain_allow = true; goto check_again; } } - return nr_pages; + return ret; } #else static long check_and_migrate_cma_pages(struct task_struct *tsk, -- 1.7.12.4 ^ permalink raw reply related [flat|nested] 26+ messages in thread
* Re: [PATCH] mm: Unsigned 'nr_pages' always larger than zero 2019-10-17 3:19 zhong jiang @ 2019-10-17 18:01 ` Ira Weiny 2019-10-18 2:01 ` zhong jiang 2019-10-17 20:42 ` John Hubbard 1 sibling, 1 reply; 26+ messages in thread From: Ira Weiny @ 2019-10-17 18:01 UTC (permalink / raw) To: zhong jiang; +Cc: akpm, vbabka, jhubbard, linux-mm On Thu, Oct 17, 2019 at 11:19:46AM +0800, zhong jiang wrote: > With the help of unsigned_lesser_than_zero.cocci. Unsigned 'nr_pages"' > compare with zero. And __get_user_pages_locked will return an long value. > > The patch use a new local variable to store the return value of > __get_user_pages_locked(). Then use it to compare with zero. > > Suggested-by: Andrew Morton <akpm@linux-foundation.org> > Signed-off-by: zhong jiang <zhongjiang@huawei.com> > --- > mm/gup.c | 8 +++++--- > 1 file changed, 5 insertions(+), 3 deletions(-) > > diff --git a/mm/gup.c b/mm/gup.c > index 8f236a3..1fe0ceb 100644 > --- a/mm/gup.c > +++ b/mm/gup.c > @@ -1443,6 +1443,7 @@ static long check_and_migrate_cma_pages(struct task_struct *tsk, > bool drain_allow = true; > bool migrate_allow = true; > LIST_HEAD(cma_page_list); > + long ret; > > check_again: > for (i = 0; i < nr_pages;) { > @@ -1504,17 +1505,18 @@ static long check_and_migrate_cma_pages(struct task_struct *tsk, > * again migrating any new CMA pages which we failed to isolate > * earlier. > */ > - nr_pages = __get_user_pages_locked(tsk, mm, start, nr_pages, > + ret = __get_user_pages_locked(tsk, mm, start, nr_pages, > pages, vmas, NULL, > gup_flags); > > - if ((nr_pages > 0) && migrate_allow) { > + nr_pages = ret; I think if ret is negative we want to return the error here. > + if ((ret > 0) && migrate_allow) { > drain_allow = true; > goto check_again; > } > } > > - return nr_pages; > + return ret; If the cma_page_list is empty doesn't this return uninitialized data? Ira > } > #else > static long check_and_migrate_cma_pages(struct task_struct *tsk, > -- > 1.7.12.4 > > ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] mm: Unsigned 'nr_pages' always larger than zero 2019-10-17 18:01 ` Ira Weiny @ 2019-10-18 2:01 ` zhong jiang 0 siblings, 0 replies; 26+ messages in thread From: zhong jiang @ 2019-10-18 2:01 UTC (permalink / raw) To: Ira Weiny; +Cc: akpm, vbabka, jhubbard, linux-mm On 2019/10/18 2:01, Ira Weiny wrote: > On Thu, Oct 17, 2019 at 11:19:46AM +0800, zhong jiang wrote: >> With the help of unsigned_lesser_than_zero.cocci. Unsigned 'nr_pages"' >> compare with zero. And __get_user_pages_locked will return an long value. >> >> The patch use a new local variable to store the return value of >> __get_user_pages_locked(). Then use it to compare with zero. >> >> Suggested-by: Andrew Morton <akpm@linux-foundation.org> >> Signed-off-by: zhong jiang <zhongjiang@huawei.com> >> --- >> mm/gup.c | 8 +++++--- >> 1 file changed, 5 insertions(+), 3 deletions(-) >> >> diff --git a/mm/gup.c b/mm/gup.c >> index 8f236a3..1fe0ceb 100644 >> --- a/mm/gup.c >> +++ b/mm/gup.c >> @@ -1443,6 +1443,7 @@ static long check_and_migrate_cma_pages(struct task_struct *tsk, >> bool drain_allow = true; >> bool migrate_allow = true; >> LIST_HEAD(cma_page_list); >> + long ret; >> >> check_again: >> for (i = 0; i < nr_pages;) { >> @@ -1504,17 +1505,18 @@ static long check_and_migrate_cma_pages(struct task_struct *tsk, >> * again migrating any new CMA pages which we failed to isolate >> * earlier. >> */ >> - nr_pages = __get_user_pages_locked(tsk, mm, start, nr_pages, >> + ret = __get_user_pages_locked(tsk, mm, start, nr_pages, >> pages, vmas, NULL, >> gup_flags); >> >> - if ((nr_pages > 0) && migrate_allow) { >> + nr_pages = ret; > I think if ret is negative we want to return the error here. I think the code works. Because we alway return the ret. we will return the error value if the ret is negative . >> + if ((ret > 0) && migrate_allow) { >> drain_allow = true; >> goto check_again; >> } >> } >> >> - return nr_pages; >> + return ret; > If the cma_page_list is empty doesn't this return uninitialized data? You're right. I miss that. Thanks for pointing out. Sincerely, zhong jiang > Ira > >> } >> #else >> static long check_and_migrate_cma_pages(struct task_struct *tsk, >> -- >> 1.7.12.4 >> >> > . > ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] mm: Unsigned 'nr_pages' always larger than zero 2019-10-17 3:19 zhong jiang 2019-10-17 18:01 ` Ira Weiny @ 2019-10-17 20:42 ` John Hubbard 2019-10-18 2:15 ` zhong jiang ` (2 more replies) 1 sibling, 3 replies; 26+ messages in thread From: John Hubbard @ 2019-10-17 20:42 UTC (permalink / raw) To: zhong jiang, akpm; +Cc: vbabka, linux-mm On 10/16/19 8:19 PM, zhong jiang wrote: > With the help of unsigned_lesser_than_zero.cocci. Unsigned 'nr_pages"' > compare with zero. And __get_user_pages_locked will return an long value. > > The patch use a new local variable to store the return value of > __get_user_pages_locked(). Then use it to compare with zero. Hi Zhong, The above are actually more like notes to yourself, and while those are good to have, but it's not yet a perfect commit description: it talks about how you got here, which is only part of the story. A higher level summary is better, and let the code itself cover the details. Like this: First line (subject): mm/gup: allow CMA migration to propagate errors back to caller Commit description: check_and_migrate_cma_pages() was recording the result of __get_user_pages_locked() in an unsigned "nr_pages" variable. Because __get_user_pages_locked() returns a signed value that can include negative errno values, this had the effect of hiding errors. Change check_and_migrate_cma_pages() implementation so that it uses a signed variable instead, and propagates the results back to the caller just as other gup internal functions do. This was discovered with the help of unsigned_lesser_than_zero.cocci. > > Suggested-by: Andrew Morton <akpm@linux-foundation.org> > Signed-off-by: zhong jiang <zhongjiang@huawei.com> > --- > mm/gup.c | 8 +++++--- > 1 file changed, 5 insertions(+), 3 deletions(-) > > diff --git a/mm/gup.c b/mm/gup.c > index 8f236a3..1fe0ceb 100644 > --- a/mm/gup.c > +++ b/mm/gup.c > @@ -1443,6 +1443,7 @@ static long check_and_migrate_cma_pages(struct task_struct *tsk, > bool drain_allow = true; > bool migrate_allow = true; > LIST_HEAD(cma_page_list); > + long ret; Ira pointed out that this needs initialization, see below. > > check_again: > for (i = 0; i < nr_pages;) { > @@ -1504,17 +1505,18 @@ static long check_and_migrate_cma_pages(struct task_struct *tsk, > * again migrating any new CMA pages which we failed to isolate > * earlier. > */ > - nr_pages = __get_user_pages_locked(tsk, mm, start, nr_pages, > + ret = __get_user_pages_locked(tsk, mm, start, nr_pages, > pages, vmas, NULL, > gup_flags); > > - if ((nr_pages > 0) && migrate_allow) { > + nr_pages = ret; Although technically correct, it makes me feel odd to see the assignment done from signed to unsigned, *before* checking for >0. And Ira is hinting at the same thing, when he asks if we can return early here. See below... > + if ((ret > 0) && migrate_allow) { > drain_allow = true; > goto check_again; > } > } > > - return nr_pages; > + return ret; > } > #else > static long check_and_migrate_cma_pages(struct task_struct *tsk, > So, overall, I'd recommend this, instead: diff --git a/mm/gup.c b/mm/gup.c index 23a9f9c9d377..72bc027037fa 100644 --- a/mm/gup.c +++ b/mm/gup.c @@ -1443,6 +1443,7 @@ static long check_and_migrate_cma_pages(struct task_struct *tsk, bool drain_allow = true; bool migrate_allow = true; LIST_HEAD(cma_page_list); + long ret = nr_pages; check_again: for (i = 0; i < nr_pages;) { @@ -1504,17 +1505,18 @@ static long check_and_migrate_cma_pages(struct task_struct *tsk, * again migrating any new CMA pages which we failed to isolate * earlier. */ - nr_pages = __get_user_pages_locked(tsk, mm, start, nr_pages, + ret = __get_user_pages_locked(tsk, mm, start, nr_pages, pages, vmas, NULL, gup_flags); - if ((nr_pages > 0) && migrate_allow) { + if ((ret > 0) && migrate_allow) { + nr_pages = ret; drain_allow = true; goto check_again; } } - return nr_pages; + return ret; } #else static long check_and_migrate_cma_pages(struct task_struct *tsk, thanks, John Hubbard NVIDIA ^ permalink raw reply related [flat|nested] 26+ messages in thread
* Re: [PATCH] mm: Unsigned 'nr_pages' always larger than zero 2019-10-17 20:42 ` John Hubbard @ 2019-10-18 2:15 ` zhong jiang 2019-10-18 14:00 ` zhong jiang 2019-10-21 12:57 ` Vlastimil Babka 2 siblings, 0 replies; 26+ messages in thread From: zhong jiang @ 2019-10-18 2:15 UTC (permalink / raw) To: John Hubbard; +Cc: akpm, vbabka, linux-mm On 2019/10/18 4:42, John Hubbard wrote: > On 10/16/19 8:19 PM, zhong jiang wrote: >> With the help of unsigned_lesser_than_zero.cocci. Unsigned 'nr_pages"' >> compare with zero. And __get_user_pages_locked will return an long value. >> >> The patch use a new local variable to store the return value of >> __get_user_pages_locked(). Then use it to compare with zero. > Hi Zhong, > > The above are actually more like notes to yourself, and while those are > good to have, but it's not yet a perfect commit description: it talks > about how you got here, which is only part of the story. A higher level > summary is better, and let the code itself cover the details. > > Like this: > > First line (subject): > > mm/gup: allow CMA migration to propagate errors back to caller > > Commit description: > > check_and_migrate_cma_pages() was recording the result of > __get_user_pages_locked() in an unsigned "nr_pages" variable. Because > __get_user_pages_locked() returns a signed value that can include > negative errno values, this had the effect of hiding errors. > > Change check_and_migrate_cma_pages() implementation so that it > uses a signed variable instead, and propagates the results back > to the caller just as other gup internal functions do. > > This was discovered with the help of unsigned_lesser_than_zero.cocci. > >> Suggested-by: Andrew Morton <akpm@linux-foundation.org> >> Signed-off-by: zhong jiang <zhongjiang@huawei.com> >> --- >> mm/gup.c | 8 +++++--- >> 1 file changed, 5 insertions(+), 3 deletions(-) >> >> diff --git a/mm/gup.c b/mm/gup.c >> index 8f236a3..1fe0ceb 100644 >> --- a/mm/gup.c >> +++ b/mm/gup.c >> @@ -1443,6 +1443,7 @@ static long check_and_migrate_cma_pages(struct task_struct *tsk, >> bool drain_allow = true; >> bool migrate_allow = true; >> LIST_HEAD(cma_page_list); >> + long ret; > Ira pointed out that this needs initialization, see below. I miss that. Thanks for pointing out. >> >> check_again: >> for (i = 0; i < nr_pages;) { >> @@ -1504,17 +1505,18 @@ static long check_and_migrate_cma_pages(struct task_struct *tsk, >> * again migrating any new CMA pages which we failed to isolate >> * earlier. >> */ >> - nr_pages = __get_user_pages_locked(tsk, mm, start, nr_pages, >> + ret = __get_user_pages_locked(tsk, mm, start, nr_pages, >> pages, vmas, NULL, >> gup_flags); >> >> - if ((nr_pages > 0) && migrate_allow) { >> + nr_pages = ret; > Although technically correct, it makes me feel odd to see the assignment > done from signed to unsigned, *before* checking for >0. And Ira is hinting > at the same thing, when he asks if we can return early here. See below... At first. I realize the same thing, But assign the value from signed to unsigned before checking for >0 doesn't matter when we can return early here. Because we only return the ret. Of course, The assignment is meaningless when we can return early. It indeed is better to do as you suggested. :-( >> + if ((ret > 0) && migrate_allow) { >> drain_allow = true; >> goto check_again; >> } >> } >> >> - return nr_pages; >> + return ret; >> } >> #else >> static long check_and_migrate_cma_pages(struct task_struct *tsk, >> > So, overall, I'd recommend this, instead: > > diff --git a/mm/gup.c b/mm/gup.c > index 23a9f9c9d377..72bc027037fa 100644 > --- a/mm/gup.c > +++ b/mm/gup.c > @@ -1443,6 +1443,7 @@ static long check_and_migrate_cma_pages(struct task_struct *tsk, > bool drain_allow = true; > bool migrate_allow = true; > LIST_HEAD(cma_page_list); > + long ret = nr_pages; > > check_again: > for (i = 0; i < nr_pages;) { > @@ -1504,17 +1505,18 @@ static long check_and_migrate_cma_pages(struct task_struct *tsk, > * again migrating any new CMA pages which we failed to isolate > * earlier. > */ > - nr_pages = __get_user_pages_locked(tsk, mm, start, nr_pages, > + ret = __get_user_pages_locked(tsk, mm, start, nr_pages, > pages, vmas, NULL, > gup_flags); > > - if ((nr_pages > 0) && migrate_allow) { > + if ((ret > 0) && migrate_allow) { > + nr_pages = ret; > drain_allow = true; > goto check_again; > } > } > > - return nr_pages; > + return ret; > } > #else > static long check_and_migrate_cma_pages(struct task_struct *tsk, > > > thanks, > > John Hubbard > NVIDIA > > . > Thanks for your suggestion. I will repost it as you has proposed. Sincerely, zhong jiang ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] mm: Unsigned 'nr_pages' always larger than zero 2019-10-17 20:42 ` John Hubbard 2019-10-18 2:15 ` zhong jiang @ 2019-10-18 14:00 ` zhong jiang 2019-10-18 16:07 ` Alexander Duyck 2019-10-21 12:57 ` Vlastimil Babka 2 siblings, 1 reply; 26+ messages in thread From: zhong jiang @ 2019-10-18 14:00 UTC (permalink / raw) To: John Hubbard; +Cc: akpm, vbabka, linux-mm On 2019/10/18 4:42, John Hubbard wrote: > On 10/16/19 8:19 PM, zhong jiang wrote: >> With the help of unsigned_lesser_than_zero.cocci. Unsigned 'nr_pages"' >> compare with zero. And __get_user_pages_locked will return an long value. >> >> The patch use a new local variable to store the return value of >> __get_user_pages_locked(). Then use it to compare with zero. > Hi Zhong, > > The above are actually more like notes to yourself, and while those are > good to have, but it's not yet a perfect commit description: it talks > about how you got here, which is only part of the story. A higher level > summary is better, and let the code itself cover the details. > > Like this: > > First line (subject): > > mm/gup: allow CMA migration to propagate errors back to caller > > Commit description: > > check_and_migrate_cma_pages() was recording the result of > __get_user_pages_locked() in an unsigned "nr_pages" variable. Because > __get_user_pages_locked() returns a signed value that can include > negative errno values, this had the effect of hiding errors. > > Change check_and_migrate_cma_pages() implementation so that it > uses a signed variable instead, and propagates the results back > to the caller just as other gup internal functions do. > > This was discovered with the help of unsigned_lesser_than_zero.cocci. > >> Suggested-by: Andrew Morton <akpm@linux-foundation.org> >> Signed-off-by: zhong jiang <zhongjiang@huawei.com> >> --- >> mm/gup.c | 8 +++++--- >> 1 file changed, 5 insertions(+), 3 deletions(-) >> >> diff --git a/mm/gup.c b/mm/gup.c >> index 8f236a3..1fe0ceb 100644 >> --- a/mm/gup.c >> +++ b/mm/gup.c >> @@ -1443,6 +1443,7 @@ static long check_and_migrate_cma_pages(struct task_struct *tsk, >> bool drain_allow = true; >> bool migrate_allow = true; >> LIST_HEAD(cma_page_list); >> + long ret; > Ira pointed out that this needs initialization, see below. > >> >> check_again: >> for (i = 0; i < nr_pages;) { >> @@ -1504,17 +1505,18 @@ static long check_and_migrate_cma_pages(struct task_struct *tsk, >> * again migrating any new CMA pages which we failed to isolate >> * earlier. >> */ >> - nr_pages = __get_user_pages_locked(tsk, mm, start, nr_pages, >> + ret = __get_user_pages_locked(tsk, mm, start, nr_pages, >> pages, vmas, NULL, >> gup_flags); >> >> - if ((nr_pages > 0) && migrate_allow) { >> + nr_pages = ret; > Although technically correct, it makes me feel odd to see the assignment > done from signed to unsigned, *before* checking for >0. And Ira is hinting > at the same thing, when he asks if we can return early here. See below... > >> + if ((ret > 0) && migrate_allow) { >> drain_allow = true; >> goto check_again; >> } >> } >> >> - return nr_pages; >> + return ret; >> } >> #else >> static long check_and_migrate_cma_pages(struct task_struct *tsk, >> > So, overall, I'd recommend this, instead: > > diff --git a/mm/gup.c b/mm/gup.c > index 23a9f9c9d377..72bc027037fa 100644 > --- a/mm/gup.c > +++ b/mm/gup.c > @@ -1443,6 +1443,7 @@ static long check_and_migrate_cma_pages(struct task_struct *tsk, > bool drain_allow = true; > bool migrate_allow = true; > LIST_HEAD(cma_page_list); > + long ret = nr_pages; > I think the ret should be assigned as zero. we will return ret if cma_page_list is empty. I miss something? Thanks, zhong jiang > check_again: > for (i = 0; i < nr_pages;) { > @@ -1504,17 +1505,18 @@ static long check_and_migrate_cma_pages(struct task_struct *tsk, > * again migrating any new CMA pages which we failed to isolate > * earlier. > */ > - nr_pages = __get_user_pages_locked(tsk, mm, start, nr_pages, > + ret = __get_user_pages_locked(tsk, mm, start, nr_pages, > pages, vmas, NULL, > gup_flags); > > - if ((nr_pages > 0) && migrate_allow) { > + if ((ret > 0) && migrate_allow) { > + nr_pages = ret; > drain_allow = true; > goto check_again; > } > } > > - return nr_pages; > + return ret; > } > #else > static long check_and_migrate_cma_pages(struct task_struct *tsk, > > > thanks, > > John Hubbard > NVIDIA > > . > ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] mm: Unsigned 'nr_pages' always larger than zero 2019-10-18 14:00 ` zhong jiang @ 2019-10-18 16:07 ` Alexander Duyck 2019-10-21 15:11 ` zhong jiang 0 siblings, 1 reply; 26+ messages in thread From: Alexander Duyck @ 2019-10-18 16:07 UTC (permalink / raw) To: zhong jiang; +Cc: John Hubbard, Andrew Morton, Vlastimil Babka, linux-mm On Fri, Oct 18, 2019 at 7:00 AM zhong jiang <zhongjiang@huawei.com> wrote: > > On 2019/10/18 4:42, John Hubbard wrote: > > On 10/16/19 8:19 PM, zhong jiang wrote: > >> With the help of unsigned_lesser_than_zero.cocci. Unsigned 'nr_pages"' > >> compare with zero. And __get_user_pages_locked will return an long value. > >> > >> The patch use a new local variable to store the return value of > >> __get_user_pages_locked(). Then use it to compare with zero. > > Hi Zhong, > > > > The above are actually more like notes to yourself, and while those are > > good to have, but it's not yet a perfect commit description: it talks > > about how you got here, which is only part of the story. A higher level > > summary is better, and let the code itself cover the details. > > > > Like this: > > > > First line (subject): > > > > mm/gup: allow CMA migration to propagate errors back to caller > > > > Commit description: > > > > check_and_migrate_cma_pages() was recording the result of > > __get_user_pages_locked() in an unsigned "nr_pages" variable. Because > > __get_user_pages_locked() returns a signed value that can include > > negative errno values, this had the effect of hiding errors. > > > > Change check_and_migrate_cma_pages() implementation so that it > > uses a signed variable instead, and propagates the results back > > to the caller just as other gup internal functions do. > > > > This was discovered with the help of unsigned_lesser_than_zero.cocci. > > > >> Suggested-by: Andrew Morton <akpm@linux-foundation.org> > >> Signed-off-by: zhong jiang <zhongjiang@huawei.com> > >> --- > >> mm/gup.c | 8 +++++--- > >> 1 file changed, 5 insertions(+), 3 deletions(-) > >> > >> diff --git a/mm/gup.c b/mm/gup.c > >> index 8f236a3..1fe0ceb 100644 > >> --- a/mm/gup.c > >> +++ b/mm/gup.c > >> @@ -1443,6 +1443,7 @@ static long check_and_migrate_cma_pages(struct task_struct *tsk, > >> bool drain_allow = true; > >> bool migrate_allow = true; > >> LIST_HEAD(cma_page_list); > >> + long ret; > > Ira pointed out that this needs initialization, see below. > > > >> > >> check_again: > >> for (i = 0; i < nr_pages;) { > >> @@ -1504,17 +1505,18 @@ static long check_and_migrate_cma_pages(struct task_struct *tsk, > >> * again migrating any new CMA pages which we failed to isolate > >> * earlier. > >> */ > >> - nr_pages = __get_user_pages_locked(tsk, mm, start, nr_pages, > >> + ret = __get_user_pages_locked(tsk, mm, start, nr_pages, > >> pages, vmas, NULL, > >> gup_flags); > >> > >> - if ((nr_pages > 0) && migrate_allow) { > >> + nr_pages = ret; > > Although technically correct, it makes me feel odd to see the assignment > > done from signed to unsigned, *before* checking for >0. And Ira is hinting > > at the same thing, when he asks if we can return early here. See below... > > > >> + if ((ret > 0) && migrate_allow) { > >> drain_allow = true; > >> goto check_again; > >> } > >> } > >> > >> - return nr_pages; > >> + return ret; > >> } > >> #else > >> static long check_and_migrate_cma_pages(struct task_struct *tsk, > >> > > So, overall, I'd recommend this, instead: > > > > diff --git a/mm/gup.c b/mm/gup.c > > index 23a9f9c9d377..72bc027037fa 100644 > > --- a/mm/gup.c > > +++ b/mm/gup.c > > @@ -1443,6 +1443,7 @@ static long check_and_migrate_cma_pages(struct task_struct *tsk, > > bool drain_allow = true; > > bool migrate_allow = true; > > LIST_HEAD(cma_page_list); > > + long ret = nr_pages; > > > I think the ret should be assigned as zero. we will return ret if cma_page_list is empty. > I miss something? That wasn't the behavior before. Before if the cma_page_list was empty nr_pages would not be modified from what what passed. I believe that is the intended behavior for this since the cma_page_list seems like a workaround to migrate out any CMA pages if present in the longterm mapping. If there are no CMA pages the return result should be nr_pages. Thanks. - Alex ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] mm: Unsigned 'nr_pages' always larger than zero 2019-10-18 16:07 ` Alexander Duyck @ 2019-10-21 15:11 ` zhong jiang 0 siblings, 0 replies; 26+ messages in thread From: zhong jiang @ 2019-10-21 15:11 UTC (permalink / raw) To: Alexander Duyck; +Cc: John Hubbard, Andrew Morton, Vlastimil Babka, linux-mm On 2019/10/19 0:07, Alexander Duyck wrote: > On Fri, Oct 18, 2019 at 7:00 AM zhong jiang <zhongjiang@huawei.com> wrote: >> On 2019/10/18 4:42, John Hubbard wrote: >>> On 10/16/19 8:19 PM, zhong jiang wrote: >>>> With the help of unsigned_lesser_than_zero.cocci. Unsigned 'nr_pages"' >>>> compare with zero. And __get_user_pages_locked will return an long value. >>>> >>>> The patch use a new local variable to store the return value of >>>> __get_user_pages_locked(). Then use it to compare with zero. >>> Hi Zhong, >>> >>> The above are actually more like notes to yourself, and while those are >>> good to have, but it's not yet a perfect commit description: it talks >>> about how you got here, which is only part of the story. A higher level >>> summary is better, and let the code itself cover the details. >>> >>> Like this: >>> >>> First line (subject): >>> >>> mm/gup: allow CMA migration to propagate errors back to caller >>> >>> Commit description: >>> >>> check_and_migrate_cma_pages() was recording the result of >>> __get_user_pages_locked() in an unsigned "nr_pages" variable. Because >>> __get_user_pages_locked() returns a signed value that can include >>> negative errno values, this had the effect of hiding errors. >>> >>> Change check_and_migrate_cma_pages() implementation so that it >>> uses a signed variable instead, and propagates the results back >>> to the caller just as other gup internal functions do. >>> >>> This was discovered with the help of unsigned_lesser_than_zero.cocci. >>> >>>> Suggested-by: Andrew Morton <akpm@linux-foundation.org> >>>> Signed-off-by: zhong jiang <zhongjiang@huawei.com> >>>> --- >>>> mm/gup.c | 8 +++++--- >>>> 1 file changed, 5 insertions(+), 3 deletions(-) >>>> >>>> diff --git a/mm/gup.c b/mm/gup.c >>>> index 8f236a3..1fe0ceb 100644 >>>> --- a/mm/gup.c >>>> +++ b/mm/gup.c >>>> @@ -1443,6 +1443,7 @@ static long check_and_migrate_cma_pages(struct task_struct *tsk, >>>> bool drain_allow = true; >>>> bool migrate_allow = true; >>>> LIST_HEAD(cma_page_list); >>>> + long ret; >>> Ira pointed out that this needs initialization, see below. >>> >>>> check_again: >>>> for (i = 0; i < nr_pages;) { >>>> @@ -1504,17 +1505,18 @@ static long check_and_migrate_cma_pages(struct task_struct *tsk, >>>> * again migrating any new CMA pages which we failed to isolate >>>> * earlier. >>>> */ >>>> - nr_pages = __get_user_pages_locked(tsk, mm, start, nr_pages, >>>> + ret = __get_user_pages_locked(tsk, mm, start, nr_pages, >>>> pages, vmas, NULL, >>>> gup_flags); >>>> >>>> - if ((nr_pages > 0) && migrate_allow) { >>>> + nr_pages = ret; >>> Although technically correct, it makes me feel odd to see the assignment >>> done from signed to unsigned, *before* checking for >0. And Ira is hinting >>> at the same thing, when he asks if we can return early here. See below... >>> >>>> + if ((ret > 0) && migrate_allow) { >>>> drain_allow = true; >>>> goto check_again; >>>> } >>>> } >>>> >>>> - return nr_pages; >>>> + return ret; >>>> } >>>> #else >>>> static long check_and_migrate_cma_pages(struct task_struct *tsk, >>>> >>> So, overall, I'd recommend this, instead: >>> >>> diff --git a/mm/gup.c b/mm/gup.c >>> index 23a9f9c9d377..72bc027037fa 100644 >>> --- a/mm/gup.c >>> +++ b/mm/gup.c >>> @@ -1443,6 +1443,7 @@ static long check_and_migrate_cma_pages(struct task_struct *tsk, >>> bool drain_allow = true; >>> bool migrate_allow = true; >>> LIST_HEAD(cma_page_list); >>> + long ret = nr_pages; >>> >> I think the ret should be assigned as zero. we will return ret if cma_page_list is empty. >> I miss something? > That wasn't the behavior before. Before if the cma_page_list was empty > nr_pages would not be modified from what what passed. I believe that > is the intended behavior for this since the cma_page_list seems like a > workaround to migrate out any CMA pages if present in the longterm > mapping. If there are no CMA pages the return result should be > nr_pages. Thanks for your clarification. Sincerely, zhong jiang > Thanks. > > - Alex > > > . > ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] mm: Unsigned 'nr_pages' always larger than zero 2019-10-17 20:42 ` John Hubbard 2019-10-18 2:15 ` zhong jiang 2019-10-18 14:00 ` zhong jiang @ 2019-10-21 12:57 ` Vlastimil Babka 2019-10-21 13:47 ` zhong jiang 2 siblings, 1 reply; 26+ messages in thread From: Vlastimil Babka @ 2019-10-21 12:57 UTC (permalink / raw) To: John Hubbard, zhong jiang, akpm; +Cc: linux-mm On 10/17/19 10:42 PM, John Hubbard wrote: > On 10/16/19 8:19 PM, zhong jiang wrote: >> With the help of unsigned_lesser_than_zero.cocci. Unsigned 'nr_pages"' >> compare with zero. And __get_user_pages_locked will return an long value. >> >> The patch use a new local variable to store the return value of >> __get_user_pages_locked(). Then use it to compare with zero. > > Hi Zhong, > > The above are actually more like notes to yourself, and while those are > good to have, but it's not yet a perfect commit description: it talks > about how you got here, which is only part of the story. A higher level > summary is better, and let the code itself cover the details. > > Like this: > > First line (subject): > > mm/gup: allow CMA migration to propagate errors back to caller > > Commit description: > > check_and_migrate_cma_pages() was recording the result of > __get_user_pages_locked() in an unsigned "nr_pages" variable. Because > __get_user_pages_locked() returns a signed value that can include > negative errno values, this had the effect of hiding errors. > > Change check_and_migrate_cma_pages() implementation so that it > uses a signed variable instead, and propagates the results back > to the caller just as other gup internal functions do. > > This was discovered with the help of unsigned_lesser_than_zero.cocci. Agreed. > >> >> Suggested-by: Andrew Morton <akpm@linux-foundation.org> >> Signed-off-by: zhong jiang <zhongjiang@huawei.com> >> --- >> mm/gup.c | 8 +++++--- >> 1 file changed, 5 insertions(+), 3 deletions(-) >> >> diff --git a/mm/gup.c b/mm/gup.c >> index 8f236a3..1fe0ceb 100644 >> --- a/mm/gup.c >> +++ b/mm/gup.c >> @@ -1443,6 +1443,7 @@ static long check_and_migrate_cma_pages(struct task_struct *tsk, >> bool drain_allow = true; >> bool migrate_allow = true; >> LIST_HEAD(cma_page_list); >> + long ret; > > Ira pointed out that this needs initialization, see below. > >> >> check_again: >> for (i = 0; i < nr_pages;) { >> @@ -1504,17 +1505,18 @@ static long check_and_migrate_cma_pages(struct task_struct *tsk, >> * again migrating any new CMA pages which we failed to isolate >> * earlier. >> */ >> - nr_pages = __get_user_pages_locked(tsk, mm, start, nr_pages, >> + ret = __get_user_pages_locked(tsk, mm, start, nr_pages, >> pages, vmas, NULL, >> gup_flags); >> >> - if ((nr_pages > 0) && migrate_allow) { >> + nr_pages = ret; > > Although technically correct, it makes me feel odd to see the assignment > done from signed to unsigned, *before* checking for >0. And Ira is hinting > at the same thing, when he asks if we can return early here. See below... > >> + if ((ret > 0) && migrate_allow) { >> drain_allow = true; >> goto check_again; >> } >> } >> >> - return nr_pages; >> + return ret; >> } >> #else >> static long check_and_migrate_cma_pages(struct task_struct *tsk, >> > > So, overall, I'd recommend this, instead: Also agreed. Zhong, can you resend it like that? > diff --git a/mm/gup.c b/mm/gup.c > index 23a9f9c9d377..72bc027037fa 100644 > --- a/mm/gup.c > +++ b/mm/gup.c > @@ -1443,6 +1443,7 @@ static long check_and_migrate_cma_pages(struct task_struct *tsk, > bool drain_allow = true; > bool migrate_allow = true; > LIST_HEAD(cma_page_list); > + long ret = nr_pages; > > check_again: > for (i = 0; i < nr_pages;) { > @@ -1504,17 +1505,18 @@ static long check_and_migrate_cma_pages(struct task_struct *tsk, > * again migrating any new CMA pages which we failed to isolate > * earlier. > */ > - nr_pages = __get_user_pages_locked(tsk, mm, start, nr_pages, > + ret = __get_user_pages_locked(tsk, mm, start, nr_pages, > pages, vmas, NULL, > gup_flags); > > - if ((nr_pages > 0) && migrate_allow) { > + if ((ret > 0) && migrate_allow) { > + nr_pages = ret; > drain_allow = true; > goto check_again; > } > } > > - return nr_pages; > + return ret; > } > #else > static long check_and_migrate_cma_pages(struct task_struct *tsk, > > > thanks, > > John Hubbard > NVIDIA > ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] mm: Unsigned 'nr_pages' always larger than zero 2019-10-21 12:57 ` Vlastimil Babka @ 2019-10-21 13:47 ` zhong jiang 0 siblings, 0 replies; 26+ messages in thread From: zhong jiang @ 2019-10-21 13:47 UTC (permalink / raw) To: Vlastimil Babka; +Cc: John Hubbard, akpm, linux-mm On 2019/10/21 20:57, Vlastimil Babka wrote: > On 10/17/19 10:42 PM, John Hubbard wrote: >> On 10/16/19 8:19 PM, zhong jiang wrote: >>> With the help of unsigned_lesser_than_zero.cocci. Unsigned 'nr_pages"' >>> compare with zero. And __get_user_pages_locked will return an long value. >>> >>> The patch use a new local variable to store the return value of >>> __get_user_pages_locked(). Then use it to compare with zero. >> Hi Zhong, >> >> The above are actually more like notes to yourself, and while those are >> good to have, but it's not yet a perfect commit description: it talks >> about how you got here, which is only part of the story. A higher level >> summary is better, and let the code itself cover the details. >> >> Like this: >> >> First line (subject): >> >> mm/gup: allow CMA migration to propagate errors back to caller >> >> Commit description: >> >> check_and_migrate_cma_pages() was recording the result of >> __get_user_pages_locked() in an unsigned "nr_pages" variable. Because >> __get_user_pages_locked() returns a signed value that can include >> negative errno values, this had the effect of hiding errors. >> >> Change check_and_migrate_cma_pages() implementation so that it >> uses a signed variable instead, and propagates the results back >> to the caller just as other gup internal functions do. >> >> This was discovered with the help of unsigned_lesser_than_zero.cocci. > Agreed. > >>> Suggested-by: Andrew Morton <akpm@linux-foundation.org> >>> Signed-off-by: zhong jiang <zhongjiang@huawei.com> >>> --- >>> mm/gup.c | 8 +++++--- >>> 1 file changed, 5 insertions(+), 3 deletions(-) >>> >>> diff --git a/mm/gup.c b/mm/gup.c >>> index 8f236a3..1fe0ceb 100644 >>> --- a/mm/gup.c >>> +++ b/mm/gup.c >>> @@ -1443,6 +1443,7 @@ static long check_and_migrate_cma_pages(struct task_struct *tsk, >>> bool drain_allow = true; >>> bool migrate_allow = true; >>> LIST_HEAD(cma_page_list); >>> + long ret; >> Ira pointed out that this needs initialization, see below. >> >>> >>> check_again: >>> for (i = 0; i < nr_pages;) { >>> @@ -1504,17 +1505,18 @@ static long check_and_migrate_cma_pages(struct task_struct *tsk, >>> * again migrating any new CMA pages which we failed to isolate >>> * earlier. >>> */ >>> - nr_pages = __get_user_pages_locked(tsk, mm, start, nr_pages, >>> + ret = __get_user_pages_locked(tsk, mm, start, nr_pages, >>> pages, vmas, NULL, >>> gup_flags); >>> >>> - if ((nr_pages > 0) && migrate_allow) { >>> + nr_pages = ret; >> Although technically correct, it makes me feel odd to see the assignment >> done from signed to unsigned, *before* checking for >0. And Ira is hinting >> at the same thing, when he asks if we can return early here. See below... >> >>> + if ((ret > 0) && migrate_allow) { >>> drain_allow = true; >>> goto check_again; >>> } >>> } >>> >>> - return nr_pages; >>> + return ret; >>> } >>> #else >>> static long check_and_migrate_cma_pages(struct task_struct *tsk, >>> >> So, overall, I'd recommend this, instead: > Also agreed. Zhong, can you resend it like that? Ok, I will resend it right now. Thanks Sincerely, zhong jiang >> diff --git a/mm/gup.c b/mm/gup.c >> index 23a9f9c9d377..72bc027037fa 100644 >> --- a/mm/gup.c >> +++ b/mm/gup.c >> @@ -1443,6 +1443,7 @@ static long check_and_migrate_cma_pages(struct task_struct *tsk, >> bool drain_allow = true; >> bool migrate_allow = true; >> LIST_HEAD(cma_page_list); >> + long ret = nr_pages; >> >> check_again: >> for (i = 0; i < nr_pages;) { >> @@ -1504,17 +1505,18 @@ static long check_and_migrate_cma_pages(struct task_struct *tsk, >> * again migrating any new CMA pages which we failed to isolate >> * earlier. >> */ >> - nr_pages = __get_user_pages_locked(tsk, mm, start, nr_pages, >> + ret = __get_user_pages_locked(tsk, mm, start, nr_pages, >> pages, vmas, NULL, >> gup_flags); >> >> - if ((nr_pages > 0) && migrate_allow) { >> + if ((ret > 0) && migrate_allow) { >> + nr_pages = ret; >> drain_allow = true; >> goto check_again; >> } >> } >> >> - return nr_pages; >> + return ret; >> } >> #else >> static long check_and_migrate_cma_pages(struct task_struct *tsk, >> >> >> thanks, >> >> John Hubbard >> NVIDIA >> > > . > ^ permalink raw reply [flat|nested] 26+ messages in thread
end of thread, other threads:[~2019-10-21 15:11 UTC | newest] Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-09-04 10:26 [PATCH] mm: Unsigned 'nr_pages' always larger than zero zhong jiang 2019-09-04 11:24 ` Vlastimil Babka 2019-09-04 18:25 ` Weiny, Ira 2019-09-04 18:39 ` Matthew Wilcox 2019-09-04 18:48 ` Andrew Morton 2019-09-04 20:20 ` Weiny, Ira 2019-09-04 20:40 ` Vlastimil Babka 2019-09-05 6:18 ` zhong jiang 2019-09-05 7:19 ` Vlastimil Babka 2019-09-05 6:19 ` John Hubbard 2019-10-16 9:07 ` zhong jiang 2019-10-17 0:49 ` Andrew Morton 2019-10-17 1:38 ` zhong jiang 2019-09-04 19:01 ` Andrew Morton 2019-09-04 20:44 ` Vlastimil Babka 2019-09-05 2:05 ` zhong jiang 2019-10-17 3:19 zhong jiang 2019-10-17 18:01 ` Ira Weiny 2019-10-18 2:01 ` zhong jiang 2019-10-17 20:42 ` John Hubbard 2019-10-18 2:15 ` zhong jiang 2019-10-18 14:00 ` zhong jiang 2019-10-18 16:07 ` Alexander Duyck 2019-10-21 15:11 ` zhong jiang 2019-10-21 12:57 ` Vlastimil Babka 2019-10-21 13:47 ` zhong jiang
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.