| Message ID | 20210203210832.113685-3-peterx@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | mm/hugetlb: Early cow on fork, and a few cleanups | expand |
On 2/3/21 1:08 PM, Peter Xu wrote: > All the regions maintained in hugetlb reserved map is inclusive on "from" but > exclusive on "to". We can break earlier even if rg->from==t because it already > means no possible intersection. > > This does not need a Fixes in all cases because when it happens (rg->from==t) > we'll not break out of the loop while we should, however the next thing we'd do > is still add the last file_region we'd need and quit the loop in the next > round. So this change is not a bugfix (since the old code should still run > okay iiuc), but we'd better still touch it up to make it logically sane. > > Signed-off-by: Peter Xu <peterx@redhat.com> > --- > mm/hugetlb.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/mm/hugetlb.c b/mm/hugetlb.c > index d2859c2aecc9..9e6ea96bf33b 100644 > --- a/mm/hugetlb.c > +++ b/mm/hugetlb.c > @@ -377,7 +377,7 @@ static long add_reservation_in_range(struct resv_map *resv, long f, long t, > /* When we find a region that starts beyond our range, we've > * finished. > */ > - if (rg->from > t) > + if (rg->from >= t) > break; > > /* Add an entry for last_accounted_offset -> rg->from, and > Changing any of this code makes me nervous. However, I agree with your analysis. The change makes the code match the comment WRT the [from, to) nature of regions. Reviewed-by: Mike Kravetz <mike.kravetz@oracle.com>
Hi: On 2021/2/4 5:08, Peter Xu wrote: > All the regions maintained in hugetlb reserved map is inclusive on "from" but > exclusive on "to". We can break earlier even if rg->from==t because it already > means no possible intersection. > > This does not need a Fixes in all cases because when it happens (rg->from==t) > we'll not break out of the loop while we should, however the next thing we'd do > is still add the last file_region we'd need and quit the loop in the next > round. So this change is not a bugfix (since the old code should still run > okay iiuc), but we'd better still touch it up to make it logically sane. > I think the difference is when we handle the rg->from == t case. Previous one is in the loop, now below the loop. But the result should be same. Thanks. Reviewed-by: Miaohe Lin <linmiaohe@huawei.com> > Signed-off-by: Peter Xu <peterx@redhat.com> > --- > mm/hugetlb.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/mm/hugetlb.c b/mm/hugetlb.c > index d2859c2aecc9..9e6ea96bf33b 100644 > --- a/mm/hugetlb.c > +++ b/mm/hugetlb.c > @@ -377,7 +377,7 @@ static long add_reservation_in_range(struct resv_map *resv, long f, long t, > /* When we find a region that starts beyond our range, we've > * finished. > */ > - if (rg->from > t) > + if (rg->from >= t) > break; > > /* Add an entry for last_accounted_offset -> rg->from, and >
diff --git a/mm/hugetlb.c b/mm/hugetlb.c index d2859c2aecc9..9e6ea96bf33b 100644 --- a/mm/hugetlb.c +++ b/mm/hugetlb.c @@ -377,7 +377,7 @@ static long add_reservation_in_range(struct resv_map *resv, long f, long t, /* When we find a region that starts beyond our range, we've * finished. */ - if (rg->from > t) + if (rg->from >= t) break; /* Add an entry for last_accounted_offset -> rg->from, and
All the regions maintained in hugetlb reserved map is inclusive on "from" but exclusive on "to". We can break earlier even if rg->from==t because it already means no possible intersection. This does not need a Fixes in all cases because when it happens (rg->from==t) we'll not break out of the loop while we should, however the next thing we'd do is still add the last file_region we'd need and quit the loop in the next round. So this change is not a bugfix (since the old code should still run okay iiuc), but we'd better still touch it up to make it logically sane. Signed-off-by: Peter Xu <peterx@redhat.com> --- mm/hugetlb.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)