Re: [dm-devel] new patchset to eliminate DM's use of BIOSET_NEED_RESCUER [was: Re: [PATCH 00/13] block: assorted cleanup for bio splitting and cloning.]

[NeilBrown]

On Tue, Nov 21 2017, Mikulas Patocka wrote:

> On Tue, 21 Nov 2017, Mike Snitzer wrote:
>
>> On Tue, Nov 21 2017 at  7:43am -0500,
>> Mike Snitzer  wrote:
>>  
>> > Decided it a better use of my time to review and then hopefully use the
>> > block-core's bio splitting infrastructure in DM.  Been meaning to do
>> > that for quite a while anyway.  This mail from you just made it all the
>> > more clear that needs doing:
>> > https://www.redhat.com/archives/dm-devel/2017-September/msg00098.html
>> > 
>> > So I will start here on this patch you proposed:
>> > https://www.redhat.com/archives/dm-devel/2017-September/msg00091.html
>> > (of note, this patch slipped through the cracks because I was recovering
>> > from injury when it originally came through).
>> > 
>> > Once DM is using q->bio_split I'll come back to this patch (aka
>> > "[1]") as a starting point for the follow-on work to remove DM's use of
>> > BIOSET_NEED_RESCUER:
>> > https://www.redhat.com/archives/dm-devel/2017-August/msg00315.html
>> 
>> Hey Neil,
>> 
>> Good news!  All your code works ;)
>> 
>> (well after 1 fixup due to a cut-n-paste bug.. the code you added to
>> dm_wq_work() to process the md->rescued bio_list was operating on
>> the md->deferred bio_list due to cut-n-paste from code you copied from
>> just below it)
>> 
>> I split your code out some to make it more reviewable.  I also tweaked
>> headers accordingly.
>> 
>> Please see this branch (which _will_ get rebased between now and the
>> 4.16 merge window):
>> https://git.kernel.org/pub/scm/linux/kernel/git/device-mapper/linux-dm.git/log/?h=dm-4.16
>> 
>> I successfully tested these changes using Mikulas' test program that
>> reproduces the snapshot deadlock:
>> https://www.redhat.com/archives/dm-devel/2017-January/msg00064.html
>> 
>> I'll throw various other DM testsuites at it to verify they all look
>> good (e.g. thinp, cache, multipath).
>> 
>> I'm open to all suggestions about changes you'd like to see (either to
>> these patches or anything you'd like to layer ontop of them).
>> 
>> Thanks for all your work, much appreciated!
>> Mike
>
> This is not correct:

Thanks for your review!

>
>2206 static void dm_wq_work(struct work_struct *work)
>2207 {
>2208 struct mapped_device *md = container_of(work, struct 
> mapped_device, work);
>2209 struct bio *bio;
>2210 int srcu_idx;
>2211 struct dm_table *map;
>2212
>2213 if (!bio_list_empty(>rescued)) {
>2214 struct bio_list list;
>2215 spin_lock_irq(>deferred_lock);
>2216 list = md->rescued;
>2217 bio_list_init(>rescued);
>2218 spin_unlock_irq(>deferred_lock);
>2219 while ((bio = bio_list_pop()))
>2220 generic_make_request(bio);
>2221 }
>
>2223 map = dm_get_live_table(md, _idx);
>2224
>2225 while (!test_bit(DMF_BLOCK_IO_FOR_SUSPEND, >flags)) {
>2226 spin_lock_irq(>deferred_lock);
>2227 bio = bio_list_pop(>deferred);
>2228 spin_unlock_irq(>deferred_lock);
>2229
>2230 if (!bio)
>2231 break;
>2232
>2233 if (dm_request_based(md))
>2234 generic_make_request(bio);
>2235 else
>2236 __split_and_process_bio(md, map, bio);
>2237 }
>2238
>2239 dm_put_live_table(md, srcu_idx);
>2240 }
>
> You can see that if we are in dm_wq_work in __split_and_process_bio, we 
> will not process md->rescued list.

Correct, but md->rescued will be empty, or irrelevant.
The first section of dm_wq_work ensures ->rescued is empty.
When __split_and_process_bio() calls generic_make_request() (indirectly
through one or more targets) they will not be recursive calls,
so nothing will be added to current->bio_list[0] and nothing
will be moved to md->rescued.  Each generic_make_request() will
completely submit the request in the lower level devel.

Some other thread could call generic_make_request on this dm device and
result in bios appeared on md->rescued.  These bios could only be a
problem if something that __split_and_process_bio calls might wait for
them.  I don't think that happens (at least I don't think it should...).


>
> The processing of md->rescued is also wrong - bios for different devices 
> must be offloaded to different helper threads, so that processing a bio 
> for a lower device doesn't depend on processing a bio for a higher device. 
> If you offload all the bios on current->bio_list to the same thread, the 
> bios still depend on each other and the deadlock will still happen.

bios on current->bio_list[0] are not allowed to depend on each other
except that later bios can depend on earlier bios.  They are all for a
lower-level 

Re: [dm-devel] new patchset to eliminate DM's use of BIOSET_NEED_RESCUER [was: Re: [PATCH 00/13] block: assorted cleanup for bio splitting and cloning.]

[NeilBrown]

On Tue, Nov 21 2017, Mikulas Patocka wrote:

> On Tue, 21 Nov 2017, Mike Snitzer wrote:
>
>> On Tue, Nov 21 2017 at  7:43am -0500,
>> Mike Snitzer  wrote:
>>  
>> > Decided it a better use of my time to review and then hopefully use the
>> > block-core's bio splitting infrastructure in DM.  Been meaning to do
>> > that for quite a while anyway.  This mail from you just made it all the
>> > more clear that needs doing:
>> > https://www.redhat.com/archives/dm-devel/2017-September/msg00098.html
>> > 
>> > So I will start here on this patch you proposed:
>> > https://www.redhat.com/archives/dm-devel/2017-September/msg00091.html
>> > (of note, this patch slipped through the cracks because I was recovering
>> > from injury when it originally came through).
>> > 
>> > Once DM is using q->bio_split I'll come back to this patch (aka
>> > "[1]") as a starting point for the follow-on work to remove DM's use of
>> > BIOSET_NEED_RESCUER:
>> > https://www.redhat.com/archives/dm-devel/2017-August/msg00315.html
>> 
>> Hey Neil,
>> 
>> Good news!  All your code works ;)
>> 
>> (well after 1 fixup due to a cut-n-paste bug.. the code you added to
>> dm_wq_work() to process the md->rescued bio_list was operating on
>> the md->deferred bio_list due to cut-n-paste from code you copied from
>> just below it)
>> 
>> I split your code out some to make it more reviewable.  I also tweaked
>> headers accordingly.
>> 
>> Please see this branch (which _will_ get rebased between now and the
>> 4.16 merge window):
>> https://git.kernel.org/pub/scm/linux/kernel/git/device-mapper/linux-dm.git/log/?h=dm-4.16
>> 
>> I successfully tested these changes using Mikulas' test program that
>> reproduces the snapshot deadlock:
>> https://www.redhat.com/archives/dm-devel/2017-January/msg00064.html
>> 
>> I'll throw various other DM testsuites at it to verify they all look
>> good (e.g. thinp, cache, multipath).
>> 
>> I'm open to all suggestions about changes you'd like to see (either to
>> these patches or anything you'd like to layer ontop of them).
>> 
>> Thanks for all your work, much appreciated!
>> Mike
>
> This is not correct:

Thanks for your review!

>
>2206 static void dm_wq_work(struct work_struct *work)
>2207 {
>2208 struct mapped_device *md = container_of(work, struct 
> mapped_device, work);
>2209 struct bio *bio;
>2210 int srcu_idx;
>2211 struct dm_table *map;
>2212
>2213 if (!bio_list_empty(>rescued)) {
>2214 struct bio_list list;
>2215 spin_lock_irq(>deferred_lock);
>2216 list = md->rescued;
>2217 bio_list_init(>rescued);
>2218 spin_unlock_irq(>deferred_lock);
>2219 while ((bio = bio_list_pop()))
>2220 generic_make_request(bio);
>2221 }
>
>2223 map = dm_get_live_table(md, _idx);
>2224
>2225 while (!test_bit(DMF_BLOCK_IO_FOR_SUSPEND, >flags)) {
>2226 spin_lock_irq(>deferred_lock);
>2227 bio = bio_list_pop(>deferred);
>2228 spin_unlock_irq(>deferred_lock);
>2229
>2230 if (!bio)
>2231 break;
>2232
>2233 if (dm_request_based(md))
>2234 generic_make_request(bio);
>2235 else
>2236 __split_and_process_bio(md, map, bio);
>2237 }
>2238
>2239 dm_put_live_table(md, srcu_idx);
>2240 }
>
> You can see that if we are in dm_wq_work in __split_and_process_bio, we 
> will not process md->rescued list.

Correct, but md->rescued will be empty, or irrelevant.
The first section of dm_wq_work ensures ->rescued is empty.
When __split_and_process_bio() calls generic_make_request() (indirectly
through one or more targets) they will not be recursive calls,
so nothing will be added to current->bio_list[0] and nothing
will be moved to md->rescued.  Each generic_make_request() will
completely submit the request in the lower level devel.

Some other thread could call generic_make_request on this dm device and
result in bios appeared on md->rescued.  These bios could only be a
problem if something that __split_and_process_bio calls might wait for
them.  I don't think that happens (at least I don't think it should...).


>
> The processing of md->rescued is also wrong - bios for different devices 
> must be offloaded to different helper threads, so that processing a bio 
> for a lower device doesn't depend on processing a bio for a higher device. 
> If you offload all the bios on current->bio_list to the same thread, the 
> bios still depend on each other and the deadlock will still happen.

bios on current->bio_list[0] are not allowed to depend on each other
except that later bios can depend on earlier bios.  They are all for a
lower-level device and should be 

Re: [dm-devel] new patchset to eliminate DM's use of BIOSET_NEED_RESCUER [was: Re: [PATCH 00/13] block: assorted cleanup for bio splitting and cloning.]

[Mikulas Patocka]

On Tue, 21 Nov 2017, Mike Snitzer wrote:

> On Tue, Nov 21 2017 at  7:43am -0500,
> Mike Snitzer  wrote:
>  
> > Decided it a better use of my time to review and then hopefully use the
> > block-core's bio splitting infrastructure in DM.  Been meaning to do
> > that for quite a while anyway.  This mail from you just made it all the
> > more clear that needs doing:
> > https://www.redhat.com/archives/dm-devel/2017-September/msg00098.html
> > 
> > So I will start here on this patch you proposed:
> > https://www.redhat.com/archives/dm-devel/2017-September/msg00091.html
> > (of note, this patch slipped through the cracks because I was recovering
> > from injury when it originally came through).
> > 
> > Once DM is using q->bio_split I'll come back to this patch (aka
> > "[1]") as a starting point for the follow-on work to remove DM's use of
> > BIOSET_NEED_RESCUER:
> > https://www.redhat.com/archives/dm-devel/2017-August/msg00315.html
> 
> Hey Neil,
> 
> Good news!  All your code works ;)
> 
> (well after 1 fixup due to a cut-n-paste bug.. the code you added to
> dm_wq_work() to process the md->rescued bio_list was operating on
> the md->deferred bio_list due to cut-n-paste from code you copied from
> just below it)
> 
> I split your code out some to make it more reviewable.  I also tweaked
> headers accordingly.
> 
> Please see this branch (which _will_ get rebased between now and the
> 4.16 merge window):
> https://git.kernel.org/pub/scm/linux/kernel/git/device-mapper/linux-dm.git/log/?h=dm-4.16
> 
> I successfully tested these changes using Mikulas' test program that
> reproduces the snapshot deadlock:
> https://www.redhat.com/archives/dm-devel/2017-January/msg00064.html
> 
> I'll throw various other DM testsuites at it to verify they all look
> good (e.g. thinp, cache, multipath).
> 
> I'm open to all suggestions about changes you'd like to see (either to
> these patches or anything you'd like to layer ontop of them).
> 
> Thanks for all your work, much appreciated!
> Mike

This is not correct:

   2206 static void dm_wq_work(struct work_struct *work)
   2207 {
   2208 struct mapped_device *md = container_of(work, struct 
mapped_device, work);
   2209 struct bio *bio;
   2210 int srcu_idx;
   2211 struct dm_table *map;
   2212
   2213 if (!bio_list_empty(>rescued)) {
   2214 struct bio_list list;
   2215 spin_lock_irq(>deferred_lock);
   2216 list = md->rescued;
   2217 bio_list_init(>rescued);
   2218 spin_unlock_irq(>deferred_lock);
   2219 while ((bio = bio_list_pop()))
   2220 generic_make_request(bio);
   2221 }
   
   2223 map = dm_get_live_table(md, _idx);
   2224
   2225 while (!test_bit(DMF_BLOCK_IO_FOR_SUSPEND, >flags)) {
   2226 spin_lock_irq(>deferred_lock);
   2227 bio = bio_list_pop(>deferred);
   2228 spin_unlock_irq(>deferred_lock);
   2229
   2230 if (!bio)
   2231 break;
   2232
   2233 if (dm_request_based(md))
   2234 generic_make_request(bio);
   2235 else
   2236 __split_and_process_bio(md, map, bio);
   2237 }
   2238
   2239 dm_put_live_table(md, srcu_idx);
   2240 }

You can see that if we are in dm_wq_work in __split_and_process_bio, we 
will not process md->rescued list.

The processing of md->rescued is also wrong - bios for different devices 
must be offloaded to different helper threads, so that processing a bio 
for a lower device doesn't depend on processing a bio for a higher device. 
If you offload all the bios on current->bio_list to the same thread, the 
bios still depend on each other and the deadlock will still happen.

Mikulas

Re: [dm-devel] new patchset to eliminate DM's use of BIOSET_NEED_RESCUER [was: Re: [PATCH 00/13] block: assorted cleanup for bio splitting and cloning.]

[Mikulas Patocka]

On Tue, 21 Nov 2017, Mike Snitzer wrote:

> On Tue, Nov 21 2017 at  7:43am -0500,
> Mike Snitzer  wrote:
>  
> > Decided it a better use of my time to review and then hopefully use the
> > block-core's bio splitting infrastructure in DM.  Been meaning to do
> > that for quite a while anyway.  This mail from you just made it all the
> > more clear that needs doing:
> > https://www.redhat.com/archives/dm-devel/2017-September/msg00098.html
> > 
> > So I will start here on this patch you proposed:
> > https://www.redhat.com/archives/dm-devel/2017-September/msg00091.html
> > (of note, this patch slipped through the cracks because I was recovering
> > from injury when it originally came through).
> > 
> > Once DM is using q->bio_split I'll come back to this patch (aka
> > "[1]") as a starting point for the follow-on work to remove DM's use of
> > BIOSET_NEED_RESCUER:
> > https://www.redhat.com/archives/dm-devel/2017-August/msg00315.html
> 
> Hey Neil,
> 
> Good news!  All your code works ;)
> 
> (well after 1 fixup due to a cut-n-paste bug.. the code you added to
> dm_wq_work() to process the md->rescued bio_list was operating on
> the md->deferred bio_list due to cut-n-paste from code you copied from
> just below it)
> 
> I split your code out some to make it more reviewable.  I also tweaked
> headers accordingly.
> 
> Please see this branch (which _will_ get rebased between now and the
> 4.16 merge window):
> https://git.kernel.org/pub/scm/linux/kernel/git/device-mapper/linux-dm.git/log/?h=dm-4.16
> 
> I successfully tested these changes using Mikulas' test program that
> reproduces the snapshot deadlock:
> https://www.redhat.com/archives/dm-devel/2017-January/msg00064.html
> 
> I'll throw various other DM testsuites at it to verify they all look
> good (e.g. thinp, cache, multipath).
> 
> I'm open to all suggestions about changes you'd like to see (either to
> these patches or anything you'd like to layer ontop of them).
> 
> Thanks for all your work, much appreciated!
> Mike

This is not correct:

   2206 static void dm_wq_work(struct work_struct *work)
   2207 {
   2208 struct mapped_device *md = container_of(work, struct 
mapped_device, work);
   2209 struct bio *bio;
   2210 int srcu_idx;
   2211 struct dm_table *map;
   2212
   2213 if (!bio_list_empty(>rescued)) {
   2214 struct bio_list list;
   2215 spin_lock_irq(>deferred_lock);
   2216 list = md->rescued;
   2217 bio_list_init(>rescued);
   2218 spin_unlock_irq(>deferred_lock);
   2219 while ((bio = bio_list_pop()))
   2220 generic_make_request(bio);
   2221 }
   
   2223 map = dm_get_live_table(md, _idx);
   2224
   2225 while (!test_bit(DMF_BLOCK_IO_FOR_SUSPEND, >flags)) {
   2226 spin_lock_irq(>deferred_lock);
   2227 bio = bio_list_pop(>deferred);
   2228 spin_unlock_irq(>deferred_lock);
   2229
   2230 if (!bio)
   2231 break;
   2232
   2233 if (dm_request_based(md))
   2234 generic_make_request(bio);
   2235 else
   2236 __split_and_process_bio(md, map, bio);
   2237 }
   2238
   2239 dm_put_live_table(md, srcu_idx);
   2240 }

You can see that if we are in dm_wq_work in __split_and_process_bio, we 
will not process md->rescued list.

The processing of md->rescued is also wrong - bios for different devices 
must be offloaded to different helper threads, so that processing a bio 
for a lower device doesn't depend on processing a bio for a higher device. 
If you offload all the bios on current->bio_list to the same thread, the 
bios still depend on each other and the deadlock will still happen.

Mikulas

Re: [PATCH 00/13] block: assorted cleanup for bio splitting and cloning.

[Mike Snitzer]

On Tue, Nov 21 2017 at  2:44pm -0500,
NeilBrown  wrote:

> On Tue, Nov 21 2017, Mike Snitzer wrote:
> 
> > On Mon, Nov 20 2017 at  8:35pm -0500,
> > Mike Snitzer  wrote:
> >
> >> On Mon, Nov 20 2017 at  7:34pm -0500,
> >> NeilBrown  wrote:
> >> 
> >> > On Mon, Nov 20 2017, Mike Snitzer wrote:
> >> > 
> >> > >
> >> > > But I've now queued this patch for once Linus gets back (reverts DM
> >> > > changes from commit 47e0fb461f):
> >> > > https://git.kernel.org/pub/scm/linux/kernel/git/device-mapper/linux-dm.git/commit/?h=for-next=c9fdc42ba23eabd1ba7aef199fb9bb4b4fe5c545
> >> > 
> >> > This patch does two things.
> >> > 1/ It removes the BIOSET_NEED_RESCUER flag from biosets created by dm.
> >> >   This a functional changed over the code from before my patches.
> >> >   Previously, all biosets were given a rescuer thread.
> >> >   After my patch set, biosets only got a rescuer thread if
> >> >   BIOSET_NEED_RESCUER was passed, and it was passed for all biosets.
> >> >   I then removed it from places were I was certain it wasn't needed.
> >> >   I didn't remove it from dm because I wasn't certain.  Your
> >> >   patch does remove the flags, which I think is incorrect - see below.
> >
> > Yeap, definitely was incorrect.  I've dropped the patch.
> >
> >> > 2/ It changes flush_current_bio_list() so that bios allocated from a
> >> >bioset that does not have a rescue_workqueue are now added to
> >> >the ->rescue_list for their bio_set, and ->rescue_work is queued
> >> >on the NULL ->rescue_workqueue, resulting in a NULL dereference.
> >> >I suspect you don't want this.
> >
> > Yes, I see that now.
> >
> >> > The patch description claims that the patch fixes something, but it
> >> > isn't clear to me what it is meant to be fixing.
> >> > 
> >> > It makes reference to  dbba42d8 which is described as removing an unused
> >> > bioset process, though what it actually does is remove an used bioset
> >> > (and obvious the process disappears with it).  My patch doesn't change
> >> > that behavior.
> >> 
> >> Well I looked at this because Zdenek reported that with more recent
> >> kernels he is seeing the "bioset" per DM device again (whereas it was
> >> thought to be removed with mikulas' commit dbba42d8 -- but that commit
> >> removed "bioset" only in terms of q->bio_split.
> >
> > I think Zdenek triggered a false-positive that DM had magically sprouted
> > a new "bioset" rescue_workqueue.  Reality is I cannot see how each
> > bio-based DM device can avoid having one.  And the commit d67a5f4b59
> > ("dm: flush queued bios when process blocks to avoid deadlock") I
> > referenced earlier very much makes DM depend on it even more.
> >
> > So apologies for being so off-base (by looking to prematurely revert
> > DM's use of BIOSET_NEED_RESCUER, etc).
> >
> >> > Please see
> >> >https://www.redhat.com/archives/dm-devel/2017-August/msg00310.html
> >
> > I'll very likely pick these up for 4.16 shortly.  But hope to work
> > through complete removal of DM's use of BIOSET_NEED_RESCUER for 4.16 as
> > well.
> >
> >> > and
> >> >https://www.redhat.com/archives/dm-devel/2017-August/msg00315.html
> >
> > This one [1] needs a lot of review and testing.  Particularly against this
> > test case that Mikulas created to reproduce the snapshot deadlock (same
> > deadlock that motivated commit dbba42d8):
> > https://www.redhat.com/archives/dm-devel/2017-January/msg00064.html
> 
> Thanks for that link.  I'll try to make time to experiment with the test
> code and confirm my proposed approach doesn't break it.
> 
> >
> >> > for which the thread continues:
> >> >https://www.redhat.com/archives/dm-devel/2017-September/msg1.html
> >
> > Wish I could clone myself (or Kent, the world needs 2 Kents!) and pursue
> > this: https://www.redhat.com/archives/dm-devel/2014-May/msg00100.html
> 
> In that email Kent mentions "punt off to a per request_queue workqueue".
> 
> That "per request_queue workqueue" is what I'm trying to get rid of.  I
> don't think this is a good direction.
> 
> >
> > Short of that, how would you like to proceed?
> 
> I'd like to confirm that my approach
> 1/ doesn't re-introduce a deadlock
> 2/ doesn't hurt performance
> and then merge it.
> 
> Though to be honest, I don't recall exactly what "my approach" is.
> Your next email picks out two important patches which probably cover
> it.  If/when I get to do the testing I'll let you know how it goes.

I _think_ I've done the heavy lifting of what you likely had in mind
( please see: https://lkml.org/lkml/2017/11/21/567 )

Now what is left is another once-over from you to verify you're happy
with the code and patch headers, etc.

Mike

Re: [PATCH 00/13] block: assorted cleanup for bio splitting and cloning.

[Mike Snitzer]

On Tue, Nov 21 2017 at  2:44pm -0500,
NeilBrown  wrote:

> On Tue, Nov 21 2017, Mike Snitzer wrote:
> 
> > On Mon, Nov 20 2017 at  8:35pm -0500,
> > Mike Snitzer  wrote:
> >
> >> On Mon, Nov 20 2017 at  7:34pm -0500,
> >> NeilBrown  wrote:
> >> 
> >> > On Mon, Nov 20 2017, Mike Snitzer wrote:
> >> > 
> >> > >
> >> > > But I've now queued this patch for once Linus gets back (reverts DM
> >> > > changes from commit 47e0fb461f):
> >> > > https://git.kernel.org/pub/scm/linux/kernel/git/device-mapper/linux-dm.git/commit/?h=for-next=c9fdc42ba23eabd1ba7aef199fb9bb4b4fe5c545
> >> > 
> >> > This patch does two things.
> >> > 1/ It removes the BIOSET_NEED_RESCUER flag from biosets created by dm.
> >> >   This a functional changed over the code from before my patches.
> >> >   Previously, all biosets were given a rescuer thread.
> >> >   After my patch set, biosets only got a rescuer thread if
> >> >   BIOSET_NEED_RESCUER was passed, and it was passed for all biosets.
> >> >   I then removed it from places were I was certain it wasn't needed.
> >> >   I didn't remove it from dm because I wasn't certain.  Your
> >> >   patch does remove the flags, which I think is incorrect - see below.
> >
> > Yeap, definitely was incorrect.  I've dropped the patch.
> >
> >> > 2/ It changes flush_current_bio_list() so that bios allocated from a
> >> >bioset that does not have a rescue_workqueue are now added to
> >> >the ->rescue_list for their bio_set, and ->rescue_work is queued
> >> >on the NULL ->rescue_workqueue, resulting in a NULL dereference.
> >> >I suspect you don't want this.
> >
> > Yes, I see that now.
> >
> >> > The patch description claims that the patch fixes something, but it
> >> > isn't clear to me what it is meant to be fixing.
> >> > 
> >> > It makes reference to  dbba42d8 which is described as removing an unused
> >> > bioset process, though what it actually does is remove an used bioset
> >> > (and obvious the process disappears with it).  My patch doesn't change
> >> > that behavior.
> >> 
> >> Well I looked at this because Zdenek reported that with more recent
> >> kernels he is seeing the "bioset" per DM device again (whereas it was
> >> thought to be removed with mikulas' commit dbba42d8 -- but that commit
> >> removed "bioset" only in terms of q->bio_split.
> >
> > I think Zdenek triggered a false-positive that DM had magically sprouted
> > a new "bioset" rescue_workqueue.  Reality is I cannot see how each
> > bio-based DM device can avoid having one.  And the commit d67a5f4b59
> > ("dm: flush queued bios when process blocks to avoid deadlock") I
> > referenced earlier very much makes DM depend on it even more.
> >
> > So apologies for being so off-base (by looking to prematurely revert
> > DM's use of BIOSET_NEED_RESCUER, etc).
> >
> >> > Please see
> >> >https://www.redhat.com/archives/dm-devel/2017-August/msg00310.html
> >
> > I'll very likely pick these up for 4.16 shortly.  But hope to work
> > through complete removal of DM's use of BIOSET_NEED_RESCUER for 4.16 as
> > well.
> >
> >> > and
> >> >https://www.redhat.com/archives/dm-devel/2017-August/msg00315.html
> >
> > This one [1] needs a lot of review and testing.  Particularly against this
> > test case that Mikulas created to reproduce the snapshot deadlock (same
> > deadlock that motivated commit dbba42d8):
> > https://www.redhat.com/archives/dm-devel/2017-January/msg00064.html
> 
> Thanks for that link.  I'll try to make time to experiment with the test
> code and confirm my proposed approach doesn't break it.
> 
> >
> >> > for which the thread continues:
> >> >https://www.redhat.com/archives/dm-devel/2017-September/msg1.html
> >
> > Wish I could clone myself (or Kent, the world needs 2 Kents!) and pursue
> > this: https://www.redhat.com/archives/dm-devel/2014-May/msg00100.html
> 
> In that email Kent mentions "punt off to a per request_queue workqueue".
> 
> That "per request_queue workqueue" is what I'm trying to get rid of.  I
> don't think this is a good direction.
> 
> >
> > Short of that, how would you like to proceed?
> 
> I'd like to confirm that my approach
> 1/ doesn't re-introduce a deadlock
> 2/ doesn't hurt performance
> and then merge it.
> 
> Though to be honest, I don't recall exactly what "my approach" is.
> Your next email picks out two important patches which probably cover
> it.  If/when I get to do the testing I'll let you know how it goes.

I _think_ I've done the heavy lifting of what you likely had in mind
( please see: https://lkml.org/lkml/2017/11/21/567 )

Now what is left is another once-over from you to verify you're happy
with the code and patch headers, etc.

Mike

new patchset to eliminate DM's use of BIOSET_NEED_RESCUER [was: Re: [PATCH 00/13] block: assorted cleanup for bio splitting and cloning.]

[Mike Snitzer]

On Tue, Nov 21 2017 at  7:43am -0500,
Mike Snitzer  wrote:
 
> Decided it a better use of my time to review and then hopefully use the
> block-core's bio splitting infrastructure in DM.  Been meaning to do
> that for quite a while anyway.  This mail from you just made it all the
> more clear that needs doing:
> https://www.redhat.com/archives/dm-devel/2017-September/msg00098.html
> 
> So I will start here on this patch you proposed:
> https://www.redhat.com/archives/dm-devel/2017-September/msg00091.html
> (of note, this patch slipped through the cracks because I was recovering
> from injury when it originally came through).
> 
> Once DM is using q->bio_split I'll come back to this patch (aka
> "[1]") as a starting point for the follow-on work to remove DM's use of
> BIOSET_NEED_RESCUER:
> https://www.redhat.com/archives/dm-devel/2017-August/msg00315.html

Hey Neil,

Good news!  All your code works ;)

(well after 1 fixup due to a cut-n-paste bug.. the code you added to
dm_wq_work() to process the md->rescued bio_list was operating on
the md->deferred bio_list due to cut-n-paste from code you copied from
just below it)

I split your code out some to make it more reviewable.  I also tweaked
headers accordingly.

Please see this branch (which _will_ get rebased between now and the
4.16 merge window):
https://git.kernel.org/pub/scm/linux/kernel/git/device-mapper/linux-dm.git/log/?h=dm-4.16

I successfully tested these changes using Mikulas' test program that
reproduces the snapshot deadlock:
https://www.redhat.com/archives/dm-devel/2017-January/msg00064.html

I'll throw various other DM testsuites at it to verify they all look
good (e.g. thinp, cache, multipath).

I'm open to all suggestions about changes you'd like to see (either to
these patches or anything you'd like to layer ontop of them).

Thanks for all your work, much appreciated!
Mike

new patchset to eliminate DM's use of BIOSET_NEED_RESCUER [was: Re: [PATCH 00/13] block: assorted cleanup for bio splitting and cloning.]

[Mike Snitzer]

On Tue, Nov 21 2017 at  7:43am -0500,
Mike Snitzer  wrote:
 
> Decided it a better use of my time to review and then hopefully use the
> block-core's bio splitting infrastructure in DM.  Been meaning to do
> that for quite a while anyway.  This mail from you just made it all the
> more clear that needs doing:
> https://www.redhat.com/archives/dm-devel/2017-September/msg00098.html
> 
> So I will start here on this patch you proposed:
> https://www.redhat.com/archives/dm-devel/2017-September/msg00091.html
> (of note, this patch slipped through the cracks because I was recovering
> from injury when it originally came through).
> 
> Once DM is using q->bio_split I'll come back to this patch (aka
> "[1]") as a starting point for the follow-on work to remove DM's use of
> BIOSET_NEED_RESCUER:
> https://www.redhat.com/archives/dm-devel/2017-August/msg00315.html

Hey Neil,

Good news!  All your code works ;)

(well after 1 fixup due to a cut-n-paste bug.. the code you added to
dm_wq_work() to process the md->rescued bio_list was operating on
the md->deferred bio_list due to cut-n-paste from code you copied from
just below it)

I split your code out some to make it more reviewable.  I also tweaked
headers accordingly.

Please see this branch (which _will_ get rebased between now and the
4.16 merge window):
https://git.kernel.org/pub/scm/linux/kernel/git/device-mapper/linux-dm.git/log/?h=dm-4.16

I successfully tested these changes using Mikulas' test program that
reproduces the snapshot deadlock:
https://www.redhat.com/archives/dm-devel/2017-January/msg00064.html

I'll throw various other DM testsuites at it to verify they all look
good (e.g. thinp, cache, multipath).

I'm open to all suggestions about changes you'd like to see (either to
these patches or anything you'd like to layer ontop of them).

Thanks for all your work, much appreciated!
Mike

Re: [dm-devel] [PATCH 00/13] block: assorted cleanup for bio splitting and cloning.

[NeilBrown]

On Tue, Nov 21 2017, Mike Snitzer wrote:

> On Mon, Nov 20 2017 at  8:35pm -0500,
> Mike Snitzer  wrote:
>
>> On Mon, Nov 20 2017 at  7:34pm -0500,
>> NeilBrown  wrote:
>> 
>> > On Mon, Nov 20 2017, Mike Snitzer wrote:
>> > 
>> > >
>> > > But I've now queued this patch for once Linus gets back (reverts DM
>> > > changes from commit 47e0fb461f):
>> > > https://git.kernel.org/pub/scm/linux/kernel/git/device-mapper/linux-dm.git/commit/?h=for-next=c9fdc42ba23eabd1ba7aef199fb9bb4b4fe5c545
>> > 
>> > This patch does two things.
>> > 1/ It removes the BIOSET_NEED_RESCUER flag from biosets created by dm.
>> >   This a functional changed over the code from before my patches.
>> >   Previously, all biosets were given a rescuer thread.
>> >   After my patch set, biosets only got a rescuer thread if
>> >   BIOSET_NEED_RESCUER was passed, and it was passed for all biosets.
>> >   I then removed it from places were I was certain it wasn't needed.
>> >   I didn't remove it from dm because I wasn't certain.  Your
>> >   patch does remove the flags, which I think is incorrect - see below.
>
> Yeap, definitely was incorrect.  I've dropped the patch.
>
>> > 2/ It changes flush_current_bio_list() so that bios allocated from a
>> >bioset that does not have a rescue_workqueue are now added to
>> >the ->rescue_list for their bio_set, and ->rescue_work is queued
>> >on the NULL ->rescue_workqueue, resulting in a NULL dereference.
>> >I suspect you don't want this.
>
> Yes, I see that now.
>
>> > The patch description claims that the patch fixes something, but it
>> > isn't clear to me what it is meant to be fixing.
>> > 
>> > It makes reference to  dbba42d8 which is described as removing an unused
>> > bioset process, though what it actually does is remove an used bioset
>> > (and obvious the process disappears with it).  My patch doesn't change
>> > that behavior.
>> 
>> Well I looked at this because Zdenek reported that with more recent
>> kernels he is seeing the "bioset" per DM device again (whereas it was
>> thought to be removed with mikulas' commit dbba42d8 -- but that commit
>> removed "bioset" only in terms of q->bio_split.
>
> I think Zdenek triggered a false-positive that DM had magically sprouted
> a new "bioset" rescue_workqueue.  Reality is I cannot see how each
> bio-based DM device can avoid having one.  And the commit d67a5f4b59
> ("dm: flush queued bios when process blocks to avoid deadlock") I
> referenced earlier very much makes DM depend on it even more.
>
> So apologies for being so off-base (by looking to prematurely revert
> DM's use of BIOSET_NEED_RESCUER, etc).
>
>> > Please see
>> >https://www.redhat.com/archives/dm-devel/2017-August/msg00310.html
>
> I'll very likely pick these up for 4.16 shortly.  But hope to work
> through complete removal of DM's use of BIOSET_NEED_RESCUER for 4.16 as
> well.
>
>> > and
>> >https://www.redhat.com/archives/dm-devel/2017-August/msg00315.html
>
> This one [1] needs a lot of review and testing.  Particularly against this
> test case that Mikulas created to reproduce the snapshot deadlock (same
> deadlock that motivated commit dbba42d8):
> https://www.redhat.com/archives/dm-devel/2017-January/msg00064.html

Thanks for that link.  I'll try to make time to experiment with the test
code and confirm my proposed approach doesn't break it.

>
>> > for which the thread continues:
>> >https://www.redhat.com/archives/dm-devel/2017-September/msg1.html
>
> Wish I could clone myself (or Kent, the world needs 2 Kents!) and pursue
> this: https://www.redhat.com/archives/dm-devel/2014-May/msg00100.html

In that email Kent mentions "punt off to a per request_queue workqueue".

That "per request_queue workqueue" is what I'm trying to get rid of.  I
don't think this is a good direction.

>
> Short of that, how would you like to proceed?

I'd like to confirm that my approach
1/ doesn't re-introduce a deadlock
2/ doesn't hurt performance
and then merge it.

Though to be honest, I don't recall exactly what "my approach" is.
Your next email picks out two important patches which probably cover
it.  If/when I get to do the testing I'll let you know how it goes.

Thanks,
NeilBrown


>
>> > That would then just leave bcache  I find it a bit of a challenge to
>> > reason about the code in bcache, but if we can remove
>> > BIOSET_NEED_RESCUER from dm, that will be an extra incentive for me to 
>> > learn :-)
>> 
>> I'm all for properly removing BIOSET_NEED_RESCUER from DM.
>
> Should we work to make [1] (above) sure it fixes Mikulas' test case?
>
> I'll set in on reviewing and playing with [1] now.
>
> Thanks,
> Mike
>
> --
> dm-devel mailing list
> dm-de...@redhat.com
> https://www.redhat.com/mailman/listinfo/dm-devel


signature.asc
Description: PGP signature

Re: [dm-devel] [PATCH 00/13] block: assorted cleanup for bio splitting and cloning.

[NeilBrown]

On Tue, Nov 21 2017, Mike Snitzer wrote:

> On Mon, Nov 20 2017 at  8:35pm -0500,
> Mike Snitzer  wrote:
>
>> On Mon, Nov 20 2017 at  7:34pm -0500,
>> NeilBrown  wrote:
>> 
>> > On Mon, Nov 20 2017, Mike Snitzer wrote:
>> > 
>> > >
>> > > But I've now queued this patch for once Linus gets back (reverts DM
>> > > changes from commit 47e0fb461f):
>> > > https://git.kernel.org/pub/scm/linux/kernel/git/device-mapper/linux-dm.git/commit/?h=for-next=c9fdc42ba23eabd1ba7aef199fb9bb4b4fe5c545
>> > 
>> > This patch does two things.
>> > 1/ It removes the BIOSET_NEED_RESCUER flag from biosets created by dm.
>> >   This a functional changed over the code from before my patches.
>> >   Previously, all biosets were given a rescuer thread.
>> >   After my patch set, biosets only got a rescuer thread if
>> >   BIOSET_NEED_RESCUER was passed, and it was passed for all biosets.
>> >   I then removed it from places were I was certain it wasn't needed.
>> >   I didn't remove it from dm because I wasn't certain.  Your
>> >   patch does remove the flags, which I think is incorrect - see below.
>
> Yeap, definitely was incorrect.  I've dropped the patch.
>
>> > 2/ It changes flush_current_bio_list() so that bios allocated from a
>> >bioset that does not have a rescue_workqueue are now added to
>> >the ->rescue_list for their bio_set, and ->rescue_work is queued
>> >on the NULL ->rescue_workqueue, resulting in a NULL dereference.
>> >I suspect you don't want this.
>
> Yes, I see that now.
>
>> > The patch description claims that the patch fixes something, but it
>> > isn't clear to me what it is meant to be fixing.
>> > 
>> > It makes reference to  dbba42d8 which is described as removing an unused
>> > bioset process, though what it actually does is remove an used bioset
>> > (and obvious the process disappears with it).  My patch doesn't change
>> > that behavior.
>> 
>> Well I looked at this because Zdenek reported that with more recent
>> kernels he is seeing the "bioset" per DM device again (whereas it was
>> thought to be removed with mikulas' commit dbba42d8 -- but that commit
>> removed "bioset" only in terms of q->bio_split.
>
> I think Zdenek triggered a false-positive that DM had magically sprouted
> a new "bioset" rescue_workqueue.  Reality is I cannot see how each
> bio-based DM device can avoid having one.  And the commit d67a5f4b59
> ("dm: flush queued bios when process blocks to avoid deadlock") I
> referenced earlier very much makes DM depend on it even more.
>
> So apologies for being so off-base (by looking to prematurely revert
> DM's use of BIOSET_NEED_RESCUER, etc).
>
>> > Please see
>> >https://www.redhat.com/archives/dm-devel/2017-August/msg00310.html
>
> I'll very likely pick these up for 4.16 shortly.  But hope to work
> through complete removal of DM's use of BIOSET_NEED_RESCUER for 4.16 as
> well.
>
>> > and
>> >https://www.redhat.com/archives/dm-devel/2017-August/msg00315.html
>
> This one [1] needs a lot of review and testing.  Particularly against this
> test case that Mikulas created to reproduce the snapshot deadlock (same
> deadlock that motivated commit dbba42d8):
> https://www.redhat.com/archives/dm-devel/2017-January/msg00064.html

Thanks for that link.  I'll try to make time to experiment with the test
code and confirm my proposed approach doesn't break it.

>
>> > for which the thread continues:
>> >https://www.redhat.com/archives/dm-devel/2017-September/msg1.html
>
> Wish I could clone myself (or Kent, the world needs 2 Kents!) and pursue
> this: https://www.redhat.com/archives/dm-devel/2014-May/msg00100.html

In that email Kent mentions "punt off to a per request_queue workqueue".

That "per request_queue workqueue" is what I'm trying to get rid of.  I
don't think this is a good direction.

>
> Short of that, how would you like to proceed?

I'd like to confirm that my approach
1/ doesn't re-introduce a deadlock
2/ doesn't hurt performance
and then merge it.

Though to be honest, I don't recall exactly what "my approach" is.
Your next email picks out two important patches which probably cover
it.  If/when I get to do the testing I'll let you know how it goes.

Thanks,
NeilBrown


>
>> > That would then just leave bcache  I find it a bit of a challenge to
>> > reason about the code in bcache, but if we can remove
>> > BIOSET_NEED_RESCUER from dm, that will be an extra incentive for me to 
>> > learn :-)
>> 
>> I'm all for properly removing BIOSET_NEED_RESCUER from DM.
>
> Should we work to make [1] (above) sure it fixes Mikulas' test case?
>
> I'll set in on reviewing and playing with [1] now.
>
> Thanks,
> Mike
>
> --
> dm-devel mailing list
> dm-de...@redhat.com
> https://www.redhat.com/mailman/listinfo/dm-devel


signature.asc
Description: PGP signature

Re: [PATCH 00/13] block: assorted cleanup for bio splitting and cloning.

[Mike Snitzer]

On Tue, Nov 21 2017 at  7:10am -0500,
Mike Snitzer  wrote:

> On Mon, Nov 20 2017 at  8:35pm -0500,
> Mike Snitzer  wrote:
> 
> > On Mon, Nov 20 2017 at  7:34pm -0500,
> > NeilBrown  wrote:
> > 
> > > Please see
> > >https://www.redhat.com/archives/dm-devel/2017-August/msg00310.html
> 
> I'll very likely pick these up for 4.16 shortly.  But hope to work
> through complete removal of DM's use of BIOSET_NEED_RESCUER for 4.16 as
> well.
> 
> > > and
> > >https://www.redhat.com/archives/dm-devel/2017-August/msg00315.html
> 
> This one [1] needs a lot of review and testing.  Particularly against this
> test case that Mikulas created to reproduce the snapshot deadlock (same
> deadlock that motivated commit dbba42d8):
> https://www.redhat.com/archives/dm-devel/2017-January/msg00064.html
...
> Short of that, how would you like to proceed?
> 
> > > That would then just leave bcache  I find it a bit of a challenge to
> > > reason about the code in bcache, but if we can remove
> > > BIOSET_NEED_RESCUER from dm, that will be an extra incentive for me to 
> > > learn :-)
> > 
> > I'm all for properly removing BIOSET_NEED_RESCUER from DM.
> 
> Should we work to make [1] (above) sure it fixes Mikulas' test case?
> 
> I'll set in on reviewing and playing with [1] now.

Decided it a better use of my time to review and then hopefully use the
block-core's bio splitting infrastructure in DM.  Been meaning to do
that for quite a while anyway.  This mail from you just made it all the
more clear that needs doing:
https://www.redhat.com/archives/dm-devel/2017-September/msg00098.html

So I will start here on this patch you proposed:
https://www.redhat.com/archives/dm-devel/2017-September/msg00091.html
(of note, this patch slipped through the cracks because I was recovering
from injury when it originally came through).

Once DM is using q->bio_split I'll come back to this patch (aka
"[1]") as a starting point for the follow-on work to remove DM's use of
BIOSET_NEED_RESCUER:
https://www.redhat.com/archives/dm-devel/2017-August/msg00315.html

Mike

Re: [PATCH 00/13] block: assorted cleanup for bio splitting and cloning.

[Mike Snitzer]

On Tue, Nov 21 2017 at  7:10am -0500,
Mike Snitzer  wrote:

> On Mon, Nov 20 2017 at  8:35pm -0500,
> Mike Snitzer  wrote:
> 
> > On Mon, Nov 20 2017 at  7:34pm -0500,
> > NeilBrown  wrote:
> > 
> > > Please see
> > >https://www.redhat.com/archives/dm-devel/2017-August/msg00310.html
> 
> I'll very likely pick these up for 4.16 shortly.  But hope to work
> through complete removal of DM's use of BIOSET_NEED_RESCUER for 4.16 as
> well.
> 
> > > and
> > >https://www.redhat.com/archives/dm-devel/2017-August/msg00315.html
> 
> This one [1] needs a lot of review and testing.  Particularly against this
> test case that Mikulas created to reproduce the snapshot deadlock (same
> deadlock that motivated commit dbba42d8):
> https://www.redhat.com/archives/dm-devel/2017-January/msg00064.html
...
> Short of that, how would you like to proceed?
> 
> > > That would then just leave bcache  I find it a bit of a challenge to
> > > reason about the code in bcache, but if we can remove
> > > BIOSET_NEED_RESCUER from dm, that will be an extra incentive for me to 
> > > learn :-)
> > 
> > I'm all for properly removing BIOSET_NEED_RESCUER from DM.
> 
> Should we work to make [1] (above) sure it fixes Mikulas' test case?
> 
> I'll set in on reviewing and playing with [1] now.

Decided it a better use of my time to review and then hopefully use the
block-core's bio splitting infrastructure in DM.  Been meaning to do
that for quite a while anyway.  This mail from you just made it all the
more clear that needs doing:
https://www.redhat.com/archives/dm-devel/2017-September/msg00098.html

So I will start here on this patch you proposed:
https://www.redhat.com/archives/dm-devel/2017-September/msg00091.html
(of note, this patch slipped through the cracks because I was recovering
from injury when it originally came through).

Once DM is using q->bio_split I'll come back to this patch (aka
"[1]") as a starting point for the follow-on work to remove DM's use of
BIOSET_NEED_RESCUER:
https://www.redhat.com/archives/dm-devel/2017-August/msg00315.html

Mike

Re: [PATCH 00/13] block: assorted cleanup for bio splitting and cloning.

[Mike Snitzer]

On Mon, Nov 20 2017 at  8:35pm -0500,
Mike Snitzer  wrote:

> On Mon, Nov 20 2017 at  7:34pm -0500,
> NeilBrown  wrote:
> 
> > On Mon, Nov 20 2017, Mike Snitzer wrote:
> > 
> > >
> > > But I've now queued this patch for once Linus gets back (reverts DM
> > > changes from commit 47e0fb461f):
> > > https://git.kernel.org/pub/scm/linux/kernel/git/device-mapper/linux-dm.git/commit/?h=for-next=c9fdc42ba23eabd1ba7aef199fb9bb4b4fe5c545
> > 
> > This patch does two things.
> > 1/ It removes the BIOSET_NEED_RESCUER flag from biosets created by dm.
> >   This a functional changed over the code from before my patches.
> >   Previously, all biosets were given a rescuer thread.
> >   After my patch set, biosets only got a rescuer thread if
> >   BIOSET_NEED_RESCUER was passed, and it was passed for all biosets.
> >   I then removed it from places were I was certain it wasn't needed.
> >   I didn't remove it from dm because I wasn't certain.  Your
> >   patch does remove the flags, which I think is incorrect - see below.

Yeap, definitely was incorrect.  I've dropped the patch.

> > 2/ It changes flush_current_bio_list() so that bios allocated from a
> >bioset that does not have a rescue_workqueue are now added to
> >the ->rescue_list for their bio_set, and ->rescue_work is queued
> >on the NULL ->rescue_workqueue, resulting in a NULL dereference.
> >I suspect you don't want this.

Yes, I see that now.

> > The patch description claims that the patch fixes something, but it
> > isn't clear to me what it is meant to be fixing.
> > 
> > It makes reference to  dbba42d8 which is described as removing an unused
> > bioset process, though what it actually does is remove an used bioset
> > (and obvious the process disappears with it).  My patch doesn't change
> > that behavior.
> 
> Well I looked at this because Zdenek reported that with more recent
> kernels he is seeing the "bioset" per DM device again (whereas it was
> thought to be removed with mikulas' commit dbba42d8 -- but that commit
> removed "bioset" only in terms of q->bio_split.

I think Zdenek triggered a false-positive that DM had magically sprouted
a new "bioset" rescue_workqueue.  Reality is I cannot see how each
bio-based DM device can avoid having one.  And the commit d67a5f4b59
("dm: flush queued bios when process blocks to avoid deadlock") I
referenced earlier very much makes DM depend on it even more.

So apologies for being so off-base (by looking to prematurely revert
DM's use of BIOSET_NEED_RESCUER, etc).

> > Please see
> >https://www.redhat.com/archives/dm-devel/2017-August/msg00310.html

I'll very likely pick these up for 4.16 shortly.  But hope to work
through complete removal of DM's use of BIOSET_NEED_RESCUER for 4.16 as
well.

> > and
> >https://www.redhat.com/archives/dm-devel/2017-August/msg00315.html

This one [1] needs a lot of review and testing.  Particularly against this
test case that Mikulas created to reproduce the snapshot deadlock (same
deadlock that motivated commit dbba42d8):
https://www.redhat.com/archives/dm-devel/2017-January/msg00064.html

> > for which the thread continues:
> >https://www.redhat.com/archives/dm-devel/2017-September/msg1.html

Wish I could clone myself (or Kent, the world needs 2 Kents!) and pursue
this: https://www.redhat.com/archives/dm-devel/2014-May/msg00100.html

Short of that, how would you like to proceed?

> > That would then just leave bcache  I find it a bit of a challenge to
> > reason about the code in bcache, but if we can remove
> > BIOSET_NEED_RESCUER from dm, that will be an extra incentive for me to 
> > learn :-)
> 
> I'm all for properly removing BIOSET_NEED_RESCUER from DM.

Should we work to make [1] (above) sure it fixes Mikulas' test case?

I'll set in on reviewing and playing with [1] now.

Thanks,
Mike

Re: [PATCH 00/13] block: assorted cleanup for bio splitting and cloning.

[Mike Snitzer]

On Mon, Nov 20 2017 at  8:35pm -0500,
Mike Snitzer  wrote:

> On Mon, Nov 20 2017 at  7:34pm -0500,
> NeilBrown  wrote:
> 
> > On Mon, Nov 20 2017, Mike Snitzer wrote:
> > 
> > >
> > > But I've now queued this patch for once Linus gets back (reverts DM
> > > changes from commit 47e0fb461f):
> > > https://git.kernel.org/pub/scm/linux/kernel/git/device-mapper/linux-dm.git/commit/?h=for-next=c9fdc42ba23eabd1ba7aef199fb9bb4b4fe5c545
> > 
> > This patch does two things.
> > 1/ It removes the BIOSET_NEED_RESCUER flag from biosets created by dm.
> >   This a functional changed over the code from before my patches.
> >   Previously, all biosets were given a rescuer thread.
> >   After my patch set, biosets only got a rescuer thread if
> >   BIOSET_NEED_RESCUER was passed, and it was passed for all biosets.
> >   I then removed it from places were I was certain it wasn't needed.
> >   I didn't remove it from dm because I wasn't certain.  Your
> >   patch does remove the flags, which I think is incorrect - see below.

Yeap, definitely was incorrect.  I've dropped the patch.

> > 2/ It changes flush_current_bio_list() so that bios allocated from a
> >bioset that does not have a rescue_workqueue are now added to
> >the ->rescue_list for their bio_set, and ->rescue_work is queued
> >on the NULL ->rescue_workqueue, resulting in a NULL dereference.
> >I suspect you don't want this.

Yes, I see that now.

> > The patch description claims that the patch fixes something, but it
> > isn't clear to me what it is meant to be fixing.
> > 
> > It makes reference to  dbba42d8 which is described as removing an unused
> > bioset process, though what it actually does is remove an used bioset
> > (and obvious the process disappears with it).  My patch doesn't change
> > that behavior.
> 
> Well I looked at this because Zdenek reported that with more recent
> kernels he is seeing the "bioset" per DM device again (whereas it was
> thought to be removed with mikulas' commit dbba42d8 -- but that commit
> removed "bioset" only in terms of q->bio_split.

I think Zdenek triggered a false-positive that DM had magically sprouted
a new "bioset" rescue_workqueue.  Reality is I cannot see how each
bio-based DM device can avoid having one.  And the commit d67a5f4b59
("dm: flush queued bios when process blocks to avoid deadlock") I
referenced earlier very much makes DM depend on it even more.

So apologies for being so off-base (by looking to prematurely revert
DM's use of BIOSET_NEED_RESCUER, etc).

> > Please see
> >https://www.redhat.com/archives/dm-devel/2017-August/msg00310.html

I'll very likely pick these up for 4.16 shortly.  But hope to work
through complete removal of DM's use of BIOSET_NEED_RESCUER for 4.16 as
well.

> > and
> >https://www.redhat.com/archives/dm-devel/2017-August/msg00315.html

This one [1] needs a lot of review and testing.  Particularly against this
test case that Mikulas created to reproduce the snapshot deadlock (same
deadlock that motivated commit dbba42d8):
https://www.redhat.com/archives/dm-devel/2017-January/msg00064.html

> > for which the thread continues:
> >https://www.redhat.com/archives/dm-devel/2017-September/msg1.html

Wish I could clone myself (or Kent, the world needs 2 Kents!) and pursue
this: https://www.redhat.com/archives/dm-devel/2014-May/msg00100.html

Short of that, how would you like to proceed?

> > That would then just leave bcache  I find it a bit of a challenge to
> > reason about the code in bcache, but if we can remove
> > BIOSET_NEED_RESCUER from dm, that will be an extra incentive for me to 
> > learn :-)
> 
> I'm all for properly removing BIOSET_NEED_RESCUER from DM.

Should we work to make [1] (above) sure it fixes Mikulas' test case?

I'll set in on reviewing and playing with [1] now.

Thanks,
Mike

Re: [PATCH 00/13] block: assorted cleanup for bio splitting and cloning.

[Mike Snitzer]

On Mon, Nov 20 2017 at  7:34pm -0500,
NeilBrown  wrote:

> On Mon, Nov 20 2017, Mike Snitzer wrote:
> 
> > On Sun, Jun 18, 2017 at 5:36 PM, NeilBrown  wrote:
> >> On Sun, Jun 18 2017, Jens Axboe wrote:
> >>
> >>> On Sun, Jun 18 2017, NeilBrown wrote:
>  This is a resend of my series of patches working
>  towards removing the bioset work queues.
> 
>  This set is based on for-4.13/block.
> 
>  It incorporates the revised versions of all the patches that were
>  resent following feedback on the last set.
> 
>  It also includes a minor grammatic improvement to a comment, and
>  simple changes to compensate for a couple of changes to the block tree
>  since the last posting.
> 
>  I hope to eventually get rid of the new BIOSET_NEED_RESCUER flag,
>  but that needs work in dm and probably bcache first.
> >>>
> >>> Thanks Neil, applied.
> >>
> >> Thanks a lot Jens.
> >
> > I missed this line of work until now.  Not quite sure why I wasn't
> > cc'd or my review/ack required for the DM changes in this patchset.
> 
> Hi Mike,
>  I'm sorry you weren't included on those.  My thinking at the time was
>  probably that they were purely cosmetic changes which made no
>  functional difference to dm.  That is no excuse though and I do
>  apologize.
> 
> >
> > But I've now queued this patch for once Linus gets back (reverts DM
> > changes from commit 47e0fb461f):
> > https://git.kernel.org/pub/scm/linux/kernel/git/device-mapper/linux-dm.git/commit/?h=for-next=c9fdc42ba23eabd1ba7aef199fb9bb4b4fe5c545
> 
> This patch does two things.
> 1/ It removes the BIOSET_NEED_RESCUER flag from biosets created by dm.
>   This a functional changed over the code from before my patches.
>   Previously, all biosets were given a rescuer thread.
>   After my patch set, biosets only got a rescuer thread if
>   BIOSET_NEED_RESCUER was passed, and it was passed for all biosets.
>   I then removed it from places were I was certain it wasn't needed.
>   I didn't remove it from dm because I wasn't certain.  Your
>   patch does remove the flags, which I think is incorrect - see below.
> 
> 2/ It changes flush_current_bio_list() so that bios allocated from a
>bioset that does not have a rescue_workqueue are now added to
>the ->rescue_list for their bio_set, and ->rescue_work is queued
>on the NULL ->rescue_workqueue, resulting in a NULL dereference.
>I suspect you don't want this.
> 
> The patch description claims that the patch fixes something, but it
> isn't clear to me what it is meant to be fixing.
> 
> It makes reference to  dbba42d8 which is described as removing an unused
> bioset process, though what it actually does is remove an used bioset
> (and obvious the process disappears with it).  My patch doesn't change
> that behavior.

Well I looked at this because Zdenek reported that with more recent
kernels he is seeing the "bioset" per DM device again (whereas it was
thought to be removed with mikulas' commit dbba42d8 -- but that commit
removed "bioset" only in terms of q->bio_split.

Looks like the q->bio_split bioset is created with BIOSET_NEED_RESCUER
but DM calls bioset_free() for q->bio_split.  Strikes me as odd that
"bioset" was removed DM devices until it recently reappeared.
Especially if what you say is accurate (that BIOSET_NEED_RESCUER was
implied with the old code.. I believe you!)

I tried to quickly answer how "bioset" is now re-appearing for DM
devices but I obviously need to put more time to it.

(And my goodness does this bioset rescue workqueue need a better name
than "bioset"! Should include the bdevname too)

> > As is, it is my understanding that DM has no need for a bio_set's
> > rescue_workqueue.  So its removal would appear to only be gated by
> > bcache?
> >
> > But I could be mistaken, moving forward please let me know what you
> > feel needs doing in DM to make it a better citizen.
> 
> I think you are mistaken.
> Please see
>https://www.redhat.com/archives/dm-devel/2017-August/msg00310.html
> and
>https://www.redhat.com/archives/dm-devel/2017-August/msg00315.html
> for which the thread continues:
>https://www.redhat.com/archives/dm-devel/2017-September/msg1.html
> 
> These were sent to you, though most of the conversation happened with
> Mikulas.
> 
> I think that the patches in those threads explain why dm currently needs
> rescuer threads, and shows how dm can be changed to no longer need the
> rescuer.  I would appreciate your thoughts on these patches.  I can
> resend them if that would help.

No need to resend.  I'll work through the old threads.

> That would then just leave bcache  I find it a bit of a challenge to
> reason about the code in bcache, but if we can remove
> BIOSET_NEED_RESCUER from dm, that will be an extra incentive for me to learn 
> :-)

I'm all for properly removing BIOSET_NEED_RESCUER from DM.

I'll look closer at all of this in the morning (for now I'm backing the

Re: [PATCH 00/13] block: assorted cleanup for bio splitting and cloning.

[Mike Snitzer]

On Mon, Nov 20 2017 at  7:34pm -0500,
NeilBrown  wrote:

> On Mon, Nov 20 2017, Mike Snitzer wrote:
> 
> > On Sun, Jun 18, 2017 at 5:36 PM, NeilBrown  wrote:
> >> On Sun, Jun 18 2017, Jens Axboe wrote:
> >>
> >>> On Sun, Jun 18 2017, NeilBrown wrote:
>  This is a resend of my series of patches working
>  towards removing the bioset work queues.
> 
>  This set is based on for-4.13/block.
> 
>  It incorporates the revised versions of all the patches that were
>  resent following feedback on the last set.
> 
>  It also includes a minor grammatic improvement to a comment, and
>  simple changes to compensate for a couple of changes to the block tree
>  since the last posting.
> 
>  I hope to eventually get rid of the new BIOSET_NEED_RESCUER flag,
>  but that needs work in dm and probably bcache first.
> >>>
> >>> Thanks Neil, applied.
> >>
> >> Thanks a lot Jens.
> >
> > I missed this line of work until now.  Not quite sure why I wasn't
> > cc'd or my review/ack required for the DM changes in this patchset.
> 
> Hi Mike,
>  I'm sorry you weren't included on those.  My thinking at the time was
>  probably that they were purely cosmetic changes which made no
>  functional difference to dm.  That is no excuse though and I do
>  apologize.
> 
> >
> > But I've now queued this patch for once Linus gets back (reverts DM
> > changes from commit 47e0fb461f):
> > https://git.kernel.org/pub/scm/linux/kernel/git/device-mapper/linux-dm.git/commit/?h=for-next=c9fdc42ba23eabd1ba7aef199fb9bb4b4fe5c545
> 
> This patch does two things.
> 1/ It removes the BIOSET_NEED_RESCUER flag from biosets created by dm.
>   This a functional changed over the code from before my patches.
>   Previously, all biosets were given a rescuer thread.
>   After my patch set, biosets only got a rescuer thread if
>   BIOSET_NEED_RESCUER was passed, and it was passed for all biosets.
>   I then removed it from places were I was certain it wasn't needed.
>   I didn't remove it from dm because I wasn't certain.  Your
>   patch does remove the flags, which I think is incorrect - see below.
> 
> 2/ It changes flush_current_bio_list() so that bios allocated from a
>bioset that does not have a rescue_workqueue are now added to
>the ->rescue_list for their bio_set, and ->rescue_work is queued
>on the NULL ->rescue_workqueue, resulting in a NULL dereference.
>I suspect you don't want this.
> 
> The patch description claims that the patch fixes something, but it
> isn't clear to me what it is meant to be fixing.
> 
> It makes reference to  dbba42d8 which is described as removing an unused
> bioset process, though what it actually does is remove an used bioset
> (and obvious the process disappears with it).  My patch doesn't change
> that behavior.

Well I looked at this because Zdenek reported that with more recent
kernels he is seeing the "bioset" per DM device again (whereas it was
thought to be removed with mikulas' commit dbba42d8 -- but that commit
removed "bioset" only in terms of q->bio_split.

Looks like the q->bio_split bioset is created with BIOSET_NEED_RESCUER
but DM calls bioset_free() for q->bio_split.  Strikes me as odd that
"bioset" was removed DM devices until it recently reappeared.
Especially if what you say is accurate (that BIOSET_NEED_RESCUER was
implied with the old code.. I believe you!)

I tried to quickly answer how "bioset" is now re-appearing for DM
devices but I obviously need to put more time to it.

(And my goodness does this bioset rescue workqueue need a better name
than "bioset"! Should include the bdevname too)

> > As is, it is my understanding that DM has no need for a bio_set's
> > rescue_workqueue.  So its removal would appear to only be gated by
> > bcache?
> >
> > But I could be mistaken, moving forward please let me know what you
> > feel needs doing in DM to make it a better citizen.
> 
> I think you are mistaken.
> Please see
>https://www.redhat.com/archives/dm-devel/2017-August/msg00310.html
> and
>https://www.redhat.com/archives/dm-devel/2017-August/msg00315.html
> for which the thread continues:
>https://www.redhat.com/archives/dm-devel/2017-September/msg1.html
> 
> These were sent to you, though most of the conversation happened with
> Mikulas.
> 
> I think that the patches in those threads explain why dm currently needs
> rescuer threads, and shows how dm can be changed to no longer need the
> rescuer.  I would appreciate your thoughts on these patches.  I can
> resend them if that would help.

No need to resend.  I'll work through the old threads.

> That would then just leave bcache  I find it a bit of a challenge to
> reason about the code in bcache, but if we can remove
> BIOSET_NEED_RESCUER from dm, that will be an extra incentive for me to learn 
> :-)

I'm all for properly removing BIOSET_NEED_RESCUER from DM.

I'll look closer at all of this in the morning (for now I'm backing the
patch I referenced out from 

Re: [dm-devel] [PATCH 00/13] block: assorted cleanup for bio splitting and cloning.

[NeilBrown]

On Mon, Nov 20 2017, Mike Snitzer wrote:

> On Sun, Jun 18, 2017 at 5:36 PM, NeilBrown  wrote:
>> On Sun, Jun 18 2017, Jens Axboe wrote:
>>
>>> On Sun, Jun 18 2017, NeilBrown wrote:
 This is a resend of my series of patches working
 towards removing the bioset work queues.

 This set is based on for-4.13/block.

 It incorporates the revised versions of all the patches that were
 resent following feedback on the last set.

 It also includes a minor grammatic improvement to a comment, and
 simple changes to compensate for a couple of changes to the block tree
 since the last posting.

 I hope to eventually get rid of the new BIOSET_NEED_RESCUER flag,
 but that needs work in dm and probably bcache first.
>>>
>>> Thanks Neil, applied.
>>
>> Thanks a lot Jens.
>
> I missed this line of work until now.  Not quite sure why I wasn't
> cc'd or my review/ack required for the DM changes in this patchset.

Hi Mike,
 I'm sorry you weren't included on those.  My thinking at the time was
 probably that they were purely cosmetic changes which made no
 functional difference to dm.  That is no excuse though and I do
 apologize.

>
> But I've now queued this patch for once Linus gets back (reverts DM
> changes from commit 47e0fb461f):
> https://git.kernel.org/pub/scm/linux/kernel/git/device-mapper/linux-dm.git/commit/?h=for-next=c9fdc42ba23eabd1ba7aef199fb9bb4b4fe5c545

This patch does two things.
1/ It removes the BIOSET_NEED_RESCUER flag from biosets created by dm.
  This a functional changed over the code from before my patches.
  Previously, all biosets were given a rescuer thread.
  After my patch set, biosets only got a rescuer thread if
  BIOSET_NEED_RESCUER was passed, and it was passed for all biosets.
  I then removed it from places were I was certain it wasn't needed.
  I didn't remove it from dm because I wasn't certain.  Your
  patch does remove the flags, which I think is incorrect - see below.

2/ It changes flush_current_bio_list() so that bios allocated from a
   bioset that does not have a rescue_workqueue are now added to
   the ->rescue_list for their bio_set, and ->rescue_work is queued
   on the NULL ->rescue_workqueue, resulting in a NULL dereference.
   I suspect you don't want this.

The patch description claims that the patch fixes something, but it
isn't clear to me what it is meant to be fixing.

It makes reference to  dbba42d8 which is described as removing an unused
bioset process, though what it actually does is remove an used bioset
(and obvious the process disappears with it).  My patch doesn't change
that behavior.

>
> As is, it is my understanding that DM has no need for a bio_set's
> rescue_workqueue.  So its removal would appear to only be gated by
> bcache?
>
> But I could be mistaken, moving forward please let me know what you
> feel needs doing in DM to make it a better citizen.

I think you are mistaken.
Please see
   https://www.redhat.com/archives/dm-devel/2017-August/msg00310.html
and
   https://www.redhat.com/archives/dm-devel/2017-August/msg00315.html
for which the thread continues:
   https://www.redhat.com/archives/dm-devel/2017-September/msg1.html

These were sent to you, though most of the conversation happened with
Mikulas.

I think that the patches in those threads explain why dm currently needs
rescuer threads, and shows how dm can be changed to no longer need the
rescuer.  I would appreciate your thoughts on these patches.  I can
resend them if that would help.

That would then just leave bcache  I find it a bit of a challenge to
reason about the code in bcache, but if we can remove
BIOSET_NEED_RESCUER from dm, that will be an extra incentive for me to learn :-)

Thanks,
NeilBrown


>
> Thanks,
> Mike
>
> --
> dm-devel mailing list
> dm-de...@redhat.com
> https://www.redhat.com/mailman/listinfo/dm-devel


signature.asc
Description: PGP signature

Re: [dm-devel] [PATCH 00/13] block: assorted cleanup for bio splitting and cloning.

[NeilBrown]

On Mon, Nov 20 2017, Mike Snitzer wrote:

> On Sun, Jun 18, 2017 at 5:36 PM, NeilBrown  wrote:
>> On Sun, Jun 18 2017, Jens Axboe wrote:
>>
>>> On Sun, Jun 18 2017, NeilBrown wrote:
 This is a resend of my series of patches working
 towards removing the bioset work queues.

 This set is based on for-4.13/block.

 It incorporates the revised versions of all the patches that were
 resent following feedback on the last set.

 It also includes a minor grammatic improvement to a comment, and
 simple changes to compensate for a couple of changes to the block tree
 since the last posting.

 I hope to eventually get rid of the new BIOSET_NEED_RESCUER flag,
 but that needs work in dm and probably bcache first.
>>>
>>> Thanks Neil, applied.
>>
>> Thanks a lot Jens.
>
> I missed this line of work until now.  Not quite sure why I wasn't
> cc'd or my review/ack required for the DM changes in this patchset.

Hi Mike,
 I'm sorry you weren't included on those.  My thinking at the time was
 probably that they were purely cosmetic changes which made no
 functional difference to dm.  That is no excuse though and I do
 apologize.

>
> But I've now queued this patch for once Linus gets back (reverts DM
> changes from commit 47e0fb461f):
> https://git.kernel.org/pub/scm/linux/kernel/git/device-mapper/linux-dm.git/commit/?h=for-next=c9fdc42ba23eabd1ba7aef199fb9bb4b4fe5c545

This patch does two things.
1/ It removes the BIOSET_NEED_RESCUER flag from biosets created by dm.
  This a functional changed over the code from before my patches.
  Previously, all biosets were given a rescuer thread.
  After my patch set, biosets only got a rescuer thread if
  BIOSET_NEED_RESCUER was passed, and it was passed for all biosets.
  I then removed it from places were I was certain it wasn't needed.
  I didn't remove it from dm because I wasn't certain.  Your
  patch does remove the flags, which I think is incorrect - see below.

2/ It changes flush_current_bio_list() so that bios allocated from a
   bioset that does not have a rescue_workqueue are now added to
   the ->rescue_list for their bio_set, and ->rescue_work is queued
   on the NULL ->rescue_workqueue, resulting in a NULL dereference.
   I suspect you don't want this.

The patch description claims that the patch fixes something, but it
isn't clear to me what it is meant to be fixing.

It makes reference to  dbba42d8 which is described as removing an unused
bioset process, though what it actually does is remove an used bioset
(and obvious the process disappears with it).  My patch doesn't change
that behavior.

>
> As is, it is my understanding that DM has no need for a bio_set's
> rescue_workqueue.  So its removal would appear to only be gated by
> bcache?
>
> But I could be mistaken, moving forward please let me know what you
> feel needs doing in DM to make it a better citizen.

I think you are mistaken.
Please see
   https://www.redhat.com/archives/dm-devel/2017-August/msg00310.html
and
   https://www.redhat.com/archives/dm-devel/2017-August/msg00315.html
for which the thread continues:
   https://www.redhat.com/archives/dm-devel/2017-September/msg1.html

These were sent to you, though most of the conversation happened with
Mikulas.

I think that the patches in those threads explain why dm currently needs
rescuer threads, and shows how dm can be changed to no longer need the
rescuer.  I would appreciate your thoughts on these patches.  I can
resend them if that would help.

That would then just leave bcache  I find it a bit of a challenge to
reason about the code in bcache, but if we can remove
BIOSET_NEED_RESCUER from dm, that will be an extra incentive for me to learn :-)

Thanks,
NeilBrown


>
> Thanks,
> Mike
>
> --
> dm-devel mailing list
> dm-de...@redhat.com
> https://www.redhat.com/mailman/listinfo/dm-devel


signature.asc
Description: PGP signature

Re: [PATCH 00/13] block: assorted cleanup for bio splitting and cloning.

[Mike Snitzer]

On Sun, Jun 18, 2017 at 5:36 PM, NeilBrown  wrote:
> On Sun, Jun 18 2017, Jens Axboe wrote:
>
>> On Sun, Jun 18 2017, NeilBrown wrote:
>>> This is a resend of my series of patches working
>>> towards removing the bioset work queues.
>>>
>>> This set is based on for-4.13/block.
>>>
>>> It incorporates the revised versions of all the patches that were
>>> resent following feedback on the last set.
>>>
>>> It also includes a minor grammatic improvement to a comment, and
>>> simple changes to compensate for a couple of changes to the block tree
>>> since the last posting.
>>>
>>> I hope to eventually get rid of the new BIOSET_NEED_RESCUER flag,
>>> but that needs work in dm and probably bcache first.
>>
>> Thanks Neil, applied.
>
> Thanks a lot Jens.

I missed this line of work until now.  Not quite sure why I wasn't
cc'd or my review/ack required for the DM changes in this patchset.

But I've now queued this patch for once Linus gets back (reverts DM
changes from commit 47e0fb461f):
https://git.kernel.org/pub/scm/linux/kernel/git/device-mapper/linux-dm.git/commit/?h=for-next=c9fdc42ba23eabd1ba7aef199fb9bb4b4fe5c545

As is, it is my understanding that DM has no need for a bio_set's
rescue_workqueue.  So its removal would appear to only be gated by
bcache?

But I could be mistaken, moving forward please let me know what you
feel needs doing in DM to make it a better citizen.

Thanks,
Mike

Re: [PATCH 00/13] block: assorted cleanup for bio splitting and cloning.

[Mike Snitzer]

On Sun, Jun 18, 2017 at 5:36 PM, NeilBrown  wrote:
> On Sun, Jun 18 2017, Jens Axboe wrote:
>
>> On Sun, Jun 18 2017, NeilBrown wrote:
>>> This is a resend of my series of patches working
>>> towards removing the bioset work queues.
>>>
>>> This set is based on for-4.13/block.
>>>
>>> It incorporates the revised versions of all the patches that were
>>> resent following feedback on the last set.
>>>
>>> It also includes a minor grammatic improvement to a comment, and
>>> simple changes to compensate for a couple of changes to the block tree
>>> since the last posting.
>>>
>>> I hope to eventually get rid of the new BIOSET_NEED_RESCUER flag,
>>> but that needs work in dm and probably bcache first.
>>
>> Thanks Neil, applied.
>
> Thanks a lot Jens.

I missed this line of work until now.  Not quite sure why I wasn't
cc'd or my review/ack required for the DM changes in this patchset.

But I've now queued this patch for once Linus gets back (reverts DM
changes from commit 47e0fb461f):
https://git.kernel.org/pub/scm/linux/kernel/git/device-mapper/linux-dm.git/commit/?h=for-next=c9fdc42ba23eabd1ba7aef199fb9bb4b4fe5c545

As is, it is my understanding that DM has no need for a bio_set's
rescue_workqueue.  So its removal would appear to only be gated by
bcache?

But I could be mistaken, moving forward please let me know what you
feel needs doing in DM to make it a better citizen.

Thanks,
Mike

Re: [PATCH 00/13] block: assorted cleanup for bio splitting and cloning.

[NeilBrown]

On Sun, Jun 18 2017, Jens Axboe wrote:

> On Sun, Jun 18 2017, NeilBrown wrote:
>> This is a resend of my series of patches working
>> towards removing the bioset work queues.
>> 
>> This set is based on for-4.13/block.
>> 
>> It incorporates the revised versions of all the patches that were
>> resent following feedback on the last set.
>> 
>> It also includes a minor grammatic improvement to a comment, and
>> simple changes to compensate for a couple of changes to the block tree
>> since the last posting.
>> 
>> I hope to eventually get rid of the new BIOSET_NEED_RESCUER flag,
>> but that needs work in dm and probably bcache first.
>
> Thanks Neil, applied.

Thanks a lot Jens.

NeilBrown


signature.asc
Description: PGP signature

Re: [PATCH 00/13] block: assorted cleanup for bio splitting and cloning.

[NeilBrown]

On Sun, Jun 18 2017, Jens Axboe wrote:

> On Sun, Jun 18 2017, NeilBrown wrote:
>> This is a resend of my series of patches working
>> towards removing the bioset work queues.
>> 
>> This set is based on for-4.13/block.
>> 
>> It incorporates the revised versions of all the patches that were
>> resent following feedback on the last set.
>> 
>> It also includes a minor grammatic improvement to a comment, and
>> simple changes to compensate for a couple of changes to the block tree
>> since the last posting.
>> 
>> I hope to eventually get rid of the new BIOSET_NEED_RESCUER flag,
>> but that needs work in dm and probably bcache first.
>
> Thanks Neil, applied.

Thanks a lot Jens.

NeilBrown


signature.asc
Description: PGP signature

Re: [PATCH 00/13] block: assorted cleanup for bio splitting and cloning.

[Jens Axboe]

On Sun, Jun 18 2017, NeilBrown wrote:
> This is a resend of my series of patches working
> towards removing the bioset work queues.
> 
> This set is based on for-4.13/block.
> 
> It incorporates the revised versions of all the patches that were
> resent following feedback on the last set.
> 
> It also includes a minor grammatic improvement to a comment, and
> simple changes to compensate for a couple of changes to the block tree
> since the last posting.
> 
> I hope to eventually get rid of the new BIOSET_NEED_RESCUER flag,
> but that needs work in dm and probably bcache first.

Thanks Neil, applied.

-- 
Jens Axboe

Re: [PATCH 00/13] block: assorted cleanup for bio splitting and cloning.

[Jens Axboe]

On Sun, Jun 18 2017, NeilBrown wrote:
> This is a resend of my series of patches working
> towards removing the bioset work queues.
> 
> This set is based on for-4.13/block.
> 
> It incorporates the revised versions of all the patches that were
> resent following feedback on the last set.
> 
> It also includes a minor grammatic improvement to a comment, and
> simple changes to compensate for a couple of changes to the block tree
> since the last posting.
> 
> I hope to eventually get rid of the new BIOSET_NEED_RESCUER flag,
> but that needs work in dm and probably bcache first.

Thanks Neil, applied.

-- 
Jens Axboe

Re: [PATCH 00/13] block: assorted cleanup for bio splitting and cloning.

[NeilBrown]

On Fri, Jun 16 2017, Jens Axboe wrote:

> On 06/16/2017 01:34 AM, Christoph Hellwig wrote:
>> On Fri, Jun 16, 2017 at 05:30:50PM +1000, NeilBrown wrote:
>>> I've pushed the new version to the same place.  Do you actually want
>>> me to re-post all the patches?
>> 
>> I personally prefer to always have patches on the list, but I can't
>> speak for Jens of course.
>
> Yes please, I'd prefer them posted again as well.

Done.
Thanks,
NeilBrown


signature.asc
Description: PGP signature

Re: [PATCH 00/13] block: assorted cleanup for bio splitting and cloning.

[NeilBrown]

On Fri, Jun 16 2017, Jens Axboe wrote:

> On 06/16/2017 01:34 AM, Christoph Hellwig wrote:
>> On Fri, Jun 16, 2017 at 05:30:50PM +1000, NeilBrown wrote:
>>> I've pushed the new version to the same place.  Do you actually want
>>> me to re-post all the patches?
>> 
>> I personally prefer to always have patches on the list, but I can't
>> speak for Jens of course.
>
> Yes please, I'd prefer them posted again as well.

Done.
Thanks,
NeilBrown


signature.asc
Description: PGP signature

Re: [PATCH 00/13] block: assorted cleanup for bio splitting and cloning.

[NeilBrown]

On Fri, Jun 16 2017, Jens Axboe wrote:

> On 06/16/2017 01:34 AM, Christoph Hellwig wrote:
>> On Fri, Jun 16, 2017 at 05:30:50PM +1000, NeilBrown wrote:
>>> I've pushed the new version to the same place.  Do you actually want
>>> me to re-post all the patches?
>> 
>> I personally prefer to always have patches on the list, but I can't
>> speak for Jens of course.
>
> Yes please, I'd prefer them posted again as well.

Done - to lkml and linux-block.

Thanks,
NeilBrown


signature.asc
Description: PGP signature

Re: [PATCH 00/13] block: assorted cleanup for bio splitting and cloning.

[NeilBrown]

On Fri, Jun 16 2017, Jens Axboe wrote:

> On 06/16/2017 01:34 AM, Christoph Hellwig wrote:
>> On Fri, Jun 16, 2017 at 05:30:50PM +1000, NeilBrown wrote:
>>> I've pushed the new version to the same place.  Do you actually want
>>> me to re-post all the patches?
>> 
>> I personally prefer to always have patches on the list, but I can't
>> speak for Jens of course.
>
> Yes please, I'd prefer them posted again as well.

Done - to lkml and linux-block.

Thanks,
NeilBrown


signature.asc
Description: PGP signature

[PATCH 00/13] block: assorted cleanup for bio splitting and cloning.

[NeilBrown]

This is a resend of my series of patches working
towards removing the bioset work queues.

This set is based on for-4.13/block.

It incorporates the revised versions of all the patches that were
resent following feedback on the last set.

It also includes a minor grammatic improvement to a comment, and
simple changes to compensate for a couple of changes to the block tree
since the last posting.

I hope to eventually get rid of the new BIOSET_NEED_RESCUER flag,
but that needs work in dm and probably bcache first.

Thanks,
NeilBrown


---

NeilBrown (13):
  blk: remove bio_set arg from blk_queue_split()
  blk: replace bioset_create_nobvec() with a flags arg to bioset_create()
  blk: make the bioset rescue_workqueue optional.
  blk: use non-rescuing bioset for q->bio_split.
  block: Improvements to bounce-buffer handling
  rbd: use bio_clone_fast() instead of bio_clone()
  drbd: use bio_clone_fast() instead of bio_clone()
  pktcdvd: use bio_clone_fast() instead of bio_clone()
  lightnvm/pblk-read: use bio_clone_fast()
  xen-blkfront: remove bio splitting.
  bcache: use kmalloc to allocate bio in bch_data_verify()
  block: remove bio_clone() and all references.
  block: don't check for BIO_MAX_PAGES in blk_bio_segment_split()


 Documentation/block/biodoc.txt  |2 -
 block/bio.c |   73 ---
 block/blk-core.c|4 +-
 block/blk-merge.c   |   31 ++-
 block/blk-mq.c  |2 -
 block/bounce.c  |   32 ---
 drivers/block/drbd/drbd_int.h   |3 +
 drivers/block/drbd/drbd_main.c  |   13 ++
 drivers/block/drbd/drbd_req.c   |2 -
 drivers/block/drbd/drbd_req.h   |2 -
 drivers/block/pktcdvd.c |   14 +--
 drivers/block/ps3vram.c |2 -
 drivers/block/rbd.c |   16 +++-
 drivers/block/rsxx/dev.c|2 -
 drivers/block/umem.c|2 -
 drivers/block/xen-blkfront.c|   54 +-
 drivers/lightnvm/pblk-init.c|   16 ++--
 drivers/lightnvm/pblk-read.c|2 -
 drivers/lightnvm/pblk.h |1 
 drivers/lightnvm/rrpc.c |2 -
 drivers/md/bcache/debug.c   |2 -
 drivers/md/bcache/super.c   |8 +++-
 drivers/md/dm-crypt.c   |3 +
 drivers/md/dm-io.c  |3 +
 drivers/md/dm.c |5 +-
 drivers/md/md.c |6 +--
 drivers/md/raid1.c  |2 -
 drivers/md/raid10.c |2 -
 drivers/md/raid5-cache.c|2 -
 drivers/md/raid5-ppl.c  |2 -
 drivers/md/raid5.c  |2 -
 drivers/s390/block/dcssblk.c|2 -
 drivers/s390/block/xpram.c  |2 -
 drivers/target/target_core_iblock.c |2 -
 fs/block_dev.c  |2 -
 fs/btrfs/extent_io.c|3 +
 fs/xfs/xfs_super.c  |3 +
 include/linux/bio.h |   12 ++
 include/linux/blkdev.h  |3 -
 39 files changed, 168 insertions(+), 173 deletions(-)

--
Signature

[PATCH 00/13] block: assorted cleanup for bio splitting and cloning.

[NeilBrown]

This is a resend of my series of patches working
towards removing the bioset work queues.

This set is based on for-4.13/block.

It incorporates the revised versions of all the patches that were
resent following feedback on the last set.

It also includes a minor grammatic improvement to a comment, and
simple changes to compensate for a couple of changes to the block tree
since the last posting.

I hope to eventually get rid of the new BIOSET_NEED_RESCUER flag,
but that needs work in dm and probably bcache first.

Thanks,
NeilBrown


---

NeilBrown (13):
  blk: remove bio_set arg from blk_queue_split()
  blk: replace bioset_create_nobvec() with a flags arg to bioset_create()
  blk: make the bioset rescue_workqueue optional.
  blk: use non-rescuing bioset for q->bio_split.
  block: Improvements to bounce-buffer handling
  rbd: use bio_clone_fast() instead of bio_clone()
  drbd: use bio_clone_fast() instead of bio_clone()
  pktcdvd: use bio_clone_fast() instead of bio_clone()
  lightnvm/pblk-read: use bio_clone_fast()
  xen-blkfront: remove bio splitting.
  bcache: use kmalloc to allocate bio in bch_data_verify()
  block: remove bio_clone() and all references.
  block: don't check for BIO_MAX_PAGES in blk_bio_segment_split()


 Documentation/block/biodoc.txt  |2 -
 block/bio.c |   73 ---
 block/blk-core.c|4 +-
 block/blk-merge.c   |   31 ++-
 block/blk-mq.c  |2 -
 block/bounce.c  |   32 ---
 drivers/block/drbd/drbd_int.h   |3 +
 drivers/block/drbd/drbd_main.c  |   13 ++
 drivers/block/drbd/drbd_req.c   |2 -
 drivers/block/drbd/drbd_req.h   |2 -
 drivers/block/pktcdvd.c |   14 +--
 drivers/block/ps3vram.c |2 -
 drivers/block/rbd.c |   16 +++-
 drivers/block/rsxx/dev.c|2 -
 drivers/block/umem.c|2 -
 drivers/block/xen-blkfront.c|   54 +-
 drivers/lightnvm/pblk-init.c|   16 ++--
 drivers/lightnvm/pblk-read.c|2 -
 drivers/lightnvm/pblk.h |1 
 drivers/lightnvm/rrpc.c |2 -
 drivers/md/bcache/debug.c   |2 -
 drivers/md/bcache/super.c   |8 +++-
 drivers/md/dm-crypt.c   |3 +
 drivers/md/dm-io.c  |3 +
 drivers/md/dm.c |5 +-
 drivers/md/md.c |6 +--
 drivers/md/raid1.c  |2 -
 drivers/md/raid10.c |2 -
 drivers/md/raid5-cache.c|2 -
 drivers/md/raid5-ppl.c  |2 -
 drivers/md/raid5.c  |2 -
 drivers/s390/block/dcssblk.c|2 -
 drivers/s390/block/xpram.c  |2 -
 drivers/target/target_core_iblock.c |2 -
 fs/block_dev.c  |2 -
 fs/btrfs/extent_io.c|3 +
 fs/xfs/xfs_super.c  |3 +
 include/linux/bio.h |   12 ++
 include/linux/blkdev.h  |3 -
 39 files changed, 168 insertions(+), 173 deletions(-)

--
Signature

Re: [PATCH 00/13] block: assorted cleanup for bio splitting and cloning.

[Jens Axboe]

On 06/16/2017 01:34 AM, Christoph Hellwig wrote:
> On Fri, Jun 16, 2017 at 05:30:50PM +1000, NeilBrown wrote:
>> I've pushed the new version to the same place.  Do you actually want
>> me to re-post all the patches?
> 
> I personally prefer to always have patches on the list, but I can't
> speak for Jens of course.

Yes please, I'd prefer them posted again as well.

-- 
Jens Axboe

Re: [PATCH 00/13] block: assorted cleanup for bio splitting and cloning.

[Jens Axboe]

On 06/16/2017 01:34 AM, Christoph Hellwig wrote:
> On Fri, Jun 16, 2017 at 05:30:50PM +1000, NeilBrown wrote:
>> I've pushed the new version to the same place.  Do you actually want
>> me to re-post all the patches?
> 
> I personally prefer to always have patches on the list, but I can't
> speak for Jens of course.

Yes please, I'd prefer them posted again as well.

-- 
Jens Axboe

Re: [PATCH 00/13] block: assorted cleanup for bio splitting and cloning.

[Christoph Hellwig]

On Fri, Jun 16, 2017 at 05:30:50PM +1000, NeilBrown wrote:
> I've pushed the new version to the same place.  Do you actually want
> me to re-post all the patches?

I personally prefer to always have patches on the list, but I can't
speak for Jens of course.

Re: [PATCH 00/13] block: assorted cleanup for bio splitting and cloning.

[Christoph Hellwig]

On Fri, Jun 16, 2017 at 05:30:50PM +1000, NeilBrown wrote:
> I've pushed the new version to the same place.  Do you actually want
> me to re-post all the patches?

I personally prefer to always have patches on the list, but I can't
speak for Jens of course.

Re: [PATCH 00/13] block: assorted cleanup for bio splitting and cloning.

[NeilBrown]

On Thu, Jun 15 2017, Christoph Hellwig wrote:

> On Fri, Jun 16, 2017 at 03:54:30PM +1000, NeilBrown wrote:
>> Hi Jens,
>>  I didn't hear back ... have you had a chance to look?
>>  In case it helps, you can pull the full set, based on a recent Linus tree,
>>  from
>>  git://neil.brown.name/linux bioset
>> 
>>  or I can resend the patches if you like.
>
> Can yo uresend it on top of the latest for-4.13/block tree?  It has
> a lot of changes that could potentially conflict.

One trivial-to-fix conflict in xen-blkfront (a function I remove was
changed slightly).

I've pushed the new version to the same place.  Do you actually want
me to re-post all the patches?

NeilBrown




signature.asc
Description: PGP signature

Re: [PATCH 00/13] block: assorted cleanup for bio splitting and cloning.

[NeilBrown]

On Thu, Jun 15 2017, Christoph Hellwig wrote:

> On Fri, Jun 16, 2017 at 03:54:30PM +1000, NeilBrown wrote:
>> Hi Jens,
>>  I didn't hear back ... have you had a chance to look?
>>  In case it helps, you can pull the full set, based on a recent Linus tree,
>>  from
>>  git://neil.brown.name/linux bioset
>> 
>>  or I can resend the patches if you like.
>
> Can yo uresend it on top of the latest for-4.13/block tree?  It has
> a lot of changes that could potentially conflict.

One trivial-to-fix conflict in xen-blkfront (a function I remove was
changed slightly).

I've pushed the new version to the same place.  Do you actually want
me to re-post all the patches?

NeilBrown




signature.asc
Description: PGP signature

Re: [PATCH 00/13] block: assorted cleanup for bio splitting and cloning.

[Christoph Hellwig]

On Fri, Jun 16, 2017 at 03:54:30PM +1000, NeilBrown wrote:
> Hi Jens,
>  I didn't hear back ... have you had a chance to look?
>  In case it helps, you can pull the full set, based on a recent Linus tree,
>  from
>  git://neil.brown.name/linux bioset
> 
>  or I can resend the patches if you like.

Can yo uresend it on top of the latest for-4.13/block tree?  It has
a lot of changes that could potentially conflict.

Re: [PATCH 00/13] block: assorted cleanup for bio splitting and cloning.

[Christoph Hellwig]

On Fri, Jun 16, 2017 at 03:54:30PM +1000, NeilBrown wrote:
> Hi Jens,
>  I didn't hear back ... have you had a chance to look?
>  In case it helps, you can pull the full set, based on a recent Linus tree,
>  from
>  git://neil.brown.name/linux bioset
> 
>  or I can resend the patches if you like.

Can yo uresend it on top of the latest for-4.13/block tree?  It has
a lot of changes that could potentially conflict.

Re: [PATCH 00/13] block: assorted cleanup for bio splitting and cloning.

[NeilBrown]

On Thu, May 11 2017, NeilBrown wrote:

> On Tue, May 02 2017, NeilBrown wrote:
>
>> This is a revision of my series of patches working
>> towards removing the bioset work queues.
>
> Hi Jens,
>  could I get some feed-back about your thoughts on this series?
>  Will you apply it?  When?  Do I need to resend anything?
>  Would you like a git-pull request?  If so, what should I base it on?
>  There is a minor conflict with drivers/block/zram/zram_drv.c
>  as it dropped the call to blk_queue_split() recently, but otherwise it
>  still applies.

Hi Jens,
 I didn't hear back ... have you had a chance to look?
 In case it helps, you can pull the full set, based on a recent Linus tree,
 from
 git://neil.brown.name/linux bioset

 or I can resend the patches if you like.

Thanks,
NeilBrown


>
> Thanks,
> NeilBrown
>
>
>>
>> This set is based on Linus' tree as for today (2nd May) plus
>> the for-linus branch from Shaohua's md/raid tree.
>>
>> This series adds a fix for the new lightnvm/pblk-read code
>> and discards bioset_create_nobvec() in favor of a flag arg to
>> bioset_create().  There are also minor fixes and a little
>> code clean-up.
>>
>> I hope to eventually get rid of the new BIOSET_NEED_RESCUER flag,
>> but that needs work ing dm and probably bcache first.
>>
>> Thanks,
>> NeilBrown
>>
>>
>> ---
>>
>> NeilBrown (13):
>>   blk: remove bio_set arg from blk_queue_split()
>>   blk: replace bioset_create_nobvec() with a flags arg to bioset_create()
>>   blk: make the bioset rescue_workqueue optional.
>>   blk: use non-rescuing bioset for q->bio_split.
>>   block: Improvements to bounce-buffer handling
>>   rbd: use bio_clone_fast() instead of bio_clone()
>>   drbd: use bio_clone_fast() instead of bio_clone()
>>   pktcdvd: use bio_clone_fast() instead of bio_clone()
>>   lightnvm/pblk-read: use bio_clone_fast()
>>   xen-blkfront: remove bio splitting.
>>   bcache: use kmalloc to allocate bio in bch_data_verify()
>>   block: remove bio_clone() and all references.
>>   block: don't check for BIO_MAX_PAGES in blk_bio_segment_split()
>>
>>
>>  Documentation/block/biodoc.txt  |2 -
>>  block/bio.c |   72 
>> ---
>>  block/blk-core.c|4 +-
>>  block/blk-merge.c   |   31 ++-
>>  block/blk-mq.c  |2 -
>>  block/bounce.c  |   32 +---
>>  drivers/block/drbd/drbd_int.h   |3 +
>>  drivers/block/drbd/drbd_main.c  |   11 +
>>  drivers/block/drbd/drbd_req.c   |2 -
>>  drivers/block/drbd/drbd_req.h   |2 -
>>  drivers/block/pktcdvd.c |   14 +--
>>  drivers/block/ps3vram.c |2 -
>>  drivers/block/rbd.c |   16 +++-
>>  drivers/block/rsxx/dev.c|2 -
>>  drivers/block/umem.c|2 -
>>  drivers/block/xen-blkfront.c|   54 +-
>>  drivers/block/zram/zram_drv.c   |2 -
>>  drivers/lightnvm/pblk-init.c|   16 ++--
>>  drivers/lightnvm/pblk-read.c|2 -
>>  drivers/lightnvm/pblk.h |1 
>>  drivers/lightnvm/rrpc.c |2 -
>>  drivers/md/bcache/debug.c   |2 -
>>  drivers/md/bcache/super.c   |6 ++-
>>  drivers/md/dm-crypt.c   |2 -
>>  drivers/md/dm-io.c  |2 -
>>  drivers/md/dm.c |5 +-
>>  drivers/md/md.c |6 +--
>>  drivers/md/raid1.c  |2 -
>>  drivers/md/raid10.c |2 -
>>  drivers/md/raid5-cache.c|2 -
>>  drivers/md/raid5-ppl.c  |2 -
>>  drivers/md/raid5.c  |2 -
>>  drivers/s390/block/dcssblk.c|2 -
>>  drivers/s390/block/xpram.c  |2 -
>>  drivers/target/target_core_iblock.c |2 -
>>  fs/block_dev.c  |2 -
>>  fs/btrfs/extent_io.c|3 +
>>  fs/xfs/xfs_super.c  |3 +
>>  include/linux/bio.h |   12 ++
>>  include/linux/blkdev.h  |3 -
>>  40 files changed, 162 insertions(+), 174 deletions(-)
>>
>> --
>> Signature


signature.asc
Description: PGP signature

Re: [PATCH 00/13] block: assorted cleanup for bio splitting and cloning.

[NeilBrown]

On Thu, May 11 2017, NeilBrown wrote:

> On Tue, May 02 2017, NeilBrown wrote:
>
>> This is a revision of my series of patches working
>> towards removing the bioset work queues.
>
> Hi Jens,
>  could I get some feed-back about your thoughts on this series?
>  Will you apply it?  When?  Do I need to resend anything?
>  Would you like a git-pull request?  If so, what should I base it on?
>  There is a minor conflict with drivers/block/zram/zram_drv.c
>  as it dropped the call to blk_queue_split() recently, but otherwise it
>  still applies.

Hi Jens,
 I didn't hear back ... have you had a chance to look?
 In case it helps, you can pull the full set, based on a recent Linus tree,
 from
 git://neil.brown.name/linux bioset

 or I can resend the patches if you like.

Thanks,
NeilBrown


>
> Thanks,
> NeilBrown
>
>
>>
>> This set is based on Linus' tree as for today (2nd May) plus
>> the for-linus branch from Shaohua's md/raid tree.
>>
>> This series adds a fix for the new lightnvm/pblk-read code
>> and discards bioset_create_nobvec() in favor of a flag arg to
>> bioset_create().  There are also minor fixes and a little
>> code clean-up.
>>
>> I hope to eventually get rid of the new BIOSET_NEED_RESCUER flag,
>> but that needs work ing dm and probably bcache first.
>>
>> Thanks,
>> NeilBrown
>>
>>
>> ---
>>
>> NeilBrown (13):
>>   blk: remove bio_set arg from blk_queue_split()
>>   blk: replace bioset_create_nobvec() with a flags arg to bioset_create()
>>   blk: make the bioset rescue_workqueue optional.
>>   blk: use non-rescuing bioset for q->bio_split.
>>   block: Improvements to bounce-buffer handling
>>   rbd: use bio_clone_fast() instead of bio_clone()
>>   drbd: use bio_clone_fast() instead of bio_clone()
>>   pktcdvd: use bio_clone_fast() instead of bio_clone()
>>   lightnvm/pblk-read: use bio_clone_fast()
>>   xen-blkfront: remove bio splitting.
>>   bcache: use kmalloc to allocate bio in bch_data_verify()
>>   block: remove bio_clone() and all references.
>>   block: don't check for BIO_MAX_PAGES in blk_bio_segment_split()
>>
>>
>>  Documentation/block/biodoc.txt  |2 -
>>  block/bio.c |   72 
>> ---
>>  block/blk-core.c|4 +-
>>  block/blk-merge.c   |   31 ++-
>>  block/blk-mq.c  |2 -
>>  block/bounce.c  |   32 +---
>>  drivers/block/drbd/drbd_int.h   |3 +
>>  drivers/block/drbd/drbd_main.c  |   11 +
>>  drivers/block/drbd/drbd_req.c   |2 -
>>  drivers/block/drbd/drbd_req.h   |2 -
>>  drivers/block/pktcdvd.c |   14 +--
>>  drivers/block/ps3vram.c |2 -
>>  drivers/block/rbd.c |   16 +++-
>>  drivers/block/rsxx/dev.c|2 -
>>  drivers/block/umem.c|2 -
>>  drivers/block/xen-blkfront.c|   54 +-
>>  drivers/block/zram/zram_drv.c   |2 -
>>  drivers/lightnvm/pblk-init.c|   16 ++--
>>  drivers/lightnvm/pblk-read.c|2 -
>>  drivers/lightnvm/pblk.h |1 
>>  drivers/lightnvm/rrpc.c |2 -
>>  drivers/md/bcache/debug.c   |2 -
>>  drivers/md/bcache/super.c   |6 ++-
>>  drivers/md/dm-crypt.c   |2 -
>>  drivers/md/dm-io.c  |2 -
>>  drivers/md/dm.c |5 +-
>>  drivers/md/md.c |6 +--
>>  drivers/md/raid1.c  |2 -
>>  drivers/md/raid10.c |2 -
>>  drivers/md/raid5-cache.c|2 -
>>  drivers/md/raid5-ppl.c  |2 -
>>  drivers/md/raid5.c  |2 -
>>  drivers/s390/block/dcssblk.c|2 -
>>  drivers/s390/block/xpram.c  |2 -
>>  drivers/target/target_core_iblock.c |2 -
>>  fs/block_dev.c  |2 -
>>  fs/btrfs/extent_io.c|3 +
>>  fs/xfs/xfs_super.c  |3 +
>>  include/linux/bio.h |   12 ++
>>  include/linux/blkdev.h  |3 -
>>  40 files changed, 162 insertions(+), 174 deletions(-)
>>
>> --
>> Signature


signature.asc
Description: PGP signature

Re: [PATCH 00/13] block: assorted cleanup for bio splitting and cloning.

[NeilBrown]

On Tue, May 02 2017, NeilBrown wrote:

> This is a revision of my series of patches working
> towards removing the bioset work queues.

Hi Jens,
 could I get some feed-back about your thoughts on this series?
 Will you apply it?  When?  Do I need to resend anything?
 Would you like a git-pull request?  If so, what should I base it on?
 There is a minor conflict with drivers/block/zram/zram_drv.c
 as it dropped the call to blk_queue_split() recently, but otherwise it
 still applies.

Thanks,
NeilBrown


>
> This set is based on Linus' tree as for today (2nd May) plus
> the for-linus branch from Shaohua's md/raid tree.
>
> This series adds a fix for the new lightnvm/pblk-read code
> and discards bioset_create_nobvec() in favor of a flag arg to
> bioset_create().  There are also minor fixes and a little
> code clean-up.
>
> I hope to eventually get rid of the new BIOSET_NEED_RESCUER flag,
> but that needs work ing dm and probably bcache first.
>
> Thanks,
> NeilBrown
>
>
> ---
>
> NeilBrown (13):
>   blk: remove bio_set arg from blk_queue_split()
>   blk: replace bioset_create_nobvec() with a flags arg to bioset_create()
>   blk: make the bioset rescue_workqueue optional.
>   blk: use non-rescuing bioset for q->bio_split.
>   block: Improvements to bounce-buffer handling
>   rbd: use bio_clone_fast() instead of bio_clone()
>   drbd: use bio_clone_fast() instead of bio_clone()
>   pktcdvd: use bio_clone_fast() instead of bio_clone()
>   lightnvm/pblk-read: use bio_clone_fast()
>   xen-blkfront: remove bio splitting.
>   bcache: use kmalloc to allocate bio in bch_data_verify()
>   block: remove bio_clone() and all references.
>   block: don't check for BIO_MAX_PAGES in blk_bio_segment_split()
>
>
>  Documentation/block/biodoc.txt  |2 -
>  block/bio.c |   72 
> ---
>  block/blk-core.c|4 +-
>  block/blk-merge.c   |   31 ++-
>  block/blk-mq.c  |2 -
>  block/bounce.c  |   32 +---
>  drivers/block/drbd/drbd_int.h   |3 +
>  drivers/block/drbd/drbd_main.c  |   11 +
>  drivers/block/drbd/drbd_req.c   |2 -
>  drivers/block/drbd/drbd_req.h   |2 -
>  drivers/block/pktcdvd.c |   14 +--
>  drivers/block/ps3vram.c |2 -
>  drivers/block/rbd.c |   16 +++-
>  drivers/block/rsxx/dev.c|2 -
>  drivers/block/umem.c|2 -
>  drivers/block/xen-blkfront.c|   54 +-
>  drivers/block/zram/zram_drv.c   |2 -
>  drivers/lightnvm/pblk-init.c|   16 ++--
>  drivers/lightnvm/pblk-read.c|2 -
>  drivers/lightnvm/pblk.h |1 
>  drivers/lightnvm/rrpc.c |2 -
>  drivers/md/bcache/debug.c   |2 -
>  drivers/md/bcache/super.c   |6 ++-
>  drivers/md/dm-crypt.c   |2 -
>  drivers/md/dm-io.c  |2 -
>  drivers/md/dm.c |5 +-
>  drivers/md/md.c |6 +--
>  drivers/md/raid1.c  |2 -
>  drivers/md/raid10.c |2 -
>  drivers/md/raid5-cache.c|2 -
>  drivers/md/raid5-ppl.c  |2 -
>  drivers/md/raid5.c  |2 -
>  drivers/s390/block/dcssblk.c|2 -
>  drivers/s390/block/xpram.c  |2 -
>  drivers/target/target_core_iblock.c |2 -
>  fs/block_dev.c  |2 -
>  fs/btrfs/extent_io.c|3 +
>  fs/xfs/xfs_super.c  |3 +
>  include/linux/bio.h |   12 ++
>  include/linux/blkdev.h  |3 -
>  40 files changed, 162 insertions(+), 174 deletions(-)
>
> --
> Signature


signature.asc
Description: PGP signature

Re: [PATCH 00/13] block: assorted cleanup for bio splitting and cloning.

[NeilBrown]

On Tue, May 02 2017, NeilBrown wrote:

> This is a revision of my series of patches working
> towards removing the bioset work queues.

Hi Jens,
 could I get some feed-back about your thoughts on this series?
 Will you apply it?  When?  Do I need to resend anything?
 Would you like a git-pull request?  If so, what should I base it on?
 There is a minor conflict with drivers/block/zram/zram_drv.c
 as it dropped the call to blk_queue_split() recently, but otherwise it
 still applies.

Thanks,
NeilBrown


>
> This set is based on Linus' tree as for today (2nd May) plus
> the for-linus branch from Shaohua's md/raid tree.
>
> This series adds a fix for the new lightnvm/pblk-read code
> and discards bioset_create_nobvec() in favor of a flag arg to
> bioset_create().  There are also minor fixes and a little
> code clean-up.
>
> I hope to eventually get rid of the new BIOSET_NEED_RESCUER flag,
> but that needs work ing dm and probably bcache first.
>
> Thanks,
> NeilBrown
>
>
> ---
>
> NeilBrown (13):
>   blk: remove bio_set arg from blk_queue_split()
>   blk: replace bioset_create_nobvec() with a flags arg to bioset_create()
>   blk: make the bioset rescue_workqueue optional.
>   blk: use non-rescuing bioset for q->bio_split.
>   block: Improvements to bounce-buffer handling
>   rbd: use bio_clone_fast() instead of bio_clone()
>   drbd: use bio_clone_fast() instead of bio_clone()
>   pktcdvd: use bio_clone_fast() instead of bio_clone()
>   lightnvm/pblk-read: use bio_clone_fast()
>   xen-blkfront: remove bio splitting.
>   bcache: use kmalloc to allocate bio in bch_data_verify()
>   block: remove bio_clone() and all references.
>   block: don't check for BIO_MAX_PAGES in blk_bio_segment_split()
>
>
>  Documentation/block/biodoc.txt  |2 -
>  block/bio.c |   72 
> ---
>  block/blk-core.c|4 +-
>  block/blk-merge.c   |   31 ++-
>  block/blk-mq.c  |2 -
>  block/bounce.c  |   32 +---
>  drivers/block/drbd/drbd_int.h   |3 +
>  drivers/block/drbd/drbd_main.c  |   11 +
>  drivers/block/drbd/drbd_req.c   |2 -
>  drivers/block/drbd/drbd_req.h   |2 -
>  drivers/block/pktcdvd.c |   14 +--
>  drivers/block/ps3vram.c |2 -
>  drivers/block/rbd.c |   16 +++-
>  drivers/block/rsxx/dev.c|2 -
>  drivers/block/umem.c|2 -
>  drivers/block/xen-blkfront.c|   54 +-
>  drivers/block/zram/zram_drv.c   |2 -
>  drivers/lightnvm/pblk-init.c|   16 ++--
>  drivers/lightnvm/pblk-read.c|2 -
>  drivers/lightnvm/pblk.h |1 
>  drivers/lightnvm/rrpc.c |2 -
>  drivers/md/bcache/debug.c   |2 -
>  drivers/md/bcache/super.c   |6 ++-
>  drivers/md/dm-crypt.c   |2 -
>  drivers/md/dm-io.c  |2 -
>  drivers/md/dm.c |5 +-
>  drivers/md/md.c |6 +--
>  drivers/md/raid1.c  |2 -
>  drivers/md/raid10.c |2 -
>  drivers/md/raid5-cache.c|2 -
>  drivers/md/raid5-ppl.c  |2 -
>  drivers/md/raid5.c  |2 -
>  drivers/s390/block/dcssblk.c|2 -
>  drivers/s390/block/xpram.c  |2 -
>  drivers/target/target_core_iblock.c |2 -
>  fs/block_dev.c  |2 -
>  fs/btrfs/extent_io.c|3 +
>  fs/xfs/xfs_super.c  |3 +
>  include/linux/bio.h |   12 ++
>  include/linux/blkdev.h  |3 -
>  40 files changed, 162 insertions(+), 174 deletions(-)
>
> --
> Signature


signature.asc
Description: PGP signature

[PATCH 00/13] block: assorted cleanup for bio splitting and cloning.

[NeilBrown]

This is a revision of my series of patches working
towards removing the bioset work queues.

This set is based on Linus' tree as for today (2nd May) plus
the for-linus branch from Shaohua's md/raid tree.

This series adds a fix for the new lightnvm/pblk-read code
and discards bioset_create_nobvec() in favor of a flag arg to
bioset_create().  There are also minor fixes and a little
code clean-up.

I hope to eventually get rid of the new BIOSET_NEED_RESCUER flag,
but that needs work ing dm and probably bcache first.

Thanks,
NeilBrown


---

NeilBrown (13):
  blk: remove bio_set arg from blk_queue_split()
  blk: replace bioset_create_nobvec() with a flags arg to bioset_create()
  blk: make the bioset rescue_workqueue optional.
  blk: use non-rescuing bioset for q->bio_split.
  block: Improvements to bounce-buffer handling
  rbd: use bio_clone_fast() instead of bio_clone()
  drbd: use bio_clone_fast() instead of bio_clone()
  pktcdvd: use bio_clone_fast() instead of bio_clone()
  lightnvm/pblk-read: use bio_clone_fast()
  xen-blkfront: remove bio splitting.
  bcache: use kmalloc to allocate bio in bch_data_verify()
  block: remove bio_clone() and all references.
  block: don't check for BIO_MAX_PAGES in blk_bio_segment_split()


 Documentation/block/biodoc.txt  |2 -
 block/bio.c |   72 ---
 block/blk-core.c|4 +-
 block/blk-merge.c   |   31 ++-
 block/blk-mq.c  |2 -
 block/bounce.c  |   32 +---
 drivers/block/drbd/drbd_int.h   |3 +
 drivers/block/drbd/drbd_main.c  |   11 +
 drivers/block/drbd/drbd_req.c   |2 -
 drivers/block/drbd/drbd_req.h   |2 -
 drivers/block/pktcdvd.c |   14 +--
 drivers/block/ps3vram.c |2 -
 drivers/block/rbd.c |   16 +++-
 drivers/block/rsxx/dev.c|2 -
 drivers/block/umem.c|2 -
 drivers/block/xen-blkfront.c|   54 +-
 drivers/block/zram/zram_drv.c   |2 -
 drivers/lightnvm/pblk-init.c|   16 ++--
 drivers/lightnvm/pblk-read.c|2 -
 drivers/lightnvm/pblk.h |1 
 drivers/lightnvm/rrpc.c |2 -
 drivers/md/bcache/debug.c   |2 -
 drivers/md/bcache/super.c   |6 ++-
 drivers/md/dm-crypt.c   |2 -
 drivers/md/dm-io.c  |2 -
 drivers/md/dm.c |5 +-
 drivers/md/md.c |6 +--
 drivers/md/raid1.c  |2 -
 drivers/md/raid10.c |2 -
 drivers/md/raid5-cache.c|2 -
 drivers/md/raid5-ppl.c  |2 -
 drivers/md/raid5.c  |2 -
 drivers/s390/block/dcssblk.c|2 -
 drivers/s390/block/xpram.c  |2 -
 drivers/target/target_core_iblock.c |2 -
 fs/block_dev.c  |2 -
 fs/btrfs/extent_io.c|3 +
 fs/xfs/xfs_super.c  |3 +
 include/linux/bio.h |   12 ++
 include/linux/blkdev.h  |3 -
 40 files changed, 162 insertions(+), 174 deletions(-)

--
Signature

[PATCH 00/13] block: assorted cleanup for bio splitting and cloning.

[NeilBrown]

This is a revision of my series of patches working
towards removing the bioset work queues.

This set is based on Linus' tree as for today (2nd May) plus
the for-linus branch from Shaohua's md/raid tree.

This series adds a fix for the new lightnvm/pblk-read code
and discards bioset_create_nobvec() in favor of a flag arg to
bioset_create().  There are also minor fixes and a little
code clean-up.

I hope to eventually get rid of the new BIOSET_NEED_RESCUER flag,
but that needs work ing dm and probably bcache first.

Thanks,
NeilBrown


---

NeilBrown (13):
  blk: remove bio_set arg from blk_queue_split()
  blk: replace bioset_create_nobvec() with a flags arg to bioset_create()
  blk: make the bioset rescue_workqueue optional.
  blk: use non-rescuing bioset for q->bio_split.
  block: Improvements to bounce-buffer handling
  rbd: use bio_clone_fast() instead of bio_clone()
  drbd: use bio_clone_fast() instead of bio_clone()
  pktcdvd: use bio_clone_fast() instead of bio_clone()
  lightnvm/pblk-read: use bio_clone_fast()
  xen-blkfront: remove bio splitting.
  bcache: use kmalloc to allocate bio in bch_data_verify()
  block: remove bio_clone() and all references.
  block: don't check for BIO_MAX_PAGES in blk_bio_segment_split()


 Documentation/block/biodoc.txt  |2 -
 block/bio.c |   72 ---
 block/blk-core.c|4 +-
 block/blk-merge.c   |   31 ++-
 block/blk-mq.c  |2 -
 block/bounce.c  |   32 +---
 drivers/block/drbd/drbd_int.h   |3 +
 drivers/block/drbd/drbd_main.c  |   11 +
 drivers/block/drbd/drbd_req.c   |2 -
 drivers/block/drbd/drbd_req.h   |2 -
 drivers/block/pktcdvd.c |   14 +--
 drivers/block/ps3vram.c |2 -
 drivers/block/rbd.c |   16 +++-
 drivers/block/rsxx/dev.c|2 -
 drivers/block/umem.c|2 -
 drivers/block/xen-blkfront.c|   54 +-
 drivers/block/zram/zram_drv.c   |2 -
 drivers/lightnvm/pblk-init.c|   16 ++--
 drivers/lightnvm/pblk-read.c|2 -
 drivers/lightnvm/pblk.h |1 
 drivers/lightnvm/rrpc.c |2 -
 drivers/md/bcache/debug.c   |2 -
 drivers/md/bcache/super.c   |6 ++-
 drivers/md/dm-crypt.c   |2 -
 drivers/md/dm-io.c  |2 -
 drivers/md/dm.c |5 +-
 drivers/md/md.c |6 +--
 drivers/md/raid1.c  |2 -
 drivers/md/raid10.c |2 -
 drivers/md/raid5-cache.c|2 -
 drivers/md/raid5-ppl.c  |2 -
 drivers/md/raid5.c  |2 -
 drivers/s390/block/dcssblk.c|2 -
 drivers/s390/block/xpram.c  |2 -
 drivers/target/target_core_iblock.c |2 -
 fs/block_dev.c  |2 -
 fs/btrfs/extent_io.c|3 +
 fs/xfs/xfs_super.c  |3 +
 include/linux/bio.h |   12 ++
 include/linux/blkdev.h  |3 -
 40 files changed, 162 insertions(+), 174 deletions(-)

--
Signature