DPDK patches and discussions
 help / color / mirror / Atom feed
From: Chas Williams <3chas3@gmail.com>
To: Ferruh Yigit <ferruh.yigit@intel.com>,
	Luca Boccassi <bluca@debian.org>,
	maxime.coquelin@redhat.com, dev@dpdk.org
Subject: Re: [dpdk-dev] [PATCH 1/2] net/virtio: do not re-enter clean up routines
Date: Fri, 2 Nov 2018 11:19:49 -0400	[thread overview]
Message-ID: <b571e391-f403-6ec5-aa16-646b6bc7ed5c@gmail.com> (raw)
In-Reply-To: <a60db72f-8547-5428-1c21-0ed77767ac45@intel.com>



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 <ciwillia@brocade.com>
> 
> <...>
> 
>>> @@ -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.

> [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;
> 

  reply	other threads:[~2018-11-02 15:19 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-07-17 23:05 [dpdk-dev] [PATCH 0/2] Some small changes to net/virtio Charles (Chas) Williams
2017-07-17 23:05 ` [dpdk-dev] [PATCH 1/2] net/virtio: do not re-enter clean up routines Charles (Chas) Williams
2018-11-01 14:45   ` Luca Boccassi
2018-11-02  9:57     ` Maxime Coquelin
2018-11-02 11:03       ` Maxime Coquelin
2018-11-02 11:14         ` Luca Boccassi
2018-11-02 14:33     ` Ferruh Yigit
2018-11-02 15:19       ` Chas Williams [this message]
2018-11-02 16:48         ` Maxime Coquelin
2018-11-02 17:17           ` Ferruh Yigit
2017-07-17 23:05 ` [dpdk-dev] [PATCH 2/2] net/virtio: register/unregister intr handler on start/stop Charles (Chas) Williams
2017-07-18 11:50 ` [dpdk-dev] [PATCH 0/2] Some small changes to net/virtio Ferruh Yigit
2017-07-18 11:52   ` Charles (Chas) Williams
2018-08-15 13:51     ` Luca Boccassi
2018-08-27 13:41       ` Ferruh Yigit
2018-08-27 13:48         ` Ferruh Yigit
2018-08-27 14:52           ` Gavin Hu

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=b571e391-f403-6ec5-aa16-646b6bc7ed5c@gmail.com \
    --to=3chas3@gmail.com \
    --cc=bluca@debian.org \
    --cc=dev@dpdk.org \
    --cc=ferruh.yigit@intel.com \
    --cc=maxime.coquelin@redhat.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).