Looks good,

Reviewed-by: Christoph Hellwig <[email protected]>

Some comments on further work I'd like to see in this area, though:

> +     spin_lock(&inode->i_lock);
> +     time_out_leases(inode);
>       for (before = &inode->i_flock;
>                       ((fl = *before) != NULL) && IS_LEASE(fl);
>                       before = &fl->fl_next) {
>               if (fl->fl_file != filp)
>                       continue;
> -             return fl->fl_lmops->lm_change(before, F_UNLCK);
> +             error = fl->fl_lmops->lm_change(before, F_UNLCK);
>       }

We really should split a lm_release from lm_change, the way it is
used is highly confusing.  In addition I think a lot of code
currently in lease_modify should move here instead, e.g. something like:


        if (fl->fl_file != filp)
                continue;

        fl = *before;
        fl->fl_type = F_UNLCK;
        lease_clear_pending(fl, F_UNLCK);
        locks_wake_up_blocks(fl);
        if (fl->fl_lmops->lm_delete)
                fl->fl_lmops->lm_delete(fl);
        locks_delete_lock(before, NULL);

with lm_delete for user space leases as:

static void lease_delete(struct file_lock *fl)
{
        struct file *filp = fl->fl_file;

        f_delown(filp);
        filp->f_owner.signum = 0;
        fasync_helper(0, fl->fl_file, 0, &fl->fl_fasync);
        if (fl->fl_fasync != NULL) {
                printk(KERN_ERR "locks_delete_lock: fasync == %p\n",
                                fl->fl_fasync);
                fl->fl_fasync = NULL;
        }
}

and a NULL implementation for delegations.
--
To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to