On Tue, Apr 11 2017, Shaohua Li wrote:

> On Wed, Apr 12, 2017 at 09:27:07AM +1000, Neil Brown wrote:
>> On Tue, Apr 11 2017, Shaohua Li wrote:
>> 
>> > On Wed, Apr 05, 2017 at 02:05:50PM +1000, Neil Brown wrote:
>> >> This is part of my little project to make bio splitting
>> >> in Linux uniform and dead-lock free, in a way that will mean
>> >> that we can get rid of all the bioset threads.
>> >> 
>> >> The basic approach is that when a bio needs to be split, we call
>> >> bio_split(), bio_chain() and then generic_make_request().
>> >> We then proceed to handle the remainder without further splitting.
>> >> Recent changes to generic_make_request() ensure that this will
>> >> be safe from deadlocks, providing each bioset is used only once
>> >> in the stack.
>> >> 
>> >> This leads to simpler code in various places.  In particular, the
>> >> splitting of bios that is needed to work around known bad blocks
>> >> is now much less complex.  There is only ever one r1bio per bio.
>> >> 
>> >> As you can see from
>> >>  10 files changed, 335 insertions(+), 540 deletions(-)
>> >> there is a net reduction in code.
>> >
>> > Looks good and makes code simpler, applied, thanks Neil! The patch 1 and 6 need
>> > comments in the code to explain how deadlock is avoided though. Care to send a
>> > new patch?
>> 
>> It isn't clear to me what sort of comment you want, or where it should
>> go.
>> It might make sense to have a comment near bio_split() explaining how to
>> use it (i.e. explaining the pattern used in various patches here), but
>> I don't see what sort of comments would help in raid1.c or raid10.c
>> ??
>
> Both raid1.c and raid10.c have comments why we need offload the bio to
> raid1d/raid10d to avoid deadlock before, we also have comments to explain why
> we do bio_split() and then generic_make_request() before. Now these info are
> lost, so I hope we can add it back why the new way (bio_split and follow
> generic_make_request of next part) can avoid deadlock. That will be very
> helpful for others.

Those comments were needed before because of design flaws in
generic_make_request().  It make it too easy to trigger deadlocks.
With the recent changes to generic_make_request(), deadlocks are much
harder to trigger so there is less need for documentation on how to be
careful.

It would be good to have some clear documentation for bio_split()
describing how it should be used.  Once I have tidied up all the users
of bio_split() and removed the bioset work queues, I plan to add that
documentation.

Thanks,
NeilBrown