On Thu, Feb 6, 2025 at 6:06 PM Peilin Ye <yepeilin@xxxxxxxxxx> wrote: > > Introduce BPF instructions with load-acquire and store-release > semantics, as discussed in [1]. The following new flags are defined: > > BPF_ATOMIC_LOAD 0x10 > BPF_ATOMIC_STORE 0x20 > BPF_ATOMIC_TYPE(imm) ((imm) & 0xf0) > > BPF_RELAXED 0x0 > BPF_ACQUIRE 0x1 > BPF_RELEASE 0x2 > BPF_ACQ_REL 0x3 > BPF_SEQ_CST 0x4 I still don't like this. Earlier you said: > If yes, I think we either: > > (a) add more flags to imm<4-7>: maybe LOAD_SEQ_CST (0x3) and > STORE_SEQ_CST (0x6); need to skip OR (0x4) and AND (0x5) used by > RMW atomics > (b) specify memorder in imm<0-3> > > I chose (b) for fewer "What would be a good numerical value so that RMW > atomics won't need to use it in imm<4-7>?" questions to answer. > > If we're having dedicated fields for memorder, I think it's better to > define all possible values once and for all, just so that e.g. 0x2 will > always mean RELEASE in a memorder field. Initially I defined all six of > them [2], then Yonghong suggested dropping CONSUME [3]. I don't think we should be defining "all possible values", since these are the values that llvm and C model supports, but do we have any plans to support anything bug ld_acq/st_rel ? I haven't heard anything. What even the meaning of BPF_ATOMIC_LOAD | BPF_ACQ_REL ? What does the verifier suppose to do? reject for now? and then what? Map to what insn? These values might imply that bpf infra is supposed to map all the values to cpu instructions, but that's not what we're doing here. We're only dealing with two specific instructions. We're not defining a memory model for all future new instructions. pw-bot: cr
