Any suggestion for my question in the last e-mail?
Should the WARN_ON() be exist or not?

It is also related to skip_copy feature and I think it seems make
sense that maybe R5_UPTODATE should not be set in this case.
But I'm not really sure if my thinking is correct or not.

However, if it does not have to exist, would you mind delete it as
well or just modify the code?



2016-05-02 22:45 GMT+08:00 Joey Liao [off-list ref]:

Hi Shaohua,

Many thanks for your reply.

How about the following WARN_ON() in handle_stripe_clean_event()???

if (test_and_clear_bit(R5_SkipCopy, &dev->flags)) {
    WARN_ON(test_bit(R5_UPTODATE, &dev->flags));
}

Sometimes it will be triggered as well but I can't find the exactly
way to reproduce it.
I would like to know if it is suitable to remove it as well or not.


2016-04-30 5:17 GMT+08:00 Shaohua Li [off-list ref]:

quoted

On Fri, Apr 29, 2016 at 06:54:10PM +0800, Joey Liao wrote:

quoted

Hi all,

For improving the I/O performance, we try to enable the raid 5
skip_copy feature by default. It indeed has some benefit, however, it
will always (not a random issue) dump the following call trace
repeatedly when we try to write the raid 5 block device. It is related
to the following codes in handle_stripe_clean_event() in raid5.c.

WARN_ON(test_bit(R5_SkipCopy, &dev->flags));
WARN_ON(dev->page != dev->orig_page);

Is it a known bug for skip_copy feature?
Does it do harm to the data integrity?
If we would like to prevent this call trace, for your suggestion how
should we do to modify the source code?

Looks the two WARN_ON should be deleted. if the dev has R5_LOCKED, it's legit
the dev has SkipCopy set and page != orig_page. I'll delete the code. This will
not harm to data integrity.

Thanks,
Shaohua