On Fri, Jul 14, 2017 at 08:22:31AM -0600, Jens Axboe wrote:
>  /*
>   * drivers should _never_ use the all version - the bio may have been split
> - * before it got to the driver and the driver won't own all of it
> + * before it got to the driver and the driver won't own all of it.
> + *
> + * Don't use this on cloned bio's.
>   */
>  #define bio_for_each_segment_all(bvl, bio, i)                                
> \
> +     WARN_ON_ONCE(bio_flagged(bio, BIO_CLONED));                     \
>       for (i = 0, bvl = (bio)->bi_io_vec; i < (bio)->bi_vcnt; i++, bvl++)

This needs a multiline statement protection, otherwise

if (...)
        bio_for_each_segment_all(...) {
                ...
        }

would expand to

if (...)
        WARN_ON_ONCE(...);

bio_for_each_segment_all(...) { ... }

However "do { ... } while(0)" cannot be used here, so WARN_ON_ONCE could
be moved to the initialization block of for:

#define bio_for_each_segment_all(bvl, bio, i)                           \
        for (WARN_ON_ONCE(bio_flagged(bio, BIO_CLONED)),                \
             i = 0, bvl = (bio)->bi_io_vec; i < (bio)->bi_vcnt; i++, bvl++)

I haven't found any code using the exact broken pattern. So this does
not explain the crash Liu Bo reports.

Reply via email to