On Mon, 2010-08-16 at 15:50 +0800, Bian Naimeng wrote:
> >>>>>> We should clear NFS_DELEGATED_STATE bit for inode->open_states after return delegation.
>
> ... snip ...
>
> >> +
> >> + list_for_each_entry(state, &nfsi->open_states, inode_states) {
> >> + clear_bit(NFS_DELEGATED_STATE, &state->flags);
> >> + }
> >> +
> >> spin_unlock(&inode->i_lock);
> >> - return 0;
> >> + return err;
> >> }
> >>
> >
> > I still don't see why this should be necessary. In the case of a server
> > reboot or network partition, the state recovery thread ought to be
> > taking care of this for us.
> >
>
> A open state can be found at nfsi->open_states and owner->so_states always, but
> it not be found at nfsi->open_files until we call nfs4_intent_set_file.
>
> At _nfs4_do_open, we will invoke nfs4_return_incompatible_delegation to return a
> delegation, a open stateid which set NFS_DELEGATED_STATE bit may not be found at
> nfsi->open_files, but it still at nfsi->open_states, so we do not clear
> NFS_DELEGATED_STATE bit for it.
>
> Then _nfs4_do_open will find it by nfs4_get_open_state, and we still use it
> as a delegation, some error will occur.
>
OK. I see agree that is a race, but AFAICS, your fix means that we end
up with an nfs_state structure that has cleared NFS_DELEGATED_STATE, but
that did not recover its open stateid.
Cheers
Trond
--
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
