linux-block.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Edward Hsieh <edwardh@synology.com>
To: Christoph Hellwig <hch@infradead.org>
Cc: axboe@kernel.dk, neilb@suse.com, linux-block@vger.kernel.org,
	linux-kernel@vger.kernel.org, s3t@synology.com,
	bingjingc@synology.com, cccheng@synology.com,
	Wade Liang <wadel@synology.com>
Subject: Re: [PATCH v3] block: fix trace completion for chained bio
Date: Thu, 24 Jun 2021 14:50:30 +0800	[thread overview]
Message-ID: <1204c32a-e3b3-95dd-b2b5-b9f6eef4f022@synology.com> (raw)
In-Reply-To: <YMmDxl9abff+wulm@infradead.org>



On 6/16/2021 12:53 PM, Christoph Hellwig wrote:
> On Wed, Jun 16, 2021 at 11:08:10AM +0800, edwardh wrote:
>> @@ -1400,18 +1404,13 @@ void bio_endio(struct bio *bio)
>>   	if (bio->bi_end_io == bio_chain_endio) {
>>   		bio = __bio_chain_endio(bio);
>>   		goto again;
>> +	} else {
>> +		blk_throtl_bio_endio(bio);
>> +		/* release cgroup info */
>> +		bio_uninit(bio);
>> +		if (bio->bi_end_io)
>> +			bio->bi_end_io(bio);
> 
> No need for an else after a goto.
> 

We are suggested by Neil Brown in the last version that from the
comment, the bio_chain_endio handling is used *only* to avoid
deep recursion so it should be at the end of the function.
Therefore, the position of blk_throtl_bio_endio() and bio_uninit()
are a bit odd.

We believe that blk_throtl_bio_endio() and bio_uninit() is in
the correct position now. And adding an else closure is our
attempt to make it more clear.

If it's not necessary then V2 patch should work to fix the
missing completion traces. Should we resend PATCH V2?

Thank you,
Edward

  reply	other threads:[~2021-06-24  6:50 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-16  3:08 [PATCH v3] block: fix trace completion for chained bio edwardh
2021-06-16  4:53 ` Christoph Hellwig
2021-06-24  6:50   ` Edward Hsieh [this message]
2021-06-24  7:02     ` Christoph Hellwig

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1204c32a-e3b3-95dd-b2b5-b9f6eef4f022@synology.com \
    --to=edwardh@synology.com \
    --cc=axboe@kernel.dk \
    --cc=bingjingc@synology.com \
    --cc=cccheng@synology.com \
    --cc=hch@infradead.org \
    --cc=linux-block@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=neilb@suse.com \
    --cc=s3t@synology.com \
    --cc=wadel@synology.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).