From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753353AbbJSKEp (ORCPT ); Mon, 19 Oct 2015 06:04:45 -0400 Received: from userp1040.oracle.com ([156.151.31.81]:48379 "EHLO userp1040.oracle.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750876AbbJSKEn (ORCPT ); Mon, 19 Oct 2015 06:04:43 -0400 Message-ID: <5624C004.2040901@oracle.com> Date: Mon, 19 Oct 2015 18:03:48 +0800 From: Bob Liu User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130308 Thunderbird/17.0.4 MIME-Version: 1.0 To: =?windows-1252?Q?Roger_Pau_Monn=E9?= CC: xen-devel@lists.xen.org, david.vrabel@citrix.com, linux-kernel@vger.kernel.org, konrad.wilk@oracle.com, felipe.franciosi@citrix.com, axboe@fb.com, hch@infradead.org, avanzini.arianna@gmail.com, rafal.mielniczuk@citrix.com, boris.ostrovsky@oracle.com, jonathan.davies@citrix.com Subject: Re: [PATCH v3 7/9] xen/blkback: separate ring information out of struct xen_blkif References: <1441456782-31318-1-git-send-email-bob.liu@oracle.com> <1441456782-31318-8-git-send-email-bob.liu@oracle.com> <56128F54.1090907@citrix.com> <56188F4A.3060802@oracle.com> <5624B997.4020907@citrix.com> In-Reply-To: <5624B997.4020907@citrix.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 8bit X-Source-IP: userv0021.oracle.com [156.151.31.71] Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 10/19/2015 05:36 PM, Roger Pau Monné wrote: > El 10/10/15 a les 6.08, Bob Liu ha escrit: >> On 10/05/2015 10:55 PM, Roger Pau Monné wrote: >>> The same for the pool of persistent grants, it should be per-device and >>> not per-ring. >>> >>> And I think this issue is far worse than the others, because a frontend >>> might use a persistent grant on different queues, forcing the backend >>> map the grant several times for each queue, this is not acceptable IMO. >>> >> >> Hi Roger, >> >> I realize it would make things complicate if making persistent grant per-device instead of per-queue. >> Extra locks are required to protect the per-device pool on both blkfront and blkback. > > Yes, I realize this, but without having at least a prototype it's hard > to tell if contention is going to be a problem or not. We already use a > red-black tree to store persistent grants, which should be quite fast > when performing searches. > > IMHO, we are doing things backwards, we should have investigated first > if using per-device was a problem, and if it indeed was a problem then > move to per-queue. Using per-device just required adding locks around > the functions to get/put grants and friends, leaving the data structures > untouched (per-device). > >> AFAIR, there was a discussion before about dropping persistent grant map at all. >> The only reason we left this feature was backward compatibility. >> So that I think we should not complicate xen-block code any more because of a going to be dropped feature. >> >> How about disable feature-persistent if multi-queue was used? > > This is not what we have done in the past, also there's no way for > blkback to tell the frontend that persistent grants and multiqueue > cannot be used at the same time. Blkback puts all supported features on > xenstore before knowing anything about the frontend. > > Also, if you want to do it per-queue instead of per-device the limits > need to be properly adjusted, not just the persistent grants one, but > also the queue of cached free pages. This also implies that each queue > it's going to have it's own LRU and purge task. > Okay, then I'll update with a per-device version. For blkfront there would be two locks used, one for per-device and the other for per-ring. For blkback, an new lock would be added to protect the red-black tree e.g. in add_persistent_gnt(). -- Regards, -Bob