* [PATCH 0/3] s390/vmem: minor cleanup
@ 2020-11-10 9:36 Alexander Gordeev
2020-11-10 9:36 ` [PATCH 1/3] s390/vmem: remove redundant check Alexander Gordeev
` (2 more replies)
0 siblings, 3 replies; 10+ messages in thread
From: Alexander Gordeev @ 2020-11-10 9:36 UTC (permalink / raw)
To: linux-s390, David Hildenbrand; +Cc: Alexander Gordeev, Heiko Carstens
Hi David,
Nothing really fancy here, just couple of cleanups
Alexander Gordeev (3):
s390/vmem: remove redundant check
s390/vmem: fix possible memory overwrite
s390/vmem: make variable and function names consistent
arch/s390/mm/vmem.c | 42 ++++++++++++++++++++++--------------------
1 file changed, 22 insertions(+), 20 deletions(-)
--
2.26.0
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 1/3] s390/vmem: remove redundant check
2020-11-10 9:36 [PATCH 0/3] s390/vmem: minor cleanup Alexander Gordeev
@ 2020-11-10 9:36 ` Alexander Gordeev
2020-11-10 9:41 ` David Hildenbrand
2020-11-17 18:43 ` Heiko Carstens
2020-11-10 9:36 ` [PATCH 2/3] s390/vmem: fix possible memory overwrite Alexander Gordeev
2020-11-10 9:36 ` [PATCH 3/3] s390/vmem: make variable and function names consistent Alexander Gordeev
2 siblings, 2 replies; 10+ messages in thread
From: Alexander Gordeev @ 2020-11-10 9:36 UTC (permalink / raw)
To: linux-s390, David Hildenbrand; +Cc: Alexander Gordeev, Heiko Carstens
Signed-off-by: Alexander Gordeev <agordeev@linux.ibm.com>
---
arch/s390/mm/vmem.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/s390/mm/vmem.c b/arch/s390/mm/vmem.c
index b239f2ba93b09..56ab9bb770f3a 100644
--- a/arch/s390/mm/vmem.c
+++ b/arch/s390/mm/vmem.c
@@ -223,7 +223,7 @@ static int __ref modify_pmd_table(pud_t *pud, unsigned long addr,
if (!add) {
if (pmd_none(*pmd))
continue;
- if (pmd_large(*pmd) && !add) {
+ if (pmd_large(*pmd)) {
if (IS_ALIGNED(addr, PMD_SIZE) &&
IS_ALIGNED(next, PMD_SIZE)) {
if (!direct)
--
2.26.0
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 2/3] s390/vmem: fix possible memory overwrite
2020-11-10 9:36 [PATCH 0/3] s390/vmem: minor cleanup Alexander Gordeev
2020-11-10 9:36 ` [PATCH 1/3] s390/vmem: remove redundant check Alexander Gordeev
@ 2020-11-10 9:36 ` Alexander Gordeev
2020-11-10 9:41 ` David Hildenbrand
2020-11-10 9:36 ` [PATCH 3/3] s390/vmem: make variable and function names consistent Alexander Gordeev
2 siblings, 1 reply; 10+ messages in thread
From: Alexander Gordeev @ 2020-11-10 9:36 UTC (permalink / raw)
To: linux-s390, David Hildenbrand; +Cc: Alexander Gordeev, Heiko Carstens
If for whatever reason the sub-PMD region to be used is less
than struct page size (e.g in the future), then it is possible
to overwrite beyond the region size.
Signed-off-by: Alexander Gordeev <agordeev@linux.ibm.com>
---
arch/s390/mm/vmem.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/arch/s390/mm/vmem.c b/arch/s390/mm/vmem.c
index 56ab9bb770f3a..d7f25884061f4 100644
--- a/arch/s390/mm/vmem.c
+++ b/arch/s390/mm/vmem.c
@@ -91,13 +91,15 @@ static void vmemmap_flush_unused_pmd(void)
static void __vmemmap_use_sub_pmd(unsigned long start, unsigned long end)
{
+ unsigned long size = min(end - start, sizeof(struct page));
+
/*
* As we expect to add in the same granularity as we remove, it's
* sufficient to mark only some piece used to block the memmap page from
* getting removed (just in case the memmap never gets initialized,
* e.g., because the memory block never gets onlined).
*/
- memset(__va(start), 0, sizeof(struct page));
+ memset(__va(start), 0, size);
}
static void vmemmap_use_sub_pmd(unsigned long start, unsigned long end)
--
2.26.0
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 3/3] s390/vmem: make variable and function names consistent
2020-11-10 9:36 [PATCH 0/3] s390/vmem: minor cleanup Alexander Gordeev
2020-11-10 9:36 ` [PATCH 1/3] s390/vmem: remove redundant check Alexander Gordeev
2020-11-10 9:36 ` [PATCH 2/3] s390/vmem: fix possible memory overwrite Alexander Gordeev
@ 2020-11-10 9:36 ` Alexander Gordeev
2020-11-17 18:44 ` Heiko Carstens
2 siblings, 1 reply; 10+ messages in thread
From: Alexander Gordeev @ 2020-11-10 9:36 UTC (permalink / raw)
To: linux-s390, David Hildenbrand; +Cc: Alexander Gordeev, Heiko Carstens
Rename some variable and functions to better clarify
what they are and what they do.
Signed-off-by: Alexander Gordeev <agordeev@linux.ibm.com>
---
arch/s390/mm/vmem.c | 36 ++++++++++++++++++------------------
1 file changed, 18 insertions(+), 18 deletions(-)
diff --git a/arch/s390/mm/vmem.c b/arch/s390/mm/vmem.c
index d7f25884061f4..4bb198db6aa01 100644
--- a/arch/s390/mm/vmem.c
+++ b/arch/s390/mm/vmem.c
@@ -76,20 +76,20 @@ static void vmem_pte_free(unsigned long *table)
/*
* The unused vmemmap range, which was not yet memset(PAGE_UNUSED) ranges
- * from unused_pmd_start to next PMD_SIZE boundary.
+ * from unused_sub_pmd_start to next PMD_SIZE boundary.
*/
-static unsigned long unused_pmd_start;
+static unsigned long unused_sub_pmd_start;
-static void vmemmap_flush_unused_pmd(void)
+static void vmemmap_flush_unused_sub_pmd(void)
{
- if (!unused_pmd_start)
+ if (!unused_sub_pmd_start)
return;
- memset(__va(unused_pmd_start), PAGE_UNUSED,
- ALIGN(unused_pmd_start, PMD_SIZE) - unused_pmd_start);
- unused_pmd_start = 0;
+ memset(__va(unused_sub_pmd_start), PAGE_UNUSED,
+ ALIGN(unused_sub_pmd_start, PMD_SIZE) - unused_sub_pmd_start);
+ unused_sub_pmd_start = 0;
}
-static void __vmemmap_use_sub_pmd(unsigned long start, unsigned long end)
+static void vmemmap_mark_sub_pmd_used(unsigned long start, unsigned long end)
{
unsigned long size = min(end - start, sizeof(struct page));
@@ -108,24 +108,24 @@ static void vmemmap_use_sub_pmd(unsigned long start, unsigned long end)
* We only optimize if the new used range directly follows the
* previously unused range (esp., when populating consecutive sections).
*/
- if (unused_pmd_start == start) {
- unused_pmd_start = end;
- if (likely(IS_ALIGNED(unused_pmd_start, PMD_SIZE)))
- unused_pmd_start = 0;
+ if (unused_sub_pmd_start == start) {
+ unused_sub_pmd_start = end;
+ if (likely(IS_ALIGNED(unused_sub_pmd_start, PMD_SIZE)))
+ unused_sub_pmd_start = 0;
return;
}
- vmemmap_flush_unused_pmd();
- __vmemmap_use_sub_pmd(start, end);
+ vmemmap_flush_unused_sub_pmd();
+ vmemmap_mark_sub_pmd_used(start, end);
}
static void vmemmap_use_new_sub_pmd(unsigned long start, unsigned long end)
{
void *page = __va(ALIGN_DOWN(start, PMD_SIZE));
- vmemmap_flush_unused_pmd();
+ vmemmap_flush_unused_sub_pmd();
/* Could be our memmap page is filled with PAGE_UNUSED already ... */
- __vmemmap_use_sub_pmd(start, end);
+ vmemmap_mark_sub_pmd_used(start, end);
/* Mark the unused parts of the new memmap page PAGE_UNUSED. */
if (!IS_ALIGNED(start, PMD_SIZE))
@@ -136,7 +136,7 @@ static void vmemmap_use_new_sub_pmd(unsigned long start, unsigned long end)
* unused range in the populated PMD.
*/
if (!IS_ALIGNED(end, PMD_SIZE))
- unused_pmd_start = end;
+ unused_sub_pmd_start = end;
}
/* Returns true if the PMD is completely unused and can be freed. */
@@ -144,7 +144,7 @@ static bool vmemmap_unuse_sub_pmd(unsigned long start, unsigned long end)
{
void *page = __va(ALIGN_DOWN(start, PMD_SIZE));
- vmemmap_flush_unused_pmd();
+ vmemmap_flush_unused_sub_pmd();
memset(__va(start), PAGE_UNUSED, end - start);
return !memchr_inv(page, PAGE_UNUSED, PMD_SIZE);
}
--
2.26.0
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 2/3] s390/vmem: fix possible memory overwrite
2020-11-10 9:36 ` [PATCH 2/3] s390/vmem: fix possible memory overwrite Alexander Gordeev
@ 2020-11-10 9:41 ` David Hildenbrand
2020-11-10 10:50 ` Alexander Gordeev
0 siblings, 1 reply; 10+ messages in thread
From: David Hildenbrand @ 2020-11-10 9:41 UTC (permalink / raw)
To: Alexander Gordeev, linux-s390; +Cc: Heiko Carstens
On 10.11.20 10:36, Alexander Gordeev wrote:
> If for whatever reason the sub-PMD region to be used is less
> than struct page size (e.g in the future), then it is possible
> to overwrite beyond the region size.
>
> Signed-off-by: Alexander Gordeev <agordeev@linux.ibm.com>
> ---
> arch/s390/mm/vmem.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/arch/s390/mm/vmem.c b/arch/s390/mm/vmem.c
> index 56ab9bb770f3a..d7f25884061f4 100644
> --- a/arch/s390/mm/vmem.c
> +++ b/arch/s390/mm/vmem.c
> @@ -91,13 +91,15 @@ static void vmemmap_flush_unused_pmd(void)
>
> static void __vmemmap_use_sub_pmd(unsigned long start, unsigned long end)
> {
> + unsigned long size = min(end - start, sizeof(struct page));
> +
> /*
> * As we expect to add in the same granularity as we remove, it's
> * sufficient to mark only some piece used to block the memmap page from
> * getting removed (just in case the memmap never gets initialized,
> * e.g., because the memory block never gets onlined).
> */
> - memset(__va(start), 0, sizeof(struct page));
> + memset(__va(start), 0, size);
> }
>
> static void vmemmap_use_sub_pmd(unsigned long start, unsigned long end)
>
I don't really see a need for that. Can you spell out one possible
configuration that would trigger that in the future? It's sounds very
unlikely and I have the feeling there might be more to change at other
points.
--
Thanks,
David / dhildenb
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/3] s390/vmem: remove redundant check
2020-11-10 9:36 ` [PATCH 1/3] s390/vmem: remove redundant check Alexander Gordeev
@ 2020-11-10 9:41 ` David Hildenbrand
2020-11-17 18:43 ` Heiko Carstens
1 sibling, 0 replies; 10+ messages in thread
From: David Hildenbrand @ 2020-11-10 9:41 UTC (permalink / raw)
To: Alexander Gordeev, linux-s390; +Cc: Heiko Carstens
On 10.11.20 10:36, Alexander Gordeev wrote:
> Signed-off-by: Alexander Gordeev <agordeev@linux.ibm.com>
> ---
> arch/s390/mm/vmem.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/s390/mm/vmem.c b/arch/s390/mm/vmem.c
> index b239f2ba93b09..56ab9bb770f3a 100644
> --- a/arch/s390/mm/vmem.c
> +++ b/arch/s390/mm/vmem.c
> @@ -223,7 +223,7 @@ static int __ref modify_pmd_table(pud_t *pud, unsigned long addr,
> if (!add) {
> if (pmd_none(*pmd))
> continue;
> - if (pmd_large(*pmd) && !add) {
> + if (pmd_large(*pmd)) {
> if (IS_ALIGNED(addr, PMD_SIZE) &&
> IS_ALIGNED(next, PMD_SIZE)) {
> if (!direct)
>
Reviewed-by: David Hildenbrand <david@redhat.com>
--
Thanks,
David / dhildenb
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/3] s390/vmem: fix possible memory overwrite
2020-11-10 9:41 ` David Hildenbrand
@ 2020-11-10 10:50 ` Alexander Gordeev
2020-11-17 18:44 ` Heiko Carstens
0 siblings, 1 reply; 10+ messages in thread
From: Alexander Gordeev @ 2020-11-10 10:50 UTC (permalink / raw)
To: David Hildenbrand; +Cc: linux-s390, Heiko Carstens
On Tue, Nov 10, 2020 at 10:41:14AM +0100, David Hildenbrand wrote:
> On 10.11.20 10:36, Alexander Gordeev wrote:
> >If for whatever reason the sub-PMD region to be used is less
> >than struct page size (e.g in the future), then it is possible
> >to overwrite beyond the region size.
> >
> >Signed-off-by: Alexander Gordeev <agordeev@linux.ibm.com>
> >---
> > arch/s390/mm/vmem.c | 4 +++-
> > 1 file changed, 3 insertions(+), 1 deletion(-)
> >
> >diff --git a/arch/s390/mm/vmem.c b/arch/s390/mm/vmem.c
> >index 56ab9bb770f3a..d7f25884061f4 100644
> >--- a/arch/s390/mm/vmem.c
> >+++ b/arch/s390/mm/vmem.c
> >@@ -91,13 +91,15 @@ static void vmemmap_flush_unused_pmd(void)
> > static void __vmemmap_use_sub_pmd(unsigned long start, unsigned long end)
> > {
> >+ unsigned long size = min(end - start, sizeof(struct page));
> >+
> > /*
> > * As we expect to add in the same granularity as we remove, it's
> > * sufficient to mark only some piece used to block the memmap page from
> > * getting removed (just in case the memmap never gets initialized,
> > * e.g., because the memory block never gets onlined).
> > */
> >- memset(__va(start), 0, sizeof(struct page));
> >+ memset(__va(start), 0, size);
> > }
> > static void vmemmap_use_sub_pmd(unsigned long start, unsigned long end)
> >
>
> I don't really see a need for that. Can you spell out one possible
> configuration that would trigger that in the future? It's sounds
> very unlikely and I have the feeling there might be more to change
> at other points.
No configuration in mind. But dependency on struct page is the only
obstacle that prevents the whole thing to become generic (unless I
am missing something). Moreover, the memset() would not be needed
also - just a single non-PAGE_UNUSED word within [start..end) should
be enough.
> --
> Thanks,
>
> David / dhildenb
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/3] s390/vmem: remove redundant check
2020-11-10 9:36 ` [PATCH 1/3] s390/vmem: remove redundant check Alexander Gordeev
2020-11-10 9:41 ` David Hildenbrand
@ 2020-11-17 18:43 ` Heiko Carstens
1 sibling, 0 replies; 10+ messages in thread
From: Heiko Carstens @ 2020-11-17 18:43 UTC (permalink / raw)
To: Alexander Gordeev; +Cc: linux-s390, David Hildenbrand
On Tue, Nov 10, 2020 at 10:36:21AM +0100, Alexander Gordeev wrote:
> Signed-off-by: Alexander Gordeev <agordeev@linux.ibm.com>
> ---
> arch/s390/mm/vmem.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/s390/mm/vmem.c b/arch/s390/mm/vmem.c
> index b239f2ba93b09..56ab9bb770f3a 100644
> --- a/arch/s390/mm/vmem.c
> +++ b/arch/s390/mm/vmem.c
> @@ -223,7 +223,7 @@ static int __ref modify_pmd_table(pud_t *pud, unsigned long addr,
> if (!add) {
> if (pmd_none(*pmd))
> continue;
> - if (pmd_large(*pmd) && !add) {
> + if (pmd_large(*pmd)) {
Applied, thanks!
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/3] s390/vmem: fix possible memory overwrite
2020-11-10 10:50 ` Alexander Gordeev
@ 2020-11-17 18:44 ` Heiko Carstens
0 siblings, 0 replies; 10+ messages in thread
From: Heiko Carstens @ 2020-11-17 18:44 UTC (permalink / raw)
To: Alexander Gordeev; +Cc: David Hildenbrand, linux-s390
On Tue, Nov 10, 2020 at 11:50:52AM +0100, Alexander Gordeev wrote:
> > > static void __vmemmap_use_sub_pmd(unsigned long start, unsigned long end)
> > > {
> > >+ unsigned long size = min(end - start, sizeof(struct page));
> > >+
> > > /*
> > > * As we expect to add in the same granularity as we remove, it's
> > > * sufficient to mark only some piece used to block the memmap page from
> > > * getting removed (just in case the memmap never gets initialized,
> > > * e.g., because the memory block never gets onlined).
> > > */
> > >- memset(__va(start), 0, sizeof(struct page));
> > >+ memset(__va(start), 0, size);
> > > }
> > > static void vmemmap_use_sub_pmd(unsigned long start, unsigned long end)
> > >
> >
> > I don't really see a need for that. Can you spell out one possible
> > configuration that would trigger that in the future? It's sounds
> > very unlikely and I have the feeling there might be more to change
> > at other points.
>
> No configuration in mind. But dependency on struct page is the only
> obstacle that prevents the whole thing to become generic (unless I
> am missing something). Moreover, the memset() would not be needed
> also - just a single non-PAGE_UNUSED word within [start..end) should
> be enough.
Well, I agree with David here - so not applying.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 3/3] s390/vmem: make variable and function names consistent
2020-11-10 9:36 ` [PATCH 3/3] s390/vmem: make variable and function names consistent Alexander Gordeev
@ 2020-11-17 18:44 ` Heiko Carstens
0 siblings, 0 replies; 10+ messages in thread
From: Heiko Carstens @ 2020-11-17 18:44 UTC (permalink / raw)
To: Alexander Gordeev; +Cc: linux-s390, David Hildenbrand
On Tue, Nov 10, 2020 at 10:36:23AM +0100, Alexander Gordeev wrote:
> Rename some variable and functions to better clarify
> what they are and what they do.
>
> Signed-off-by: Alexander Gordeev <agordeev@linux.ibm.com>
> ---
> arch/s390/mm/vmem.c | 36 ++++++++++++++++++------------------
> 1 file changed, 18 insertions(+), 18 deletions(-)
Applied, thanks.
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2020-11-17 18:44 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-10 9:36 [PATCH 0/3] s390/vmem: minor cleanup Alexander Gordeev
2020-11-10 9:36 ` [PATCH 1/3] s390/vmem: remove redundant check Alexander Gordeev
2020-11-10 9:41 ` David Hildenbrand
2020-11-17 18:43 ` Heiko Carstens
2020-11-10 9:36 ` [PATCH 2/3] s390/vmem: fix possible memory overwrite Alexander Gordeev
2020-11-10 9:41 ` David Hildenbrand
2020-11-10 10:50 ` Alexander Gordeev
2020-11-17 18:44 ` Heiko Carstens
2020-11-10 9:36 ` [PATCH 3/3] s390/vmem: make variable and function names consistent Alexander Gordeev
2020-11-17 18:44 ` Heiko Carstens
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.