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
--
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
