[PATCH 00/24] ide-tape: remove pipelined mode operation

[PATCH 00/24] ide-tape: remove pipelined mode operation

[Borislav Petkov]

Hi Bart,

here's the 1st draft of the pipeline removal series. As the diffstat below openly
states it, a lot of code got removed - even more than the cleanup series we did
earlier. There are several issues that we need to address concerning these
patches:

1. only compile-tested since i don't have the hardware, i.e. longer -mm brewing is
advisable the least.

2. I have left the tape->merge_stage buffer structure along with its
alloc/free functions intact for now, for simplicity. The next step would be
to go and carefully audit the code and then remove that last piece
too and use allocations on the stack instead. I guess we still expect
Jens's response on whether blk_{get,put}_request is the way to go here.

Jens?

3. After applying those we're moving into the direction of a lightweight
ide-tape driver so the stanza for its removal in
Documentation/feature-removal-schedule.txt becomes incorrect.

4. ... (i'm sure you're gonna have some more :))

 Documentation/ide/ide-tape.txt |  211 +++------
 drivers/ide/ide-tape.c         | 1032 +++-------------------------------------
 2 files changed, 134 insertions(+), 1109 deletions(-)

[PATCH 01/24] ide-tape: remove idetape_pipeline_active()

[Borislav Petkov]

This function was simply a wrapper for a test_bit() macro so remove it and
use the macro instead.

Signed-off-by: Borislav Petkov <petkovbb@gmail.com>
---
 drivers/ide/ide-tape.c |   29 +++++++++++------------------
 1 files changed, 11 insertions(+), 18 deletions(-)

diff --git a/drivers/ide/ide-tape.c b/drivers/ide/ide-tape.c
index 3f9fbd8..792c76e 100644
--- a/drivers/ide/ide-tape.c
+++ b/drivers/ide/ide-tape.c
@@ -1605,14 +1605,6 @@ out:
 }
 
 /* Pipeline related functions */
-static inline int idetape_pipeline_active(idetape_tape_t *tape)
-{
-	int rc1, rc2;
-
-	rc1 = test_bit(IDETAPE_FLAG_PIPELINE_ACTIVE, &tape->flags);
-	rc2 = (tape->active_data_rq != NULL);
-	return rc1;
-}
 
 /*
  * The function below uses __get_free_page to allocate a pipeline stage, along
@@ -2058,7 +2050,7 @@ static int __idetape_discard_read_pipeline(ide_drive_t *drive)
 
 	spin_lock_irqsave(&tape->lock, flags);
 	tape->next_stage = NULL;
-	if (idetape_pipeline_active(tape))
+	if (test_bit(IDETAPE_FLAG_PIPELINE_ACTIVE, &tape->flags))
 		idetape_wait_for_request(drive, tape->active_data_rq);
 	spin_unlock_irqrestore(&tape->lock, flags);
 
@@ -2131,7 +2123,7 @@ static int idetape_queue_rw_tail(ide_drive_t *drive, int cmd, int blocks,
 
 	debug_log(DBG_SENSE, "%s: cmd=%d\n", __func__, cmd);
 
-	if (idetape_pipeline_active(tape)) {
+	if (test_bit(IDETAPE_FLAG_PIPELINE_ACTIVE, &tape->flags)) {
 		printk(KERN_ERR "ide-tape: bug: the pipeline is active in %s\n",
 				__func__);
 		return (0);
@@ -2162,8 +2154,7 @@ static void idetape_plug_pipeline(ide_drive_t *drive)
 
 	if (tape->next_stage == NULL)
 		return;
-	if (!idetape_pipeline_active(tape)) {
-		set_bit(IDETAPE_FLAG_PIPELINE_ACTIVE, &tape->flags);
+	if (!test_and_set_bit(IDETAPE_FLAG_PIPELINE_ACTIVE, &tape->flags)) {
 		idetape_activate_next_stage(drive);
 		(void) ide_do_drive_cmd(drive, tape->active_data_rq, ide_end);
 	}
@@ -2242,13 +2233,14 @@ static int idetape_add_chrdev_write_request(ide_drive_t *drive, int blocks)
 	/* Attempt to allocate a new stage. Beware possible race conditions. */
 	while ((new_stage = idetape_kmalloc_stage(tape)) == NULL) {
 		spin_lock_irqsave(&tape->lock, flags);
-		if (idetape_pipeline_active(tape)) {
+		if (test_bit(IDETAPE_FLAG_PIPELINE_ACTIVE, &tape->flags)) {
 			idetape_wait_for_request(drive, tape->active_data_rq);
 			spin_unlock_irqrestore(&tape->lock, flags);
 		} else {
 			spin_unlock_irqrestore(&tape->lock, flags);
 			idetape_plug_pipeline(drive);
-			if (idetape_pipeline_active(tape))
+			if (test_bit(IDETAPE_FLAG_PIPELINE_ACTIVE,
+					&tape->flags))
 				continue;
 			/*
 			 * The machine is short on memory. Fallback to non-
@@ -2277,7 +2269,7 @@ static int idetape_add_chrdev_write_request(ide_drive_t *drive, int blocks)
 	 * starting to service requests, so that we will be able to keep up with
 	 * the higher speeds of the tape.
 	 */
-	if (!idetape_pipeline_active(tape)) {
+	if (!test_bit(IDETAPE_FLAG_PIPELINE_ACTIVE, &tape->flags)) {
 		if (tape->nr_stages >= tape->max_stages * 9 / 10 ||
 			tape->nr_stages >= tape->max_stages -
 			tape->uncontrolled_pipeline_head_speed * 3 * 1024 /
@@ -2304,10 +2296,11 @@ static void idetape_wait_for_pipeline(ide_drive_t *drive)
 	idetape_tape_t *tape = drive->driver_data;
 	unsigned long flags;
 
-	while (tape->next_stage || idetape_pipeline_active(tape)) {
+	while (tape->next_stage || test_bit(IDETAPE_FLAG_PIPELINE_ACTIVE,
+						&tape->flags)) {
 		idetape_plug_pipeline(drive);
 		spin_lock_irqsave(&tape->lock, flags);
-		if (idetape_pipeline_active(tape))
+		if (test_bit(IDETAPE_FLAG_PIPELINE_ACTIVE, &tape->flags))
 			idetape_wait_for_request(drive, tape->active_data_rq);
 		spin_unlock_irqrestore(&tape->lock, flags);
 	}
@@ -2464,7 +2457,7 @@ static int idetape_init_read(ide_drive_t *drive, int max_stages)
 			new_stage = idetape_kmalloc_stage(tape);
 		}
 	}
-	if (!idetape_pipeline_active(tape)) {
+	if (!test_bit(IDETAPE_FLAG_PIPELINE_ACTIVE, &tape->flags)) {
 		if (tape->nr_pending_stages >= 3 * max_stages / 4) {
 			tape->measure_insert_time = 1;
 			tape->insert_time = jiffies;
-- 
1.5.4.1

Re: [PATCH 01/24] ide-tape: remove idetape_pipeline_active()

[Bartlomiej Zolnierkiewicz]

On Saturday 01 March 2008, Borislav Petkov wrote:
> This function was simply a wrapper for a test_bit() macro so remove it and
> use the macro instead.
> 
> Signed-off-by: Borislav Petkov <petkovbb@gmail.com>

applied

[PATCH 02/24] ide-tape: remove pipeline-specific code from idetape_add_chrdev_write_request

[Borislav Petkov]

Instead of plugging the request into the pipeline, queue it straight on the
device's request queue through idetape_queue_rw_tail().

Signed-off-by: Borislav Petkov <petkovbb@gmail.com>
---
 drivers/ide/ide-tape.c |   81 ++---------------------------------------------
 1 files changed, 4 insertions(+), 77 deletions(-)

diff --git a/drivers/ide/ide-tape.c b/drivers/ide/ide-tape.c
index 792c76e..abf3efa 100644
--- a/drivers/ide/ide-tape.c
+++ b/drivers/ide/ide-tape.c
@@ -2123,12 +2123,6 @@ static int idetape_queue_rw_tail(ide_drive_t *drive, int cmd, int blocks,
 
 	debug_log(DBG_SENSE, "%s: cmd=%d\n", __func__, cmd);
 
-	if (test_bit(IDETAPE_FLAG_PIPELINE_ACTIVE, &tape->flags)) {
-		printk(KERN_ERR "ide-tape: bug: the pipeline is active in %s\n",
-				__func__);
-		return (0);
-	}
-
 	idetape_init_rq(&rq, cmd);
 	rq.rq_disk = tape->disk;
 	rq.special = (void *)bh;
@@ -2140,10 +2134,9 @@ static int idetape_queue_rw_tail(ide_drive_t *drive, int cmd, int blocks,
 	if ((cmd & (REQ_IDETAPE_READ | REQ_IDETAPE_WRITE)) == 0)
 		return 0;
 
-	if (tape->merge_stage)
-		idetape_init_merge_stage(tape);
 	if (rq.errors == IDETAPE_ERROR_GENERAL)
 		return -EIO;
+
 	return (tape->blk_size * (blocks-rq.current_nr_sectors));
 }
 
@@ -2210,81 +2203,15 @@ static void idetape_wait_first_stage(ide_drive_t *drive)
 	spin_unlock_irqrestore(&tape->lock, flags);
 }
 
-/*
- * Try to add a character device originated write request to our pipeline. In
- * case we don't succeed, we revert to non-pipelined operation mode for this
- * request. In order to accomplish that, we
- *
- * 1. Try to allocate a new pipeline stage.
- * 2. If we can't, wait for more and more requests to be serviced and try again
- * each time.
- * 3. If we still can't allocate a stage, fallback to non-pipelined operation
- * mode for this request.
- */
+/* Queue up a character device originated write request. */
 static int idetape_add_chrdev_write_request(ide_drive_t *drive, int blocks)
 {
 	idetape_tape_t *tape = drive->driver_data;
-	idetape_stage_t *new_stage;
-	unsigned long flags;
-	struct request *rq;
 
 	debug_log(DBG_CHRDEV, "Enter %s\n", __func__);
 
-	/* Attempt to allocate a new stage. Beware possible race conditions. */
-	while ((new_stage = idetape_kmalloc_stage(tape)) == NULL) {
-		spin_lock_irqsave(&tape->lock, flags);
-		if (test_bit(IDETAPE_FLAG_PIPELINE_ACTIVE, &tape->flags)) {
-			idetape_wait_for_request(drive, tape->active_data_rq);
-			spin_unlock_irqrestore(&tape->lock, flags);
-		} else {
-			spin_unlock_irqrestore(&tape->lock, flags);
-			idetape_plug_pipeline(drive);
-			if (test_bit(IDETAPE_FLAG_PIPELINE_ACTIVE,
-					&tape->flags))
-				continue;
-			/*
-			 * The machine is short on memory. Fallback to non-
-			 * pipelined operation mode for this request.
-			 */
-			return idetape_queue_rw_tail(drive, REQ_IDETAPE_WRITE,
-						blocks, tape->merge_stage->bh);
-		}
-	}
-	rq = &new_stage->rq;
-	idetape_init_rq(rq, REQ_IDETAPE_WRITE);
-	/* Doesn't actually matter - We always assume sequential access */
-	rq->sector = tape->first_frame;
-	rq->current_nr_sectors = blocks;
-	rq->nr_sectors = blocks;
-
-	idetape_switch_buffers(tape, new_stage);
-	idetape_add_stage_tail(drive, new_stage);
-	tape->pipeline_head++;
-	idetape_calculate_speeds(drive);
-
-	/*
-	 * Estimate whether the tape has stopped writing by checking if our
-	 * write pipeline is currently empty. If we are not writing anymore,
-	 * wait for the pipeline to be almost completely full (90%) before
-	 * starting to service requests, so that we will be able to keep up with
-	 * the higher speeds of the tape.
-	 */
-	if (!test_bit(IDETAPE_FLAG_PIPELINE_ACTIVE, &tape->flags)) {
-		if (tape->nr_stages >= tape->max_stages * 9 / 10 ||
-			tape->nr_stages >= tape->max_stages -
-			tape->uncontrolled_pipeline_head_speed * 3 * 1024 /
-			tape->blk_size) {
-			tape->measure_insert_time = 1;
-			tape->insert_time = jiffies;
-			tape->insert_size = 0;
-			tape->insert_speed = 0;
-			idetape_plug_pipeline(drive);
-		}
-	}
-	if (test_and_clear_bit(IDETAPE_FLAG_PIPELINE_ERR, &tape->flags))
-		/* Return a deferred error */
-		return -EIO;
-	return blocks;
+	return idetape_queue_rw_tail(drive, REQ_IDETAPE_WRITE, blocks,
+					tape->merge_stage->bh);
 }
 
 /*
-- 
1.5.4.1

Re: [PATCH 02/24] ide-tape: remove pipeline-specific code from idetape_add_chrdev_write_request

[Bartlomiej Zolnierkiewicz]

On Saturday 01 March 2008, Borislav Petkov wrote:
> Instead of plugging the request into the pipeline, queue it straight on the
> device's request queue through idetape_queue_rw_tail().
> 
> Signed-off-by: Borislav Petkov <petkovbb@gmail.com>
> ---
>  drivers/ide/ide-tape.c |   81 ++---------------------------------------------
>  1 files changed, 4 insertions(+), 77 deletions(-)
> 
> diff --git a/drivers/ide/ide-tape.c b/drivers/ide/ide-tape.c
> index 792c76e..abf3efa 100644
> --- a/drivers/ide/ide-tape.c
> +++ b/drivers/ide/ide-tape.c
> @@ -2123,12 +2123,6 @@ static int idetape_queue_rw_tail(ide_drive_t *drive, int cmd, int blocks,
>  
>  	debug_log(DBG_SENSE, "%s: cmd=%d\n", __func__, cmd);
>  
> -	if (test_bit(IDETAPE_FLAG_PIPELINE_ACTIVE, &tape->flags)) {
> -		printk(KERN_ERR "ide-tape: bug: the pipeline is active in %s\n",
> -				__func__);
> -		return (0);
> -	}
> -
>  	idetape_init_rq(&rq, cmd);
>  	rq.rq_disk = tape->disk;
>  	rq.special = (void *)bh;
> @@ -2140,10 +2134,9 @@ static int idetape_queue_rw_tail(ide_drive_t *drive, int cmd, int blocks,
>  	if ((cmd & (REQ_IDETAPE_READ | REQ_IDETAPE_WRITE)) == 0)
>  		return 0;
>  
> -	if (tape->merge_stage)
> -		idetape_init_merge_stage(tape);
>  	if (rq.errors == IDETAPE_ERROR_GENERAL)
>  		return -EIO;
> +
>  	return (tape->blk_size * (blocks-rq.current_nr_sectors));
>  }

These two changes to idetape_queue_rw_tail() don't look correct
as the pipeline mode is still used by read requests.

> @@ -2210,81 +2203,15 @@ static void idetape_wait_first_stage(ide_drive_t *drive)
>  	spin_unlock_irqrestore(&tape->lock, flags);
>  }
>  
> -/*
> - * Try to add a character device originated write request to our pipeline. In
> - * case we don't succeed, we revert to non-pipelined operation mode for this
> - * request. In order to accomplish that, we
> - *
> - * 1. Try to allocate a new pipeline stage.
> - * 2. If we can't, wait for more and more requests to be serviced and try again
> - * each time.
> - * 3. If we still can't allocate a stage, fallback to non-pipelined operation
> - * mode for this request.
> - */
> +/* Queue up a character device originated write request. */
>  static int idetape_add_chrdev_write_request(ide_drive_t *drive, int blocks)
>  {
>  	idetape_tape_t *tape = drive->driver_data;
> -	idetape_stage_t *new_stage;
> -	unsigned long flags;
> -	struct request *rq;
>  
>  	debug_log(DBG_CHRDEV, "Enter %s\n", __func__);
>  
> -	/* Attempt to allocate a new stage. Beware possible race conditions. */
> -	while ((new_stage = idetape_kmalloc_stage(tape)) == NULL) {
> -		spin_lock_irqsave(&tape->lock, flags);
> -		if (test_bit(IDETAPE_FLAG_PIPELINE_ACTIVE, &tape->flags)) {
> -			idetape_wait_for_request(drive, tape->active_data_rq);
> -			spin_unlock_irqrestore(&tape->lock, flags);
> -		} else {
> -			spin_unlock_irqrestore(&tape->lock, flags);
> -			idetape_plug_pipeline(drive);
> -			if (test_bit(IDETAPE_FLAG_PIPELINE_ACTIVE,
> -					&tape->flags))
> -				continue;

Can all the above code be safely removed (are you sure that there are no
hidden interactions)?  Even if so I would prefer that it is left intact by
this patch to ease the review.

The main change to idetape_add_chrdev_write_request is fine.

Re: [PATCH 02/24] ide-tape: remove pipeline-specific code from idetape_add_chrdev_write_request

[Borislav Petkov]

On Sun, Mar 02, 2008 at 07:33:05PM +0100, Bartlomiej Zolnierkiewicz wrote:
> On Saturday 01 March 2008, Borislav Petkov wrote:
> > Instead of plugging the request into the pipeline, queue it straight on the
> > device's request queue through idetape_queue_rw_tail().
> > 
> > Signed-off-by: Borislav Petkov <petkovbb@gmail.com>
> > ---
> >  drivers/ide/ide-tape.c |   81 ++---------------------------------------------
> >  1 files changed, 4 insertions(+), 77 deletions(-)
> > 
> > diff --git a/drivers/ide/ide-tape.c b/drivers/ide/ide-tape.c
> > index 792c76e..abf3efa 100644
> > --- a/drivers/ide/ide-tape.c
> > +++ b/drivers/ide/ide-tape.c
> > @@ -2123,12 +2123,6 @@ static int idetape_queue_rw_tail(ide_drive_t *drive, int cmd, int blocks,
> >  
> >  	debug_log(DBG_SENSE, "%s: cmd=%d\n", __func__, cmd);
> >  
> > -	if (test_bit(IDETAPE_FLAG_PIPELINE_ACTIVE, &tape->flags)) {
> > -		printk(KERN_ERR "ide-tape: bug: the pipeline is active in %s\n",
> > -				__func__);
> > -		return (0);
> > -	}
> > -
> >  	idetape_init_rq(&rq, cmd);
> >  	rq.rq_disk = tape->disk;
> >  	rq.special = (void *)bh;
> > @@ -2140,10 +2134,9 @@ static int idetape_queue_rw_tail(ide_drive_t *drive, int cmd, int blocks,
> >  	if ((cmd & (REQ_IDETAPE_READ | REQ_IDETAPE_WRITE)) == 0)
> >  		return 0;
> >  
> > -	if (tape->merge_stage)
> > -		idetape_init_merge_stage(tape);
> >  	if (rq.errors == IDETAPE_ERROR_GENERAL)
> >  		return -EIO;
> > +
> >  	return (tape->blk_size * (blocks-rq.current_nr_sectors));
> >  }
> 
> These two changes to idetape_queue_rw_tail() don't look correct
> as the pipeline mode is still used by read requests.

Wrt first hunk read rq pipeline functionality is removed in the following
patch. Would it then be better to merge the two patches? Actually, do we need
to keep the driver functional in between the patches of those series for
the purposes of git bisection or only compile-testing it is enough? Cause
after applying the whole series you get pipelined mode ripped out anyway and
intermittent states with partially functional pipeline are of no importance, no?

Wrt the second one you're right, this one should stay in for now until
tape->merge_stage has been removed.

> > @@ -2210,81 +2203,15 @@ static void idetape_wait_first_stage(ide_drive_t *drive)
> >  	spin_unlock_irqrestore(&tape->lock, flags);
> >  }
> >  
> > -/*
> > - * Try to add a character device originated write request to our pipeline. In
> > - * case we don't succeed, we revert to non-pipelined operation mode for this
> > - * request. In order to accomplish that, we
> > - *
> > - * 1. Try to allocate a new pipeline stage.
> > - * 2. If we can't, wait for more and more requests to be serviced and try again
> > - * each time.
> > - * 3. If we still can't allocate a stage, fallback to non-pipelined operation
> > - * mode for this request.
> > - */
> > +/* Queue up a character device originated write request. */
> >  static int idetape_add_chrdev_write_request(ide_drive_t *drive, int blocks)
> >  {
> >  	idetape_tape_t *tape = drive->driver_data;
> > -	idetape_stage_t *new_stage;
> > -	unsigned long flags;
> > -	struct request *rq;
> >  
> >  	debug_log(DBG_CHRDEV, "Enter %s\n", __func__);
> >  
> > -	/* Attempt to allocate a new stage. Beware possible race conditions. */
> > -	while ((new_stage = idetape_kmalloc_stage(tape)) == NULL) {
> > -		spin_lock_irqsave(&tape->lock, flags);
> > -		if (test_bit(IDETAPE_FLAG_PIPELINE_ACTIVE, &tape->flags)) {
> > -			idetape_wait_for_request(drive, tape->active_data_rq);
> > -			spin_unlock_irqrestore(&tape->lock, flags);
> > -		} else {
> > -			spin_unlock_irqrestore(&tape->lock, flags);
> > -			idetape_plug_pipeline(drive);
> > -			if (test_bit(IDETAPE_FLAG_PIPELINE_ACTIVE,
> > -					&tape->flags))
> > -				continue;
> 
> Can all the above code be safely removed (are you sure that there are no
> hidden interactions)?  Even if so I would prefer that it is left intact by
> this patch to ease the review.

This code does exactly what the comment above explains: it tries to free
the pipeline for yet another request by plugging it with the already queued
ones and if it can't do so it simply queues the request in non-pipelined
mode. What the patch does is remove the plugging and waiting. If we
leave this intact we'll be missing the point of the whole exercise.

-- 
Regards/Gruß,
    Boris.

Re: [PATCH 02/24] ide-tape: remove pipeline-specific code from idetape_add_chrdev_write_request

[Bartlomiej Zolnierkiewicz]

On Sunday 02 March 2008, Borislav Petkov wrote:
> On Sun, Mar 02, 2008 at 07:33:05PM +0100, Bartlomiej Zolnierkiewicz wrote:
> > On Saturday 01 March 2008, Borislav Petkov wrote:
> > > Instead of plugging the request into the pipeline, queue it straight on the
> > > device's request queue through idetape_queue_rw_tail().
> > > 
> > > Signed-off-by: Borislav Petkov <petkovbb@gmail.com>
> > > ---
> > >  drivers/ide/ide-tape.c |   81 ++---------------------------------------------
> > >  1 files changed, 4 insertions(+), 77 deletions(-)
> > > 
> > > diff --git a/drivers/ide/ide-tape.c b/drivers/ide/ide-tape.c
> > > index 792c76e..abf3efa 100644
> > > --- a/drivers/ide/ide-tape.c
> > > +++ b/drivers/ide/ide-tape.c
> > > @@ -2123,12 +2123,6 @@ static int idetape_queue_rw_tail(ide_drive_t *drive, int cmd, int blocks,
> > >  
> > >  	debug_log(DBG_SENSE, "%s: cmd=%d\n", __func__, cmd);
> > >  
> > > -	if (test_bit(IDETAPE_FLAG_PIPELINE_ACTIVE, &tape->flags)) {
> > > -		printk(KERN_ERR "ide-tape: bug: the pipeline is active in %s\n",
> > > -				__func__);
> > > -		return (0);
> > > -	}
> > > -
> > >  	idetape_init_rq(&rq, cmd);
> > >  	rq.rq_disk = tape->disk;
> > >  	rq.special = (void *)bh;
> > > @@ -2140,10 +2134,9 @@ static int idetape_queue_rw_tail(ide_drive_t *drive, int cmd, int blocks,
> > >  	if ((cmd & (REQ_IDETAPE_READ | REQ_IDETAPE_WRITE)) == 0)
> > >  		return 0;
> > >  
> > > -	if (tape->merge_stage)
> > > -		idetape_init_merge_stage(tape);
> > >  	if (rq.errors == IDETAPE_ERROR_GENERAL)
> > >  		return -EIO;
> > > +
> > >  	return (tape->blk_size * (blocks-rq.current_nr_sectors));
> > >  }
> > 
> > These two changes to idetape_queue_rw_tail() don't look correct
> > as the pipeline mode is still used by read requests.
> 
> Wrt first hunk read rq pipeline functionality is removed in the following
> patch. Would it then be better to merge the two patches? Actually, do we need

Merging patches together would cause increased complexity.

The best solution would be to move this hunk to the patch which removes
IDETAPE_FLAG_PIPELINE_ACTIVE flag.

> to keep the driver functional in between the patches of those series for
> the purposes of git bisection or only compile-testing it is enough? Cause

We want to keep the driver functional in between the patches, especially
given that it shouldn't be much more difficult to do so.

> after applying the whole series you get pipelined mode ripped out anyway and
> intermittent states with partially functional pipeline are of no importance, no?

We always want fully bisectable patches:

- if the patch series accidentaly completely breaks the code we want to be
  able narrow it down to the guilty change

- if the patch series causes some regressions (ie. worse performance) we
  also want to see which exact change caused it

[ Nothing changes here and removal of pipeline functionality can't be an
  excuse for not sticking to the standard procedure. ]

> Wrt the second one you're right, this one should stay in for now until
> tape->merge_stage has been removed.
> 
> > > @@ -2210,81 +2203,15 @@ static void idetape_wait_first_stage(ide_drive_t *drive)
> > >  	spin_unlock_irqrestore(&tape->lock, flags);
> > >  }
> > >  
> > > -/*
> > > - * Try to add a character device originated write request to our pipeline. In
> > > - * case we don't succeed, we revert to non-pipelined operation mode for this
> > > - * request. In order to accomplish that, we
> > > - *
> > > - * 1. Try to allocate a new pipeline stage.
> > > - * 2. If we can't, wait for more and more requests to be serviced and try again
> > > - * each time.
> > > - * 3. If we still can't allocate a stage, fallback to non-pipelined operation
> > > - * mode for this request.
> > > - */
> > > +/* Queue up a character device originated write request. */
> > >  static int idetape_add_chrdev_write_request(ide_drive_t *drive, int blocks)
> > >  {
> > >  	idetape_tape_t *tape = drive->driver_data;
> > > -	idetape_stage_t *new_stage;
> > > -	unsigned long flags;
> > > -	struct request *rq;
> > >  
> > >  	debug_log(DBG_CHRDEV, "Enter %s\n", __func__);
> > >  
> > > -	/* Attempt to allocate a new stage. Beware possible race conditions. */
> > > -	while ((new_stage = idetape_kmalloc_stage(tape)) == NULL) {
> > > -		spin_lock_irqsave(&tape->lock, flags);
> > > -		if (test_bit(IDETAPE_FLAG_PIPELINE_ACTIVE, &tape->flags)) {
> > > -			idetape_wait_for_request(drive, tape->active_data_rq);
> > > -			spin_unlock_irqrestore(&tape->lock, flags);
> > > -		} else {
> > > -			spin_unlock_irqrestore(&tape->lock, flags);
> > > -			idetape_plug_pipeline(drive);
> > > -			if (test_bit(IDETAPE_FLAG_PIPELINE_ACTIVE,
> > > -					&tape->flags))
> > > -				continue;
> > 
> > Can all the above code be safely removed (are you sure that there are no
> > hidden interactions)?  Even if so I would prefer that it is left intact by
> > this patch to ease the review.
> 
> This code does exactly what the comment above explains: it tries to free
> the pipeline for yet another request by plugging it with the already queued
> ones and if it can't do so it simply queues the request in non-pipelined
> mode. What the patch does is remove the plugging and waiting. If we
> leave this intact we'll be missing the point of the whole exercise.

idetape_empty_write_pipeline() which is called for _reads_ calls
idetape_add_chrdev_write_request() - could you tell if there are none
hidden interactions caused by not waiting for the active request to
finish?

[ I don't know and it is really not obvious from looking at ide-tape.c
  (it is quite complicated code) - that is why I want to play it safe. ]

Thanks,
Bart

Re: [PATCH 02/24] ide-tape: remove pipeline-specific code from idetape_add_chrdev_write_request

[Borislav Petkov]

On Mon, Mar 03, 2008 at 12:16:29AM +0100, Bartlomiej Zolnierkiewicz wrote:
> 
> On Sunday 02 March 2008, Borislav Petkov wrote:
> > On Sun, Mar 02, 2008 at 07:33:05PM +0100, Bartlomiej Zolnierkiewicz wrote:
> > > On Saturday 01 March 2008, Borislav Petkov wrote:
> > > > Instead of plugging the request into the pipeline, queue it straight on the
> > > > device's request queue through idetape_queue_rw_tail().
> > > > 
> > > > Signed-off-by: Borislav Petkov <petkovbb@gmail.com>
> > > > ---
> > > >  drivers/ide/ide-tape.c |   81 ++---------------------------------------------
> > > >  1 files changed, 4 insertions(+), 77 deletions(-)
> > > > 
> > > > diff --git a/drivers/ide/ide-tape.c b/drivers/ide/ide-tape.c
> > > > index 792c76e..abf3efa 100644
> > > > --- a/drivers/ide/ide-tape.c
> > > > +++ b/drivers/ide/ide-tape.c
> > > > @@ -2123,12 +2123,6 @@ static int idetape_queue_rw_tail(ide_drive_t *drive, int cmd, int blocks,
> > > >  
> > > >  	debug_log(DBG_SENSE, "%s: cmd=%d\n", __func__, cmd);
> > > >  
> > > > -	if (test_bit(IDETAPE_FLAG_PIPELINE_ACTIVE, &tape->flags)) {
> > > > -		printk(KERN_ERR "ide-tape: bug: the pipeline is active in %s\n",
> > > > -				__func__);
> > > > -		return (0);
> > > > -	}
> > > > -
> > > >  	idetape_init_rq(&rq, cmd);
> > > >  	rq.rq_disk = tape->disk;
> > > >  	rq.special = (void *)bh;
> > > > @@ -2140,10 +2134,9 @@ static int idetape_queue_rw_tail(ide_drive_t *drive, int cmd, int blocks,
> > > >  	if ((cmd & (REQ_IDETAPE_READ | REQ_IDETAPE_WRITE)) == 0)
> > > >  		return 0;
> > > >  
> > > > -	if (tape->merge_stage)
> > > > -		idetape_init_merge_stage(tape);
> > > >  	if (rq.errors == IDETAPE_ERROR_GENERAL)
> > > >  		return -EIO;
> > > > +
> > > >  	return (tape->blk_size * (blocks-rq.current_nr_sectors));
> > > >  }
> > > 
> > > These two changes to idetape_queue_rw_tail() don't look correct
> > > as the pipeline mode is still used by read requests.
> > 
> > Wrt first hunk read rq pipeline functionality is removed in the following
> > patch. Would it then be better to merge the two patches? Actually, do we need
> 
> Merging patches together would cause increased complexity.
> 
> The best solution would be to move this hunk to the patch which removes
> IDETAPE_FLAG_PIPELINE_ACTIVE flag.
> 
> > to keep the driver functional in between the patches of those series for
> > the purposes of git bisection or only compile-testing it is enough? Cause
> 
> We want to keep the driver functional in between the patches, especially
> given that it shouldn't be much more difficult to do so.
> 
> > after applying the whole series you get pipelined mode ripped out anyway and
> > intermittent states with partially functional pipeline are of no importance, no?
> 
> We always want fully bisectable patches:
> 
> - if the patch series accidentaly completely breaks the code we want to be
>   able narrow it down to the guilty change
> 
> - if the patch series causes some regressions (ie. worse performance) we
>   also want to see which exact change caused it
> 
> [ Nothing changes here and removal of pipeline functionality can't be an
>   excuse for not sticking to the standard procedure. ]

Ok this changes the approach vector :). Will redo the patches from this angle
and send them in small b(i|y)tes :) in order to review them easier/faster.

Thanks.
-- 
Regards/Gruß,
    Boris.

Re: [PATCH 02/24] ide-tape: remove pipeline-specific code from idetape_add_chrdev_write_request

[Bartlomiej Zolnierkiewicz]

On Monday 03 March 2008, Borislav Petkov wrote:
> On Mon, Mar 03, 2008 at 12:16:29AM +0100, Bartlomiej Zolnierkiewicz wrote:
> > 
> > On Sunday 02 March 2008, Borislav Petkov wrote:
> > > On Sun, Mar 02, 2008 at 07:33:05PM +0100, Bartlomiej Zolnierkiewicz wrote:
> > > > On Saturday 01 March 2008, Borislav Petkov wrote:
> > > > > Instead of plugging the request into the pipeline, queue it straight on the
> > > > > device's request queue through idetape_queue_rw_tail().
> > > > > 
> > > > > Signed-off-by: Borislav Petkov <petkovbb@gmail.com>
> > > > > ---
> > > > >  drivers/ide/ide-tape.c |   81 ++---------------------------------------------
> > > > >  1 files changed, 4 insertions(+), 77 deletions(-)
> > > > > 
> > > > > diff --git a/drivers/ide/ide-tape.c b/drivers/ide/ide-tape.c
> > > > > index 792c76e..abf3efa 100644
> > > > > --- a/drivers/ide/ide-tape.c
> > > > > +++ b/drivers/ide/ide-tape.c
> > > > > @@ -2123,12 +2123,6 @@ static int idetape_queue_rw_tail(ide_drive_t *drive, int cmd, int blocks,
> > > > >  
> > > > >  	debug_log(DBG_SENSE, "%s: cmd=%d\n", __func__, cmd);
> > > > >  
> > > > > -	if (test_bit(IDETAPE_FLAG_PIPELINE_ACTIVE, &tape->flags)) {
> > > > > -		printk(KERN_ERR "ide-tape: bug: the pipeline is active in %s\n",
> > > > > -				__func__);
> > > > > -		return (0);
> > > > > -	}
> > > > > -
> > > > >  	idetape_init_rq(&rq, cmd);
> > > > >  	rq.rq_disk = tape->disk;
> > > > >  	rq.special = (void *)bh;
> > > > > @@ -2140,10 +2134,9 @@ static int idetape_queue_rw_tail(ide_drive_t *drive, int cmd, int blocks,
> > > > >  	if ((cmd & (REQ_IDETAPE_READ | REQ_IDETAPE_WRITE)) == 0)
> > > > >  		return 0;
> > > > >  
> > > > > -	if (tape->merge_stage)
> > > > > -		idetape_init_merge_stage(tape);
> > > > >  	if (rq.errors == IDETAPE_ERROR_GENERAL)
> > > > >  		return -EIO;
> > > > > +
> > > > >  	return (tape->blk_size * (blocks-rq.current_nr_sectors));
> > > > >  }
> > > > 
> > > > These two changes to idetape_queue_rw_tail() don't look correct
> > > > as the pipeline mode is still used by read requests.
> > > 
> > > Wrt first hunk read rq pipeline functionality is removed in the following
> > > patch. Would it then be better to merge the two patches? Actually, do we need
> > 
> > Merging patches together would cause increased complexity.
> > 
> > The best solution would be to move this hunk to the patch which removes
> > IDETAPE_FLAG_PIPELINE_ACTIVE flag.
> > 
> > > to keep the driver functional in between the patches of those series for
> > > the purposes of git bisection or only compile-testing it is enough? Cause
> > 
> > We want to keep the driver functional in between the patches, especially
> > given that it shouldn't be much more difficult to do so.
> > 
> > > after applying the whole series you get pipelined mode ripped out anyway and
> > > intermittent states with partially functional pipeline are of no importance, no?
> > 
> > We always want fully bisectable patches:
> > 
> > - if the patch series accidentaly completely breaks the code we want to be
> >   able narrow it down to the guilty change
> > 
> > - if the patch series causes some regressions (ie. worse performance) we
> >   also want to see which exact change caused it
> > 
> > [ Nothing changes here and removal of pipeline functionality can't be an
> >   excuse for not sticking to the standard procedure. ]
> 
> Ok this changes the approach vector :). Will redo the patches from this angle
> and send them in small b(i|y)tes :) in order to review them easier/faster.

Thanks.  I'll be waiting with review/merge for the re-done patch series then.

Bart

[PATCH 03/24] ide-tape: remove pipeline-specific code from idetape_add_chrdev_read_request

[Borislav Petkov]

We fall back to non-pipelined operation through idetape_queue_rw_tail with cmd
set to REQ_IDETAPE_READ. Also, remove idetape_switch_buffers() since it becomes
unused.

Signed-off-by: Borislav Petkov <petkovbb@gmail.com>
---
 drivers/ide/ide-tape.c |   46 +---------------------------------------------
 1 files changed, 1 insertions(+), 45 deletions(-)

diff --git a/drivers/ide/ide-tape.c b/drivers/ide/ide-tape.c
index abf3efa..e919d41 100644
--- a/drivers/ide/ide-tape.c
+++ b/drivers/ide/ide-tape.c
@@ -1776,16 +1776,6 @@ static void idetape_init_merge_stage(idetape_tape_t *tape)
 	}
 }
 
-static void idetape_switch_buffers(idetape_tape_t *tape, idetape_stage_t *stage)
-{
-	struct idetape_bh *tmp;
-
-	tmp = stage->bh;
-	stage->bh = tape->merge_stage->bh;
-	tape->merge_stage->bh = tmp;
-	idetape_init_merge_stage(tape);
-}
-
 /* Add a new stage at the end of the pipeline. */
 static void idetape_add_stage_tail(ide_drive_t *drive, idetape_stage_t *stage)
 {
@@ -2403,9 +2393,6 @@ static int idetape_init_read(ide_drive_t *drive, int max_stages)
 static int idetape_add_chrdev_read_request(ide_drive_t *drive, int blocks)
 {
 	idetape_tape_t *tape = drive->driver_data;
-	unsigned long flags;
-	struct request *rq_ptr;
-	int bytes_read;
 
 	debug_log(DBG_PROCS, "Enter %s, %d blocks\n", __func__, blocks);
 
@@ -2413,39 +2400,8 @@ static int idetape_add_chrdev_read_request(ide_drive_t *drive, int blocks)
 	if (test_bit(IDETAPE_FLAG_FILEMARK, &tape->flags))
 		return 0;
 
-	/* Wait for the next block to reach the head of the pipeline. */
-	idetape_init_read(drive, tape->max_stages);
-	if (tape->first_stage == NULL) {
-		if (test_bit(IDETAPE_FLAG_PIPELINE_ERR, &tape->flags))
-			return 0;
-		return idetape_queue_rw_tail(drive, REQ_IDETAPE_READ, blocks,
+	return idetape_queue_rw_tail(drive, REQ_IDETAPE_READ, blocks,
 					tape->merge_stage->bh);
-	}
-	idetape_wait_first_stage(drive);
-	rq_ptr = &tape->first_stage->rq;
-	bytes_read = tape->blk_size * (rq_ptr->nr_sectors -
-					rq_ptr->current_nr_sectors);
-	rq_ptr->nr_sectors = 0;
-	rq_ptr->current_nr_sectors = 0;
-
-	if (rq_ptr->errors == IDETAPE_ERROR_EOD)
-		return 0;
-	else {
-		idetape_switch_buffers(tape, tape->first_stage);
-		if (rq_ptr->errors == IDETAPE_ERROR_FILEMARK)
-			set_bit(IDETAPE_FLAG_FILEMARK, &tape->flags);
-		spin_lock_irqsave(&tape->lock, flags);
-		idetape_remove_stage_head(drive);
-		spin_unlock_irqrestore(&tape->lock, flags);
-		tape->pipeline_head++;
-		idetape_calculate_speeds(drive);
-	}
-	if (bytes_read > blocks * tape->blk_size) {
-		printk(KERN_ERR "ide-tape: bug: trying to return more bytes"
-				" than requested\n");
-		bytes_read = blocks * tape->blk_size;
-	}
-	return (bytes_read);
 }
 
 static void idetape_pad_zeros(ide_drive_t *drive, int bcount)
-- 
1.5.4.1

Re: [PATCH 03/24] ide-tape: remove pipeline-specific code from idetape_add_chrdev_read_request

[Bartlomiej Zolnierkiewicz]

On Saturday 01 March 2008, Borislav Petkov wrote:
> We fall back to non-pipelined operation through idetape_queue_rw_tail with cmd
> set to REQ_IDETAPE_READ. Also, remove idetape_switch_buffers() since it becomes
> unused.
> 
> Signed-off-by: Borislav Petkov <petkovbb@gmail.com>
> ---
>  drivers/ide/ide-tape.c |   46 +---------------------------------------------
>  1 files changed, 1 insertions(+), 45 deletions(-)
> 
> diff --git a/drivers/ide/ide-tape.c b/drivers/ide/ide-tape.c
> index abf3efa..e919d41 100644
> --- a/drivers/ide/ide-tape.c
> +++ b/drivers/ide/ide-tape.c
> @@ -1776,16 +1776,6 @@ static void idetape_init_merge_stage(idetape_tape_t *tape)
>  	}
>  }
>  
> -static void idetape_switch_buffers(idetape_tape_t *tape, idetape_stage_t *stage)
> -{
> -	struct idetape_bh *tmp;
> -
> -	tmp = stage->bh;
> -	stage->bh = tape->merge_stage->bh;
> -	tape->merge_stage->bh = tmp;
> -	idetape_init_merge_stage(tape);
> -}
> -
>  /* Add a new stage at the end of the pipeline. */
>  static void idetape_add_stage_tail(ide_drive_t *drive, idetape_stage_t *stage)
>  {
> @@ -2403,9 +2393,6 @@ static int idetape_init_read(ide_drive_t *drive, int max_stages)
>  static int idetape_add_chrdev_read_request(ide_drive_t *drive, int blocks)
>  {
>  	idetape_tape_t *tape = drive->driver_data;
> -	unsigned long flags;
> -	struct request *rq_ptr;
> -	int bytes_read;
>  
>  	debug_log(DBG_PROCS, "Enter %s, %d blocks\n", __func__, blocks);
>  
> @@ -2413,39 +2400,8 @@ static int idetape_add_chrdev_read_request(ide_drive_t *drive, int blocks)
>  	if (test_bit(IDETAPE_FLAG_FILEMARK, &tape->flags))
>  		return 0;
>  
> -	/* Wait for the next block to reach the head of the pipeline. */
> -	idetape_init_read(drive, tape->max_stages);

Can it be simply removed?  Why?

> -	if (tape->first_stage == NULL) {
> -		if (test_bit(IDETAPE_FLAG_PIPELINE_ERR, &tape->flags))
> -			return 0;
> -		return idetape_queue_rw_tail(drive, REQ_IDETAPE_READ, blocks,
> +	return idetape_queue_rw_tail(drive, REQ_IDETAPE_READ, blocks,
>  					tape->merge_stage->bh);

Looking at the driver code tape->first_stage is not always NULL,
seems like ide_tape_add_stage_tail() should vanish first?

Re: [PATCH 03/24] ide-tape: remove pipeline-specific code from idetape_add_chrdev_read_request

[Borislav Petkov]

On Sun, Mar 02, 2008 at 07:36:12PM +0100, Bartlomiej Zolnierkiewicz wrote:
> On Saturday 01 March 2008, Borislav Petkov wrote:
> > We fall back to non-pipelined operation through idetape_queue_rw_tail with cmd
> > set to REQ_IDETAPE_READ. Also, remove idetape_switch_buffers() since it becomes
> > unused.
> > 
> > Signed-off-by: Borislav Petkov <petkovbb@gmail.com>
> > ---
> >  drivers/ide/ide-tape.c |   46 +---------------------------------------------
> >  1 files changed, 1 insertions(+), 45 deletions(-)
> > 
> > diff --git a/drivers/ide/ide-tape.c b/drivers/ide/ide-tape.c
> > index abf3efa..e919d41 100644
> > --- a/drivers/ide/ide-tape.c
> > +++ b/drivers/ide/ide-tape.c
> > @@ -1776,16 +1776,6 @@ static void idetape_init_merge_stage(idetape_tape_t *tape)
> >  	}
> >  }
> >  
> > -static void idetape_switch_buffers(idetape_tape_t *tape, idetape_stage_t *stage)
> > -{
> > -	struct idetape_bh *tmp;
> > -
> > -	tmp = stage->bh;
> > -	stage->bh = tape->merge_stage->bh;
> > -	tape->merge_stage->bh = tmp;
> > -	idetape_init_merge_stage(tape);
> > -}
> > -
> >  /* Add a new stage at the end of the pipeline. */
> >  static void idetape_add_stage_tail(ide_drive_t *drive, idetape_stage_t *stage)
> >  {
> > @@ -2403,9 +2393,6 @@ static int idetape_init_read(ide_drive_t *drive, int max_stages)
> >  static int idetape_add_chrdev_read_request(ide_drive_t *drive, int blocks)
> >  {
> >  	idetape_tape_t *tape = drive->driver_data;
> > -	unsigned long flags;
> > -	struct request *rq_ptr;
> > -	int bytes_read;
> >  
> >  	debug_log(DBG_PROCS, "Enter %s, %d blocks\n", __func__, blocks);
> >  
> > @@ -2413,39 +2400,8 @@ static int idetape_add_chrdev_read_request(ide_drive_t *drive, int blocks)
> >  	if (test_bit(IDETAPE_FLAG_FILEMARK, &tape->flags))
> >  		return 0;
> >  
> > -	/* Wait for the next block to reach the head of the pipeline. */
> > -	idetape_init_read(drive, tape->max_stages);
> 
> Can it be simply removed?  Why?

Bugger, actually this one _has_ to stay since it inits the tape->merge_stage
buffer.

> > -	if (tape->first_stage == NULL) {
> > -		if (test_bit(IDETAPE_FLAG_PIPELINE_ERR, &tape->flags))
> > -			return 0;
> > -		return idetape_queue_rw_tail(drive, REQ_IDETAPE_READ, blocks,
> > +	return idetape_queue_rw_tail(drive, REQ_IDETAPE_READ, blocks,
> >  					tape->merge_stage->bh);
> 
> Looking at the driver code tape->first_stage is not always NULL,
> seems like ide_tape_add_stage_tail() should vanish first?

At this moment there are no more stages in the pipeline besides only
tape->merge_stage. But this boils down to whether we want to keep the driver
functional in intermittent stages of the removal, as i pointed out before...

-- 
Regards/Gruß,
    Boris.

[PATCH 04/24] ide-tape: remove pipeline-specific code from idetape_chrdev_write

[Borislav Petkov]

Also, remove unused stage-parameter from idetape_copy_stage_from_user()

Signed-off-by: Borislav Petkov <petkovbb@gmail.com>
---
 drivers/ide/ide-tape.c |   15 ++++-----------
 1 files changed, 4 insertions(+), 11 deletions(-)

diff --git a/drivers/ide/ide-tape.c b/drivers/ide/ide-tape.c
index e919d41..4a064c1 100644
--- a/drivers/ide/ide-tape.c
+++ b/drivers/ide/ide-tape.c
@@ -1700,7 +1700,7 @@ static idetape_stage_t *idetape_kmalloc_stage(idetape_tape_t *tape)
 }
 
 static int idetape_copy_stage_from_user(idetape_tape_t *tape,
-		idetape_stage_t *stage, const char __user *buf, int n)
+					const char __user *buf, int n)
 {
 	struct idetape_bh *bh = tape->bh;
 	int count;
@@ -2696,8 +2696,6 @@ static ssize_t idetape_chrdev_write(struct file *file, const char __user *buf,
 
 	/* Initialize write operation */
 	if (tape->chrdev_dir != IDETAPE_DIR_WRITE) {
-		if (tape->chrdev_dir == IDETAPE_DIR_READ)
-			idetape_discard_read_pipeline(drive, 1);
 		if (tape->merge_stage || tape->merge_stage_size) {
 			printk(KERN_ERR "ide-tape: merge_stage_size "
 				"should be 0 now\n");
@@ -2729,8 +2727,6 @@ static ssize_t idetape_chrdev_write(struct file *file, const char __user *buf,
 	}
 	if (count == 0)
 		return (0);
-	if (tape->restart_speed_control_req)
-		idetape_restart_speed_control(drive);
 	if (tape->merge_stage_size) {
 		if (tape->merge_stage_size >= tape->stage_size) {
 			printk(KERN_ERR "ide-tape: bug: merge buf too big\n");
@@ -2739,8 +2735,7 @@ static ssize_t idetape_chrdev_write(struct file *file, const char __user *buf,
 		actually_written = min((unsigned int)
 				(tape->stage_size - tape->merge_stage_size),
 				(unsigned int)count);
-		if (idetape_copy_stage_from_user(tape, tape->merge_stage, buf,
-						 actually_written))
+		if (idetape_copy_stage_from_user(tape, buf, actually_written))
 				ret = -EFAULT;
 		buf += actually_written;
 		tape->merge_stage_size += actually_written;
@@ -2756,8 +2751,7 @@ static ssize_t idetape_chrdev_write(struct file *file, const char __user *buf,
 	}
 	while (count >= tape->stage_size) {
 		ssize_t retval;
-		if (idetape_copy_stage_from_user(tape, tape->merge_stage, buf,
-						 tape->stage_size))
+		if (idetape_copy_stage_from_user(tape, buf, tape->stage_size))
 			ret = -EFAULT;
 		buf += tape->stage_size;
 		count -= tape->stage_size;
@@ -2768,8 +2762,7 @@ static ssize_t idetape_chrdev_write(struct file *file, const char __user *buf,
 	}
 	if (count) {
 		actually_written += count;
-		if (idetape_copy_stage_from_user(tape, tape->merge_stage, buf,
-						 count))
+		if (idetape_copy_stage_from_user(tape, buf, count))
 			ret = -EFAULT;
 		tape->merge_stage_size += count;
 	}
-- 
1.5.4.1

Re: [PATCH 04/24] ide-tape: remove pipeline-specific code from idetape_chrdev_write

[Bartlomiej Zolnierkiewicz]

On Saturday 01 March 2008, Borislav Petkov wrote:
> Also, remove unused stage-parameter from idetape_copy_stage_from_user()

Changes like this one are the best to put into separate patches
at the very beginning of the patch series.

> Signed-off-by: Borislav Petkov <petkovbb@gmail.com>
> ---
>  drivers/ide/ide-tape.c |   15 ++++-----------
>  1 files changed, 4 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/ide/ide-tape.c b/drivers/ide/ide-tape.c
> index e919d41..4a064c1 100644
> --- a/drivers/ide/ide-tape.c
> +++ b/drivers/ide/ide-tape.c
> @@ -1700,7 +1700,7 @@ static idetape_stage_t *idetape_kmalloc_stage(idetape_tape_t *tape)
>  }
>  
>  static int idetape_copy_stage_from_user(idetape_tape_t *tape,
> -		idetape_stage_t *stage, const char __user *buf, int n)
> +					const char __user *buf, int n)
>  {
>  	struct idetape_bh *bh = tape->bh;
>  	int count;
> @@ -2696,8 +2696,6 @@ static ssize_t idetape_chrdev_write(struct file *file, const char __user *buf,
>  
>  	/* Initialize write operation */
>  	if (tape->chrdev_dir != IDETAPE_DIR_WRITE) {
> -		if (tape->chrdev_dir == IDETAPE_DIR_READ)
> -			idetape_discard_read_pipeline(drive, 1);

Why this is OK thing to do?

Are you sure that there are no hidden side-effects?

>  		if (tape->merge_stage || tape->merge_stage_size) {
>  			printk(KERN_ERR "ide-tape: merge_stage_size "
>  				"should be 0 now\n");
> @@ -2729,8 +2727,6 @@ static ssize_t idetape_chrdev_write(struct file *file, const char __user *buf,
>  	}
>  	if (count == 0)
>  		return (0);
> -	if (tape->restart_speed_control_req)
> -		idetape_restart_speed_control(drive);

ditto

tape->restart_speed_control_req can still be non-zero.

Re: [PATCH 04/24] ide-tape: remove pipeline-specific code from idetape_chrdev_write

[Borislav Petkov]

On Sun, Mar 02, 2008 at 07:41:28PM +0100, Bartlomiej Zolnierkiewicz wrote:
> On Saturday 01 March 2008, Borislav Petkov wrote:
> > Also, remove unused stage-parameter from idetape_copy_stage_from_user()
> 
> Changes like this one are the best to put into separate patches
> at the very beginning of the patch series.
> 
> > Signed-off-by: Borislav Petkov <petkovbb@gmail.com>
> > ---
> >  drivers/ide/ide-tape.c |   15 ++++-----------
> >  1 files changed, 4 insertions(+), 11 deletions(-)
> > 
> > diff --git a/drivers/ide/ide-tape.c b/drivers/ide/ide-tape.c
> > index e919d41..4a064c1 100644
> > --- a/drivers/ide/ide-tape.c
> > +++ b/drivers/ide/ide-tape.c
> > @@ -1700,7 +1700,7 @@ static idetape_stage_t *idetape_kmalloc_stage(idetape_tape_t *tape)
> >  }
> >  
> >  static int idetape_copy_stage_from_user(idetape_tape_t *tape,
> > -		idetape_stage_t *stage, const char __user *buf, int n)
> > +					const char __user *buf, int n)
> >  {
> >  	struct idetape_bh *bh = tape->bh;
> >  	int count;
> > @@ -2696,8 +2696,6 @@ static ssize_t idetape_chrdev_write(struct file *file, const char __user *buf,
> >  
> >  	/* Initialize write operation */
> >  	if (tape->chrdev_dir != IDETAPE_DIR_WRITE) {
> > -		if (tape->chrdev_dir == IDETAPE_DIR_READ)
> > -			idetape_discard_read_pipeline(drive, 1);
> 
> Why this is OK thing to do?
> 
> Are you sure that there are no hidden side-effects?
> 
> >  		if (tape->merge_stage || tape->merge_stage_size) {
> >  			printk(KERN_ERR "ide-tape: merge_stage_size "
> >  				"should be 0 now\n");
> > @@ -2729,8 +2727,6 @@ static ssize_t idetape_chrdev_write(struct file *file, const char __user *buf,
> >  	}
> >  	if (count == 0)
> >  		return (0);
> > -	if (tape->restart_speed_control_req)
> > -		idetape_restart_speed_control(drive);
> 
> ditto
> 
> tape->restart_speed_control_req can still be non-zero.

Same as above: my main focus was ease of review and not keeping pipelining
functional at all times.

-- 
Regards/Gruß,
    Boris.

Re: [PATCH 04/24] ide-tape: remove pipeline-specific code from idetape_chrdev_write

[Bartlomiej Zolnierkiewicz]

On Sunday 02 March 2008, Borislav Petkov wrote:
> On Sun, Mar 02, 2008 at 07:41:28PM +0100, Bartlomiej Zolnierkiewicz wrote:
> > On Saturday 01 March 2008, Borislav Petkov wrote:
> > > Also, remove unused stage-parameter from idetape_copy_stage_from_user()
> > 
> > Changes like this one are the best to put into separate patches
> > at the very beginning of the patch series.
> > 
> > > Signed-off-by: Borislav Petkov <petkovbb@gmail.com>
> > > ---
> > >  drivers/ide/ide-tape.c |   15 ++++-----------
> > >  1 files changed, 4 insertions(+), 11 deletions(-)
> > > 
> > > diff --git a/drivers/ide/ide-tape.c b/drivers/ide/ide-tape.c
> > > index e919d41..4a064c1 100644
> > > --- a/drivers/ide/ide-tape.c
> > > +++ b/drivers/ide/ide-tape.c
> > > @@ -1700,7 +1700,7 @@ static idetape_stage_t *idetape_kmalloc_stage(idetape_tape_t *tape)
> > >  }
> > >  
> > >  static int idetape_copy_stage_from_user(idetape_tape_t *tape,
> > > -		idetape_stage_t *stage, const char __user *buf, int n)
> > > +					const char __user *buf, int n)
> > >  {
> > >  	struct idetape_bh *bh = tape->bh;
> > >  	int count;
> > > @@ -2696,8 +2696,6 @@ static ssize_t idetape_chrdev_write(struct file *file, const char __user *buf,
> > >  
> > >  	/* Initialize write operation */
> > >  	if (tape->chrdev_dir != IDETAPE_DIR_WRITE) {
> > > -		if (tape->chrdev_dir == IDETAPE_DIR_READ)
> > > -			idetape_discard_read_pipeline(drive, 1);
> > 
> > Why this is OK thing to do?
> > 
> > Are you sure that there are no hidden side-effects?
> > 
> > >  		if (tape->merge_stage || tape->merge_stage_size) {
> > >  			printk(KERN_ERR "ide-tape: merge_stage_size "
> > >  				"should be 0 now\n");
> > > @@ -2729,8 +2727,6 @@ static ssize_t idetape_chrdev_write(struct file *file, const char __user *buf,
> > >  	}
> > >  	if (count == 0)
> > >  		return (0);
> > > -	if (tape->restart_speed_control_req)
> > > -		idetape_restart_speed_control(drive);
> > 
> > ditto
> > 
> > tape->restart_speed_control_req can still be non-zero.
> 
> Same as above: my main focus was ease of review and not keeping pipelining
> functional at all times.

The review is _not_ easied by introducing new & not completely defined
states of the operation...

[ i.e. the speed control feedback loop should be either left alone or
  removed altogether in some pre/post-patch instead of subtle changes
  in the intermediate patches. ]

Could please recast the patches?

Thanks,
Bart

[PATCH 05/24] ide-tape: cleanup pipeline-specific code from idetape_init_read

[Borislav Petkov]

Also, remove idetape_kmalloc_stage() and idetape_add_stage_tail() since they've
become unused, as a result.

Signed-off-by: Borislav Petkov <petkovbb@gmail.com>
---
 drivers/ide/ide-tape.c |   73 +++--------------------------------------------
 1 files changed, 5 insertions(+), 68 deletions(-)

diff --git a/drivers/ide/ide-tape.c b/drivers/ide/ide-tape.c
index 4a064c1..6bca29b 100644
--- a/drivers/ide/ide-tape.c
+++ b/drivers/ide/ide-tape.c
@@ -1684,21 +1684,6 @@ abort:
 	return NULL;
 }
 
-static idetape_stage_t *idetape_kmalloc_stage(idetape_tape_t *tape)
-{
-	idetape_stage_t *cache_stage = tape->cache_stage;
-
-	debug_log(DBG_PROCS, "Enter %s\n", __func__);
-
-	if (tape->nr_stages >= tape->max_stages)
-		return NULL;
-	if (cache_stage != NULL) {
-		tape->cache_stage = NULL;
-		return cache_stage;
-	}
-	return __idetape_kmalloc_stage(tape, 0, 0);
-}
-
 static int idetape_copy_stage_from_user(idetape_tape_t *tape,
 					const char __user *buf, int n)
 {
@@ -1776,30 +1761,8 @@ static void idetape_init_merge_stage(idetape_tape_t *tape)
 	}
 }
 
-/* Add a new stage at the end of the pipeline. */
-static void idetape_add_stage_tail(ide_drive_t *drive, idetape_stage_t *stage)
-{
-	idetape_tape_t *tape = drive->driver_data;
-	unsigned long flags;
-
-	debug_log(DBG_PROCS, "Enter %s\n", __func__);
-
-	spin_lock_irqsave(&tape->lock, flags);
-	stage->next = NULL;
-	if (tape->last_stage != NULL)
-		tape->last_stage->next = stage;
-	else
-		tape->first_stage = stage;
-		tape->next_stage  = stage;
-	tape->last_stage = stage;
-	if (tape->next_stage == NULL)
-		tape->next_stage = tape->last_stage;
-	tape->nr_stages++;
-	tape->nr_pending_stages++;
-	spin_unlock_irqrestore(&tape->lock, flags);
-}
-
-/* Install a completion in a pending request and sleep until it is serviced. The
+/*
+ * Install a completion in a pending request and sleep until it is serviced. The
  * caller should ensure that the request will not be serviced before we install
  * the completion (usually by disabling interrupts).
  */
@@ -2318,17 +2281,12 @@ static void idetape_restart_speed_control(ide_drive_t *drive)
 static int idetape_init_read(ide_drive_t *drive, int max_stages)
 {
 	idetape_tape_t *tape = drive->driver_data;
-	idetape_stage_t *new_stage;
 	struct request rq;
 	int bytes_read;
 	u16 blocks = *(u16 *)&tape->caps[12];
 
 	/* Initialize read operation */
 	if (tape->chrdev_dir != IDETAPE_DIR_READ) {
-		if (tape->chrdev_dir == IDETAPE_DIR_WRITE) {
-			idetape_empty_write_pipeline(drive);
-			idetape_flush_tape_buffers(drive);
-		}
 		if (tape->merge_stage || tape->merge_stage_size) {
 			printk(KERN_ERR "ide-tape: merge_stage_size should be"
 					 " 0 now\n");
@@ -2347,8 +2305,8 @@ static int idetape_init_read(ide_drive_t *drive, int max_stages)
 		 */
 		if (drive->dsc_overlap) {
 			bytes_read = idetape_queue_rw_tail(drive,
-							REQ_IDETAPE_READ, 0,
-							tape->merge_stage->bh);
+							 REQ_IDETAPE_READ, 0,
+							 tape->merge_stage->bh);
 			if (bytes_read < 0) {
 				__idetape_kfree_stage(tape->merge_stage);
 				tape->merge_stage = NULL;
@@ -2357,32 +2315,11 @@ static int idetape_init_read(ide_drive_t *drive, int max_stages)
 			}
 		}
 	}
-	if (tape->restart_speed_control_req)
-		idetape_restart_speed_control(drive);
 	idetape_init_rq(&rq, REQ_IDETAPE_READ);
 	rq.sector = tape->first_frame;
 	rq.nr_sectors = blocks;
 	rq.current_nr_sectors = blocks;
-	if (!test_bit(IDETAPE_FLAG_PIPELINE_ERR, &tape->flags) &&
-	    tape->nr_stages < max_stages) {
-		new_stage = idetape_kmalloc_stage(tape);
-		while (new_stage != NULL) {
-			new_stage->rq = rq;
-			idetape_add_stage_tail(drive, new_stage);
-			if (tape->nr_stages >= max_stages)
-				break;
-			new_stage = idetape_kmalloc_stage(tape);
-		}
-	}
-	if (!test_bit(IDETAPE_FLAG_PIPELINE_ACTIVE, &tape->flags)) {
-		if (tape->nr_pending_stages >= 3 * max_stages / 4) {
-			tape->measure_insert_time = 1;
-			tape->insert_time = jiffies;
-			tape->insert_size = 0;
-			tape->insert_speed = 0;
-			idetape_plug_pipeline(drive);
-		}
-	}
+
 	return 0;
 }
 
-- 
1.5.4.1

Re: [PATCH 05/24] ide-tape: cleanup pipeline-specific code from idetape_init_read

[Bartlomiej Zolnierkiewicz]

On Saturday 01 March 2008, Borislav Petkov wrote:
> Also, remove idetape_kmalloc_stage() and idetape_add_stage_tail() since they've
> become unused, as a result.

I wonder whether some of these changes should be done before patch #2 or #3?

It could be that it would make patches #2-4 obvious, ie.

> -	if (!test_bit(IDETAPE_FLAG_PIPELINE_ERR, &tape->flags) &&
> -	    tape->nr_stages < max_stages) {
> -		new_stage = idetape_kmalloc_stage(tape);
> -		while (new_stage != NULL) {
> -			new_stage->rq = rq;
> -			idetape_add_stage_tail(drive, new_stage);
> -			if (tape->nr_stages >= max_stages)
> -				break;
> -			new_stage = idetape_kmalloc_stage(tape);
> -		}
> -	}

I can see now that after this change ->first_stage will be always NULL

[PATCH 06/24] ide-tape: remove unused stage-parameter from idetape_copy_stage_to_user

[Borislav Petkov]

Signed-off-by: Borislav Petkov <petkovbb@gmail.com>
---
 drivers/ide/ide-tape.c |   11 ++++-------
 1 files changed, 4 insertions(+), 7 deletions(-)

diff --git a/drivers/ide/ide-tape.c b/drivers/ide/ide-tape.c
index 6bca29b..a54c1cb 100644
--- a/drivers/ide/ide-tape.c
+++ b/drivers/ide/ide-tape.c
@@ -1717,7 +1717,7 @@ static int idetape_copy_stage_from_user(idetape_tape_t *tape,
 }
 
 static int idetape_copy_stage_to_user(idetape_tape_t *tape, char __user *buf,
-		idetape_stage_t *stage, int n)
+				      int n)
 {
 	struct idetape_bh *bh = tape->bh;
 	int count;
@@ -2576,8 +2576,7 @@ static ssize_t idetape_chrdev_read(struct file *file, char __user *buf,
 	if (tape->merge_stage_size) {
 		actually_read = min((unsigned int)(tape->merge_stage_size),
 				    (unsigned int)count);
-		if (idetape_copy_stage_to_user(tape, buf, tape->merge_stage,
-					       actually_read))
+		if (idetape_copy_stage_to_user(tape, buf, actually_read))
 			ret = -EFAULT;
 		buf += actually_read;
 		tape->merge_stage_size -= actually_read;
@@ -2587,8 +2586,7 @@ static ssize_t idetape_chrdev_read(struct file *file, char __user *buf,
 		bytes_read = idetape_add_chrdev_read_request(drive, ctl);
 		if (bytes_read <= 0)
 			goto finish;
-		if (idetape_copy_stage_to_user(tape, buf, tape->merge_stage,
-					       bytes_read))
+		if (idetape_copy_stage_to_user(tape, buf, bytes_read))
 			ret = -EFAULT;
 		buf += bytes_read;
 		count -= bytes_read;
@@ -2599,8 +2597,7 @@ static ssize_t idetape_chrdev_read(struct file *file, char __user *buf,
 		if (bytes_read <= 0)
 			goto finish;
 		temp = min((unsigned long)count, (unsigned long)bytes_read);
-		if (idetape_copy_stage_to_user(tape, buf, tape->merge_stage,
-					       temp))
+		if (idetape_copy_stage_to_user(tape, buf, temp))
 			ret = -EFAULT;
 		actually_read += temp;
 		tape->merge_stage_size = bytes_read-temp;
-- 
1.5.4.1

Re: [PATCH 06/24] ide-tape: remove unused stage-parameter from idetape_copy_stage_to_user

[Bartlomiej Zolnierkiewicz]

On Saturday 01 March 2008, Borislav Petkov wrote:
> Signed-off-by: Borislav Petkov <petkovbb@gmail.com>

please move it at the beginning of the patch series

Lets stop at this patch for now and get changes from patches #2-6
into IDE tree first before moving on patches #7-24.

Thanks,
Bart

[PATCH 07/24] ide-tape: remove pipeline-specific code bits from idetape_chrdev_read

[Borislav Petkov]

Also, simplify multiple-nested if construct.

Signed-off-by: Borislav Petkov <petkovbb@gmail.com>
---
 drivers/ide/ide-tape.c |   16 ++++++++--------
 1 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/ide/ide-tape.c b/drivers/ide/ide-tape.c
index a54c1cb..99d6b29 100644
--- a/drivers/ide/ide-tape.c
+++ b/drivers/ide/ide-tape.c
@@ -2278,7 +2278,7 @@ static void idetape_restart_speed_control(ide_drive_t *drive)
 		tape->uncontrolled_previous_head_time = jiffies;
 }
 
-static int idetape_init_read(ide_drive_t *drive, int max_stages)
+static int idetape_init_read(ide_drive_t *drive)
 {
 	idetape_tape_t *tape = drive->driver_data;
 	struct request rq;
@@ -2562,13 +2562,13 @@ static ssize_t idetape_chrdev_read(struct file *file, char __user *buf,
 
 	debug_log(DBG_CHRDEV, "Enter %s, count %Zd\n", __func__, count);
 
-	if (tape->chrdev_dir != IDETAPE_DIR_READ) {
-		if (test_bit(IDETAPE_FLAG_DETECT_BS, &tape->flags))
-			if (count > tape->blk_size &&
-			    (count % tape->blk_size) == 0)
-				tape->user_bs_factor = count / tape->blk_size;
-	}
-	rc = idetape_init_read(drive, tape->max_stages);
+	if (tape->chrdev_dir != IDETAPE_DIR_READ &&
+	    test_bit(IDETAPE_FLAG_DETECT_BS, &tape->flags) &&
+	    count > tape->blk_size &&
+	    (count % tape->blk_size) == 0)
+		tape->user_bs_factor = count / tape->blk_size;
+
+	rc = idetape_init_read(drive);
 	if (rc < 0)
 		return rc;
 	if (count == 0)
-- 
1.5.4.1

[PATCH 08/24] ide-tape: remove pipeline-specific code from idetape_space_over_filemarks

[Borislav Petkov]

Also, remove idetape_wait_first_stage() too since it becomes unused.

Signed-off-by: Borislav Petkov <petkovbb@gmail.com>
---
 drivers/ide/ide-tape.c |   65 +-----------------------------------------------
 1 files changed, 1 insertions(+), 64 deletions(-)

diff --git a/drivers/ide/ide-tape.c b/drivers/ide/ide-tape.c
index 99d6b29..d4a2e73 100644
--- a/drivers/ide/ide-tape.c
+++ b/drivers/ide/ide-tape.c
@@ -2143,19 +2143,6 @@ static void idetape_create_space_cmd(struct ide_atapi_pc *pc, int count, u8 cmd)
 	pc->idetape_callback = &idetape_pc_callback;
 }
 
-static void idetape_wait_first_stage(ide_drive_t *drive)
-{
-	idetape_tape_t *tape = drive->driver_data;
-	unsigned long flags;
-
-	if (tape->first_stage == NULL)
-		return;
-	spin_lock_irqsave(&tape->lock, flags);
-	if (tape->active_stage == tape->first_stage)
-		idetape_wait_for_request(drive, tape->active_data_rq);
-	spin_unlock_irqrestore(&tape->lock, flags);
-}
-
 /* Queue up a character device originated write request. */
 static int idetape_add_chrdev_write_request(ide_drive_t *drive, int blocks)
 {
@@ -2446,73 +2433,23 @@ static int idetape_blkdev_ioctl(ide_drive_t *drive, unsigned int cmd,
 	return 0;
 }
 
-/*
- * The function below is now a bit more complicated than just passing the
- * command to the tape since we may have crossed some filemarks during our
- * pipelined read-ahead mode. As a minor side effect, the pipeline enables us to
- * support MTFSFM when the filemark is in our internal pipeline even if the tape
- * doesn't support spacing over filemarks in the reverse direction.
- */
 static int idetape_space_over_filemarks(ide_drive_t *drive, short mt_op,
 					int mt_count)
 {
 	idetape_tape_t *tape = drive->driver_data;
 	struct ide_atapi_pc pc;
-	unsigned long flags;
 	int retval, count = 0;
 	int sprev = !!(tape->caps[4] & 0x20);
 
 	if (mt_count == 0)
 		return 0;
+
 	if (MTBSF == mt_op || MTBSFM == mt_op) {
 		if (!sprev)
 			return -EIO;
 		mt_count = -mt_count;
 	}
 
-	if (tape->chrdev_dir == IDETAPE_DIR_READ) {
-		/* its a read-ahead buffer, scan it for crossed filemarks. */
-		tape->merge_stage_size = 0;
-		if (test_and_clear_bit(IDETAPE_FLAG_FILEMARK, &tape->flags))
-			++count;
-		while (tape->first_stage != NULL) {
-			if (count == mt_count) {
-				if (mt_op == MTFSFM)
-					set_bit(IDETAPE_FLAG_FILEMARK,
-						&tape->flags);
-				return 0;
-			}
-			spin_lock_irqsave(&tape->lock, flags);
-			if (tape->first_stage == tape->active_stage) {
-				/*
-				 * We have reached the active stage in the read
-				 * pipeline. There is no point in allowing the
-				 * drive to continue reading any farther, so we
-				 * stop the pipeline.
-				 *
-				 * This section should be moved to a separate
-				 * subroutine because similar operations are
-				 * done in __idetape_discard_read_pipeline(),
-				 * for example.
-				 */
-				tape->next_stage = NULL;
-				spin_unlock_irqrestore(&tape->lock, flags);
-				idetape_wait_first_stage(drive);
-				tape->next_stage = tape->first_stage->next;
-			} else
-				spin_unlock_irqrestore(&tape->lock, flags);
-			if (tape->first_stage->rq.errors ==
-					IDETAPE_ERROR_FILEMARK)
-				++count;
-			idetape_remove_stage_head(drive);
-		}
-		idetape_discard_read_pipeline(drive, 0);
-	}
-
-	/*
-	 * The filemark was not found in our internal pipeline;	now we can issue
-	 * the space command.
-	 */
 	switch (mt_op) {
 	case MTFSF:
 	case MTBSF:
-- 
1.5.4.1

[PATCH 09/24] ide-tape: remove pipeline-specific code from idetape_mtioctop

[Borislav Petkov]

Signed-off-by: Borislav Petkov <petkovbb@gmail.com>
---
 drivers/ide/ide-tape.c |    8 --------
 1 files changed, 0 insertions(+), 8 deletions(-)

diff --git a/drivers/ide/ide-tape.c b/drivers/ide/ide-tape.c
index d4a2e73..dd11c7b 100644
--- a/drivers/ide/ide-tape.c
+++ b/drivers/ide/ide-tape.c
@@ -2693,7 +2693,6 @@ static int idetape_mtioctop(ide_drive_t *drive, short mt_op, int mt_count)
 	case MTWEOF:
 		if (tape->write_prot)
 			return -EACCES;
-		idetape_discard_read_pipeline(drive, 1);
 		for (i = 0; i < mt_count; i++) {
 			retval = idetape_write_filemark(drive);
 			if (retval)
@@ -2701,12 +2700,10 @@ static int idetape_mtioctop(ide_drive_t *drive, short mt_op, int mt_count)
 		}
 		return 0;
 	case MTREW:
-		idetape_discard_read_pipeline(drive, 0);
 		if (idetape_rewind_tape(drive))
 			return -EIO;
 		return 0;
 	case MTLOAD:
-		idetape_discard_read_pipeline(drive, 0);
 		idetape_create_load_unload_cmd(drive, &pc,
 					       IDETAPE_LU_LOAD_MASK);
 		return idetape_queue_pc_tail(drive, &pc);
@@ -2721,7 +2718,6 @@ static int idetape_mtioctop(ide_drive_t *drive, short mt_op, int mt_count)
 				if (!idetape_queue_pc_tail(drive, &pc))
 					tape->door_locked = DOOR_UNLOCKED;
 		}
-		idetape_discard_read_pipeline(drive, 0);
 		idetape_create_load_unload_cmd(drive, &pc,
 					      !IDETAPE_LU_LOAD_MASK);
 		retval = idetape_queue_pc_tail(drive, &pc);
@@ -2729,10 +2725,8 @@ static int idetape_mtioctop(ide_drive_t *drive, short mt_op, int mt_count)
 			clear_bit(IDETAPE_FLAG_MEDIUM_PRESENT, &tape->flags);
 		return retval;
 	case MTNOP:
-		idetape_discard_read_pipeline(drive, 0);
 		return idetape_flush_tape_buffers(drive);
 	case MTRETEN:
-		idetape_discard_read_pipeline(drive, 0);
 		idetape_create_load_unload_cmd(drive, &pc,
 			IDETAPE_LU_RETENSION_MASK | IDETAPE_LU_LOAD_MASK);
 		return idetape_queue_pc_tail(drive, &pc);
@@ -2754,11 +2748,9 @@ static int idetape_mtioctop(ide_drive_t *drive, short mt_op, int mt_count)
 			set_bit(IDETAPE_FLAG_DETECT_BS, &tape->flags);
 		return 0;
 	case MTSEEK:
-		idetape_discard_read_pipeline(drive, 0);
 		return idetape_position_tape(drive,
 			mt_count * tape->user_bs_factor, tape->partition, 0);
 	case MTSETPART:
-		idetape_discard_read_pipeline(drive, 0);
 		return idetape_position_tape(drive, 0, mt_count, 0);
 	case MTFSR:
 	case MTBSR:
-- 
1.5.4.1

[RFCPATCH 10/24] ide-tape: remove pipeline-specific code from idetape_chrdev_ioctl

[Borislav Petkov]

The function used to compute the block_offset in order to write it
into mtget.mt_blkno, among others, by going over the stages still
present in the pipeline and adding the sectors left to submit in each
request. This was being done in idetape_pipeline_size() so remove it.
Since we do non-pipelined operation only, compute the offset now
from the sectors left to submit in the currently active request.

Signed-off-by: Borislav Petkov <petkovbb@gmail.com>
---
 drivers/ide/ide-tape.c |   39 +++++++++------------------------------
 1 files changed, 9 insertions(+), 30 deletions(-)

diff --git a/drivers/ide/ide-tape.c b/drivers/ide/ide-tape.c
index dd11c7b..447b7b4 100644
--- a/drivers/ide/ide-tape.c
+++ b/drivers/ide/ide-tape.c
@@ -2353,27 +2353,6 @@ static void idetape_pad_zeros(ide_drive_t *drive, int bcount)
 	}
 }
 
-static int idetape_pipeline_size(ide_drive_t *drive)
-{
-	idetape_tape_t *tape = drive->driver_data;
-	idetape_stage_t *stage;
-	struct request *rq;
-	int size = 0;
-
-	idetape_wait_for_pipeline(drive);
-	stage = tape->first_stage;
-	while (stage != NULL) {
-		rq = &stage->rq;
-		size += tape->blk_size * (rq->nr_sectors -
-				rq->current_nr_sectors);
-		if (rq->errors == IDETAPE_ERROR_FILEMARK)
-			size += tape->blk_size;
-		stage = stage->next;
-	}
-	size += tape->merge_stage_size;
-	return size;
-}
-
 /*
  * Rewinds the tape to the Beginning Of the current Partition (BOP). We
  * currently support only one partition.
@@ -2779,7 +2758,7 @@ static int idetape_mtioctop(ide_drive_t *drive, short mt_op, int mt_count)
 
 /*
  * Our character device ioctls. General mtio.h magnetic io commands are
- * supported here, and not in the corresponding block interface. Our own
+ * supported here and not in the corresponding block interface. Our own
  * ide-tape ioctls are supported on both interfaces.
  */
 static int idetape_chrdev_ioctl(struct inode *inode, struct file *file,
@@ -2795,18 +2774,20 @@ static int idetape_chrdev_ioctl(struct inode *inode, struct file *file,
 
 	debug_log(DBG_CHRDEV, "Enter %s, cmd=%u\n", __func__, cmd);
 
-	tape->restart_speed_control_req = 1;
-	if (tape->chrdev_dir == IDETAPE_DIR_WRITE) {
-		idetape_empty_write_pipeline(drive);
+	if (tape->chrdev_dir == IDETAPE_DIR_WRITE)
 		idetape_flush_tape_buffers(drive);
-	}
+
 	if (cmd == MTIOCGET || cmd == MTIOCPOS) {
-		block_offset = idetape_pipeline_size(drive) /
-			(tape->blk_size * tape->user_bs_factor);
+		struct request *rq = tape->active_data_rq;
+
+		block_offset = (rq->nr_sectors - rq->current_nr_sectors) /
+			       (tape->blk_size * tape->user_bs_factor);
+
 		position = idetape_read_position(drive);
 		if (position < 0)
 			return -EIO;
 	}
+
 	switch (cmd) {
 	case MTIOCTOP:
 		if (copy_from_user(&mtop, argp, sizeof(struct mtop)))
@@ -2832,8 +2813,6 @@ static int idetape_chrdev_ioctl(struct inode *inode, struct file *file,
 			return -EFAULT;
 		return 0;
 	default:
-		if (tape->chrdev_dir == IDETAPE_DIR_READ)
-			idetape_discard_read_pipeline(drive, 1);
 		return idetape_blkdev_ioctl(drive, cmd, arg);
 	}
 }
-- 
1.5.4.1

[PATCH 11/24] ide-tape: remove pipeline-specific code from idetape_blkdev_ioctl

[Borislav Petkov]

Also, strip struct idetape_config down to a single int due to unused/removed
members.

Signed-off-by: Borislav Petkov <petkovbb@gmail.com>
---
 drivers/ide/ide-tape.c |   17 +++++------------
 1 files changed, 5 insertions(+), 12 deletions(-)

diff --git a/drivers/ide/ide-tape.c b/drivers/ide/ide-tape.c
index 447b7b4..0ebb745 100644
--- a/drivers/ide/ide-tape.c
+++ b/drivers/ide/ide-tape.c
@@ -2384,26 +2384,19 @@ static int idetape_blkdev_ioctl(ide_drive_t *drive, unsigned int cmd,
 {
 	idetape_tape_t *tape = drive->driver_data;
 	void __user *argp = (void __user *)arg;
-
-	struct idetape_config {
-		int dsc_rw_frequency;
-		int dsc_media_access_frequency;
-		int nr_stages;
-	} config;
+	int dsc_rw_frequency;
 
 	debug_log(DBG_PROCS, "Enter %s\n", __func__);
 
 	switch (cmd) {
 	case 0x0340:
-		if (copy_from_user(&config, argp, sizeof(config)))
+		if (copy_from_user(&dsc_rw_frequency, argp, sizeof(int)))
 			return -EFAULT;
-		tape->best_dsc_rw_freq = config.dsc_rw_frequency;
-		tape->max_stages = config.nr_stages;
+		tape->best_dsc_rw_freq = dsc_rw_frequency;
 		break;
 	case 0x0350:
-		config.dsc_rw_frequency = (int) tape->best_dsc_rw_freq;
-		config.nr_stages = tape->max_stages;
-		if (copy_to_user(argp, &config, sizeof(config)))
+		dsc_rw_frequency = (int) tape->best_dsc_rw_freq;
+		if (copy_to_user(argp, &dsc_rw_frequency, sizeof(int)))
 			return -EFAULT;
 		break;
 	default:
-- 
1.5.4.1

[PATCH 12/24] ide-tape: remove idetape_empty_write_pipeline

[Borislav Petkov]

Signed-off-by: Borislav Petkov <petkovbb@gmail.com>
---
 drivers/ide/ide-tape.c |   75 ------------------------------------------------
 1 files changed, 0 insertions(+), 75 deletions(-)

diff --git a/drivers/ide/ide-tape.c b/drivers/ide/ide-tape.c
index 0ebb745..3e80515 100644
--- a/drivers/ide/ide-tape.c
+++ b/drivers/ide/ide-tape.c
@@ -2173,80 +2173,6 @@ static void idetape_wait_for_pipeline(ide_drive_t *drive)
 	}
 }
 
-static void idetape_empty_write_pipeline(ide_drive_t *drive)
-{
-	idetape_tape_t *tape = drive->driver_data;
-	int blocks, min;
-	struct idetape_bh *bh;
-
-	if (tape->chrdev_dir != IDETAPE_DIR_WRITE) {
-		printk(KERN_ERR "ide-tape: bug: Trying to empty write pipeline,"
-				" but we are not writing.\n");
-		return;
-	}
-	if (tape->merge_stage_size > tape->stage_size) {
-		printk(KERN_ERR "ide-tape: bug: merge_buffer too big\n");
-		tape->merge_stage_size = tape->stage_size;
-	}
-	if (tape->merge_stage_size) {
-		blocks = tape->merge_stage_size / tape->blk_size;
-		if (tape->merge_stage_size % tape->blk_size) {
-			unsigned int i;
-
-			blocks++;
-			i = tape->blk_size - tape->merge_stage_size %
-				tape->blk_size;
-			bh = tape->bh->b_reqnext;
-			while (bh) {
-				atomic_set(&bh->b_count, 0);
-				bh = bh->b_reqnext;
-			}
-			bh = tape->bh;
-			while (i) {
-				if (bh == NULL) {
-					printk(KERN_INFO "ide-tape: bug,"
-							 " bh NULL\n");
-					break;
-				}
-				min = min(i, (unsigned int)(bh->b_size -
-						atomic_read(&bh->b_count)));
-				memset(bh->b_data + atomic_read(&bh->b_count),
-						0, min);
-				atomic_add(min, &bh->b_count);
-				i -= min;
-				bh = bh->b_reqnext;
-			}
-		}
-		(void) idetape_add_chrdev_write_request(drive, blocks);
-		tape->merge_stage_size = 0;
-	}
-	idetape_wait_for_pipeline(drive);
-	if (tape->merge_stage != NULL) {
-		__idetape_kfree_stage(tape->merge_stage);
-		tape->merge_stage = NULL;
-	}
-	clear_bit(IDETAPE_FLAG_PIPELINE_ERR, &tape->flags);
-	tape->chrdev_dir = IDETAPE_DIR_NONE;
-
-	/*
-	 * On the next backup, perform the feedback loop again. (I don't want to
-	 * keep sense information between backups, as some systems are
-	 * constantly on, and the system load can be totally different on the
-	 * next backup).
-	 */
-	tape->max_stages = tape->min_pipeline;
-	if (tape->first_stage != NULL ||
-	    tape->next_stage != NULL ||
-	    tape->last_stage != NULL ||
-	    tape->nr_stages != 0) {
-		printk(KERN_ERR "ide-tape: ide-tape pipeline bug, "
-			"first_stage %p, next_stage %p, "
-			"last_stage %p, nr_stages %d\n",
-			tape->first_stage, tape->next_stage,
-			tape->last_stage, tape->nr_stages);
-	}
-}
-
 static void idetape_restart_speed_control(ide_drive_t *drive)
 {
 	idetape_tape_t *tape = drive->driver_data;
@@ -2923,7 +2849,6 @@ static void idetape_write_release(ide_drive_t *drive, unsigned int minor)
 {
 	idetape_tape_t *tape = drive->driver_data;
 
-	idetape_empty_write_pipeline(drive);
 	tape->merge_stage = __idetape_kmalloc_stage(tape, 1, 0);
 	if (tape->merge_stage != NULL) {
 		idetape_pad_zeros(drive, tape->blk_size *
-- 
1.5.4.1

[PATCH 13/24] ide-tape: remove pipeline-specific code from idetape_chrdev_release

[Borislav Petkov]

Also, remove orphaned functions idetape_{discard_read,plug,wait_for}_pipeline.
Furthermore, tape->cache_stage becomes also unused, so remove it too. Finally,
simplify 4-level-nested if-condition into a single compound one in
idetape_chrdev_release().

Signed-off-by: Borislav Petkov <petkovbb@gmail.com>
---
 drivers/ide/ide-tape.c |   78 +++++-------------------------------------------
 1 files changed, 8 insertions(+), 70 deletions(-)

diff --git a/drivers/ide/ide-tape.c b/drivers/ide/ide-tape.c
index 3e80515..3cb6e3d 100644
--- a/drivers/ide/ide-tape.c
+++ b/drivers/ide/ide-tape.c
@@ -366,7 +366,6 @@ typedef struct ide_tape_obj {
 	/* New requests will be added to the pipeline here */
 	idetape_stage_t *last_stage;
 	/* Optional free stage which we can use */
-	idetape_stage_t *cache_stage;
 	int pages_per_stage;
 	/* Wasted space in each stage */
 	int excess_bh_size;
@@ -2045,25 +2044,6 @@ static int idetape_position_tape(ide_drive_t *drive, unsigned int block,
 	return (idetape_queue_pc_tail(drive, &pc));
 }
 
-static void idetape_discard_read_pipeline(ide_drive_t *drive,
-					  int restore_position)
-{
-	idetape_tape_t *tape = drive->driver_data;
-	int cnt;
-	int seek, position;
-
-	cnt = __idetape_discard_read_pipeline(drive);
-	if (restore_position) {
-		position = idetape_read_position(drive);
-		seek = position > cnt ? position - cnt : 0;
-		if (idetape_position_tape(drive, seek, 0, 0)) {
-			printk(KERN_INFO "ide-tape: %s: position_tape failed in"
-					 " discard_pipeline()\n", tape->name);
-			return;
-		}
-	}
-}
-
 /*
  * Generate a read/write request for the block device interface and wait for it
  * to be serviced.
@@ -2093,19 +2073,6 @@ static int idetape_queue_rw_tail(ide_drive_t *drive, int cmd, int blocks,
 	return (tape->blk_size * (blocks-rq.current_nr_sectors));
 }
 
-/* start servicing the pipeline stages, starting from tape->next_stage. */
-static void idetape_plug_pipeline(ide_drive_t *drive)
-{
-	idetape_tape_t *tape = drive->driver_data;
-
-	if (tape->next_stage == NULL)
-		return;
-	if (!test_and_set_bit(IDETAPE_FLAG_PIPELINE_ACTIVE, &tape->flags)) {
-		idetape_activate_next_stage(drive);
-		(void) ide_do_drive_cmd(drive, tape->active_data_rq, ide_end);
-	}
-}
-
 static void idetape_create_inquiry_cmd(struct ide_atapi_pc *pc)
 {
 	idetape_init_pc(pc);
@@ -2154,25 +2121,6 @@ static int idetape_add_chrdev_write_request(ide_drive_t *drive, int blocks)
 					tape->merge_stage->bh);
 }
 
-/*
- * Wait until all pending pipeline requests are serviced. Typically called on
- * device close.
- */
-static void idetape_wait_for_pipeline(ide_drive_t *drive)
-{
-	idetape_tape_t *tape = drive->driver_data;
-	unsigned long flags;
-
-	while (tape->next_stage || test_bit(IDETAPE_FLAG_PIPELINE_ACTIVE,
-						&tape->flags)) {
-		idetape_plug_pipeline(drive);
-		spin_lock_irqsave(&tape->lock, flags);
-		if (test_bit(IDETAPE_FLAG_PIPELINE_ACTIVE, &tape->flags))
-			idetape_wait_for_request(drive, tape->active_data_rq);
-		spin_unlock_irqrestore(&tape->lock, flags);
-	}
-}
-
 static void idetape_restart_speed_control(ide_drive_t *drive)
 {
 	idetape_tape_t *tape = drive->driver_data;
@@ -2875,26 +2823,16 @@ static int idetape_chrdev_release(struct inode *inode, struct file *filp)
 
 	if (tape->chrdev_dir == IDETAPE_DIR_WRITE)
 		idetape_write_release(drive, minor);
-	if (tape->chrdev_dir == IDETAPE_DIR_READ) {
-		if (minor < 128)
-			idetape_discard_read_pipeline(drive, 1);
-		else
-			idetape_wait_for_pipeline(drive);
-	}
-	if (tape->cache_stage != NULL) {
-		__idetape_kfree_stage(tape->cache_stage);
-		tape->cache_stage = NULL;
-	}
+
 	if (minor < 128 && test_bit(IDETAPE_FLAG_MEDIUM_PRESENT, &tape->flags))
 		(void) idetape_rewind_tape(drive);
-	if (tape->chrdev_dir == IDETAPE_DIR_NONE) {
-		if (tape->door_locked == DOOR_LOCKED) {
-			if (idetape_create_prevent_cmd(drive, &pc, 0)) {
-				if (!idetape_queue_pc_tail(drive, &pc))
-					tape->door_locked = DOOR_UNLOCKED;
-			}
-		}
-	}
+
+	if (tape->chrdev_dir == IDETAPE_DIR_NONE &&
+	    tape->door_locked == DOOR_LOCKED &&
+	    idetape_create_prevent_cmd(drive, &pc, 0) &&
+	    !idetape_queue_pc_tail(drive, &pc))
+		tape->door_locked = DOOR_UNLOCKED;
+
 	clear_bit(IDETAPE_FLAG_BUSY, &tape->flags);
 	ide_tape_put(tape);
 	unlock_kernel();
-- 
1.5.4.1

[PATCH 14/24] ide-tape: remove __idetape_discard_read_pipeline

[Borislav Petkov]

Signed-off-by: Borislav Petkov <petkovbb@gmail.com>
---
 drivers/ide/ide-tape.c |   49 ------------------------------------------------
 1 files changed, 0 insertions(+), 49 deletions(-)

diff --git a/drivers/ide/ide-tape.c b/drivers/ide/ide-tape.c
index 3cb6e3d..a3a45b5 100644
--- a/drivers/ide/ide-tape.c
+++ b/drivers/ide/ide-tape.c
@@ -1973,52 +1973,6 @@ static int idetape_create_prevent_cmd(ide_drive_t *drive,
 	return 1;
 }
 
-static int __idetape_discard_read_pipeline(ide_drive_t *drive)
-{
-	idetape_tape_t *tape = drive->driver_data;
-	unsigned long flags;
-	int cnt;
-
-	if (tape->chrdev_dir != IDETAPE_DIR_READ)
-		return 0;
-
-	/* Remove merge stage. */
-	cnt = tape->merge_stage_size / tape->blk_size;
-	if (test_and_clear_bit(IDETAPE_FLAG_FILEMARK, &tape->flags))
-		++cnt;		/* Filemarks count as 1 sector */
-	tape->merge_stage_size = 0;
-	if (tape->merge_stage != NULL) {
-		__idetape_kfree_stage(tape->merge_stage);
-		tape->merge_stage = NULL;
-	}
-
-	/* Clear pipeline flags. */
-	clear_bit(IDETAPE_FLAG_PIPELINE_ERR, &tape->flags);
-	tape->chrdev_dir = IDETAPE_DIR_NONE;
-
-	/* Remove pipeline stages. */
-	if (tape->first_stage == NULL)
-		return 0;
-
-	spin_lock_irqsave(&tape->lock, flags);
-	tape->next_stage = NULL;
-	if (test_bit(IDETAPE_FLAG_PIPELINE_ACTIVE, &tape->flags))
-		idetape_wait_for_request(drive, tape->active_data_rq);
-	spin_unlock_irqrestore(&tape->lock, flags);
-
-	while (tape->first_stage != NULL) {
-		struct request *rq_ptr = &tape->first_stage->rq;
-
-		cnt += rq_ptr->nr_sectors - rq_ptr->current_nr_sectors;
-		if (rq_ptr->errors == IDETAPE_ERROR_FILEMARK)
-			++cnt;
-		idetape_remove_stage_head(drive);
-	}
-	tape->nr_pending_stages = 0;
-	tape->max_stages = tape->min_pipeline;
-	return cnt;
-}
-
 /*
  * Position the tape to the requested block using the LOCATE packet command.
  * A READ POSITION command is then issued to check where we are positioned. Like
@@ -2028,12 +1982,9 @@ static int __idetape_discard_read_pipeline(ide_drive_t *drive)
 static int idetape_position_tape(ide_drive_t *drive, unsigned int block,
 		u8 partition, int skip)
 {
-	idetape_tape_t *tape = drive->driver_data;
 	int retval;
 	struct ide_atapi_pc pc;
 
-	if (tape->chrdev_dir == IDETAPE_DIR_READ)
-		__idetape_discard_read_pipeline(drive);
 	idetape_wait_ready(drive, 60 * 5 * HZ);
 	idetape_create_locate_cmd(drive, &pc, block, partition, skip);
 	retval = idetape_queue_pc_tail(drive, &pc);
-- 
1.5.4.1

[PATCH 15/24] ide-tape: remove pipeline-specific code from idetape_end_request

[Borislav Petkov]

As a result, remove orphaned

idetape_activate_next_stage
idetape_remove_stage_head
idetape_abort_pipeline
idetape_wait_for_request
idetape_kfree_stage

too.

Signed-off-by: Borislav Petkov <petkovbb@gmail.com>
---
 drivers/ide/ide-tape.c |  172 +-----------------------------------------------
 1 files changed, 1 insertions(+), 171 deletions(-)

diff --git a/drivers/ide/ide-tape.c b/drivers/ide/ide-tape.c
index a3a45b5..93e42e6 100644
--- a/drivers/ide/ide-tape.c
+++ b/drivers/ide/ide-tape.c
@@ -673,28 +673,6 @@ static void idetape_analyze_error(ide_drive_t *drive, u8 *sense)
 	}
 }
 
-static void idetape_activate_next_stage(ide_drive_t *drive)
-{
-	idetape_tape_t *tape = drive->driver_data;
-	idetape_stage_t *stage = tape->next_stage;
-	struct request *rq = &stage->rq;
-
-	debug_log(DBG_PROCS, "Enter %s\n", __func__);
-
-	if (stage == NULL) {
-		printk(KERN_ERR "ide-tape: bug: Trying to activate a non"
-				" existing stage\n");
-		return;
-	}
-
-	rq->rq_disk = tape->disk;
-	rq->buffer = NULL;
-	rq->special = (void *)stage->bh;
-	tape->active_data_rq = rq;
-	tape->active_stage = stage;
-	tape->next_stage = stage->next;
-}
-
 /* Free a stage along with its related buffers completely. */
 static void __idetape_kfree_stage(idetape_stage_t *stage)
 {
@@ -717,84 +695,12 @@ static void __idetape_kfree_stage(idetape_stage_t *stage)
 	kfree(stage);
 }
 
-static void idetape_kfree_stage(idetape_tape_t *tape, idetape_stage_t *stage)
-{
-	__idetape_kfree_stage(stage);
-}
-
-/*
- * Remove tape->first_stage from the pipeline. The caller should avoid race
- * conditions.
- */
-static void idetape_remove_stage_head(ide_drive_t *drive)
-{
-	idetape_tape_t *tape = drive->driver_data;
-	idetape_stage_t *stage;
-
-	debug_log(DBG_PROCS, "Enter %s\n", __func__);
-
-	if (tape->first_stage == NULL) {
-		printk(KERN_ERR "ide-tape: bug: tape->first_stage is NULL\n");
-		return;
-	}
-	if (tape->active_stage == tape->first_stage) {
-		printk(KERN_ERR "ide-tape: bug: Trying to free our active "
-				"pipeline stage\n");
-		return;
-	}
-	stage = tape->first_stage;
-	tape->first_stage = stage->next;
-	idetape_kfree_stage(tape, stage);
-	tape->nr_stages--;
-	if (tape->first_stage == NULL) {
-		tape->last_stage = NULL;
-		if (tape->next_stage != NULL)
-			printk(KERN_ERR "ide-tape: bug: tape->next_stage !="
-					" NULL\n");
-		if (tape->nr_stages)
-			printk(KERN_ERR "ide-tape: bug: nr_stages should be 0 "
-					"now\n");
-	}
-}
-
-/*
- * This will free all the pipeline stages starting from new_last_stage->next
- * to the end of the list, and point tape->last_stage to new_last_stage.
- */
-static void idetape_abort_pipeline(ide_drive_t *drive,
-				   idetape_stage_t *new_last_stage)
-{
-	idetape_tape_t *tape = drive->driver_data;
-	idetape_stage_t *stage = new_last_stage->next;
-	idetape_stage_t *nstage;
-
-	debug_log(DBG_PROCS, "%s: Enter %s\n", tape->name, __func__);
-
-	while (stage) {
-		nstage = stage->next;
-		idetape_kfree_stage(tape, stage);
-		--tape->nr_stages;
-		--tape->nr_pending_stages;
-		stage = nstage;
-	}
-	if (new_last_stage)
-		new_last_stage->next = NULL;
-	tape->last_stage = new_last_stage;
-	tape->next_stage = NULL;
-}
-
-/*
- * Finish servicing a request and insert a pending pipeline request into the
- * main device queue.
- */
+/* Finish servicing a request. */
 static int idetape_end_request(ide_drive_t *drive, int uptodate, int nr_sects)
 {
 	struct request *rq = HWGROUP(drive)->rq;
 	idetape_tape_t *tape = drive->driver_data;
-	unsigned long flags;
 	int error;
-	int remove_stage = 0;
-	idetape_stage_t *active_stage;
 
 	debug_log(DBG_PROCS, "Enter %s\n", __func__);
 
@@ -812,61 +718,8 @@ static int idetape_end_request(ide_drive_t *drive, int uptodate, int nr_sects)
 		return 0;
 	}
 
-	spin_lock_irqsave(&tape->lock, flags);
-
-	/* The request was a pipelined data transfer request */
-	if (tape->active_data_rq == rq) {
-		active_stage = tape->active_stage;
-		tape->active_stage = NULL;
-		tape->active_data_rq = NULL;
-		tape->nr_pending_stages--;
-		if (rq->cmd[0] & REQ_IDETAPE_WRITE) {
-			remove_stage = 1;
-			if (error) {
-				set_bit(IDETAPE_FLAG_PIPELINE_ERR,
-					&tape->flags);
-				if (error == IDETAPE_ERROR_EOD)
-					idetape_abort_pipeline(drive,
-								active_stage);
-			}
-		} else if (rq->cmd[0] & REQ_IDETAPE_READ) {
-			if (error == IDETAPE_ERROR_EOD) {
-				set_bit(IDETAPE_FLAG_PIPELINE_ERR,
-					&tape->flags);
-				idetape_abort_pipeline(drive, active_stage);
-			}
-		}
-		if (tape->next_stage != NULL) {
-			idetape_activate_next_stage(drive);
-
-			/* Insert the next request into the request queue. */
-			(void)ide_do_drive_cmd(drive, tape->active_data_rq,
-						ide_end);
-		} else if (!error) {
-			/*
-			 * This is a part of the feedback loop which tries to
-			 * find the optimum number of stages. We are starting
-			 * from a minimum maximum number of stages, and if we
-			 * sense that the pipeline is empty, we try to increase
-			 * it, until we reach the user compile time memory
-			 * limit.
-			 */
-			int i = (tape->max_pipeline - tape->min_pipeline) / 10;
-
-			tape->max_stages += max(i, 1);
-			tape->max_stages = max(tape->max_stages,
-						tape->min_pipeline);
-			tape->max_stages = min(tape->max_stages,
-						tape->max_pipeline);
-		}
-	}
 	ide_end_drive_cmd(drive, 0, 0);
 
-	if (remove_stage)
-		idetape_remove_stage_head(drive);
-	if (tape->active_data_rq == NULL)
-		clear_bit(IDETAPE_FLAG_PIPELINE_ACTIVE, &tape->flags);
-	spin_unlock_irqrestore(&tape->lock, flags);
 	return 0;
 }
 
@@ -1760,29 +1613,6 @@ static void idetape_init_merge_stage(idetape_tape_t *tape)
 	}
 }
 
-/*
- * Install a completion in a pending request and sleep until it is serviced. The
- * caller should ensure that the request will not be serviced before we install
- * the completion (usually by disabling interrupts).
- */
-static void idetape_wait_for_request(ide_drive_t *drive, struct request *rq)
-{
-	DECLARE_COMPLETION_ONSTACK(wait);
-	idetape_tape_t *tape = drive->driver_data;
-
-	if (rq == NULL || !blk_special_request(rq)) {
-		printk(KERN_ERR "ide-tape: bug: Trying to sleep on non-valid"
-				 " request\n");
-		return;
-	}
-	rq->end_io_data = &wait;
-	rq->end_io = blk_end_sync_rq;
-	spin_unlock_irq(&tape->lock);
-	wait_for_completion(&wait);
-	/* The stage and its struct request have been deallocated */
-	spin_lock_irq(&tape->lock);
-}
-
 static ide_startstop_t idetape_read_position_callback(ide_drive_t *drive)
 {
 	idetape_tape_t *tape = drive->driver_data;
-- 
1.5.4.1

[PATCH 16/24] ide-tape: remove idetape_calculate_speeds

[Borislav Petkov]

Signed-off-by: Borislav Petkov <petkovbb@gmail.com>
---
 drivers/ide/ide-tape.c |   64 ------------------------------------------------
 1 files changed, 0 insertions(+), 64 deletions(-)

diff --git a/drivers/ide/ide-tape.c b/drivers/ide/ide-tape.c
index 93e42e6..cfc11bb 100644
--- a/drivers/ide/ide-tape.c
+++ b/drivers/ide/ide-tape.c
@@ -1187,69 +1187,6 @@ static void idetape_create_mode_sense_cmd(struct ide_atapi_pc *pc, u8 page_code)
 	pc->idetape_callback = &idetape_pc_callback;
 }
 
-static void idetape_calculate_speeds(ide_drive_t *drive)
-{
-	idetape_tape_t *tape = drive->driver_data;
-
-	if (time_after(jiffies,
-			tape->controlled_pipeline_head_time + 120 * HZ)) {
-		tape->controlled_previous_pipeline_head =
-			tape->controlled_last_pipeline_head;
-		tape->controlled_previous_head_time =
-			tape->controlled_pipeline_head_time;
-		tape->controlled_last_pipeline_head = tape->pipeline_head;
-		tape->controlled_pipeline_head_time = jiffies;
-	}
-	if (time_after(jiffies, tape->controlled_pipeline_head_time + 60 * HZ))
-		tape->controlled_pipeline_head_speed = (tape->pipeline_head -
-				tape->controlled_last_pipeline_head) * 32 * HZ /
-				(jiffies - tape->controlled_pipeline_head_time);
-	else if (time_after(jiffies, tape->controlled_previous_head_time))
-		tape->controlled_pipeline_head_speed = (tape->pipeline_head -
-				tape->controlled_previous_pipeline_head) * 32 *
-			HZ / (jiffies - tape->controlled_previous_head_time);
-
-	if (tape->nr_pending_stages < tape->max_stages/*- 1 */) {
-		/* -1 for read mode error recovery */
-		if (time_after(jiffies, tape->uncontrolled_previous_head_time +
-					10 * HZ)) {
-			tape->uncontrolled_pipeline_head_time = jiffies;
-			tape->uncontrolled_pipeline_head_speed =
-				(tape->pipeline_head -
-				 tape->uncontrolled_previous_pipeline_head) *
-				32 * HZ / (jiffies -
-					tape->uncontrolled_previous_head_time);
-		}
-	} else {
-		tape->uncontrolled_previous_head_time = jiffies;
-		tape->uncontrolled_previous_pipeline_head = tape->pipeline_head;
-		if (time_after(jiffies, tape->uncontrolled_pipeline_head_time +
-					30 * HZ))
-			tape->uncontrolled_pipeline_head_time = jiffies;
-
-	}
-	tape->pipeline_head_speed = max(tape->uncontrolled_pipeline_head_speed,
-					tape->controlled_pipeline_head_speed);
-
-	if (tape->speed_control == 1) {
-		if (tape->nr_pending_stages >= tape->max_stages / 2)
-			tape->max_insert_speed = tape->pipeline_head_speed +
-				(1100 - tape->pipeline_head_speed) * 2 *
-				(tape->nr_pending_stages - tape->max_stages / 2)
-				/ tape->max_stages;
-		else
-			tape->max_insert_speed = 500 +
-				(tape->pipeline_head_speed - 500) * 2 *
-				tape->nr_pending_stages / tape->max_stages;
-
-		if (tape->nr_pending_stages >= tape->max_stages * 99 / 100)
-			tape->max_insert_speed = 5000;
-	} else
-		tape->max_insert_speed = tape->speed_control;
-
-	tape->max_insert_speed = max(tape->max_insert_speed, 500);
-}
-
 static ide_startstop_t idetape_media_access_finished(ide_drive_t *drive)
 {
 	idetape_tape_t *tape = drive->driver_data;
@@ -1402,7 +1339,6 @@ static ide_startstop_t idetape_do_request(ide_drive_t *drive,
 	if (time_after(jiffies, tape->insert_time))
 		tape->insert_speed = tape->insert_size / 1024 * HZ /
 					(jiffies - tape->insert_time);
-	idetape_calculate_speeds(drive);
 	if (!test_and_clear_bit(IDETAPE_FLAG_IGNORE_DSC, &tape->flags) &&
 	    (stat & SEEK_STAT) == 0) {
 		if (postponed_rq == NULL) {
-- 
1.5.4.1

[PATCH 17/24] ide-tape: remove pipeline-specific code from idetape_chrdev_open

[Borislav Petkov]

As a result, remove orphaned idetape_restart_speed_control. Also, unnest
if-condition in idetape_chrdev_open.

Signed-off-by: Borislav Petkov <petkovbb@gmail.com>
---
 drivers/ide/ide-tape.c |   38 ++++++--------------------------------
 1 files changed, 6 insertions(+), 32 deletions(-)

diff --git a/drivers/ide/ide-tape.c b/drivers/ide/ide-tape.c
index cfc11bb..5f57bdb 100644
--- a/drivers/ide/ide-tape.c
+++ b/drivers/ide/ide-tape.c
@@ -424,7 +424,6 @@ typedef struct ide_tape_obj {
 	int uncontrolled_previous_pipeline_head;
 	unsigned long controlled_previous_head_time;
 	unsigned long uncontrolled_previous_head_time;
-	int restart_speed_control_req;
 
 	u32 debug_mask;
 } idetape_tape_t;
@@ -1838,24 +1837,6 @@ static int idetape_add_chrdev_write_request(ide_drive_t *drive, int blocks)
 					tape->merge_stage->bh);
 }
 
-static void idetape_restart_speed_control(ide_drive_t *drive)
-{
-	idetape_tape_t *tape = drive->driver_data;
-
-	tape->restart_speed_control_req = 0;
-	tape->pipeline_head = 0;
-	tape->controlled_last_pipeline_head = 0;
-	tape->controlled_previous_pipeline_head = 0;
-	tape->uncontrolled_previous_pipeline_head = 0;
-	tape->controlled_pipeline_head_speed = 5000;
-	tape->pipeline_head_speed = 5000;
-	tape->uncontrolled_pipeline_head_speed = 0;
-	tape->controlled_pipeline_head_time =
-		tape->uncontrolled_pipeline_head_time = jiffies;
-	tape->controlled_previous_head_time =
-		tape->uncontrolled_previous_head_time = jiffies;
-}
-
 static int idetape_init_read(ide_drive_t *drive)
 {
 	idetape_tape_t *tape = drive->driver_data;
@@ -2470,9 +2451,6 @@ static int idetape_chrdev_open(struct inode *inode, struct file *filp)
 	if (!test_bit(IDETAPE_FLAG_ADDRESS_VALID, &tape->flags))
 		(void)idetape_rewind_tape(drive);
 
-	if (tape->chrdev_dir != IDETAPE_DIR_READ)
-		clear_bit(IDETAPE_FLAG_PIPELINE_ERR, &tape->flags);
-
 	/* Read block size and write protect status from drive. */
 	ide_tape_get_bsize_from_bdesc(drive);
 
@@ -2493,16 +2471,12 @@ static int idetape_chrdev_open(struct inode *inode, struct file *filp)
 	}
 
 	/* Lock the tape drive door so user can't eject. */
-	if (tape->chrdev_dir == IDETAPE_DIR_NONE) {
-		if (idetape_create_prevent_cmd(drive, &pc, 1)) {
-			if (!idetape_queue_pc_tail(drive, &pc)) {
-				if (tape->door_locked != DOOR_EXPLICITLY_LOCKED)
-					tape->door_locked = DOOR_LOCKED;
-			}
-		}
-	}
-	idetape_restart_speed_control(drive);
-	tape->restart_speed_control_req = 0;
+	if (tape->chrdev_dir == IDETAPE_DIR_NONE &&
+	    idetape_create_prevent_cmd(drive, &pc, 1) &&
+	    !idetape_queue_pc_tail(drive, &pc) &&
+	    tape->door_locked != DOOR_EXPLICITLY_LOCKED)
+		tape->door_locked = DOOR_LOCKED;
+
 	return 0;
 
 out_put_tape:
-- 
1.5.4.1

[PATCH 18/24] ide-tape: remove pipeline-specific code from idetape_setup

[Borislav Petkov]

Signed-off-by: Borislav Petkov <petkovbb@gmail.com>
---
 drivers/ide/ide-tape.c |   40 ++++------------------------------------
 1 files changed, 4 insertions(+), 36 deletions(-)

diff --git a/drivers/ide/ide-tape.c b/drivers/ide/ide-tape.c
index 5f57bdb..2718006 100644
--- a/drivers/ide/ide-tape.c
+++ b/drivers/ide/ide-tape.c
@@ -2699,11 +2699,10 @@ static inline void idetape_add_settings(ide_drive_t *drive) { ; }
  */
 static void idetape_setup(ide_drive_t *drive, idetape_tape_t *tape, int minor)
 {
-	unsigned long t1, tmid, tn, t;
+	unsigned long t;
 	int speed;
 	int stage_size;
 	u8 gcw[2];
-	struct sysinfo si;
 	u16 *ctl = (u16 *)&tape->caps[12];
 
 	spin_lock_init(&tape->lock);
@@ -2730,10 +2729,6 @@ static void idetape_setup(ide_drive_t *drive, idetape_tape_t *tape, int minor)
 	if (((gcw[0] & 0x60) >> 5) == 1)
 		set_bit(IDETAPE_FLAG_DRQ_INTERRUPT, &tape->flags);
 
-	tape->min_pipeline = 10;
-	tape->max_pipeline = 10;
-	tape->max_stages   = 10;
-
 	idetape_get_inquiry_results(drive);
 	idetape_get_mode_sense_results(drive);
 	ide_tape_get_bsize_from_bdesc(drive);
@@ -2751,36 +2746,10 @@ static void idetape_setup(ide_drive_t *drive, idetape_tape_t *tape, int minor)
 		tape->excess_bh_size = PAGE_SIZE - stage_size % PAGE_SIZE;
 	}
 
-	/* Select the "best" DSC read/write polling freq and pipeline size. */
+	/* select the "best" DSC read/write polling freq */
 	speed = max(*(u16 *)&tape->caps[14], *(u16 *)&tape->caps[8]);
 
-	tape->max_stages = speed * 1000 * 10 / tape->stage_size;
-
-	/* Limit memory use for pipeline to 10% of physical memory */
-	si_meminfo(&si);
-	if (tape->max_stages * tape->stage_size >
-			si.totalram * si.mem_unit / 10)
-		tape->max_stages =
-			si.totalram * si.mem_unit / (10 * tape->stage_size);
-
-	tape->max_stages   = min(tape->max_stages, IDETAPE_MAX_PIPELINE_STAGES);
-	tape->min_pipeline = min(tape->max_stages, IDETAPE_MIN_PIPELINE_STAGES);
-	tape->max_pipeline =
-		min(tape->max_stages * 2, IDETAPE_MAX_PIPELINE_STAGES);
-	if (tape->max_stages == 0) {
-		tape->max_stages   = 1;
-		tape->min_pipeline = 1;
-		tape->max_pipeline = 1;
-	}
-
-	t1 = (tape->stage_size * HZ) / (speed * 1000);
-	tmid = (*(u16 *)&tape->caps[16] * 32 * HZ) / (speed * 125);
-	tn = (IDETAPE_FIFO_THRESHOLD * tape->stage_size * HZ) / (speed * 1000);
-
-	if (tape->max_stages)
-		t = tn;
-	else
-		t = t1;
+	t = (tape->stage_size * HZ) / (speed * 1000);
 
 	/*
 	 * Ensure that the number we got makes sense; limit it within
@@ -2790,11 +2759,10 @@ static void idetape_setup(ide_drive_t *drive, idetape_tape_t *tape, int minor)
 				min_t(unsigned long, t, IDETAPE_DSC_RW_MAX),
 				IDETAPE_DSC_RW_MIN);
 	printk(KERN_INFO "ide-tape: %s <-> %s: %dKBps, %d*%dkB buffer, "
-		"%dkB pipeline, %lums tDSC%s\n",
+		"%lums tDSC%s\n",
 		drive->name, tape->name, *(u16 *)&tape->caps[14],
 		(*(u16 *)&tape->caps[16] * 512) / tape->stage_size,
 		tape->stage_size / 1024,
-		tape->max_stages * tape->stage_size / 1024,
 		tape->best_dsc_rw_freq * 1000 / HZ,
 		drive->using_dma ? ", DMA":"");
 
-- 
1.5.4.1

[PATCH 19/24] ide-tape: remove pipelined mode parameters

[Borislav Petkov]

Signed-off-by: Borislav Petkov <petkovbb@gmail.com>
---
 drivers/ide/ide-tape.c |   18 ------------------
 1 files changed, 0 insertions(+), 18 deletions(-)

diff --git a/drivers/ide/ide-tape.c b/drivers/ide/ide-tape.c
index 2718006..a5f29ea 100644
--- a/drivers/ide/ide-tape.c
+++ b/drivers/ide/ide-tape.c
@@ -75,24 +75,6 @@ enum {
 
 
 /*
- * Pipelined mode parameters.
- *
- * We try to use the minimum number of stages which is enough to keep the tape
- * constantly streaming. To accomplish that, we implement a feedback loop around
- * the maximum number of stages:
- *
- * We start from MIN maximum stages (we will not even use MIN stages if we don't
- * need them), increment it by RATE*(MAX-MIN) whenever we sense that the
- * pipeline is empty, until we reach the optimum value or until we reach MAX.
- *
- * Setting the following parameter to 0 is illegal: the pipelined mode cannot be
- * disabled (idetape_calculate_speeds() divides by tape->max_stages.)
- */
-#define IDETAPE_MIN_PIPELINE_STAGES	  1
-#define IDETAPE_MAX_PIPELINE_STAGES	400
-#define IDETAPE_INCREASE_STAGES_RATE	 20
-
-/*
  * After each failed packet command we issue a request sense command and retry
  * the packet command IDETAPE_MAX_PC_RETRIES times.
  *
-- 
1.5.4.1

[PATCH 20/24] ide-tape: remove pipelined mode tape control flags

[Borislav Petkov]

Signed-off-by: Borislav Petkov <petkovbb@gmail.com>
---
 drivers/ide/ide-tape.c |   14 +++++---------
 1 files changed, 5 insertions(+), 9 deletions(-)

diff --git a/drivers/ide/ide-tape.c b/drivers/ide/ide-tape.c
index a5f29ea..68c9c09 100644
--- a/drivers/ide/ide-tape.c
+++ b/drivers/ide/ide-tape.c
@@ -206,19 +206,15 @@ enum {
 	/* 0 When the tape position is unknown */
 	IDETAPE_FLAG_ADDRESS_VALID	= (1 <<	1),
 	/* Device already opened */
-	IDETAPE_FLAG_BUSY			= (1 << 2),
-	/* Error detected in a pipeline stage */
-	IDETAPE_FLAG_PIPELINE_ERR	= (1 <<	3),
+	IDETAPE_FLAG_BUSY		= (1 << 2),
 	/* Attempt to auto-detect the current user block size */
-	IDETAPE_FLAG_DETECT_BS		= (1 << 4),
+	IDETAPE_FLAG_DETECT_BS		= (1 << 3),
 	/* Currently on a filemark */
-	IDETAPE_FLAG_FILEMARK		= (1 << 5),
+	IDETAPE_FLAG_FILEMARK		= (1 << 4),
 	/* DRQ interrupt device */
-	IDETAPE_FLAG_DRQ_INTERRUPT	= (1 << 6),
-	/* pipeline active */
-	IDETAPE_FLAG_PIPELINE_ACTIVE	= (1 << 7),
+	IDETAPE_FLAG_DRQ_INTERRUPT	= (1 << 5),
 	/* 0 = no tape is loaded, so we don't rewind after ejecting */
-	IDETAPE_FLAG_MEDIUM_PRESENT	= (1 << 8),
+	IDETAPE_FLAG_MEDIUM_PRESENT	= (1 << 6),
 };
 
 /* A pipeline stage. */
-- 
1.5.4.1

[PATCH 21/24] ide-tape: remove pipeline-specific members from struct ide_tape_obj

[Borislav Petkov]

Most of them have become unused after removing the pipelined
functionality - the rest is only being written to.

Signed-off-by: Borislav Petkov <petkovbb@gmail.com>
---
 drivers/ide/ide-tape.c |   95 +-----------------------------------------------
 1 files changed, 2 insertions(+), 93 deletions(-)

diff --git a/drivers/ide/ide-tape.c b/drivers/ide/ide-tape.c
index 68c9c09..9810253 100644
--- a/drivers/ide/ide-tape.c
+++ b/drivers/ide/ide-tape.c
@@ -323,26 +323,6 @@ typedef struct ide_tape_obj {
 	char *b_data;
 	int b_count;
 
-	/*
-	 * Pipeline parameters.
-	 *
-	 * To accomplish non-pipelined mode, we simply set the following
-	 * variables to zero (or NULL, where appropriate).
-	 */
-	/* Number of currently used stages */
-	int nr_stages;
-	/* Number of pending stages */
-	int nr_pending_stages;
-	/* We will not allocate more than this number of stages */
-	int max_stages, min_pipeline, max_pipeline;
-	/* The first stage which will be removed from the pipeline */
-	idetape_stage_t *first_stage;
-	/* The currently active stage */
-	idetape_stage_t *active_stage;
-	/* Will be serviced after the currently active request */
-	idetape_stage_t *next_stage;
-	/* New requests will be added to the pipeline here */
-	idetape_stage_t *last_stage;
 	/* Optional free stage which we can use */
 	int pages_per_stage;
 	/* Wasted space in each stage */
@@ -365,43 +345,8 @@ typedef struct ide_tape_obj {
 	/* the tape is write protected (hardware or opened as read-only) */
 	char write_prot;
 
-	/*
-	 * Limit the number of times a request can be postponed, to avoid an
-	 * infinite postpone deadlock.
-	 */
-	int postpone_cnt;
-
-	/*
-	 * Measures number of frames:
-	 *
-	 * 1. written/read to/from the driver pipeline (pipeline_head).
-	 * 2. written/read to/from the tape buffers (idetape_bh).
-	 * 3. written/read by the tape to/from the media (tape_head).
-	 */
-	int pipeline_head;
-	int buffer_head;
-	int tape_head;
-	int last_tape_head;
-
-	/* Speed control at the tape buffers input/output */
-	unsigned long insert_time;
+	/* size control at the tape buffers input/output */
 	int insert_size;
-	int insert_speed;
-	int max_insert_speed;
-	int measure_insert_time;
-
-	/* Speed regulation negative feedback loop */
-	int speed_control;
-	int pipeline_head_speed;
-	int controlled_pipeline_head_speed;
-	int uncontrolled_pipeline_head_speed;
-	int controlled_last_pipeline_head;
-	unsigned long uncontrolled_pipeline_head_time;
-	unsigned long controlled_pipeline_head_time;
-	int controlled_previous_pipeline_head;
-	int uncontrolled_previous_pipeline_head;
-	unsigned long controlled_previous_head_time;
-	unsigned long uncontrolled_previous_head_time;
 
 	u32 debug_mask;
 } idetape_tape_t;
@@ -1200,15 +1145,8 @@ static ide_startstop_t idetape_rw_callback(ide_drive_t *drive)
 	tape->avg_size += blocks * tape->blk_size;
 	tape->insert_size += blocks * tape->blk_size;
 	if (tape->insert_size > 1024 * 1024)
-		tape->measure_insert_time = 1;
-	if (tape->measure_insert_time) {
-		tape->measure_insert_time = 0;
-		tape->insert_time = jiffies;
 		tape->insert_size = 0;
-	}
-	if (time_after(jiffies, tape->insert_time))
-		tape->insert_speed = tape->insert_size / 1024 * HZ /
-					(jiffies - tape->insert_time);
+
 	if (time_after_eq(jiffies, tape->avg_time + HZ)) {
 		tape->avg_speed = tape->avg_size * HZ /
 				(jiffies - tape->avg_time) / 1024;
@@ -1313,9 +1251,6 @@ static ide_startstop_t idetape_do_request(ide_drive_t *drive,
 		drive->post_reset = 0;
 	}
 
-	if (time_after(jiffies, tape->insert_time))
-		tape->insert_speed = tape->insert_size / 1024 * HZ /
-					(jiffies - tape->insert_time);
 	if (!test_and_clear_bit(IDETAPE_FLAG_IGNORE_DSC, &tape->flags) &&
 	    (stat & SEEK_STAT) == 0) {
 		if (postponed_rq == NULL) {
@@ -1339,16 +1274,12 @@ static ide_startstop_t idetape_do_request(ide_drive_t *drive,
 		return ide_stopped;
 	}
 	if (rq->cmd[0] & REQ_IDETAPE_READ) {
-		tape->buffer_head++;
-		tape->postpone_cnt = 0;
 		pc = idetape_next_pc_storage(drive);
 		idetape_create_read_cmd(tape, pc, rq->current_nr_sectors,
 					(struct idetape_bh *)rq->special);
 		goto out;
 	}
 	if (rq->cmd[0] & REQ_IDETAPE_WRITE) {
-		tape->buffer_head++;
-		tape->postpone_cnt = 0;
 		pc = idetape_next_pc_storage(drive);
 		idetape_create_write_cmd(tape, pc, rq->current_nr_sectors,
 					 (struct idetape_bh *)rq->special);
@@ -2628,18 +2559,6 @@ static void idetape_add_settings(ide_drive_t *drive)
 
 	ide_add_setting(drive, "buffer", SETTING_READ, TYPE_SHORT, 0, 0xffff,
 			1, 2, (u16 *)&tape->caps[16], NULL);
-	ide_add_setting(drive, "pipeline_min", SETTING_RW, TYPE_INT, 1, 0xffff,
-			tape->stage_size / 1024, 1, &tape->min_pipeline, NULL);
-	ide_add_setting(drive, "pipeline", SETTING_RW, TYPE_INT, 1, 0xffff,
-			tape->stage_size / 1024, 1, &tape->max_stages, NULL);
-	ide_add_setting(drive, "pipeline_max", SETTING_RW, TYPE_INT, 1,	0xffff,
-			tape->stage_size / 1024, 1, &tape->max_pipeline, NULL);
-	ide_add_setting(drive, "pipeline_used",	SETTING_READ, TYPE_INT, 0,
-			0xffff,	tape->stage_size / 1024, 1, &tape->nr_stages,
-			NULL);
-	ide_add_setting(drive, "pipeline_pending", SETTING_READ, TYPE_INT, 0,
-			0xffff, tape->stage_size / 1024, 1,
-			&tape->nr_pending_stages, NULL);
 	ide_add_setting(drive, "speed", SETTING_READ, TYPE_SHORT, 0, 0xffff,
 			1, 1, (u16 *)&tape->caps[14], NULL);
 	ide_add_setting(drive, "stage", SETTING_READ, TYPE_INT,	0, 0xffff, 1,
@@ -2649,12 +2568,6 @@ static void idetape_add_settings(ide_drive_t *drive)
 			NULL);
 	ide_add_setting(drive, "dsc_overlap", SETTING_RW, TYPE_BYTE, 0, 1, 1,
 			1, &drive->dsc_overlap, NULL);
-	ide_add_setting(drive, "pipeline_head_speed_c", SETTING_READ, TYPE_INT,
-			0, 0xffff, 1, 1, &tape->controlled_pipeline_head_speed,
-			NULL);
-	ide_add_setting(drive, "pipeline_head_speed_u", SETTING_READ, TYPE_INT,
-			0, 0xffff, 1, 1,
-			&tape->uncontrolled_pipeline_head_speed, NULL);
 	ide_add_setting(drive, "avg_speed", SETTING_READ, TYPE_INT, 0, 0xffff,
 			1, 1, &tape->avg_speed, NULL);
 	ide_add_setting(drive, "debug_mask", SETTING_RW, TYPE_INT, 0, 0xffff, 1,
@@ -2699,8 +2612,6 @@ static void idetape_setup(ide_drive_t *drive, idetape_tape_t *tape, int minor)
 	tape->name[2] = '0' + minor;
 	tape->chrdev_dir = IDETAPE_DIR_NONE;
 	tape->pc = tape->pc_stack;
-	tape->max_insert_speed = 10000;
-	tape->speed_control = 1;
 	*((unsigned short *) &gcw) = drive->id->config;
 
 	/* Command packet DRQ type */
@@ -2764,8 +2675,6 @@ static void ide_tape_release(struct kref *kref)
 	ide_drive_t *drive = tape->drive;
 	struct gendisk *g = tape->disk;
 
-	BUG_ON(tape->first_stage != NULL || tape->merge_stage_size);
-
 	drive->dsc_overlap = 0;
 	drive->driver_data = NULL;
 	device_destroy(idetape_sysfs_class, MKDEV(IDETAPE_MAJOR, tape->minor));
-- 
1.5.4.1

[PATCH 22/24] ide-tape: remove misc references to pipelined operation in the comments

[Borislav Petkov]

Signed-off-by: Borislav Petkov <petkovbb@gmail.com>
---
 drivers/ide/ide-tape.c |   15 ++++-----------
 1 files changed, 4 insertions(+), 11 deletions(-)

diff --git a/drivers/ide/ide-tape.c b/drivers/ide/ide-tape.c
index 9810253..07e08a3 100644
--- a/drivers/ide/ide-tape.c
+++ b/drivers/ide/ide-tape.c
@@ -267,9 +267,7 @@ typedef struct ide_tape_obj {
 	 * While polling for DSC we use postponed_rq to postpone the current
 	 * request so that ide.c will be able to service pending requests on the
 	 * other device. Note that at most we will have only one DSC (usually
-	 * data transfer) request in the device request queue. Additional
-	 * requests can be queued in our internal pipeline, but they will be
-	 * visible to ide.c only one at a time.
+	 * data transfer) request in the device request queue.
 	 */
 	struct request *postponed_rq;
 	/* The time in which we started polling for DSC */
@@ -309,10 +307,8 @@ typedef struct ide_tape_obj {
 	 * At most, there is only one ide-tape originated data transfer request
 	 * in the device request queue. This allows ide.c to easily service
 	 * requests from the other device when we postpone our active request.
-	 * In the pipelined operation mode, we use our internal pipeline
-	 * structure to hold more data requests. The data buffer size is chosen
-	 * based on the tape's recommendation.
 	 */
+
 	/* ptr to the request which is waiting in the device request queue */
 	struct request *active_data_rq;
 	/* Data buffer size chosen based on the tape's recommendation */
@@ -1792,8 +1788,7 @@ static int idetape_init_read(ide_drive_t *drive)
 }
 
 /*
- * Called from idetape_chrdev_read() to service a character device read request
- * and add read-ahead requests to our pipeline.
+ * Called from idetape_chrdev_read() to service a character device read request.
  */
 static int idetape_add_chrdev_read_request(ide_drive_t *drive, int blocks)
 {
@@ -2112,8 +2107,7 @@ static int idetape_write_filemark(ide_drive_t *drive)
  *
  * Note: MTBSF and MTBSFM are not supported when the tape doesn't support
  * spacing over filemarks in the reverse direction. In this case, MTFSFM is also
- * usually not supported (it is supported in the rare case in which we crossed
- * the filemark during our read-ahead pipelined operation mode).
+ * usually not supported.
  *
  * The following commands are currently not supported:
  *
@@ -2129,7 +2123,6 @@ static int idetape_mtioctop(ide_drive_t *drive, short mt_op, int mt_count)
 	debug_log(DBG_ERR, "Handling MTIOCTOP ioctl: mt_op=%d, mt_count=%d\n",
 			mt_op, mt_count);
 
-	/* Commands which need our pipelined read-ahead stages. */
 	switch (mt_op) {
 	case MTFSF:
 	case MTFSFM:
-- 
1.5.4.1

[PATCH 23/24] ide-tape: remove pipelined mode description from Documentation/ide/ide-tape.txt

[Borislav Petkov]

Signed-off-by: Borislav Petkov <petkovbb@gmail.com>
---
 Documentation/ide/ide-tape.txt |   79 ----------------------------------------
 1 files changed, 0 insertions(+), 79 deletions(-)

diff --git a/Documentation/ide/ide-tape.txt b/Documentation/ide/ide-tape.txt
index 658f271..51f596b 100644
--- a/Documentation/ide/ide-tape.txt
+++ b/Documentation/ide/ide-tape.txt
@@ -8,8 +8,6 @@
  * interface, on the other hand, creates new requests, adds them
  * to the request-list of the block device, and waits for their completion.
  *
- * Pipelined operation mode is now supported on both reads and writes.
- *
  * The block device major and minor numbers are determined from the
  * tape's relative position in the ide interfaces, as explained in ide.c.
  *
@@ -45,83 +43,6 @@
  *
  * | Special care is recommended.  Have Fun!
  *
- *
- * An overview of the pipelined operation mode.
- *
- * In the pipelined write mode, we will usually just add requests to our
- * pipeline and return immediately, before we even start to service them. The
- * user program will then have enough time to prepare the next request while
- * we are still busy servicing previous requests. In the pipelined read mode,
- * the situation is similar - we add read-ahead requests into the pipeline,
- * before the user even requested them.
- *
- * The pipeline can be viewed as a "safety net" which will be activated when
- * the system load is high and prevents the user backup program from keeping up
- * with the current tape speed. At this point, the pipeline will get
- * shorter and shorter but the tape will still be streaming at the same speed.
- * Assuming we have enough pipeline stages, the system load will hopefully
- * decrease before the pipeline is completely empty, and the backup program
- * will be able to "catch up" and refill the pipeline again.
- *
- * When using the pipelined mode, it would be best to disable any type of
- * buffering done by the user program, as ide-tape already provides all the
- * benefits in the kernel, where it can be done in a more efficient way.
- * As we will usually not block the user program on a request, the most
- * efficient user code will then be a simple read-write-read-... cycle.
- * Any additional logic will usually just slow down the backup process.
- *
- * Using the pipelined mode, I get a constant over 400 KBps throughput,
- * which seems to be the maximum throughput supported by my tape.
- *
- * However, there are some downfalls:
- *
- *	1.	We use memory (for data buffers) in proportional to the number
- *		of pipeline stages (each stage is about 26 KB with my tape).
- *	2.	In the pipelined write mode, we cheat and postpone error codes
- *		to the user task. In read mode, the actual tape position
- *		will be a bit further than the last requested block.
- *
- * Concerning (1):
- *
- *	1.	We allocate stages dynamically only when we need them. When
- *		we don't need them, we don't consume additional memory. In
- *		case we can't allocate stages, we just manage without them
- *		(at the expense of decreased throughput) so when Linux is
- *		tight in memory, we will not pose additional difficulties.
- *
- *	2.	The maximum number of stages (which is, in fact, the maximum
- *		amount of memory) which we allocate is limited by the compile
- *		time parameter IDETAPE_MAX_PIPELINE_STAGES.
- *
- *	3.	The maximum number of stages is a controlled parameter - We
- *		don't start from the user defined maximum number of stages
- *		but from the lower IDETAPE_MIN_PIPELINE_STAGES (again, we
- *		will not even allocate this amount of stages if the user
- *		program can't handle the speed). We then implement a feedback
- *		loop which checks if the pipeline is empty, and if it is, we
- *		increase the maximum number of stages as necessary until we
- *		reach the optimum value which just manages to keep the tape
- *		busy with minimum allocated memory or until we reach
- *		IDETAPE_MAX_PIPELINE_STAGES.
- *
- * Concerning (2):
- *
- *	In pipelined write mode, ide-tape can not return accurate error codes
- *	to the user program since we usually just add the request to the
- *      pipeline without waiting for it to be serviced. In case an error
- *      occurs, I will report it on the next user request.
- *
- *	In the pipelined read mode, subsequent read requests or forward
- *	filemark spacing will perform correctly, as we preserve all blocks
- *	and filemarks which we encountered during our excess read-ahead.
- *
- *	For accurate tape positioning and error reporting, disabling
- *	pipelined mode might be the best option.
- *
- * You can enable/disable/tune the pipelined operation mode by adjusting
- * the compile time parameters below.
- *
- *
  *	Possible improvements.
  *
  *	1.	Support for the ATAPI overlap protocol.
-- 
1.5.4.1

[PATCH 24/24] ide-tape: remove comments markup from Documentation/ide/ide-tape.txt

[Borislav Petkov]

Signed-off-by: Borislav Petkov <petkovbb@gmail.com>
---
 Documentation/ide/ide-tape.txt |  132 ++++++++++++++++++++--------------------
 1 files changed, 65 insertions(+), 67 deletions(-)

diff --git a/Documentation/ide/ide-tape.txt b/Documentation/ide/ide-tape.txt
index 51f596b..3f348a0 100644
--- a/Documentation/ide/ide-tape.txt
+++ b/Documentation/ide/ide-tape.txt
@@ -1,67 +1,65 @@
-/*
- * IDE ATAPI streaming tape driver.
- *
- * This driver is a part of the Linux ide driver.
- *
- * The driver, in co-operation with ide.c, basically traverses the
- * request-list for the block device interface. The character device
- * interface, on the other hand, creates new requests, adds them
- * to the request-list of the block device, and waits for their completion.
- *
- * The block device major and minor numbers are determined from the
- * tape's relative position in the ide interfaces, as explained in ide.c.
- *
- * The character device interface consists of the following devices:
- *
- * ht0		major 37, minor 0	first  IDE tape, rewind on close.
- * ht1		major 37, minor 1	second IDE tape, rewind on close.
- * ...
- * nht0		major 37, minor 128	first  IDE tape, no rewind on close.
- * nht1		major 37, minor 129	second IDE tape, no rewind on close.
- * ...
- *
- * The general magnetic tape commands compatible interface, as defined by
- * include/linux/mtio.h, is accessible through the character device.
- *
- * General ide driver configuration options, such as the interrupt-unmask
- * flag, can be configured by issuing an ioctl to the block device interface,
- * as any other ide device.
- *
- * Our own ide-tape ioctl's can be issued to either the block device or
- * the character device interface.
- *
- * Maximal throughput with minimal bus load will usually be achieved in the
- * following scenario:
- *
- *	1.	ide-tape is operating in the pipelined operation mode.
- *	2.	No buffering is performed by the user backup program.
- *
- * Testing was done with a 2 GB CONNER CTMA 4000 IDE ATAPI Streaming Tape Drive.
- *
- * Here are some words from the first releases of hd.c, which are quoted
- * in ide.c and apply here as well:
- *
- * | Special care is recommended.  Have Fun!
- *
- *	Possible improvements.
- *
- *	1.	Support for the ATAPI overlap protocol.
- *
- *		In order to maximize bus throughput, we currently use the DSC
- *		overlap method which enables ide.c to service requests from the
- *		other device while the tape is busy executing a command. The
- *		DSC overlap method involves polling the tape's status register
- *		for the DSC bit, and servicing the other device while the tape
- *		isn't ready.
- *
- *		In the current QIC development standard (December 1995),
- *		it is recommended that new tape drives will *in addition*
- *		implement the ATAPI overlap protocol, which is used for the
- *		same purpose - efficient use of the IDE bus, but is interrupt
- *		driven and thus has much less CPU overhead.
- *
- *		ATAPI overlap is likely to be supported in most new ATAPI
- *		devices, including new ATAPI cdroms, and thus provides us
- *		a method by which we can achieve higher throughput when
- *		sharing a (fast) ATA-2 disk with any (slow) new ATAPI device.
- */
+IDE ATAPI streaming tape driver.
+
+This driver is a part of the Linux ide driver.
+
+The driver, in co-operation with ide.c, basically traverses the
+request-list for the block device interface. The character device
+interface, on the other hand, creates new requests, adds them
+to the request-list of the block device, and waits for their completion.
+
+The block device major and minor numbers are determined from the
+tape's relative position in the ide interfaces, as explained in ide.c.
+
+The character device interface consists of the following devices:
+
+ht0		major 37, minor 0	first  IDE tape, rewind on close.
+ht1		major 37, minor 1	second IDE tape, rewind on close.
+...
+nht0		major 37, minor 128	first  IDE tape, no rewind on close.
+nht1		major 37, minor 129	second IDE tape, no rewind on close.
+...
+
+The general magnetic tape commands compatible interface, as defined by
+include/linux/mtio.h, is accessible through the character device.
+
+General ide driver configuration options, such as the interrupt-unmask
+flag, can be configured by issuing an ioctl to the block device interface,
+as any other ide device.
+
+Our own ide-tape ioctl's can be issued to either the block device or
+the character device interface.
+
+Maximal throughput with minimal bus load will usually be achieved in the
+following scenario:
+
+     1.	ide-tape is operating in the pipelined operation mode.
+     2.	No buffering is performed by the user backup program.
+
+Testing was done with a 2 GB CONNER CTMA 4000 IDE ATAPI Streaming Tape Drive.
+
+Here are some words from the first releases of hd.c, which are quoted
+in ide.c and apply here as well:
+
+| Special care is recommended.  Have Fun!
+
+Possible improvements:
+
+1. Support for the ATAPI overlap protocol.
+
+In order to maximize bus throughput, we currently use the DSC
+overlap method which enables ide.c to service requests from the
+other device while the tape is busy executing a command. The
+DSC overlap method involves polling the tape's status register
+for the DSC bit, and servicing the other device while the tape
+isn't ready.
+
+In the current QIC development standard (December 1995),
+it is recommended that new tape drives will *in addition*
+implement the ATAPI overlap protocol, which is used for the
+same purpose - efficient use of the IDE bus, but is interrupt
+driven and thus has much less CPU overhead.
+
+ATAPI overlap is likely to be supported in most new ATAPI
+devices, including new ATAPI cdroms, and thus provides us
+a method by which we can achieve higher throughput when
+sharing a (fast) ATA-2 disk with any (slow) new ATAPI device.
-- 
1.5.4.1

Re: [PATCH 00/24] ide-tape: remove pipelined mode operation

[Jens Axboe]

On Sat, Mar 01 2008, Borislav Petkov wrote:
> Hi Bart,
> 
> here's the 1st draft of the pipeline removal series. As the diffstat below openly
> states it, a lot of code got removed - even more than the cleanup series we did
> earlier. There are several issues that we need to address concerning these
> patches:
> 
> 1. only compile-tested since i don't have the hardware, i.e. longer -mm brewing is
> advisable the least.
> 
> 2. I have left the tape->merge_stage buffer structure along with its
> alloc/free functions intact for now, for simplicity. The next step would be
> to go and carefully audit the code and then remove that last piece
> too and use allocations on the stack instead. I guess we still expect
> Jens's response on whether blk_{get,put}_request is the way to go here.
> 
> Jens?

Hm, I have not seen any questions regarding this directed my way :-)
Please point me to the original question and I'll take a look at it.

-- 
Jens Axboe

Re: [PATCH 00/24] ide-tape: remove pipelined mode operation

[Borislav Petkov]

On Sat, Mar 01, 2008 at 10:55:18AM +0100, Jens Axboe wrote:
> On Sat, Mar 01 2008, Borislav Petkov wrote:
> > Hi Bart,
> > 
> > here's the 1st draft of the pipeline removal series. As the diffstat below openly
> > states it, a lot of code got removed - even more than the cleanup series we did
> > earlier. There are several issues that we need to address concerning these
> > patches:
> > 
> > 1. only compile-tested since i don't have the hardware, i.e. longer -mm brewing is
> > advisable the least.
> > 
> > 2. I have left the tape->merge_stage buffer structure along with its
> > alloc/free functions intact for now, for simplicity. The next step would be
> > to go and carefully audit the code and then remove that last piece
> > too and use allocations on the stack instead. I guess we still expect
> > Jens's response on whether blk_{get,put}_request is the way to go here.
> > 
> > Jens?
> 
> Hm, I have not seen any questions regarding this directed my way :-)
> Please point me to the original question and I'll take a look at it.

Hi Jens,

sorry but maybe we weren't that explicit, here's a pointer to the relevant
thread: http://www.mail-archive.com/linux-ide@vger.kernel.org/msg15541.html. It
boils down to removing the statically allocated arrays of buffers for pc and rq
structs in ide-floppy and ide-tape, and using GFP_ATOMIC stack memory instead.
Bart's idea was to even go a step further and even avoid allocation errors in
out-of-mem situations by reusing requests from the request queue but wasn't sure
whether this'll fly and wanted to run it by you...

Thanks.

-- 
Regards/Gruß,
    Boris.

Re: [PATCH 00/24] ide-tape: remove pipelined mode operation

[Jens Axboe]

On Sat, Mar 01 2008, Borislav Petkov wrote:
> On Sat, Mar 01, 2008 at 10:55:18AM +0100, Jens Axboe wrote:
> > On Sat, Mar 01 2008, Borislav Petkov wrote:
> > > Hi Bart,
> > > 
> > > here's the 1st draft of the pipeline removal series. As the diffstat below openly
> > > states it, a lot of code got removed - even more than the cleanup series we did
> > > earlier. There are several issues that we need to address concerning these
> > > patches:
> > > 
> > > 1. only compile-tested since i don't have the hardware, i.e. longer -mm brewing is
> > > advisable the least.
> > > 
> > > 2. I have left the tape->merge_stage buffer structure along with its
> > > alloc/free functions intact for now, for simplicity. The next step would be
> > > to go and carefully audit the code and then remove that last piece
> > > too and use allocations on the stack instead. I guess we still expect
> > > Jens's response on whether blk_{get,put}_request is the way to go here.
> > > 
> > > Jens?
> > 
> > Hm, I have not seen any questions regarding this directed my way :-)
> > Please point me to the original question and I'll take a look at it.
> 
> Hi Jens,
> 
> sorry but maybe we weren't that explicit, here's a pointer to the
> relevant thread:
> http://www.mail-archive.com/linux-ide@vger.kernel.org/msg15541.html.
> It boils down to removing the statically allocated arrays of buffers
> for pc and rq structs in ide-floppy and ide-tape, and using GFP_ATOMIC
> stack memory instead.  Bart's idea was to even go a step further and
> even avoid allocation errors in out-of-mem situations by reusing
> requests from the request queue but wasn't sure whether this'll fly
> and wanted to run it by you...

That sounds like asking for trouble, if you ask me (well you did :-)

And being a very rarely exercised path (if at all, you should be very
unlucky to NOT get a request even with GFP_ATOMIC), then it wont even
get tested properly. So I'd say stick to GFP_ATOMIC, and just plug the
device and retry if you get into allocation errors.

-- 
Jens Axboe

Re: [PATCH 00/24] ide-tape: remove pipelined mode operation

[Adrian Bunk]

On Sat, Mar 01, 2008 at 09:58:24AM +0100, Borislav Petkov wrote:
> Hi Bart,
> 
> here's the 1st draft of the pipeline removal series. As the diffstat below openly
> states it, a lot of code got removed - even more than the cleanup series we did
> earlier.
>...

After seeing commit d48567dd43868b3d2e1fcc33ee76dc2d38a1ddeb that 
schedules ide-tape for removal I'm wondering whether any cleanups of 
this driver make any sense at all:

What:   ide-tape driver
When:   July 2008
Files:  drivers/ide/ide-tape.c
Why:    This driver might not have any users anymore and maintaining it for no
        reason is an effort no one wants to make.
Who:    Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>, Borislav Petkov
        <petkovbb@googlemail.com>


Your patches will go in 2.6.26 and the driver will be removed
in 2.6.27 ...


cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

Re: [PATCH 00/24] ide-tape: remove pipelined mode operation

[Borislav Petkov]

On Sat, Mar 01, 2008 at 12:20:38PM +0200, Adrian Bunk wrote:
> On Sat, Mar 01, 2008 at 09:58:24AM +0100, Borislav Petkov wrote:
> > Hi Bart,
> > 
> > here's the 1st draft of the pipeline removal series. As the diffstat below openly
> > states it, a lot of code got removed - even more than the cleanup series we did
> > earlier.
> >...
> 
> After seeing commit d48567dd43868b3d2e1fcc33ee76dc2d38a1ddeb that 
> schedules ide-tape for removal I'm wondering whether any cleanups of 
> this driver make any sense at all:

Hi Adrian,

you're right and I'm expecting Bart's input on that. However, removing
tape support in ide altogether is probably not the right thing to do and
Bart wanted to keep some kind of a basic, ide-tape "light" version in so,
yes, the above commit is misleading.

Here's a fix, Bart please apply.

--
>From 11c41d7760dd0b8f4cd1ab3076c86a2c4beec4de Mon Sep 17 00:00:00 2001
From: Borislav Petkov <petkovbb@gmail.com>
Date: Sat, 1 Mar 2008 16:31:17 +0100
Subject: [PATCH] ide-tape: keep a light version in ide tree

Keep a light version of the driver in for the
small amount of ide tape hardware still using it.

Signed-off-by: Borislav Petkov <petkovbb@gmail.com>
---
 Documentation/feature-removal-schedule.txt |   10 ----------
 1 files changed, 0 insertions(+), 10 deletions(-)

diff --git a/Documentation/feature-removal-schedule.txt b/Documentation/feature-removal-schedule.txt
index ba899ff..4d3aa51 100644
--- a/Documentation/feature-removal-schedule.txt
+++ b/Documentation/feature-removal-schedule.txt
@@ -172,16 +172,6 @@ Who:	Len Brown <len.brown@intel.com>
 
 ---------------------------
 
-What:	ide-tape driver
-When:	July 2008
-Files:	drivers/ide/ide-tape.c
-Why:	This driver might not have any users anymore and maintaining it for no
-	reason is an effort no one wants to make.
-Who:	Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>, Borislav Petkov
-	<petkovbb@googlemail.com>
-
----------------------------
-
 What: libata spindown skipping and warning
 When: Dec 2008
 Why:  Some halt(8) implementations synchronize caches for and spin
-- 
1.5.4.1

-- 
Regards/Gruß,
    Boris.

Re: [PATCH 00/24] ide-tape: remove pipelined mode operation

[Bartlomiej Zolnierkiewicz]

On Saturday 01 March 2008, Borislav Petkov wrote:
> On Sat, Mar 01, 2008 at 12:20:38PM +0200, Adrian Bunk wrote:
> > On Sat, Mar 01, 2008 at 09:58:24AM +0100, Borislav Petkov wrote:
> > > Hi Bart,
> > > 
> > > here's the 1st draft of the pipeline removal series. As the diffstat below openly
> > > states it, a lot of code got removed - even more than the cleanup series we did
> > > earlier.
> > >...
> > 
> > After seeing commit d48567dd43868b3d2e1fcc33ee76dc2d38a1ddeb that 
> > schedules ide-tape for removal I'm wondering whether any cleanups of 
> > this driver make any sense at all:
> 
> Hi Adrian,
> 
> you're right and I'm expecting Bart's input on that. However, removing
> tape support in ide altogether is probably not the right thing to do and
> Bart wanted to keep some kind of a basic, ide-tape "light" version in so,
> yes, the above commit is misleading.
> 
> Here's a fix, Bart please apply.
> 
> --
> From 11c41d7760dd0b8f4cd1ab3076c86a2c4beec4de Mon Sep 17 00:00:00 2001
> From: Borislav Petkov <petkovbb@gmail.com>
> Date: Sat, 1 Mar 2008 16:31:17 +0100
> Subject: [PATCH] ide-tape: keep a light version in ide tree
> 
> Keep a light version of the driver in for the
> small amount of ide tape hardware still using it.
> 
> Signed-off-by: Borislav Petkov <petkovbb@gmail.com>
> ---
>  Documentation/feature-removal-schedule.txt |   10 ----------
>  1 files changed, 0 insertions(+), 10 deletions(-)

Since there was also an warning in the driver I reverted the original
commit instead.

>From ca4e2ab5b2764562fe3d41b95b27e6bbd4733d66 Mon Sep 17 00:00:00 2001
From: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
Date: Sat, 22 Mar 2008 16:44:27 +0100
Subject: [PATCH] Revert "ide-tape: schedule driver for removal after 6 months"

This reverts commit d48567dd43868b3d2e1fcc33ee76dc2d38a1ddeb.

Borislav is working on ide-tape "light" version instead.

Cc: Borislav Petkov <petkovbb@gmail.com>
Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
---
 Documentation/feature-removal-schedule.txt |   10 ----------
 drivers/ide/ide-tape.c                     |    5 -----
 2 files changed, 0 insertions(+), 15 deletions(-)

diff --git a/Documentation/feature-removal-schedule.txt b/Documentation/feature-removal-schedule.txt
index c1d1fd0..bf0e3df 100644
--- a/Documentation/feature-removal-schedule.txt
+++ b/Documentation/feature-removal-schedule.txt
@@ -172,16 +172,6 @@ Who:	Len Brown <len.brown@intel.com>
 
 ---------------------------
 
-What:	ide-tape driver
-When:	July 2008
-Files:	drivers/ide/ide-tape.c
-Why:	This driver might not have any users anymore and maintaining it for no
-	reason is an effort no one wants to make.
-Who:	Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>, Borislav Petkov
-	<petkovbb@googlemail.com>
-
----------------------------
-
 What: libata spindown skipping and warning
 When: Dec 2008
 Why:  Some halt(8) implementations synchronize caches for and spin
diff --git a/drivers/ide/ide-tape.c b/drivers/ide/ide-tape.c
index 43e0e05..0598ecf 100644
--- a/drivers/ide/ide-tape.c
+++ b/drivers/ide/ide-tape.c
@@ -3765,11 +3765,6 @@ static int ide_tape_probe(ide_drive_t *drive)
 	g->fops = &idetape_block_ops;
 	ide_register_region(g);
 
-	printk(KERN_WARNING "It is possible that this driver does not have any"
-		" users anymore and, as a result, it will be REMOVED soon."
-		" Please notify Bart <bzolnier@gmail.com> or Boris"
-		" <petkovbb@gmail.com> in case you still need it.\n");
-
 	return 0;
 
 out_free_tape:
-- 
1.5.3.3