* [PATCH] lib/test_kho: Check if KHO is enabled
@ 2025-11-06 22:06 Pasha Tatashin
2025-11-07 10:24 ` Pratyush Yadav
2025-11-09 7:31 ` Mike Rapoport
0 siblings, 2 replies; 5+ messages in thread
From: Pasha Tatashin @ 2025-11-06 22:06 UTC (permalink / raw)
To: akpm, pasha.tatashin, rppt, graf, linux-kernel, kexec, linux-mm,
pratyush
We must check whether KHO is enabled prior to issuing KHO commands,
otherwise KHO internal data structures are not initialized.
Fixes: b753522bed0b ("kho: add test for kexec handover")
Reported-by: kernel test robot <oliver.sang@intel.com>
Closes: https://lore.kernel.org/oe-lkp/202511061629.e242724-lkp@intel.com
Signed-off-by: Pasha Tatashin <pasha.tatashin@soleen.com>
---
lib/test_kho.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/lib/test_kho.c b/lib/test_kho.c
index 025ea251a186..85b60d87a50a 100644
--- a/lib/test_kho.c
+++ b/lib/test_kho.c
@@ -315,6 +315,9 @@ static int __init kho_test_init(void)
phys_addr_t fdt_phys;
int err;
+ if (!kho_is_enabled())
+ return 0;
+
err = kho_retrieve_subtree(KHO_TEST_FDT, &fdt_phys);
if (!err)
return kho_test_restore(fdt_phys);
--
2.51.2.1041.gc1ab5b90ca-goog
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] lib/test_kho: Check if KHO is enabled
2025-11-06 22:06 [PATCH] lib/test_kho: Check if KHO is enabled Pasha Tatashin
@ 2025-11-07 10:24 ` Pratyush Yadav
2025-11-07 11:15 ` Pasha Tatashin
2025-11-09 7:31 ` Mike Rapoport
1 sibling, 1 reply; 5+ messages in thread
From: Pratyush Yadav @ 2025-11-07 10:24 UTC (permalink / raw)
To: Pasha Tatashin; +Cc: akpm, rppt, graf, linux-kernel, kexec, linux-mm, pratyush
On Thu, Nov 06 2025, Pasha Tatashin wrote:
> We must check whether KHO is enabled prior to issuing KHO commands,
> otherwise KHO internal data structures are not initialized.
Should we have this check in the KHO APIs instead? This check is easy
enough to miss.
>
> Fixes: b753522bed0b ("kho: add test for kexec handover")
>
Nit: these blank lines would probably mess up trailer parsing for
tooling.
> Reported-by: kernel test robot <oliver.sang@intel.com>
> Closes: https://lore.kernel.org/oe-lkp/202511061629.e242724-lkp@intel.com
>
> Signed-off-by: Pasha Tatashin <pasha.tatashin@soleen.com>
> ---
> lib/test_kho.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/lib/test_kho.c b/lib/test_kho.c
> index 025ea251a186..85b60d87a50a 100644
> --- a/lib/test_kho.c
> +++ b/lib/test_kho.c
> @@ -315,6 +315,9 @@ static int __init kho_test_init(void)
> phys_addr_t fdt_phys;
> int err;
>
> + if (!kho_is_enabled())
> + return 0;
> +
> err = kho_retrieve_subtree(KHO_TEST_FDT, &fdt_phys);
> if (!err)
> return kho_test_restore(fdt_phys);
--
Regards,
Pratyush Yadav
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] lib/test_kho: Check if KHO is enabled
2025-11-07 10:24 ` Pratyush Yadav
@ 2025-11-07 11:15 ` Pasha Tatashin
2025-11-07 16:07 ` Pratyush Yadav
0 siblings, 1 reply; 5+ messages in thread
From: Pasha Tatashin @ 2025-11-07 11:15 UTC (permalink / raw)
To: Pratyush Yadav; +Cc: akpm, rppt, graf, linux-kernel, kexec, linux-mm
On Fri, Nov 7, 2025 at 5:24 AM Pratyush Yadav <pratyush@kernel.org> wrote:
>
> On Thu, Nov 06 2025, Pasha Tatashin wrote:
>
> > We must check whether KHO is enabled prior to issuing KHO commands,
> > otherwise KHO internal data structures are not initialized.
>
> Should we have this check in the KHO APIs instead? This check is easy
> enough to miss.
I considered adding a kho_is_enabled() check to every KHO API, but it
seems unnecessary.
In-kernel users of KHO, like reserve_mem and the upcoming LUO, are
already expected to check if KHO is enabled before doing extra
preservation work. I anticipate any future in-kernel users will follow
the same pattern.
We could add a WARN_ON(!kho_is_enabled()) to the internal API calls,
but I don't think it's needed. We already catch this condition with
other WARN_ONs, as shown by this report.
>
> >
> > Fixes: b753522bed0b ("kho: add test for kexec handover")
> >
>
> Nit: these blank lines would probably mess up trailer parsing for
> tooling.
Hm, if so, the blank line should be removed.
Thank you,
Pasha
>
> > Reported-by: kernel test robot <oliver.sang@intel.com>
> > Closes: https://lore.kernel.org/oe-lkp/202511061629.e242724-lkp@intel.com
> >
> > Signed-off-by: Pasha Tatashin <pasha.tatashin@soleen.com>
> > ---
> > lib/test_kho.c | 3 +++
> > 1 file changed, 3 insertions(+)
> >
> > diff --git a/lib/test_kho.c b/lib/test_kho.c
> > index 025ea251a186..85b60d87a50a 100644
> > --- a/lib/test_kho.c
> > +++ b/lib/test_kho.c
> > @@ -315,6 +315,9 @@ static int __init kho_test_init(void)
> > phys_addr_t fdt_phys;
> > int err;
> >
> > + if (!kho_is_enabled())
> > + return 0;
> > +
> > err = kho_retrieve_subtree(KHO_TEST_FDT, &fdt_phys);
> > if (!err)
> > return kho_test_restore(fdt_phys);
>
> --
> Regards,
> Pratyush Yadav
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] lib/test_kho: Check if KHO is enabled
2025-11-07 11:15 ` Pasha Tatashin
@ 2025-11-07 16:07 ` Pratyush Yadav
0 siblings, 0 replies; 5+ messages in thread
From: Pratyush Yadav @ 2025-11-07 16:07 UTC (permalink / raw)
To: Pasha Tatashin
Cc: Pratyush Yadav, akpm, rppt, graf, linux-kernel, kexec, linux-mm
On Fri, Nov 07 2025, Pasha Tatashin wrote:
> On Fri, Nov 7, 2025 at 5:24 AM Pratyush Yadav <pratyush@kernel.org> wrote:
>>
>> On Thu, Nov 06 2025, Pasha Tatashin wrote:
>>
>> > We must check whether KHO is enabled prior to issuing KHO commands,
>> > otherwise KHO internal data structures are not initialized.
>>
>> Should we have this check in the KHO APIs instead? This check is easy
>> enough to miss.
>
> I considered adding a kho_is_enabled() check to every KHO API, but it
> seems unnecessary.
>
> In-kernel users of KHO, like reserve_mem and the upcoming LUO, are
> already expected to check if KHO is enabled before doing extra
> preservation work. I anticipate any future in-kernel users will follow
> the same pattern.
Hmm, fair enough. I suppose we can always change this later if it causes
more pain.
Reviewed-by: Pratyush Yadav <pratyush@kernel.org>
[...]
--
Regards,
Pratyush Yadav
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] lib/test_kho: Check if KHO is enabled
2025-11-06 22:06 [PATCH] lib/test_kho: Check if KHO is enabled Pasha Tatashin
2025-11-07 10:24 ` Pratyush Yadav
@ 2025-11-09 7:31 ` Mike Rapoport
1 sibling, 0 replies; 5+ messages in thread
From: Mike Rapoport @ 2025-11-09 7:31 UTC (permalink / raw)
To: Pasha Tatashin; +Cc: akpm, graf, linux-kernel, kexec, linux-mm, pratyush
On Thu, Nov 06, 2025 at 05:06:35PM -0500, Pasha Tatashin wrote:
> We must check whether KHO is enabled prior to issuing KHO commands,
> otherwise KHO internal data structures are not initialized.
>
> Fixes: b753522bed0b ("kho: add test for kexec handover")
>
> Reported-by: kernel test robot <oliver.sang@intel.com>
> Closes: https://lore.kernel.org/oe-lkp/202511061629.e242724-lkp@intel.com
>
> Signed-off-by: Pasha Tatashin <pasha.tatashin@soleen.com>
Reviewed-by: Mike Rapoport (Microsoft) <rppt@kernel.org>
> ---
> lib/test_kho.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/lib/test_kho.c b/lib/test_kho.c
> index 025ea251a186..85b60d87a50a 100644
> --- a/lib/test_kho.c
> +++ b/lib/test_kho.c
> @@ -315,6 +315,9 @@ static int __init kho_test_init(void)
> phys_addr_t fdt_phys;
> int err;
>
> + if (!kho_is_enabled())
> + return 0;
> +
> err = kho_retrieve_subtree(KHO_TEST_FDT, &fdt_phys);
> if (!err)
> return kho_test_restore(fdt_phys);
> --
> 2.51.2.1041.gc1ab5b90ca-goog
>
--
Sincerely yours,
Mike.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2025-11-09 7:31 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-11-06 22:06 [PATCH] lib/test_kho: Check if KHO is enabled Pasha Tatashin
2025-11-07 10:24 ` Pratyush Yadav
2025-11-07 11:15 ` Pasha Tatashin
2025-11-07 16:07 ` Pratyush Yadav
2025-11-09 7:31 ` Mike Rapoport
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).