| Message ID | 20220601124417.2872001-3-chenwandun@huawei.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | a few cleanup and bugfixes about shmem | expand |
On 01.06.22 14:44, Chen Wandun wrote: > If addr is not PAGE_SIZE aligned, return -EINVAL directly. Why is this one to be treated in a special way compared to all of the other related checks? > Besides, use macro offset_in_page to check addr is not > PAGE_SIZE aligned case. Using offset_in_page() LGTM. > > Signed-off-by: Chen Wandun <chenwandun@huawei.com> > --- > mm/shmem.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/mm/shmem.c b/mm/shmem.c > index 80c361c3d82c..1136dd7da9e5 100644 > --- a/mm/shmem.c > +++ b/mm/shmem.c > @@ -2143,8 +2143,8 @@ unsigned long shmem_get_unmapped_area(struct file *file, > return addr; > if (IS_ERR_VALUE(addr)) > return addr; > - if (addr & ~PAGE_MASK) > - return addr; > + if (offset_in_page(addr)) > + return -EINVAL; > if (addr > TASK_SIZE - len) > return addr; > > @@ -2197,7 +2197,7 @@ unsigned long shmem_get_unmapped_area(struct file *file, > inflated_addr = get_area(NULL, uaddr, inflated_len, 0, flags); > if (IS_ERR_VALUE(inflated_addr)) > return addr; > - if (inflated_addr & ~PAGE_MASK) > + if (offset_in_page(inflated_addr)) > return addr; > > inflated_offset = inflated_addr & (HPAGE_PMD_SIZE-1);
On 01.06.22 15:37, David Hildenbrand wrote: > On 01.06.22 14:44, Chen Wandun wrote: >> If addr is not PAGE_SIZE aligned, return -EINVAL directly. > > Why is this one to be treated in a special way compared to all of the > other related checks? Ah, I see you modify other places in other patches. Maybe just combine all these return value changes into a single patch? That makes it look less "special".
On 2022/6/1 22:14, David Hildenbrand wrote: > On 01.06.22 15:37, David Hildenbrand wrote: >> On 01.06.22 14:44, Chen Wandun wrote: >>> If addr is not PAGE_SIZE aligned, return -EINVAL directly. >> Why is this one to be treated in a special way compared to all of the >> other related checks? > Ah, I see you modify other places in other patches. Maybe just combine > all these return value changes into a single patch? That makes it look > less "special". OK, patch 2 and patch 3 are used to return error value directly, I will combine these patch to a single patch in next version. Thanks. > > >
diff --git a/mm/shmem.c b/mm/shmem.c index 80c361c3d82c..1136dd7da9e5 100644 --- a/mm/shmem.c +++ b/mm/shmem.c @@ -2143,8 +2143,8 @@ unsigned long shmem_get_unmapped_area(struct file *file, return addr; if (IS_ERR_VALUE(addr)) return addr; - if (addr & ~PAGE_MASK) - return addr; + if (offset_in_page(addr)) + return -EINVAL; if (addr > TASK_SIZE - len) return addr; @@ -2197,7 +2197,7 @@ unsigned long shmem_get_unmapped_area(struct file *file, inflated_addr = get_area(NULL, uaddr, inflated_len, 0, flags); if (IS_ERR_VALUE(inflated_addr)) return addr; - if (inflated_addr & ~PAGE_MASK) + if (offset_in_page(inflated_addr)) return addr; inflated_offset = inflated_addr & (HPAGE_PMD_SIZE-1);
If addr is not PAGE_SIZE aligned, return -EINVAL directly. Besides, use macro offset_in_page to check addr is not PAGE_SIZE aligned case. Signed-off-by: Chen Wandun <chenwandun@huawei.com> --- mm/shmem.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)