From mboxrd@z Thu Jan 1 00:00:00 1970 From: Maxime Coquelin Subject: Re: [PATCH 1/2] net/virtio: do not re-enter clean up routines Date: Fri, 2 Nov 2018 17:48:05 +0100 Message-ID: <2997986d-11f6-c170-087f-ff08c067d93c@redhat.com> References: <1500332723-10812-1-git-send-email-ciwillia@brocade.com> <1500332723-10812-2-git-send-email-ciwillia@brocade.com> <1541083532.4849.8.camel@debian.org> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit To: Chas Williams <3chas3@gmail.com>, Ferruh Yigit , Luca Boccassi , dev@dpdk.org Return-path: Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by dpdk.org (Postfix) with ESMTP id 479E71B107 for ; Fri, 2 Nov 2018 17:48:10 +0100 (CET) In-Reply-To: Content-Language: en-US List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On 11/2/18 4:19 PM, Chas Williams wrote: > > > On 11/02/2018 10:33 AM, Ferruh Yigit wrote: >> On 11/1/2018 2:45 PM, Luca Boccassi wrote: >>> On Mon, 2017-07-17 at 19:05 -0400, Charles (Chas) Williams wrote: >>>> .dev_uninit calls .dev_stop and .dev_close.  The work that is done in >>>> those routines doesn't need repeated.  Use started and opened to >>>> track >>>> the adapter's status. >>>> >>>> Signed-off-by: Chas Williams >> >> <...> >> >>>> @@ -253,7 +254,7 @@ struct virtio_hw { >>>>       uint64_t    req_guest_features; >>>>       uint64_t    guest_features; >>>>       uint32_t    max_queue_pairs; >>>> -    uint16_t    started; >>>> +    bool        started; >>>>       uint16_t    max_mtu; >>>>       uint16_t    vtnet_hdr_size; >>>>       uint8_t        vlan_strip; >>>> @@ -268,6 +269,7 @@ struct virtio_hw { >>>>       struct virtio_pci_common_cfg *common_cfg; >>>>       struct virtio_net_config *dev_cfg; >>>>       void        *virtio_user_dev; >>>> +    bool        opened; >> >> This is already merged into next-net-virtio, but I would like to >> hightlight the >> checkpatch warning about `bool` usage in struct [1]. >> Briefly it suggests preferring primitive data types against `bool` in >> structures >> because its size is not clear. >> >> What do you think about it, do you have strong opinion to have them as >> bool? > > Yes, I suppose I do.  bool is the "proper" representation for yes/no. > The arguments cited aren't convincing. > > The size of bool is the size of bool. The compiler gets to make that > decision.  I don't get to decide the size of int either.  The size and > alignemnt of bool should be optimal because your compiler probably knows > more about the target than you do.  If your compiler can't figure out > alignment in a structure, please fix the compiler. > > bool is a primitive data type per the C99 standard. I would like to keep it as bool too. >> [1] >> CHECK:BOOL_MEMBER: Avoid using bool structure members because of possible >> alignment issues - see: https://lkml.org/lkml/2017/11/21/384 >> #85: FILE: drivers/net/virtio/virtio_pci.h:234: >> +       bool        started; >> >> CHECK:BOOL_MEMBER: Avoid using bool structure members because of possible >> alignment issues - see: https://lkml.org/lkml/2017/11/21/384 >> #93: FILE: drivers/net/virtio/virtio_pci.h:260: >> +       bool        opened; >> BTW, I don't get this warning when running checkpatch, what kenrel version is it coming from? Thanks, Maxime