Re: false positive lockdep splat with loop device

From: Ilya Dryomov
Date: Tue Oct 10 2017 - 05:43:24 EST


On Tue, Oct 10, 2017 at 11:16 AM, Ilya Dryomov <idryomov@xxxxxxxxx> wrote:
> On Thu, Oct 5, 2017 at 6:33 PM, Christoph Hellwig <hch@xxxxxxxxxxxxx> wrote:
>> Does the patch below fix the warning for you?
>>
>> --
>> From 28aae7104425433d39e6142adcd5b88dc5b0ad5f Mon Sep 17 00:00:00 2001
>> From: Christoph Hellwig <hch@xxxxxx>
>> Date: Thu, 5 Oct 2017 18:31:02 +0200
>> Subject: block: use DECLARE_COMPLETION_ONSTACK in submit_bio_wait
>>
>> This way we get our lockdep annotations right in case multiple layers
>> in the stack use submit_bio_wait.
>>
>> It also happens to simplify the code by getting rid of the submit_bio_ret
>> structure.
>>
>> Signed-off-by: Christoph Hellwig <hch@xxxxxx>
>> ---
>> block/bio.c | 19 +++++--------------
>> 1 file changed, 5 insertions(+), 14 deletions(-)
>>
>> diff --git a/block/bio.c b/block/bio.c
>> index 8338304ea256..4e18e959fc0a 100644
>> --- a/block/bio.c
>> +++ b/block/bio.c
>> @@ -917,17 +917,9 @@ int bio_iov_iter_get_pages(struct bio *bio, struct iov_iter *iter)
>> }
>> EXPORT_SYMBOL_GPL(bio_iov_iter_get_pages);
>>
>> -struct submit_bio_ret {
>> - struct completion event;
>> - int error;
>> -};
>> -
>> static void submit_bio_wait_endio(struct bio *bio)
>> {
>> - struct submit_bio_ret *ret = bio->bi_private;
>> -
>> - ret->error = blk_status_to_errno(bio->bi_status);
>> - complete(&ret->event);
>> + complete(bio->bi_private);
>> }
>>
>> /**
>> @@ -943,16 +935,15 @@ static void submit_bio_wait_endio(struct bio *bio)
>> */
>> int submit_bio_wait(struct bio *bio)
>> {
>> - struct submit_bio_ret ret;
>> + DECLARE_COMPLETION_ONSTACK(done);
>>
>> - init_completion(&ret.event);
>> - bio->bi_private = &ret;
>> + bio->bi_private = &done;
>> bio->bi_end_io = submit_bio_wait_endio;
>> bio->bi_opf |= REQ_SYNC;
>> submit_bio(bio);
>> - wait_for_completion_io(&ret.event);
>> + wait_for_completion_io(&done);
>>
>> - return ret.error;
>> + return blk_status_to_errno(bio->bi_status);
>> }
>> EXPORT_SYMBOL(submit_bio_wait);
>
> No, it doesn't -- the splat is a little bit more complicated, but
> fundamentally the same thing.

Easily triggered with generic/361 too, BTW.

Thanks,

Ilya