On Fri, 24 Jan 2014 17:39:45 -0700
Trond Myklebust <trond.myklebust@xxxxxxxxxxxxxxx> wrote:
>
> On Jan 24, 2014, at 14:21, Jeff Layton <jlayton@xxxxxxxxxx> wrote:
>
> > On Fri, 24 Jan 2014 11:46:41 -0700
> > Trond Myklebust <trond.myklebust@xxxxxxxxxxxxxxx> wrote:
> >>
> >> Convert your patch to use wait_on_bit(), and then to call
> >> wait_on_bit_lock() if and only if you see NFS_INO_INVALID_DATA is set.
> >>
> >
> > I think that too would be racy...
> >
> > We have to clear NFS_INO_INVALID_DATA while holding the i_lock, but we
> > can't wait_on_bit_lock() under that. So (pseudocode):
> >
> > wait_on_bit
> > take i_lock
> > check and clear NFS_INO_INVALID_DATA
> > drop i_lock
> > wait_on_bit_lock
> >
> > ...so between dropping the i_lock and wait_on_bit_lock, we have a place
> > where another task could check the flag and find it clear.
>
>
> for(;;) {
> wait_on_bit(NFS_INO_INVALIDATING)
> /* Optimisation: don’t lock NFS_INO_INVALIDATING
> * if NFS_INO_INVALID_DATA was cleared while we waited.
> */
> if (!test_bit(NFS_INO_INVALID_DATA))
> return;
> if (!test_and_set_bit_lock(NFS_INO_INVALIDATING))
> break;
> }
> spin_lock(inode->i_lock);
> if (!test_and_clear_bit(NFS_INO_INVALID_DATA)) {
> spin_unlock(inode->i_lock);
> goto out_raced;
> }
> ….
> out_raced:
> clear_bit(NFS_INO_INVALIDATING)
> wake_up_bit(NFS_INO_INVALIDATING)
>
>
> --
> Trond Myklebust
> Linux NFS client maintainer
>
Hmm maybe. OTOH, if we're using atomic bitops do we need to deal with
the spinlock? I'll ponder it over the weekend and give it a harder
look on Monday.
Thanks for the thoughts so far...
--
Jeff Layton <jlayton@xxxxxxxxxx>
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
