On Wed, 2024-01-17 at 09:07 +1100, NeilBrown wrote:
> On Wed, 17 Jan 2024, Jeff Layton wrote:
> > In a future patch, we're going to split file leases into their own
> > structure. Since a lot of the underlying machinery uses the same fields
> > move those into a new file_lock_core, and embed that inside struct
> > file_lock.
> >
> > Signed-off-by: Jeff Layton <jlayton@xxxxxxxxxx>
> > ---
> > include/linux/filelock.h | 9 +++++++--
> > 1 file changed, 7 insertions(+), 2 deletions(-)
> >
> > diff --git a/include/linux/filelock.h b/include/linux/filelock.h
> > index 95e868e09e29..7825511c1c11 100644
> > --- a/include/linux/filelock.h
> > +++ b/include/linux/filelock.h
> > @@ -85,8 +85,9 @@ bool opens_in_grace(struct net *);
> > *
> > * Obviously, the last two criteria only matter for POSIX locks.
> > */
> > -struct file_lock {
> > - struct file_lock *fl_blocker; /* The lock, that is blocking us */
> > +
> > +struct file_lock_core {
> > + struct file_lock *fl_blocker; /* The lock that is blocking us */
> > struct list_head fl_list; /* link into file_lock_context */
> > struct hlist_node fl_link; /* node in global lists */
> > struct list_head fl_blocked_requests; /* list of requests with
> > @@ -102,6 +103,10 @@ struct file_lock {
> > int fl_link_cpu; /* what cpu's list is this on? */
> > wait_queue_head_t fl_wait;
> > struct file *fl_file;
> > +};
> > +
> > +struct file_lock {
> > + struct file_lock_core fl_core;
> > loff_t fl_start;
> > loff_t fl_end;
> >
>
> If I we doing this, I would rename all the fields in file_lock_core to
> have an "flc_" prefix, and add some #defines like
>
> #define fl_list fl_core.flc_list
>
> so there would be no need to squash this with later patches to achieve
> bisectability.
>
> The #defines would be removed after the coccinelle scripts etc are
> applied.
>
> I would also do the "convert some internal functions" patches *before*
> the bulk conversion of fl_foo to fl_code.flc_foo so that those functions
> don't get patched twice.
>
> But this is all personal preference. If you prefer your approach,
> please leave it that way. The only clear benefit of my approach is that
> you don't need to squash patches together, and that is probably not a
> big deal.
>
I considered going back and doing that. It would allow me to break this
up into smaller patches, but I think that basically means doing all of
this work over again. I'll probably stick with this approach, unless I
end up needing to respin for other reasons.
--
Jeff Layton <jlayton@xxxxxxxxxx>
