[PATCH] IDE update for 2.6.7-rc3 [11/12]

[PATCH] IDE update for 2.6.7-rc3 [11/12]

[Bartlomiej Zolnierkiewicz]

[PATCH] ide: kill task_[un]map_rq()

PIO handlers under CONFIG_IDE_TASKFILE_IO=n are never used for bio
based requests (rq->bio is always NULL) so we can use rq->buffer
directly instead of calling ide_[un]map_buffer().

Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@elka.pw.edu.pl>

 linux-2.6.7-rc3-bzolnier/drivers/ide/ide-taskfile.c |   25 ++++----------------
 1 files changed, 5 insertions(+), 20 deletions(-)

diff -puN drivers/ide/ide-taskfile.c~ide_task_rq_mapping drivers/ide/ide-taskfile.c
--- linux-2.6.7-rc3/drivers/ide/ide-taskfile.c~ide_task_rq_mapping	2004-06-11 16:28:38.227656176 +0200
+++ linux-2.6.7-rc3-bzolnier/drivers/ide/ide-taskfile.c	2004-06-11 16:29:20.524226120 +0200
@@ -301,9 +301,6 @@ EXPORT_SYMBOL(task_no_data_intr);
  */
 #ifndef CONFIG_IDE_TASKFILE_IO
 
-#define task_map_rq(rq, flags)		ide_map_buffer((rq), (flags))
-#define task_unmap_rq(rq, buf, flags)	ide_unmap_buffer((rq), (buf), (flags))
-
 /*
  * Handler for command with PIO data-in phase, READ
  */
@@ -313,7 +310,6 @@ ide_startstop_t task_in_intr (ide_drive_
 	ide_hwif_t *hwif	= HWIF(drive);
 	char *pBuf		= NULL;
 	u8 stat;
-	unsigned long flags;
 
 	if (!OK_STAT(stat = hwif->INB(IDE_STATUS_REG),DATA_READY,BAD_R_STAT)) {
 		if (stat & (ERR_STAT|DRQ_STAT)) {
@@ -327,11 +323,10 @@ ide_startstop_t task_in_intr (ide_drive_
 		}
 	}
 
-	pBuf = task_map_rq(rq, &flags);
+	pBuf = rq->buffer + task_rq_offset(rq);
 	DTF("Read: %p, rq->current_nr_sectors: %d, stat: %02x\n",
 		pBuf, (int) rq->current_nr_sectors, stat);
 	taskfile_input_data(drive, pBuf, SECTOR_WORDS);
-	task_unmap_rq(rq, pBuf, &flags);
 
 	/* FIXME: check drive status */
 	if (--rq->current_nr_sectors <= 0)
@@ -358,7 +353,6 @@ ide_startstop_t task_mulin_intr (ide_dri
 	char *pBuf		= NULL;
 	unsigned int msect	= drive->mult_count;
 	unsigned int nsect;
-	unsigned long flags;
 	u8 stat;
 
 	if (!OK_STAT(stat = hwif->INB(IDE_STATUS_REG),DATA_READY,BAD_R_STAT)) {
@@ -375,12 +369,11 @@ ide_startstop_t task_mulin_intr (ide_dri
 		nsect = rq->current_nr_sectors;
 		if (nsect > msect)
 			nsect = msect;
-		pBuf = task_map_rq(rq, &flags);
+		pBuf = rq->buffer + task_rq_offset(rq);
 		DTF("Multiread: %p, nsect: %d, msect: %d, " \
 			" rq->current_nr_sectors: %d\n",
 			pBuf, nsect, msect, rq->current_nr_sectors);
 		taskfile_input_data(drive, pBuf, nsect * SECTOR_WORDS);
-		task_unmap_rq(rq, pBuf, &flags);
 		rq->errors = 0;
 		rq->current_nr_sectors -= nsect;
 		msect -= nsect;
@@ -404,8 +397,6 @@ EXPORT_SYMBOL(task_mulin_intr);
  */
 ide_startstop_t pre_task_out_intr (ide_drive_t *drive, struct request *rq)
 {
-	char *pBuf		= NULL;
-	unsigned long flags;
 	ide_startstop_t startstop;
 
 	if (ide_wait_stat(&startstop, drive, DATA_READY,
@@ -416,10 +407,8 @@ ide_startstop_t pre_task_out_intr (ide_d
 		return startstop;
 	}
 	/* For Write_sectors we need to stuff the first sector */
-	pBuf = task_map_rq(rq, &flags);
-	taskfile_output_data(drive, pBuf, SECTOR_WORDS);
+	taskfile_output_data(drive, rq->buffer + task_rq_offset(rq), SECTOR_WORDS);
 	rq->current_nr_sectors--;
-	task_unmap_rq(rq, pBuf, &flags);
 	return ide_started;
 }
 
@@ -435,7 +424,6 @@ ide_startstop_t task_out_intr (ide_drive
 	ide_hwif_t *hwif	= HWIF(drive);
 	struct request *rq	= HWGROUP(drive)->rq;
 	char *pBuf		= NULL;
-	unsigned long flags;
 	u8 stat;
 
 	if (!OK_STAT(stat = hwif->INB(IDE_STATUS_REG), DRIVE_READY, drive->bad_wstat)) {
@@ -450,11 +438,10 @@ ide_startstop_t task_out_intr (ide_drive
 			return ide_stopped;
 	if ((rq->current_nr_sectors==1) ^ (stat & DRQ_STAT)) {
 		rq = HWGROUP(drive)->rq;
-		pBuf = task_map_rq(rq, &flags);
+		pBuf = rq->buffer + task_rq_offset(rq);
 		DTF("write: %p, rq->current_nr_sectors: %d\n",
 			pBuf, (int) rq->current_nr_sectors);
 		taskfile_output_data(drive, pBuf, SECTOR_WORDS);
-		task_unmap_rq(rq, pBuf, &flags);
 		rq->errors = 0;
 		rq->current_nr_sectors--;
 	}
@@ -507,7 +494,6 @@ ide_startstop_t task_mulout_intr (ide_dr
 	char *pBuf			= NULL;
 	unsigned int msect		= drive->mult_count;
 	unsigned int nsect;
-	unsigned long flags;
 
 	if (!OK_STAT(stat, DATA_READY, BAD_R_STAT) || !rq->current_nr_sectors) {
 		if (stat & (ERR_STAT|DRQ_STAT)) {
@@ -536,13 +522,12 @@ ide_startstop_t task_mulout_intr (ide_dr
 		nsect = rq->current_nr_sectors;
 		if (nsect > msect)
 			nsect = msect;
-		pBuf = task_map_rq(rq, &flags);
+		pBuf = rq->buffer + task_rq_offset(rq);
 		DTF("Multiwrite: %p, nsect: %d, msect: %d, " \
 			"rq->current_nr_sectors: %ld\n",
 			pBuf, nsect, msect, rq->current_nr_sectors);
 		msect -= nsect;
 		taskfile_output_data(drive, pBuf, nsect * SECTOR_WORDS);
-		task_unmap_rq(rq, pBuf, &flags);
 		rq->current_nr_sectors -= nsect;
 
 		/* FIXME: check drive status */

_

Re: [PATCH] IDE update for 2.6.7-rc3 [11/12]

[Jens Axboe]

On Fri, Jun 11 2004, Bartlomiej Zolnierkiewicz wrote:
> 
> [PATCH] ide: kill task_[un]map_rq()
> 
> PIO handlers under CONFIG_IDE_TASKFILE_IO=n are never used for bio
> based requests (rq->bio is always NULL) so we can use rq->buffer
> directly instead of calling ide_[un]map_buffer().

Not so sure if it's ever used for something requiring performance, and
even if it isn't then it may still be worth it to keep the mapping and
instead fix the task setup to map in user data with blk_rq_map_user() by
fixing up ide_taskfile_ioctl().

It would make HDIO_DRIVE_TASKFILE a whole lot nicer.

-- 
Jens Axboe

Re: [PATCH] IDE update for 2.6.7-rc3 [11/12]

[Jens Axboe]

On Fri, Jun 11 2004, Bartlomiej Zolnierkiewicz wrote:
> On Friday 11 of June 2004 19:01, Jens Axboe wrote:
> > On Fri, Jun 11 2004, Bartlomiej Zolnierkiewicz wrote:
> > > [PATCH] ide: kill task_[un]map_rq()
> > >
> > > PIO handlers under CONFIG_IDE_TASKFILE_IO=n are never used for bio
> > > based requests (rq->bio is always NULL) so we can use rq->buffer
> > > directly instead of calling ide_[un]map_buffer().
> >
> > Not so sure if it's ever used for something requiring performance, and
> > even if it isn't then it may still be worth it to keep the mapping and
> > instead fix the task setup to map in user data with blk_rq_map_user() by
> > fixing up ide_taskfile_ioctl().
> 
> I agree about blk_rq_map_user() (I even did it once around 2.5.60-70).
> However I think that we are better off (slowly) killing old taskfile
> handlers completely (this patch is for old taskfile handlers only)
> and using CONFIG_IDE_TASKFILE_IO versions (which know about rq->bio).

Ah these are the old handlers. Then I fully agree, nuk the mappings.

-- 
Jens Axboe

Re: [PATCH] IDE update for 2.6.7-rc3 [11/12]

[Bartlomiej Zolnierkiewicz]

On Friday 11 of June 2004 19:01, Jens Axboe wrote:
> On Fri, Jun 11 2004, Bartlomiej Zolnierkiewicz wrote:
> > [PATCH] ide: kill task_[un]map_rq()
> >
> > PIO handlers under CONFIG_IDE_TASKFILE_IO=n are never used for bio
> > based requests (rq->bio is always NULL) so we can use rq->buffer
> > directly instead of calling ide_[un]map_buffer().
>
> Not so sure if it's ever used for something requiring performance, and
> even if it isn't then it may still be worth it to keep the mapping and
> instead fix the task setup to map in user data with blk_rq_map_user() by
> fixing up ide_taskfile_ioctl().

I agree about blk_rq_map_user() (I even did it once around 2.5.60-70).
However I think that we are better off (slowly) killing old taskfile
handlers completely (this patch is for old taskfile handlers only)
and using CONFIG_IDE_TASKFILE_IO versions (which know about rq->bio).

> It would make HDIO_DRIVE_TASKFILE a whole lot nicer.