lkml.org 
[lkml]   [2014]   [Nov]   [3]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    SubjectRe: [PATCH 1/1] ceph: Deletion of unnecessary checks before two function calls
    From
    On Sun, Nov 2, 2014 at 5:20 PM, SF Markus Elfring
    <elfring@users.sourceforge.net> wrote:
    > The functions ceph_put_snap_context() and iput() test whether their argument
    > is NULL and then return immediately. Thus the test around the call
    > is not needed.
    >
    > This issue was detected by using the Coccinelle software.
    >
    > Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
    > ---
    > fs/ceph/caps.c | 3 +--
    > fs/ceph/mds_client.c | 6 ++----
    > fs/ceph/snap.c | 9 +++------
    > 3 files changed, 6 insertions(+), 12 deletions(-)

    [CC'ed Zheng]

    Applied, but see below.

    >
    > diff --git a/fs/ceph/caps.c b/fs/ceph/caps.c
    > index 6d1cd45..7d99fc8 100644
    > --- a/fs/ceph/caps.c
    > +++ b/fs/ceph/caps.c
    > @@ -3136,8 +3136,7 @@ flush_cap_releases:
    > done:
    > mutex_unlock(&session->s_mutex);
    > done_unlocked:
    > - if (inode)
    > - iput(inode);
    > + iput(inode);
    > return;
    >
    > bad:
    > diff --git a/fs/ceph/mds_client.c b/fs/ceph/mds_client.c
    > index bad07c0..3b0ab05 100644
    > --- a/fs/ceph/mds_client.c
    > +++ b/fs/ceph/mds_client.c
    > @@ -523,8 +523,7 @@ void ceph_mdsc_release_request(struct kref *kref)
    > }
    > if (req->r_locked_dir)
    > ceph_put_cap_refs(ceph_inode(req->r_locked_dir), CEPH_CAP_PIN);
    > - if (req->r_target_inode)
    > - iput(req->r_target_inode);
    > + iput(req->r_target_inode);
    > if (req->r_dentry)
    > dput(req->r_dentry);

    dput() also checks for NULL argument, but the check is wrapped into
    unlikely(), which is why I presume it wasn't picked up. It would be
    great if you could improve your coccinelle script to handle
    {un,}likely() as well.

    > if (req->r_old_dentry)
    > @@ -995,8 +994,7 @@ out:
    > session->s_cap_iterator = NULL;
    > spin_unlock(&session->s_cap_lock);
    >
    > - if (last_inode)
    > - iput(last_inode);
    > + iput(last_inode);
    > if (old_cap)
    > ceph_put_cap(session->s_mdsc, old_cap);
    >
    > diff --git a/fs/ceph/snap.c b/fs/ceph/snap.c
    > index f01645a..c1cc993 100644
    > --- a/fs/ceph/snap.c
    > +++ b/fs/ceph/snap.c
    > @@ -365,8 +365,7 @@ static int build_snap_context(struct ceph_snap_realm *realm)
    > realm->ino, realm, snapc, snapc->seq,
    > (unsigned int) snapc->num_snaps);
    >
    > - if (realm->cached_context)
    > - ceph_put_snap_context(realm->cached_context);
    > + ceph_put_snap_context(realm->cached_context);
    > realm->cached_context = snapc;
    > return 0;
    >
    > @@ -590,15 +589,13 @@ static void queue_realm_cap_snaps(struct ceph_snap_realm
    > *realm)

    The patch was corrupted, that should have been a single line. I fixed
    it up but you may want to look into your email client settings.

    Thanks,

    Ilya


    \
     
     \ /
      Last update: 2014-11-03 11:41    [W:4.054 / U:0.052 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site