From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-ob0-f177.google.com (mail-ob0-f177.google.com [209.85.214.177]) by dpdk.org (Postfix) with ESMTP id 268859AA3 for ; Wed, 25 Feb 2015 11:22:16 +0100 (CET) Received: by mail-ob0-f177.google.com with SMTP id wp18so2755792obc.8 for ; Wed, 25 Feb 2015 02:22:15 -0800 (PST) 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=ny9v4UARjEs+Dv5PR1U3O/WQTRkv+7JQNKWZBMjcQkI=; b=cLweBVdQJ4KvQbNMw7VN4gpFqBI0vF/o/Kk8wy/fpsi+QODO6btFidWksukBFCdTtw v0hH/kKonZ9u3LFn+GqNb3A1xp7aNYO7DaF8mhWMpo4/cTdcE4PdkaZZY82NQRRzWanN O2G8NbmxvMUJ7iiqjtzXlvrJ9y1R7RTmJ0TCamoyk3uxw4Ar6PvUvCSRUzig/Jj0hxQ1 NMc8hQJK7jfez/q7YulFysXT6SnQ2cSXa3yHAo6gWW/zcGbnxud3+NNoBClmHvRMls20 uaCb+Kobni6kLEiB2O9QFe8ccnfSjvsV20EGSL1CRqyMQEGENs1UusxyYwDdZVCybjKp Qu5Q== X-Gm-Message-State: ALoCoQnc5D4KyBb6bgjOVTT7cqMMvKr8KPlNkXtBeJ/TJXktp5iCyCLtJeThaUaovNv+JtqKPtgZ MIME-Version: 1.0 X-Received: by 10.182.153.71 with SMTP id ve7mr1680490obb.76.1424859735591; Wed, 25 Feb 2015 02:22:15 -0800 (PST) Received: by 10.76.133.162 with HTTP; Wed, 25 Feb 2015 02:22:15 -0800 (PST) In-Reply-To: References: <1424710542-14637-1-git-send-email-danny.zhou@intel.com> <1424710542-14637-6-git-send-email-danny.zhou@intel.com> Date: Wed, 25 Feb 2015 11:22:15 +0100 Message-ID: From: David Marchand To: "Zhou, Danny" Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable X-Content-Filtered-By: Mailman/MimeDel 2.1.15 Cc: "dev@dpdk.org" Subject: Re: [dpdk-dev] [PATCH v5 5/6] eal: add per rx queue interrupt handling based on VFIO X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 25 Feb 2015 10:22:16 -0000 Hello Danny, On Wed, Feb 25, 2015 at 7:58 AM, Zhou, Danny wrote: > > +int > +rte_intr_wait_rx_pkt(struct rte_intr_handle *intr_handle, uint8_t > queue_id) > +{ > + struct epoll_event ev; > + unsigned numfds =3D 0; > + > + if (!intr_handle || intr_handle->fd < 0 || intr_handle->uio_cfg_f= d > < 0) > + return -1; > + if (queue_id >=3D VFIO_MAX_QUEUE_ID) > + return -1; > + > + /* create epoll fd */ > + int pfd =3D epoll_create(1); > + if (pfd < 0) { > + RTE_LOG(ERR, EAL, "Cannot create epoll instance\n"); > + return -1; > + } > > > > Why recreate the epoll instance at each call to this function ? > > > > DZ: To avoid recreating the epoll instance for each queue, the struct > rte_intr_handle(or a new structure added to ethdev) > > should be extended by adding fields storing per-queue pfd. This way, it > could reduce user/kernel context switch overhead > > when calling epoll_create() each time. > > > > Sounds good? > You don't need a epfd per queue. And hardcoding epfd =3D=3D eventfd will gi= ve a not very usable api. Plus, epoll is something linux-specific, so you can't move it out of eal/linux. I suppose you need an abstraction here (and in the future we could add something for bsd ?). > > Looking at this patchset, I think there is a design issue. > > eal does not need to know about portid neither queueid. > > > > eal can provide an api to retrieve the interrupt fds, configure an epoll > instance, wait on an epoll instance etc... > > ethdev is then responsible to setup the mapping between port id / queue i= d > and interrupt fds by asking the eal about those fds. > > > > This would result in an eal api even simpler and we could add other fds i= n > a single epoll fd for other uses. > > > > DZ: The queueid is just an index to the queue related eventfd array store= d > in EAL. If this array is still in the EAL and ethdev can apply for it and > setup mapping for certain queue, there > > might be issue for multiple-process use case where the fd resources > allocated for secondary process are not freed if the secondary process > exits unexpectedly. > Not sure I follow you. If a secondary process exits, the eventfds created in primary process should still be valid and reusable. Why would you need to free them ? Something to do with vfio ? > > Probably we can setup the eventfd array inside ethdev, and we just need > EAL API to wait for ethdev=E2=80=99fd. So application invokes ethdev API = with > portid and queueid, and ethdev calls eal > > API to wait on a ethdev fd which correlates with the specified portid and > queueid. > > > > Sounds ok to you? > eventfds creation can not be handled by ethdev, since it needs infrastructure and informations from within the eal/linux. Again, do we need an abstraction ? ethdev must be the one that does the mappings between port/queue and eventfds (or any object that represents a way to wake up for a given port/queue). --=20 David Marchand