On 04/30/2017 11:00 PM, NeilBrown wrote:

On Mon, Apr 24 2017, Christoph Hellwig wrote:

quoted

On Mon, Apr 24, 2017 at 11:51:01AM +1000, NeilBrown wrote:

quoted

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.

quoted

quoted

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