Re: [RFC v3 2/5] bpf: Explicitly align bpf_res_spin_lock

[Finn Thain <fthain@linux-m68k.org>]

[-- Attachment #1: Type: text/plain, Size: 2391 bytes --]


On Wed, 8 Oct 2025, Alexei Starovoitov wrote:

> On Tue, Oct 7, 2025 at 4:50 PM Finn Thain <fthain@linux-m68k.org> wrote:
> >
> > Align bpf_res_spin_lock to avoid a BUILD_BUG_ON() when the alignment
> > changes, as it will do on m68k when, in a subsequent patch, the minimum
> > alignment of the atomic_t member of struct rqspinlock gets increased.
> > Drop the BUILD_BUG_ON() as it is now redundant.
> >
> > Cc: Martin KaFai Lau <martin.lau@linux.dev>
> > Cc: Eduard Zingerman <eddyz87@gmail.com>
> > Cc: Song Liu <song@kernel.org>
> > Cc: Yonghong Song <yonghong.song@linux.dev>
> > Cc: John Fastabend <john.fastabend@gmail.com>
> > Cc: KP Singh <kpsingh@kernel.org>
> > Cc: Stanislav Fomichev <sdf@fomichev.me>
> > Cc: Hao Luo <haoluo@google.com>
> > Cc: Jiri Olsa <jolsa@kernel.org>
> > ---
> >  include/asm-generic/rqspinlock.h | 2 +-
> >  kernel/bpf/rqspinlock.c          | 1 -
> >  2 files changed, 1 insertion(+), 2 deletions(-)
> >
> > diff --git a/include/asm-generic/rqspinlock.h b/include/asm-generic/rqspinlock.h
> > index 6d4244d643df..eac2dcd31b83 100644
> > --- a/include/asm-generic/rqspinlock.h
> > +++ b/include/asm-generic/rqspinlock.h
> > @@ -28,7 +28,7 @@ struct rqspinlock {
> >   */
> >  struct bpf_res_spin_lock {
> >         u32 val;
> > -};
> > +} __aligned(__alignof__(struct rqspinlock));
> 
> I don't follow.
> In the next patch you do:
> 
> typedef struct {
> - int counter;
> + int __aligned(sizeof(int)) counter;
> } atomic_t;
> 
> so it was 4 and still 4 ?
> Are you saying 'int' on m68k is not 4 byte aligned by default,
> so you have to force 4 byte align?
> 

Right. __alignof(int) == 2 on m68k.

> >  struct qspinlock;
> >  #ifdef CONFIG_QUEUED_SPINLOCKS
> > diff --git a/kernel/bpf/rqspinlock.c b/kernel/bpf/rqspinlock.c
> > index 338305c8852c..a88a0e9749d7 100644
> > --- a/kernel/bpf/rqspinlock.c
> > +++ b/kernel/bpf/rqspinlock.c
> > @@ -671,7 +671,6 @@ __bpf_kfunc int bpf_res_spin_lock(struct bpf_res_spin_lock *lock)
> >         int ret;
> >
> >         BUILD_BUG_ON(sizeof(rqspinlock_t) != sizeof(struct bpf_res_spin_lock));
> > -       BUILD_BUG_ON(__alignof__(rqspinlock_t) != __alignof__(struct bpf_res_spin_lock));
> 
> Why delete it? Didn't you make them equal in the above hunk?
> 

I deleted it because it's tautological. I think "do not repeat yourself" 
applies here.