[PATCH 1/2] ide-tape: change IDE_AFLAG_IGNORE_DSC non-atomically

[PATCH 1/2] ide-tape: change IDE_AFLAG_IGNORE_DSC non-atomically

[Borislav Petkov]

There are two sites where the flag is being changed: ide_retry_pc
and idetape_do_request. Both codepaths are protected by hwif->busy
(ide_lock_port) and therefore we shouldn't need the atomic accesses. The
only problem would be the compiler reordering the accesses, therefore the
optimization barrier.

Spotted-by: Jiri Slaby <jirislaby@gmail.com>
Signed-off-by: Borislav Petkov <petkovbb@gmail.com>
---
 drivers/ide/ide-atapi.c |    2 +-
 drivers/ide/ide-tape.c  |   21 ++++++++++++++++-----
 2 files changed, 17 insertions(+), 6 deletions(-)

diff --git a/drivers/ide/ide-atapi.c b/drivers/ide/ide-atapi.c
index afe5a43..fbcb851 100644
--- a/drivers/ide/ide-atapi.c
+++ b/drivers/ide/ide-atapi.c
@@ -258,7 +258,7 @@ void ide_retry_pc(ide_drive_t *drive)
 	pc->req_xfer = sense_rq->data_len;
 
 	if (drive->media == ide_tape)
-		set_bit(IDE_AFLAG_IGNORE_DSC, &drive->atapi_flags);
+		drive->atapi_flags |= IDE_AFLAG_IGNORE_DSC;
 
 	if (ide_queue_sense_rq(drive, pc))
 		ide_complete_rq(drive, -EIO, blk_rq_bytes(drive->hwif->rq));
diff --git a/drivers/ide/ide-tape.c b/drivers/ide/ide-tape.c
index 203bbea..4ff50cc 100644
--- a/drivers/ide/ide-tape.c
+++ b/drivers/ide/ide-tape.c
@@ -656,15 +656,24 @@ static ide_startstop_t idetape_do_request(ide_drive_t *drive,
 
 	if ((drive->dev_flags & IDE_DFLAG_DSC_OVERLAP) == 0 &&
 	    (rq->cmd[13] & REQ_IDETAPE_PC2) == 0)
-		set_bit(IDE_AFLAG_IGNORE_DSC, &drive->atapi_flags);
+		drive->atapi_flags |= IDE_AFLAG_IGNORE_DSC;
 
 	if (drive->dev_flags & IDE_DFLAG_POST_RESET) {
-		set_bit(IDE_AFLAG_IGNORE_DSC, &drive->atapi_flags);
+		drive->atapi_flags |= IDE_AFLAG_IGNORE_DSC;
 		drive->dev_flags &= ~IDE_DFLAG_POST_RESET;
 	}
 
-	if (!test_and_clear_bit(IDE_AFLAG_IGNORE_DSC, &drive->atapi_flags) &&
-	    (stat & ATA_DSC) == 0) {
+	/*
+	 * This is a precaution for IDE_AFLAG_IGNORE_DSC being conditionally set
+	 * above. We don't need a stronger enforcement of ordering because the
+	 * read below cannot precede the earlier write out-of-order since it is
+	 * to the same location. Also, since we have the ide port locked during
+	 * the ->do_request(), we only have to be aware of gcc reordering stuff.
+	 */
+	barrier();
+
+	if (!(drive->atapi_flags & IDE_AFLAG_IGNORE_DSC) &&
+	    !(stat & ATA_DSC)) {
 		if (postponed_rq == NULL) {
 			tape->dsc_polling_start = jiffies;
 			tape->dsc_poll_freq = tape->best_dsc_rw_freq;
@@ -684,7 +693,9 @@ static ide_startstop_t idetape_do_request(ide_drive_t *drive,
 			tape->dsc_poll_freq = IDETAPE_DSC_MA_SLOW;
 		idetape_postpone_request(drive);
 		return ide_stopped;
-	}
+	} else
+		drive->atapi_flags &= ~IDE_AFLAG_IGNORE_DSC;
+
 	if (rq->cmd[13] & REQ_IDETAPE_READ) {
 		pc = &tape->queued_pc;
 		ide_tape_create_rw_cmd(tape, pc, rq, READ_6);
-- 
1.6.3.1

Re: [PATCH 1/2] ide-tape: change IDE_AFLAG_IGNORE_DSC non-atomically

[Bartlomiej Zolnierkiewicz]

Hi,

On Tuesday 02 June 2009 09:05:07 Borislav Petkov wrote:
> There are two sites where the flag is being changed: ide_retry_pc
> and idetape_do_request. Both codepaths are protected by hwif->busy
> (ide_lock_port) and therefore we shouldn't need the atomic accesses. The
> only problem would be the compiler reordering the accesses, therefore the
> optimization barrier.
> 
> Spotted-by: Jiri Slaby <jirislaby@gmail.com>
> Signed-off-by: Borislav Petkov <petkovbb@gmail.com>

[...]

> --- a/drivers/ide/ide-tape.c
> +++ b/drivers/ide/ide-tape.c
> @@ -656,15 +656,24 @@ static ide_startstop_t idetape_do_request(ide_drive_t *drive,
>  
>  	if ((drive->dev_flags & IDE_DFLAG_DSC_OVERLAP) == 0 &&
>  	    (rq->cmd[13] & REQ_IDETAPE_PC2) == 0)
> -		set_bit(IDE_AFLAG_IGNORE_DSC, &drive->atapi_flags);
> +		drive->atapi_flags |= IDE_AFLAG_IGNORE_DSC;
>  
>  	if (drive->dev_flags & IDE_DFLAG_POST_RESET) {
> -		set_bit(IDE_AFLAG_IGNORE_DSC, &drive->atapi_flags);
> +		drive->atapi_flags |= IDE_AFLAG_IGNORE_DSC;
>  		drive->dev_flags &= ~IDE_DFLAG_POST_RESET;
>  	}
>  
> -	if (!test_and_clear_bit(IDE_AFLAG_IGNORE_DSC, &drive->atapi_flags) &&
> -	    (stat & ATA_DSC) == 0) {
> +	/*
> +	 * This is a precaution for IDE_AFLAG_IGNORE_DSC being conditionally set
> +	 * above. We don't need a stronger enforcement of ordering because the
> +	 * read below cannot precede the earlier write out-of-order since it is
> +	 * to the same location. Also, since we have the ide port locked during
> +	 * the ->do_request(), we only have to be aware of gcc reordering stuff.
> +	 */
> +	barrier();

Are you seeing a real problem with gcc here?  No sane compiler should need
a barrier() here (we would probably need zillions of them in kernel if it
really does).

Re: [PATCH 1/2] ide-tape: change IDE_AFLAG_IGNORE_DSC non-atomically

[Borislav Petkov]

Hi,

On Tue, Jun 2, 2009 at 12:18 PM, Bartlomiej Zolnierkiewicz
<bzolnier@gmail.com> wrote:

..

>> --- a/drivers/ide/ide-tape.c
>> +++ b/drivers/ide/ide-tape.c
>> @@ -656,15 +656,24 @@ static ide_startstop_t idetape_do_request(ide_drive_t *drive,
>>
>>       if ((drive->dev_flags & IDE_DFLAG_DSC_OVERLAP) == 0 &&
>>           (rq->cmd[13] & REQ_IDETAPE_PC2) == 0)
>> -             set_bit(IDE_AFLAG_IGNORE_DSC, &drive->atapi_flags);
>> +             drive->atapi_flags |= IDE_AFLAG_IGNORE_DSC;
>>
>>       if (drive->dev_flags & IDE_DFLAG_POST_RESET) {
>> -             set_bit(IDE_AFLAG_IGNORE_DSC, &drive->atapi_flags);
>> +             drive->atapi_flags |= IDE_AFLAG_IGNORE_DSC;
>>               drive->dev_flags &= ~IDE_DFLAG_POST_RESET;
>>       }
>>
>> -     if (!test_and_clear_bit(IDE_AFLAG_IGNORE_DSC, &drive->atapi_flags) &&
>> -         (stat & ATA_DSC) == 0) {
>> +     /*
>> +      * This is a precaution for IDE_AFLAG_IGNORE_DSC being conditionally set
>> +      * above. We don't need a stronger enforcement of ordering because the
>> +      * read below cannot precede the earlier write out-of-order since it is
>> +      * to the same location. Also, since we have the ide port locked during
>> +      * the ->do_request(), we only have to be aware of gcc reordering stuff.
>> +      */
>> +     barrier();
>
> Are you seeing a real problem with gcc here?  No sane compiler should need
> a barrier() here (we would probably need zillions of them in kernel if it
> really does).

No, this is just a precaution. The asm I checked looked fine but since
the flag is set and right afterwards checked, it will be bad if this
somehow got reordered. I actually haven't checked whether anything like
that would be possible, at all.

-- 
Regards/Gruss,
Boris

Re: [PATCH 1/2] ide-tape: change IDE_AFLAG_IGNORE_DSC non-atomically

[Bartlomiej Zolnierkiewicz]

On Tuesday 02 June 2009 15:08:27 Borislav Petkov wrote:
> Hi,
> 
> On Tue, Jun 2, 2009 at 12:18 PM, Bartlomiej Zolnierkiewicz
> <bzolnier@gmail.com> wrote:
> 
> ..
> 
> >> --- a/drivers/ide/ide-tape.c
> >> +++ b/drivers/ide/ide-tape.c
> >> @@ -656,15 +656,24 @@ static ide_startstop_t idetape_do_request(ide_drive_t *drive,
> >>
> >>       if ((drive->dev_flags & IDE_DFLAG_DSC_OVERLAP) == 0 &&
> >>           (rq->cmd[13] & REQ_IDETAPE_PC2) == 0)
> >> -             set_bit(IDE_AFLAG_IGNORE_DSC, &drive->atapi_flags);
> >> +             drive->atapi_flags |= IDE_AFLAG_IGNORE_DSC;
> >>
> >>       if (drive->dev_flags & IDE_DFLAG_POST_RESET) {
> >> -             set_bit(IDE_AFLAG_IGNORE_DSC, &drive->atapi_flags);
> >> +             drive->atapi_flags |= IDE_AFLAG_IGNORE_DSC;
> >>               drive->dev_flags &= ~IDE_DFLAG_POST_RESET;
> >>       }
> >>
> >> -     if (!test_and_clear_bit(IDE_AFLAG_IGNORE_DSC, &drive->atapi_flags) &&
> >> -         (stat & ATA_DSC) == 0) {
> >> +     /*
> >> +      * This is a precaution for IDE_AFLAG_IGNORE_DSC being conditionally set
> >> +      * above. We don't need a stronger enforcement of ordering because the
> >> +      * read below cannot precede the earlier write out-of-order since it is
> >> +      * to the same location. Also, since we have the ide port locked during
> >> +      * the ->do_request(), we only have to be aware of gcc reordering stuff.
> >> +      */
> >> +     barrier();
> >
> > Are you seeing a real problem with gcc here?  No sane compiler should need
> > a barrier() here (we would probably need zillions of them in kernel if it
> > really does).
> 
> No, this is just a precaution. The asm I checked looked fine but since
> the flag is set and right afterwards checked, it will be bad if this
> somehow got reordered. I actually haven't checked whether anything like
> that would be possible, at all.

Please remove it then.

Re: [PATCH 1/2] ide-tape: change IDE_AFLAG_IGNORE_DSC non-atomically

[Borislav Petkov]

Hi,

> > > Are you seeing a real problem with gcc here?  No sane compiler should need
> > > a barrier() here (we would probably need zillions of them in kernel if it
> > > really does).
> > 
> > No, this is just a precaution. The asm I checked looked fine but since
> > the flag is set and right afterwards checked, it will be bad if this
> > somehow got reordered. I actually haven't checked whether anything like
> > that would be possible, at all.
> 
> Please remove it then.

---

From: Borislav Petkov <petkovbb@gmail.com>
Date: Mon, 1 Jun 2009 13:43:49 +0200
Subject: [PATCH 1/2] ide-tape: change IDE_AFLAG_IGNORE_DSC non-atomically

There are two sites where the flag is being changed: ide_retry_pc
and idetape_do_request. Both codepaths are protected by hwif->busy
(ide_lock_port) and therefore we shouldn't need the atomic accesses.

Spotted-by: Jiri Slaby <jirislaby@gmail.com>
Signed-off-by: Borislav Petkov <petkovbb@gmail.com>
---
 drivers/ide/ide-atapi.c |    2 +-
 drivers/ide/ide-tape.c  |   12 +++++++-----
 2 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/drivers/ide/ide-atapi.c b/drivers/ide/ide-atapi.c
index afe5a43..fbcb851 100644
--- a/drivers/ide/ide-atapi.c
+++ b/drivers/ide/ide-atapi.c
@@ -258,7 +258,7 @@ void ide_retry_pc(ide_drive_t *drive)
 	pc->req_xfer = sense_rq->data_len;
 
 	if (drive->media == ide_tape)
-		set_bit(IDE_AFLAG_IGNORE_DSC, &drive->atapi_flags);
+		drive->atapi_flags |= IDE_AFLAG_IGNORE_DSC;
 
 	if (ide_queue_sense_rq(drive, pc))
 		ide_complete_rq(drive, -EIO, blk_rq_bytes(drive->hwif->rq));
diff --git a/drivers/ide/ide-tape.c b/drivers/ide/ide-tape.c
index 203bbea..f1d3c7b 100644
--- a/drivers/ide/ide-tape.c
+++ b/drivers/ide/ide-tape.c
@@ -656,15 +656,15 @@ static ide_startstop_t idetape_do_request(ide_drive_t *drive,
 
 	if ((drive->dev_flags & IDE_DFLAG_DSC_OVERLAP) == 0 &&
 	    (rq->cmd[13] & REQ_IDETAPE_PC2) == 0)
-		set_bit(IDE_AFLAG_IGNORE_DSC, &drive->atapi_flags);
+		drive->atapi_flags |= IDE_AFLAG_IGNORE_DSC;
 
 	if (drive->dev_flags & IDE_DFLAG_POST_RESET) {
-		set_bit(IDE_AFLAG_IGNORE_DSC, &drive->atapi_flags);
+		drive->atapi_flags |= IDE_AFLAG_IGNORE_DSC;
 		drive->dev_flags &= ~IDE_DFLAG_POST_RESET;
 	}
 
-	if (!test_and_clear_bit(IDE_AFLAG_IGNORE_DSC, &drive->atapi_flags) &&
-	    (stat & ATA_DSC) == 0) {
+	if (!(drive->atapi_flags & IDE_AFLAG_IGNORE_DSC) &&
+	    !(stat & ATA_DSC)) {
 		if (postponed_rq == NULL) {
 			tape->dsc_polling_start = jiffies;
 			tape->dsc_poll_freq = tape->best_dsc_rw_freq;
@@ -684,7 +684,9 @@ static ide_startstop_t idetape_do_request(ide_drive_t *drive,
 			tape->dsc_poll_freq = IDETAPE_DSC_MA_SLOW;
 		idetape_postpone_request(drive);
 		return ide_stopped;
-	}
+	} else
+		drive->atapi_flags &= ~IDE_AFLAG_IGNORE_DSC;
+
 	if (rq->cmd[13] & REQ_IDETAPE_READ) {
 		pc = &tape->queued_pc;
 		ide_tape_create_rw_cmd(tape, pc, rq, READ_6);
-- 
1.6.3.1


-- 
Regards/Gruss,
    Boris.

[PATCH 2/2] ide-tape: fix IDE_AFLAG_* atomic accesses

[Borislav Petkov]

These flags used to be bit numbers and now are single bits in the
->atapi_flags vector. Use them properly.

Spotted-by: Jiri Slaby <jirislaby@gmail.com>
Signed-off-by: Borislav Petkov <petkovbb@gmail.com>
---
 drivers/ide/ide-tape.c |   43 ++++++++++++++++++++++++++-----------------
 1 files changed, 26 insertions(+), 17 deletions(-)

diff --git a/drivers/ide/ide-tape.c b/drivers/ide/ide-tape.c
index 4ff50cc..a305a88 100644
--- a/drivers/ide/ide-tape.c
+++ b/drivers/ide/ide-tape.c
@@ -397,7 +397,8 @@ static int ide_tape_callback(ide_drive_t *drive, int dsc)
 		if (readpos[0] & 0x4) {
 			printk(KERN_INFO "ide-tape: Block location is unknown"
 					 "to the tape\n");
-			clear_bit(IDE_AFLAG_ADDRESS_VALID, &drive->atapi_flags);
+			clear_bit(ilog2(IDE_AFLAG_ADDRESS_VALID),
+				  &drive->atapi_flags);
 			uptodate = 0;
 			err = IDE_DRV_ERROR_GENERAL;
 		} else {
@@ -406,7 +407,8 @@ static int ide_tape_callback(ide_drive_t *drive, int dsc)
 
 			tape->partition = readpos[1];
 			tape->first_frame = be32_to_cpup((__be32 *)&readpos[4]);
-			set_bit(IDE_AFLAG_ADDRESS_VALID, &drive->atapi_flags);
+			set_bit(ilog2(IDE_AFLAG_ADDRESS_VALID),
+				&drive->atapi_flags);
 		}
 	}
 
@@ -755,7 +757,7 @@ static int idetape_wait_ready(ide_drive_t *drive, unsigned long timeout)
 	int load_attempted = 0;
 
 	/* Wait for the tape to become ready */
-	set_bit(IDE_AFLAG_MEDIUM_PRESENT, &drive->atapi_flags);
+	set_bit(ilog2(IDE_AFLAG_MEDIUM_PRESENT), &drive->atapi_flags);
 	timeout += jiffies;
 	while (time_before(jiffies, timeout)) {
 		if (ide_do_test_unit_ready(drive, disk) == 0)
@@ -831,7 +833,7 @@ static void __ide_tape_discard_merge_buffer(ide_drive_t *drive)
 	if (tape->chrdev_dir != IDETAPE_DIR_READ)
 		return;
 
-	clear_bit(IDE_AFLAG_FILEMARK, &drive->atapi_flags);
+	clear_bit(ilog2(IDE_AFLAG_FILEMARK), &drive->atapi_flags);
 	tape->valid = 0;
 	if (tape->buf != NULL) {
 		kfree(tape->buf);
@@ -1124,7 +1126,8 @@ static int idetape_space_over_filemarks(ide_drive_t *drive, short mt_op,
 
 	if (tape->chrdev_dir == IDETAPE_DIR_READ) {
 		tape->valid = 0;
-		if (test_and_clear_bit(IDE_AFLAG_FILEMARK, &drive->atapi_flags))
+		if (test_and_clear_bit(ilog2(IDE_AFLAG_FILEMARK),
+				       &drive->atapi_flags))
 			++count;
 		ide_tape_discard_merge_buffer(drive, 0);
 	}
@@ -1179,7 +1182,7 @@ 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(IDE_AFLAG_DETECT_BS, &drive->atapi_flags))
+		if (test_bit(ilog2(IDE_AFLAG_DETECT_BS), &drive->atapi_flags))
 			if (count > tape->blk_size &&
 			    (count % tape->blk_size) == 0)
 				tape->user_bs_factor = count / tape->blk_size;
@@ -1195,7 +1198,8 @@ static ssize_t idetape_chrdev_read(struct file *file, char __user *buf,
 		/* refill if staging buffer is empty */
 		if (!tape->valid) {
 			/* If we are at a filemark, nothing more to read */
-			if (test_bit(IDE_AFLAG_FILEMARK, &drive->atapi_flags))
+			if (test_bit(ilog2(IDE_AFLAG_FILEMARK),
+				     &drive->atapi_flags))
 				break;
 			/* read */
 			if (idetape_queue_rw_tail(drive, REQ_IDETAPE_READ,
@@ -1213,7 +1217,7 @@ static ssize_t idetape_chrdev_read(struct file *file, char __user *buf,
 		done += todo;
 	}
 
-	if (!done && test_bit(IDE_AFLAG_FILEMARK, &drive->atapi_flags)) {
+	if (!done && test_bit(ilog2(IDE_AFLAG_FILEMARK), &drive->atapi_flags)) {
 		debug_log(DBG_SENSE, "%s: spacing over filemark\n", tape->name);
 
 		idetape_space_over_filemarks(drive, MTFSF, 1);
@@ -1347,7 +1351,8 @@ static int idetape_mtioctop(ide_drive_t *drive, short mt_op, int mt_count)
 		ide_tape_discard_merge_buffer(drive, 0);
 		retval = ide_do_start_stop(drive, disk, !IDETAPE_LU_LOAD_MASK);
 		if (!retval)
-			clear_bit(IDE_AFLAG_MEDIUM_PRESENT, &drive->atapi_flags);
+			clear_bit(ilog2(IDE_AFLAG_MEDIUM_PRESENT),
+				  &drive->atapi_flags);
 		return retval;
 	case MTNOP:
 		ide_tape_discard_merge_buffer(drive, 0);
@@ -1369,9 +1374,11 @@ static int idetape_mtioctop(ide_drive_t *drive, short mt_op, int mt_count)
 			    mt_count % tape->blk_size)
 				return -EIO;
 			tape->user_bs_factor = mt_count / tape->blk_size;
-			clear_bit(IDE_AFLAG_DETECT_BS, &drive->atapi_flags);
+			clear_bit(ilog2(IDE_AFLAG_DETECT_BS),
+				  &drive->atapi_flags);
 		} else
-			set_bit(IDE_AFLAG_DETECT_BS, &drive->atapi_flags);
+			set_bit(ilog2(IDE_AFLAG_DETECT_BS),
+				&drive->atapi_flags);
 		return 0;
 	case MTSEEK:
 		ide_tape_discard_merge_buffer(drive, 0);
@@ -1516,20 +1523,20 @@ static int idetape_chrdev_open(struct inode *inode, struct file *filp)
 
 	filp->private_data = tape;
 
-	if (test_and_set_bit(IDE_AFLAG_BUSY, &drive->atapi_flags)) {
+	if (test_and_set_bit(ilog2(IDE_AFLAG_BUSY), &drive->atapi_flags)) {
 		retval = -EBUSY;
 		goto out_put_tape;
 	}
 
 	retval = idetape_wait_ready(drive, 60 * HZ);
 	if (retval) {
-		clear_bit(IDE_AFLAG_BUSY, &drive->atapi_flags);
+		clear_bit(ilog2(IDE_AFLAG_BUSY), &drive->atapi_flags);
 		printk(KERN_ERR "ide-tape: %s: drive not ready\n", tape->name);
 		goto out_put_tape;
 	}
 
 	idetape_read_position(drive);
-	if (!test_bit(IDE_AFLAG_ADDRESS_VALID, &drive->atapi_flags))
+	if (!test_bit(ilog2(IDE_AFLAG_ADDRESS_VALID), &drive->atapi_flags))
 		(void)idetape_rewind_tape(drive);
 
 	/* Read block size and write protect status from drive. */
@@ -1545,7 +1552,7 @@ static int idetape_chrdev_open(struct inode *inode, struct file *filp)
 	if (tape->write_prot) {
 		if ((filp->f_flags & O_ACCMODE) == O_WRONLY ||
 		    (filp->f_flags & O_ACCMODE) == O_RDWR) {
-			clear_bit(IDE_AFLAG_BUSY, &drive->atapi_flags);
+			clear_bit(ilog2(IDE_AFLAG_BUSY), &drive->atapi_flags);
 			retval = -EROFS;
 			goto out_put_tape;
 		}
@@ -1602,15 +1609,17 @@ static int idetape_chrdev_release(struct inode *inode, struct file *filp)
 			ide_tape_discard_merge_buffer(drive, 1);
 	}
 
-	if (minor < 128 && test_bit(IDE_AFLAG_MEDIUM_PRESENT, &drive->atapi_flags))
+	if (minor < 128 && test_bit(ilog2(IDE_AFLAG_MEDIUM_PRESENT),
+				    &drive->atapi_flags))
 		(void) idetape_rewind_tape(drive);
+
 	if (tape->chrdev_dir == IDETAPE_DIR_NONE) {
 		if (tape->door_locked == DOOR_LOCKED) {
 			if (!ide_set_media_lock(drive, tape->disk, 0))
 				tape->door_locked = DOOR_UNLOCKED;
 		}
 	}
-	clear_bit(IDE_AFLAG_BUSY, &drive->atapi_flags);
+	clear_bit(ilog2(IDE_AFLAG_BUSY), &drive->atapi_flags);
 	ide_tape_put(tape);
 	unlock_kernel();
 	return 0;
-- 
1.6.3.1

Re: [PATCH 0/2] ide-tape: fix flags

[Bartlomiej Zolnierkiewicz]

On Tuesday 02 June 2009 09:05:06 Borislav Petkov wrote:
> Hi Bart,
> 
> those fix a stupid flags misuse that got spotted by Jiri Slaby. Please
> pay special attention to the first patch. FWIW, those can go as hot
> fixes right away and we might catch 30-rc8 since it's not out yet.

The problem is that patches seem to be against ide-2.6.git/for-next...

Re: [PATCH 0/2] ide-tape: fix flags

[Borislav Petkov]

On Fri, Jun 05, 2009 at 08:34:06PM +0200, Bartlomiej Zolnierkiewicz wrote:
> On Tuesday 02 June 2009 09:05:06 Borislav Petkov wrote:
> > Hi Bart,
> > 
> > those fix a stupid flags misuse that got spotted by Jiri Slaby. Please
> > pay special attention to the first patch. FWIW, those can go as hot
> > fixes right away and we might catch 30-rc8 since it's not out yet.
> 
> The problem is that patches seem to be against ide-2.6.git/for-next...

Sh*t, this is hitting the broken buffer allocation that Tejun removed
(I'm getting the same OOM-kill as the one Tejun's mentioning here
http://marc.info/?l=linux-ide&m=124191711928890) so even with those
patches, ide-tape is still broken.

I'm guessing backport all of Tejun's ide-tape cleanup stuff after Linus
pulls them into 31-rc1?

-- 
Regards/Gruss,
    Boris.

Re: [PATCH 0/2] ide-tape: fix flags

[Bartlomiej Zolnierkiewicz]

[ added Jiri to cc: ]

On Saturday 06 June 2009 19:58:36 Borislav Petkov wrote:
> On Fri, Jun 05, 2009 at 08:34:06PM +0200, Bartlomiej Zolnierkiewicz wrote:
> > On Tuesday 02 June 2009 09:05:06 Borislav Petkov wrote:
> > > Hi Bart,
> > > 
> > > those fix a stupid flags misuse that got spotted by Jiri Slaby. Please
> > > pay special attention to the first patch. FWIW, those can go as hot
> > > fixes right away and we might catch 30-rc8 since it's not out yet.
> > 
> > The problem is that patches seem to be against ide-2.6.git/for-next...
> 
> Sh*t, this is hitting the broken buffer allocation that Tejun removed
> (I'm getting the same OOM-kill as the one Tejun's mentioning here
> http://marc.info/?l=linux-ide&m=124191711928890) so even with those
> patches, ide-tape is still broken.
> 
> I'm guessing backport all of Tejun's ide-tape cleanup stuff after Linus
> pulls them into 31-rc1?

Seems so, though it may be too much work for too little gain
(we still have other open ide-tape issues like DMA related one).

In the meantime I applied your patches to for-next..