IDE TCQ #4

Quoted 3 lines · Jens Axboe

Quoted 2 lines

NULL pointer ...

Quoted 2 lines

Could you decode that? It doesn't look like any of your drives support TCQ, it should have enabled them right here:

They were already decoded... Also others reported that - after accessing /proc/ide/ide0/hda/identify system dies... I believe that passing hand-created request to ide_raw_taskfile corrupts drive->free_req, and so subsequent drive command after this cat finds that drive->free_req.next is NULL and dies.

Quoted 4 lines · Petr Vandrovec

Quoted 3 lines · Jens Axboe

Quoted 2 lines

NULL pointer ...

Quoted 8 lines

Quoted 2 lines

Could you decode that? It doesn't look like any of your drives support TCQ, it should have enabled them right here:

They were already decoded... Also others reported that - after accessing /proc/ide/ide0/hda/identify system dies... I believe that passing hand-created request to ide_raw_taskfile corrupts drive->free_req, and so subsequent drive command after this cat finds that drive->free_req.next is NULL and dies.

ide_raw_taskfile() sets rq.special to &ar - and &ar is on the stack, in this function. Later it falls through to __ide_end_request(), which does

  ar = rq->special;
  ...
  if (ar)
    ata_ar_put(drive, ar);

which adds this ar into drive's free_req chain unconditionally. Maybe ata_ar_put should check for ar_queue validity. And where ar_queue member is initialized (or at least cleared) in this case at all?

Unfortunately here my knowledge ends.

Dear Mailing List,

I've discovered that even if I insmod (or modprobe) the proper USB modules for my 2.4.16 kernel, and if I use the USB mouse afterwards, "lsmod" reveals that the modules seem to be "(0) unused" - despite the USB mouse is in action!

Users of the 2.4.18-kernel affirmed same kernel behaviour.

If I rmmod the USB modules, they subsequently allow to be removed from kernelspace, and the USB mouse cursor , ofcourse , stops instantly - despite he was just in use.

Is this a bug or a feature? :-)

yes this looks like a silly problem. the fix should be to have ata_ar_get() set ATA_AR_RETURN in ar_flags:

        if (!list_empty(&drive->free_req)) {
                ar = list_ata_entry(drive->free_req.next);
                list_del(&ar->ar_queue);
                ata_ar_init(drive, ar);
                ar->ar_flags |= ATA_AR_RETURN;
        }

and then only have ata_ar_put() readd it to the list when it is set:

static inline void ata_ar_put(ide_drive_t *drive, struct ata_request *ar)

{
        if (ar->ar_flags & ATA_AR_RETURN)
                list_add(&ar->ar_queue, &drive->free_req);
	...

Then you can also remove the ata_ar_put() conditional in ide_end_drive_cmd(), just call ata_ar_put() unconditionally.

Quoted 19 lines · Jens Axboe

yes this looks like a silly problem. the fix should be to have ata_ar_get() set ATA_AR_RETURN in ar_flags:

       if (!list_empty(&drive->free_req)) {
               ar = list_ata_entry(drive->free_req.next);
               list_del(&ar->ar_queue);
               ata_ar_init(drive, ar);
               ar->ar_flags |= ATA_AR_RETURN;
       }

and then only have ata_ar_put() readd it to the list when it is set:

static inline void ata_ar_put(ide_drive_t *drive, struct ata_request *ar)

{
       if (ar->ar_flags & ATA_AR_RETURN)
               list_add(&ar->ar_queue, &drive->free_req);
	...

Quoted 2 lines

Then you can also remove the ata_ar_put() conditional in ide_end_drive_cmd(), just call ata_ar_put() unconditionally.

Well something similar is already in IDE 37... I have just invented a flag ATA_AR_STATIC which get's set in ide_raw_taskfile ata_ar_put ich then checking for if (!(ar->ar_flags & ATA_AR_STATIC))...

It has the desired effect in practice.

Quoted 30 lines · Martin Dalecki

Quoted 14 lines · Jens Axboe

yes this looks like a silly problem. the fix should be to have ata_ar_get() set ATA_AR_RETURN in ar_flags:

      if (!list_empty(&drive->free_req)) {
              ar = list_ata_entry(drive->free_req.next);
              list_del(&ar->ar_queue);
              ata_ar_init(drive, ar);
              ar->ar_flags |= ATA_AR_RETURN;
      }

and then only have ata_ar_put() readd it to the list when it is set:

static inline void ata_ar_put(ide_drive_t *drive, struct ata_request *ar)

Quoted 7 lines

{
      if (ar->ar_flags & ATA_AR_RETURN)
              list_add(&ar->ar_queue, &drive->free_req);
	...

Then you can also remove the ata_ar_put() conditional in ide_end_drive_cmd(), just call ata_ar_put() unconditionally.

Well something similar is already in IDE 37... I have just invented a flag ATA_AR_STATIC which get's set in ide_raw_taskfile ata_ar_put ich then checking for if (!(ar->ar_flags & ATA_AR_STATIC))...

Quoted 1 line

It has the desired effect in practice.

sure, just used ATA_AR_RETURN since it was there already. I'm not particularly fond of that name though, and ATA_AR_STATIC isn't too good either imo. how about ATA_AR_POOL? with the same semantics as ATA_AR_RETURN, ie return to pool if flag is set.