On Wed, 2025-01-29 at 18:06 -0500, Jeff Layton wrote:
> On Wed, 2025-01-29 at 18:20 +0100, Andreas Schwab wrote:
> > On Jan 29 2025, Andreas Schwab wrote:
> >
> > > My guess would be that something in inode_set_ctime_current is going
> > > wrong.
> >
> > The bug is in the arch_cmpxchg macros in <asm/cmpxchg.h>, they mishandle
> > atomic exchange of u32 values:
> >
> > # fs/inode.c:2802: if (cns == now.tv_nsec && inode->i_ctime_sec == now.tv_sec) {
> > #NO_APP
> > ld a5,-96(s0) # _33, now.tv_nsec
> > # fs/inode.c:2802: if (cns == now.tv_nsec && inode->i_ctime_sec == now.tv_sec) {
> > slli a4,s2,32 #, _49, cns
> > srli a4,a4,32 #, _49, _49
> > # fs/inode.c:2802: if (cns == now.tv_nsec && inode->i_ctime_sec == now.tv_sec) {
> > beq a4,a5,.L1248 #, _49, _33,
> > .L1205:
> > addi a3,s1,120 #, __ai_ptr, inode
> > .L1221:
> > # fs/inode.c:2809: if (try_cmpxchg(&inode->i_ctime_nsec, &cur, now.tv_nsec)) {
> > #APP
> > # 2809 "fs/inode.c" 1
> > 0: lr.w a2, 0(a3) # __ret, *__ai_ptr_20
> > bne a2, a4, 1f # __ret, _49
> >
> > Here the unsigned extended value of cur (a4) is compared with the sign
> > extended value of inode->i_ctime_nsec (a2). They cannot match if cur
> > has I_CTIME_QUERIED set (the sign bit). The lr/sc loop is exited before
> > the store conditional is executed.
> >
> > sc.w.rl a1, a5, 0(a3) # __rc, _33, *__ai_ptr_20
> > bnez a1, 0b # __rc
> > fence rw,rw
> > 1:
> >
> > # 0 "" 2
> > #NO_APP
> > sext.w a5,a2 # __ret, __ret
> >
> > A redundant sign extension of the current contents of inode->i_ctime_nsec.
> >
> > # fs/inode.c:2809: if (try_cmpxchg(&inode->i_ctime_nsec, &cur, now.tv_nsec)) {
> > bne a5,s2,.L1211 #, __ret, cns,
> >
> > Here the sign extended value of inode->i_ctime_nsec (a5) is compared
> > with the sign extended expected value (s2). They match, and try_cmpxchg
> > returns true, but the store never happend.
> >
>
> Wow, nice catch! Had me worried there for a bit!
>
For the record:
Bit 30 is also unused. When I wrote these patches, I considered using
that instead of the sign bit since I wasn't sure whether I might run
into problems using the sign bit.
Obviously, fixing the macro seems like the better solution, but if
fixing this efficiently is a problem, then moving I_CTIME_QUERIED to
bit 30 is also an option.
Cheers,
--
Jeff Layton <jlayton@xxxxxxxxxx>
