Re: fr1-2.16 patch for 2.6.10 kernel

In article <[email protected]> you wrote:
> Sorry for the delay, i was on holidays. Lots of snow!
> 
> I'm going to merge all of these patches and test it soon.

These are the parts I think you are interested in.

To stop the bitmap instead of leaving it running just in case ..

(raid1.c)

@@ -696,6 +999,14 @@

        mempool_destroy(conf->r1buf_pool);
        conf->r1buf_pool = NULL;
+
+#ifdef CONFIG_MD_FR1
+        if (conf->bitmap) {
+                struct bitmap *bitmap = conf->bitmap;
+                // bitmap->print_stats(bitmap);
+                bitmap->stop(bitmap);
+        }
+#endif /* CONFIG_MD_FR1 */
 }

 static int raid1_spare_active(mddev_t *mddev)


Mind you, "stop" only sets a bit. To really flush it one has to run
remove_bitmap(conf) and then create_bitmap(conf).


The part in raid1.c where you were getting all the oops on second
reinding (because of traversing a kfreed pointer to rdev, I think)
I rewrote as follows to first of all do the testing slowly with plenty
of noise so that we can see what happens, and then to kfree the rdev
that we have now refused to kfree earlier, in case we find it.

@@ -734,9 +1045,43 @@
        mirror_info_t *p;

        spin_lock_irq(&conf->device_lock);
-       for (mirror=0; mirror < mddev->raid_disks; mirror++)
+       for (mirror=0; mirror < mddev->raid_disks; mirror++) {
+#ifdef CONFIG_MD_FR1
+            /*
+             * allow a disk which has only been set faulty but not
+             * removed yet to be reinserted, thus triggering a hot
+             * repair.
+             */
+               p = &conf->mirrors[mirror]; 
+                if (unlikely(!p->rdev))
+                        goto insert_or_replace;
+               printk(KERN_DEBUG "raid1: testing p->rdev %p\n", p->rdev);
+                if (unlikely(p->rdev == rdev))
+                        goto insert_or_replace;
+               printk(KERN_DEBUG "raid1: testing p->rdev->bdev %p\n",
+                        p->rdev->bdev);
+                if (!p->rdev->bdev)
+                        goto insert_or_replace; // weird!
+               printk(KERN_DEBUG "raid1: testing p->rdev->bdev->bd_dev %x\n",
+                        p->rdev->bdev->bd_dev);
+                if (p->rdev->bdev->bd_dev == rdev->bdev->bd_dev)
+                        goto insert_or_replace;
+                continue;
+
+insert_or_replace:
+               if (1) {
+                        if (p->rdev && p->rdev != rdev) {
+                               /* kill the rdev left by export_rdev() */
+                               printk(KERN_INFO
+                                "raid1: late free of exported rdev %p\n",
+                                        p->rdev);
+                                kfree(p->rdev);
+                        }
+                       p->rdev = rdev;
+#else
                if ( !(p=conf->mirrors+mirror)->rdev) {
                        p->rdev = rdev;
+#endif /* CONFIG_MD_FR1 */

                        blk_queue_stack_limits(mddev->queue,
                                               rdev->bdev->bd_disk->queue);

The suppressed kfree is very badly done at present, in md.c:

@@ -1088,6 +1227,16 @@
        md_autodetect_dev(rdev->bdev->bd_dev);
 #endif
        unlock_rdev(rdev);
+#ifdef MD_BITMAP_SUPPORT
+#ifdef CONFIG_MD_FR1
+            /* FIXME only kfree if pers hot_add does not kfree instead.
+             * Use some flag in future! 
+             */
+       printk(KERN_WARNING
+                "md: WARNING: delaying free of exported rdev %p\n", rdev);
+        if (0)
+#endif /* CONFIG_MD_FR1 */
+#endif /* MD_BITMAP_SUPPORT */
        kfree(rdev);
 }

The problem is that this is a layering violation - the md layer must
not kfree the raid struct if the raid1 layer will remove it later
instead. It needs further study. There are several ways out of the
problem ...

One is to have the raid1 code mark something with a flag if it can do
the delayed kfree.  The problem is that in md.c we are in a routine that
has no access to the mddev and mddev->conf structs that usually contain
that kind of info.  Probably the best we can do is recover the
superblock of the rdev (if it has one) and look for a mark there.  At
least we can test for level_to_pers(sb->level) == RAID1 and CONFIG_FR1
set, but this is messy.  The sb is messy to recover and may not alays be
there.

Another possibility is to look for somewhere in the md level where it
can be seen later if the rdev kfree has been delayed, and do it then.
It is done in the raid1 layer at present in a pers->hot_add frm
check_recovery in md.c, here:

                if (mddev->degraded) {
                        ITERATE_RDEV(mddev,rdev,rtmp)
                                if (rdev->raid_disk < 0
                                    && !rdev->faulty) {

(we have made the old disk look like a spare - not inserted and not
faulty - in order to get here)

                                        if (mddev->pers->hot_add_disk(mddev,rdev))
                                                spares++;
                                        else
                                                break;
                                }
                }       

It may be possible to try and find the entry in
conf->mirrors[raid_disk].rdev before launching the hot add, then look
afterwards to see if it has changed, then kfree the earlier value if it
has.  Unfortunately, "conf" and like stuff is known only to the raid1
layer.

Peter