Re: [PATCH 000 of 35] Refactor block layer to improve support for stacked devices.

["John Stoffel" <john@stoffel.org>]

Tejun> Avi Kivity wrote:
>> NeilBrown wrote:
>>> To achieve this, the "for_each" macros are now somewhat more complex.
>>> For example, rq_for_each_segment is:
>>> 
>>> #define bio_for_each_segment_offset(bv, bio, _i, offs, _size)        \
>>> for (_i.i = 0, _i.offset = (bio)->bi_offset + offs,        \
>>> _i.size = min_t(int, _size, (bio)->bi_size - offs);       \
>>> _i.i < (bio)->bi_vcnt && _i.size > 0;            \
>>> _i.i++)                            \
>>> if (bv = *bio_iovec_idx((bio), _i.i),            \
>>> bv.bv_offset += _i.offset,                \
>>> bv.bv_len <= _i.offset                \
>>> ? (_i.offset -= bv.bv_len, 0)            \
>>> : (bv.bv_len -= _i.offset,                \
>>> _i.offset = 0,                    \
>>> bv.bv_len < _i.size                \
>>> ? (_i.size -= bv.bv_len, 1)            \
>>> : (bv.bv_len = _i.size,                \
>>> _i.size = 0,                    \
>>> bv.bv_len > 0)))
>>> 
>>> #define bio_for_each_segment(bv, bio, __i)                \
>>> bio_for_each_segment_offset(bv, bio, __i, 0, (bio)->bi_size)
>>> 
>>> It does some with some explanatory text in a comment, but it is still
>>> a bit daunting.  Any suggestions on making this more approachable
>>> would be very welcome.
>>> 
>>> 
>> 
>> Well, I hesitate to state the obvious, but how about:
>> 
>> #define bio_for_each_segment_offset(bv, bio, _i, offs, _size) \
>> for (bio_iterator_init(&_i, ...); bio_iterator_cont(&_i, ...);
>> bio_iterator_advance(&_i, ...)) \
>> if (bio_iterator_want_segment(&_i, ...))
>> 
>> While this doesn't remove the complexity, at least it's readable.

Tejun> Violently seconded.

How about it be made into a real function instead?  I was reading
through the patch, but got timed out yesterday, so take this with a
grain of salt.  

I thought I saw a couple of macros defined to use this macro yet
again.  Which I figured might be a problem is the passed in variables
get munged.

In any case, why does something so complicated need to be a macro, why
not a function instead?

John
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/