lkml.org 
[lkml]   [2018]   [Aug]   [9]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [PATCH 3/5] fs/locks: change all *_conflict() functions to return a new enum.
    From
    Date
    On Thu, 2018-08-09 at 12:04 +1000, NeilBrown wrote:
    > In a future patch we will need to differentiate between conflicts that
    > are "transitive" and those that aren't.
    > A "transitive" conflict is defined as one where any lock that
    > conflicts with the first (newly requested) lock would conflict with
    > the existing lock.
    >
    > So change posix_locks_conflict(), flock_locks_conflict() (both
    > currently returning int) and leases_conflict() (currently returning
    > bool) to return "enum conflict".
    > Add locks_transitive_overlap() to make it possible to compute the
    > correct conflict for posix locks.
    >
    > The FL_NO_CONFLICT value is zero, so current code which only tests the
    > truth value of these functions will still work the same way.
    >
    > And convert some
    > return (foo);
    > to
    > return foo;
    >
    > Signed-off-by: NeilBrown <neilb@suse.com>
    > ---
    > fs/locks.c | 64 ++++++++++++++++++++++++++++++++++++++++++++++--------------
    > 1 file changed, 49 insertions(+), 15 deletions(-)
    >
    > diff --git a/fs/locks.c b/fs/locks.c
    > index b4812da2a374..d06658b2dc7a 100644
    > --- a/fs/locks.c
    > +++ b/fs/locks.c
    > @@ -139,6 +139,16 @@
    > #define IS_OFDLCK(fl) (fl->fl_flags & FL_OFDLCK)
    > #define IS_REMOTELCK(fl) (fl->fl_pid <= 0)
    >
    > +/* A transitive conflict is one where the first lock conflicts with
    > + * the second lock, and any other lock that conflicts with the
    > + * first lock, also conflicts with the second lock.
    > + */
    > +enum conflict {
    > + FL_NO_CONFLICT = 0,
    > + FL_CONFLICT,
    > + FL_TRANSITIVE_CONFLICT,
    > +};
    > +
    > static inline bool is_remote_lock(struct file *filp)
    > {
    > return likely(!(filp->f_path.dentry->d_sb->s_flags & SB_NOREMOTELOCK));
    > @@ -612,6 +622,15 @@ static inline int locks_overlap(struct file_lock *fl1, struct file_lock *fl2)
    > (fl2->fl_end >= fl1->fl_start));
    > }
    >
    > +/* Check for transitive-overlap - true if any lock that overlaps
    > + * the first lock must overlap the seconds
    > + */
    > +static inline bool locks_transitive_overlap(struct file_lock *fl1,
    > + struct file_lock *fl2)
    > +{
    > + return (fl1->fl_start >= fl2->fl_start) &&
    > + (fl1->fl_end <= fl2->fl_end);
    > +}
    > /*
    > * Check whether two locks have the same owner.
    > */
    > @@ -793,47 +812,61 @@ locks_delete_lock_ctx(struct file_lock *fl, struct list_head *dispose)
    > /* Determine if lock sys_fl blocks lock caller_fl. Common functionality
    > * checks for shared/exclusive status of overlapping locks.
    > */
    > -static int locks_conflict(struct file_lock *caller_fl, struct file_lock *sys_fl)
    > +static enum conflict locks_conflict(struct file_lock *caller_fl,
    > + struct file_lock *sys_fl)
    > {
    > if (sys_fl->fl_type == F_WRLCK)
    > - return 1;
    > + return FL_TRANSITIVE_CONFLICT;
    > if (caller_fl->fl_type == F_WRLCK)
    > - return 1;
    > - return 0;
    > + return FL_CONFLICT;
    > + return FL_NO_CONFLICT;
    > }
    >
    > /* Determine if lock sys_fl blocks lock caller_fl. POSIX specific
    > * checking before calling the locks_conflict().
    > */
    > -static int posix_locks_conflict(struct file_lock *caller_fl, struct file_lock *sys_fl)
    > +static enum conflict posix_locks_conflict(struct file_lock *caller_fl,
    > + struct file_lock *sys_fl)
    > {
    > /* POSIX locks owned by the same process do not conflict with
    > * each other.
    > */
    > if (posix_same_owner(caller_fl, sys_fl))
    > - return (0);
    > + return FL_NO_CONFLICT;
    >
    > /* Check whether they overlap */
    > if (!locks_overlap(caller_fl, sys_fl))
    > - return 0;
    > + return FL_NO_CONFLICT;
    >
    > - return (locks_conflict(caller_fl, sys_fl));
    > + switch (locks_conflict(caller_fl, sys_fl)) {
    > + default:

    Maybe BUG or WARN here or something? locks_conflict should never return
    values that aren't in the enum.

    > + case FL_NO_CONFLICT:
    > + return FL_NO_CONFLICT;
    > + case FL_CONFLICT:
    > + return FL_CONFLICT;
    > + case FL_TRANSITIVE_CONFLICT:
    > + if (locks_transitive_overlap(caller_fl, sys_fl))
    > + return FL_TRANSITIVE_CONFLICT;
    > + else
    > + return FL_CONFLICT;
    > + }
    > }
    >
    > /* Determine if lock sys_fl blocks lock caller_fl. FLOCK specific
    > * checking before calling the locks_conflict().
    > */
    > -static int flock_locks_conflict(struct file_lock *caller_fl, struct file_lock *sys_fl)
    > +static enum conflict flock_locks_conflict(struct file_lock *caller_fl,
    > + struct file_lock *sys_fl)
    > {
    > /* FLOCK locks referring to the same filp do not conflict with
    > * each other.
    > */
    > if (caller_fl->fl_file == sys_fl->fl_file)
    > - return (0);
    > + return FL_NO_CONFLICT;
    > if ((caller_fl->fl_type & LOCK_MAND) || (sys_fl->fl_type & LOCK_MAND))
    > - return 0;
    > + return FL_NO_CONFLICT;
    >
    > - return (locks_conflict(caller_fl, sys_fl));
    > + return locks_conflict(caller_fl, sys_fl);
    > }
    >
    > void
    > @@ -1435,12 +1468,13 @@ static void time_out_leases(struct inode *inode, struct list_head *dispose)
    > }
    > }
    >
    > -static bool leases_conflict(struct file_lock *lease, struct file_lock *breaker)
    > +static enum conflict leases_conflict(struct file_lock *lease,
    > + struct file_lock *breaker)
    > {
    > if ((breaker->fl_flags & FL_LAYOUT) != (lease->fl_flags & FL_LAYOUT))
    > - return false;
    > + return FL_NO_CONFLICT;
    > if ((breaker->fl_flags & FL_DELEG) && (lease->fl_flags & FL_LEASE))
    > - return false;
    > + return FL_NO_CONFLICT;
    > return locks_conflict(breaker, lease);
    > }
    >
    >
    >
    --
    Jeff Layton <jlayton@kernel.org>

    \
     
     \ /
      Last update: 2018-08-09 13:10    [W:4.053 / U:0.696 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site