Re: [PATCH 00/13] convert block layer to bioset_init()/mempool_init()

2018-05-22 Thread Mike Snitzer
On Tue, May 22 2018 at  2:41am -0400,
Christoph Hellwig  wrote:

> On Mon, May 21, 2018 at 07:38:55PM -0400, Kent Overstreet wrote:
> > On Mon, May 21, 2018 at 02:24:32PM -0400, Mike Snitzer wrote:
> > > Every single data structure change in this series should be reviewed for
> > > unforeseen alignment consequences.  Jens seemed to say that is
> > > worthwhile.  Not sure if he'll do it or we divide it up.  If we divide
> > > it up a temp topic branch should be published for others to inspect.
> > > 
> > > Could be alignment hasn't been a historic concern for a bunch of the
> > > data structures changed in this series.. if so then all we can do is fix
> > > up any obvious potential for false sharing.
> > 
> > Honestly, I almost never worry about alignment... the very few times I do 
> > care,
> > I use __cacheline_aligned_in_smp.
> 
> And that generally is the right stratgey.  If Mike really doesn't want
> a change we can just open code the kmalloc for the bio set there, the
> important point is that we should not keep the old API around for no
> good reason.

Again, I never said I didn't want these changes.  I just thought it
worthwhile to mention the potential for alignment quirks arising.

Reality is false sharing is quite uncommon.  So my concern was/is pretty
niche and unlikely to be applicable.

That said, I did take the opportunity to look at all the DM structures
that were modified.  The bio_set and mempool_t structs are so large that
they span multiple cachelines as is.  So I focused on eliminating
unnecessary spanning of cachelines (by non-bio_set and non-mempool_t
members) and eliminated most holes in DM structs.  This is the result:
http://people.redhat.com/msnitzer/dm-4.18-struct-reorg/

Compare before.txt vs after_kent.txt vs after_mike.txt

Nothing staggering or special.  Just something I'll add once Kent's
latest changes land.

But, I also eliminated 2 4-byte holes in struct bio_set, Jens please
feel free to pick this up (if you think it useful):

From: Mike Snitzer 
Subject: [PATCH] block: eliminate 2 4-byte holes in bio_set structure

Signed-off-by: Mike Snitzer 
---
 include/linux/bio.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/bio.h b/include/linux/bio.h
index 5e472fc..e6fc692 100644
--- a/include/linux/bio.h
+++ b/include/linux/bio.h
@@ -735,7 +735,6 @@ static inline void bio_inc_remaining(struct bio *bio)
 
 struct bio_set {
struct kmem_cache *bio_slab;
-   unsigned int front_pad;
 
mempool_t bio_pool;
mempool_t bvec_pool;
@@ -743,6 +742,7 @@ struct bio_set {
mempool_t bio_integrity_pool;
mempool_t bvec_integrity_pool;
 #endif
+   unsigned int front_pad;
 
/*
 * Deadlock avoidance for stacking block drivers: see comments in



Re: [PATCH 00/13] convert block layer to bioset_init()/mempool_init()

2018-05-22 Thread Christoph Hellwig
On Mon, May 21, 2018 at 07:38:55PM -0400, Kent Overstreet wrote:
> On Mon, May 21, 2018 at 02:24:32PM -0400, Mike Snitzer wrote:
> > Every single data structure change in this series should be reviewed for
> > unforeseen alignment consequences.  Jens seemed to say that is
> > worthwhile.  Not sure if he'll do it or we divide it up.  If we divide
> > it up a temp topic branch should be published for others to inspect.
> > 
> > Could be alignment hasn't been a historic concern for a bunch of the
> > data structures changed in this series.. if so then all we can do is fix
> > up any obvious potential for false sharing.
> 
> Honestly, I almost never worry about alignment... the very few times I do 
> care,
> I use __cacheline_aligned_in_smp.

And that generally is the right stratgey.  If Mike really doesn't want
a change we can just open code the kmalloc for the bio set there, the
important point is that we should not keep the old API around for no
good reason.


Re: [PATCH 00/13] convert block layer to bioset_init()/mempool_init()

2018-05-21 Thread Kent Overstreet
On Mon, May 21, 2018 at 02:24:32PM -0400, Mike Snitzer wrote:
> Every single data structure change in this series should be reviewed for
> unforeseen alignment consequences.  Jens seemed to say that is
> worthwhile.  Not sure if he'll do it or we divide it up.  If we divide
> it up a temp topic branch should be published for others to inspect.
> 
> Could be alignment hasn't been a historic concern for a bunch of the
> data structures changed in this series.. if so then all we can do is fix
> up any obvious potential for false sharing.

Honestly, I almost never worry about alignment... the very few times I do care,
I use __cacheline_aligned_in_smp.

If alignment is a concern in any of those structs, there really ought to be a
comment indicating it. I very much doubt anything I touched was performance
sensitive enough for it to be an issue, though. And if there is a performance
impact, it should be oughtweighed by the reduced pointer chasing.

If you disagree, I don't mind leaving the device mapper patch out, it really
makes no difference to me. I could glance over for alignment issues but I feel
like my analysis would not be terribly valuable to you considering I've already
said my position on alignment is "meh, don't care" :)


Re: [PATCH 00/13] convert block layer to bioset_init()/mempool_init()

2018-05-21 Thread Mike Snitzer
On Mon, May 21 2018 at  1:37pm -0400,
Kent Overstreet  wrote:

> 
> Uh, you came across as upset and paranoid to me too. Chalk it up to email? :)

Awesome.  See how easy it is to make someone with purely constructive
questions and feedback come off as upset and paranoid?

The tipping point occurs when bait is set with:
"It's not like ".
Then:
"Let's focus on getting it reviewed, rather than pontificate on what
could potentially go all wrong with this."

Message received: less pontificating, more doing!

And here I thought that focusing on what could go wrong (across all code
touched) was review.  But OK, I'm the one that made this all weird ;)

It is what it is at this point.

> I personally don't care, I have no horse in this race. This particular patch
> series wasn't my idea, Christoph wanted all these conversions done so
> bioset_create() could be deleted. If you want us to hold off on the dm patch 
> for
> awhile until someone can get around to testing it or whatever (since I don't
> have tests for everything I pushed) that would be perfectly fine by me.

As I clarified already: this isn't about DM.

Every single data structure change in this series should be reviewed for
unforeseen alignment consequences.  Jens seemed to say that is
worthwhile.  Not sure if he'll do it or we divide it up.  If we divide
it up a temp topic branch should be published for others to inspect.

Could be alignment hasn't been a historic concern for a bunch of the
data structures changed in this series.. if so then all we can do is fix
up any obvious potential for false sharing.

Mike


Re: [PATCH 00/13] convert block layer to bioset_init()/mempool_init()

2018-05-21 Thread Kent Overstreet
On Mon, May 21, 2018 at 12:09:14PM -0400, Mike Snitzer wrote:
> On Mon, May 21 2018 at 11:36am -0400,
> Jens Axboe  wrote:
> 
> > On 5/21/18 9:18 AM, Mike Snitzer wrote:
> > > On Mon, May 21 2018 at 11:09am -0400,
> > > Jens Axboe  wrote:
> > > 
> > >> On 5/21/18 9:04 AM, Mike Snitzer wrote:
> > >>> On Mon, May 21 2018 at 10:52am -0400,
> > >>> Jens Axboe  wrote:
> > >>>
> ...
> >  IMHO you're making a big deal out of something that should not be.
> > >>>
> > >>> I raised an issue that had seemingly not been considered at all.  Not
> > >>> making a big deal.  Raising it for others' benefit.
> > >>>
> >  If the dm bits are that sensitive and cache line honed to perfection
> >  already due to previous regressions in that area, then it might
> >  not be a bad idea to have some compile checks for false cacheline
> >  sharing between sensitive members, or spilling of a sub-struct
> >  into multiple cachelines.
> > 
> >  It's not like this was pushed behind your back. It's posted for
> >  review. It's quite possible the net change is a win for dm. Let's
> >  focus on getting it reviewed, rather than pontificate on what
> >  could potentially go all wrong with this.
> > >>>
> > >>> Why are you making this personal?  Or purely about DM?  I'm merely
> > >>> pointing out this change isn't something that can be given a quick
> > >>> blanket "looks good".
> > >>
> > >> I'm not making this personal at all?! You raised a (valid) concern,
> > >> I'm merely stating why I don't think it's a high risk issue. I'm
> > >> assuming your worry is related to dm, as those are the reports
> > >> that would ultimately land on your desk.
> > > 
> > > Then we'll just agree to disagree with what this implies: "It's not like
> > > this was pushed behind your back."
> > 
> > I'm afraid you've lost me now - it was not pushed behind your back,
> > it was posted for review, with you on the CC list. Not trying to
> > be deliberately dense here, I just don't see what our disagreement is.
> 
> You're having an off day ;)  Mondays and all?
> 
> I just raised an alignment concern that needs to be considered during
> review by all stakeholders.  Wasn't upset at all.  Maybe my email tone
> came off otherwise?
> 
> And then you got confused by me pointing out how it was weird for you to
> suggest I felt this stuff was pushed behind my back.. and went on to
> think I really think that.  It's almost like you're a confused hypnotist
> seeding me with paranoid dilutions. ;)

Uh, you came across as upset and paranoid to me too. Chalk it up to email? :)

I personally don't care, I have no horse in this race. This particular patch
series wasn't my idea, Christoph wanted all these conversions done so
bioset_create() could be deleted. If you want us to hold off on the dm patch for
awhile until someone can get around to testing it or whatever (since I don't
have tests for everything I pushed) that would be perfectly fine by me.


Re: [PATCH 00/13] convert block layer to bioset_init()/mempool_init()

2018-05-21 Thread Jens Axboe
On 5/21/18 10:09 AM, Mike Snitzer wrote:
> On Mon, May 21 2018 at 11:36am -0400,
> Jens Axboe  wrote:
> 
>> On 5/21/18 9:18 AM, Mike Snitzer wrote:
>>> On Mon, May 21 2018 at 11:09am -0400,
>>> Jens Axboe  wrote:
>>>
 On 5/21/18 9:04 AM, Mike Snitzer wrote:
> On Mon, May 21 2018 at 10:52am -0400,
> Jens Axboe  wrote:
>
> ...
>> IMHO you're making a big deal out of something that should not be.
>
> I raised an issue that had seemingly not been considered at all.  Not
> making a big deal.  Raising it for others' benefit.
>
>> If the dm bits are that sensitive and cache line honed to perfection
>> already due to previous regressions in that area, then it might
>> not be a bad idea to have some compile checks for false cacheline
>> sharing between sensitive members, or spilling of a sub-struct
>> into multiple cachelines.
>>
>> It's not like this was pushed behind your back. It's posted for
>> review. It's quite possible the net change is a win for dm. Let's
>> focus on getting it reviewed, rather than pontificate on what
>> could potentially go all wrong with this.
>
> Why are you making this personal?  Or purely about DM?  I'm merely
> pointing out this change isn't something that can be given a quick
> blanket "looks good".

 I'm not making this personal at all?! You raised a (valid) concern,
 I'm merely stating why I don't think it's a high risk issue. I'm
 assuming your worry is related to dm, as those are the reports
 that would ultimately land on your desk.
>>>
>>> Then we'll just agree to disagree with what this implies: "It's not like
>>> this was pushed behind your back."
>>
>> I'm afraid you've lost me now - it was not pushed behind your back,
>> it was posted for review, with you on the CC list. Not trying to
>> be deliberately dense here, I just don't see what our disagreement is.
> 
> You're having an off day ;)  Mondays and all?
> 
> I just raised an alignment concern that needs to be considered during
> review by all stakeholders.  Wasn't upset at all.  Maybe my email tone
> came off otherwise?
> 
> And then you got confused by me pointing out how it was weird for you to
> suggest I felt this stuff was pushed behind my back.. and went on to
> think I really think that.  It's almost like you're a confused hypnotist
> seeding me with paranoid dilutions. ;)
> 
> /me waits for Jens to snap his fingers so he can just slowly step away
> from this line of discussion that is solidly dead now...

Mike, wtf are you talking about.

>>> Reality is I'm fine with the change.  Just think there is follow-on work
>>> (now or later) that is needed.
>>
>> It's not hard to run the quick struct layout checks or alignment. If
>> there's a concern, that should be done now, instead of being deferred to
>> later. There's no point merging something that we expect to have
>> follow-on work. If that's the case, then it should not be merged. There
>> are no dependencies in the patchset, except that the last patch
>> obviously can't be applied until all of the previous ones are in.
> 
> Cool, sounds good.
> 
>> I took a quick look at the struct mapped_device layout, which I'm
>> assuming is the most important on the dm side. It's pretty big to
>> begin with, obviously this makes it bigger since we're now
>> embedding the bio_sets. On my config, doesn't show any false sharing
>> that would be problematic, or layout changes that would worry me. FWIW.
> 
> Great, thanks, do you happen to have a tree you can push so others can
> get a quick compile and look at the series fully applied?

I do not, I simply ran a quick analysis of the layout, then applied the
full patchset, and repeated that exercise. Literally a 5 minute thing.
I haven't applied the series, so haven't pushed anything out.

> BTW, I'm upstream DM maintainer but I have a role in caring for related
> subsystems (e.g. block core, etc) in downstream releases.. *cough* RHEL.
> So this alignment concern wasn't ever purely about DM.

I tend to care about that too...

-- 
Jens Axboe



Re: [PATCH 00/13] convert block layer to bioset_init()/mempool_init()

2018-05-21 Thread Mike Snitzer
On Mon, May 21 2018 at 11:36am -0400,
Jens Axboe  wrote:

> On 5/21/18 9:18 AM, Mike Snitzer wrote:
> > On Mon, May 21 2018 at 11:09am -0400,
> > Jens Axboe  wrote:
> > 
> >> On 5/21/18 9:04 AM, Mike Snitzer wrote:
> >>> On Mon, May 21 2018 at 10:52am -0400,
> >>> Jens Axboe  wrote:
> >>>
...
>  IMHO you're making a big deal out of something that should not be.
> >>>
> >>> I raised an issue that had seemingly not been considered at all.  Not
> >>> making a big deal.  Raising it for others' benefit.
> >>>
>  If the dm bits are that sensitive and cache line honed to perfection
>  already due to previous regressions in that area, then it might
>  not be a bad idea to have some compile checks for false cacheline
>  sharing between sensitive members, or spilling of a sub-struct
>  into multiple cachelines.
> 
>  It's not like this was pushed behind your back. It's posted for
>  review. It's quite possible the net change is a win for dm. Let's
>  focus on getting it reviewed, rather than pontificate on what
>  could potentially go all wrong with this.
> >>>
> >>> Why are you making this personal?  Or purely about DM?  I'm merely
> >>> pointing out this change isn't something that can be given a quick
> >>> blanket "looks good".
> >>
> >> I'm not making this personal at all?! You raised a (valid) concern,
> >> I'm merely stating why I don't think it's a high risk issue. I'm
> >> assuming your worry is related to dm, as those are the reports
> >> that would ultimately land on your desk.
> > 
> > Then we'll just agree to disagree with what this implies: "It's not like
> > this was pushed behind your back."
> 
> I'm afraid you've lost me now - it was not pushed behind your back,
> it was posted for review, with you on the CC list. Not trying to
> be deliberately dense here, I just don't see what our disagreement is.

You're having an off day ;)  Mondays and all?

I just raised an alignment concern that needs to be considered during
review by all stakeholders.  Wasn't upset at all.  Maybe my email tone
came off otherwise?

And then you got confused by me pointing out how it was weird for you to
suggest I felt this stuff was pushed behind my back.. and went on to
think I really think that.  It's almost like you're a confused hypnotist
seeding me with paranoid dilutions. ;)

/me waits for Jens to snap his fingers so he can just slowly step away
from this line of discussion that is solidly dead now...

> > Reality is I'm fine with the change.  Just think there is follow-on work
> > (now or later) that is needed.
> 
> It's not hard to run the quick struct layout checks or alignment. If
> there's a concern, that should be done now, instead of being deferred to
> later. There's no point merging something that we expect to have
> follow-on work. If that's the case, then it should not be merged. There
> are no dependencies in the patchset, except that the last patch
> obviously can't be applied until all of the previous ones are in.

Cool, sounds good.

> I took a quick look at the struct mapped_device layout, which I'm
> assuming is the most important on the dm side. It's pretty big to
> begin with, obviously this makes it bigger since we're now
> embedding the bio_sets. On my config, doesn't show any false sharing
> that would be problematic, or layout changes that would worry me. FWIW.

Great, thanks, do you happen to have a tree you can push so others can
get a quick compile and look at the series fully applied?

BTW, I'm upstream DM maintainer but I have a role in caring for related
subsystems (e.g. block core, etc) in downstream releases.. *cough* RHEL.
So this alignment concern wasn't ever purely about DM.

Mike


Re: [PATCH 00/13] convert block layer to bioset_init()/mempool_init()

2018-05-21 Thread Jens Axboe
On 5/21/18 9:18 AM, Mike Snitzer wrote:
> On Mon, May 21 2018 at 11:09am -0400,
> Jens Axboe  wrote:
> 
>> On 5/21/18 9:04 AM, Mike Snitzer wrote:
>>> On Mon, May 21 2018 at 10:52am -0400,
>>> Jens Axboe  wrote:
>>>
 On 5/21/18 8:47 AM, Mike Snitzer wrote:
> On Mon, May 21 2018 at 10:36am -0400,
> Jens Axboe  wrote:
>
>> On 5/21/18 8:31 AM, Mike Snitzer wrote:
>>> On Mon, May 21 2018 at 10:19am -0400,
>>> Jens Axboe  wrote:
>>>
 On 5/21/18 8:03 AM, Mike Snitzer wrote:
> On Sun, May 20 2018 at  6:25pm -0400,
> Kent Overstreet  wrote:
>
>> Jens - this series does the rest of the conversions that Christoph 
>> wanted, and
>> drops bioset_create().
>>
>> Only lightly tested, but the changes are pretty mechanical. Based on 
>> your
>> for-next tree.
>
> By switching 'mempool_t *' to 'mempool_t' and 'bio_set *' to 'bio_set'
> you've altered the alignment of members in data structures.  So I'll
> need to audit all the data structures you've modified in DM.
>
> Could we get the backstory on _why_ you're making this change?
> Would go a long way to helping me appreciate why this is a good use of
> anyone's time.

 Yeah, it's in the first series, it gets rid of a pointer indirection.
>>>
>>> "Allows mempools to be embedded in other structs, getting rid of a
>>> pointer indirection from allocation fastpaths."
>>>
>>> So this is about using contiguous memory or avoiding partial allocation
>>> failure?  Or both?
>>>
>>> Or more to it?  Just trying to fully appreciate the theory behind the
>>> perceived associated benefit.
>>
>> It's about avoiding a pointer indirection. Instead of having to
>> follow a pointer to get to that struct, it's simple offset math off
>> your main structure.
>>
>>> I do think the increased risk of these embedded bio_set and mempool_t
>>> themselves crossing cachelines, or struct members that follow them doing
>>> so, really detracts from these types of changes.
>>
>> Definitely something to look out for, though most of them should be
>> per-dev structures and not in-flight structures. That makes it a bit
>> less sensitive. But can't hurt to audit the layouts and adjust if
>> necessary. This is why it's posted for review :-)
>
> This isn't something that is easily caught upfront.  Yes we can all be
> busy little beavers with pahole to audit alignment.  But chances are
> most people won't do it.
>
> Reality is there is potential for a regression due to false sharing to
> creep in if a hot struct member suddenly starts straddling a cacheline.
> That type of NUMA performance killer is pretty insidious and somewhat
> tedious to hunt down even when looking for it with specialized tools:
> https://joemario.github.io/blog/2016/09/01/c2c-blog/

 IMHO you're making a big deal out of something that should not be.
>>>
>>> I raised an issue that had seemingly not been considered at all.  Not
>>> making a big deal.  Raising it for others' benefit.
>>>
 If the dm bits are that sensitive and cache line honed to perfection
 already due to previous regressions in that area, then it might
 not be a bad idea to have some compile checks for false cacheline
 sharing between sensitive members, or spilling of a sub-struct
 into multiple cachelines.

 It's not like this was pushed behind your back. It's posted for
 review. It's quite possible the net change is a win for dm. Let's
 focus on getting it reviewed, rather than pontificate on what
 could potentially go all wrong with this.
>>>
>>> Why are you making this personal?  Or purely about DM?  I'm merely
>>> pointing out this change isn't something that can be given a quick
>>> blanket "looks good".
>>
>> I'm not making this personal at all?! You raised a (valid) concern,
>> I'm merely stating why I don't think it's a high risk issue. I'm
>> assuming your worry is related to dm, as those are the reports
>> that would ultimately land on your desk.
> 
> Then we'll just agree to disagree with what this implies: "It's not like
> this was pushed behind your back."

I'm afraid you've lost me now - it was not pushed behind your back,
it was posted for review, with you on the CC list. Not trying to
be deliberately dense here, I just don't see what our disagreement is.

> Reality is I'm fine with the change.  Just think there is follow-on work
> (now or later) that is needed.

It's not hard to run the quick struct layout checks or alignment. If
there's a concern, that should be done now, instead of being deferred to
later. There's no point merging something that we expect to have
follow-on work. If that's the 

Re: [PATCH 00/13] convert block layer to bioset_init()/mempool_init()

2018-05-21 Thread Jens Axboe
On 5/21/18 9:12 AM, David Sterba wrote:
> On Mon, May 21, 2018 at 08:19:58AM -0600, Jens Axboe wrote:
>> On 5/21/18 8:03 AM, Mike Snitzer wrote:
>>> On Sun, May 20 2018 at  6:25pm -0400,
>>> Kent Overstreet  wrote:
>>>
 Jens - this series does the rest of the conversions that Christoph wanted, 
 and
 drops bioset_create().

 Only lightly tested, but the changes are pretty mechanical. Based on your
 for-next tree.
>>>
>>> By switching 'mempool_t *' to 'mempool_t' and 'bio_set *' to 'bio_set'
>>> you've altered the alignment of members in data structures.  So I'll
>>> need to audit all the data structures you've modified in DM.
>>>
>>> Could we get the backstory on _why_ you're making this change?
>>> Would go a long way to helping me appreciate why this is a good use of
>>> anyone's time.
>>
>> Yeah, it's in the first series, it gets rid of a pointer indirection.
> 
> This should to be also mentioned the changelog of each patch. There are
> 12 subsystems changed, this could be about 10 maintainers and I guess
> everybody has the same question why the change is made.

Agree, the justification should be in this series as well, of course.
Kent, might not be a bad idea to resend with a more descriptive
cover letter.

-- 
Jens Axboe



Re: [PATCH 00/13] convert block layer to bioset_init()/mempool_init()

2018-05-21 Thread Mike Snitzer
On Mon, May 21 2018 at 11:09am -0400,
Jens Axboe  wrote:

> On 5/21/18 9:04 AM, Mike Snitzer wrote:
> > On Mon, May 21 2018 at 10:52am -0400,
> > Jens Axboe  wrote:
> > 
> >> On 5/21/18 8:47 AM, Mike Snitzer wrote:
> >>> On Mon, May 21 2018 at 10:36am -0400,
> >>> Jens Axboe  wrote:
> >>>
>  On 5/21/18 8:31 AM, Mike Snitzer wrote:
> > On Mon, May 21 2018 at 10:19am -0400,
> > Jens Axboe  wrote:
> >
> >> On 5/21/18 8:03 AM, Mike Snitzer wrote:
> >>> On Sun, May 20 2018 at  6:25pm -0400,
> >>> Kent Overstreet  wrote:
> >>>
>  Jens - this series does the rest of the conversions that Christoph 
>  wanted, and
>  drops bioset_create().
> 
>  Only lightly tested, but the changes are pretty mechanical. Based on 
>  your
>  for-next tree.
> >>>
> >>> By switching 'mempool_t *' to 'mempool_t' and 'bio_set *' to 'bio_set'
> >>> you've altered the alignment of members in data structures.  So I'll
> >>> need to audit all the data structures you've modified in DM.
> >>>
> >>> Could we get the backstory on _why_ you're making this change?
> >>> Would go a long way to helping me appreciate why this is a good use of
> >>> anyone's time.
> >>
> >> Yeah, it's in the first series, it gets rid of a pointer indirection.
> >
> > "Allows mempools to be embedded in other structs, getting rid of a
> > pointer indirection from allocation fastpaths."
> >
> > So this is about using contiguous memory or avoiding partial allocation
> > failure?  Or both?
> >
> > Or more to it?  Just trying to fully appreciate the theory behind the
> > perceived associated benefit.
> 
>  It's about avoiding a pointer indirection. Instead of having to
>  follow a pointer to get to that struct, it's simple offset math off
>  your main structure.
> 
> > I do think the increased risk of these embedded bio_set and mempool_t
> > themselves crossing cachelines, or struct members that follow them doing
> > so, really detracts from these types of changes.
> 
>  Definitely something to look out for, though most of them should be
>  per-dev structures and not in-flight structures. That makes it a bit
>  less sensitive. But can't hurt to audit the layouts and adjust if
>  necessary. This is why it's posted for review :-)
> >>>
> >>> This isn't something that is easily caught upfront.  Yes we can all be
> >>> busy little beavers with pahole to audit alignment.  But chances are
> >>> most people won't do it.
> >>>
> >>> Reality is there is potential for a regression due to false sharing to
> >>> creep in if a hot struct member suddenly starts straddling a cacheline.
> >>> That type of NUMA performance killer is pretty insidious and somewhat
> >>> tedious to hunt down even when looking for it with specialized tools:
> >>> https://joemario.github.io/blog/2016/09/01/c2c-blog/
> >>
> >> IMHO you're making a big deal out of something that should not be.
> > 
> > I raised an issue that had seemingly not been considered at all.  Not
> > making a big deal.  Raising it for others' benefit.
> > 
> >> If the dm bits are that sensitive and cache line honed to perfection
> >> already due to previous regressions in that area, then it might
> >> not be a bad idea to have some compile checks for false cacheline
> >> sharing between sensitive members, or spilling of a sub-struct
> >> into multiple cachelines.
> >>
> >> It's not like this was pushed behind your back. It's posted for
> >> review. It's quite possible the net change is a win for dm. Let's
> >> focus on getting it reviewed, rather than pontificate on what
> >> could potentially go all wrong with this.
> > 
> > Why are you making this personal?  Or purely about DM?  I'm merely
> > pointing out this change isn't something that can be given a quick
> > blanket "looks good".
> 
> I'm not making this personal at all?! You raised a (valid) concern,
> I'm merely stating why I don't think it's a high risk issue. I'm
> assuming your worry is related to dm, as those are the reports
> that would ultimately land on your desk.

Then we'll just agree to disagree with what this implies: "It's not like
this was pushed behind your back."

Reality is I'm fine with the change.  Just think there is follow-on work
(now or later) that is needed.

Enough said.


Re: [PATCH 00/13] convert block layer to bioset_init()/mempool_init()

2018-05-21 Thread David Sterba
On Mon, May 21, 2018 at 08:19:58AM -0600, Jens Axboe wrote:
> On 5/21/18 8:03 AM, Mike Snitzer wrote:
> > On Sun, May 20 2018 at  6:25pm -0400,
> > Kent Overstreet  wrote:
> > 
> >> Jens - this series does the rest of the conversions that Christoph wanted, 
> >> and
> >> drops bioset_create().
> >>
> >> Only lightly tested, but the changes are pretty mechanical. Based on your
> >> for-next tree.
> > 
> > By switching 'mempool_t *' to 'mempool_t' and 'bio_set *' to 'bio_set'
> > you've altered the alignment of members in data structures.  So I'll
> > need to audit all the data structures you've modified in DM.
> > 
> > Could we get the backstory on _why_ you're making this change?
> > Would go a long way to helping me appreciate why this is a good use of
> > anyone's time.
> 
> Yeah, it's in the first series, it gets rid of a pointer indirection.

This should to be also mentioned the changelog of each patch. There are
12 subsystems changed, this could be about 10 maintainers and I guess
everybody has the same question why the change is made.

The conversion is not exactly the same in all patches, the simple
pointer -> static variable can be possibly covered by the same generic
text but as Mike points out the alignment changes should be at least
mentioned for consideration otherwise.


Re: [PATCH 00/13] convert block layer to bioset_init()/mempool_init()

2018-05-21 Thread Jens Axboe
On 5/21/18 9:04 AM, Mike Snitzer wrote:
> On Mon, May 21 2018 at 10:52am -0400,
> Jens Axboe  wrote:
> 
>> On 5/21/18 8:47 AM, Mike Snitzer wrote:
>>> On Mon, May 21 2018 at 10:36am -0400,
>>> Jens Axboe  wrote:
>>>
 On 5/21/18 8:31 AM, Mike Snitzer wrote:
> On Mon, May 21 2018 at 10:19am -0400,
> Jens Axboe  wrote:
>
>> On 5/21/18 8:03 AM, Mike Snitzer wrote:
>>> On Sun, May 20 2018 at  6:25pm -0400,
>>> Kent Overstreet  wrote:
>>>
 Jens - this series does the rest of the conversions that Christoph 
 wanted, and
 drops bioset_create().

 Only lightly tested, but the changes are pretty mechanical. Based on 
 your
 for-next tree.
>>>
>>> By switching 'mempool_t *' to 'mempool_t' and 'bio_set *' to 'bio_set'
>>> you've altered the alignment of members in data structures.  So I'll
>>> need to audit all the data structures you've modified in DM.
>>>
>>> Could we get the backstory on _why_ you're making this change?
>>> Would go a long way to helping me appreciate why this is a good use of
>>> anyone's time.
>>
>> Yeah, it's in the first series, it gets rid of a pointer indirection.
>
> "Allows mempools to be embedded in other structs, getting rid of a
> pointer indirection from allocation fastpaths."
>
> So this is about using contiguous memory or avoiding partial allocation
> failure?  Or both?
>
> Or more to it?  Just trying to fully appreciate the theory behind the
> perceived associated benefit.

 It's about avoiding a pointer indirection. Instead of having to
 follow a pointer to get to that struct, it's simple offset math off
 your main structure.

> I do think the increased risk of these embedded bio_set and mempool_t
> themselves crossing cachelines, or struct members that follow them doing
> so, really detracts from these types of changes.

 Definitely something to look out for, though most of them should be
 per-dev structures and not in-flight structures. That makes it a bit
 less sensitive. But can't hurt to audit the layouts and adjust if
 necessary. This is why it's posted for review :-)
>>>
>>> This isn't something that is easily caught upfront.  Yes we can all be
>>> busy little beavers with pahole to audit alignment.  But chances are
>>> most people won't do it.
>>>
>>> Reality is there is potential for a regression due to false sharing to
>>> creep in if a hot struct member suddenly starts straddling a cacheline.
>>> That type of NUMA performance killer is pretty insidious and somewhat
>>> tedious to hunt down even when looking for it with specialized tools:
>>> https://joemario.github.io/blog/2016/09/01/c2c-blog/
>>
>> IMHO you're making a big deal out of something that should not be.
> 
> I raised an issue that had seemingly not been considered at all.  Not
> making a big deal.  Raising it for others' benefit.
> 
>> If the dm bits are that sensitive and cache line honed to perfection
>> already due to previous regressions in that area, then it might
>> not be a bad idea to have some compile checks for false cacheline
>> sharing between sensitive members, or spilling of a sub-struct
>> into multiple cachelines.
>>
>> It's not like this was pushed behind your back. It's posted for
>> review. It's quite possible the net change is a win for dm. Let's
>> focus on getting it reviewed, rather than pontificate on what
>> could potentially go all wrong with this.
> 
> Why are you making this personal?  Or purely about DM?  I'm merely
> pointing out this change isn't something that can be given a quick
> blanket "looks good".

I'm not making this personal at all?! You raised a (valid) concern,
I'm merely stating why I don't think it's a high risk issue. I'm
assuming your worry is related to dm, as those are the reports
that would ultimately land on your desk.

-- 
Jens Axboe



Re: [PATCH 00/13] convert block layer to bioset_init()/mempool_init()

2018-05-21 Thread Mike Snitzer
On Mon, May 21 2018 at 10:52am -0400,
Jens Axboe  wrote:

> On 5/21/18 8:47 AM, Mike Snitzer wrote:
> > On Mon, May 21 2018 at 10:36am -0400,
> > Jens Axboe  wrote:
> > 
> >> On 5/21/18 8:31 AM, Mike Snitzer wrote:
> >>> On Mon, May 21 2018 at 10:19am -0400,
> >>> Jens Axboe  wrote:
> >>>
>  On 5/21/18 8:03 AM, Mike Snitzer wrote:
> > On Sun, May 20 2018 at  6:25pm -0400,
> > Kent Overstreet  wrote:
> >
> >> Jens - this series does the rest of the conversions that Christoph 
> >> wanted, and
> >> drops bioset_create().
> >>
> >> Only lightly tested, but the changes are pretty mechanical. Based on 
> >> your
> >> for-next tree.
> >
> > By switching 'mempool_t *' to 'mempool_t' and 'bio_set *' to 'bio_set'
> > you've altered the alignment of members in data structures.  So I'll
> > need to audit all the data structures you've modified in DM.
> >
> > Could we get the backstory on _why_ you're making this change?
> > Would go a long way to helping me appreciate why this is a good use of
> > anyone's time.
> 
>  Yeah, it's in the first series, it gets rid of a pointer indirection.
> >>>
> >>> "Allows mempools to be embedded in other structs, getting rid of a
> >>> pointer indirection from allocation fastpaths."
> >>>
> >>> So this is about using contiguous memory or avoiding partial allocation
> >>> failure?  Or both?
> >>>
> >>> Or more to it?  Just trying to fully appreciate the theory behind the
> >>> perceived associated benefit.
> >>
> >> It's about avoiding a pointer indirection. Instead of having to
> >> follow a pointer to get to that struct, it's simple offset math off
> >> your main structure.
> >>
> >>> I do think the increased risk of these embedded bio_set and mempool_t
> >>> themselves crossing cachelines, or struct members that follow them doing
> >>> so, really detracts from these types of changes.
> >>
> >> Definitely something to look out for, though most of them should be
> >> per-dev structures and not in-flight structures. That makes it a bit
> >> less sensitive. But can't hurt to audit the layouts and adjust if
> >> necessary. This is why it's posted for review :-)
> > 
> > This isn't something that is easily caught upfront.  Yes we can all be
> > busy little beavers with pahole to audit alignment.  But chances are
> > most people won't do it.
> > 
> > Reality is there is potential for a regression due to false sharing to
> > creep in if a hot struct member suddenly starts straddling a cacheline.
> > That type of NUMA performance killer is pretty insidious and somewhat
> > tedious to hunt down even when looking for it with specialized tools:
> > https://joemario.github.io/blog/2016/09/01/c2c-blog/
> 
> IMHO you're making a big deal out of something that should not be.

I raised an issue that had seemingly not been considered at all.  Not
making a big deal.  Raising it for others' benefit.

> If the dm bits are that sensitive and cache line honed to perfection
> already due to previous regressions in that area, then it might
> not be a bad idea to have some compile checks for false cacheline
> sharing between sensitive members, or spilling of a sub-struct
> into multiple cachelines.
> 
> It's not like this was pushed behind your back. It's posted for
> review. It's quite possible the net change is a win for dm. Let's
> focus on getting it reviewed, rather than pontificate on what
> could potentially go all wrong with this.

Why are you making this personal?  Or purely about DM?  I'm merely
pointing out this change isn't something that can be given a quick
blanket "looks good".

Mike


Re: [PATCH 00/13] convert block layer to bioset_init()/mempool_init()

2018-05-21 Thread Jens Axboe
On 5/21/18 8:47 AM, Mike Snitzer wrote:
> On Mon, May 21 2018 at 10:36am -0400,
> Jens Axboe  wrote:
> 
>> On 5/21/18 8:31 AM, Mike Snitzer wrote:
>>> On Mon, May 21 2018 at 10:19am -0400,
>>> Jens Axboe  wrote:
>>>
 On 5/21/18 8:03 AM, Mike Snitzer wrote:
> On Sun, May 20 2018 at  6:25pm -0400,
> Kent Overstreet  wrote:
>
>> Jens - this series does the rest of the conversions that Christoph 
>> wanted, and
>> drops bioset_create().
>>
>> Only lightly tested, but the changes are pretty mechanical. Based on your
>> for-next tree.
>
> By switching 'mempool_t *' to 'mempool_t' and 'bio_set *' to 'bio_set'
> you've altered the alignment of members in data structures.  So I'll
> need to audit all the data structures you've modified in DM.
>
> Could we get the backstory on _why_ you're making this change?
> Would go a long way to helping me appreciate why this is a good use of
> anyone's time.

 Yeah, it's in the first series, it gets rid of a pointer indirection.
>>>
>>> "Allows mempools to be embedded in other structs, getting rid of a
>>> pointer indirection from allocation fastpaths."
>>>
>>> So this is about using contiguous memory or avoiding partial allocation
>>> failure?  Or both?
>>>
>>> Or more to it?  Just trying to fully appreciate the theory behind the
>>> perceived associated benefit.
>>
>> It's about avoiding a pointer indirection. Instead of having to
>> follow a pointer to get to that struct, it's simple offset math off
>> your main structure.
>>
>>> I do think the increased risk of these embedded bio_set and mempool_t
>>> themselves crossing cachelines, or struct members that follow them doing
>>> so, really detracts from these types of changes.
>>
>> Definitely something to look out for, though most of them should be
>> per-dev structures and not in-flight structures. That makes it a bit
>> less sensitive. But can't hurt to audit the layouts and adjust if
>> necessary. This is why it's posted for review :-)
> 
> This isn't something that is easily caught upfront.  Yes we can all be
> busy little beavers with pahole to audit alignment.  But chances are
> most people won't do it.
> 
> Reality is there is potential for a regression due to false sharing to
> creep in if a hot struct member suddenly starts straddling a cacheline.
> That type of NUMA performance killer is pretty insidious and somewhat
> tedious to hunt down even when looking for it with specialized tools:
> https://joemario.github.io/blog/2016/09/01/c2c-blog/

IMHO you're making a big deal out of something that should not be.
If the dm bits are that sensitive and cache line honed to perfection
already due to previous regressions in that area, then it might
not be a bad idea to have some compile checks for false cacheline
sharing between sensitive members, or spilling of a sub-struct
into multiple cachelines.

It's not like this was pushed behind your back. It's posted for
review. It's quite possible the net change is a win for dm. Let's
focus on getting it reviewed, rather than pontificate on what
could potentially go all wrong with this.

-- 
Jens Axboe



Re: [PATCH 00/13] convert block layer to bioset_init()/mempool_init()

2018-05-21 Thread Mike Snitzer
On Mon, May 21 2018 at 10:36am -0400,
Jens Axboe  wrote:

> On 5/21/18 8:31 AM, Mike Snitzer wrote:
> > On Mon, May 21 2018 at 10:19am -0400,
> > Jens Axboe  wrote:
> > 
> >> On 5/21/18 8:03 AM, Mike Snitzer wrote:
> >>> On Sun, May 20 2018 at  6:25pm -0400,
> >>> Kent Overstreet  wrote:
> >>>
>  Jens - this series does the rest of the conversions that Christoph 
>  wanted, and
>  drops bioset_create().
> 
>  Only lightly tested, but the changes are pretty mechanical. Based on your
>  for-next tree.
> >>>
> >>> By switching 'mempool_t *' to 'mempool_t' and 'bio_set *' to 'bio_set'
> >>> you've altered the alignment of members in data structures.  So I'll
> >>> need to audit all the data structures you've modified in DM.
> >>>
> >>> Could we get the backstory on _why_ you're making this change?
> >>> Would go a long way to helping me appreciate why this is a good use of
> >>> anyone's time.
> >>
> >> Yeah, it's in the first series, it gets rid of a pointer indirection.
> > 
> > "Allows mempools to be embedded in other structs, getting rid of a
> > pointer indirection from allocation fastpaths."
> > 
> > So this is about using contiguous memory or avoiding partial allocation
> > failure?  Or both?
> > 
> > Or more to it?  Just trying to fully appreciate the theory behind the
> > perceived associated benefit.
> 
> It's about avoiding a pointer indirection. Instead of having to
> follow a pointer to get to that struct, it's simple offset math off
> your main structure.
> 
> > I do think the increased risk of these embedded bio_set and mempool_t
> > themselves crossing cachelines, or struct members that follow them doing
> > so, really detracts from these types of changes.
> 
> Definitely something to look out for, though most of them should be
> per-dev structures and not in-flight structures. That makes it a bit
> less sensitive. But can't hurt to audit the layouts and adjust if
> necessary. This is why it's posted for review :-)

This isn't something that is easily caught upfront.  Yes we can all be
busy little beavers with pahole to audit alignment.  But chances are
most people won't do it.

Reality is there is potential for a regression due to false sharing to
creep in if a hot struct member suddenly starts straddling a cacheline.
That type of NUMA performance killer is pretty insidious and somewhat
tedious to hunt down even when looking for it with specialized tools:
https://joemario.github.io/blog/2016/09/01/c2c-blog/


Re: [PATCH 00/13] convert block layer to bioset_init()/mempool_init()

2018-05-21 Thread Jens Axboe
On 5/21/18 8:31 AM, Mike Snitzer wrote:
> On Mon, May 21 2018 at 10:19am -0400,
> Jens Axboe  wrote:
> 
>> On 5/21/18 8:03 AM, Mike Snitzer wrote:
>>> On Sun, May 20 2018 at  6:25pm -0400,
>>> Kent Overstreet  wrote:
>>>
 Jens - this series does the rest of the conversions that Christoph wanted, 
 and
 drops bioset_create().

 Only lightly tested, but the changes are pretty mechanical. Based on your
 for-next tree.
>>>
>>> By switching 'mempool_t *' to 'mempool_t' and 'bio_set *' to 'bio_set'
>>> you've altered the alignment of members in data structures.  So I'll
>>> need to audit all the data structures you've modified in DM.
>>>
>>> Could we get the backstory on _why_ you're making this change?
>>> Would go a long way to helping me appreciate why this is a good use of
>>> anyone's time.
>>
>> Yeah, it's in the first series, it gets rid of a pointer indirection.
> 
> "Allows mempools to be embedded in other structs, getting rid of a
> pointer indirection from allocation fastpaths."
> 
> So this is about using contiguous memory or avoiding partial allocation
> failure?  Or both?
> 
> Or more to it?  Just trying to fully appreciate the theory behind the
> perceived associated benefit.

It's about avoiding a pointer indirection. Instead of having to
follow a pointer to get to that struct, it's simple offset math off
your main structure.

> I do think the increased risk of these embedded bio_set and mempool_t
> themselves crossing cachelines, or struct members that follow them doing
> so, really detracts from these types of changes.

Definitely something to look out for, though most of them should be
per-dev structures and not in-flight structures. That makes it a bit
less sensitive. But can't hurt to audit the layouts and adjust if
necessary. This is why it's posted for review :-)

-- 
Jens Axboe



Re: [PATCH 00/13] convert block layer to bioset_init()/mempool_init()

2018-05-21 Thread Mike Snitzer
On Mon, May 21 2018 at 10:19am -0400,
Jens Axboe  wrote:

> On 5/21/18 8:03 AM, Mike Snitzer wrote:
> > On Sun, May 20 2018 at  6:25pm -0400,
> > Kent Overstreet  wrote:
> > 
> >> Jens - this series does the rest of the conversions that Christoph wanted, 
> >> and
> >> drops bioset_create().
> >>
> >> Only lightly tested, but the changes are pretty mechanical. Based on your
> >> for-next tree.
> > 
> > By switching 'mempool_t *' to 'mempool_t' and 'bio_set *' to 'bio_set'
> > you've altered the alignment of members in data structures.  So I'll
> > need to audit all the data structures you've modified in DM.
> > 
> > Could we get the backstory on _why_ you're making this change?
> > Would go a long way to helping me appreciate why this is a good use of
> > anyone's time.
> 
> Yeah, it's in the first series, it gets rid of a pointer indirection.

"Allows mempools to be embedded in other structs, getting rid of a
pointer indirection from allocation fastpaths."

So this is about using contiguous memory or avoiding partial allocation
failure?  Or both?

Or more to it?  Just trying to fully appreciate the theory behind the
perceived associated benefit.

I do think the increased risk of these embedded bio_set and mempool_t
themselves crossing cachelines, or struct members that follow them doing
so, really detracts from these types of changes.

Thanks,
Mike


Re: [PATCH 00/13] convert block layer to bioset_init()/mempool_init()

2018-05-21 Thread Jens Axboe
On 5/20/18 4:25 PM, Kent Overstreet wrote:
> Jens - this series does the rest of the conversions that Christoph wanted, and
> drops bioset_create().
> 
> Only lightly tested, but the changes are pretty mechanical. Based on your
> for-next tree.

Looks good to me. I'll let it simmer for a bit to give folks a chance
to comment on it.

-- 
Jens Axboe



Re: [PATCH 00/13] convert block layer to bioset_init()/mempool_init()

2018-05-21 Thread Jens Axboe
On 5/21/18 8:03 AM, Mike Snitzer wrote:
> On Sun, May 20 2018 at  6:25pm -0400,
> Kent Overstreet  wrote:
> 
>> Jens - this series does the rest of the conversions that Christoph wanted, 
>> and
>> drops bioset_create().
>>
>> Only lightly tested, but the changes are pretty mechanical. Based on your
>> for-next tree.
> 
> By switching 'mempool_t *' to 'mempool_t' and 'bio_set *' to 'bio_set'
> you've altered the alignment of members in data structures.  So I'll
> need to audit all the data structures you've modified in DM.
> 
> Could we get the backstory on _why_ you're making this change?
> Would go a long way to helping me appreciate why this is a good use of
> anyone's time.

Yeah, it's in the first series, it gets rid of a pointer indirection.

-- 
Jens Axboe



Re: [PATCH 00/13] convert block layer to bioset_init()/mempool_init()

2018-05-21 Thread Mike Snitzer
On Sun, May 20 2018 at  6:25pm -0400,
Kent Overstreet  wrote:

> Jens - this series does the rest of the conversions that Christoph wanted, and
> drops bioset_create().
> 
> Only lightly tested, but the changes are pretty mechanical. Based on your
> for-next tree.

By switching 'mempool_t *' to 'mempool_t' and 'bio_set *' to 'bio_set'
you've altered the alignment of members in data structures.  So I'll
need to audit all the data structures you've modified in DM.

Could we get the backstory on _why_ you're making this change?
Would go a long way to helping me appreciate why this is a good use of
anyone's time.

Thanks,
Mike


Re: [PATCH 00/13] convert block layer to bioset_init()/mempool_init()

2018-05-20 Thread Kent Overstreet
Thanks - sending it to him

On Sun, May 20, 2018 at 7:08 PM, NeilBrown  wrote:
> On Sun, May 20 2018, Kent Overstreet wrote:
>
>> Jens - this series does the rest of the conversions that Christoph wanted, 
>> and
>> drops bioset_create().
>>
>> Only lightly tested, but the changes are pretty mechanical. Based on your
>> for-next tree.
>>
>> It's also in the for-jens branch at 
>> https://evilpiepirate.org/git/bcachefs.git
>>
>> Kent Overstreet (12):
>>   block: convert bounce, q->bio_split to bioset_init()/mempool_init()
>>   drbd: convert to bioset_init()/mempool_init()
>>   pktcdvd: convert to bioset_init()/mempool_init()
>>   lightnvm: convert to bioset_init()/mempool_init()
>>   bcache: convert to bioset_init()/mempool_init()
>>   md: convert to bioset_init()/mempool_init()
>
> Hi Kent,
>  this conversion looks really good, thanks for Ccing me on it.
>  However as Shaohua Li is now the maintainer of md, it probably should
>  have gone to him as well.
>
> Thanks,
> NeilBrown


Re: [PATCH 00/13] convert block layer to bioset_init()/mempool_init()

2018-05-20 Thread NeilBrown
On Sun, May 20 2018, Kent Overstreet wrote:

> Jens - this series does the rest of the conversions that Christoph wanted, and
> drops bioset_create().
>
> Only lightly tested, but the changes are pretty mechanical. Based on your
> for-next tree.
>
> It's also in the for-jens branch at https://evilpiepirate.org/git/bcachefs.git
>
> Kent Overstreet (12):
>   block: convert bounce, q->bio_split to bioset_init()/mempool_init()
>   drbd: convert to bioset_init()/mempool_init()
>   pktcdvd: convert to bioset_init()/mempool_init()
>   lightnvm: convert to bioset_init()/mempool_init()
>   bcache: convert to bioset_init()/mempool_init()
>   md: convert to bioset_init()/mempool_init()

Hi Kent,
 this conversion looks really good, thanks for Ccing me on it.
 However as Shaohua Li is now the maintainer of md, it probably should
 have gone to him as well.

Thanks,
NeilBrown


signature.asc
Description: PGP signature


[PATCH 00/13] convert block layer to bioset_init()/mempool_init()

2018-05-20 Thread Kent Overstreet
Jens - this series does the rest of the conversions that Christoph wanted, and
drops bioset_create().

Only lightly tested, but the changes are pretty mechanical. Based on your
for-next tree.

It's also in the for-jens branch at https://evilpiepirate.org/git/bcachefs.git

Kent Overstreet (12):
  block: convert bounce, q->bio_split to bioset_init()/mempool_init()
  drbd: convert to bioset_init()/mempool_init()
  pktcdvd: convert to bioset_init()/mempool_init()
  lightnvm: convert to bioset_init()/mempool_init()
  bcache: convert to bioset_init()/mempool_init()
  md: convert to bioset_init()/mempool_init()
  dm: convert to bioset_init()/mempool_init()
  target: convert to bioset_init()/mempool_init()
  fs: convert block_dev.c to bioset_init()
  btrfs: convert to bioset_init()/mempool_init()
  xfs: convert to bioset_init()/mempool_init()
  block: Drop bioset_create()

 block/bio.c | 61 +--
 block/blk-core.c|  7 +--
 block/blk-merge.c   |  8 +--
 block/blk-sysfs.c   |  3 +-
 block/bounce.c  | 47 +-
 drivers/block/drbd/drbd_bitmap.c|  4 +-
 drivers/block/drbd/drbd_int.h   | 10 ++--
 drivers/block/drbd/drbd_main.c  | 71 ++-
 drivers/block/drbd/drbd_receiver.c  |  6 +--
 drivers/block/drbd/drbd_req.c   |  4 +-
 drivers/block/drbd/drbd_req.h   |  2 +-
 drivers/block/pktcdvd.c | 50 +--
 drivers/lightnvm/pblk-core.c| 30 ++--
 drivers/lightnvm/pblk-init.c| 72 +--
 drivers/lightnvm/pblk-read.c|  4 +-
 drivers/lightnvm/pblk-recovery.c|  2 +-
 drivers/lightnvm/pblk-write.c   |  8 +--
 drivers/lightnvm/pblk.h | 14 +++---
 drivers/md/bcache/bcache.h  | 10 ++--
 drivers/md/bcache/bset.c| 13 ++---
 drivers/md/bcache/bset.h|  2 +-
 drivers/md/bcache/btree.c   |  4 +-
 drivers/md/bcache/io.c  |  4 +-
 drivers/md/bcache/request.c | 18 +++
 drivers/md/bcache/super.c   | 38 ++-
 drivers/md/dm-bio-prison-v1.c   | 13 ++---
 drivers/md/dm-bio-prison-v2.c   | 13 ++---
 drivers/md/dm-cache-target.c| 25 +-
 drivers/md/dm-core.h|  4 +-
 drivers/md/dm-crypt.c   | 60 +++
 drivers/md/dm-integrity.c   | 15 +++---
 drivers/md/dm-io.c  | 29 +--
 drivers/md/dm-kcopyd.c  | 22 +
 drivers/md/dm-log-userspace-base.c  | 19 
 drivers/md/dm-region-hash.c | 23 -
 drivers/md/dm-rq.c  |  2 +-
 drivers/md/dm-snap.c| 17 +++
 drivers/md/dm-thin.c| 32 ++--
 drivers/md/dm-verity-fec.c  | 55 +++--
 drivers/md/dm-verity-fec.h  |  8 +--
 drivers/md/dm-zoned-target.c| 13 +++--
 drivers/md/dm.c | 55 +
 drivers/md/md-faulty.c  |  2 +-
 drivers/md/md-linear.c  |  2 +-
 drivers/md/md-multipath.c   | 17 ---
 drivers/md/md-multipath.h   |  2 +-
 drivers/md/md.c | 61 +--
 drivers/md/md.h |  4 +-
 drivers/md/raid0.c  |  5 +-
 drivers/md/raid1.c  | 76 ++---
 drivers/md/raid1.h  |  6 +--
 drivers/md/raid10.c | 60 +++
 drivers/md/raid10.h |  6 +--
 drivers/md/raid5-cache.c| 43 
 drivers/md/raid5-ppl.c  | 42 +++-
 drivers/md/raid5.c  | 12 ++---
 drivers/md/raid5.h  |  2 +-
 drivers/target/target_core_iblock.c | 14 +++---
 drivers/target/target_core_iblock.h |  2 +-
 fs/block_dev.c  |  9 ++--
 fs/btrfs/extent_io.c| 25 +-
 fs/xfs/xfs_aops.c   |  2 +-
 fs/xfs/xfs_aops.h   |  2 +-
 fs/xfs/xfs_super.c  | 11 ++---
 include/linux/bio.h | 11 +++--
 include/linux/blkdev.h  |  2 +-
 include/linux/pktcdvd.h |  2 +-
 67 files changed, 606 insertions(+), 711 deletions(-)

-- 
2.17.0