On 07.03.2024 14:30, Oleksii wrote: > On Wed, 2024-03-06 at 16:31 +0100, Jan Beulich wrote: >> On 26.02.2024 18:38, Oleksii Kurochko wrote: >>> --- /dev/null >>> +++ b/xen/arch/riscv/include/asm/atomic.h >>> @@ -0,0 +1,296 @@ >>> +/* SPDX-License-Identifier: GPL-2.0-only */ >>> +/* >>> + * Taken and modified from Linux. >>> + * >>> + * The following changes were done: >>> + * - * atomic##prefix##_*xchg_*(atomic##prefix##_t *v, c_t n) were >>> updated >>> + * to use__*xchg_generic() >>> + * - drop casts in write_atomic() as they are unnecessary >>> + * - drop introduction of WRITE_ONCE() and READ_ONCE(). >>> + * Xen provides ACCESS_ONCE() >>> + * - remove zero-length array access in read_atomic() >>> + * - drop defines similar to pattern >>> + * #define atomic_add_return_relaxed atomic_add_return_relaxed >>> + * - move not RISC-V specific functions to asm-generic/atomics- >>> ops.h >>> + * >>> + * Copyright (C) 2007 Red Hat, Inc. All Rights Reserved. >>> + * Copyright (C) 2012 Regents of the University of California >>> + * Copyright (C) 2017 SiFive >>> + * Copyright (C) 2024 Vates SAS >>> + */ >>> + >>> +#ifndef _ASM_RISCV_ATOMIC_H >>> +#define _ASM_RISCV_ATOMIC_H >>> + >>> +#include <xen/atomic.h> >>> + >>> +#include <asm/cmpxchg.h> >>> +#include <asm/fence.h> >>> +#include <asm/io.h> >>> +#include <asm/system.h> >>> + >>> +#include <asm-generic/atomic-ops.h> >> >> While, because of the forward decls in xen/atomic.h, having this >> #include >> works, I wonder if it wouldn't better be placed further down. The >> compiler >> will likely have an easier time when it sees the inline definitions >> ahead >> of any uses. > Do you mean to move it after #define __atomic_release_fence() ?
Perhaps yet further down, at least after the arithmetic ops were defined. >>> --- /dev/null >>> +++ b/xen/include/asm-generic/atomic-ops.h >>> @@ -0,0 +1,92 @@ >>> +#/* SPDX-License-Identifier: GPL-2.0 */ >>> +#ifndef _ASM_GENERIC_ATOMIC_OPS_H_ >>> +#define _ASM_GENERIC_ATOMIC_OPS_H_ >>> + >>> +#include <xen/atomic.h> >>> +#include <xen/lib.h> >> >> If I'm not mistaken this header provides default implementations for >> every >> xen/atomic.h-provided forward inline declaration that can be >> synthesized >> from other atomic functions. I think a comment to this effect would >> want >> adding somewhere here. > I think we can drop this inclusion here as inclusion of asm- > generic/atomic-ops.h will be always go with inclusion of xen/atomic.h. I'm okay with dropping that include, but that wasn't the purpose of my comment. I was rather asking for a comment to be added here stating what is (not) to be present in this header. Jan
