per-cpu areas

Linus, please apply (retransmit, neatened a little).

Thanks,
Rusty.

diff -urN -I \$.*\$ --exclude TAGS -X /home/rusty/devel/kernel/kernel-patches/current-dontdiff --minimal linux-2.5.3/include/linux/smp.h working-2.5.3-percpu/include/linux/smp.h
@@ -11,6 +11,7 @@
 #ifdef CONFIG_SMP
 
 #include <linux/kernel.h>
+#include <linux/compiler.h>
 #include <asm/smp.h>
 
 /*

@@ -71,7 +72,17 @@
 #define MSG_RESCHEDULE		0x0003	/* Reschedule request from master CPU*/
 #define MSG_CALL_FUNCTION       0x0004  /* Call function on all other CPUs */
 
-#else
+#define __per_cpu_data	__attribute__((section(".data.percpu")))
+
+#ifndef __HAVE_ARCH_PER_CPU
+extern unsigned long __per_cpu_offset[NR_CPUS];
+
+/* var is in discarded region: offset to particular copy we want */
+#define per_cpu(var, cpu) RELOC_HIDE(var, per_cpu_offset(cpu))
+
+#define this_cpu(var) per_cpu(var, smp_processor_id())
+#endif /* !__HAVE_ARCH_PER_CPU */
+#else /* !SMP */
 
 /*
  *	These macros fold the SMP functionality into a single CPU system

@@ -88,6 +99,9 @@
 #define cpu_online_map				1
 static inline void smp_send_reschedule(int cpu) { }
 static inline void smp_send_reschedule_all(void) { }
+#define __per_cpu_data
+#define per_cpu(var, cpu)			var
+#define this_cpu(var)				var
 
 #endif
 #endif

diff -urN -I \$.*\$ --exclude TAGS -X /home/rusty/devel/kernel/kernel-patches/current-dontdiff --minimal linux-2.5.3/init/main.c working-2.5.3-percpu/init/main.c
@@ -272,8 +272,32 @@
 #define smp_init()	do { } while (0)
 #endif
 
+static inline void setup_per_cpu_areas(void)
+{
+}
 #else
 
+#ifndef __HAVE_ARCH_PER_CPU
+unsigned long __per_cpu_offset[NR_CPUS];
+
+static void __init setup_per_cpu_areas(void)
+{
+	unsigned long size, i;
+	char *ptr;
+	/* Created by linker magic */
+	extern char __per_cpu_start[], __per_cpu_end[];
+
+	/* Copy section for each CPU (we discard the original) */
+	size = ALIGN(__per_cpu_end - __per_cpu_start, SMP_CACHE_BYTES);
+	ptr = alloc_bootmem(size * NR_CPUS);
+
+	for (i = 0; i < NR_CPUS; i++, ptr += size) {
+		__per_cpu_offset[i] = ptr - __per_cpu_start;
+		memcpy(ptr, __per_cpu_start, size);
+	}
+}
+#endif /* !__HAVE_ARCH_PER_CPU */
+
 /* Called by boot processor to activate the rest. */
 static void __init smp_init(void)
 {

@@ -316,6 +340,7 @@
 	lock_kernel();
 	printk(linux_banner);
 	setup_arch(&command_line);
+	setup_per_cpu_areas();
 	printk("Kernel command line: %s\n", saved_command_line);
 	parse_options(command_line);
 	trap_init();

diff -urN -I \$.*\$ --exclude TAGS -X /home/rusty/devel/kernel/kernel-patches/current-dontdiff --minimal linux-2.5.3/include/linux/compiler.h working-2.5.3-percpu/include/linux/compiler.h
@@ -13,4 +13,11 @@
 #define likely(x)	__builtin_expect((x),1)
 #define unlikely(x)	__builtin_expect((x),0)
 
+/* This macro obfuscates arithmetic on a variable address so that gcc
+   shouldn't recognize the original var, and make assumptions about it */
+	strcpy(s, "xxx"+X) => memcpy(s, "xxx"+X, 4-X) */
+#define RELOC_HIDE(var, off)						\
+  ({ __typeof__(&(var)) __ptr;					\
+    __asm__ ("" : "=g"(__ptr) : "0"((void *)&(var) + (off)));	\
+    *__ptr; })
 #endif /* __LINUX_COMPILER_H */

diff -urN -I \$.*\$ --exclude TAGS -X /home/rusty/devel/kernel/kernel-patches/current-dontdiff --minimal linux-2.5.3/include/linux/cache.h working-2.5.3-percpu/include/linux/cache.h
@@ -4,8 +4,10 @@
 #include <linux/config.h>
 #include <asm/cache.h>
 
+#define ALIGN(x,a) (((x)+(a)-1)&~((a)-1))
+
 #ifndef L1_CACHE_ALIGN
-#define L1_CACHE_ALIGN(x) (((x)+(L1_CACHE_BYTES-1))&~(L1_CACHE_BYTES-1))
+#define L1_CACHE_ALIGN(x) ALIGN(x, L1_CACHE_BYTES)
 #endif
 
 #ifndef SMP_CACHE_BYTES

diff -urN -I \$.*\$ --exclude TAGS -X /home/rusty/devel/kernel/kernel-patches/current-dontdiff --minimal linux-2.5.3/arch/i386/vmlinux.lds working-2.5.3-percpu/arch/i386/vmlinux.lds
@@ -58,6 +58,10 @@
 	*(.initcall7.init)
   }
   __initcall_end = .;
+  . = ALIGN(32);
+  __per_cpu_start = .;
+  .data.percpu  : { *(.data.percpu) }
+  __per_cpu_end = .;
   . = ALIGN(4096);
   __init_end = .;
 

diff -urN -I \$.*\$ --exclude TAGS -X /home/rusty/devel/kernel/kernel-patches/current-dontdiff --minimal linux-2.5.3/arch/ppc/vmlinux.lds working-2.5.3-percpu/arch/ppc/vmlinux.lds
@@ -104,6 +104,10 @@
 	*(.initcall7.init)
   }
   __initcall_end = .;
+  . = ALIGN(32);
+  __per_cpu_start = .;
+  .data.percpu  : { *(.data.percpu) }
+  __per_cpu_end = .;
   . = ALIGN(4096);
   __init_end = .;
 

--
Anyone who quotes me in their sig is an idiot. -- Rusty Russell.

There is no such a thing like a ide-clean-15. Never was. But here comes what has been done in ide-clean-16:

- Apply a patch for the initialization of the second PIIX channel. Found by Daniel Quinlan <[email protected]>

- Apply a patch for the DMA initialization of the serverworks chip. Ken Brownfield <[email protected]>

- Make the ata_operations methods immune against device type drivers, which donot provide them by separating the access to them out. Audit them all.

- Resynchronize with 2.5.6-pre1.

- Remove unused IDE_DRIVE_CMD, IDE_DRIVE_TASK, IDE_DRIVE_TASK_MASK, IDE_DRIVE_TASKFILE macros.

- Disable configuration of the task file stuff. It is going to go away and will be replaced by a truly abstract interface based on functionality and *not* direct mess-up of hardware.

- Resync with 2.5.6-pre2.

- Add HPT entries to the fall-back list, since otherwise the driver won'trecognize the drives. We will have to make this the default behavior for allnot recognized host chip types.

- Fix compilation with no PCI host chip support enabled.

- Apply the overflow fixes for HPT366 by Vojtech Pavlik.

- Kill the one-shoot functions ide_wait_cmd_taks() ide_wait_cmd() by moving them to the places where they are actually used. Fix a potential buffer overflow on the way.

- Fix usage of ide.c as module. Thanks to Adam J. Richter for figuring out what was wrong.

- Various cleanups all along as well as removal of TONS of unfinished/dead code.

I think it's sometimes better to remove stuff, which isn't there, instead of hoping for a "magical day" where it will be finished.

I intend to kill the largely overdesigned taskfile stuff. It's broken by design to provide this micro level of device access to *anybody*. Operating systems should try to present the functionality of devices in a convenient way to the user and not just mapp it directly to user space.

diff -ur linux-2.5.5/arch/alpha/defconfig linux/arch/alpha/defconfig
@@ -237,7 +237,6 @@
 # CONFIG_BLK_DEV_IDETAPE is not set
 # CONFIG_BLK_DEV_IDEFLOPPY is not set
 # CONFIG_BLK_DEV_IDESCSI is not set
-# CONFIG_IDE_TASK_IOCTL is not set
 
 #
 # IDE chipset support/bugfixes

diff -ur linux-2.5.5/arch/arm/def-configs/shannon linux/arch/arm/def-configs/shannon
@@ -378,7 +378,6 @@
 # CONFIG_BLK_DEV_IDECD is not set
 # CONFIG_BLK_DEV_IDETAPE is not set
 # CONFIG_BLK_DEV_IDEFLOPPY is not set
-# CONFIG_IDE_TASK_IOCTL is not set
 
 #
 # IDE chipset support/bugfixes

diff -ur linux-2.5.5/arch/i386/defconfig linux/arch/i386/defconfig
@@ -239,7 +239,6 @@
 # CONFIG_BLK_DEV_IDETAPE is not set
 # CONFIG_BLK_DEV_IDEFLOPPY is not set
 # CONFIG_BLK_DEV_IDESCSI is not set
-CONFIG_IDE_TASK_IOCTL=y
 
 #
 # IDE chipset support/bugfixes

diff -ur linux-2.5.5/arch/sparc64/defconfig linux/arch/sparc64/defconfig
@@ -273,7 +273,6 @@
 # CONFIG_BLK_DEV_IDETAPE is not set
 # CONFIG_BLK_DEV_IDEFLOPPY is not set
 # CONFIG_BLK_DEV_IDESCSI is not set
-# CONFIG_IDE_TASK_IOCTL is not set
 
 #
 # IDE chipset support/bugfixes

diff -ur linux-2.5.5/arch/x86_64/defconfig linux/arch/x86_64/defconfig
@@ -188,7 +188,6 @@
 # CONFIG_BLK_DEV_IDETAPE is not set
 # CONFIG_BLK_DEV_IDEFLOPPY is not set
 # CONFIG_BLK_DEV_IDESCSI is not set
-# CONFIG_IDE_TASK_IOCTL is not set
 
 #
 # IDE chipset support/bugfixes

diff -ur linux-2.5.5/drivers/ide/Config.help linux/drivers/ide/Config.help
@@ -807,14 +807,6 @@
 
   If you are unsure, say N here.
 
-CONFIG_IDE_TASK_IOCTL
-  This is a direct raw access to the media.  It is a complex but
-  elegant solution to test and validate the domain of the hardware and
-  perform below the driver data recovery if needed.  This is the most
-  basic form of media-forensics.
-
-  If you are unsure, say N here.
-
 CONFIG_BLK_DEV_IDEDMA_FORCED
   This is an old piece of lost code from Linux 2.0 Kernels.
 

diff -ur linux-2.5.5/drivers/ide/Config.in linux/drivers/ide/Config.in
@@ -33,9 +33,7 @@
    dep_tristate '  Include IDE/ATAPI FLOPPY support' CONFIG_BLK_DEV_IDEFLOPPY $CONFIG_BLK_DEV_IDE
    dep_tristate '  SCSI emulation support' CONFIG_BLK_DEV_IDESCSI $CONFIG_BLK_DEV_IDE $CONFIG_SCSI
 
-   bool '  IDE Taskfile Access' CONFIG_IDE_TASK_IOCTL
-
-   comment 'IDE chipset support/bugfixes'
+   comment 'IDE chipset support'
    if [ "$CONFIG_BLK_DEV_IDE" != "n" ]; then
       dep_bool '  CMD640 chipset bugfix/support' CONFIG_BLK_DEV_CMD640 $CONFIG_X86
       dep_bool '    CMD640 enhanced support' CONFIG_BLK_DEV_CMD640_ENHANCED $CONFIG_BLK_DEV_CMD640

diff -ur linux-2.5.5/drivers/ide/hpt366.c linux/drivers/ide/hpt366.c
@@ -374,7 +374,8 @@
 		class_rev &= 0xff;
 
 		p += sprintf(p, "\nController: %d\n", i);
-		p += sprintf(p, "Chipset: HPT%s\n", chipset_nums[class_rev]);
+		p += sprintf(p, "Chipset: HPT%s\n",
+			class_rev < sizeof(chipset_nums) / sizeof(char *) ? chipset_nums[class_rev] : "???");
 		p += sprintf(p, "--------------- Primary Channel "
 				"--------------- Secondary Channel "
 				"--------------\n");

@@ -1119,12 +1120,11 @@
 	if (test != 0x08)
 		pci_write_config_byte(dev, PCI_MAX_LAT, 0x08);
 
-	if (pci_rev_check_hpt3xx(dev)) {
+	if (pci_rev_check_hpt3xx(dev))
 		init_hpt370(dev);
+
+	if (n_hpt_devs < HPT366_MAX_DEVS)
 		hpt_devs[n_hpt_devs++] = dev;
-	} else {
-		hpt_devs[n_hpt_devs++] = dev;
-	}
 	
 #if defined(DISPLAY_HPT366_TIMINGS) && defined(CONFIG_PROC_FS)
 	if (!hpt366_proc) {

diff -ur linux-2.5.5/drivers/ide/ide-geometry.c linux/drivers/ide/ide-geometry.c
@@ -14,14 +14,14 @@
 #if defined(CONFIG_BLK_DEV_IDE) || defined(CONFIG_BLK_DEV_IDE_MODULE)
 
 extern ide_drive_t * get_info_ptr(kdev_t);
-extern unsigned long current_capacity (ide_drive_t *);
 
 /*
  * If heads is nonzero: find a translation with this many heads and S=63.
  * Otherwise: find out how OnTrack Disk Manager would translate the disk.
  */
 static void
-ontrack(ide_drive_t *drive, int heads, unsigned int *c, int *h, int *s) {
+ontrack(ide_drive_t *drive, int heads, unsigned int *c, int *h, int *s)
+{
 	static const byte dm_head_vals[] = {4, 8, 16, 32, 64, 128, 255, 0};
 	const byte *headp = dm_head_vals;
 	unsigned long total;

@@ -34,7 +34,7 @@
 	 * computes a geometry different from what OnTrack uses.]
 	 */
 
-	total = ata_ops(drive)->capacity(drive);
+	total = ata_capacity(drive);
 
 	*s = 63;
 

@@ -136,7 +136,7 @@
 		ret = 1;
 	}
 
-	drive->part[0].nr_sects = current_capacity(drive);
+	drive->part[0].nr_sects = ata_capacity(drive);
 
 	if (ret)
 		printk("%s%s [%d/%d/%d]", msg, msg1,

diff -ur linux-2.5.5/drivers/ide/ide-pci.c linux/drivers/ide/ide-pci.c
@@ -303,7 +303,9 @@
 #ifdef CONFIG_BLK_DEV_IT8172
 	{PCI_VENDOR_ID_ITE, PCI_DEVICE_ID_ITE_IT8172G, pci_init_it8172,	NULL, ide_init_it8172, NULL, {{0x00,0x00,0x00}, {0x40,0x00,0x01}}, ON_BOARD, 0, 0 },
 #endif
-	/* Those are id's of chips we don't deal currently with. */
+	/* Those are id's of chips we don't deal currently with,
+	 * but which still need some generic quirk handling.
+	 */
 	{PCI_VENDOR_ID_PCTECH, PCI_DEVICE_ID_PCTECH_SAMURAI_IDE, NULL, NULL, NULL, NULL, {{0x00,0x00,0x00}, {0x00,0x00,0x00}}, ON_BOARD, 0, 0 },
 	{PCI_VENDOR_ID_CMD, PCI_DEVICE_ID_CMD_640, NULL, NULL, IDE_IGNORE, NULL, {{0x00,0x00,0x00}, {0x00,0x00,0x00}}, ON_BOARD, 0, 0 },
 	{PCI_VENDOR_ID_NS, PCI_DEVICE_ID_NS_87410, NULL, NULL, NULL, NULL, {{0x43,0x08,0x08}, {0x47,0x08,0x08}}, ON_BOARD, 0, 0 },

@@ -313,6 +315,7 @@
 	{PCI_VENDOR_ID_UMC, PCI_DEVICE_ID_UMC_UM8886A, NULL, NULL, NULL, NULL, {{0x00,0x00,0x00}, {0x00,0x00,0x00}}, ON_BOARD, 0, ATA_F_FIXIRQ },
 	{PCI_VENDOR_ID_UMC, PCI_DEVICE_ID_UMC_UM8886BF, NULL, NULL, NULL, NULL, {{0x00,0x00,0x00}, {0x00,0x00,0x00}}, ON_BOARD, 0, ATA_F_FIXIRQ },
 	{PCI_VENDOR_ID_VIA, PCI_DEVICE_ID_VIA_82C561, NULL, NULL, NULL, NULL, {{0x00,0x00,0x00}, {0x00,0x00,0x00}}, ON_BOARD, 0, ATA_F_NOADMA },
+	{PCI_VENDOR_ID_TTI, PCI_DEVICE_ID_TTI_HPT366, NULL, NULL, IDE_NO_DRIVER, NULL, {{0x00,0x00,0x00}, {0x00,0x00,0x00}}, OFF_BOARD, 240, ATA_F_IRQ | ATA_F_HPTHACK },
 	{0, 0, NULL, NULL, NULL, NULL, {{0x00,0x00,0x00}, {0x00,0x00,0x00}}, ON_BOARD, 0 }};
 
 /*

diff -ur linux-2.5.5/drivers/ide/ide-probe.c linux/drivers/ide/ide-probe.c
@@ -471,9 +471,11 @@
 	sprintf(hwif->device.bus_id, "%04x", hwif->io_ports[IDE_DATA_OFFSET]);
 	sprintf(hwif->device.name, "ide");
 	hwif->device.driver_data = hwif;
+#ifdef CONFIG_BLK_DEV_IDEPCI
 	if (hwif->pci_dev)
 		hwif->device.parent = &hwif->pci_dev->dev;
 	else
+#endif
 		hwif->device.parent = NULL; /* Would like to do = &device_legacy */
 	device_register(&hwif->device);
 

diff -ur linux-2.5.5/drivers/ide/ide-proc.c linux/drivers/ide/ide-proc.c
@@ -353,7 +353,7 @@
 	if (!driver)
 		len = sprintf(page, "(none)\n");
         else
-		len = sprintf(page,"%llu\n", (unsigned long long) drive->driver->capacity(drive));
+		len = sprintf(page,"%llu\n", (unsigned long long) ata_capacity(drive));
 	PROC_IDE_READ_RETURN(page,start,off,count,eof,len);
 }
 

diff -ur linux-2.5.5/drivers/ide/ide-tape.c linux/drivers/ide/ide-tape.c
@@ -6140,7 +6140,12 @@
 
 #endif
 
-static int idetape_reinit(ide_drive_t *drive);
+static void idetape_revalidate(ide_drive_t *_dummy)
+{
+	/* We don't have to handle any partition information here, which is the
+	 * default behaviour of this method.
+	 */
+}
 
 /*
  *	IDE subdriver functions, registered with ide.c

@@ -6149,18 +6154,16 @@
 	owner:			THIS_MODULE,
 	cleanup:		idetape_cleanup,
 	standby:		NULL,
-	flushcache:		NULL,
 	do_request:		idetape_do_request,
 	end_request:		idetape_end_request,
 	ioctl:			idetape_blkdev_ioctl,
 	open:			idetape_blkdev_open,
 	release:		idetape_blkdev_release,
-	media_change:		NULL,
-	revalidate:		NULL,
+	check_media_change:	NULL,
+	revalidate:		idetape_revalidate,
 	pre_reset:		idetape_pre_reset,
 	capacity:		NULL,
-	proc:			idetape_proc,
-	driver_reinit:		idetape_reinit,
+	proc:			idetape_proc
 };
 
 /*

@@ -6175,12 +6178,6 @@
 	release:	idetape_chrdev_release,
 };
 
-/* This will propably just go entierly away... */
-static int idetape_reinit (ide_drive_t *drive)
-{
-	return 1;
-}
-
 MODULE_DESCRIPTION("ATAPI Streaming TAPE Driver");
 MODULE_LICENSE("GPL");
 

diff -ur linux-2.5.5/drivers/ide/ide-taskfile.c linux/drivers/ide/ide-taskfile.c
@@ -1750,6 +1750,29 @@
 	return err;
 }
 
+/*
+ * Issue ATA command and wait for completion. use for implementing commands in
+ * kernel.
+ *
+ * The caller has to make sure buf is never NULL!
+ */
+static int ide_wait_cmd(ide_drive_t *drive, int cmd, int nsect, int feature, int sectors, byte *argbuf)
+{
+	struct request rq;
+
+	/* FIXME: Do we really have to zero out the buffer?
+	 */
+	memset(argbuf, 0, 4 + SECTOR_WORDS * 4 * sectors);
+	ide_init_drive_cmd(&rq);
+	rq.buffer = argbuf;
+	*argbuf++ = cmd;
+	*argbuf++ = nsect;
+	*argbuf++ = feature;
+	*argbuf++ = sectors;
+
+	return ide_do_drive_cmd(drive, &rq, ide_wait);
+}
+
 int ide_cmd_ioctl (ide_drive_t *drive, struct inode *inode, struct file *file, unsigned int cmd, unsigned long arg)
 {
 	int err = 0;

@@ -1806,12 +1829,20 @@
 int ide_task_ioctl (ide_drive_t *drive, struct inode *inode, struct file *file, unsigned int cmd, unsigned long arg)
 {
 	int err = 0;
-	byte args[7], *argbuf = args;
+	u8 args[7];
+	u8 *argbuf;
 	int argsize = 7;
+	struct request rq;
+
+	argbuf = args;
 
 	if (copy_from_user(args, (void *)arg, 7))
 		return -EFAULT;
-	err = ide_wait_cmd_task(drive, argbuf);
+
+	ide_init_drive_cmd(&rq);
+	rq.flags = REQ_DRIVE_TASK;
+	rq.buffer = argbuf;
+	err = ide_do_drive_cmd(drive, &rq, ide_wait);
 	if (copy_to_user((void *)arg, argbuf, argsize))
 		err = -EFAULT;
 	return err;

diff -ur linux-2.5.5/drivers/ide/piix.c linux/drivers/ide/piix.c
@@ -214,7 +214,7 @@
 {
 	unsigned long flags;
 	u16 master_data;
-	byte slave_data;
+	u8 slave_data;
 	int is_slave		= (&HWIF(drive)->drives[1] == drive);
 	int master_port		= HWIF(drive)->index ? 0x42 : 0x40;
 	int slave_port		= 0x44;

diff -ur linux-2.5.5/drivers/ide/serverworks.c linux/drivers/ide/serverworks.c
@@ -660,8 +660,10 @@
 	hwif->autodma = 0;
 #else /* CONFIG_BLK_DEV_IDEDMA */
 	if (hwif->dma_base) {
+#ifdef CONFIG_IDEDMA_AUTO
 		if (!noautodma)
 			hwif->autodma = 1;
+#endif
 		hwif->dmaproc = &svwks_dmaproc;
 		hwif->highmem = 1;
 	} else {

diff -ur linux-2.5.5/drivers/scsi/ide-scsi.c linux/drivers/scsi/ide-scsi.c
@@ -536,7 +536,11 @@
 	return 0;
 }
 
-static int idescsi_reinit(ide_drive_t *drive);
+static void idescsi_revalidate(ide_drive_t *_dummy)
+{
+	/* The partition information will be handled by the SCSI layer.
+	 */
+}
 
 /*
  *	IDE subdriver functions, registered with ide.c

@@ -545,26 +549,19 @@
 	owner:			THIS_MODULE,
 	cleanup:		idescsi_cleanup,
 	standby:		NULL,
-	flushcache:		NULL,
 	do_request:		idescsi_do_request,
 	end_request:		idescsi_end_request,
 	ioctl:			NULL,
 	open:			idescsi_open,
 	release:		idescsi_ide_release,
-	media_change:		NULL,
-	revalidate:		NULL,
+	check_media_change:	NULL,
+	revalidate:		idescsi_revalidate,
 	pre_reset:		NULL,
 	capacity:		NULL,
 	special:		NULL,
-	proc:			NULL,
-	driver_reinit:		idescsi_reinit,
+	proc:			NULL
 };
 
-static int idescsi_reinit (ide_drive_t *drive)
-{
-	return 0;
-}
-
 /*
  *	idescsi_init will register the driver for each scsi.
  */

Could you elaborate just a tad on that.

Thanks,
Zwane

The task file stuff was basically providing a mapping between every single possible ATA/ATAPI/whatever device command to the ioctl interface. It was for example including mechanisms to initiate DMA transfers into nirvana. This is not a good thing
for the following reasons:

1. The standard itself changes quite frequently.

2. I would rather expect that the cornercases (read: not used by Win32) of this interface will not be implemented properly or the implementations will be either broken or not complete. This is making this interface really questionable for general use.
As an added bonus this is introducing magnificient possibilities for failure or even true hardware breakdown (possible).
Bah. It is right now the fact that 80% of this stuff isn't implemented on older drives.

3. The code implementing it is of really really poor quality and very heavy wight. It makes it really hard to target the true problems the driver has on a far more trivial level.

4. One should have an ioctl setting the drivers silence policy based on a switch choice value for example instead of an ioctl, which contains the corresponding ATA command packet. This would allow for example to support drives in the future if the most common method of configuring this changes. Or one could for example identify silent operation mode of disks with rotation speed trottle on DVD drives.

5. It is necessary to integrate most features found there with other kernel functionality context (suspend and reset come to mind as first). It doesn't therefore make *any* sense to expose this interface to user space.

(This is the abstraction problem.)

And infally:

5. No body is using it as of now and therefore nobody should miss it. No other OS out there has something similar, so maybe they don't need it? I find it allways ridiculous to see that Win32's ATA drivers are by the fact 3 smaller then the one found in linux.

6. Some special commands found there (unless issuing IRQ's) can be entierly handled in userspace.

Home it helps.

PS. If you are still in doubt, please:

1. Look at ide-taskfile.c and vommit ;-).
2. Try to show me a usufull programm using this.
3. Count the number of "options" of the IDE driver entry in menuconfig.

That is a very bold statement which is incorrect. I remember reading at least one post to lkml from a company who is using the Taskfile ioctls and having a lot of praise for them. Sorry I delete older lkml stuff so I can't quote you the post but I am sure you could find it in the archives if you tried to look.

Considering your massive amount of changes which are, A) completely uncoordinated with Andre, and B) going well beyond what one could consider cleanup, I am still waiting to see you place your name as IDE maintainer... Clearly you have taken over maintenance now so stand up to it.

Best regards,

Anton

  "I've not lost my mind. It's backed up on tape somewhere." - Unknown
-- 
Anton Altaparmakov <aia21 at cam.ac.uk> (replace at with @)
Linux NTFS Maintainer / WWW: http://linux-ntfs.sf.net/
ICQ: 8561279 / WWW: http://www-stu.christs.cam.ac.uk/~aia21/

I know several people using them, and for some ioctl operations they are required. In fact without taskfile ioctl stuff I can't make my laptop resume correctly for example. (it needs proper drive please wake up sequences to go the ibm microdrive)

At this point I think you've overstepped the mark. The taskfile stuff is rather important. If you don't understand why please read the ATA6 standard and look at the sequence of phases. Think of it as a scsi sequencer in kernel space because the hardware isnt bright enough to do it.

Alan

I would rather love to see a systrace of it in action to see which API this is actually really using. ;-).

Reminds me - read the ACPI specification on IDE devices too - thats also relevant to the taskfile discussion.

It can't be older then 2.5.4 becouse before it just wasn't there.

Old is a few days-a week on my time scale but the post i am referring to is going at least a month or so back. And of course it can be older, you seem to be forgetting that Andre's IDE patches have been around for a _long_ time now for 2.4.x...

Anton

  "I've not lost my mind. It's backed up on tape somewhere." - Unknown
-- 
Anton Altaparmakov <aia21 at cam.ac.uk> (replace at with @)
Linux NTFS Maintainer / WWW: http://linux-ntfs.sf.net/
ICQ: 8561279 / WWW: http://www-stu.christs.cam.ac.uk/~aia21/

Yes it was before that. I recall the email too but can't find the reference.

Zwane

Wrong. Its been in the 2.4 IDE patches that anyone doing serious IDE work has been using for oh 9 months or more..

But is a short time ago that IDE problems appeared in 2.4.18... Please just have a look at the actual code in ide-taskfile.c and ask yourself whatever it is *really* stable and usable. Using an editor with syntax highlighting for code parts commented out by #if 0 .. #endif will reveal most of the trouble instantly.

2.4.18 base doesn't have this code so that smells like manure to me.

Irrespective of the implementation you need the functionality. If you want to say "hey this implementation sucks" - I agree in part. If you want to remove it then its time for the big flashing "BOGUS" sign

Here is that reference, chances are there are more non vocal people using the interface than what you read on LKML.

---------- Forwarded message ----------
Date: 21 Jan 2002 17:46:13 -0500
From: John Zedlewski <[email protected]>
To: [email protected]
Subject: IDE patch + Taskfile

Like everybody else, I wanted to chime in about my happiness with the IDE patch (I'm using it via the 2.4.18-pre3-mjc patch collection). Works beautifully on my laptop with an IBM Travelstar (DJSA-220) and an IDE-interface IBM microdrive (via a compactflash adapter).

But I also wanted to REALLY thank Andre and friends for the new TASKFILE ioctl. I'm doing a lot of low-level performance testing on the Microdrive and the old ioctl interface was completely inadequate. For instance, several of the CFA_* IDE commands were unusable from userspace before taskfile came along and now they're a snap...

Thanks again!
--JRZ

Hugh... Apparently I mus have missed something. Sorry screw up on my part. I have apparently grepped the wrong patch set.

So finally we agree on the "hey this implementation sucks" part. OK?

While the taskfile interface is very down-to-basics and a bit extreme in one end, it's also very useful for eg vendors doing testing and certification. So in that respect it's pretty powerful, I hope Martin isn't just planning a stripped down interface akin to what we have in 2.4 and earlier.

Jens Axboe

No quite my plan is:

1. Rip it off.
2. Reimplement stuff if and only if someone really shows pressure for using it.

The "command parsing" excess is certainly going to go.

Its maybe handy actually. Without command parsing I can tell the drive to do anything without good control - you know say like all the upcoming SSSCA encrypt chunks of your harddisk so you can never get them back stuff.

The important bit is that for each command you must know the sequence of phases. Get it wrong and your storage system goes off to visit undefined states. I don't like my disks in undefined states because it tends to leave them with undefined content.

Two things I do think wants considering

#1 Can the same thing be done by passing the command and sequence of
transitions from user space (scsi generic takes that approach but scsi is a little more forgiving since the bogus transition will screw your command in a "oh whoops" detectable manner). IDE has a nice habit of explaining you screwed up by scribbling on the disk and/or locking solid

#2 Shoot all the little routines and make them into a table.

That would tidy it no end.

I will just try the aproach 2 first.

Someone has, I'll forward you the stuff.

Agree

Jens Axboe

Thank's in advance!

Martin - please go and use a macintosh for 8 weeks then come back. The unix philosophy is make the simple things easy make the complex possible. Without that low level IDE access you might as well stop hacking on the IDE code because eventually nobody will be able to use your IDE code anyway

I would add to that than rather than killing the taskfile stuff, it should instead be generalized and any IDE access be done via a taskfile.

I don't comment on the actual implementation quality as I didn't look into it closely, but the point of passing requests as taskfile's down to the lowest level driver allow more consistency between the various pieces of the driver, more easily hooking of the low level taskfile "apply" code to accomodate MMIO or strangely mapped IDE controllers, etc...

Alan: BTW, Apple's Darwin has a nice ATA layer implementation that happens to be completely taskfile based :) Ask Andre what he thinks about their ATA & SCSI layer, except from bloat due to their C++ implementation, their overall design is actually really nice !

Ben.

Add "and do as much in user-space" in esp. if the stuff is anyway only suitable for root and we agree. My main objection is just the way the driver commands are exposed to user space and the way they are validated in kernel space.

I have looked at it and come to the following conclusions:

1. Indeed the code quality found there is *excellent* nothing comparable with the messy crude found currently in linux.

The state machine (the finite automata one i mean) is *sweet* and can be even a subject to proper formal validation. This is quite contrary to the interrupt handler pointer passing games done under linux right now. There are clearly defined command categories determined by device state instead of the particular transport layer. (reset, synchroneous, asynchronous). There is proper separation between the device types using the same hardware bus layer but completely different transport layers (ATA vers ATAPI - which is SCSI in disguise).

2. It convinced me that the current task-file interface in linux is inadequate. So Alan please bear with me if your CF format microdrive will have to not wakeup properly for some time... The current mess will just have to go before something more adequate can go in.

3. Someone had too much time at apple, becouse the C++ found there doesn't apparently contain anything that couldn't be expressed without any pain in plain C with structs containing function pointers ;-).

I thought you stated that no one else was using something similar? Or were you refererring to the userland accessible ioctls? I believe FreeBSD-CURRENT might also have something in the works.

Before removing the current "mess", would it be too much to ask to provide a patches which remove and incorporate your proposed changes?

I was referring to the userland accessible ioctls going through HDIO_DRIVE_TASKFILE - a recent introduction in 2.5.3. And I didn't found anything comparable in the Apple code.
After looking at the code I can actually assure you that FreeBSD-CURRENT does not contain anything comparable to this.

MD> 2. It convinced me that the current task-file interface in linux

MD>     is inadequate. So Alan please bear with me if your CF format
MD>     microdrive will have to not wakeup properly for some time...
MD>     The current mess will just have to go before something more
MD>     adequate can go in.

Why not keep the existing taskfile implementation in until you complete the elegant implementation?

Meelis Roos ([email protected])

It's still there ;-). However doing it this way is nearly impossible since there is no continuous transition line.

Fortunately you have the 2.5 kernel to do just this type of thing,
and others who don't share the "it's not perfect, rewrite from scratch" may be able to find a clean way to provide true RAW access for root."

Can't disagree, I never understood how people who can understand
inheritance can be fuddled by pointers to functions.

bill davidsen <[email protected]>
  CTO, TMR Associates, Inc
Doing interesting things with little computers since 1979.

Hehe, right :) Though they actually limit the C++ usage in the kernel to a kind of "embedded C++", that is without multiple inheritance, RTTI or exceptions. C++ is the main reason why I call it "bloated".

Ben.

In article <[email protected]>,

One thing I'd love to see in C is default values for structure members in initializers.

Pretty much everything else is trivially done with structures and function pointers - once you allocate things dynamically you can (and should) trivially and logically just make the allocator initialize the needed fields too. But for static allocations and static initializers you cannot cleanly do the same thing - you have to add explicit code that knows about each statically allocated entry.

That's basically the only piece of object constructors that I consider really _useful_, with the rest just being syntactic fluff.

Linus

Can we also expect a patch to remove the scb's from the scsi midlayer from you ?
I mean, if a standard specifies a nice *common* command packet format I'd expect the midlayer
to create such packets. Taskfile is exactly that... why removing it ?

Show me a usage of it! No please no abstract telling how usufull it *could* be. Look at the excessive implementation code. Look at the ATA standard and compare this to SCSI.

Note that taskfiles are not being removed from IDE. Just direct (and parsed and filtered) interface to userspace. Does the scsi midlayer export the SCBs directly to userspace?

Vojtech Pavlik
SuSE Labs

Yes. And whats more the scsi generic layer has a hell of a lot less ioctls than IDE because of that. With something like scsi enclosure, scsi smart, scsi scanners its a godsend to be able to just say "Ok OS take a hike, I wish to chat with this device exactly as I damn well please".

[Reminds me - anyone got any Linux enclosure monitoring code ?]

Please note that the plenthora of IDE ioctl comes from the fact that the ioctl interface is trying to export every single possible IDE command as an specific ioctl. This mechanism just got even extendid and called "taskfile".

It should.

I think it's a mistake to remove the taskfile interface.

It provides a way for people to directly validate the lowest level IDE interface, without interference from upper layers. It also provides access to userspace for important features that -should not- be in the kernel, like SMART monitoring and security features.

Jeff

Jeff Garzik      |
Building 1024    |
MandrakeSoft     | Choose life.

Well, Martin promised to reimplement it better later if there is demand for it, and it seems there is. So I suppose it's going away only to be replaced by something better.

There was a flamewar about this some time ago - whether the kernel should or should not parse the taskfile access to prevent possibly dangerous commands sent to the drive - if this is to be used for validation, then all commands need to be allowed, which also will enable to thin the code considerably.

Vojtech Pavlik
SuSE Labs

Please note that the code in question present currently there is quite frankly not functional. And that's the main problem with it - It's hard to guarantee proper functionality of the mail functionality if there is something "catching it's eggs from behind". Please note further that the excessive copying between a variable called frequently args and the "task file" register set just shows that the API the ide-taskfile is trying to implement is not right. Further on the excessive command format validation found in ide-taskfile.c is just too much of a burden.

- phone: +49 214 8656 283
- job:   eVision-Ventures AG, LEV .de (MY OPINIONS ARE MY OWN!)
- langs: de_DE.ISO8859-1, en_US, pl_PL.ISO8859-2, last ressort: ru_RU.KOI8-R