From owner-freebsd-stable@FreeBSD.ORG Fri Apr 1 21:04:15 2011 Return-Path: Delivered-To: stable@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 8D0E6106566B; Fri, 1 Apr 2011 21:04:15 +0000 (UTC) (envelope-from to.my.trociny@gmail.com) Received: from mail-fx0-f54.google.com (mail-fx0-f54.google.com [209.85.161.54]) by mx1.freebsd.org (Postfix) with ESMTP id BBAB88FC15; Fri, 1 Apr 2011 21:04:14 +0000 (UTC) Received: by fxm11 with SMTP id 11so3765853fxm.13 for ; Fri, 01 Apr 2011 14:04:13 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:from:to:cc:subject:references:x-comment-to :sender:date:in-reply-to:message-id:user-agent:mime-version :content-type; bh=CxVGeAmE3HAOdZ5OnHd/qKNnoFFe1b5Sq7b3qWxeKUM=; b=PGFoLtYit+ZEeuA4htf/04R4yA7PMCvx+zQgckMm0I68rZGPlanOJoOj7NSau5rvha gTwPSTIAkm2HaCNhwPfS4VB1EN70dcNwEZPA2D1OKeoz5VUgv0pNawSH7wnHtzIsESaj zlc5U+CwYuRk5OLqeMwoe4fvP02wgiZWCLcv8= DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=from:to:cc:subject:references:x-comment-to:sender:date:in-reply-to :message-id:user-agent:mime-version:content-type; b=Xe+DRN7iyLuPQlIa3mj86qDwqYiGh69OSi+4kx/MSQx5JekD+sO2uEa01xBJ4UKi+9 v/kcarertyIfqZ0Q473sIw6IvXWJqo4VLNsgBFDAY6xFcwARQfjf9AqhJzsp+c68NpCr 1rs95Vgo2DaHawGpjKEPlnnLU/hv6RgEHE3bc= Received: by 10.223.10.141 with SMTP id p13mr545807fap.109.1301691853549; Fri, 01 Apr 2011 14:04:13 -0700 (PDT) Received: from localhost ([95.69.172.154]) by mx.google.com with ESMTPS id 17sm891504far.43.2011.04.01.14.04.11 (version=TLSv1/SSLv3 cipher=OTHER); Fri, 01 Apr 2011 14:04:12 -0700 (PDT) From: Mikolaj Golub To: Victor Balada Diaz References: <20110326003348.GQ36706@equilibrium.bsdes.net> <20110401174354.GE1289@equilibrium.bsdes.net> X-Comment-To: Victor Balada Diaz Sender: Mikolaj Golub Date: Sat, 02 Apr 2011 00:04:09 +0300 In-Reply-To: <20110401174354.GE1289@equilibrium.bsdes.net> (Victor Balada Diaz's message of "Fri, 1 Apr 2011 19:43:54 +0200") Message-ID: <86pqp53cqe.fsf@kopusha.home.net> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/23.2 (berkeley-unix) MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" Cc: Kostik Belousov , stable@freebsd.org, Pawel Jakub Dawidek Subject: Re: geli(4) memory leak X-BeenThere: freebsd-stable@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Production branch of FreeBSD source code List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 01 Apr 2011 21:04:15 -0000 --=-=-= On Fri, 1 Apr 2011 19:43:54 +0200 Victor Balada Diaz wrote: VBD> On Sat, Mar 26, 2011 at 01:33:48AM +0100, Victor Balada Diaz wrote: >> Hello, >> >> I'm trying to setup a new geli disk and i'm seeing what looks like a memory leak. >> After initializing the device i've tried to do the dd command from /dev/random >> like this one: >> >> dd if=/dev/random of=/dev/da0p1.eli bs=1m >> VBD> Hello again, VBD> I've found the cause of the memory leak and i attach a patch to fix it. I hope VBD> the patch is good enough to get committed or at least helps someone made a better VBD> patch and commit it. Patched file is src/sys/geom/eli/g_eli.c VBD> The problem happens when you're using data integrity verification and you need VBD> to write more than MAXPHYS. If you look at g_eli_integrity.c:314 you'll VBD> see that geli creates a second request to write all that's needed. VBD> Each of the request get the callback to g_eli_write_done once they're done. The VBD> first request will get up to g_eli.c:209 and find that there are still requests VBD> pending so instead of calling g_io_deliver to notify it's written data, it just VBD> returns and waits until all requests are done to say everything's OK. The problem VBD> is that once you return, you're leaking this g_bio. You can see with vmstat -z how VBD> g_bio increases and never releases memory. VBD> I just destroy the current bio before returning and that prevents the memory leak. For me your patch look correct. But the same issue is for read :-). Also, to avoid the leak I think we can just do g_destroy_bio() before "all sectors" check. See the attached patch (had some testing). -- Mikolaj Golub --=-=-= Content-Type: text/x-patch Content-Disposition: attachment; filename=g_eli.c.patch Index: sys/geom/eli/g_eli.c =================================================================== --- sys/geom/eli/g_eli.c (revision 220168) +++ sys/geom/eli/g_eli.c (working copy) @@ -160,13 +160,13 @@ g_eli_read_done(struct bio *bp) pbp = bp->bio_parent; if (pbp->bio_error == 0) pbp->bio_error = bp->bio_error; + g_destroy_bio(bp); /* * Do we have all sectors already? */ pbp->bio_inbed++; if (pbp->bio_inbed < pbp->bio_children) return; - g_destroy_bio(bp); sc = pbp->bio_to->geom->softc; if (pbp->bio_error != 0) { G_ELI_LOGREQ(0, pbp, "%s() failed", __func__); @@ -202,6 +202,7 @@ g_eli_write_done(struct bio *bp) if (bp->bio_error != 0) pbp->bio_error = bp->bio_error; } + g_destroy_bio(bp); /* * Do we have all sectors already? */ @@ -215,7 +216,6 @@ g_eli_write_done(struct bio *bp) pbp->bio_error); pbp->bio_completed = 0; } - g_destroy_bio(bp); /* * Write is finished, send it up. */ --=-=-=--