From: Santosh Shukla <sshukla@mvista.com>
To: "Xie, Huawei" <huawei.xie@intel.com>
Cc: dpdk <dev@dpdk.org>
Subject: Re: [dpdk-dev] [PATCH v7 2/4] virtio: Introduce config RTE_VIRTIO_INC_VECTOR
Date: Mon, 22 Feb 2016 09:44:07 +0530 [thread overview]
Message-ID: <CAAyOgsbB4hpJn9U3bY8Vo+tQKUtQZGaA3ONb9iMxFMrt5+4meg@mail.gmail.com> (raw)
In-Reply-To: <C37D651A908B024F974696C65296B57B4C5EC09E@SHSMSX101.ccr.corp.intel.com>
On Mon, Feb 22, 2016 at 7:33 AM, Xie, Huawei <huawei.xie@intel.com> wrote:
> On 2/19/2016 2:42 PM, Yuanhan Liu wrote:
>> On Fri, Feb 19, 2016 at 10:16:42AM +0530, Santosh Shukla wrote:
>>> On Tue, Feb 16, 2016 at 8:35 AM, Yuanhan Liu
>>> <yuanhan.liu@linux.intel.com> wrote:
>>>> On Mon, Feb 15, 2016 at 04:48:36PM +0530, Santosh Shukla wrote:
>>>>> Hi Yuanhan,
>>>>>
>>>>> On Mon, Feb 15, 2016 at 4:27 PM, Yuanhan Liu
>>>>> <yuanhan.liu@linux.intel.com> wrote:
>>>>>> On Mon, Feb 15, 2016 at 03:22:11PM +0530, Santosh Shukla wrote:
>>>>>>> Hi Yuanhan,
>>>>>>>
>>>>>>> I guess you are back from vacation.
>>>>>>>
>>>>>>> Can you pl. review this patch, Except this patch, rest of patches
>>>>>>> received ack-by:
>>>>>> I had a quick glimpse of the comments from Thomas: he made a good point.
>>>>>> I will have a deeper thought tomorrow, to see what I can do to fix it.
>>>>>>
>>>>> I agree to what Thomas pointed out about runtime mode switch (vectored
>>>>> vs non-vectored). I have a proposal in my mind and Like to know you
>>>>> opinion:
>>>>>
>>>>> - need for apis like is_arch_support_vec().
>>>>>
>>>>> if (is_arch_support_vec())
>>>>> simpple_xxxx = 1 /* Switch code path to vector mode */
>>>>> else
>>>>> simple_xxxx = 0 /* Switch code path to non-vector mode */
>>>>>
>>>>> That api should reside to arch file. i.e.. arch like i686/arm{for
>>>>> implementation not exist so say no supported} will return 0 and for
>>>>> x86_64 = 1
>>>> I was thinking that Thomas meant to something like below (like what
>>>> we did at rte_memcpy.h):
>>>>
>>>> #ifdef RTE_MACHINE_CPUFLAG_SSE (or whatever)
>>>>
>>>> /* with vec here */
>>>>
>>>> #else
>>>>
>>>> /* without vec here */
>>>>
>>>> #endif
>>>>
>>>> I mean, you have to bypass the build first; otherwise, you can't
>>>> go that further to runtime, right?
>>>>
>>> I meant: move virtio_recv_pkt_vec() implementation in
>>> lib/libeal_rte/xx/include/arch/xx/virtio_vec.h. virtio driver to check
>>> for CPUFLAG supported or not and then use _recv_pkt() call back
>>> function from arch files. This approach will avoid #ifdef ARCH
>>> clutter.
>> Moving virtio stuff to eal looks wrong to me.
>>
>> Huawei, please comment on this.
>>
>> --yliu
>>
>
> This issue doesn't apply to virtio driver only but to all other PMDs,
> unless they are assumed to run on only one arch. As we are close to
> release, for the time being, i prefer to use RTE_MACHINE_CPUFLAG_. Later
> we look for other elegant solutions, like moving different arch specific
> optimizations into the arch directory under driver/virtio/ directory?
> Other thoughts?
>
Creating arch specifics files in driver/virtio/: approach look okay to
me. It look alike to my proposal except eal. I choose eal so that one
api and its implementation stays in arch files, no ifdef clutter. I
guess - Same doable in virtio directory too, create arch files and
keep arch specific implementation their.
so, +1 to approach.
>
>
next prev parent reply other threads:[~2016-02-22 4:14 UTC|newest]
Thread overview: 25+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-02-07 13:51 [dpdk-dev] [PATCH v7 0/4] Add virtio support for arm/arm64 Santosh Shukla
2016-02-07 13:51 ` [dpdk-dev] [PATCH v7 1/4] eal/linux: never check iopl for arm Santosh Shukla
2016-02-18 5:26 ` Santosh Shukla
2016-02-07 13:51 ` [dpdk-dev] [PATCH v7 2/4] virtio: Introduce config RTE_VIRTIO_INC_VECTOR Santosh Shukla
2016-02-07 21:25 ` Thomas Monjalon
2016-02-08 5:45 ` Santosh Shukla
[not found] ` <CAAyOgsZO+6+kFZZZM203fPR3AmVYB0v7j3-f+DawZOCuR-AVvQ@mail.gmail.com>
[not found] ` <20160215105743.GB21426@yliu-dev.sh.intel.com>
[not found] ` <CAAyOgsaT5TcsPfum8x6yzAJAz=5N+c5QebEn7KCyJn7oK=VMsw@mail.gmail.com>
2016-02-16 3:05 ` Yuanhan Liu
2016-02-19 4:46 ` Santosh Shukla
2016-02-19 6:42 ` Yuanhan Liu
2016-02-22 2:03 ` Xie, Huawei
2016-02-22 4:14 ` Santosh Shukla [this message]
2016-02-22 10:22 ` Thomas Monjalon
2016-02-07 13:51 ` [dpdk-dev] [PATCH v7 3/4] eal/linux: vfio: ignore mapping for ioport region Santosh Shukla
2016-02-08 9:15 ` Burakov, Anatoly
2016-02-18 5:26 ` Santosh Shukla
2016-02-07 13:51 ` [dpdk-dev] [PATCH v7 4/4] eal/linux: vfio: add pci ioport support Santosh Shukla
2016-02-08 8:51 ` David Marchand
2016-02-08 9:40 ` Santosh Shukla
2016-02-21 14:17 ` [dpdk-dev] [PATCH v9 0/3] Add virtio support for arm/arm64 Santosh Shukla
2016-02-21 14:17 ` [dpdk-dev] [PATCH v9 1/3] eal/linux: never check iopl for arm Santosh Shukla
2016-02-21 14:18 ` [dpdk-dev] [PATCH v9 2/3] eal/linux: vfio: ignore mapping for ioport region Santosh Shukla
2016-02-21 14:18 ` [dpdk-dev] [PATCH v9 3/3] eal/linux: vfio: add pci ioport support Santosh Shukla
2016-02-22 5:41 ` [dpdk-dev] [PATCH v9 0/3] Add virtio support for arm/arm64 Yuanhan Liu
2016-02-23 6:11 ` Santosh Shukla
2016-02-24 10:45 ` Thomas Monjalon
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CAAyOgsbB4hpJn9U3bY8Vo+tQKUtQZGaA3ONb9iMxFMrt5+4meg@mail.gmail.com \
--to=sshukla@mvista.com \
--cc=dev@dpdk.org \
--cc=huawei.xie@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).