| Message ID | 7fec1c9f906ee120ebae606de59f9f70efb79aff.1700761381.git.oleksii.kurochko@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Enable build of full Xen for RISC-V | expand |
On 24.11.2023 11:30, Oleksii Kurochko wrote: > The definition of __read_mostly should be removed in: > https://lore.kernel.org/xen-devel/f25eb5c9-7c14-6e23-8535-2c66772b333e@suse.com/ Andrew, can we settle on what to do with that patch? If you don't like me putting __read_mostly in xen/cache.h (consistent with __ro_after_init), would you please make an alternative suggestion? Personally I don't really understand why that patch hasn't long gone in. If further reorg is wanted, it can always be done subsequently. In whatever adjustments to the patch you want me to make to get past your objection, please make sure that it doesn't end up scope creeping. Jan > The patch introduces it in arch-specific header to not > block enabling of full Xen build for RISC-V. > > Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com> > --- > - update the commit message > --- > xen/arch/riscv/include/asm/cache.h | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/xen/arch/riscv/include/asm/cache.h b/xen/arch/riscv/include/asm/cache.h > index 69573eb051..94bd94db53 100644 > --- a/xen/arch/riscv/include/asm/cache.h > +++ b/xen/arch/riscv/include/asm/cache.h > @@ -3,4 +3,6 @@ > #ifndef _ASM_RISCV_CACHE_H > #define _ASM_RISCV_CACHE_H > > +#define __read_mostly __section(".data.read_mostly") > + > #endif /* _ASM_RISCV_CACHE_H */
On 12/12/2023 5:04 pm, Jan Beulich wrote: > On 24.11.2023 11:30, Oleksii Kurochko wrote: >> The definition of __read_mostly should be removed in: >> https://lore.kernel.org/xen-devel/f25eb5c9-7c14-6e23-8535-2c66772b333e@suse.com/ > Andrew, can we settle on what to do with that patch? If you don't like me > putting __read_mostly in xen/cache.h (consistent with __ro_after_init), > would you please make an alternative suggestion? xen/linkage.h? xen/sections.h? Sorry - I didn't mean to block it specifically, but I do think xen/cache.h is the wrong place for both to live and that it's a small enough change to warrant sorting out nicely once and for all. ~Andrew
On 21.12.2023 16:23, Andrew Cooper wrote: > On 12/12/2023 5:04 pm, Jan Beulich wrote: >> On 24.11.2023 11:30, Oleksii Kurochko wrote: >>> The definition of __read_mostly should be removed in: >>> https://lore.kernel.org/xen-devel/f25eb5c9-7c14-6e23-8535-2c66772b333e@suse.com/ >> Andrew, can we settle on what to do with that patch? If you don't like me >> putting __read_mostly in xen/cache.h (consistent with __ro_after_init), >> would you please make an alternative suggestion? > > xen/linkage.h? xen/sections.h? Well, that's the problem: There's no xen/sections.h (and I don't see why we'd introduce one just for this), and while xen/linkage.h is about to appear for the entry point annotations, I don't think it is a reasonable fit. Otoh with __ro_after_init ... > Sorry - I didn't mean to block it specifically, but I do think > xen/cache.h is the wrong place for both to live and that it's a small > enough change to warrant sorting out nicely once and for all. ... living in xen/cache.h already, __read_mostly is even more logical to put there than __ro_after_init. For the latter I agree the purpose isn't really cache related, while for the former it is. From your original reply to the patch submission, I have to admit I don't really understand what "micro-architectural" detail you mean: The goal of avoiding unnecessary cache line ping-pong doesn't look to be what you mean, as imo that's a valid (and generic) goal. And that's what I think is the reason the #define presently lives in asm/cache.h, justifying my moving of it to xen/cache.h. So if you can't get yourself to accept xen/cache.h as the new (even if only temporary) location, I think you can be expected to make a better proposal. With "better" there meaning you supplying a reason for why you think that placement is better than xen/cache.h. For example, if I knew what you expected xen/sections.h to further contain (in the long run), I might find myself agreeing to that. Yet other section annotation #define-s live elsewhere anyway, with - in particular - __init and friends imo not likely to move out of their present header (xen/init.h). Jan
diff --git a/xen/arch/riscv/include/asm/cache.h b/xen/arch/riscv/include/asm/cache.h index 69573eb051..94bd94db53 100644 --- a/xen/arch/riscv/include/asm/cache.h +++ b/xen/arch/riscv/include/asm/cache.h @@ -3,4 +3,6 @@ #ifndef _ASM_RISCV_CACHE_H #define _ASM_RISCV_CACHE_H +#define __read_mostly __section(".data.read_mostly") + #endif /* _ASM_RISCV_CACHE_H */
The definition of __read_mostly should be removed in: https://lore.kernel.org/xen-devel/f25eb5c9-7c14-6e23-8535-2c66772b333e@suse.com/ The patch introduces it in arch-specific header to not block enabling of full Xen build for RISC-V. Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com> --- - update the commit message --- xen/arch/riscv/include/asm/cache.h | 2 ++ 1 file changed, 2 insertions(+)