Shaohua Li <shli@xxxxxx> writes:
> On Mon, Oct 12, 2015 at 04:37:55PM +1100, Neil Brown wrote:
>> Shaohua Li <shli@xxxxxx> writes:
>>
>> > journal disk can be faulty. The Journal and Faulty aren't exclusive with
>> > each other.
>> >
>> > Signed-off-by: Shaohua Li <shli@xxxxxx>
>> > ---
>> > drivers/md/md.c | 10 +++++++---
>> > 1 file changed, 7 insertions(+), 3 deletions(-)
>> >
>> > diff --git a/drivers/md/md.c b/drivers/md/md.c
>> > index 0729cc7..daf42bb 100644
>> > --- a/drivers/md/md.c
>> > +++ b/drivers/md/md.c
>> > @@ -5857,7 +5857,8 @@ static int get_disk_info(struct mddev *mddev, void __user * arg)
>> > else if (test_bit(In_sync, &rdev->flags)) {
>> > info.state |= (1<<MD_DISK_ACTIVE);
>> > info.state |= (1<<MD_DISK_SYNC);
>> > - } else if (test_bit(Journal, &rdev->flags))
>> > + }
>> > + if (test_bit(Journal, &rdev->flags))
>> > info.state |= (1<<MD_DISK_JOURNAL);
>> > if (test_bit(WriteMostly, &rdev->flags))
>> > info.state |= (1<<MD_DISK_WRITEMOSTLY);
>> > @@ -7335,18 +7336,21 @@ static int md_seq_show(struct seq_file *seq, void *v)
>> > rcu_read_lock();
>> > rdev_for_each_rcu(rdev, mddev) {
>> > char b[BDEVNAME_SIZE];
>> > + bool skip = false;
>> > seq_printf(seq, " %s[%d]",
>> > bdevname(rdev->bdev,b), rdev->desc_nr);
>> > if (test_bit(WriteMostly, &rdev->flags))
>> > seq_printf(seq, "(W)");
>> > if (test_bit(Faulty, &rdev->flags)) {
>> > seq_printf(seq, "(F)");
>> > - continue;
>> > + skip = true;
>> > }
>> > if (test_bit(Journal, &rdev->flags)) {
>> > seq_printf(seq, "(J)");
>> > - continue;
>> > + skip = true;
>> > }
>> > + if (skip)
>> > + continue;
>>
>> Is this 'skip' stuff really needed? What would go wrong if we just left
>> it out? An active journal will have raid_disk>=0 now won't it? And
>> if we don't support replacement of journals (which I suspect we
>> shouldn't) then (R) will never get reported.
>>
>> So can we just drop the 'skip'??
>
> Sounds good. Let me know if I should resend this one alone or the
> series.
Just send this one alone. I'll apply the rest from the previous
posting.
I'm not entirely convinced about the race issue, but I don't really
want to think about it today and as you seem certain we will go with the
current code for now.
If I think of something that does convince me, it can always be fixed
later.
Thanks,
NeilBrown
Attachment:
signature.asc
Description: PGP signature
