diff mbox series

of: of_reserved_mem: clean-up reserved memory with no-map

Message ID 20240428125505.434962-1-skseofh@gmail.com
State Superseded
Headers show
Series of: of_reserved_mem: clean-up reserved memory with no-map | expand

Checks

Context Check Description
robh/checkpatch success
robh/patch-applied fail build log

Commit Message

DaeRo Lee April 28, 2024, 12:55 p.m. UTC
From: Daero Lee <daero_le.lee@samsung.com>

In early_init_dt_reserve_memory we only add memory w/o no-map flag to
memblock.reserved. But we need to add memory w/ no-map flag to
memblock.reserved, because NOMAP and memblock.reserved are semantically
different.

Signed-off-by: Daero Lee <daero_le.lee@samsung.com>
---
 drivers/of/of_reserved_mem.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

DaeRo Lee April 28, 2024, 1:10 p.m. UTC | #1
2024년 4월 28일 (일) 오후 9:55, <skseofh@gmail.com>님이 작성:
>
> From: Daero Lee <daero_le.lee@samsung.com>
>
> In early_init_dt_reserve_memory we only add memory w/o no-map flag to
> memblock.reserved. But we need to add memory w/ no-map flag to
> memblock.reserved, because NOMAP and memblock.reserved are semantically
> different.
>
> Signed-off-by: Daero Lee <daero_le.lee@samsung.com>
> ---
>  drivers/of/of_reserved_mem.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/of/of_reserved_mem.c b/drivers/of/of_reserved_mem.c
> index 8236ecae2953..1c916da8adaf 100644
> --- a/drivers/of/of_reserved_mem.c
> +++ b/drivers/of/of_reserved_mem.c
> @@ -91,7 +91,8 @@ static int __init early_init_dt_reserve_memory(phys_addr_t base,
>                     memblock_is_region_reserved(base, size))
>                         return -EBUSY;
>
> -               return memblock_mark_nomap(base, size);
> +               if (memblock_mark_nomap(base, size))
> +                       return;
Sorry. The return value is wrong.

Here is what I want to do:

--- a/drivers/of/of_reserved_mem.c
+++ b/drivers/of/of_reserved_mem.c
@@ -81,6 +81,7 @@ static void __init
fdt_reserved_mem_save_node(unsigned long node, const char *un
 static int __init early_init_dt_reserve_memory(phys_addr_t base,
                                               phys_addr_t size, bool nomap)
 {
+       int err = 0;
        if (nomap) {
                /*
                 * If the memory is already reserved (by another region), we
@@ -91,7 +92,10 @@ static int __init
early_init_dt_reserve_memory(phys_addr_t base,
                    memblock_is_region_reserved(base, size))
                        return -EBUSY;

-               return memblock_mark_nomap(base, size);
+
+               err = memblock_mark_nomap(base, size);
+               if (err)
+                       return err;
        }
        return memblock_reserve(base, size);
 }


Regards,
DaeRo Lee
Mike Rapoport April 29, 2024, 2:14 p.m. UTC | #2
On Sun, Apr 28, 2024 at 10:10:17PM +0900, DaeRo Lee wrote:
> 2024년 4월 28일 (일) 오후 9:55, <skseofh@gmail.com>님이 작성:
> >
> > From: Daero Lee <daero_le.lee@samsung.com>
> >
> > In early_init_dt_reserve_memory we only add memory w/o no-map flag to
> > memblock.reserved. But we need to add memory w/ no-map flag to
> > memblock.reserved, because NOMAP and memblock.reserved are semantically
> > different.
> >
> > Signed-off-by: Daero Lee <daero_le.lee@samsung.com>
> > ---
> >  drivers/of/of_reserved_mem.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/of/of_reserved_mem.c b/drivers/of/of_reserved_mem.c
> > index 8236ecae2953..1c916da8adaf 100644
> > --- a/drivers/of/of_reserved_mem.c
> > +++ b/drivers/of/of_reserved_mem.c
> > @@ -91,7 +91,8 @@ static int __init early_init_dt_reserve_memory(phys_addr_t base,
> >                     memblock_is_region_reserved(base, size))
> >                         return -EBUSY;
> >
> > -               return memblock_mark_nomap(base, size);
> > +               if (memblock_mark_nomap(base, size))
> > +                       return;
> Sorry. The return value is wrong.
> 
> Here is what I want to do:
> 
> --- a/drivers/of/of_reserved_mem.c
> +++ b/drivers/of/of_reserved_mem.c
> @@ -81,6 +81,7 @@ static void __init
> fdt_reserved_mem_save_node(unsigned long node, const char *un
>  static int __init early_init_dt_reserve_memory(phys_addr_t base,
>                                                phys_addr_t size, bool nomap)
>  {
> +       int err = 0;
>         if (nomap) {
>                 /*
>                  * If the memory is already reserved (by another region), we
> @@ -91,7 +92,10 @@ static int __init
> early_init_dt_reserve_memory(phys_addr_t base,
>                     memblock_is_region_reserved(base, size))
>                         return -EBUSY;
> 
> -               return memblock_mark_nomap(base, size);
> +
> +               err = memblock_mark_nomap(base, size);
> +               if (err)
> +                       return err;
>         }
>         return memblock_reserve(base, size);
>  }

Makes sense to me.

> Regards,
> DaeRo Lee
kernel test robot May 1, 2024, 1:39 a.m. UTC | #3
Hi,

kernel test robot noticed the following build warnings:

[auto build test WARNING on robh/for-next]
[also build test WARNING on linus/master v6.9-rc6 next-20240430]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/skseofh-gmail-com/of-of_reserved_mem-clean-up-reserved-memory-with-no-map/20240430-144643
base:   https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
patch link:    https://lore.kernel.org/r/20240428125505.434962-1-skseofh%40gmail.com
patch subject: [PATCH] of: of_reserved_mem: clean-up reserved memory with no-map
config: openrisc-allnoconfig (https://download.01.org/0day-ci/archive/20240501/202405010907.PHM9xSMi-lkp@intel.com/config)
compiler: or1k-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240501/202405010907.PHM9xSMi-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202405010907.PHM9xSMi-lkp@intel.com/

All warnings (new ones prefixed by >>):

   drivers/of/of_reserved_mem.c: In function 'early_init_dt_reserve_memory':
>> drivers/of/of_reserved_mem.c:95:25: warning: 'return' with no value, in function returning non-void [-Wreturn-type]
      95 |                         return;
         |                         ^~~~~~
   drivers/of/of_reserved_mem.c:81:19: note: declared here
      81 | static int __init early_init_dt_reserve_memory(phys_addr_t base,
         |                   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~


vim +/return +95 drivers/of/of_reserved_mem.c

    80	
    81	static int __init early_init_dt_reserve_memory(phys_addr_t base,
    82						       phys_addr_t size, bool nomap)
    83	{
    84		if (nomap) {
    85			/*
    86			 * If the memory is already reserved (by another region), we
    87			 * should not allow it to be marked nomap, but don't worry
    88			 * if the region isn't memory as it won't be mapped.
    89			 */
    90			if (memblock_overlaps_region(&memblock.memory, base, size) &&
    91			    memblock_is_region_reserved(base, size))
    92				return -EBUSY;
    93	
    94			if (memblock_mark_nomap(base, size))
  > 95				return;
    96		}
    97		return memblock_reserve(base, size);
    98	}
    99
kernel test robot May 1, 2024, 4:59 a.m. UTC | #4
Hi,

kernel test robot noticed the following build warnings:

[auto build test WARNING on robh/for-next]
[also build test WARNING on linus/master v6.9-rc6 next-20240430]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/skseofh-gmail-com/of-of_reserved_mem-clean-up-reserved-memory-with-no-map/20240430-144643
base:   https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
patch link:    https://lore.kernel.org/r/20240428125505.434962-1-skseofh%40gmail.com
patch subject: [PATCH] of: of_reserved_mem: clean-up reserved memory with no-map
config: arm-defconfig (https://download.01.org/0day-ci/archive/20240501/202405011208.qsZQwChO-lkp@intel.com/config)
compiler: clang version 14.0.6 (https://github.com/llvm/llvm-project.git f28c006a5895fc0e329fe15fead81e37457cb1d1)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240501/202405011208.qsZQwChO-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202405011208.qsZQwChO-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> drivers/of/of_reserved_mem.c:95:4: warning: non-void function 'early_init_dt_reserve_memory' should return a value [-Wreturn-type]
                           return;
                           ^
   1 warning generated.


vim +/early_init_dt_reserve_memory +95 drivers/of/of_reserved_mem.c

    80	
    81	static int __init early_init_dt_reserve_memory(phys_addr_t base,
    82						       phys_addr_t size, bool nomap)
    83	{
    84		if (nomap) {
    85			/*
    86			 * If the memory is already reserved (by another region), we
    87			 * should not allow it to be marked nomap, but don't worry
    88			 * if the region isn't memory as it won't be mapped.
    89			 */
    90			if (memblock_overlaps_region(&memblock.memory, base, size) &&
    91			    memblock_is_region_reserved(base, size))
    92				return -EBUSY;
    93	
    94			if (memblock_mark_nomap(base, size))
  > 95				return;
    96		}
    97		return memblock_reserve(base, size);
    98	}
    99
diff mbox series

Patch

diff --git a/drivers/of/of_reserved_mem.c b/drivers/of/of_reserved_mem.c
index 8236ecae2953..1c916da8adaf 100644
--- a/drivers/of/of_reserved_mem.c
+++ b/drivers/of/of_reserved_mem.c
@@ -91,7 +91,8 @@  static int __init early_init_dt_reserve_memory(phys_addr_t base,
 		    memblock_is_region_reserved(base, size))
 			return -EBUSY;
 
-		return memblock_mark_nomap(base, size);
+		if (memblock_mark_nomap(base, size))
+			return;
 	}
 	return memblock_reserve(base, size);
 }