Tejun Heo wrote: > Albert Lee wrote: > >> This patch makes qc->tf well preserved, except we still have >> qc->tf got overwritten by ata_pio_bytes(). >> >> Maybe we also need the attached follow-up patch? >> >> -- >> albert >> >> --- 02_atapi_pio_bytes/drivers/scsi/libata-core.c 2006-05-18 >> 14:20:32.000000000 +0800 >> +++ 03_atapi_pio_bytes/drivers/scsi/libata-core.c 2006-05-18 >> 14:25:41.000000000 +0800 >> @@ -3861,10 +3861,10 @@ static void atapi_pio_bytes(struct ata_q >> unsigned int ireason, bc_lo, bc_hi, bytes; >> int i_write, do_write = (qc->tf.flags & ATA_TFLAG_WRITE) ? 1 : 0; >> >> - ap->ops->tf_read(ap, &qc->tf); >> - ireason = qc->tf.nsect; >> - bc_lo = qc->tf.lbam; >> - bc_hi = qc->tf.lbah; >> + ap->ops->tf_read(ap, &qc->result_tf); >> + ireason = qc->result_tf.nsect; >> + bc_lo = qc->result_tf.lbam; >> + bc_hi = qc->result_tf.lbah; >> bytes = (bc_hi << 8) | bc_lo; >> >> /* shall be cleared to zero, indicating xfer of data */ >> > > I actually think above should be done with local variable. It's not > comannd TF nor result TF. > You're right. But local variable adds 18+ extra bytes to the kernel stack. And - For normal completion, qc->result_tf is not relevant. - For error, qc->result_tf is later overwritten by ata_qc_complete(). Maybe we can use qc->result_tf for temp storage of intermediate TF here and save some stack usage? -- albert - : send the line "unsubscribe linux-ide" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
