From patchwork Thu Sep 11 08:31:11 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ilya Dryomov X-Patchwork-Id: 4883521 Return-Path: X-Original-To: patchwork-ceph-devel@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.19.201]) by patchwork2.web.kernel.org (Postfix) with ESMTP id B0FEFC0338 for ; Thu, 11 Sep 2014 08:31:29 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 6AABC20211 for ; Thu, 11 Sep 2014 08:31:28 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 91F052017A for ; Thu, 11 Sep 2014 08:31:26 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753409AbaIKIbS (ORCPT ); Thu, 11 Sep 2014 04:31:18 -0400 Received: from mail-qa0-f47.google.com ([209.85.216.47]:60278 "EHLO mail-qa0-f47.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753605AbaIKIbM (ORCPT ); Thu, 11 Sep 2014 04:31:12 -0400 Received: by mail-qa0-f47.google.com with SMTP id x12so18494923qac.20 for ; Thu, 11 Sep 2014 01:31:12 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:date :message-id:subject:from:to:cc:content-type; bh=IJf23ynvvybZktiDJvtOmncvAxqiSOhwIbDZCcKM3Cs=; b=HBFqR1SiaUE2mk4g3NEEjpAPIbWybhKg+bDroHCcA52SnCMzMmNYF81SE1SgYCUe/O 6Vyr8+1S+C55/B0Rp7OETgY8uN/scS37zMF8xRC5IO5So0kGIYFlNdHJwji3JQppSGjx 8tqFjW0SIJQgxAQ9mds4kAP/S8IFRwdq3d9iBdsXvTzyYI5GBCSPO55NpdOsrGZMuKMX Lm5+R8Bvl7W3V98sNDmi5tMoNGzcwtiJ53p0cZDYbSLEIZtyn/pojgPCOVqkujMKVSP+ vUaaaTnzv/4XnfD1Md5wVU89yuF2QXDWB+mjWk535QZ3lz/jdO/vAEzGZyez3csUiEvj 5WnQ== X-Gm-Message-State: ALoCoQngft2l6ME4sfL8NP+4GBidth5D8Sxp6NcLcgzCOf/0uvpNANi2JuvfhJ7I9mfOvHKtV1+A MIME-Version: 1.0 X-Received: by 10.224.24.68 with SMTP id u4mr20595721qab.71.1410424272064; Thu, 11 Sep 2014 01:31:12 -0700 (PDT) Received: by 10.140.94.114 with HTTP; Thu, 11 Sep 2014 01:31:11 -0700 (PDT) In-Reply-To: <5410FDE5.5090804@ieee.org> References: <1410394821-13054-1-git-send-email-roy.qing.li@gmail.com> <5410FDE5.5090804@ieee.org> Date: Thu, 11 Sep 2014 12:31:11 +0400 Message-ID: Subject: Re: [PATCH] libceph: fix a memory leak in handle_watch_notify From: Ilya Dryomov To: Alex Elder Cc: roy.qing.li@gmail.com, Sage Weil , Ceph Development Sender: ceph-devel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: ceph-devel@vger.kernel.org X-Spam-Status: No, score=-9.4 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_HI, RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=ham version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On Thu, Sep 11, 2014 at 5:41 AM, Alex Elder wrote: > On 09/10/2014 07:20 PM, roy.qing.li@gmail.com wrote: >> >> From: Li RongQing >> >> event_work should be freed when adding it to queue failed >> >> Signed-off-by: Li RongQing > > > Looks good. > > Reviewed-by: Alex Elder Hmm, queue_work() returns %false if @work was already on a queue, %true otherwise, so this seems bogus to me. I'd go with something like this (mangled). From c0711eee447b199b1c2193460fce8c9d958f23f4 Mon Sep 17 00:00:00 2001 From: Ilya Dryomov Date: Thu, 11 Sep 2014 12:18:53 +0400 Subject: [PATCH] libceph: don't try checking queue_work() return value queue_work() doesn't "fail to queue", it returns false if work was already on a queue, which can't happen here since we allocate event_work right before we queue it. So don't bother at all. Signed-off-by: Ilya Dryomov --- net/ceph/osd_client.c | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) } diff --git a/net/ceph/osd_client.c b/net/ceph/osd_client.c index 0f569d322405..952e9c254cc7 100644 --- a/net/ceph/osd_client.c +++ b/net/ceph/osd_client.c @@ -2355,26 +2355,21 @@ static void handle_watch_notify(struct ceph_osd_client *osdc, if (event) { event_work = kmalloc(sizeof(*event_work), GFP_NOIO); if (!event_work) { - dout("ERROR: could not allocate event_work\n"); - goto done_err; + pr_err("couldn't allocate event_work\n"); + ceph_osdc_put_event(event); + return; } INIT_WORK(&event_work->work, do_event_work); event_work->event = event; event_work->ver = ver; event_work->notify_id = notify_id; event_work->opcode = opcode; - if (!queue_work(osdc->notify_wq, &event_work->work)) { - dout("WARNING: failed to queue notify event work\n"); - goto done_err; - } + + queue_work(osdc->notify_wq, &event_work->work); } return; -done_err: - ceph_osdc_put_event(event); - return; - bad: pr_err("osdc handle_watch_notify corrupt msg\n");