Re: false positive lockdep splat with loop device

[Ilya Dryomov]

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