linux-kernel - Re: [PATCH v2] block: trace completion of all bios.

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 24 Mar 2017 11:06:09 +1100
From:   NeilBrown <neilb@...e.com>
To:     Ming Lei <tom.leiming@...il.com>
Cc:     Christoph Hellwig <hch@...radead.org>,
        Jens Axboe <axboe@...nel.dk>, linux-block@...r.kernel.org,
        linux-raid@...r.kernel.org, dm-devel@...hat.com,
        Alasdair Kergon <agk@...hat.com>,
        Mike Snitzer <snitzer@...hat.com>,
        Shaohua Li <shli@...nel.org>, linux-kernel@...r.kernel.org,
        "Martin K . Petersen" <martin.petersen@...cle.com>
Subject: Re: [PATCH v2] block: trace completion of all bios.

On Thu, Mar 23 2017, Ming Lei wrote:

> On Thu, Mar 23, 2017 at 05:29:02PM +1100, NeilBrown wrote:
>> 
>>  /**
>> + * bio_endio_notrace - end I/O on a bio without tracing
>> + * @bio:	bio
>> + *
>> + * Description:
>> + *   bio_endio_notrace() will end I/O on the whole bio.
>> + *   bio_endio_notrace() should only be call if a completion trace
>> + *   event is not needed.  This can be the case if a request-level
>> + *   completion event has already been generated, if the bio is
>> + *   being completed early, before it was even queued.
>> + *
>> + **/
>> +void bio_endio_notrace(struct bio *bio)
>> +{
>> +again:
...
>> +
>> +	if (bio->bi_bdev)
>> +		trace_block_bio_complete(bdev_get_queue(bio->bi_bdev),
>> +					 bio, bio->bi_error);
>
> The notrace version still traces?

Ugh.  Thanks :-(

>
>> +	if (bio->bi_end_io)
>> +		bio->bi_end_io(bio);
>> +}
>> +EXPORT_SYMBOL(bio_endio_notrace);
>
> It isn't a good idea to duplicate bio_endio here, and any change on
> bio_endio() may be needed for this _notrace version too in future.

I uhmed and arhhed about that.  The function is so small....
But I've had a change of heart.  I don't think that having separate
bio_endio() and bio_endio_notrace() is such a good idea. It is too easy
to use the wrong one.  It is much better to make it automatically do the
right thing.
So following is a new patch - more thoroughly tested - which handles
more cases, and doesn't need any follow-up changes for filesystems.

Thanks,
Neilbrown

Download attachment "signature.asc" of type "application/pgp-signature" (833 bytes)