From owner-freebsd-xen@freebsd.org Mon Jun 18 12:41:44 2018 Return-Path: Delivered-To: freebsd-xen@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 6FBF51004AE5 for ; Mon, 18 Jun 2018 12:41:44 +0000 (UTC) (envelope-from pratiy0100@gmail.com) Received: from mail-wm0-f43.google.com (mail-wm0-f43.google.com [74.125.82.43]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (Client CN "smtp.gmail.com", Issuer "Google Internet Authority G2" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id F2AE07D065; Mon, 18 Jun 2018 12:41:43 +0000 (UTC) (envelope-from pratiy0100@gmail.com) Received: by mail-wm0-f43.google.com with SMTP id p126-v6so13846827wmb.2; Mon, 18 Jun 2018 05:41:43 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:from:date:message-id:subject:to:cc; bh=42j14pnwHPSY8vEOQ0+/bTPrSeYeZ+PSQq4Sx4tWyc0=; b=e6GMKnl4mJIl8pUKccos8qVdZt03GNani2Td3G/qrvrDkiKeQZHfl6aDXkFWEQXmRr doFWlkxseVUXK0OdxZnRNPvw0nJHFv3OXSIKrze9QNzXRENRaZZwHAJ/iQ0Vc21APCun ecidJ4TFLlN3MIIMmPUVbXna3be2tYQATUCC+D9bArsiLGYWYQ1uAb5HUcRkUrlXRAyi y5KBdRwal2IJ8V4vWFUpzR9k8v+z2PxZZQvfz+0I/ePWqx17NYSrQtWbwD/1NZS/i7yX G2zRzLy4fvjBGEajMeebfHwxTV9jtisan/gGvdK0gNqfv2jyYvEZY6+EQAuXD1giqWPM aefw== X-Gm-Message-State: APt69E0249gyhpQE+fiD7FJaQVf71aAWC9Z10fRBbPu7QK4N+a4Bba0B xdIqHF1Ru40EohbcsDU33gcwWWmd X-Google-Smtp-Source: ADUXVKIjB1ldAfP/BQhkV+44KUNX78W3VZ+QYKAzIEj7O/p1I5Xcy3bkHTs0TazD1XoBR3WGKfgPbA== X-Received: by 2002:a50:a247:: with SMTP id 65-v6mr11065360edl.189.1529325696672; Mon, 18 Jun 2018 05:41:36 -0700 (PDT) Received: from mail-wm0-f42.google.com (mail-wm0-f42.google.com. [74.125.82.42]) by smtp.gmail.com with ESMTPSA id f2-v6sm5821379edq.28.2018.06.18.05.41.36 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 18 Jun 2018 05:41:36 -0700 (PDT) Received: by mail-wm0-f42.google.com with SMTP id v131-v6so15315097wma.1; Mon, 18 Jun 2018 05:41:36 -0700 (PDT) X-Received: by 2002:a50:e70d:: with SMTP id a13-v6mr11201661edn.158.1529325696437; Mon, 18 Jun 2018 05:41:36 -0700 (PDT) MIME-Version: 1.0 From: Pratyush Yadav Date: Mon, 18 Jun 2018 18:11:00 +0530 X-Gmail-Original-Message-ID: Message-ID: Subject: gnttab_end_foreign_access_ref() leaking grant entries? To: FreeBSD-Xen Cc: =?UTF-8?Q?Roger_Pau_Monn=C3=A9?= Content-Type: text/plain; charset="UTF-8" X-BeenThere: freebsd-xen@freebsd.org X-Mailman-Version: 2.1.26 Precedence: list List-Id: Discussion of the freebsd port to xen - implementation and usage List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 18 Jun 2018 12:41:44 -0000 Hi everyone, I was looking at gnttab_end_foreign_access_ref() and I notice that while gnttab_end_foreign_access_ref() ends the foreign access, it does not free the grant reference. gnttab_end_foreign_access() free the reference by calling put_free_entry(ref). gnttab_end_foreign_access_references() also frees the grant entries. Shouldn't gnttab_end_foreign_access_ref() also free the grant entry? It is an inconsistency at best and a bug at worst. Is it ok if I submit a patch that calls put_free_entry(ref) in gnttab_end_foreign_access_ref()? -- Regards, Pratyush Yadav From owner-freebsd-xen@freebsd.org Mon Jun 18 13:03:06 2018 Return-Path: Delivered-To: freebsd-xen@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 46A501005EE3 for ; Mon, 18 Jun 2018 13:03:06 +0000 (UTC) (envelope-from royger@freebsd.org) Received: from smtp.freebsd.org (smtp.freebsd.org [96.47.72.83]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client CN "smtp.freebsd.org", Issuer "Let's Encrypt Authority X3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id EC97B7DD1E; Mon, 18 Jun 2018 13:03:05 +0000 (UTC) (envelope-from royger@freebsd.org) Received: from localhost (80.red-88-8-240.dynamicip.rima-tde.net [88.8.240.80]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) (Authenticated sender: royger) by smtp.freebsd.org (Postfix) with ESMTPSA id 614E813569; Mon, 18 Jun 2018 13:03:05 +0000 (UTC) (envelope-from royger@freebsd.org) Date: Mon, 18 Jun 2018 15:02:55 +0200 From: Roger Pau =?utf-8?B?TW9ubsOp?= To: Pratyush Yadav Cc: FreeBSD-Xen Subject: Re: gnttab_end_foreign_access_ref() leaking grant entries? Message-ID: <20180618130255.3eflhi62m7wv7jpv@mac.bytemobile.com> References: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: NeoMutt/20180512 X-BeenThere: freebsd-xen@freebsd.org X-Mailman-Version: 2.1.26 Precedence: list List-Id: Discussion of the freebsd port to xen - implementation and usage List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 18 Jun 2018 13:03:06 -0000 On Mon, Jun 18, 2018 at 06:11:00PM +0530, Pratyush Yadav wrote: > Hi everyone, > > I was looking at gnttab_end_foreign_access_ref() and I notice that > while gnttab_end_foreign_access_ref() ends the foreign access, it does > not free the grant reference. > > gnttab_end_foreign_access() free the reference by calling put_free_entry(ref). > > gnttab_end_foreign_access_references() also frees the grant entries. > > Shouldn't gnttab_end_foreign_access_ref() also free the grant entry? > It is an inconsistency at best and a bug at worst. I think the point of using gnttab_end_foreign_access_ref is that it doesn't free the reference. The naming convention of grant reference related functions is just horrible, but I think this one is actually clear, it just removes the foreign access permissions of this reference, but it doesn't free it. I think I would rather rename gnttab_end_foreign_access to gnttab_end_foreign_access_and_free (and the same with the rest of the end_foreign_ family of functions). This is more descriptive. In any case, this is not a bug. Current netfront code makes use of this function in order to keep a pool of grant references, so by changing gnttab_end_foreign_access_ref to free the references you will break netfront. And now I realize this is something that you will have to take into account for the busdma grant reference project. You will have to introduce a flag that allows creating a dmamap with pre-allocated references that are not freed until the map is destroyed. But let's focus on that after the initial implementation is done. Roger.