On Sat, March 28, 2009 3:19 am, Andre Noll wrote:
> On 19:53, NeilBrown wrote:
>> - if (logical_sector >= conf->expand_progress) {
>> + if (mddev->delta_disks < 0
>> + ? logical_sector < conf->reshape_progress
>> + : logical_sector >= conf->reshape_progress) {
>> disks = conf->previous_raid_disks;
>> previous = 1;
>> } else {
>> - if (logical_sector >= conf->expand_lo) {
>> + if (mddev->delta_disks < 0
>> + ? logical_sector < conf->reshape_safe
>> + : logical_sector >= conf->reshape_safe) {
>> spin_unlock_irq(&conf->device_lock);
>> schedule();
>> goto retry;
>
> Is it only me who finds such code hard to comprehend? Given that
> the patch adds checks of the form
>
> (delta < 0 && s < r) || (delta >= 0 && s >= r)
They are really of the form
delta < 0 ? s < r : s >= r
I guess I could use something like
static inline inorder(mddev_t *mddev, sector_t a, sector_t b)
{
if (mddev->delta_disks < 0)
return b > a;
else
return a <= b;
}
However sometimes it is '<' vs '>=' and sometimes '<' vs '>',
so I'm not sure it would apply universally.....
>
> at several locations, it might make sense to introduce a marco or an
> inline function for this check.
>
>> + /* reshape_progress is the leading edge of a 'reshape'
>> + * It has value MaxSector when no expand is happening
>
> s/expand/reshape
Thanks.
Fixed.
NeilBrown
>
> Regards
> Andre
> --
> The only person who always got his work done by Friday was Robinson Crusoe
>
--
To unsubscribe from this list: send the line "unsubscribe linux-raid" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
