Re: [PATCH 01/20] filelock: split common fields into struct file_lock_core

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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>





[Index of Archives]     [Linux Filesystem Development]     [Linux USB Development]     [Linux Media Development]     [Video for Linux]     [Linux NILFS]     [Linux Audio Users]     [Yosemite Info]     [Linux SCSI]

  Powered by Linux