From patchwork Wed Jun 26 00:29:54 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: majianpeng X-Patchwork-Id: 2781511 Return-Path: X-Original-To: patchwork-ceph-devel@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.19.201]) by patchwork1.web.kernel.org (Postfix) with ESMTP id AE4779F245 for ; Wed, 26 Jun 2013 00:29:58 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 1E952201F8 for ; Wed, 26 Jun 2013 00:29:58 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 97303201F6 for ; Wed, 26 Jun 2013 00:29:57 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751678Ab3FZA34 (ORCPT ); Tue, 25 Jun 2013 20:29:56 -0400 Received: from mail-pa0-f41.google.com ([209.85.220.41]:54113 "EHLO mail-pa0-f41.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751590Ab3FZA3z (ORCPT ); Tue, 25 Jun 2013 20:29:55 -0400 Received: by mail-pa0-f41.google.com with SMTP id bj3so13508180pad.28 for ; Tue, 25 Jun 2013 17:29:55 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=date:from:to:cc:reply-to:subject:references:x-priority:x-guid :x-has-attach:x-mailer:mime-version:message-id:content-type :content-transfer-encoding; bh=XToIUzSbdcgKAAxJzrXzQ5WmPfpmycJf/VGeGFR2Jxc=; b=hx3GlsI2zI1KSbaokBwdt7TQ/d3FAVR/1AjiZERHxB5Sd4flMfQn6O8fZpGiIBVwyl mGl4LoonoVH3nKPZzsMwWksV/K1TdkA/mvH1nniSZNWCn6eXD5WiwxQh5F7+vEqcMOsf +FLbeqskJL11Fz+yqqg3qVLlwLDJDGQN8SKVsl8odz5qHZNqmWYndG8S15R6lBzuQbf+ mzhN7Nt0AZRowvsxKJUscsXrEIajRVZA9u8cT+xLYgzn1MEXOX0sG7hkKolyJRvmUzXX MLXI80Oclc2DjuFdKYOSelWp/xhuOi/NsOezkXioLlgZLL3lDak7qLZI/sUSJtKP0Kjs coPQ== X-Received: by 10.66.160.163 with SMTP id xl3mr2138621pab.201.1372206595158; Tue, 25 Jun 2013 17:29:55 -0700 (PDT) Received: from majianpeng ([218.242.10.182]) by mx.google.com with ESMTPSA id ag4sm25370256pbc.20.2013.06.25.17.29.51 for (version=TLSv1 cipher=RC4-SHA bits=128/128); Tue, 25 Jun 2013 17:29:54 -0700 (PDT) Date: Wed, 26 Jun 2013 08:29:54 +0800 From: majianpeng To: sage Cc: ceph-devel Reply-To: majianpeng Subject: Re: Re: [PATCH][TRIVIAL] ceph: Free the previous caps if alloc failed. References: <201306251527405618532@gmail.com>, X-Priority: 3 X-GUID: 8DEF28E7-0EA6-4D71-8ED0-E2C80CF84D53 X-Has-Attach: no X-Mailer: Foxmail 7.0.1.90[en] Mime-Version: 1.0 Message-ID: <201306260829496656833@gmail.com> Sender: ceph-devel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: ceph-devel@vger.kernel.org X-Spam-Status: No, score=-8.1 required=5.0 tests=BAYES_00, DKIM_ADSP_CUSTOM_MED, DKIM_SIGNED, FREEMAIL_FROM, RCVD_IN_DNSWL_HI, RP_MATCHES_RCVD, T_DKIM_INVALID, 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 >I think in this case we don't want to drop other caps. This basically >means we weren't able to maintain the desired level of reserves, but we >will continue to try to fill it periodically, and not reaching the desired >point is not a reason to throw out what we have. You'll note that the one >caller ignores the return value.. > I see.But the code don't. > for (i = have; i < need; i++) { > cap = kmem_cache_alloc(ceph_cap_cachep, GFP_NOFS); > if (!cap) { > ret = -ENOMEM; > goto out_alloc_count; > } > list_add(&cap->caps_item, &newcaps); > alloc++; > } > BUG_ON(have + alloc != need); For the caps which allocated, those can't add into '&mdsc->caps_list'. So if allocate failed,it will cause memory leak. By your thought, the code maybe like BTY, The function which call ceph_reserve_caps don't care the result. And from you comment, this will periodically do.So i think the function proto will be viod ceph_reserve_caps() { } How about this? Thanks! Jianpeng Ma >sage > > >On Tue, 25 Jun 2013, majianpeng wrote: > >> Signed-off-by: Jianpeng Ma >> --- >> fs/ceph/caps.c | 5 +++++ >> 1 file changed, 5 insertions(+) >> >> diff --git a/fs/ceph/caps.c b/fs/ceph/caps.c >> index da0f9b8..626b0ec 100644 >> --- a/fs/ceph/caps.c >> +++ b/fs/ceph/caps.c >> @@ -203,6 +203,11 @@ out_alloc_count: >> /* we didn't manage to reserve as much as we needed */ >> pr_warning("reserve caps ctx=%p ENOMEM need=%d got=%d\n", >> ctx, need, have); >> + if (alloc) { >> + struct ceph_cap *tmp; >> + list_for_each_entry_safe(cap, tmp, &newcaps, caps_item) >> + kmem_cache_free(ceph_cap_cachep, cap); >> + } >> return ret; >> } >> >> -- >> 1.8.1.2 >> diff --git a/fs/ceph/caps.c b/fs/ceph/caps.c index da0f9b8..d5d5eca 100644 --- a/fs/ceph/caps.c +++ b/fs/ceph/caps.c @@ -176,12 +176,11 @@ int ceph_reserve_caps(struct ceph_mds_client *mdsc, cap = kmem_cache_alloc(ceph_cap_cachep, GFP_NOFS); if (!cap) { ret = -ENOMEM; - goto out_alloc_count; + break; } list_add(&cap->caps_item, &newcaps); alloc++; } - BUG_ON(have + alloc != need); spin_lock(&mdsc->caps_list_lock); mdsc->caps_total_count += alloc;