Failed to attached the patch.
Andre Hedrick
Linux Disk Certification Project Linux ATA Development
---------- Forwarded message ----------
Date: Mon, 21 Jan 2002 01:48:10 -0800 (PST)
From: Andre Hedrick <[email protected]>
To: Jens Axboe <[email protected]>
Cc: Davide Libenzi <[email protected]>,
Anton Altaparmakov <[email protected]>,
Linus Torvalds <[email protected]>,
lkml <[email protected]>
Subject: Re: Linux 2.5.3-pre1-aia1
On Mon, 21 Jan 2002, Jens Axboe wrote:
> On Mon, Jan 21 2002, Andre Hedrick wrote:
> > > This really sucks, it means we cannot safely use multi mode for a
> > > variety of request sizes. I agree with your earlier comment. Here's what
> > > I think we should be doing: when requesting multi mode, limit to 8
> > > sectors like in your patch. This is by far the most commen multiple,
> > > that's why. When starting a request, use WIN_MULT* only for cases where
> > > (rq->nr_sectors % drive->mult_count) == 0. If that doesn't hold, simply
> > > use WIN_READ or WIN_WRITE.
> > >
> > > Applied the 2.5.3-pre2 sched SMP fix, booting -pre2 and then hacking up
> > > a patch.
> >
> > Why I have already done it, just take and apply.
>
> Cool, where is it?
Attached, and please do not pick and choose.
I moved and added things for a reason as not to loose hard work, because
of writing the ISR's to the purity of the spec, and then we modify ISR's
to fit the kernel and not the other way around. I do have a just reason
to request a MEMPOOL, which would be exclusively used for PIO operations.
Then we get out of the mess we are in and get in to serious compliance to
how the hardware works.
Thus in the offline comments about the creation of an ata_request_struct,
a mempool allocation for PIO is justified. Since the correct solution of
DMA timeouts is to void the request and assume no data down is valid.
Thus PIO is next.
If we look at the overhead in the generation of a new request for each and
every time we do a PIO transfer it is scary. Think about this issue for
more than the time it takes to hit the delete key.
Respectfully,
Andre Hedrick
Linux Disk Certification Project Linux ATA Development
On Mon, Jan 21 2002, Andre Hedrick wrote:
> @@ -189,13 +189,14 @@
> memset(&taskfile, 0, sizeof(task_struct_t));
> memset(&hobfile, 0, sizeof(hob_struct_t));
>
> - sectors = rq->nr_sectors;
> if (sectors == 256)
> sectors = 0;
> - if (command == WIN_MULTWRITE_EXT || command == WIN_MULTWRITE) {
> + if (command == WIN_MULTWRITE) {
> sectors = drive->mult_count;
> if (sectors > rq->current_nr_sectors)
> sectors = rq->current_nr_sectors;
> + if (sectors != drive->mult_count)
> + command = WIN_WRITE;
I think this is too restrictive, something ala
if (sectors % drive->mult_count)
command = WIN_WRITE;
should suffice. However, we still need to cover the read... So something
like this maybe:
if (sectors % drive->mult_count)
don't use multi write _or_ read, use WIN_{READ,WRITE}
/* usual setup */
> - sectors = rq->nr_sectors;
> - if (sectors == 256)
> + if (sectors == 65536)
> sectors = 0;
Yeah this was my silly, sorry.
--
Jens Axboe
On Mon, Jan 21 2002, Jens Axboe wrote:
> On Mon, Jan 21 2002, Andre Hedrick wrote:
> > @@ -189,13 +189,14 @@
> > memset(&taskfile, 0, sizeof(task_struct_t));
> > memset(&hobfile, 0, sizeof(hob_struct_t));
> >
> > - sectors = rq->nr_sectors;
> > if (sectors == 256)
> > sectors = 0;
> > - if (command == WIN_MULTWRITE_EXT || command == WIN_MULTWRITE) {
> > + if (command == WIN_MULTWRITE) {
> > sectors = drive->mult_count;
> > if (sectors > rq->current_nr_sectors)
> > sectors = rq->current_nr_sectors;
> > + if (sectors != drive->mult_count)
> > + command = WIN_WRITE;
>
> I think this is too restrictive, something ala
>
> if (sectors % drive->mult_count)
> command = WIN_WRITE;
>
> should suffice. However, we still need to cover the read... So something
> like this maybe:
>
> if (sectors % drive->mult_count)
> don't use multi write _or_ read, use WIN_{READ,WRITE}
>
> /* usual setup */
Patch attached, note it contains other stuff, the get_command change is
the one to note...
--- /opt/kernel/linux-2.5.3-pre2/drivers/ide/ide-disk.c Mon Jan 21 00:43:13 2002
+++ drivers/ide/ide-disk.c Mon Jan 21 03:37:20 2002
@@ -139,18 +142,33 @@
}
#endif /* CONFIG_BLK_DEV_PDC4030 */
+ /*
+ * get a new command (push ar further down to avoid grabbing lock here
+ */
+ spin_lock_irqsave(drive->queue.queue_lock, flags);
+ ar = ata_ar_get(drive);
+ spin_unlock_irqrestore(drive->queue.queue_lock, flags);
+
+ BUG_ON(!ar);
+ ar->ar_rq = rq;
+
if ((drive->id->cfs_enable_2 & 0x0400) && (drive->addressing)) /* 48-bit LBA */
- return lba_48_rw_disk(drive, rq, (unsigned long long) block);
+ return lba_48_rw_disk(drive, ar, block);
if (drive->select.b.lba) /* 28-bit LBA */
- return lba_28_rw_disk(drive, rq, (unsigned long) block);
+ return lba_28_rw_disk(drive, ar, block);
/* 28-bit CHS : DIE DIE DIE piece of legacy crap!!! */
- return chs_rw_disk(drive, rq, (unsigned long) block);
+ return chs_rw_disk(drive, ar, block);
}
-static task_ioreg_t get_command (ide_drive_t *drive, int cmd)
+static task_ioreg_t get_command (ide_drive_t *drive, struct request *rq)
{
int lba48bit = (drive->id->cfs_enable_2 & 0x0400) ? 1 : 0;
+ int multi_count = drive->mult_count;
+ int cmd = rq_data_dir(rq);
+
+ if (rq->nr_sectors % multi_count)
+ multi_count = 0;
#if 1
lba48bit = drive->addressing;
@@ -172,14 +190,15 @@
return WIN_NOP;
}
-static ide_startstop_t chs_rw_disk (ide_drive_t *drive, struct request *rq, unsigned long block)
+static ide_startstop_t chs_rw_disk (ide_drive_t *drive, ata_request_t *ar, sector_t block)
{
struct hd_drive_task_hdr taskfile;
struct hd_drive_hob_hdr hobfile;
- ide_task_t args;
+ ide_task_t *args = &ar->ar_task;
int sectors;
+ struct request *rq = ar->ar_rq;
- task_ioreg_t command = get_command(drive, rq_data_dir(rq));
+ task_ioreg_t command = get_command(drive, rq);
unsigned int track = (block / drive->sect);
unsigned int sect = (block % drive->sect) + 1;
@@ -192,7 +211,7 @@
sectors = rq->nr_sectors;
if (sectors == 256)
sectors = 0;
- if (command == WIN_MULTWRITE_EXT || command == WIN_MULTWRITE) {
+ if (command == WIN_MULTWRITE) {
sectors = drive->mult_count;
if (sectors > rq->current_nr_sectors)
sectors = rq->current_nr_sectors;
@@ -215,33 +234,33 @@
printk("buffer=0x%08lx\n", (unsigned long) rq->buffer);
#endif
- memcpy(args.tfRegister, &taskfile, sizeof(struct hd_drive_task_hdr));
- memcpy(args.hobRegister, &hobfile, sizeof(struct hd_drive_hob_hdr));
- args.command_type = ide_cmd_type_parser(&args);
- args.prehandler = ide_pre_handler_parser(&taskfile, &hobfile);
- args.handler = ide_handler_parser(&taskfile, &hobfile);
- args.posthandler = NULL;
- args.rq = (struct request *) rq;
- args.block = block;
- rq->special = NULL;
- rq->special = (ide_task_t *)&args;
+ memcpy(args->tfRegister, &taskfile, sizeof(struct hd_drive_task_hdr));
+ memcpy(args->hobRegister, &hobfile, sizeof(struct hd_drive_hob_hdr));
+ args->command_type = ide_cmd_type_parser(args);
+ args->prehandler = ide_pre_handler_parser(&taskfile, &hobfile);
+ args->handler = ide_handler_parser(&taskfile, &hobfile);
+ args->posthandler = NULL;
+ args->ar = ar;
+ args->block = block;
+ rq->special = ar;
- return do_rw_taskfile(drive, &args);
+ return do_rw_taskfile(drive, args);
}
-static ide_startstop_t lba_28_rw_disk (ide_drive_t *drive, struct request *rq, unsigned long block)
+static ide_startstop_t lba_28_rw_disk (ide_drive_t *drive, ata_request_t *ar, sector_t block)
{
struct hd_drive_task_hdr taskfile;
struct hd_drive_hob_hdr hobfile;
- ide_task_t args;
+ ide_task_t *args = &ar->ar_task;
int sectors;
+ struct request *rq = ar->ar_rq;
- task_ioreg_t command = get_command(drive, rq_data_dir(rq));
+ task_ioreg_t command = get_command(drive, rq);
sectors = rq->nr_sectors;
if (sectors == 256)
sectors = 0;
- if (command == WIN_MULTWRITE_EXT || command == WIN_MULTWRITE) {
+ if (command == WIN_MULTWRITE) {
sectors = drive->mult_count;
if (sectors > rq->current_nr_sectors)
sectors = rq->current_nr_sectors;
@@ -267,18 +286,17 @@
printk("buffer=0x%08lx\n", (unsigned long) rq->buffer);
#endif
- memcpy(args.tfRegister, &taskfile, sizeof(struct hd_drive_task_hdr));
- memcpy(args.hobRegister, &hobfile, sizeof(struct hd_drive_hob_hdr));
- args.command_type = ide_cmd_type_parser(&args);
- args.prehandler = ide_pre_handler_parser(&taskfile, &hobfile);
- args.handler = ide_handler_parser(&taskfile, &hobfile);
- args.posthandler = NULL;
- args.rq = (struct request *) rq;
- args.block = block;
- rq->special = NULL;
- rq->special = (ide_task_t *)&args;
+ memcpy(args->tfRegister, &taskfile, sizeof(struct hd_drive_task_hdr));
+ memcpy(args->hobRegister, &hobfile, sizeof(struct hd_drive_hob_hdr));
+ args->command_type = ide_cmd_type_parser(args);
+ args->prehandler = ide_pre_handler_parser(&taskfile, &hobfile);
+ args->handler = ide_handler_parser(&taskfile, &hobfile);
+ args->posthandler = NULL;
+ args->ar = ar;
+ args->block = block;
+ rq->special = ar;
- return do_rw_taskfile(drive, &args);
+ return do_rw_taskfile(drive, args);
}
/*
@@ -287,22 +305,23 @@
* 1073741822 == 549756 MB or 48bit addressing fake drive
*/
-static ide_startstop_t lba_48_rw_disk (ide_drive_t *drive, struct request *rq, unsigned long long block)
+static ide_startstop_t lba_48_rw_disk (ide_drive_t *drive, ata_request_t *ar, sector_t block)
{
struct hd_drive_task_hdr taskfile;
struct hd_drive_hob_hdr hobfile;
- ide_task_t args;
+ ide_task_t *args = &ar->ar_task;
int sectors;
+ struct request *rq = ar->ar_rq;
- task_ioreg_t command = get_command(drive, rq_data_dir(rq));
+ task_ioreg_t command = get_command(drive, rq);
memset(&taskfile, 0, sizeof(task_struct_t));
memset(&hobfile, 0, sizeof(hob_struct_t));
sectors = rq->nr_sectors;
- if (sectors == 256)
+ if (sectors == 65536)
sectors = 0;
- if (command == WIN_MULTWRITE_EXT || command == WIN_MULTWRITE) {
+ if (command == WIN_MULTWRITE_EXT) {
sectors = drive->mult_count;
if (sectors > rq->current_nr_sectors)
sectors = rq->current_nr_sectors;
@@ -311,11 +330,6 @@
taskfile.sector_count = sectors;
hobfile.sector_count = sectors >> 8;
- if (rq->nr_sectors == 65536) {
- taskfile.sector_count = 0x00;
- hobfile.sector_count = 0x00;
- }
-
taskfile.sector_number = block; /* low lba */
taskfile.low_cylinder = (block>>=8); /* mid lba */
taskfile.high_cylinder = (block>>=8); /* hi lba */
@@ -336,18 +350,17 @@
printk("buffer=0x%08lx\n", (unsigned long) rq->buffer);
#endif
- memcpy(args.tfRegister, &taskfile, sizeof(struct hd_drive_task_hdr));
- memcpy(args.hobRegister, &hobfile, sizeof(struct hd_drive_hob_hdr));
- args.command_type = ide_cmd_type_parser(&args);
- args.prehandler = ide_pre_handler_parser(&taskfile, &hobfile);
- args.handler = ide_handler_parser(&taskfile, &hobfile);
- args.posthandler = NULL;
- args.rq = (struct request *) rq;
- args.block = block;
- rq->special = NULL;
- rq->special = (ide_task_t *)&args;
+ memcpy(args->tfRegister, &taskfile, sizeof(struct hd_drive_task_hdr));
+ memcpy(args->hobRegister, &hobfile, sizeof(struct hd_drive_hob_hdr));
+ args->command_type = ide_cmd_type_parser(args);
+ args->prehandler = ide_pre_handler_parser(&taskfile, &hobfile);
+ args->handler = ide_handler_parser(&taskfile, &hobfile);
+ args->posthandler = NULL;
+ args->ar = ar;
+ args->block = block;
+ rq->special = ar;
- return do_rw_taskfile(drive, &args);
+ return do_rw_taskfile(drive, args);
}
static int idedisk_open (struct inode *inode, struct file *filp, ide_drive_t *drive)
--
Jens Axboe
On Mon, 21 Jan 2002, Jens Axboe wrote:
>
> I think this is too restrictive, something ala
>
> if (sectors % drive->mult_count)
> command = WIN_WRITE;
As far as I can see, mult_count will always be a power of two, so we might
consider:
if (sectors & (drive->mult_count-1))
--
"Love the dolphins," she advised him. "Write by W.A.S.T.E.."