On Mon, May 14, 2018 at 07:31:53AM -0400, Andrew W Elble wrote:
>
> >> + /*
> >> + * This delegation is doomed, tell the recall logic
> >> + * that it's being destroyed here.
> >> + */
> >> +
> >> + if (status) {
> >> + dp->dl_time++;
> >> + list_del_init(&dp->dl_recall_lru);
> >> + dp->dl_stid.sc_type = NFS4_CLOSED_DELEG_STID;
> >> + }
> >
> > I'm trying to figure out if this fixes an actual bug. The code should
> > be able to deal with a callback on an already unhashed delegation, so I
> > think you're right it would at worst just be an unnecessary recall.
>
> But an 'normal' unhashed delegation would have a persistent refcount,
> this one would not. If the recall code gets a hold of it, it will
> place it on nn->del_recall_lru, and then free it in nfsd4_cb_recall_release()?
Sounds right. Do you see any bug there?
> > This won't catch every such case (could be that nfsd4_cb_recall_prepare
> > already ran and we're too late), so I wonder if this is worth it.
> >
> > More interesting to me is what exactly it would take to hit this
> > case.... Another thread would have to have succesfully hashed a
> > delegation for this client and file to make our hash_delegation_locked
> > fail. So there would be two leases for the same file and client, but
> > with different delegation pointers as the fl_owner. I *think* we handle
> > that OK. But it was likely problematic previously when we were still
> > using the file pointer as the fl_owner.
>
> I'm thinking this is more easily hit via fp->fi_had_conflict, if a lease
> break comes in at the right time?
Could be.
--b.
--
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
