netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Gal Pressman <galpress@amazon.com>
To: Jason Gunthorpe <jgg@ziepe.ca>
Cc: Joe Perches <joe@perches.com>,
	Bart Van Assche <bvanassche@acm.org>,
	Stephen Warren <swarren@wwwdotorg.org>,
	Tariq Toukan <tariqt@mellanox.com>, <xavier.huwei@huawei.com>,
	<netdev@vger.kernel.org>, <linux-rdma@vger.kernel.org>,
	Doug Ledford <dledford@redhat.com>,
	"Stephen Warren" <swarren@nvidia.com>,
	Christoph Hellwig <hch@lst.de>,
	Andrew Morton <akpm@linux-foundation.org>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Jonathan Corbet <corbet@lwn.net>, <linux-kernel@vger.kernel.org>
Subject: Re: rfc: bool structure members (was Re: [PATCH V3] net/mlx4: Get rid of page operation after dma_alloc_coherent)
Date: Tue, 25 Dec 2018 10:41:16 +0200	[thread overview]
Message-ID: <71005ff2-e6b0-ca97-0d4b-6e6f7b491760@amazon.com> (raw)
In-Reply-To: <20181224231228.GB17036@ziepe.ca>

On 25-Dec-18 01:12, Jason Gunthorpe wrote:
> On Sun, Dec 23, 2018 at 06:42:20PM +0200, Gal Pressman wrote:
>> On 22-Dec-18 01:52, Jason Gunthorpe wrote:
>>> On Thu, Dec 20, 2018 at 09:12:43PM -0800, Joe Perches wrote:
>>>> Care to submit a coding_style.rst patch or
>>>> improve the one below this?
>>>
>>> I took yours and revised it a little bit. I spent some time looking at
>>> assembly and decided to drop the performance note, the number of cases
>>> that run into overhead seems pretty small and probably already
>>> requires !! to be correct. There is also an equally unlikely gain, ie
>>> 'if (a & b)' optimizes a tiny bit better for bool types.
>>>
>>> I also added a small intro on bool, as I know some people are
>>> unfamiliar with C11 _Bool and might think bool is just '#define bool
>>> u8'
>>>
>>> (for those added to the cc) I'm looking at cases, like the patch that
>>> spawned this, where the struct has a single bool and no performance
>>> considerations. As CH said, seeing that get converted to int due to
>>> checkpatch is worse than having used bool. Using u8 won't make this
>>> struct smaller or faster.
>>>
>>
>> Since a "Using bool" section is added, perhaps it's worth documenting the bool
>> usage as a function parameter [1]?
>>
>> [1] https://www.spinics.net/lists/linux-rdma/msg72336.html
> 
> I'm not really sure how to express that as something concrete.. That
> specific case clearly called out for a flags as it was a widely used
> API - maybe less spread out cases like static functions or something
> are OK??
> 
> Jason
> 

Sounds reasonable, sometimes adding flags and enum for a single bool function
parameter is a bit too much IMO. For a widely used API it makes sense.

  reply	other threads:[~2018-12-25  8:41 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-12-19 18:20 [PATCH V3] net/mlx4: Get rid of page operation after dma_alloc_coherent Stephen Warren
2018-12-20 17:43 ` Jason Gunthorpe
2018-12-20 17:44   ` Christoph Hellwig
2018-12-20 17:49     ` Bart Van Assche
2018-12-21  2:25       ` rfc: bool structure members (was Re: [PATCH V3] net/mlx4: Get rid of page operation after dma_alloc_coherent) Joe Perches
2018-12-21  2:40         ` Andrew Morton
2018-12-21  3:31         ` Jason Gunthorpe
2018-12-21  5:12           ` Joe Perches
2018-12-21 23:52             ` Jason Gunthorpe
2018-12-23 16:42               ` Gal Pressman
2018-12-23 16:53                 ` Al Viro
2018-12-24 22:08                   ` Jason Gunthorpe
2018-12-24 23:12                 ` Jason Gunthorpe
2018-12-25  8:41                   ` Gal Pressman [this message]
2019-01-02 16:29   ` [PATCH V3] net/mlx4: Get rid of page operation after dma_alloc_coherent Stephen Warren
2019-01-03  7:48     ` Christoph Hellwig
2019-01-03 14:32     ` Tariq Toukan

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=71005ff2-e6b0-ca97-0d4b-6e6f7b491760@amazon.com \
    --to=galpress@amazon.com \
    --cc=akpm@linux-foundation.org \
    --cc=bvanassche@acm.org \
    --cc=corbet@lwn.net \
    --cc=dledford@redhat.com \
    --cc=hch@lst.de \
    --cc=jgg@ziepe.ca \
    --cc=joe@perches.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-rdma@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=swarren@nvidia.com \
    --cc=swarren@wwwdotorg.org \
    --cc=tariqt@mellanox.com \
    --cc=torvalds@linux-foundation.org \
    --cc=xavier.huwei@huawei.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).