Re: [PATCH 20/20] filelock: split leases out of struct file_lock

Jeff Layton
2024-01-17T12:45:15Z

On Wed, 2024-01-17 at 09:44 +1100, NeilBrown wrote:
> On Wed, 17 Jan 2024, Jeff Layton wrote:
> > Add a new struct file_lease and move the lease-specific fields from
> > struct file_lock to it. Convert the appropriate API calls to take
> > struct file_lease instead, and convert the callers to use them.
>
> I think that splitting of struct lease_manager_operations out from
> lock_manager_operations should be mentioned here too.
>

Will do.

>
> >
> > +struct file_lease {
> > + struct file_lock_core fl_core;
> > + struct fasync_struct * fl_fasync; /* for lease break notifications */
> > + /* for lease breaks: */
> > + unsigned long fl_break_time;
> > + unsigned long fl_downgrade_time;
> > + const struct lease_manager_operations *fl_lmops; /* Callbacks for lockmanagers */
>
> comment should be "Callbacks for leasemanagers". Or maybe
> "lease managers".
>
> It is unfortunate that "lock" and "lease" both start with 'l' as we now
> have two quite different fields in different structures with the same
> name - fl_lmops.
>

Hah, I had sort of considered that an advantage since I didn't need to
change as many call sites! Still, I get your point that having distinct
names is preferable.

I can change this to be distinct. I'll just need to come up with a
reasonable variable name (never my strong suit).

--
Jeff Layton <jlayton@kernel.org>