All of lore.kernel.org
 help / color / mirror / Atom feed
From: Qu Wenruo <wqu@suse.com>
To: dsterba@suse.cz, Qu Wenruo <quwenruo.btrfs@gmx.com>,
	Nikolay Borisov <nborisov@suse.com>,
	linux-btrfs@vger.kernel.org, David Sterba <dsterba@suse.com>
Subject: Re: [PATCH v3 2/3] btrfs: remove btrfs_bio_alloc() helper
Date: Mon, 20 Sep 2021 20:42:55 +0800	[thread overview]
Message-ID: <0597fd82-d528-3aae-16c2-9d227a705529@suse.com> (raw)
In-Reply-To: <20210920124126.GK9286@twin.jikos.cz>



On 2021/9/20 20:41, David Sterba wrote:
> On Mon, Sep 20, 2021 at 06:33:14PM +0800, Qu Wenruo wrote:
>>
>>
>> On 2021/9/17 20:49, Nikolay Borisov wrote:
>>>
>>>
>>> On 17.09.21 г. 15:43, David Sterba wrote:
>>>> So should we add another helper that takes the exact number and drop the
>>>> parameter everwhere is 0 so it's just btrfs_io_bio_alloc() with the
>>>> fallback?
>>>
>>> But by adding another helper we just introduce more indirection.
>>>
>>> Actually I'd argue that if 0 is a sane default then BIO_MAX_VECS cannot
>>> be any worse because:
>>>
>>> a) It's a number which is as good as 0
>>> b) It's even named. So this is technically better than a plain 0
>>>
>>
>> Any final call on this?
>>
>> I hope this could be an example for future optional parameters.
>>
>> We have some existing codes using two different inline functions, and
>> both of them call a internal but exported function with "__" prefix.
>>
>> We also have call sites passing all needed parameters just like Nikolay
>> suggested.
> 
> I'm fine with explicitly using BIO_MAX_VECS instead of 0. I'll update it
> in the patch, no need to resend.
> 
Thank you very much.

I'll no longer use this tricky way any more.

Thanks,
Qu


  reply	other threads:[~2021-09-20 12:43 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-15  7:17 [PATCH v3 0/3] btrfs: btrfs_bio and btrfs_io_bio rename Qu Wenruo
2021-09-15  7:17 ` [PATCH v3 1/3] btrfs: rename btrfs_bio to btrfs_io_context Qu Wenruo
2021-09-17 11:19   ` David Sterba
2021-09-17 11:24     ` Qu Wenruo
2021-09-17 11:27       ` Qu Wenruo
2021-09-17 11:33         ` David Sterba
2021-09-15  7:17 ` [PATCH v3 2/3] btrfs: remove btrfs_bio_alloc() helper Qu Wenruo
2021-09-17 12:27   ` Nikolay Borisov
2021-09-17 12:33     ` Qu Wenruo
2021-09-17 12:34       ` Nikolay Borisov
2021-09-17 12:43     ` David Sterba
2021-09-17 12:49       ` Nikolay Borisov
2021-09-20 10:33         ` Qu Wenruo
2021-09-20 12:41           ` David Sterba
2021-09-20 12:42             ` Qu Wenruo [this message]
2021-09-23  5:57   ` Qu Wenruo
2021-09-15  7:17 ` [PATCH v3 3/3] btrfs: rename struct btrfs_io_bio to btrfs_logical_bio Qu Wenruo
2021-09-17 11:39   ` David Sterba
2021-09-20  7:04   ` Nikolay Borisov
2021-09-20 12:23     ` David Sterba
2021-09-20 13:10       ` David Sterba
2021-09-17 11:39 ` [PATCH v3 0/3] btrfs: btrfs_bio and btrfs_io_bio rename David Sterba

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=0597fd82-d528-3aae-16c2-9d227a705529@suse.com \
    --to=wqu@suse.com \
    --cc=dsterba@suse.com \
    --cc=dsterba@suse.cz \
    --cc=linux-btrfs@vger.kernel.org \
    --cc=nborisov@suse.com \
    --cc=quwenruo.btrfs@gmx.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.