On Mon, Aug 11, 2014 at 01:20:53PM -0400, Jeff Layton wrote: > On Mon, 11 Aug 2014 13:09:37 -0400 > "J. Bruce Fields" <bfields@xxxxxxxxxxxx> wrote: > > > On Mon, Aug 11, 2014 at 12:40:39PM -0400, Jeff Layton wrote: > > > On Mon, 11 Aug 2014 12:08:40 -0400 > > > "J. Bruce Fields" <bfields@xxxxxxxxxxxx> wrote: > > > > > > > On Sat, Aug 09, 2014 at 10:22:42AM -0400, Jeff Layton wrote: > > > > > It's possible that we'll have a nfs4_file that has nothing in its fds > > > > > array, but that has a populated fi_deleg_file field. > > > > > > > > Is it really possible? On a quick skim it looks like this is only used > > > > in the presence of lock stateid's, when we should have an open. > > > > > > > > OK with the cleanup I'm just not seeing a reason either one way or the > > > > other for the fi_deleg_file change. > > > > > > > > --b. > > > > > > > > > > You're correct. The existing code doesn't specifically require this > > > patch since find_any_file is only used with lock stateids. It > > > should be harmless but it won't hurt anything to drop it. > > > > > > I did however need this when I rebased some pnfsd patches on top of the > > > state overhaul, and it seemed like a reasonable change from a > > > "future-proofing" standpoint. > > > > So layout operations depend on this somehow? (But layouts can outlast > > delegations, so that must not be it.) > > > > I'm not opposed to future-proofing as long as we have some evidence > > about the future. > > > > The code I'm working on does depend on this, but it's may not be > correct for any arbitrary filesystem. > > This filesystem sets return_on_close, and we automatically return the > layout to the fs when all of the open and deleg stateids go away. If > the last one to drop its reference happens to be the delegation > stateid, then you may have nothing in the fi_fds slots, and no way to > get to the inode in order to return the layout. > > I've been toying with the idea of keeping an inode reference in the > layout state, but I'm not sure if it's the right thing to do... OK, makes sense, thanks for the explanation. But I think I'll drop this third patch for now. --b. > > > > Do you intend to the take the first two in the series? I would like to > > > see those go in since they move the lease removal outside of spinlocks. > > > > Yes, just waiting for -rc1 comes out to push out a for-3.18 tree. > > > > Sounds good -- no rush on them. > > Thanks! > -- > Jeff Layton <jlayton@xxxxxxxxxxxxxxx> -- 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
