lkml.org 
[lkml]   [2004]   [Mar]   [10]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRe: [PATCH] 2.4.x Linux Medley RAID Version 7
Date
On Wednesday 10 of March 2004 16:19, Thomas Horsten wrote:
> On Wed, 10 Mar 2004, Bartlomiej Zolnierkiewicz wrote:
> > I don't like the idea of having 2 drivers doing basically the same thing.
> > Please explain why we can't have one Medley software RAID driver?
> > [ Yes, I read
> > http://www.ussg.iu.edu/hypermail/linux/kernel/0308.0/0156.html ]
>
> The existing one does not work correctly:
>
> - It detects the array in the wrong way and so only catches a fraction of
> valid Medley sets (and possibly mistakes non-Medley sets or deleted sets
> for correct ones).
>
> - It's using the wrong algorithm for dealing with striped arrays where the
> drives are of different sizes.

I know.

> The reason my patch leaves it in place is that it works for some users,
> and some people pointed out last year that the usual preferred path for
> this kind of change is to leave the existing driver in place if it works
> for some people. I would personally prefer to remove it.

It is a shame you haven't done it as a set of patches for the old one.
This way it would have been merged long time ago.

> Andre has previously stated that he had no objections to this patch (an
> earlier version).

Good but I am not Andre 8).

Ideally I would like you to split your driver on a set of patches against
sil_raid.c but it will be sufficient if you clean this patch a bit.
[ Please read Documentation/CodingStyle. ]

+ if (!inode || !inode->i_rdev)
+ {
+ return -EINVAL;
+ }
if (!inode || !inode->i_rdev)
return -EINVAL;
+ if (!arg) return -EINVAL;
if (!arg)
return -EINVAL;
+ if (raid[device].access != 1)
+ {
+ atomic_set(&(raid[device].valid),1);
+ return -EBUSY;
+ }
if (raid[device].access != 1) {
atomic_set(&(raid[device].valid),1);
return -EBUSY;
}
+ if (md->raid_type != 0x0)
+ {
+ printk(KERN_INFO "Medley RAID (%02x:%02x): Sorry, this driver currently only supports striped sets (RAID level 0).\n",
+ major,minor);
+ }
Please try follow 80-column limit.

+#if LINUX_VERSION_CODE >= KERNEL_VERSION(2,4,23)
+ ide_drive_t *drvinfo = ide_info_ptr(dev, 0);
+#else
+ ide_drive_t *drvinfo = get_info_ptr(dev);
+#endif

Patch for inclusion should have this cleaned up.

+ /* If this drive is not on a PCI controller, it is not Medley RAID.
+ * Medley matches the PCI device ID with the metadata to check if it is valid. */
+ pcidev = drvinfo->hwif?drvinfo->hwif->pci_dev:NULL;
+ if (!pcidev)
+ {
+ return NULL;
+ }
+
IMHO this is redundant/bogus -> I can get drives with Medley RAID off CMD/SiI
controller and plug them into legacy ISA controller and still be happy
(hey, it is a Linux way of doing things).

+ /* A valid Medley RAID has the PCI vendor/device ID of its IDE controller,
+ * and the correct checksum. */
+ md = (void *)(bh->b_data);
+
+ if (pcidev->vendor == md->vendor_id && pcidev->device == md->product_id)
The similar thing here - ie. I would like to replug drives to on-board Intel.
When Linux is driving RAID purely in software it shouldn't matter what
controller we are using.

Thanks,
Bartlomiej

-
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/

\
 
 \ /
  Last update: 2005-03-22 14:01    [from the cache]
©2003-2011 Jasper Spaans