* Re: [PATCH] jailhouse: Hold reference returned from of_find_xxx API [not found] <20220915022343.4001331-1-windhl@126.com> @ 2022-09-15 7:00 ` Juergen Gross via Virtualization 2022-09-15 23:29 ` Srivatsa S. Bhat 1 sibling, 0 replies; 10+ messages in thread From: Juergen Gross via Virtualization @ 2022-09-15 7:00 UTC (permalink / raw) To: Liang He, srivatsa, virtualization; +Cc: wangkelin2023 [-- Attachment #1.1.1.1: Type: text/plain, Size: 457 bytes --] On 15.09.22 04:23, Liang He wrote: > In jailhouse_paravirt(), we should hold the reference returned from > of_find_compatible_node() which has increased the refcount and then > call of_node_put() with it when done. > > Fixes: 63338a38db95 ("jailhouse: Provide detection for non-x86 systems") > Signed-off-by: Liang He <windhl@126.com> > Signed-off-by: Kelin Wang <wangkelin2023@163.com> Reviewed-by: Juergen Gross <jgross@suse.com> Juergen [-- Attachment #1.1.1.2: OpenPGP public key --] [-- Type: application/pgp-keys, Size: 3149 bytes --] [-- Attachment #1.2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 495 bytes --] [-- Attachment #2: Type: text/plain, Size: 183 bytes --] _______________________________________________ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] jailhouse: Hold reference returned from of_find_xxx API [not found] <20220915022343.4001331-1-windhl@126.com> @ 2022-09-15 23:29 ` Srivatsa S. Bhat 2022-09-15 23:29 ` Srivatsa S. Bhat 1 sibling, 0 replies; 10+ messages in thread From: Srivatsa S. Bhat @ 2022-09-15 23:29 UTC (permalink / raw) To: Liang He, jgross, virtualization Cc: jailhouse-dev, mark.rutland, jan.kiszka, andy.shevchenko, robh+dt, wangkelin2023, linux-pci, Bjorn Helgaas, Thomas Gleixner [ Adding author and reviewers of commit 63338a38db95 ] On 9/14/22 7:23 PM, Liang He wrote: > In jailhouse_paravirt(), we should hold the reference returned from > of_find_compatible_node() which has increased the refcount and then > call of_node_put() with it when done. > > Fixes: 63338a38db95 ("jailhouse: Provide detection for non-x86 systems") > Signed-off-by: Liang He <windhl@126.com> > Signed-off-by: Kelin Wang <wangkelin2023@163.com> > --- > include/linux/hypervisor.h | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/include/linux/hypervisor.h b/include/linux/hypervisor.h > index 9efbc54e35e5..7fe1e8c6211c 100644 > --- a/include/linux/hypervisor.h > +++ b/include/linux/hypervisor.h > @@ -27,7 +27,11 @@ static inline void hypervisor_pin_vcpu(int cpu) > > static inline bool jailhouse_paravirt(void) > { > - return of_find_compatible_node(NULL, NULL, "jailhouse,cell"); > + struct device_node *np = of_find_compatible_node(NULL, NULL, "jailhouse,cell"); > + > + of_node_put(np); > + > + return np; > } > Thank you for the fix, but returning a pointer from a function with a bool return type looks odd. Can we also fix that up please? Regards, Srivatsa VMware Photon OS _______________________________________________ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] jailhouse: Hold reference returned from of_find_xxx API @ 2022-09-15 23:29 ` Srivatsa S. Bhat 0 siblings, 0 replies; 10+ messages in thread From: Srivatsa S. Bhat @ 2022-09-15 23:29 UTC (permalink / raw) To: Liang He, jgross, virtualization Cc: wangkelin2023, jan.kiszka, Thomas Gleixner, jailhouse-dev, mark.rutland, linux-pci, andy.shevchenko, robh+dt, Bjorn Helgaas [ Adding author and reviewers of commit 63338a38db95 ] On 9/14/22 7:23 PM, Liang He wrote: > In jailhouse_paravirt(), we should hold the reference returned from > of_find_compatible_node() which has increased the refcount and then > call of_node_put() with it when done. > > Fixes: 63338a38db95 ("jailhouse: Provide detection for non-x86 systems") > Signed-off-by: Liang He <windhl@126.com> > Signed-off-by: Kelin Wang <wangkelin2023@163.com> > --- > include/linux/hypervisor.h | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/include/linux/hypervisor.h b/include/linux/hypervisor.h > index 9efbc54e35e5..7fe1e8c6211c 100644 > --- a/include/linux/hypervisor.h > +++ b/include/linux/hypervisor.h > @@ -27,7 +27,11 @@ static inline void hypervisor_pin_vcpu(int cpu) > > static inline bool jailhouse_paravirt(void) > { > - return of_find_compatible_node(NULL, NULL, "jailhouse,cell"); > + struct device_node *np = of_find_compatible_node(NULL, NULL, "jailhouse,cell"); > + > + of_node_put(np); > + > + return np; > } > Thank you for the fix, but returning a pointer from a function with a bool return type looks odd. Can we also fix that up please? Regards, Srivatsa VMware Photon OS ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re:Re: [PATCH] jailhouse: Hold reference returned from of_find_xxx API 2022-09-15 23:29 ` Srivatsa S. Bhat (?) @ 2022-09-16 2:00 ` Liang He 2022-09-16 5:38 ` Andy Shevchenko -1 siblings, 1 reply; 10+ messages in thread From: Liang He @ 2022-09-16 2:00 UTC (permalink / raw) To: Srivatsa S. Bhat Cc: jgross, virtualization, wangkelin2023, jan.kiszka, Thomas Gleixner, jailhouse-dev, mark.rutland, linux-pci, andy.shevchenko, robh+dt, Bjorn Helgaas At 2022-09-16 07:29:06, "Srivatsa S. Bhat" <srivatsa@csail.mit.edu> wrote: > >[ Adding author and reviewers of commit 63338a38db95 ] > >On 9/14/22 7:23 PM, Liang He wrote: >> In jailhouse_paravirt(), we should hold the reference returned from >> of_find_compatible_node() which has increased the refcount and then >> call of_node_put() with it when done. >> >> Fixes: 63338a38db95 ("jailhouse: Provide detection for non-x86 systems") >> Signed-off-by: Liang He <windhl@126.com> >> Signed-off-by: Kelin Wang <wangkelin2023@163.com> >> --- >> include/linux/hypervisor.h | 6 +++++- >> 1 file changed, 5 insertions(+), 1 deletion(-) >> >> diff --git a/include/linux/hypervisor.h b/include/linux/hypervisor.h >> index 9efbc54e35e5..7fe1e8c6211c 100644 >> --- a/include/linux/hypervisor.h >> +++ b/include/linux/hypervisor.h >> @@ -27,7 +27,11 @@ static inline void hypervisor_pin_vcpu(int cpu) >> >> static inline bool jailhouse_paravirt(void) >> { >> - return of_find_compatible_node(NULL, NULL, "jailhouse,cell"); >> + struct device_node *np = of_find_compatible_node(NULL, NULL, "jailhouse,cell"); >> + >> + of_node_put(np); >> + >> + return np; >> } >> > >Thank you for the fix, but returning a pointer from a function with a >bool return type looks odd. Can we also fix that up please? > Thanks for your review, how about following patch: - return of_find_compatible_node(NULL, NULL, "jailhouse,cell"); + struct device_node *np = of_find_compatible_node(NULL, NULL, "jailhouse,cell"); + + of_node_put(np); + + return (np==NULL); > >Regards, >Srivatsa >VMware Photon OS ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: Re: [PATCH] jailhouse: Hold reference returned from of_find_xxx API 2022-09-16 2:00 ` Liang He @ 2022-09-16 5:38 ` Andy Shevchenko 0 siblings, 0 replies; 10+ messages in thread From: Andy Shevchenko @ 2022-09-16 5:38 UTC (permalink / raw) To: Liang He Cc: Srivatsa S. Bhat, jgross, virtualization, wangkelin2023, jan.kiszka, Thomas Gleixner, jailhouse-dev, mark.rutland, linux-pci, robh+dt, Bjorn Helgaas On Fri, Sep 16, 2022 at 5:02 AM Liang He <windhl@126.com> wrote: > At 2022-09-16 07:29:06, "Srivatsa S. Bhat" <srivatsa@csail.mit.edu> wrote: > >On 9/14/22 7:23 PM, Liang He wrote: .. > >> static inline bool jailhouse_paravirt(void) > >> { > >> - return of_find_compatible_node(NULL, NULL, "jailhouse,cell"); > >> + struct device_node *np = of_find_compatible_node(NULL, NULL, "jailhouse,cell"); > >> + > >> + of_node_put(np); > >> + > >> + return np; > >> } > > > >Thank you for the fix, but returning a pointer from a function with a > >bool return type looks odd. Can we also fix that up please? > > > > Thanks for your review, how about following patch: > > - return of_find_compatible_node(NULL, NULL, "jailhouse,cell"); > + struct device_node *np = of_find_compatible_node(NULL, NULL, "jailhouse,cell"); > + > + of_node_put(np); > + > + return (np==NULL); This will be opposite to the above. Perhaps you wanted return !!np; Also possible (but why?) return np ? true : false; -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: Re: [PATCH] jailhouse: Hold reference returned from of_find_xxx API @ 2022-09-16 5:38 ` Andy Shevchenko 0 siblings, 0 replies; 10+ messages in thread From: Andy Shevchenko @ 2022-09-16 5:38 UTC (permalink / raw) To: Liang He Cc: jgross, jailhouse-dev, jan.kiszka, mark.rutland, virtualization, robh+dt, wangkelin2023, linux-pci, Bjorn Helgaas, Thomas Gleixner On Fri, Sep 16, 2022 at 5:02 AM Liang He <windhl@126.com> wrote: > At 2022-09-16 07:29:06, "Srivatsa S. Bhat" <srivatsa@csail.mit.edu> wrote: > >On 9/14/22 7:23 PM, Liang He wrote: .. > >> static inline bool jailhouse_paravirt(void) > >> { > >> - return of_find_compatible_node(NULL, NULL, "jailhouse,cell"); > >> + struct device_node *np = of_find_compatible_node(NULL, NULL, "jailhouse,cell"); > >> + > >> + of_node_put(np); > >> + > >> + return np; > >> } > > > >Thank you for the fix, but returning a pointer from a function with a > >bool return type looks odd. Can we also fix that up please? > > > > Thanks for your review, how about following patch: > > - return of_find_compatible_node(NULL, NULL, "jailhouse,cell"); > + struct device_node *np = of_find_compatible_node(NULL, NULL, "jailhouse,cell"); > + > + of_node_put(np); > + > + return (np==NULL); This will be opposite to the above. Perhaps you wanted return !!np; Also possible (but why?) return np ? true : false; -- With Best Regards, Andy Shevchenko _______________________________________________ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re:Re: Re: [PATCH] jailhouse: Hold reference returned from of_find_xxx API 2022-09-16 5:38 ` Andy Shevchenko (?) @ 2022-09-16 7:07 ` Liang He 2022-09-16 8:46 ` Andy Shevchenko -1 siblings, 1 reply; 10+ messages in thread From: Liang He @ 2022-09-16 7:07 UTC (permalink / raw) To: Andy Shevchenko Cc: Srivatsa S. Bhat, jgross, virtualization, wangkelin2023, jan.kiszka, Thomas Gleixner, jailhouse-dev, mark.rutland, linux-pci, robh+dt, Bjorn Helgaas At 2022-09-16 13:38:39, "Andy Shevchenko" <andy.shevchenko@gmail.com> wrote: >On Fri, Sep 16, 2022 at 5:02 AM Liang He <windhl@126.com> wrote: >> At 2022-09-16 07:29:06, "Srivatsa S. Bhat" <srivatsa@csail.mit.edu> wrote: >> >On 9/14/22 7:23 PM, Liang He wrote: > >.. > >> >> static inline bool jailhouse_paravirt(void) >> >> { >> >> - return of_find_compatible_node(NULL, NULL, "jailhouse,cell"); >> >> + struct device_node *np = of_find_compatible_node(NULL, NULL, "jailhouse,cell"); >> >> + >> >> + of_node_put(np); >> >> + >> >> + return np; >> >> } >> > >> >Thank you for the fix, but returning a pointer from a function with a >> >bool return type looks odd. Can we also fix that up please? >> > >> >> Thanks for your review, how about following patch: >> >> - return of_find_compatible_node(NULL, NULL, "jailhouse,cell"); >> + struct device_node *np = of_find_compatible_node(NULL, NULL, "jailhouse,cell"); >> + >> + of_node_put(np); >> + >> + return (np==NULL); > >This will be opposite to the above. Perhaps you wanted Sorry, I wanted to use 'np!=NULL' > return !!np; > >Also possible (but why?) > > return np ? true : false; > So, can I chose 'return np?true: false;' as the final patch? Thanks, Liang >-- >With Best Regards, >Andy Shevchenko ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: Re: Re: [PATCH] jailhouse: Hold reference returned from of_find_xxx API 2022-09-16 7:07 ` Liang He @ 2022-09-16 8:46 ` Andy Shevchenko 0 siblings, 0 replies; 10+ messages in thread From: Andy Shevchenko @ 2022-09-16 8:46 UTC (permalink / raw) To: Liang He Cc: Srivatsa S. Bhat, jgross, virtualization, wangkelin2023, jan.kiszka, Thomas Gleixner, jailhouse-dev, mark.rutland, linux-pci, robh+dt, Bjorn Helgaas On Fri, Sep 16, 2022 at 10:09 AM Liang He <windhl@126.com> wrote: > At 2022-09-16 13:38:39, "Andy Shevchenko" <andy.shevchenko@gmail.com> wrote: > >On Fri, Sep 16, 2022 at 5:02 AM Liang He <windhl@126.com> wrote: > >> At 2022-09-16 07:29:06, "Srivatsa S. Bhat" <srivatsa@csail.mit.edu> wrote: > >> >On 9/14/22 7:23 PM, Liang He wrote: ... > >> >> static inline bool jailhouse_paravirt(void) > >> >> { > >> >> - return of_find_compatible_node(NULL, NULL, "jailhouse,cell"); > >> >> + struct device_node *np = of_find_compatible_node(NULL, NULL, "jailhouse,cell"); > >> >> + > >> >> + of_node_put(np); > >> >> + > >> >> + return np; > >> >> } > >> > > >> >Thank you for the fix, but returning a pointer from a function with a > >> >bool return type looks odd. Can we also fix that up please? > >> > >> Thanks for your review, how about following patch: > >> > >> - return of_find_compatible_node(NULL, NULL, "jailhouse,cell"); > >> + struct device_node *np = of_find_compatible_node(NULL, NULL, "jailhouse,cell"); > >> + > >> + of_node_put(np); > >> + > >> + return (np==NULL); > > >This will be opposite to the above. Perhaps you wanted > > Sorry, I wanted to use 'np!=NULL' > > > return !!np; > > > >Also possible (but why?) > > > > return np ? true : false; > > So, can I chose 'return np?true: false;' as the final patch? Of course you can, it's up to the maintainer(s) what to accept. -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: Re: Re: [PATCH] jailhouse: Hold reference returned from of_find_xxx API @ 2022-09-16 8:46 ` Andy Shevchenko 0 siblings, 0 replies; 10+ messages in thread From: Andy Shevchenko @ 2022-09-16 8:46 UTC (permalink / raw) To: Liang He Cc: jgross, jailhouse-dev, jan.kiszka, mark.rutland, virtualization, robh+dt, wangkelin2023, linux-pci, Bjorn Helgaas, Thomas Gleixner On Fri, Sep 16, 2022 at 10:09 AM Liang He <windhl@126.com> wrote: > At 2022-09-16 13:38:39, "Andy Shevchenko" <andy.shevchenko@gmail.com> wrote: > >On Fri, Sep 16, 2022 at 5:02 AM Liang He <windhl@126.com> wrote: > >> At 2022-09-16 07:29:06, "Srivatsa S. Bhat" <srivatsa@csail.mit.edu> wrote: > >> >On 9/14/22 7:23 PM, Liang He wrote: ... > >> >> static inline bool jailhouse_paravirt(void) > >> >> { > >> >> - return of_find_compatible_node(NULL, NULL, "jailhouse,cell"); > >> >> + struct device_node *np = of_find_compatible_node(NULL, NULL, "jailhouse,cell"); > >> >> + > >> >> + of_node_put(np); > >> >> + > >> >> + return np; > >> >> } > >> > > >> >Thank you for the fix, but returning a pointer from a function with a > >> >bool return type looks odd. Can we also fix that up please? > >> > >> Thanks for your review, how about following patch: > >> > >> - return of_find_compatible_node(NULL, NULL, "jailhouse,cell"); > >> + struct device_node *np = of_find_compatible_node(NULL, NULL, "jailhouse,cell"); > >> + > >> + of_node_put(np); > >> + > >> + return (np==NULL); > > >This will be opposite to the above. Perhaps you wanted > > Sorry, I wanted to use 'np!=NULL' > > > return !!np; > > > >Also possible (but why?) > > > > return np ? true : false; > > So, can I chose 'return np?true: false;' as the final patch? Of course you can, it's up to the maintainer(s) what to accept. -- With Best Regards, Andy Shevchenko _______________________________________________ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re:Re: Re: Re: [PATCH] jailhouse: Hold reference returned from of_find_xxx API 2022-09-16 8:46 ` Andy Shevchenko (?) @ 2022-09-16 8:49 ` Liang He -1 siblings, 0 replies; 10+ messages in thread From: Liang He @ 2022-09-16 8:49 UTC (permalink / raw) To: Andy Shevchenko Cc: Srivatsa S. Bhat, jgross, virtualization, wangkelin2023, jan.kiszka, Thomas Gleixner, jailhouse-dev, mark.rutland, linux-pci, robh+dt, Bjorn Helgaas At 2022-09-16 16:46:36, "Andy Shevchenko" <andy.shevchenko@gmail.com> wrote: >On Fri, Sep 16, 2022 at 10:09 AM Liang He <windhl@126.com> wrote: >> At 2022-09-16 13:38:39, "Andy Shevchenko" <andy.shevchenko@gmail.com> wrote: >> >On Fri, Sep 16, 2022 at 5:02 AM Liang He <windhl@126.com> wrote: >> >> At 2022-09-16 07:29:06, "Srivatsa S. Bhat" <srivatsa@csail.mit.edu> wrote: >> >> >On 9/14/22 7:23 PM, Liang He wrote: > >... > >> >> >> static inline bool jailhouse_paravirt(void) >> >> >> { >> >> >> - return of_find_compatible_node(NULL, NULL, "jailhouse,cell"); >> >> >> + struct device_node *np = of_find_compatible_node(NULL, NULL, "jailhouse,cell"); >> >> >> + >> >> >> + of_node_put(np); >> >> >> + >> >> >> + return np; >> >> >> } >> >> > >> >> >Thank you for the fix, but returning a pointer from a function with a >> >> >bool return type looks odd. Can we also fix that up please? >> >> >> >> Thanks for your review, how about following patch: >> >> >> >> - return of_find_compatible_node(NULL, NULL, "jailhouse,cell"); >> >> + struct device_node *np = of_find_compatible_node(NULL, NULL, "jailhouse,cell"); >> >> + >> >> + of_node_put(np); >> >> + >> >> + return (np==NULL); >> >> >This will be opposite to the above. Perhaps you wanted >> >> Sorry, I wanted to use 'np!=NULL' >> >> > return !!np; >> > >> >Also possible (but why?) >> > >> > return np ? true : false; >> >> So, can I chose 'return np?true: false;' as the final patch? > >Of course you can, it's up to the maintainer(s) what to accept. > >-- >With Best Regards, >Andy Shevchenko Thanks, I will do it now. ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2022-09-16 8:50 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <20220915022343.4001331-1-windhl@126.com>
2022-09-15 7:00 ` [PATCH] jailhouse: Hold reference returned from of_find_xxx API Juergen Gross via Virtualization
2022-09-15 23:29 ` Srivatsa S. Bhat
2022-09-15 23:29 ` Srivatsa S. Bhat
2022-09-16 2:00 ` Liang He
2022-09-16 5:38 ` Andy Shevchenko
2022-09-16 5:38 ` Andy Shevchenko
2022-09-16 7:07 ` Liang He
2022-09-16 8:46 ` Andy Shevchenko
2022-09-16 8:46 ` Andy Shevchenko
2022-09-16 8:49 ` Liang He
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.