Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1633547 > unrolled thread

Re: [PATCH 02/11] blk: make the bioset rescue_workqueue optional.

Started byNeilBrown <neilb@suse.com>
First post2017-05-01 07:10 +0200
Last post2017-05-02 05:40 +0200
Articles 3 — 2 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.


Contents

  Re: [PATCH 02/11] blk: make the bioset rescue_workqueue optional. NeilBrown <neilb@suse.com> - 2017-05-01 07:10 +0200
    Re: [PATCH 02/11] blk: make the bioset rescue_workqueue optional. Jens Axboe <axboe@kernel.dk> - 2017-05-01 16:10 +0200
      Re: [PATCH 02/11] blk: make the bioset rescue_workqueue optional. NeilBrown <neilb@suse.com> - 2017-05-02 05:40 +0200

#1633547 — Re: [PATCH 02/11] blk: make the bioset rescue_workqueue optional.

FromNeilBrown <neilb@suse.com>
Date2017-05-01 07:10 +0200
SubjectRe: [PATCH 02/11] blk: make the bioset rescue_workqueue optional.
Message-ID<tCbKV-4Lj-3@gated-at.bofh.it>

[Multipart message — attachments visible in raw view] — view raw

On Mon, Apr 24 2017, Christoph Hellwig wrote:

> On Mon, Apr 24, 2017 at 11:51:01AM +1000, NeilBrown wrote:
>> 
>> I was following the existing practice exemplified by
>> bioset_create_nobvec().
>
> Which is pretty ugly to start with..

That is a matter of personal taste.
As such, it is up to the maintainer to change it if they want it
changed.

>
>> By not changing the signature of the function, I can avoid touching
>> quite a few places where it is called.
>
> There are 13 callers of bioset_create and one caller of
> bioset_create_nobvec, and your series touches many of those.
>
> So just adding a flags argument to bioset_create and passing
> BIOSET_NEED_BVECS and BIOSET_NEED_RESUER flags to it doesn't seem
> to much of an effort, and it's going to create a much nicer and easier
> to extend interface.

If someone else submitted a patch to discard bioset_create_nobvec in
favour of BIOSET_NEED_BVECS and got it accepted, then I would rebase my
series on that.  As it is, I'm basing my patches on the style currently
present in the tree.

Of course, if Jens says he'll only take my patches if I change to style
to match your preference, I'll do that.

Thanks,
NeilBrown

[toc] | [next] | [standalone]


#1633678

FromJens Axboe <axboe@kernel.dk>
Date2017-05-01 16:10 +0200
Message-ID<tCkbv-1IO-7@gated-at.bofh.it>
In reply to#1633547
On 04/30/2017 11:00 PM, NeilBrown wrote:
> On Mon, Apr 24 2017, Christoph Hellwig wrote:
> 
>> On Mon, Apr 24, 2017 at 11:51:01AM +1000, NeilBrown wrote:
>>>
>>> I was following the existing practice exemplified by
>>> bioset_create_nobvec().
>>
>> Which is pretty ugly to start with..
> 
> That is a matter of personal taste.
> As such, it is up to the maintainer to change it if they want it
> changed.
> 
>>
>>> By not changing the signature of the function, I can avoid touching
>>> quite a few places where it is called.
>>
>> There are 13 callers of bioset_create and one caller of
>> bioset_create_nobvec, and your series touches many of those.
>>
>> So just adding a flags argument to bioset_create and passing
>> BIOSET_NEED_BVECS and BIOSET_NEED_RESUER flags to it doesn't seem
>> to much of an effort, and it's going to create a much nicer and easier
>> to extend interface.
> 
> If someone else submitted a patch to discard bioset_create_nobvec in
> favour of BIOSET_NEED_BVECS and got it accepted, then I would rebase my
> series on that.  As it is, I'm basing my patches on the style currently
> present in the tree.
> 
> Of course, if Jens says he'll only take my patches if I change to style
> to match your preference, I'll do that.

I generally tend to prefer tree wide cleanups to improve our APIs, even
if it does cause an extra bit of pain. Would you mind doing that as a
prep patch?

-- 
Jens Axboe

[toc] | [prev] | [next] | [standalone]


#1634101

FromNeilBrown <neilb@suse.com>
Date2017-05-02 05:40 +0200
Message-ID<tCwPn-Wb-3@gated-at.bofh.it>
In reply to#1633678

[Multipart message — attachments visible in raw view] — view raw

On Mon, May 01 2017, Jens Axboe wrote:

> On 04/30/2017 11:00 PM, NeilBrown wrote:
>> On Mon, Apr 24 2017, Christoph Hellwig wrote:
>> 
>>> On Mon, Apr 24, 2017 at 11:51:01AM +1000, NeilBrown wrote:
>>>>
>>>> I was following the existing practice exemplified by
>>>> bioset_create_nobvec().
>>>
>>> Which is pretty ugly to start with..
>> 
>> That is a matter of personal taste.
>> As such, it is up to the maintainer to change it if they want it
>> changed.
>> 
>>>
>>>> By not changing the signature of the function, I can avoid touching
>>>> quite a few places where it is called.
>>>
>>> There are 13 callers of bioset_create and one caller of
>>> bioset_create_nobvec, and your series touches many of those.
>>>
>>> So just adding a flags argument to bioset_create and passing
>>> BIOSET_NEED_BVECS and BIOSET_NEED_RESUER flags to it doesn't seem
>>> to much of an effort, and it's going to create a much nicer and easier
>>> to extend interface.
>> 
>> If someone else submitted a patch to discard bioset_create_nobvec in
>> favour of BIOSET_NEED_BVECS and got it accepted, then I would rebase my
>> series on that.  As it is, I'm basing my patches on the style currently
>> present in the tree.
>> 
>> Of course, if Jens says he'll only take my patches if I change to style
>> to match your preference, I'll do that.
>
> I generally tend to prefer tree wide cleanups to improve our APIs, even
> if it does cause an extra bit of pain. Would you mind doing that as a
> prep patch?

OK, will do.

I have rebased and fixed up a couple of issues.  Will repost shortly.

Thanks,
NeilBrown

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web