Re: [PATCH] fs: add missing fences to I_NEW handling

[Hillf Danton <hdanton@sina.com>]

On Mon, 6 Oct 2025 03:16:43 +0200 Mateusz Guzik wrote:
> On Mon, Oct 6, 2025 at 2:57 AM Hillf Danton <hdanton@sina.com> wrote:
> > On Mon,  6 Oct 2025 01:15:26 +0200 Mateusz Guzik wrote:
> > > Suppose there are 2 CPUs racing inode hash lookup func (say ilookup5())
> > > and unlock_new_inode().
> > >
> > > In principle the latter can clear the I_NEW flag before prior stores
> > > into the inode were made visible.
> > >
> > Given difficulty following up here, could you specify why the current
> > mem barrier [1] in unlock_new_inode() is not enough?
> >
> That fence synchronizes against threads which went to sleep.
> 
> In the example I'm providing this did not happen.
> 
>   193 static inline void wait_on_inode(struct inode *inode)
>   194 {
>   195         wait_var_event(inode_state_wait_address(inode, __I_NEW),
>   196                       !(READ_ONCE(inode->i_state) & I_NEW));
> 
>   303 #define wait_var_event(var, condition)                                  \
>   304 do {                                                                    \
>   305         might_sleep();                                                  \
>   306         if (condition)                                                  \
>   307                 break;                                                  \
> 
> I_NEW is tested here without any locks or fences.
> 
Thanks, got it but given the comment of the current mem barrier in
unlock_new_inode(), why did peterZ/Av leave such a huge chance behind?

The condition check in wait_var_event() matches waitqueue_active() in
__wake_up_bit(), and both make it run fast on both the waiter and the
waker sides, no?