> -----Original Message----- > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Xie, Huawei > Sent: Wednesday, October 21, 2015 8:16 AM > To: Stephen Hemminger; Yuanhan Liu > Cc: dev at dpdk.org; marcel at redhat.com; Michael S. Tsirkin; Changchun > Ouyang > Subject: Re: [dpdk-dev] [PATCH v7 4/8] vhost: rxtx: use queue id instead of > constant ring index > > On 10/21/2015 12:44 PM, Stephen Hemminger wrote: > > On Wed, 21 Oct 2015 11:48:10 +0800 > > Yuanhan Liu <yuanhan.liu at linux.intel.com> wrote: > > > >> > >> +static inline int __attribute__((always_inline)) > >> +is_valid_virt_queue_idx(uint32_t virtq_idx, int is_tx, uint32_t > >> max_qp_idx) > >> +{ > >> + if ((is_tx ^ (virtq_idx & 0x1)) || > >> + (virtq_idx >= max_qp_idx * VIRTIO_QNUM)) > >> + return 0; > >> + > >> + return 1; > >> +} > > minor nits: > > * this doesn't need to be marked as always inline, > > that is as they say in English "shooting a fly with a bazooka" > Stephen: > always_inline "forces" the compiler to inline this function, like a macro. > When should it be used or is it not preferred at all?
I also don't understand what's wrong with using 'always_inline' here. As I understand the author wants compiler to *always inline* that function. So seems perfectly ok to use it here. As I remember just 'inline' is sort of recommendation that compiler is free to ignore. Konstantin > > > * prefer to just return logical result rather than have conditional: > > * for booleans prefer the <stdbool.h> type boolean. > > > > static bool > > is_valid_virt_queue_idx(uint32_t virtq_idx, bool is_tx, uint32_t max_qp_idx) > > { > > return (is_tx ^ (virtq_idx & 1)) || > > virtq_idx >= max_qp_idx * VIRTIO_QNUM; > > } > >