On Tue, Aug 29 2017, J. Bruce Fields wrote: > On Mon, Aug 28, 2017 at 02:43:05PM +1000, NeilBrown wrote: >> On Fri, Aug 25 2017, J. Bruce Fields wrote: >> >> > From: "J. Bruce Fields" <bfields@xxxxxxxxxx> >> > >> > Pass around a new struct deleg_break_ctl instead of pointers to inode >> > pointers; in a future patch I want to use this to pass a little more >> > information from the nfs server to the lease code. >> >> The information you are passing from the nfs server to the lease code is >> largely ignored by the lease code and is passed back to the nfs server, >> in the sm_breaker_owns_lease call back. >> >> If try_break_deleg() passed the 'delegated_inode' pointer though to >> __break_lease(), it could pass it through any_leases_conflict() and >> leases_conflict() to the lm_breaker_owns_lease() callback. >> Then container_of() could be used to access whatever other data nfsd had >> stashed near the inode. The common code wouldn't need to know any of >> the details. > > The new information that we need is some notion of "who" (really, which > NFSv4 client) is doing the operation (unlink, whatever) that breaks the > lease. We can't get that information from an inode pointer. > > I may just not understand your suggestion. Probably I was too terse. I'm suggesting that nfsd have a local "struct deleg_break_ctl" (or whatever name you like) which contains a 'struct inode *delegated_inode' plus whatever else is useful to nfsd. Then nfsd/vfs.c, when it calls things like vfs_unlink(), passes &dbc.delegated_inode (where 'struct deleg_break_ctl dbc'). So the vfs codes doesn't know about 'struct deleg_break_ctl', it just knows about the 'struct inode ** inodep' like it does now, though with the understanding that "DELEG_NO_WAIT" in the **inodep means that same as inodep==NULL. The vfs passes this same 'struct **inode' to lm_breaker_owns_lease() and the nfsd code uses dbc = container_of(inodep, struct deleg_break_ctl, delegated_inode) to get the dbc, and it can use the other fields however it likes. Then instead of the rather task-specific name "lm_breaker_owns_lease" we could have a more general name like "lm_lease_compatible" ... or something. "lm_break_doesn't_see_this_lease_as_being_in_conflict" is a bit long, and contains "'". Thanks, NeilBrown
Attachment:
signature.asc
Description: PGP signature
