lkml.org 
[lkml]   [2010]   [Nov]   [3]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    SubjectRe: [PATCH 1/2] locks: let the caller free file_lock on ->setlease failure
    On Wed, Nov 03, 2010 at 09:40:24PM -0400, J. Bruce Fields wrote:
    > On Wed, Nov 03, 2010 at 04:41:48PM -0400, J. Bruce Fields wrote:
    > > On Sun, Oct 31, 2010 at 08:35:10AM -0400, Christoph Hellwig wrote:
    > > > Index: linux-2.6/fs/locks.c
    > > > ===================================================================
    > > > --- linux-2.6.orig/fs/locks.c 2010-10-31 07:10:07.649004084 -0400
    > > > +++ linux-2.6/fs/locks.c 2010-10-31 07:34:10.102255587 -0400
    > > > @@ -1428,8 +1425,9 @@ int generic_setlease(struct file *filp,
    > > > goto out;
    > > >
    > > > if (my_before != NULL) {
    > > > - *flp = *my_before;
    > > > error = lease->fl_lmops->fl_change(my_before, arg);
    > > > + if (!error)
    > > > + *flp = *my_before;
    > >
    > > Argh, missed this: we're leaking the passed-in lease in this case.
    >
    > We could do something like this.

    And while I was here I noticed:

    --b.

    From 072be1f975c6f839661cfc4f6c45ccb944417eea Mon Sep 17 00:00:00 2001
    From: J. Bruce Fields <bfields@redhat.com>
    Date: Wed, 3 Nov 2010 18:09:18 -0400
    Subject: [PATCH 2/2] locks: remove dead lease error-handling code

    A minor oversight from f7347ce4ee7c65415f84be915c018473e7076f31,
    "fasync: re-organize fasync entry insertion to allow it under a
    spinlock": this cleanup-on-error was only needed to handle -ENOMEM. Now
    that we're preallocating it's unneeded.

    Signed-off-by: J. Bruce Fields <bfields@redhat.com>
    ---
    fs/locks.c | 12 ++----------
    1 files changed, 2 insertions(+), 10 deletions(-)

    diff --git a/fs/locks.c b/fs/locks.c
    index 61c22f7..0e62dd3 100644
    --- a/fs/locks.c
    +++ b/fs/locks.c
    @@ -1506,7 +1506,6 @@ static int do_fcntl_add_lease(unsigned int fd, struct file *filp, long arg)
    {
    struct file_lock *fl, *ret;
    struct fasync_struct *new;
    - struct inode *inode = filp->f_path.dentry->d_inode;
    int error;

    fl = lease_alloc(filp, arg);
    @@ -1520,7 +1519,7 @@ static int do_fcntl_add_lease(unsigned int fd, struct file *filp, long arg)
    }
    ret = fl;
    lock_flocks();
    - error = __vfs_setlease(filp, arg, &fl);
    + error = __vfs_setlease(filp, arg, &ret);
    if (error) {
    unlock_flocks();
    locks_free_lock(fl);
    @@ -1538,14 +1537,7 @@ static int do_fcntl_add_lease(unsigned int fd, struct file *filp, long arg)
    if (!fasync_insert_entry(fd, filp, &ret->fl_fasync, new))
    new = NULL;

    - if (error < 0) {
    - /* remove lease just inserted by setlease */
    - fl->fl_type = F_UNLCK | F_INPROGRESS;
    - fl->fl_break_time = jiffies - 10;
    - time_out_leases(inode);
    - } else {
    - error = __f_setown(filp, task_pid(current), PIDTYPE_PID, 0);
    - }
    + error = __f_setown(filp, task_pid(current), PIDTYPE_PID, 0);
    unlock_flocks();

    out_free_fasync:
    --
    1.7.1


    \
     
     \ /
      Last update: 2010-11-04 02:43    [W:0.022 / U:1.372 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site