2009-06-02 07:05:25

by Borislav Petkov

[permalink] [raw]
Subject: [PATCH 0/2] ide-tape: fix flags

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.

Thanks,
Boris.

drivers/ide/ide-atapi.c | 2 +-
drivers/ide/ide-tape.c | 64 ++++++++++++++++++++++++++++++----------------
2 files changed, 43 insertions(+), 23 deletions(-)


2009-06-02 07:05:38

by Borislav Petkov

[permalink] [raw]
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. The
only problem would be the compiler reordering the accesses, therefore the
optimization barrier.

Spotted-by: Jiri Slaby <[email protected]>
Signed-off-by: Borislav Petkov <[email protected]>
---
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

2009-06-02 07:05:48

by Borislav Petkov

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

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

Spotted-by: Jiri Slaby <[email protected]>
Signed-off-by: Borislav Petkov <[email protected]>
---
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

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


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 <[email protected]>
> Signed-off-by: Borislav Petkov <[email protected]>

[...]

> --- 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).

2009-06-02 13:08:35

by Borislav Petkov

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

Hi,

On Tue, Jun 2, 2009 at 12:18 PM, Bartlomiej Zolnierkiewicz
<[email protected]> 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

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

On Tuesday 02 June 2009 15:08:27 Borislav Petkov wrote:
> Hi,
>
> On Tue, Jun 2, 2009 at 12:18 PM, Bartlomiej Zolnierkiewicz
> <[email protected]> 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.

2009-06-03 05:39:29

by Borislav Petkov

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

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 <[email protected]>
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 <[email protected]>
Signed-off-by: Borislav Petkov <[email protected]>
---
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.

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

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...

2009-06-06 17:58:52

by Borislav Petkov

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

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.

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


[ 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..