Re: PROBLEM REMAINS: [sata_nv ADMA breaks ATAPI] Crash on accessing DVD-RAM
Robert Hancock wrote: Can you (or others experiencing this problem) test the latest patch attached to the RH Bugzilla entry here: https://bugzilla.redhat.com/show_bug.cgi?id=351451 and see if it resolves the problem? I have one report of success so far. I've tested this patch and it seems to work OK with my hardware. Thank you! - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RESEND number 2] libata: eliminate the home grown dma padding in favour of that provided by the block layer
Tejun Heo wrote:
Some ATA controllers including SFF BMDMA and libata PIO HSM need the
number of bytes mapped in the sg table. Yeah, it can be calculated with
a simple macro but it also is a fundamentally confusing dual-sizing
which should be made as clear as possible. Plus, it can be difficult to
find out when somebody used the wrong thing, so what I'm saying is that
we need to make it easy. Anyways, please lemme work on it a bit. I'll
get back to you guys soon.
Okay, here's first draft combined patch. Only compile tested (expect
it to be broken) but it should be functionally equivalent to
ata_sg_setup_extra() based implementation albeit with shorter drain
buffer size. Several things to note...
* fsl last sg check isn't included here. Will split it out and post
it separately.
* rq-raw_data_len added. Rationales...
- All these padding and draining are to prevent controllers from
crapping themselves when data buffer is shorter than it likes it
to be. Any controller which talks MMC (or SPC for that matter)
should be ready for transfers shorter than buffer so feeding
enlarged buffer size is inherently safter than feeding the length,
so the primary data length field, rq-data_len, contains the
adjusted length.
- raw_data_len can't be easily deduced from data_len. The other way
is possible but with both aligning and draining and command
filtering, calculating it later is messy.
* Draining configuration is done in sr as it's the driver for MMC. It
can move both ways - either into SCSI midlayer as SPC and other
commands do variable length responses too or into libata if all
non-ATA controllers are happy without such workarounds. If you ask
me, I'm inclined to move it into SCSI midlayer as the added overhead
is insignificant (especially with drain_needed added) and it won't
break anything (well, theoretically, at least).
* Padding via alinging seems a bit too hacky to me. It doesn't even
cover all sg cases. I think we'll need improvements there, well,
but for the time being, this should do.
I'll test and report in a few hours.
Thanks.
Index: work/block/blk-core.c
===
--- work.orig/block/blk-core.c
+++ work/block/blk-core.c
@@ -116,6 +116,7 @@ void rq_init(struct request_queue *q, st
rq-ref_count = 1;
rq-q = q;
rq-special = NULL;
+ rq-raw_data_len = 0;
rq-data_len = 0;
rq-data = NULL;
rq-nr_phys_segments = 0;
@@ -1982,6 +1983,7 @@ void blk_rq_bio_prep(struct request_queu
rq-hard_cur_sectors = rq-current_nr_sectors;
rq-hard_nr_sectors = rq-nr_sectors = bio_sectors(bio);
rq-buffer = bio_data(bio);
+ rq-raw_data_len = bio-bi_size;
rq-data_len = bio-bi_size;
rq-bio = rq-biotail = bio;
Index: work/block/blk-map.c
===
--- work.orig/block/blk-map.c
+++ work/block/blk-map.c
@@ -19,6 +19,7 @@ int blk_rq_append_bio(struct request_que
rq-biotail-bi_next = bio;
rq-biotail = bio;
+ rq-raw_data_len += bio-bi_size;
rq-data_len += bio-bi_size;
}
return 0;
@@ -56,8 +57,10 @@ static int __blk_rq_map_user(struct requ
if (!(uaddr queue_dma_alignment(q))
!(len queue_dma_alignment(q)))
bio = bio_map_user(q, NULL, uaddr, len, reading);
- else
+ else {
bio = bio_copy_user(q, uaddr, len, reading);
+ rq-data_len = roundup(len, queue_dma_alignment(q));
+ }
if (IS_ERR(bio))
return PTR_ERR(bio);
Index: work/include/linux/blkdev.h
===
--- work.orig/include/linux/blkdev.h
+++ work/include/linux/blkdev.h
@@ -214,6 +214,7 @@ struct request {
unsigned int cmd_len;
unsigned char cmd[BLK_MAX_CDB];
+ unsigned int raw_data_len;
unsigned int data_len;
unsigned int sense_len;
void *data;
@@ -256,6 +257,7 @@ struct bio_vec;
typedef int (merge_bvec_fn) (struct request_queue *, struct bio *, struct
bio_vec *);
typedef void (prepare_flush_fn) (struct request_queue *, struct request *);
typedef void (softirq_done_fn)(struct request *);
+typedef int (dma_drain_needed_fn)(struct request *);
enum blk_queue_state {
Queue_down,
@@ -292,6 +294,7 @@ struct request_queue
merge_bvec_fn *merge_bvec_fn;
prepare_flush_fn*prepare_flush_fn;
softirq_done_fn *softirq_done_fn;
+ dma_drain_needed_fn *dma_drain_needed;
/*
* Dispatch queue sorting
@@ -696,8 +699,9 @@ extern void blk_queue_max_hw_segments(st
extern void blk_queue_max_segment_size(struct request_queue *, unsigned int);
extern void blk_queue_hardsect_size(struct request_queue *, unsigned short);
extern void
Re: [PATCH] ide-tape: dump gcw fields on error in idetape_identify_device()
On Sun, Feb 03, 2008 at 08:16:42PM +0300, Sergei Shtylyov wrote:
Bartlomiej Zolnierkiewicz wrote:
Signed-off-by: Bartlomiej Zolnierkiewicz [EMAIL PROTECTED]
Acked-by: Sergei Shtylyov [EMAIL PROTECTED]
Index: b/drivers/ide/ide-tape.c
===
--- a/drivers/ide/ide-tape.c
+++ b/drivers/ide/ide-tape.c
@@ -3852,16 +3852,17 @@ static int idetape_identify_device (ide_
/* Check that we can support this device */
- if (gcw.protocol !=2 )
-printk(KERN_ERR ide-tape: Protocol is not ATAPI\n);
+if (gcw.protocol != 2)
+printk(KERN_ERR ide-tape: Protocol (0x%02x) is not ATAPI\n,
+gcw.protocol);
else if (gcw.device_type != 1)
-printk(KERN_ERR ide-tape: Device type is not set to tape\n);
+printk(KERN_ERR ide-tape: Device type (0x%02x) is not set
+to tape\n, gcw.device_type);
else if (!gcw.removable)
printk(KERN_ERR ide-tape: The removable flag is not set\n);
else if (gcw.packet_size != 0) {
-printk(KERN_ERR ide-tape: Packet size is not 12 bytes long\n);
-if (gcw.packet_size == 1)
-printk(KERN_ERR ide-tape: Sorry, padding to 16 bytes
is still not supported\n);
+printk(KERN_ERR ide-tape: Packet size (0x%02x) is not 12
+bytes long\n, gcw.packet_size);
Shouldn't it be either packet size is not 12 byted or packet is not
12 bytes long?
I like the last one better. Will send a correction later on to Bart. Thanks.
--
Regards/Gruß,
Boris.
-
To unsubscribe from this list: send the line unsubscribe linux-ide in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: AHCI driver preferring nr_ports over port map
Jan Beulich wrote: Do you have any real case where the above behavior causes problem? It's not strictly a problem (i.e. nothing really mis-behaves), but it made me wonder why the box I saw this on gets 6 ahci device instances set up when spec as well as port map say there ought to be only 5. After looking at the ESB2 spec it seemed that behavior was clearly violating the spec: For ports that are not available, software must not read or write to registers within that port., which contradicts status being displayed for (and therefore status being read) from the 6th (not present) port. Well, two values don't agree with each other and we know for a fact that vendors sometimes get PI wrong, so we trust n_ports in such cases. We can reverse the behavior but that's likely to cause more problems than it fixes. -- tejun - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 10/22] ide-tape: shorten some function names
Signed-off-by: Borislav Petkov [EMAIL PROTECTED]
---
drivers/ide/ide-tape.c | 32 +++-
1 files changed, 15 insertions(+), 17 deletions(-)
diff --git a/drivers/ide/ide-tape.c b/drivers/ide/ide-tape.c
index ae2c76d..4fee160 100644
--- a/drivers/ide/ide-tape.c
+++ b/drivers/ide/ide-tape.c
@@ -1053,7 +1053,7 @@ static void idetape_postpone_request (ide_drive_t *drive)
* command. We will transfer some of the data (as requested by the drive) and
* will re-point interrupt handler to us. When data transfer is finished, we
* will act according to the algorithm described before
- * idetape_issue_packet_command.
+ * idetape_issue_pc.
*/
typedef void idetape_io_buf(ide_drive_t *, idetape_pc_t *, unsigned int);
@@ -1233,7 +1233,7 @@ static ide_startstop_t idetape_pc_intr(ide_drive_t *drive)
*
* The handling will be done in three stages:
*
- * 1. idetape_issue_packet_command will send the packet command to the
+ * 1. idetape_issue_pc will send the packet command to the
* drive, and will set the interrupt handler to idetape_pc_intr.
*
* 2. On each interrupt, idetape_pc_intr will be called. This step
@@ -1308,7 +1308,7 @@ static ide_startstop_t idetape_transfer_pc(ide_drive_t
*drive)
return ide_started;
}
-static ide_startstop_t idetape_issue_packet_command (ide_drive_t *drive,
idetape_pc_t *pc)
+static ide_startstop_t idetape_issue_pc(ide_drive_t *drive, idetape_pc_t *pc)
{
ide_hwif_t *hwif = drive-hwif;
idetape_tape_t *tape = drive-driver_data;
@@ -1614,7 +1614,7 @@ static ide_startstop_t idetape_do_request(ide_drive_t
*drive,
*/
if (tape-failed_pc != NULL
tape-pc-c[0] == REQUEST_SENSE) {
- return idetape_issue_packet_command(drive, tape-failed_pc);
+ return idetape_issue_pc(drive, tape-failed_pc);
}
if (postponed_rq != NULL)
if (rq != postponed_rq) {
@@ -1707,7 +1707,7 @@ static ide_startstop_t idetape_do_request(ide_drive_t
*drive,
}
BUG();
out:
- return idetape_issue_packet_command(drive, pc);
+ return idetape_issue_pc(drive, pc);
err:
printk(KERN_ERR ide-tape: Error processing request %u\n, rq-cmd[0]);
@@ -2263,11 +2263,8 @@ static int idetape_queue_rw_tail(ide_drive_t *drive, int
cmd, int blocks, struct
return (tape-blk_size * (blocks-rq.current_nr_sectors));
}
-/*
- * idetape_insert_pipeline_into_queue is used to start servicing the
- * pipeline stages, starting from tape-next_stage.
- */
-static void idetape_insert_pipeline_into_queue (ide_drive_t *drive)
+/* 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;
@@ -2359,7 +2356,7 @@ static int idetape_add_chrdev_write_request (ide_drive_t
*drive, int blocks)
spin_unlock_irqrestore(tape-lock, flags);
} else {
spin_unlock_irqrestore(tape-lock, flags);
- idetape_insert_pipeline_into_queue(drive);
+ idetape_plug_pipeline(drive);
if (idetape_pipeline_active(tape))
continue;
/*
@@ -2396,7 +2393,7 @@ static int idetape_add_chrdev_write_request (ide_drive_t
*drive, int blocks)
tape-insert_time = jiffies;
tape-insert_size = 0;
tape-insert_speed = 0;
- idetape_insert_pipeline_into_queue(drive);
+ idetape_plug_pipeline(drive);
}
}
if (test_and_clear_bit(IDETAPE_PIPELINE_ERROR, tape-flags))
@@ -2415,7 +2412,7 @@ static void idetape_wait_for_pipeline (ide_drive_t *drive)
unsigned long flags;
while (tape-next_stage || idetape_pipeline_active(tape)) {
- idetape_insert_pipeline_into_queue(drive);
+ idetape_plug_pipeline(drive);
spin_lock_irqsave(tape-lock, flags);
if (idetape_pipeline_active(tape))
idetape_wait_for_request(drive, tape-active_data_rq);
@@ -2508,7 +2505,7 @@ static void idetape_restart_speed_control (ide_drive_t
*drive)
tape-controlled_previous_head_time =
tape-uncontrolled_previous_head_time = jiffies;
}
-static int idetape_initiate_read (ide_drive_t *drive, int max_stages)
+static int idetape_init_read(ide_drive_t *drive, int max_stages)
{
idetape_tape_t *tape = drive-driver_data;
idetape_stage_t *new_stage;
@@ -2569,7 +2566,7 @@ static int idetape_initiate_read (ide_drive_t *drive, int
max_stages)
tape-insert_time = jiffies;
tape-insert_size = 0;
tape-insert_speed = 0;
-
[PATCH 14/22] ide-tape: cleanup and fix comments
Also, remove redundant ones and cleanup whitespace.
Signed-off-by: Borislav Petkov [EMAIL PROTECTED]
---
drivers/ide/ide-tape.c | 725 +++
1 files changed, 293 insertions(+), 432 deletions(-)
diff --git a/drivers/ide/ide-tape.c b/drivers/ide/ide-tape.c
index 175d507..a80f8d9 100644
--- a/drivers/ide/ide-tape.c
+++ b/drivers/ide/ide-tape.c
@@ -73,37 +73,34 @@ enum {
/*
- * Pipelined mode parameters.
+ * 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 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.
+ * 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.)
+ * 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_STAGES400
#define IDETAPE_INCREASE_STAGES_RATE20
/*
- * After each failed packet command we issue a request sense command
- * and retry the packet command IDETAPE_MAX_PC_RETRIES times.
+ * After each failed packet command we issue a request sense command and retry
+ * the packet command IDETAPE_MAX_PC_RETRIES times.
*
- * Setting IDETAPE_MAX_PC_RETRIES to 0 will disable retries.
+ * Setting IDETAPE_MAX_PC_RETRIES to 0 will disable retries.
*/
#define IDETAPE_MAX_PC_RETRIES 3
/*
- * With each packet command, we allocate a buffer of
- * IDETAPE_PC_BUFFER_SIZE bytes. This is used for several packet
- * commands (Not for READ/WRITE commands).
+ * With each packet command, we allocate a buffer of IDETAPE_PC_BUFFER_SIZE
+ * bytes. This is used for several packet commands (Not for READ/WRITE
commands)
*/
#define IDETAPE_PC_BUFFER_SIZE 256
@@ -115,48 +112,41 @@ enum {
#define IDETAPE_WAIT_CMD (900*HZ)
/*
- * The following parameter is used to select the point in the internal
- * tape fifo in which we will start to refill the buffer. Decreasing
- * the following parameter will improve the system's latency and
- * interactive response, while using a high value might improve system
- * throughput.
+ * The following parameter is used to select the point in the internal tape
fifo
+ * in which we will start to refill the buffer. Decreasing the following
+ * parameter will improve the system's latency and interactive response, while
+ * using a high value might improve system throughput.
*/
-#define IDETAPE_FIFO_THRESHOLD 2
+#define IDETAPE_FIFO_THRESHOLD 2
/*
- * DSC polling parameters.
+ * DSC polling parameters.
*
- * Polling for DSC (a single bit in the status register) is a very
- * important function in ide-tape. There are two cases in which we
- * poll for DSC:
+ * Polling for DSC (a single bit in the status register) is a very important
+ * function in ide-tape. There are two cases in which we poll for DSC:
*
- * 1. Before a read/write packet command, to ensure that we
- * can transfer data from/to the tape's data buffers, without
- * causing an actual media access. In case the tape is not
- * ready yet, we take out our request from the device
- * request queue, so that ide.c will service requests from
- * the other device on the same interface meanwhile.
*
- * 2. After the successful initialization of a media access
- * packet command, which is a command which can take a long
- * time to complete (it can be several seconds or even an hour).
+ * 1. Before a read/write packet command, to ensure that we can transfer data
+ *from/to the tape's data buffers, without causing an actual media access.
+ *In case the tape is not ready yet, we take out our request from the
device
+ *request queue, so that ide.c could service requests from the other device
+ *on the same interface in the meantime.
*
- * Again, we postpone our request in the middle to free the bus
- *
[PATCH 16/22] ide-tape: include proper headers
Signed-off-by: Borislav Petkov [EMAIL PROTECTED] --- drivers/ide/ide-tape.c |6 +++--- 1 files changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/ide/ide-tape.c b/drivers/ide/ide-tape.c index 342ec50..24f048f 100644 --- a/drivers/ide/ide-tape.c +++ b/drivers/ide/ide-tape.c @@ -39,9 +39,9 @@ #include scsi/scsi.h #include asm/byteorder.h -#include asm/irq.h -#include asm/uaccess.h -#include asm/io.h +#include linux/irq.h +#include linux/uaccess.h +#include linux/io.h #include asm/unaligned.h #include linux/mtio.h -- 1.5.3.7 - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 15/22] ide-tape: remove unused length arg from idetape_create_read_buffer_cmd()
Signed-off-by: Borislav Petkov [EMAIL PROTECTED]
---
drivers/ide/ide-tape.c |8 +---
1 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/ide/ide-tape.c b/drivers/ide/ide-tape.c
index a80f8d9..342ec50 100644
--- a/drivers/ide/ide-tape.c
+++ b/drivers/ide/ide-tape.c
@@ -1468,7 +1468,8 @@ static void idetape_create_read_cmd(idetape_tape_t *tape,
idetape_pc_t *pc, unsi
pc-flags |= PC_FL_DMA_RECOMMENDED;
}
-static void idetape_create_read_buffer_cmd(idetape_tape_t *tape, idetape_pc_t
*pc, unsigned int length, struct idetape_bh *bh)
+static void idetape_create_read_buffer_cmd(idetape_tape_t *tape,
+ idetape_pc_t *pc, struct idetape_bh *bh)
{
int size = 32768;
struct idetape_bh *p = bh;
@@ -1486,7 +1487,8 @@ static void idetape_create_read_buffer_cmd(idetape_tape_t
*tape, idetape_pc_t *p
atomic_set(p-b_count, 0);
p = p-b_reqnext;
}
- pc-request_transfer = pc-buffer_size = size;
+ pc-request_transfer = size;
+ pc-buffer_size = size;
}
static void idetape_create_write_cmd(idetape_tape_t *tape, idetape_pc_t *pc,
unsigned int length, struct idetape_bh *bh)
@@ -1607,7 +1609,7 @@ static ide_startstop_t idetape_do_request(ide_drive_t
*drive,
if (!pc)
goto err;
- idetape_create_read_buffer_cmd(tape, pc, rq-current_nr_sectors,
+ idetape_create_read_buffer_cmd(tape, pc,
(struct idetape_bh *)rq-special);
goto out;
}
--
1.5.3.7
-
To unsubscribe from this list: send the line unsubscribe linux-ide in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 17/22] ide-tape: collect module-related macro calls at the end
Signed-off-by: Borislav Petkov [EMAIL PROTECTED]
---
drivers/ide/ide-tape.c |5 ++---
1 files changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/ide/ide-tape.c b/drivers/ide/ide-tape.c
index 24f048f..cfcf5b0 100644
--- a/drivers/ide/ide-tape.c
+++ b/drivers/ide/ide-tape.c
@@ -3681,9 +3681,6 @@ failed:
return -ENODEV;
}
-MODULE_DESCRIPTION(ATAPI Streaming TAPE Driver);
-MODULE_LICENSE(GPL);
-
static void __exit idetape_exit (void)
{
driver_unregister(idetape_driver.gen_driver);
@@ -3726,3 +3723,5 @@ MODULE_ALIAS(ide:*m-tape*);
module_init(idetape_init);
module_exit(idetape_exit);
MODULE_ALIAS_CHARDEV_MAJOR(IDETAPE_MAJOR);
+MODULE_DESCRIPTION(ATAPI Streaming TAPE Driver);
+MODULE_LICENSE(GPL);
--
1.5.3.7
-
To unsubscribe from this list: send the line unsubscribe linux-ide in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 09/22] ide-tape: remove idetape_increase_max_pipeline_stages()
This function was being used only at one place so fold it in there.
Signed-off-by: Borislav Petkov [EMAIL PROTECTED]
---
drivers/ide/ide-tape.c | 36
1 files changed, 16 insertions(+), 20 deletions(-)
diff --git a/drivers/ide/ide-tape.c b/drivers/ide/ide-tape.c
index b4944ef..ae2c76d 100644
--- a/drivers/ide/ide-tape.c
+++ b/drivers/ide/ide-tape.c
@@ -771,25 +771,6 @@ static void idetape_activate_next_stage(ide_drive_t *drive)
}
/*
- * idetape_increase_max_pipeline_stages is a part of the feedback
- * loop which tries to find the optimum number of stages. In the
- * feedback loop, 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.
- */
-static void idetape_increase_max_pipeline_stages (ide_drive_t *drive)
-{
- idetape_tape_t *tape = drive-driver_data;
- int increase = (tape-max_pipeline - tape-min_pipeline) / 10;
-
- debug_log(DBG_PROCS, Enter %s\n, __func__);
-
- tape-max_stages += max(increase, 1);
- tape-max_stages = max(tape-max_stages, tape-min_pipeline);
- tape-max_stages = min(tape-max_stages, tape-max_pipeline);
-}
-
-/*
* idetape_kfree_stage calls kfree to completely free a stage, along with
* its related buffers.
*/
@@ -937,7 +918,22 @@ static int idetape_end_request(ide_drive_t *drive, int
uptodate, int nr_sects)
(void)ide_do_drive_cmd(drive, tape-active_data_rq,
ide_end);
} else if (!error) {
- idetape_increase_max_pipeline_stages(drive);
+ /*
+* 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);
--
1.5.3.7
-
To unsubscribe from this list: send the line unsubscribe linux-ide in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 12/22] ide-tape: dump gcw fields on error in idetape_identify_device()
goes before ide-tape: remove IDETAPE_DEBUG_INFO patch in IDE quilt tree
Cc: Borislav Petkov [EMAIL PROTECTED]
Signed-off-by: Bartlomiej Zolnierkiewicz [EMAIL PROTECTED]
---
drivers/ide/ide-tape.c | 13 +++--
1 files changed, 7 insertions(+), 6 deletions(-)
diff --git a/drivers/ide/ide-tape.c b/drivers/ide/ide-tape.c
index 1c4f94c..712c5df 100644
--- a/drivers/ide/ide-tape.c
+++ b/drivers/ide/ide-tape.c
@@ -3402,16 +3402,17 @@ static int idetape_identify_device (ide_drive_t *drive)
/* Check that we can support this device */
- if (gcw.protocol !=2 )
- printk(KERN_ERR ide-tape: Protocol is not ATAPI\n);
+ if (gcw.protocol != 2)
+ printk(KERN_ERR ide-tape: Protocol (0x%02x) is not ATAPI\n,
+ gcw.protocol);
else if (gcw.device_type != 1)
- printk(KERN_ERR ide-tape: Device type is not set to tape\n);
+ printk(KERN_ERR ide-tape: Device type (0x%02x) is not set
+ to tape\n, gcw.device_type);
else if (!gcw.removable)
printk(KERN_ERR ide-tape: The removable flag is not set\n);
else if (gcw.packet_size != 0) {
- printk(KERN_ERR ide-tape: Packet size is not 12 bytes long\n);
- if (gcw.packet_size == 1)
- printk(KERN_ERR ide-tape: Sorry, padding to 16 bytes
is still not supported\n);
+ printk(KERN_ERR ide-tape: Packet size (0x%02x) is not 12
+ bytes long\n, gcw.packet_size);
} else
return 1;
return 0;
--
1.5.3.7
-
To unsubscribe from this list: send the line unsubscribe linux-ide in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 04/22] ide-tape: simplify code branching in the interrupt handler
... by adding a new typedef function pointer idetape_io_buf in order to call
the proper buffer i/o handler depending on the data direction.
Signed-off-by: Borislav Petkov [EMAIL PROTECTED]
---
drivers/ide/ide-tape.c | 55 +--
1 files changed, 29 insertions(+), 26 deletions(-)
diff --git a/drivers/ide/ide-tape.c b/drivers/ide/ide-tape.c
index b15dd17..2857965 100644
--- a/drivers/ide/ide-tape.c
+++ b/drivers/ide/ide-tape.c
@@ -1105,18 +1105,22 @@ static void idetape_postpone_request (ide_drive_t
*drive)
}
/*
- * idetape_pc_intr is the usual interrupt handler which will be called
- * during a packet command. We will transfer some of the data (as
- * requested by the drive) and will re-point interrupt handler to us.
- * When data transfer is finished, we will act according to the
- * algorithm described before idetape_issue_packet_command.
- *
+ * This is the usual interrupt handler which will be called during a packet
+ * command. We will transfer some of the data (as requested by the drive) and
+ * will re-point interrupt handler to us. When data transfer is finished, we
+ * will act according to the algorithm described before
+ * idetape_issue_packet_command.
*/
-static ide_startstop_t idetape_pc_intr (ide_drive_t *drive)
+
+typedef void idetape_io_buf(ide_drive_t *, idetape_pc_t *, unsigned int);
+
+static ide_startstop_t idetape_pc_intr(ide_drive_t *drive)
{
ide_hwif_t *hwif = drive-hwif;
idetape_tape_t *tape = drive-driver_data;
idetape_pc_t *pc = tape-pc;
+ xfer_func_t *xferfunc;
+ idetape_io_buf *iobuf;
unsigned int temp;
#if SIMULATE_ERRORS
static int error_sim_count = 0;
@@ -1184,7 +1188,8 @@ static ide_startstop_t idetape_pc_intr (ide_drive_t
*drive)
debug_log(DBG_ERR, %s: I/O error\n, tape-name);
if (pc-c[0] == REQUEST_SENSE) {
- printk(KERN_ERR ide-tape: I/O error in request
sense command\n);
+ printk(KERN_ERR ide-tape: I/O error in request
+sense command\n);
return ide_do_reset(drive);
}
debug_log(DBG_ERR, [cmd %x]: check condition\n,
@@ -1223,7 +1228,7 @@ static ide_startstop_t idetape_pc_intr (ide_drive_t
*drive)
ireason = hwif-INB(IDE_IREASON_REG);
if (ireason CD) {
- printk(KERN_ERR ide-tape: CoD != 0 in idetape_pc_intr\n);
+ printk(KERN_ERR ide-tape: CoD != 0 in %s\n, __func__);
return ide_do_reset(drive);
}
if (((ireason IO) == IO) == test_bit(PC_WRITING, pc-flags)) {
@@ -1239,31 +1244,29 @@ static ide_startstop_t idetape_pc_intr (ide_drive_t
*drive)
temp = pc-actually_transferred + bcount;
if (temp pc-request_transfer) {
if (temp pc-buffer_size) {
- printk(KERN_ERR ide-tape: The tape wants to
send us more data than expected - discarding data\n);
+ printk(KERN_ERR ide-tape: The tape wants to
+ send us more data than expected
+ - discarding data\n);
idetape_discard_data(drive, bcount);
- ide_set_handler(drive, idetape_pc_intr,
IDETAPE_WAIT_CMD, NULL);
+ ide_set_handler(drive, idetape_pc_intr,
+ IDETAPE_WAIT_CMD, NULL);
return ide_started;
}
debug_log(DBG_SENSE, The tape wants to send us more
data than expected - allowing transfer\n);
-
}
- }
- if (test_bit(PC_WRITING, pc-flags)) {
- if (pc-bh != NULL)
- idetape_output_buffers(drive, pc, bcount);
- else
- /* Write the current buffer */
- hwif-atapi_output_bytes(drive, pc-current_position,
-bcount);
+ iobuf = idetape_input_buffers;
+ xferfunc = hwif-atapi_input_bytes;
} else {
- if (pc-bh != NULL)
- idetape_input_buffers(drive, pc, bcount);
- else
- /* Read the current buffer */
- hwif-atapi_input_bytes(drive, pc-current_position,
- bcount);
+ iobuf = idetape_output_buffers;
+ xferfunc = hwif-atapi_output_bytes;
}
+
+ if (pc-bh)
+ iobuf(drive, pc, bcount);
+ else
+ xferfunc(drive, pc-current_position, bcount);
+
/* Update the current
[PATCH 06/22] ide-tape: struct idetape_tape_t: remove unused members
- last_frame_position: only being written to once
- firmware_revision, product_id, vendor_id: used once, remove from struct
idetape_tape_t and deal with them locally
- firmware_revision_num: only written to once
- tape_still_time_begin: completely unused
- tape_still_time: never written to; remove corresponding code chunk
- uncontrolled_last_pipeline_head: only once written to
- blocks_in_buffer: only written to
Signed-off-by: Borislav Petkov [EMAIL PROTECTED]
---
drivers/ide/ide-tape.c | 45 +++--
1 files changed, 11 insertions(+), 34 deletions(-)
diff --git a/drivers/ide/ide-tape.c b/drivers/ide/ide-tape.c
index e0e8184..126e8a9 100644
--- a/drivers/ide/ide-tape.c
+++ b/drivers/ide/ide-tape.c
@@ -311,8 +311,6 @@ typedef struct ide_tape_obj {
u8 partition;
/* Current block */
unsigned int first_frame_position;
- unsigned int last_frame_position;
- unsigned int blocks_in_buffer;
/*
* Last error information
@@ -399,11 +397,6 @@ typedef struct ide_tape_obj {
int avg_size;
int avg_speed;
- char vendor_id[10];
- char product_id[18];
- char firmware_revision[6];
- int firmware_revision_num;
-
/* the door is currently locked */
int door_locked;
/* the tape hardware is write protected */
@@ -441,12 +434,6 @@ typedef struct ide_tape_obj {
int measure_insert_time;
/*
-* Measure tape still time, in milliseconds
-*/
- unsigned long tape_still_time_begin;
- int tape_still_time;
-
- /*
* Speed regulation negative feedback loop
*/
int speed_control;
@@ -454,7 +441,6 @@ typedef struct ide_tape_obj {
int controlled_pipeline_head_speed;
int uncontrolled_pipeline_head_speed;
int controlled_last_pipeline_head;
- int uncontrolled_last_pipeline_head;
unsigned long uncontrolled_pipeline_head_time;
unsigned long controlled_pipeline_head_time;
int controlled_previous_pipeline_head;
@@ -1696,8 +1682,6 @@ static ide_startstop_t idetape_do_request(ide_drive_t
*drive,
drive-post_reset = 0;
}
- if (tape-tape_still_time 100 tape-tape_still_time 200)
- tape-measure_insert_time = 1;
if (time_after(jiffies, tape-insert_time))
tape-insert_speed = tape-insert_size / 1024 * HZ / (jiffies -
tape-insert_time);
idetape_calculate_speeds(drive);
@@ -2010,9 +1994,6 @@ static ide_startstop_t
idetape_read_position_callback(ide_drive_t *drive)
tape-partition = readpos[1];
tape-first_frame_position =
be32_to_cpu(*(u32 *)readpos[4]);
- tape-last_frame_position =
- be32_to_cpu(*(u32 *)readpos[8]);
- tape-blocks_in_buffer = readpos[15];
set_bit(IDETAPE_ADDRESS_VALID, tape-flags);
idetape_end_request(drive, 1, 0);
}
@@ -2541,7 +2522,7 @@ static void idetape_restart_speed_control (ide_drive_t
*drive)
tape-restart_speed_control_req = 0;
tape-pipeline_head = 0;
- tape-controlled_last_pipeline_head =
tape-uncontrolled_last_pipeline_head = 0;
+ tape-controlled_last_pipeline_head = 0;
tape-controlled_previous_pipeline_head =
tape-uncontrolled_previous_pipeline_head = 0;
tape-pipeline_head_speed = tape-controlled_pipeline_head_speed = 5000;
tape-uncontrolled_pipeline_head_speed = 0;
@@ -3438,9 +3419,9 @@ static int idetape_identify_device (ide_drive_t *drive)
static void idetape_get_inquiry_results(ide_drive_t *drive)
{
- char *r;
idetape_tape_t *tape = drive-driver_data;
idetape_pc_t pc;
+ char fw_rev[6], vendor_id[10], product_id[18];
idetape_create_inquiry_cmd(pc);
if (idetape_queue_pc_tail(drive, pc)) {
@@ -3448,20 +3429,16 @@ static void idetape_get_inquiry_results(ide_drive_t
*drive)
tape-name);
return;
}
- memcpy(tape-vendor_id, pc.buffer[8], 8);
- memcpy(tape-product_id, pc.buffer[16], 16);
- memcpy(tape-firmware_revision, pc.buffer[32], 4);
-
- ide_fixstring(tape-vendor_id, 10, 0);
- ide_fixstring(tape-product_id, 18, 0);
- ide_fixstring(tape-firmware_revision, 6, 0);
- r = tape-firmware_revision;
- if (*(r + 1) == '.')
- tape-firmware_revision_num = (*r - '0') * 100 +
- (*(r + 2) - '0') * 10 + *(r + 3) - '0';
+ memcpy(vendor_id, pc.buffer[8], 8);
+ memcpy(product_id, pc.buffer[16], 16);
+ memcpy(fw_rev, pc.buffer[32], 4);
+
+ ide_fixstring(vendor_id, 10, 0);
+ ide_fixstring(product_id, 18, 0);
+ ide_fixstring(fw_rev, 6, 0);
+
printk(KERN_INFO ide-tape: %s - %s: %s %s
[PATCH 01/22] ide-tape: refactor the debug logging facility
Teach the debug logging macro to differentiate between log levels based on the
type of debug level enabled specifically instead of a threshold-based one.
Thus, convert tape-debug_level to a bitmask that is written to over /proc.
Also,
- cleanup and simplify the debug macro thus removing a lot of code lines,
- get rid of unused debug levels,
- adjust the loglevel at several places where it was simply missing (e.g.
idetape_chrdev_open())
- move the tape ptr initialization up in idetape_chrdev_open() so that we can
use it in the debug_log macro earlier in the function.
Signed-off-by: Borislav Petkov [EMAIL PROTECTED]
---
drivers/ide/ide-tape.c | 344 +---
1 files changed, 122 insertions(+), 222 deletions(-)
diff --git a/drivers/ide/ide-tape.c b/drivers/ide/ide-tape.c
index 4168a06..442d71c 100644
--- a/drivers/ide/ide-tape.c
+++ b/drivers/ide/ide-tape.c
@@ -45,6 +45,32 @@
#include asm/unaligned.h
#include linux/mtio.h
+enum {
+ /* output errors only */
+ DBG_ERR = (1 0),
+ /* output all sense key/asc */
+ DBG_SENSE = (1 1),
+ /* info regarding all chrdev-related procedures */
+ DBG_CHRDEV =(1 2),
+ /* all remaining procedures */
+ DBG_PROCS = (1 3),
+ /* buffer alloc info (pc_stack rq_stack) */
+ DBG_PCRQ_STACK =(1 4),
+};
+
+/* define to see debug info */
+#define IDETAPE_DEBUG_LOG 0
+
+#if IDETAPE_DEBUG_LOG
+#define debug_log(lvl, fmt, args...) \
+{ \
+ if (tape-debug_mask lvl) \
+ printk(KERN_INFO ide-tape: fmt, ## args);\
+}
+#else
+#define debug_log(lvl, fmt, args...) do {} while (0)
+#endif
+
/ Tunable parameters */
@@ -68,23 +94,6 @@
#define IDETAPE_INCREASE_STAGES_RATE20
/*
- * The following are used to debug the driver:
- *
- * Setting IDETAPE_DEBUG_LOG to 1 will log driver flow control.s
- *
- * Setting them to 0 will restore normal operation mode:
- *
- * 1. Disable logging normal successful operations.
- * 2. Disable self-sanity checks.
- * 3. Errors will still be logged, of course.
- *
- * All the #if DEBUG code will be removed some day, when the driver
- * is verified to be stable enough. This will make it much more
- * esthetic.
- */
-#define IDETAPE_DEBUG_LOG 0
-
-/*
* After each failed packet command we issue a request sense command
* and retry the packet command IDETAPE_MAX_PC_RETRIES times.
*
@@ -451,18 +460,7 @@ typedef struct ide_tape_obj {
unsigned long uncontrolled_previous_head_time;
int restart_speed_control_req;
-/*
- * Debug_level determines amount of debugging output;
- * can be changed using /proc/ide/hdx/settings
- * 0 : almost no debugging output
- * 1 : 0+output errors only
- * 2 : 1+output all sensekey/asc
- * 3 : 2+follow all chrdev related procedures
- * 4 : 3+follow all procedures
- * 5 : 4+include pc_stack rq_stack info
- * 6 : 5+USE_COUNT updates
- */
- int debug_level;
+ u32 debug_mask;
} idetape_tape_t;
static DEFINE_MUTEX(idetape_ref_mutex);
@@ -716,11 +714,8 @@ static idetape_pc_t *idetape_next_pc_storage (ide_drive_t
*drive)
{
idetape_tape_t *tape = drive-driver_data;
-#if IDETAPE_DEBUG_LOG
- if (tape-debug_level = 5)
- printk(KERN_INFO ide-tape: pc_stack_index=%d\n,
- tape-pc_stack_index);
-#endif /* IDETAPE_DEBUG_LOG */
+ debug_log(DBG_PCRQ_STACK, pc_stack_index=%d\n, tape-pc_stack_index);
+
if (tape-pc_stack_index == IDETAPE_PC_STACK)
tape-pc_stack_index=0;
return (tape-pc_stack[tape-pc_stack_index++]);
@@ -743,11 +738,8 @@ static struct request *idetape_next_rq_storage
(ide_drive_t *drive)
{
idetape_tape_t *tape = drive-driver_data;
-#if IDETAPE_DEBUG_LOG
- if (tape-debug_level = 5)
- printk(KERN_INFO ide-tape: rq_stack_index=%d\n,
- tape-rq_stack_index);
-#endif /* IDETAPE_DEBUG_LOG */
+ debug_log(DBG_PCRQ_STACK, rq_stack_index=%d\n, tape-rq_stack_index);
+
if (tape-rq_stack_index == IDETAPE_PC_STACK)
tape-rq_stack_index=0;
return (tape-rq_stack[tape-rq_stack_index++]);
@@ -780,17 +772,9 @@ static void idetape_analyze_error(ide_drive_t *drive, u8
*sense)
tape-sense_key = sense[2] 0xF;
tape-asc = sense[12];
tape-ascq = sense[13];
-#if IDETAPE_DEBUG_LOG
- /*
-* Without debugging, we only log an error if we decided to give up
-* retrying.
-*/
- if (tape-debug_level = 1)
- printk(KERN_INFO ide-tape: pc = %x,
Re: AHCI driver preferring nr_ports over port map
Jan Beulich wrote: I understand your concern, but I think you also understand mine. So I'm not really asking for general reversal of the logic, but to perhaps make it just a little smarter. The (not generally usable according to what you said earlier) experiment I made was to use the smaller of the two sets - if either set is empty this of course wouldn't be correct. But perhaps, if you have a non-empty port map, that should be preferred over nr_ports? Otherwise, chipset specific knowledge may need to be applied here (i.e. for ESB2, port map ought to always be used)... Yes, we can be more smart if necessary. I don't know. The hardware is clearly violating the spec which requires those two values to agree. What status values are you seeing? Hardware vendors usually don't get n_ports wrong from the start, they probably have forgotten to decrement it by one when one of the ports is plugged for some reason. I bet the silicon for the port is there and reporting offline PHY, right? -- tejun - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: AHCI driver preferring nr_ports over port map
Tejun Heo [EMAIL PROTECTED] 02/02/08 9:16 AM Jan Beulich wrote: Jeff, while I realize that Intel's documentation may not be consistent with anything more generic (which I don't know where to look for), this current behavior seems to contradict what Intel documents for ESB2: 23.3.1.4 PI – Ports Implemented Register (D31:F2) Address Offset: ABAR + 0Ch–0Fh Attribute: R/WO, RO Default Value: h Size: 32 bits This register indicates which ports are exposed to the Intel® 631xESB/632xESB I/O Controller Hub. It is loaded by platform BIOS. It indicates which ports that the device supports are available for software to use. For ports that are not available, software must not read or write to registers within that port. nr_ports is preferred over port_map when they disagree which shouldn't happen in the first place. On some earlier ahcis, PI was cleared to zero and lower nr_port number of ports should be used. The reason why nr_ports is preferred over PI comes from similar place. Hardware/BIOSen are more likely to get PI wrong than nr_ports, so... Do you have any real case where the above behavior causes problem? It's not strictly a problem (i.e. nothing really mis-behaves), but it made me wonder why the box I saw this on gets 6 ahci device instances set up when spec as well as port map say there ought to be only 5. After looking at the ESB2 spec it seemed that behavior was clearly violating the spec: For ports that are not available, software must not read or write to registers within that port., which contradicts status being displayed for (and therefore status being read) from the 6th (not present) port. Jan - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RESEND number 2] libata: eliminate the home grown dma padding in favour of that provided by the block layer
And, here's working version. I'll splite and post them tomorrow.
Thanks.
Index: work/block/blk-core.c
===
--- work.orig/block/blk-core.c
+++ work/block/blk-core.c
@@ -116,6 +116,7 @@ void rq_init(struct request_queue *q, st
rq-ref_count = 1;
rq-q = q;
rq-special = NULL;
+ rq-raw_data_len = 0;
rq-data_len = 0;
rq-data = NULL;
rq-nr_phys_segments = 0;
@@ -1982,6 +1983,7 @@ void blk_rq_bio_prep(struct request_queu
rq-hard_cur_sectors = rq-current_nr_sectors;
rq-hard_nr_sectors = rq-nr_sectors = bio_sectors(bio);
rq-buffer = bio_data(bio);
+ rq-raw_data_len = bio-bi_size;
rq-data_len = bio-bi_size;
rq-bio = rq-biotail = bio;
Index: work/block/blk-map.c
===
--- work.orig/block/blk-map.c
+++ work/block/blk-map.c
@@ -19,6 +19,7 @@ int blk_rq_append_bio(struct request_que
rq-biotail-bi_next = bio;
rq-biotail = bio;
+ rq-raw_data_len += bio-bi_size;
rq-data_len += bio-bi_size;
}
return 0;
@@ -139,6 +140,25 @@ int blk_rq_map_user(struct request_queue
ubuf += ret;
}
+ /*
+* __blk_rq_map_user() copies the buffers if starting address
+* or length aren't aligned. As the copied buffer is always
+* page aligned, we know for a fact that there's enough room
+* for padding. Extend the last bio and update rq-data_len
+* accordingly.
+*
+* On unmap, bio_uncopy_user() will use unmodified
+* bio_map_data pointed to by bio-bi_private.
+*/
+ if (len queue_dma_alignment(q)) {
+ unsigned int pad_len = (queue_dma_alignment(q) ~len) + 1;
+ struct bio *bio = rq-biotail;
+
+ bio-bi_io_vec[bio-bi_vcnt - 1].bv_len += pad_len;
+ bio-bi_size += pad_len;
+ rq-data_len += pad_len;
+ }
+
rq-buffer = rq-data = NULL;
return 0;
unmap_rq:
Index: work/include/linux/blkdev.h
===
--- work.orig/include/linux/blkdev.h
+++ work/include/linux/blkdev.h
@@ -214,6 +214,7 @@ struct request {
unsigned int cmd_len;
unsigned char cmd[BLK_MAX_CDB];
+ unsigned int raw_data_len;
unsigned int data_len;
unsigned int sense_len;
void *data;
@@ -256,6 +257,7 @@ struct bio_vec;
typedef int (merge_bvec_fn) (struct request_queue *, struct bio *, struct
bio_vec *);
typedef void (prepare_flush_fn) (struct request_queue *, struct request *);
typedef void (softirq_done_fn)(struct request *);
+typedef int (dma_drain_needed_fn)(struct request *);
enum blk_queue_state {
Queue_down,
@@ -292,6 +294,7 @@ struct request_queue
merge_bvec_fn *merge_bvec_fn;
prepare_flush_fn*prepare_flush_fn;
softirq_done_fn *softirq_done_fn;
+ dma_drain_needed_fn *dma_drain_needed;
/*
* Dispatch queue sorting
@@ -696,8 +699,9 @@ extern void blk_queue_max_hw_segments(st
extern void blk_queue_max_segment_size(struct request_queue *, unsigned int);
extern void blk_queue_hardsect_size(struct request_queue *, unsigned short);
extern void blk_queue_stack_limits(struct request_queue *t, struct
request_queue *b);
-extern int blk_queue_dma_drain(struct request_queue *q, void *buf,
- unsigned int size);
+extern int blk_queue_dma_drain(struct request_queue *q,
+ dma_drain_needed_fn *dma_drain_needed,
+ void *buf, unsigned int size);
extern void blk_queue_segment_boundary(struct request_queue *, unsigned long);
extern void blk_queue_prep_rq(struct request_queue *, prep_rq_fn *pfn);
extern void blk_queue_merge_bvec(struct request_queue *, merge_bvec_fn *);
Index: work/block/blk-merge.c
===
--- work.orig/block/blk-merge.c
+++ work/block/blk-merge.c
@@ -220,7 +220,7 @@ new_segment:
bvprv = bvec;
} /* segments in rq */
- if (q-dma_drain_size) {
+ if (q-dma_drain_size q-dma_drain_needed(rq)) {
sg-page_link = ~0x02;
sg = sg_next(sg);
sg_set_page(sg, virt_to_page(q-dma_drain_buffer),
@@ -228,6 +228,7 @@ new_segment:
((unsigned long)q-dma_drain_buffer)
(PAGE_SIZE - 1));
nsegs++;
+ rq-data_len += q-dma_drain_size;
}
if (sg)
Index: work/block/bsg.c
===
--- work.orig/block/bsg.c
+++ work/block/bsg.c
@@ -437,14 +437,14 @@ static int blk_complete_sgv4_hdr_rq(stru
}
if (rq-next_rq) {
-
Re: [PATCH 4/9] libata: normalize port_info, port_operations and sht tables
* Every driver for SFF controllers now uses ata_pci_default_filter()
unless the driver has custom implementation.
That is only needed for DMA capable devices. I guess it does no harm to
be consistent and call it anyway but you then say ..
* No reason to set ata_pci_default_filter() for PIO-only drivers.
and your patches add the calls for the CS5520 ?
diff --git a/drivers/ata/pata_cs5520.c b/drivers/ata/pata_cs5520.c
index 972ed9f..5614e76 100644
--- a/drivers/ata/pata_cs5520.c
+++ b/drivers/ata/pata_cs5520.c
@@ -160,6 +160,7 @@ static struct scsi_host_template cs5520_sht = {
static struct ata_port_operations cs5520_port_ops = {
.set_piomode= cs5520_set_piomode,
.set_dmamode= cs5520_set_dmamode,
+ .mode_filter= ata_pci_default_filter,
This case is wrong. CS5520 doesn't do DMA (just VDMA which we don't
support) and in addition doesn't use BAR4 so its not a generic PCI
controller and this is asking for trouble later.
diff --git a/drivers/ata/pata_opti.c b/drivers/ata/pata_opti.c
index 8f79447..ebb9dc1 100644
--- a/drivers/ata/pata_opti.c
+++ b/drivers/ata/pata_opti.c
@@ -184,6 +184,7 @@ static struct scsi_host_template opti_sht = {
static struct ata_port_operations opti_port_ops = {
.set_piomode= opti_set_piomode,
+ .mode_filter= ata_pci_default_filter,
PIO only
--- a/drivers/ata/pata_rz1000.c
+++ b/drivers/ata/pata_rz1000.c
@@ -72,6 +72,7 @@ static struct scsi_host_template rz1000_sht = {
static struct ata_port_operations rz1000_port_ops = {
.set_mode = rz1000_set_mode,
+ .mode_filter= ata_pci_default_filter,
PIO only
-
To unsubscribe from this list: send the line unsubscribe linux-ide in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
(fwd) Bug#11922: I/O error on blank tapes
hello borislav, may i forward you that *old* Debian kernel bug, have seen you working on ide-tape: http://bugs.debian.org/11922 no we don't carry any ide patches anymore. maybe you've already fixed it in latest? thanks -- maks - Forwarded message from Stephen Kitt [EMAIL PROTECTED] - Subject: Bug#11922: I/O error on blank tapes Date: Sat, 1 Dec 2007 19:06:18 +0100 From: Stephen Kitt [EMAIL PROTECTED] To: [EMAIL PROTECTED] Hi, This does still occur with 2.6.22; with a blank tape in my HP DDS-4 drive: $ tar tzvf /dev/nst0 tar: /dev/nst0: Cannot read: Input/output error tar: At beginning of tape, quitting now tar: Error is not recoverable: exiting now gzip: stdin: unexpected end of file tar: Child returned status 2 tar: Error exit delayed from previous errors Nothing gets logged anywhere, which fits the original bug description. This is a well-known issue: see for example http://www.sibbald.com/bacula/html-manual/Bacula_Console.html (search for blank tape). Regards, Stephen -- To UNSUBSCRIBE, email to [EMAIL PROTECTED] with a subject of unsubscribe. Trouble? Contact [EMAIL PROTECTED] - End forwarded message - - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 07/22] ide-tape: struct idetape_tape_t: shorten member names v2
Shorten some member names not too aggressively since this driver might be gone
anyway soon.
Signed-off-by: Borislav Petkov [EMAIL PROTECTED]
---
drivers/ide/ide-tape.c | 210 ++--
1 files changed, 113 insertions(+), 97 deletions(-)
diff --git a/drivers/ide/ide-tape.c b/drivers/ide/ide-tape.c
index 126e8a9..0b5ccce 100644
--- a/drivers/ide/ide-tape.c
+++ b/drivers/ide/ide-tape.c
@@ -299,10 +299,8 @@ typedef struct ide_tape_obj {
/* Timer used to poll for dsc */
struct timer_list dsc_timer;
/* Read/Write dsc polling frequency */
- unsigned long best_dsc_rw_frequency;
- /* The current polling frequency */
- unsigned long dsc_polling_frequency;
- /* Maximum waiting time */
+ unsigned long best_dsc_rw_freq;
+ unsigned long dsc_poll_freq;
unsigned long dsc_timeout;
/*
@@ -310,7 +308,7 @@ typedef struct ide_tape_obj {
*/
u8 partition;
/* Current block */
- unsigned int first_frame_position;
+ unsigned int first_frame;
/*
* Last error information
@@ -326,11 +324,8 @@ typedef struct ide_tape_obj {
/* Current character device data transfer direction */
u8 chrdev_dir;
- /*
-* Device information
-*/
- /* Usually 512 or 1024 bytes */
- unsigned short tape_block_size;
+ /* tape block size, usu. 512 or 1024 bytes */
+ unsigned short blk_size;
int user_bs_factor;
/* Copy of the tape's Capabilities and Mechanical Page */
@@ -349,8 +344,8 @@ typedef struct ide_tape_obj {
* The data buffer size is chosen based on the tape's
* recommendation.
*/
- /* Pointer to the request which is waiting in the device request queue
*/
- struct request *active_data_request;
+ /* 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 */
int stage_size;
idetape_stage_t *merge_stage;
@@ -388,7 +383,7 @@ typedef struct ide_tape_obj {
/* Status/Action flags: long for set_bit */
unsigned long flags;
/* protects the ide-tape queue */
- spinlock_t spinlock;
+ spinlock_t lock;
/*
* Measures average tape speed
@@ -750,7 +745,7 @@ static void idetape_analyze_error(ide_drive_t *drive, u8
*sense)
/* Correct pc-actually_transferred by asking the tape. */
if (test_bit(PC_DMA_ERROR, pc-flags)) {
pc-actually_transferred = pc-request_transfer -
- tape-tape_block_size *
+ tape-blk_size *
be32_to_cpu(get_unaligned((u32 *)sense[3]));
idetape_update_buffers(pc);
}
@@ -809,7 +804,7 @@ static void idetape_activate_next_stage(ide_drive_t *drive)
rq-rq_disk = tape-disk;
rq-buffer = NULL;
rq-special = (void *)stage-bh;
- tape-active_data_request = rq;
+ tape-active_data_rq = rq;
tape-active_stage = stage;
tape-next_stage = stage-next;
}
@@ -951,13 +946,13 @@ static int idetape_end_request(ide_drive_t *drive, int
uptodate, int nr_sects)
return 0;
}
- spin_lock_irqsave(tape-spinlock, flags);
+ spin_lock_irqsave(tape-lock, flags);
/* The request was a pipelined data transfer request */
- if (tape-active_data_request == rq) {
+ if (tape-active_data_rq == rq) {
active_stage = tape-active_stage;
tape-active_stage = NULL;
- tape-active_data_request = NULL;
+ tape-active_data_rq = NULL;
tape-nr_pending_stages--;
if (rq-cmd[0] REQ_IDETAPE_WRITE) {
remove_stage = 1;
@@ -978,7 +973,8 @@ static int idetape_end_request(ide_drive_t *drive, int
uptodate, int nr_sects)
/*
* Insert the next request into the request queue.
*/
- (void) ide_do_drive_cmd(drive,
tape-active_data_request, ide_end);
+ (void)ide_do_drive_cmd(drive, tape-active_data_rq,
+ ide_end);
} else if (!error) {
idetape_increase_max_pipeline_stages(drive);
}
@@ -990,9 +986,9 @@ static int idetape_end_request(ide_drive_t *drive, int
uptodate, int nr_sects)
if (remove_stage)
idetape_remove_stage_head(drive);
- if (tape-active_data_request == NULL)
+ if (tape-active_data_rq == NULL)
clear_bit(IDETAPE_PIPELINE_ACTIVE, tape-flags);
- spin_unlock_irqrestore(tape-spinlock, flags);
+ spin_unlock_irqrestore(tape-lock, flags);
return 0;
}
@@ -1089,7 +1085,7 @@
Re: [PATCH RESEND number 2] libata: eliminate the home grown dma padding in favour of that provided by the block layer
On Mon, 2008-02-04 at 23:43 +0900, Tejun Heo wrote:
And, here's working version. I'll splite and post them tomorrow.
Thanks.
Index: work/block/blk-core.c
===
--- work.orig/block/blk-core.c
+++ work/block/blk-core.c
@@ -116,6 +116,7 @@ void rq_init(struct request_queue *q, st
rq-ref_count = 1;
rq-q = q;
rq-special = NULL;
+ rq-raw_data_len = 0;
rq-data_len = 0;
rq-data = NULL;
rq-nr_phys_segments = 0;
@@ -1982,6 +1983,7 @@ void blk_rq_bio_prep(struct request_queu
rq-hard_cur_sectors = rq-current_nr_sectors;
rq-hard_nr_sectors = rq-nr_sectors = bio_sectors(bio);
rq-buffer = bio_data(bio);
+ rq-raw_data_len = bio-bi_size;
rq-data_len = bio-bi_size;
rq-bio = rq-biotail = bio;
Index: work/block/blk-map.c
===
--- work.orig/block/blk-map.c
+++ work/block/blk-map.c
@@ -19,6 +19,7 @@ int blk_rq_append_bio(struct request_que
rq-biotail-bi_next = bio;
rq-biotail = bio;
+ rq-raw_data_len += bio-bi_size;
rq-data_len += bio-bi_size;
}
return 0;
@@ -139,6 +140,25 @@ int blk_rq_map_user(struct request_queue
ubuf += ret;
}
+ /*
+ * __blk_rq_map_user() copies the buffers if starting address
+ * or length aren't aligned. As the copied buffer is always
+ * page aligned, we know for a fact that there's enough room
+ * for padding. Extend the last bio and update rq-data_len
+ * accordingly.
+ *
+ * On unmap, bio_uncopy_user() will use unmodified
+ * bio_map_data pointed to by bio-bi_private.
+ */
+ if (len queue_dma_alignment(q)) {
+ unsigned int pad_len = (queue_dma_alignment(q) ~len) + 1;
+ struct bio *bio = rq-biotail;
+
+ bio-bi_io_vec[bio-bi_vcnt - 1].bv_len += pad_len;
+ bio-bi_size += pad_len;
+ rq-data_len += pad_len;
+ }
+
rq-buffer = rq-data = NULL;
return 0;
unmap_rq:
Index: work/include/linux/blkdev.h
===
--- work.orig/include/linux/blkdev.h
+++ work/include/linux/blkdev.h
@@ -214,6 +214,7 @@ struct request {
unsigned int cmd_len;
unsigned char cmd[BLK_MAX_CDB];
+ unsigned int raw_data_len;
unsigned int data_len;
unsigned int sense_len;
void *data;
Please, no ... none of this is necessary. You're wasting four bytes in
every request from a quantity we already know in the queue.
The point of the original code is that the drain is always waiting for
you silently in the sg list, but never shown in the length. That means
it's entirely up to you whether you use it or ignore it. It also means
that there's no need for the discriminating function in block, you
either do or don't use the drain element.
@@ -256,6 +257,7 @@ struct bio_vec;
typedef int (merge_bvec_fn) (struct request_queue *, struct bio *, struct
bio_vec *);
typedef void (prepare_flush_fn) (struct request_queue *, struct request *);
typedef void (softirq_done_fn)(struct request *);
+typedef int (dma_drain_needed_fn)(struct request *);
enum blk_queue_state {
Queue_down,
@@ -292,6 +294,7 @@ struct request_queue
merge_bvec_fn *merge_bvec_fn;
prepare_flush_fn*prepare_flush_fn;
softirq_done_fn *softirq_done_fn;
+ dma_drain_needed_fn *dma_drain_needed;
Like I said, not necessary because the MMC devices are all single
command queues, so for them, you simply apply the drain buffer the whole
time in block and decide whether to use it in the issue layer
/*
* Dispatch queue sorting
@@ -696,8 +699,9 @@ extern void blk_queue_max_hw_segments(st
extern void blk_queue_max_segment_size(struct request_queue *, unsigned int);
extern void blk_queue_hardsect_size(struct request_queue *, unsigned short);
extern void blk_queue_stack_limits(struct request_queue *t, struct
request_queue *b);
-extern int blk_queue_dma_drain(struct request_queue *q, void *buf,
-unsigned int size);
+extern int blk_queue_dma_drain(struct request_queue *q,
+dma_drain_needed_fn *dma_drain_needed,
+void *buf, unsigned int size);
extern void blk_queue_segment_boundary(struct request_queue *, unsigned
long);
extern void blk_queue_prep_rq(struct request_queue *, prep_rq_fn *pfn);
extern void blk_queue_merge_bvec(struct request_queue *, merge_bvec_fn *);
Index: work/block/blk-merge.c
===
--- work.orig/block/blk-merge.c
+++ work/block/blk-merge.c
@@ -220,7 +220,7 @@ new_segment:
bvprv = bvec;
} /* segments in rq */
- if
Re: [PATCH RESEND number 2] libata: eliminate the home grown dma padding in favour of that provided by the block layer
On Mon, 2008-02-04 at 18:25 +0900, Tejun Heo wrote: Tejun Heo wrote: Some ATA controllers including SFF BMDMA and libata PIO HSM need the number of bytes mapped in the sg table. Yeah, it can be calculated with a simple macro but it also is a fundamentally confusing dual-sizing which should be made as clear as possible. Plus, it can be difficult to find out when somebody used the wrong thing, so what I'm saying is that we need to make it easy. Anyways, please lemme work on it a bit. I'll get back to you guys soon. Okay, here's first draft combined patch. Only compile tested (expect it to be broken) but it should be functionally equivalent to ata_sg_setup_extra() based implementation albeit with shorter drain buffer size. Several things to note... I noticed you posted an update ... I'll go over the code in that one. * fsl last sg check isn't included here. Will split it out and post it separately. * rq-raw_data_len added. Rationales... - All these padding and draining are to prevent controllers from crapping themselves when data buffer is shorter than it likes it to be. Any controller which talks MMC (or SPC for that matter) should be ready for transfers shorter than buffer so feeding enlarged buffer size is inherently safter than feeding the length, so the primary data length field, rq-data_len, contains the adjusted length. Actually, no they don't. Overrun termination is pretty much standard in most HBAs that do MMC (including the strange block one). Allowing an overrun is simply unsafe and a security risk, so all apart from the ATA ones allow us to terminate the transaction safely. - raw_data_len can't be easily deduced from data_len. The other way is possible but with both aligning and draining and command filtering, calculating it later is messy. Like I said, it's simply the command size plus the drain buffer which you get from the queue. I'll redo your patches to show how it can be done without adding the extra space to every request. * Draining configuration is done in sr as it's the driver for MMC. It can move both ways - either into SCSI midlayer as SPC and other commands do variable length responses too or into libata if all non-ATA controllers are happy without such workarounds. If you ask me, I'm inclined to move it into SCSI midlayer as the added overhead is insignificant (especially with drain_needed added) and it won't break anything (well, theoretically, at least). Let me think about this one. The only consumer of draining is libata. It might make sense to process the drains in sr.c but it certainly doesn't make sense to turn them on indiscriminately in there because *none* of the SCSI users needs them. * Padding via alinging seems a bit too hacky to me. It doesn't even cover all sg cases. I think we'll need improvements there, well, but for the time being, this should do. That's essentially what the dma_aligment parameter was designed for ... we can always make the way it's handled differently; I just updated block to use what exists already. I'll test and report in a few hours. Great, thanks. James - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 11/22] ide-tape: remove atomic test/set macros
Also remove flag IDETAPE_READ_ERROR since it is unused.
Signed-off-by: Borislav Petkov [EMAIL PROTECTED]
---
drivers/ide/ide-tape.c | 226 +--
1 files changed, 120 insertions(+), 106 deletions(-)
diff --git a/drivers/ide/ide-tape.c b/drivers/ide/ide-tape.c
index 4fee160..1c4f94c 100644
--- a/drivers/ide/ide-tape.c
+++ b/drivers/ide/ide-tape.c
@@ -207,24 +207,24 @@ typedef struct idetape_packet_command_s {
u8 *current_position; /* Pointer into the above
buffer */
ide_startstop_t (*callback) (ide_drive_t *);/* Called when this
packet command is completed */
u8 pc_buffer[IDETAPE_PC_BUFFER_SIZE]; /* Temporary buffer */
- unsigned long flags;/* Status/Action bit flags:
long for set_bit */
+ unsigned int flags;
} idetape_pc_t;
-/*
- * Packet command flag bits.
- */
-/* Set when an error is considered normal - We won't retry */
-#definePC_ABORT0
-/* 1 When polling for DSC on a media access command */
-#define PC_WAIT_FOR_DSC1
-/* 1 when we prefer to use DMA if possible */
-#define PC_DMA_RECOMMENDED 2
-/* 1 while DMA in progress */
-#definePC_DMA_IN_PROGRESS 3
-/* 1 when encountered problem during DMA */
-#definePC_DMA_ERROR4
-/* Data direction */
-#definePC_WRITING 5
+/* Packet command flag bits. */
+enum {
+ /* Set when an error is considered normal - We won't retry */
+ PC_FL_ABORT = (1 0),
+ /* 1 When polling for DSC on a media access command */
+ PC_FL_WAIT_FOR_DSC = (1 1),
+ /* 1 when we prefer to use DMA if possible */
+ PC_FL_DMA_RECOMMENDED = (1 2),
+ /* 1 while DMA in progress */
+ PC_FL_DMA_IN_PROGRESS = (1 3),
+ /* 1 when encountered problem during DMA */
+ PC_FL_DMA_ERROR = (1 4),
+ /* Data direction */
+ PC_FL_WRITING = (1 5),
+};
/*
* A pipeline stage.
@@ -354,8 +354,7 @@ typedef struct ide_tape_obj {
/* Wasted space in each stage */
int excess_bh_size;
- /* Status/Action flags: long for set_bit */
- unsigned long flags;
+ unsigned int flags;
/* protects the ide-tape queue */
spinlock_t lock;
@@ -458,20 +457,26 @@ static void ide_tape_put(struct ide_tape_obj *tape)
#define DOOR_LOCKED1
#define DOOR_EXPLICITLY_LOCKED 2
-/*
- * Tape flag bits values.
- */
-#define IDETAPE_IGNORE_DSC 0
-#define IDETAPE_ADDRESS_VALID 1 /* 0 When the tape position is
unknown */
-#define IDETAPE_BUSY 2 /* Device already opened */
-#define IDETAPE_PIPELINE_ERROR 3 /* Error detected in a pipeline
stage */
-#define IDETAPE_DETECT_BS 4 /* Attempt to auto-detect the
current user block size */
-#define IDETAPE_FILEMARK 5 /* Currently on a filemark */
-#define IDETAPE_DRQ_INTERRUPT 6 /* DRQ interrupt device */
-#define IDETAPE_READ_ERROR 7
-#define IDETAPE_PIPELINE_ACTIVE8 /* pipeline active */
-/* 0 = no tape is loaded, so we don't rewind after ejecting */
-#define IDETAPE_MEDIUM_PRESENT 9
+/* Tape flag bits values. */
+enum {
+ IDETAPE_FL_IGNORE_DSC = (1 0),
+ /* 0 When the tape position is unknown */
+ IDETAPE_FL_ADDRESS_VALID= (1 1),
+ /* Device already opened */
+ IDETAPE_FL_BUSY = (1 2),
+ /* Error detected in a pipeline stage */
+ IDETAPE_FL_PIPELINE_ERR = (1 3),
+ /* Attempt to auto-detect the current user block size */
+ IDETAPE_FL_DETECT_BS= (1 4),
+ /* Currently on a filemark */
+ IDETAPE_FL_FILEMARK = (1 5),
+ /* DRQ interrupt device */
+ IDETAPE_FL_DRQ_INTERRUPT= (1 6),
+ /* pipeline active */
+ IDETAPE_FL_PIPELINE_ACTIVE = (1 7),
+ /* 0 = no tape is loaded, so we don't rewind after ejecting */
+ IDETAPE_FL_MEDIUM_PRESENT = (1 8),
+};
/*
* Some defines for the READ BUFFER command
@@ -627,7 +632,7 @@ static void idetape_update_buffers (idetape_pc_t *pc)
int count;
unsigned int bcount = pc-actually_transferred;
- if (test_bit(PC_WRITING, pc-flags))
+ if (pc-flags PC_FL_WRITING)
return;
while (bcount) {
if (bh == NULL) {
@@ -703,8 +708,8 @@ static void idetape_analyze_error(ide_drive_t *drive, u8
*sense)
debug_log(DBG_ERR, pc = %x, sense key = %x, asc = %x, ascq = %x\n,
pc-c[0], tape-sense_key, tape-asc, tape-ascq);
- /* Correct pc-actually_transferred by asking the tape. */
- if (test_bit(PC_DMA_ERROR, pc-flags)) {
+ /* Correct pc-actually_transferred by
[PATCH 03/22] ide-tape: remove unreachable code chunk
tape-speed_control is set to 1 in idetape_setup(), but, in calculate_speeds()
its value is tested for being 0, 1, or 2. Remove the if-branches where
tape-speed_control != 1 since they are never executed. Also, rename
calculate_speeds() by adding driver's prefix as is with the other function
names.
Signed-off-by: Borislav Petkov [EMAIL PROTECTED]
---
drivers/ide/ide-tape.c | 22 ++
1 files changed, 10 insertions(+), 12 deletions(-)
diff --git a/drivers/ide/ide-tape.c b/drivers/ide/ide-tape.c
index c8c57ab..b15dd17 100644
--- a/drivers/ide/ide-tape.c
+++ b/drivers/ide/ide-tape.c
@@ -87,7 +87,8 @@ enum {
* the optimum value or until we reach MAX.
*
* Setting the following parameter to 0 is illegal: the pipelined mode
- * cannot be disabled (calculate_speeds() divides by tape-max_stages.)
+ * cannot be disabled (idetape_calculate_speeds() divides by
+ * tape-max_stages.)
*/
#define IDETAPE_MIN_PIPELINE_STAGES 1
#define IDETAPE_MAX_PIPELINE_STAGES400
@@ -1475,10 +1476,9 @@ static void idetape_create_mode_sense_cmd (idetape_pc_t
*pc, u8 page_code)
pc-callback = idetape_pc_callback;
}
-static void calculate_speeds(ide_drive_t *drive)
+static void idetape_calculate_speeds(ide_drive_t *drive)
{
idetape_tape_t *tape = drive-driver_data;
- int full = 125, empty = 75;
if (time_after(jiffies, tape-controlled_pipeline_head_time + 120 *
HZ)) {
tape-controlled_previous_pipeline_head =
tape-controlled_last_pipeline_head;
@@ -1505,22 +1505,20 @@ static void calculate_speeds(ide_drive_t *drive)
}
}
tape-pipeline_head_speed = max(tape-uncontrolled_pipeline_head_speed,
tape-controlled_pipeline_head_speed);
- if (tape-speed_control == 0) {
- tape-max_insert_speed = 5000;
- } else if (tape-speed_control == 1) {
+
+ 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 if (tape-speed_control == 2) {
- tape-max_insert_speed = tape-pipeline_head_speed * empty /
100 +
- (tape-pipeline_head_speed * full / 100 -
tape-pipeline_head_speed * empty / 100) * tape-nr_pending_stages /
tape-max_stages;
} else
tape-max_insert_speed = tape-speed_control;
+
tape-max_insert_speed = max(tape-max_insert_speed, 500);
}
@@ -1697,7 +1695,7 @@ static ide_startstop_t idetape_do_request(ide_drive_t
*drive,
tape-measure_insert_time = 1;
if (time_after(jiffies, tape-insert_time))
tape-insert_speed = tape-insert_size / 1024 * HZ / (jiffies -
tape-insert_time);
- calculate_speeds(drive);
+ idetape_calculate_speeds(drive);
if (!test_and_clear_bit(IDETAPE_IGNORE_DSC, tape-flags)
(stat SEEK_STAT) == 0) {
if (postponed_rq == NULL) {
@@ -2419,7 +2417,7 @@ static int idetape_add_chrdev_write_request (ide_drive_t
*drive, int blocks)
idetape_switch_buffers(tape, new_stage);
idetape_add_stage_tail(drive, new_stage);
tape-pipeline_head++;
- calculate_speeds(drive);
+ idetape_calculate_speeds(drive);
/*
* Estimate whether the tape has stopped writing by checking
@@ -2659,7 +2657,7 @@ static int idetape_add_chrdev_read_request (ide_drive_t
*drive,int blocks)
idetape_remove_stage_head(drive);
spin_unlock_irqrestore(tape-spinlock, flags);
tape-pipeline_head++;
- calculate_speeds(drive);
+ idetape_calculate_speeds(drive);
}
if (bytes_read blocks * tape-tape_block_size) {
printk(KERN_ERR ide-tape: bug: trying to return more bytes
than requested\n);
--
1.5.3.7
-
To unsubscribe from this list: send the line unsubscribe linux-ide in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 02/22] ide-tape: remove struct idetape_read_position_result_t
There should be no functional changes resulting from this patch.
Signed-off-by: Borislav Petkov [EMAIL PROTECTED]
---
drivers/ide/ide-tape.c | 49 +--
1 files changed, 18 insertions(+), 31 deletions(-)
diff --git a/drivers/ide/ide-tape.c b/drivers/ide/ide-tape.c
index 442d71c..c8c57ab 100644
--- a/drivers/ide/ide-tape.c
+++ b/drivers/ide/ide-tape.c
@@ -571,24 +571,6 @@ struct idetape_id_gcw {
unsigned protocol :2; /* Protocol type */
};
-/*
- * READ POSITION packet command - Data Format (From Table 6-57)
- */
-typedef struct {
- unsignedreserved0_10:2; /* Reserved */
- unsignedbpu :1; /* Block Position Unknown */
- unsignedreserved0_543 :3; /* Reserved */
- unsignedeop :1; /* End Of Partition */
- unsignedbop :1; /* Beginning Of Partition */
- u8 partition; /* Partition Number */
- u8 reserved2, reserved3; /* Reserved */
- u32 first_block;/* First Block Location */
- u32 last_block; /* Last Block Location
(Optional) */
- u8 reserved12; /* Reserved */
- u8 blocks_in_buffer[3];/* Blocks In Buffer -
(Optional) */
- u32 bytes_in_buffer;/* Bytes In Buffer (Optional) */
-} idetape_read_position_result_t;
-
/* Structures related to the SELECT SENSE / MODE SENSE packet commands. */
#define IDETAPE_BLOCK_DESCRIPTOR 0
#defineIDETAPE_CAPABILITIES_PAGE 0x2a
@@ -1999,30 +1981,35 @@ static void idetape_wait_for_request (ide_drive_t
*drive, struct request *rq)
spin_lock_irq(tape-spinlock);
}
-static ide_startstop_t idetape_read_position_callback (ide_drive_t *drive)
+static ide_startstop_t idetape_read_position_callback(ide_drive_t *drive)
{
idetape_tape_t *tape = drive-driver_data;
- idetape_read_position_result_t *result;
+ u8 *readpos = tape-pc-buffer;
debug_log(DBG_PROCS, Enter %s\n, __func__);
if (!tape-pc-error) {
- result = (idetape_read_position_result_t *) tape-pc-buffer;
- debug_log(DBG_SENSE, BOP - %s\n, result-bop ? Yes : No);
- debug_log(DBG_SENSE, EOP - %s\n, result-eop ? Yes : No);
+ debug_log(DBG_SENSE, BOP - %s\n,
+ !!(readpos[0] 0x80) ? Yes : No);
+ debug_log(DBG_SENSE, EOP - %s\n,
+ !!(readpos[0] 0x40) ? Yes : No);
+
+ if (!!(readpos[0] 0x4)) {
+ printk(KERN_INFO ide-tape: Block location is unknown
+ to the tape\n);
- if (result-bpu) {
- printk(KERN_INFO ide-tape: Block location is unknown
to the tape\n);
clear_bit(IDETAPE_ADDRESS_VALID, tape-flags);
idetape_end_request(drive, 0, 0);
} else {
debug_log(DBG_SENSE, Block Location - %u\n,
- ntohl(result-first_block));
-
- tape-partition = result-partition;
- tape-first_frame_position = ntohl(result-first_block);
- tape-last_frame_position = ntohl(result-last_block);
- tape-blocks_in_buffer = result-blocks_in_buffer[2];
+ be32_to_cpu(*(u32 *)readpos[4]));
+
+ tape-partition = readpos[1];
+ tape-first_frame_position =
+ be32_to_cpu(*(u32 *)readpos[4]);
+ tape-last_frame_position =
+ be32_to_cpu(*(u32 *)readpos[8]);
+ tape-blocks_in_buffer = readpos[15];
set_bit(IDETAPE_ADDRESS_VALID, tape-flags);
idetape_end_request(drive, 1, 0);
}
--
1.5.3.7
-
To unsubscribe from this list: send the line unsubscribe linux-ide in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] libata-core: unblacklist HITACHI drives
The HITACHI HDS7250SASUN500G and HITACHI HDS7225SBSUN250 drives
do not need to be blacklisted, the NCQ problem has been resolved
with the sata_nv: fix for completion handling patch.
Signed-off-by David Milburn [EMAIL PROTECTED]
---
libata-core.c |2 --
1 files changed, 2 deletions(-)
diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index bdbd55a..ba5406e 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -4154,8 +4154,6 @@ static const struct ata_blacklist_entry
ata_device_blacklist [] = {
/* NCQ is broken */
{ Maxtor *, BANC*,ATA_HORKAGE_NONCQ },
{ Maxtor 7V300F0, VA111630, ATA_HORKAGE_NONCQ },
- { HITACHI HDS7250SASUN500G*, NULL,ATA_HORKAGE_NONCQ },
- { HITACHI HDS7225SBSUN250G*, NULL,ATA_HORKAGE_NONCQ },
{ ST380817AS, 3.42, ATA_HORKAGE_NONCQ },
{ ST3160023AS,3.42, ATA_HORKAGE_NONCQ },
-
To unsubscribe from this list: send the line unsubscribe linux-ide in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 13/22] ide-tape: remove struct idetape_id_gcw
Signed-off-by: Borislav Petkov [EMAIL PROTECTED]
---
drivers/ide/ide-tape.c | 58 ---
1 files changed, 25 insertions(+), 33 deletions(-)
diff --git a/drivers/ide/ide-tape.c b/drivers/ide/ide-tape.c
index 712c5df..175d507 100644
--- a/drivers/ide/ide-tape.c
+++ b/drivers/ide/ide-tape.c
@@ -520,20 +520,6 @@ enum {
#defineIDETAPE_ERROR_FILEMARK 102
#defineIDETAPE_ERROR_EOD 103
-/*
- * The following is used to format the general configuration word of
- * the ATAPI IDENTIFY DEVICE command.
- */
-struct idetape_id_gcw {
- unsigned packet_size:2; /* Packet Size */
- unsigned reserved234:3; /* Reserved */
- unsigned drq_type :2; /* Command packet DRQ type */
- unsigned removable :1; /* Removable media */
- unsigned device_type:5; /* Device type */
- unsigned reserved13 :1; /* Reserved */
- unsigned protocol :2; /* Protocol type */
-};
-
/* Structures related to the SELECT SENSE / MODE SENSE packet commands. */
#define IDETAPE_BLOCK_DESCRIPTOR 0
#defineIDETAPE_CAPABILITIES_PAGE 0x2a
@@ -3382,37 +3368,41 @@ static int idetape_chrdev_release (struct inode *inode,
struct file *filp)
}
/*
- * idetape_identify_device is called to check the contents of the
- * ATAPI IDENTIFY command results. We return:
+ * check the contents of the ATAPI IDENTIFY command results. We return:
*
- * 1 If the tape can be supported by us, based on the information
- * we have so far.
+ * 1 - If the tape can be supported by us, based on the information we have so
+ * far.
*
- * 0 If this tape driver is not currently supported by us.
+ * 0 - If this tape driver is not currently supported by us.
*/
-static int idetape_identify_device (ide_drive_t *drive)
+static int idetape_identify_device(ide_drive_t *drive)
{
- struct idetape_id_gcw gcw;
- struct hd_driveid *id = drive-id;
+
+ u8 gcw[2];
+ u8 protocol, device_type, removable, packet_size;
if (drive-id_read == 0)
return 1;
- *((unsigned short *) gcw) = id-config;
+ *((unsigned short *) gcw) = drive-id-config;
- /* Check that we can support this device */
+ protocol= (gcw[1] 0xC0) 6;
+ device_type =gcw[1] 0x1F;
+ removable = !!(gcw[0] 0x80);
+ packet_size =gcw[0] 0x3;
- if (gcw.protocol != 2)
+ /* Check that we can support this device */
+ if (protocol != 2)
printk(KERN_ERR ide-tape: Protocol (0x%02x) is not ATAPI\n,
- gcw.protocol);
- else if (gcw.device_type != 1)
+ protocol);
+ else if (device_type != 1)
printk(KERN_ERR ide-tape: Device type (0x%02x) is not set
- to tape\n, gcw.device_type);
- else if (!gcw.removable)
+ to tape\n, device_type);
+ else if (!removable)
printk(KERN_ERR ide-tape: The removable flag is not set\n);
- else if (gcw.packet_size != 0) {
+ else if (packet_size != 0) {
printk(KERN_ERR ide-tape: Packet size (0x%02x) is not 12
- bytes long\n, gcw.packet_size);
+ bytes long\n, packet_size);
} else
return 1;
return 0;
@@ -3541,8 +3531,8 @@ static void idetape_setup (ide_drive_t *drive,
idetape_tape_t *tape, int minor)
{
unsigned long t1, tmid, tn, t;
int speed;
- struct idetape_id_gcw gcw;
int stage_size;
+ u8 gcw[2];
struct sysinfo si;
u16 *ctl = (u16 *)tape-caps[12];
@@ -3564,7 +3554,9 @@ static void idetape_setup (ide_drive_t *drive,
idetape_tape_t *tape, int minor)
tape-max_insert_speed = 1;
tape-speed_control = 1;
*((unsigned short *) gcw) = drive-id-config;
- if (gcw.drq_type == 1)
+
+ /* Command packet DRQ type */
+ if (((gcw[0] 0x60) 5) == 1)
tape-flags |= IDETAPE_FL_DRQ_INTERRUPT;
tape-min_pipeline = tape-max_pipeline = tape-max_stages = 10;
--
1.5.3.7
-
To unsubscribe from this list: send the line unsubscribe linux-ide in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
[no subject]
Hi Bart, here are the pending ide-tape patches reworked which incorporate all review points raised so far. Several new patches are appended to the original series which i thought would be reasonable to sumbit along with the others. Also, i've applied ide-tape: dump gcw fields on error in idetape_identify_device() which is #12 and which you can simply ignore. Furthermore, #32 from the original series got split up into the different logical changes it dealt with, as you requested. Documentation/feature-removal-schedule.txt | 14 +- drivers/ide/ide-tape.c | 2764 +--- 2 files changed, 1325 insertions(+), 1453 deletions(-) - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 08/22] ide-tape: remove packet command and struct request memory buffers
These buffers were always statically allocated during driver initialization no
matter what. Remove them by allocating GFP_ATOMIC memory on demand. In the case
of allocation error, we only issue error msg in the *alloc_{pc,rq} thus
postponing
the final error handling and cleanup in their callers. Packet command and
request memory is freed in idetape_end_request() which is at the end of the
request path entered from all callbacks. While at it, integrate comments above
member definitions in ide_tape_obj.
Signed-off-by: Borislav Petkov [EMAIL PROTECTED]
---
drivers/ide/ide-tape.c | 146 +---
1 files changed, 63 insertions(+), 83 deletions(-)
diff --git a/drivers/ide/ide-tape.c b/drivers/ide/ide-tape.c
index 0b5ccce..b4944ef 100644
--- a/drivers/ide/ide-tape.c
+++ b/drivers/ide/ide-tape.c
@@ -54,8 +54,6 @@ enum {
DBG_CHRDEV =(1 2),
/* all remaining procedures */
DBG_PROCS = (1 3),
- /* buffer alloc info (pc_stack rq_stack) */
- DBG_PCRQ_STACK =(1 4),
};
/* define to see debug info */
@@ -110,13 +108,6 @@ enum {
#define IDETAPE_PC_BUFFER_SIZE 256
/*
- * In various places in the driver, we need to allocate storage
- * for packet commands and requests, which will remain valid while
- * we leave the driver to wait for an interrupt or a timeout event.
- */
-#define IDETAPE_PC_STACK (10 + IDETAPE_MAX_PC_RETRIES)
-
-/*
* Some drives (for example, Seagate STT3401A Travan) require a very long
* timeout, because they don't return an interrupt or clear their busy bit
* until after the command completes (even retension commands).
@@ -255,32 +246,15 @@ typedef struct ide_tape_obj {
struct gendisk *disk;
struct kref kref;
+ /* current processed packet command */
+ idetape_pc_t *pc;
/*
-* Since a typical character device operation requires more
-* than one packet command, we provide here enough memory
-* for the maximum of interconnected packet commands.
-* The packet commands are stored in the circular array pc_stack.
-* pc_stack_index points to the last used entry, and warps around
-* to the start when we get to the last array entry.
-*
-* pc points to the current processed packet command.
-*
-* failed_pc points to the last failed packet command, or contains
-* NULL if we do not need to retry any packet command. This is
-* required since an additional packet command is needed before the
-* retry, to get detailed information on what went wrong.
+* last failed packet command or NULL if we do not need to retry any
+* packet command. This is required since an additional packet command
+* is needed before the retry, to get detailed information on what went
+* wrong.
*/
- /* Current packet command */
- idetape_pc_t *pc;
- /* Last failed packet command */
idetape_pc_t *failed_pc;
- /* Packet command stack */
- idetape_pc_t pc_stack[IDETAPE_PC_STACK];
- /* Next free packet command storage space */
- int pc_stack_index;
- struct request rq_stack[IDETAPE_PC_STACK];
- /* We implement a circular array */
- int rq_stack_index;
/*
* DSC polling variables.
@@ -670,45 +644,32 @@ static void idetape_update_buffers (idetape_pc_t *pc)
pc-bh = bh;
}
-/*
- * idetape_next_pc_storage returns a pointer to a place in which we can
- * safely store a packet command, even though we intend to leave the
- * driver. A storage space for a maximum of IDETAPE_PC_STACK packet
- * commands is allocated at initialization time.
- */
-static idetape_pc_t *idetape_next_pc_storage (ide_drive_t *drive)
+static idetape_pc_t *ide_tape_alloc_pc(void)
{
- idetape_tape_t *tape = drive-driver_data;
+ idetape_pc_t *pc;
- debug_log(DBG_PCRQ_STACK, pc_stack_index=%d\n, tape-pc_stack_index);
+ pc = kzalloc(sizeof(idetape_pc_t), GFP_ATOMIC);
+ if (!pc)
+ printk(KERN_ERR ide-tape: %s: memory allocation error.,
+ __func__);
- if (tape-pc_stack_index == IDETAPE_PC_STACK)
- tape-pc_stack_index=0;
- return (tape-pc_stack[tape-pc_stack_index++]);
+ return pc;
}
/*
- * idetape_next_rq_storage is used along with idetape_next_pc_storage.
- * Since we queue packet commands in the request queue, we need to
- * allocate a request, along with the allocation of a packet command.
+ * Since we queue packet commands in the request queue, we need to allocate a
+ * request, along with a packet command.
*/
-
-/**
- **
- * This
Re: AHCI driver preferring nr_ports over port map
Well, two values don't agree with each other and we know for a fact that vendors sometimes get PI wrong, so we trust n_ports in such cases. We can reverse the behavior but that's likely to cause more problems than it fixes. I understand your concern, but I think you also understand mine. So I'm not really asking for general reversal of the logic, but to perhaps make it just a little smarter. The (not generally usable according to what you said earlier) experiment I made was to use the smaller of the two sets - if either set is empty this of course wouldn't be correct. But perhaps, if you have a non-empty port map, that should be preferred over nr_ports? Otherwise, chipset specific knowledge may need to be applied here (i.e. for ESB2, port map ought to always be used)... Jan - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] ide: another possible ide panic fix for blk-end-request
Hi Jens, Bart, Boris, I have reviewed all blk-end-request patches again to confirm whether there are any similar problems with the last week's ide-cd panic: http://lkml.org/lkml/2008/1/29/140 And I found a possible similar bug in ide-io change: ide_end_drive_cmd() could be called for blk_pc_request() which could have bios. To complete such requests correctly, we need to pass the actual size of the request. Otherwise, __blk_end_request() returns 1 because the request still has bios, and the system will BUG() unnecessarily. The following patch fixes the bug and should be applied on top of Linus' git. Please review and apply. Cc: Bartlomiej Zolnierkiewicz [EMAIL PROTECTED] Cc: Borislav Petkov [EMAIL PROTECTED] Signed-off-by: Kiyoshi Ueda [EMAIL PROTECTED] Signed-off-by: Jun'ichi Nomura [EMAIL PROTECTED] --- drivers/ide/ide-io.c |3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) --- a/drivers/ide/ide-io.c 2008-02-01 18:20:02.0 -0500 +++ b/drivers/ide/ide-io.c 2008-02-01 18:21:50.0 -0500 @@ -388,7 +388,8 @@ void ide_end_drive_cmd (ide_drive_t *dri spin_lock_irqsave(ide_lock, flags); HWGROUP(drive)-rq = NULL; rq-errors = err; - if (__blk_end_request(rq, (rq-errors ? -EIO : 0), 0)) + if (unlikely(__blk_end_request(rq, (rq-errors ? -EIO : 0), + blk_rq_bytes(rq BUG(); spin_unlock_irqrestore(ide_lock, flags); } Thanks, Kiyoshi Ueda - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [BUG] 2.6.24 refuses to boot - ATA problem?
Gene Heskett wrote: On Sunday 03 February 2008, Ingo Molnar wrote: * Gene Heskett [EMAIL PROTECTED] wrote: I believe its the same, but lemme paste it for sure, yes: [ 26.339926] ENABLING IO-APIC IRQs [ 26.340119] ..TIMER: vector=0x31 apic1=0 pin1=0 apic2=-1 pin2=-1 [ 26.350129] ..MP-BIOS bug: 8254 timer not connected to IO-APIC [ 26.350182] ...trying to set up timer (IRQ0) through the 8259A ... failed. [ 26.350185] ...trying to set up timer as Virtual Wire IRQ... failed. [ 26.360186] ...trying to set up timer as ExtINT IRQ... works. The third line is the only line that makes it to the screen during the boot trace. Now, what does this tell us? the question would be: - if you remove the acpi_use_timer_override boot flag - and if you boot a kernel with this hack applied = do those weird PATA failures come back? If the failues do _not_ come back then the problem is somehow affected/worked-around by the IO-APIC code that generates the above 4 lines. If the failures are still the same then the above 4 lines are really just an uninteresting side-effect of the acpi_use_timer_override flag - and the real side-effects (that fixes PATA on your box) are to be found elsewhere. Sadly, the latter variant is the expected answer. Ingo And at this point, I can't tell. This reboot was from a cold start, without the argument, and cold by long enough to make the rounds about the house and pick up a beer, but not take my evening pillbox. A minute cold, maybe 2 max. The log is clean since except for a kudzu nag of some sort: .. Just to muddy your observations: it is quite possible that a cold (power-off) reboot may be required to properly observe what happens here. Cheers - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 21/22] ide-tape: bump minor driver version
Signed-off-by: Borislav Petkov [EMAIL PROTECTED] --- drivers/ide/ide-tape.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/drivers/ide/ide-tape.c b/drivers/ide/ide-tape.c index 5c5..2e6198f 100644 --- a/drivers/ide/ide-tape.c +++ b/drivers/ide/ide-tape.c @@ -15,7 +15,7 @@ * Documentation/ide/ChangeLog.ide-tape.1995-2002 */ -#define IDETAPE_VERSION 1.19 +#define IDETAPE_VERSION 1.20 #include linux/module.h #include linux/types.h -- 1.5.3.7 - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/3] libata: implement drain buffers
This just updates the libata slave configure routine to take advantage
of the block layer drain buffers. It also adjusts the size lengths in
the atapi code to add the drain buffer to the DMA length so the driver
knows it can rely on it.
I suspect I should also be checking for AHCI as well as ATA_DEV_ATAPI,
but I couldn't see how to do that easily.
Signed-off-by: James Bottomley [EMAIL PROTECTED]
---
drivers/ata/libata-scsi.c | 30 ++
1 files changed, 26 insertions(+), 4 deletions(-)
diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index 844..acf6a8b 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -826,8 +826,8 @@ static void ata_scsi_sdev_config(struct scsi_device *sdev)
sdev-max_device_blocked = 1;
}
-static void ata_scsi_dev_config(struct scsi_device *sdev,
- struct ata_device *dev)
+static int ata_scsi_dev_config(struct scsi_device *sdev,
+ struct ata_device *dev)
{
/* configure max sectors */
blk_queue_max_sectors(sdev-request_queue, dev-max_sectors);
@@ -839,6 +839,16 @@ static void ata_scsi_dev_config(struct scsi_device *sdev,
sdev-manage_start_stop = 1;
}
+ if (dev-class == ATA_DEV_ATAPI) {
+ struct request_queue *q = sdev-request_queue;
+ void *buf = kmalloc(ATAPI_MAX_DRAIN, GFP_KERNEL);
+ if (!buf) {
+ sdev_printk(KERN_ERR, sdev, drain buffer allocation
failed\n);
+ return -ENOMEM;
+ }
+ blk_queue_dma_drain(q, buf, ATAPI_MAX_DRAIN);
+ }
+
if (dev-flags ATA_DFLAG_AN)
set_bit(SDEV_EVT_MEDIA_CHANGE, sdev-supported_events);
@@ -849,6 +859,8 @@ static void ata_scsi_dev_config(struct scsi_device *sdev,
depth = min(ATA_MAX_QUEUE - 1, depth);
scsi_adjust_queue_depth(sdev, MSG_SIMPLE_TAG, depth);
}
+
+ return 0;
}
/**
@@ -867,13 +879,14 @@ int ata_scsi_slave_config(struct scsi_device *sdev)
{
struct ata_port *ap = ata_shost_to_port(sdev-host);
struct ata_device *dev = __ata_scsi_find_dev(ap, sdev);
+ int rc = 0;
ata_scsi_sdev_config(sdev);
if (dev)
- ata_scsi_dev_config(sdev, dev);
+ rc = ata_scsi_dev_config(sdev, dev);
- return 0;
+ return rc;
}
/**
@@ -895,6 +908,7 @@ void ata_scsi_slave_destroy(struct scsi_device *sdev)
struct ata_port *ap = ata_shost_to_port(sdev-host);
unsigned long flags;
struct ata_device *dev;
+ struct request_queue *q = sdev-request_queue;
if (!ap-ops-error_handler)
return;
@@ -908,6 +922,10 @@ void ata_scsi_slave_destroy(struct scsi_device *sdev)
ata_port_schedule_eh(ap);
}
spin_unlock_irqrestore(ap-lock, flags);
+
+ kfree(q-dma_drain_buffer);
+ q-dma_drain_buffer = NULL;
+ q-dma_drain_size = 0;
}
/**
@@ -2478,6 +2496,8 @@ static unsigned int atapi_xlat(struct ata_queued_cmd *qc)
qc-tf.command = ATA_CMD_PACKET;
qc-nbytes = scsi_bufflen(scmd);
+ if (blk_pc_request(scmd-request))
+ qc-nbytes += blk_rq_drain_size(scmd-request);
/* check whether ATAPI DMA is safe */
if (!using_pio ata_check_atapi_dma(qc))
@@ -2814,6 +2834,8 @@ static unsigned int ata_scsi_pass_thru(struct
ata_queued_cmd *qc)
* cover scatter/gather case.
*/
qc-nbytes = scsi_bufflen(scmd);
+ if (ata_is_atapi(qc-tf.protocol))
+ qc-nbytes += blk_rq_drain_size(scmd-request);
/* request result TF and be quiet about device error */
qc-flags |= ATA_QCFLAG_RESULT_TF | ATA_QCFLAG_QUIET;
--
1.5.3.8
-
To unsubscribe from this list: send the line unsubscribe linux-ide in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/3] block: add blk_rq_drain_size() API
Signed-off-by: James Bottomley [EMAIL PROTECTED]
---
include/linux/blkdev.h |5 +
1 files changed, 5 insertions(+), 0 deletions(-)
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 90392a9..a526066 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -676,6 +676,11 @@ extern void blk_complete_request(struct request *);
extern unsigned int blk_rq_bytes(struct request *rq);
extern unsigned int blk_rq_cur_bytes(struct request *rq);
+static inline int blk_rq_drain_size(struct request *rq)
+{
+ return rq-q-dma_drain_size;
+}
+
static inline void blkdev_dequeue_request(struct request *req)
{
elv_dequeue_request(req-q, req);
--
1.5.3.8
-
To unsubscribe from this list: send the line unsubscribe linux-ide in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 22/22] ide-tape: schedule driver for removal after 6 months in case it turns out
Signed-off-by: Borislav Petkov [EMAIL PROTECTED] --- Documentation/feature-removal-schedule.txt | 14 -- drivers/ide/ide-tape.c |5 + 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/Documentation/feature-removal-schedule.txt b/Documentation/feature-removal-schedule.txt index a7d9d17..147bb63 100644 --- a/Documentation/feature-removal-schedule.txt +++ b/Documentation/feature-removal-schedule.txt @@ -264,8 +264,6 @@ Who:Thomas Gleixner [EMAIL PROTECTED] --- - What: i2c-i810, i2c-prosavage and i2c-savage4 When: May 2008 Why: These drivers are superseded by i810fb, intelfb and savagefb. @@ -338,3 +336,15 @@ Why: The support code for the old firmware hurts code readability/maintainabilit and slightly hurts runtime performance. Bugfixes for the old firmware are not provided by Broadcom anymore. Who: Michael Buesch [EMAIL PROTECTED] + +--- + +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 [EMAIL PROTECTED], Borislav Petkov + [EMAIL PROTECTED] + +--- diff --git a/drivers/ide/ide-tape.c b/drivers/ide/ide-tape.c index 2e6198f..86f206b 100644 --- a/drivers/ide/ide-tape.c +++ b/drivers/ide/ide-tape.c @@ -3803,6 +3803,11 @@ 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 [EMAIL PROTECTED] or Boris +[EMAIL PROTECTED] in case you still need it.\n); + return 0; out_free_tape: -- 1.5.3.7 - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 19/22] ide-tape: fix syntax error in idetape_identify_device()
Spotted by Sergei Shtylyov.
CC: Sergei Shtylyov [EMAIL PROTECTED]
Signed-off-by: Borislav Petkov [EMAIL PROTECTED]
---
drivers/ide/ide-tape.c |4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/ide/ide-tape.c b/drivers/ide/ide-tape.c
index 4a12c0b..7e998c4 100644
--- a/drivers/ide/ide-tape.c
+++ b/drivers/ide/ide-tape.c
@@ -3273,8 +3273,8 @@ static int idetape_identify_device(ide_drive_t *drive)
else if (!removable)
printk(KERN_ERR ide-tape: The removable flag is not set\n);
else if (packet_size != 0) {
- printk(KERN_ERR ide-tape: Packet size (0x%02x) is not 12
- bytes long\n, packet_size);
+ printk(KERN_ERR ide-tape: Packet size (0x%02x) is not 12
+bytes\n, packet_size);
} else
return 1;
return 0;
--
1.5.3.7
-
To unsubscribe from this list: send the line unsubscribe linux-ide in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 05/22] ide-tape: remove typedef idetape_chrdev_direction_t
.. and replace it with plain enums.
Signed-off-by: Borislav Petkov [EMAIL PROTECTED]
---
drivers/ide/ide-tape.c | 62 ---
1 files changed, 32 insertions(+), 30 deletions(-)
diff --git a/drivers/ide/ide-tape.c b/drivers/ide/ide-tape.c
index 2857965..e0e8184 100644
--- a/drivers/ide/ide-tape.c
+++ b/drivers/ide/ide-tape.c
@@ -184,11 +184,13 @@ enum {
/*
* For general magnetic tape device compatibility.
*/
-typedef enum {
- idetape_direction_none,
- idetape_direction_read,
- idetape_direction_write
-} idetape_chrdev_direction_t;
+
+/* tape directions */
+enum {
+ IDETAPE_DIR_NONE = (1 0),
+ IDETAPE_DIR_READ = (1 1),
+ IDETAPE_DIR_WRITE = (1 2),
+};
struct idetape_bh {
u32 b_size;
@@ -324,7 +326,7 @@ typedef struct ide_tape_obj {
/* device name */
char name[4];
/* Current character device data transfer direction */
- idetape_chrdev_direction_t chrdev_direction;
+ u8 chrdev_dir;
/*
* Device information
@@ -1916,7 +1918,7 @@ static void idetape_init_merge_stage (idetape_tape_t
*tape)
struct idetape_bh *bh = tape-merge_stage-bh;
tape-bh = bh;
- if (tape-chrdev_direction == idetape_direction_write)
+ if (tape-chrdev_dir == IDETAPE_DIR_WRITE)
atomic_set(bh-b_count, 0);
else {
tape-b_data = bh-b_data;
@@ -2187,7 +2189,7 @@ static int __idetape_discard_read_pipeline (ide_drive_t
*drive)
unsigned long flags;
int cnt;
- if (tape-chrdev_direction != idetape_direction_read)
+ if (tape-chrdev_dir != IDETAPE_DIR_READ)
return 0;
/* Remove merge stage. */
@@ -2202,7 +2204,7 @@ static int __idetape_discard_read_pipeline (ide_drive_t
*drive)
/* Clear pipeline flags. */
clear_bit(IDETAPE_PIPELINE_ERROR, tape-flags);
- tape-chrdev_direction = idetape_direction_none;
+ tape-chrdev_dir = IDETAPE_DIR_NONE;
/* Remove pipeline stages. */
if (tape-first_stage == NULL)
@@ -2242,7 +2244,7 @@ static int idetape_position_tape (ide_drive_t *drive,
unsigned int block, u8 par
int retval;
idetape_pc_t pc;
- if (tape-chrdev_direction == idetape_direction_read)
+ 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);
@@ -2469,7 +2471,7 @@ static void idetape_empty_write_pipeline (ide_drive_t
*drive)
int blocks, min;
struct idetape_bh *bh;
- if (tape-chrdev_direction != idetape_direction_write) {
+ 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;
}
@@ -2512,7 +2514,7 @@ static void idetape_empty_write_pipeline (ide_drive_t
*drive)
tape-merge_stage = NULL;
}
clear_bit(IDETAPE_PIPELINE_ERROR, tape-flags);
- tape-chrdev_direction = idetape_direction_none;
+ tape-chrdev_dir = IDETAPE_DIR_NONE;
/*
* On the next backup, perform the feedback loop again.
@@ -2556,8 +2558,8 @@ static int idetape_initiate_read (ide_drive_t *drive, int
max_stages)
u16 blocks = *(u16 *)tape-caps[12];
/* Initialize read operation */
- if (tape-chrdev_direction != idetape_direction_read) {
- if (tape-chrdev_direction == idetape_direction_write) {
+ if (tape-chrdev_dir != IDETAPE_DIR_READ) {
+ if (tape-chrdev_dir == IDETAPE_DIR_WRITE) {
idetape_empty_write_pipeline(drive);
idetape_flush_tape_buffers(drive);
}
@@ -2567,7 +2569,7 @@ static int idetape_initiate_read (ide_drive_t *drive, int
max_stages)
}
if ((tape-merge_stage = __idetape_kmalloc_stage(tape, 0, 0))
== NULL)
return -ENOMEM;
- tape-chrdev_direction = idetape_direction_read;
+ tape-chrdev_dir = IDETAPE_DIR_READ;
/*
* Issue a read 0 command to ensure that DSC handshake
@@ -2581,7 +2583,7 @@ static int idetape_initiate_read (ide_drive_t *drive, int
max_stages)
if (bytes_read 0) {
__idetape_kfree_stage(tape-merge_stage);
tape-merge_stage = NULL;
- tape-chrdev_direction = idetape_direction_none;
+ tape-chrdev_dir = IDETAPE_DIR_NONE;
return bytes_read;
}
}
@@ -2802,7 +2804,7 @@ static int idetape_space_over_filemarks (ide_drive_t
*drive,short mt_op,int mt_c
mt_count = -
[PATCH 1/3] libata: eliminate the home grown dma padding in favour of that provided by the block layer
ATA requires that all DMA transfers begin and end on word boundaries.
Because of this, a large amount of machinery grew up in ide to adjust
scatterlists on this basis. However, as of 2.5, the block layer has a
dma_alignment variable which ensures both the beginning and length of a
DMA transfer are aligned on the dma_alignment boundary. Although the
block layer does adjust the beginning of the transfer to ensure this
happens, it doesn't actually adjust the length, it merely makes sure
that space is allocated for transfers beyond the declared length. The
upshot of this is that scatterlists may be padded to any size between
the actual length and the length adjusted to the dma_alignment safely
knowing that memory is allocated in this region.
Right at the moment, SCSI takes the default dma_aligment which is on a
512 byte boundary. Note that this aligment only applies to transfers
coming in from user space. However, since all kernel allocations are
automatically aligned on a minimum of 32 byte boundaries, it is safe to
adjust them in this manner as well.
Signed-off-by: James Bottomley [EMAIL PROTECTED]
---
drivers/ata/ahci.c|5 --
drivers/ata/libata-core.c | 146 +---
drivers/ata/libata-scsi.c | 23 +-
drivers/ata/pata_icside.c |8 --
drivers/ata/sata_fsl.c| 17 +
drivers/ata/sata_mv.c |6 +--
drivers/ata/sata_sil24.c |5 --
drivers/scsi/ipr.c|4 +-
drivers/scsi/libsas/sas_ata.c |4 +-
include/linux/libata.h| 28 +
10 files changed, 31 insertions(+), 215 deletions(-)
diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c
index 27c8d56..e75966b 100644
--- a/drivers/ata/ahci.c
+++ b/drivers/ata/ahci.c
@@ -1979,16 +1979,11 @@ static int ahci_port_start(struct ata_port *ap)
struct ahci_port_priv *pp;
void *mem;
dma_addr_t mem_dma;
- int rc;
pp = devm_kzalloc(dev, sizeof(*pp), GFP_KERNEL);
if (!pp)
return -ENOMEM;
- rc = ata_pad_alloc(ap, dev);
- if (rc)
- return rc;
-
mem = dmam_alloc_coherent(dev, AHCI_PORT_PRIV_DMA_SZ, mem_dma,
GFP_KERNEL);
if (!mem)
diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index bdbd55a..679a404 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -60,6 +60,8 @@
#include linux/io.h
#include scsi/scsi.h
#include scsi/scsi_cmnd.h
+#include scsi/scsi_device.h
+#include scsi/scsi_dbg.h
#include scsi/scsi_host.h
#include linux/libata.h
#include asm/semaphore.h
@@ -4476,30 +4478,13 @@ void ata_sg_clean(struct ata_queued_cmd *qc)
struct ata_port *ap = qc-ap;
struct scatterlist *sg = qc-sg;
int dir = qc-dma_dir;
- void *pad_buf = NULL;
WARN_ON(sg == NULL);
- VPRINTK(unmapping %u sg elements\n, qc-mapped_n_elem);
+ VPRINTK(unmapping %u sg elements\n, qc-n_elem);
- /* if we padded the buffer out to 32-bit bound, and data
-* xfer direction is from-device, we must copy from the
-* pad buffer back into the supplied buffer
-*/
- if (qc-pad_len !(qc-tf.flags ATA_TFLAG_WRITE))
- pad_buf = ap-pad + (qc-tag * ATA_DMA_PAD_SZ);
-
- if (qc-mapped_n_elem)
- dma_unmap_sg(ap-dev, sg, qc-mapped_n_elem, dir);
- /* restore last sg */
- if (qc-last_sg)
- *qc-last_sg = qc-saved_last_sg;
- if (pad_buf) {
- struct scatterlist *psg = qc-extra_sg[1];
- void *addr = kmap_atomic(sg_page(psg), KM_IRQ0);
- memcpy(addr + psg-offset, pad_buf, qc-pad_len);
- kunmap_atomic(addr, KM_IRQ0);
- }
+ if (qc-n_elem)
+ dma_unmap_sg(ap-dev, sg, qc-n_elem, dir);
qc-flags = ~ATA_QCFLAG_DMAMAP;
qc-sg = NULL;
@@ -4765,97 +4750,6 @@ void ata_sg_init(struct ata_queued_cmd *qc, struct
scatterlist *sg,
qc-cursg = qc-sg;
}
-static unsigned int ata_sg_setup_extra(struct ata_queued_cmd *qc,
- unsigned int *n_elem_extra,
- unsigned int *nbytes_extra)
-{
- struct ata_port *ap = qc-ap;
- unsigned int n_elem = qc-n_elem;
- struct scatterlist *lsg, *copy_lsg = NULL, *tsg = NULL, *esg = NULL;
-
- *n_elem_extra = 0;
- *nbytes_extra = 0;
-
- /* needs padding? */
- qc-pad_len = qc-nbytes 3;
-
- if (likely(!qc-pad_len))
- return n_elem;
-
- /* locate last sg and save it */
- lsg = sg_last(qc-sg, n_elem);
- qc-last_sg = lsg;
- qc-saved_last_sg = *lsg;
-
- sg_init_table(qc-extra_sg, ARRAY_SIZE(qc-extra_sg));
-
- if (qc-pad_len) {
- struct scatterlist *psg = qc-extra_sg[1];
- void *pad_buf = ap-pad + (qc-tag * ATA_DMA_PAD_SZ);
- unsigned int
Re: (fwd) Bug#11922: I/O error on blank tapes
On Mon, 2008-02-04 at 22:28 +0100, Borislav Petkov wrote: On Mon, Feb 04, 2008 at 03:22:06PM +0100, maximilian attems wrote: (Added Bart to CC) hello borislav, may i forward you that *old* Debian kernel bug, have seen you working on ide-tape: http://bugs.debian.org/11922 no we don't carry any ide patches anymore. maybe you've already fixed it in latest? thanks -- maks - Forwarded message from Stephen Kitt [EMAIL PROTECTED] - Subject: Bug#11922: I/O error on blank tapes Date: Sat, 1 Dec 2007 19:06:18 +0100 From: Stephen Kitt [EMAIL PROTECTED] To: [EMAIL PROTECTED] Hi, This does still occur with 2.6.22; with a blank tape in my HP DDS-4 drive: $ tar tzvf /dev/nst0 tar: /dev/nst0: Cannot read: Input/output error That's a SCSI tape, not an IDE one. I cc'd the SCSI list James tar: At beginning of tape, quitting now tar: Error is not recoverable: exiting now gzip: stdin: unexpected end of file tar: Child returned status 2 tar: Error exit delayed from previous errors Nothing gets logged anywhere, which fits the original bug description. This is a well-known issue: see for example http://www.sibbald.com/bacula/html-manual/Bacula_Console.html (search for blank tape). Regards, Stephen -- To UNSUBSCRIBE, email to [EMAIL PROTECTED] with a subject of unsubscribe. Trouble? Contact [EMAIL PROTECTED] - End forwarded message - Hi Maks, we're currently in the process of aggressively cleaning up ide-tape. However, this brings (almost) no functional changes to the driver and we haven't looked at any bugs that might exist. Actually, I wanted to probe the community to see whether anyone is using ide-tape at all, and if not, to remove it completely. Since i don't have the hardware, i'm gonna have to ask you (or Stephen) to wait until all changes have entered mainline and then to try to reproduce the bug again after having enabled debugging (IDETAPE_DEBUG_LOG) and send me the syslog output. Thanks. - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: (fwd) Bug#11922: I/O error on blank tapes
On Mon, Feb 04, 2008 at 03:22:06PM +0100, maximilian attems wrote: (Added Bart to CC) hello borislav, may i forward you that *old* Debian kernel bug, have seen you working on ide-tape: http://bugs.debian.org/11922 no we don't carry any ide patches anymore. maybe you've already fixed it in latest? thanks -- maks - Forwarded message from Stephen Kitt [EMAIL PROTECTED] - Subject: Bug#11922: I/O error on blank tapes Date: Sat, 1 Dec 2007 19:06:18 +0100 From: Stephen Kitt [EMAIL PROTECTED] To: [EMAIL PROTECTED] Hi, This does still occur with 2.6.22; with a blank tape in my HP DDS-4 drive: $ tar tzvf /dev/nst0 tar: /dev/nst0: Cannot read: Input/output error tar: At beginning of tape, quitting now tar: Error is not recoverable: exiting now gzip: stdin: unexpected end of file tar: Child returned status 2 tar: Error exit delayed from previous errors Nothing gets logged anywhere, which fits the original bug description. This is a well-known issue: see for example http://www.sibbald.com/bacula/html-manual/Bacula_Console.html (search for blank tape). Regards, Stephen -- To UNSUBSCRIBE, email to [EMAIL PROTECTED] with a subject of unsubscribe. Trouble? Contact [EMAIL PROTECTED] - End forwarded message - Hi Maks, we're currently in the process of aggressively cleaning up ide-tape. However, this brings (almost) no functional changes to the driver and we haven't looked at any bugs that might exist. Actually, I wanted to probe the community to see whether anyone is using ide-tape at all, and if not, to remove it completely. Since i don't have the hardware, i'm gonna have to ask you (or Stephen) to wait until all changes have entered mainline and then to try to reproduce the bug again after having enabled debugging (IDETAPE_DEBUG_LOG) and send me the syslog output. Thanks. -- Regards/Gruß, Boris. - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RESEND number 2] libata: eliminate the home grown dma padding in favour of that provided by the block layer
On Tue, 2008-02-05 at 09:43 +0900, Tejun Heo wrote: Hello, James Bottomley wrote: No, it doesn't. Drain needs to extend the sg table too The patches do extend the sg table. Hmm... Where? It's the block component; it's already in git head with this patch: commit fa0ccd837e3dddb44c7db2f128a8bb7e4eabc21a Author: James Bottomley [EMAIL PROTECTED] Date: Thu Jan 10 11:30:36 2008 -0600 block: implement drain buffers The description is pretty reasonable. The way it works is by making block add the drain buffer to the sg table as the last element. The way it works is if you simply use the length block gives you, you find yourself doing DMA to every element in the sg table except the last one. If you expand the length to anything up to blk_rq_drain_size() you then use the last element for the drain. and it makes controllers lax about buffer overruns for commands they shouldn't be. So adjust the qc-nbytes to show no drain element in libata ... although the extra element gets mapped, the HBA will be none the wiser if there's zero length allocated to it. Thus the entire system behaves exactly like the drain element isn't present. The block layer is really just making the drain element available ... you don't have to use it. Adjusting qc-nbytes isn't enough. libata will have to trim at the end of sglist. With both draining and padding, it means libata will have to trim one sg entry and a few more bytes. Block layer is behaving differently and in a very noticeable way, it's sending inconsistent values in data_len and sg list. No, that's the point. With all the space available, you don't need to trim at all ... you simply don't use it. The rationale is the way the sg element stuff works. If you program a DMA table (PRD table in IDE speak) into a controller, it doesn't care if you don't use the last few pieces. With the default length set, the DMA controller won't use the last programmed element, but it's their if you expand the length or tell it to for an overrun. Gee.. What's the deal with extra four bytes? If you're worried about the size of struct request, chopping off PC request part into a separate struct would be far better strategy than making what's already confusing more confusing. Well, we are trying to slim down the request structure and scsi_cmnd structure. However, there's no inherent advantage to counting this all in the block layer. Think of it just as a transparent passthrough. You've told the block layer to make sure you have room for a drain element when it constructs the SG list, which it does. Whether you use it (by adding blk_rq_drain_size() on to the actual length you hand down) or not is entirely up to you ... there's no need to push the decision on that one into the block layer, since use or not is pretty much a cost free thing. Okay, I see your point. The biggest problem is that by doing that blk layer breaks a pretty important rule - keeping data len consistent with sg list and the bugs it's gonna cause will be very subtle and dangerous. This is definitely where we can spend four extra bytes. The lengths don't have to be consistent (otherwise we wouldn't need to calculate them separately) the request reported length just has to be less than the sg table actual length. We even make use of this property in SCSI when we can't fit a request into a single command ... we are allowed to chunk up the request as we complete it. I'm not saying it's best practice, but it is acceptable behaviour. James - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RESEND number 2] libata: eliminate the home grown dma padding in favour of that provided by the block layer
Hello, James Bottomley wrote: No, it doesn't. Drain needs to extend the sg table too The patches do extend the sg table. Hmm... Where? and it makes controllers lax about buffer overruns for commands they shouldn't be. So adjust the qc-nbytes to show no drain element in libata ... although the extra element gets mapped, the HBA will be none the wiser if there's zero length allocated to it. Thus the entire system behaves exactly like the drain element isn't present. The block layer is really just making the drain element available ... you don't have to use it. Adjusting qc-nbytes isn't enough. libata will have to trim at the end of sglist. With both draining and padding, it means libata will have to trim one sg entry and a few more bytes. Block layer is behaving differently and in a very noticeable way, it's sending inconsistent values in data_len and sg list. Gee.. What's the deal with extra four bytes? If you're worried about the size of struct request, chopping off PC request part into a separate struct would be far better strategy than making what's already confusing more confusing. Well, we are trying to slim down the request structure and scsi_cmnd structure. However, there's no inherent advantage to counting this all in the block layer. Think of it just as a transparent passthrough. You've told the block layer to make sure you have room for a drain element when it constructs the SG list, which it does. Whether you use it (by adding blk_rq_drain_size() on to the actual length you hand down) or not is entirely up to you ... there's no need to push the decision on that one into the block layer, since use or not is pretty much a cost free thing. Okay, I see your point. The biggest problem is that by doing that blk layer breaks a pretty important rule - keeping data len consistent with sg list and the bugs it's gonna cause will be very subtle and dangerous. This is definitely where we can spend four extra bytes. Thanks. -- tejun - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] enclosure: add support for enclosure services
--- On Sun, 2/3/08, James Bottomley [EMAIL PROTECTED] wrote: The enclosure misc device is really just a library providing sysfs support for physical enclosure devices and their components. Who is the target audience/user of those facilities? a) The kernel itself needing to read/write SES pages? b) A user space application using sysfs to read/write SES pages? At the moment SES device management is done via an application (user-space) and a user-space library used by the application and /dev/sgX to send SCSI commands to the SES device. One could have a very good argument to not bloat the kernel with this but leave it to a user-space application and a library to do all this and communicate with the SES device via the kernel's /dev/sgX. Luben - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RESEND number 2] libata: eliminate the home grown dma padding in favour of that provided by the block layer
On Tue, 2008-02-05 at 09:06 +0900, Tejun Heo wrote: James Bottomley wrote: Plus, tape devices are also ATAPI and since the problem seems to be handling of ATAPI pio commands, they need all of this too. It really is, I think, better just to do the setup in the libata slave configure if the device is atapi. If no SCSI HBA needs it, fine. For all the rest, I think code demonstrates better ... well, except the sata_fsl.c update ... you leave in qc-n_iter, but it's only ever set to zero ... doesn't that cause this driver to break? Probably. I was gonna cross check with your patch while splitting. They seem mostly identical so I'll probably use your patch with some modifications. I analysed all the places you use the expanded data length. To keep the true length in the request and add the drain if necessary, I think requires this update over my published libata drain patch. I've also added the request accessor to the drain buffer. Could you verify this actually works? If it does, it's better than all the additions to block and sr. It would probably help to resend the series rebased against git current. No, it doesn't. Drain needs to extend the sg table too The patches do extend the sg table. and it makes controllers lax about buffer overruns for commands they shouldn't be. So adjust the qc-nbytes to show no drain element in libata ... although the extra element gets mapped, the HBA will be none the wiser if there's zero length allocated to it. Thus the entire system behaves exactly like the drain element isn't present. The block layer is really just making the drain element available ... you don't have to use it. Gee.. What's the deal with extra four bytes? If you're worried about the size of struct request, chopping off PC request part into a separate struct would be far better strategy than making what's already confusing more confusing. Well, we are trying to slim down the request structure and scsi_cmnd structure. However, there's no inherent advantage to counting this all in the block layer. Think of it just as a transparent passthrough. You've told the block layer to make sure you have room for a drain element when it constructs the SG list, which it does. Whether you use it (by adding blk_rq_drain_size() on to the actual length you hand down) or not is entirely up to you ... there's no need to push the decision on that one into the block layer, since use or not is pretty much a cost free thing. James - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] enclosure: add support for enclosure services
On Mon, 2008-02-04 at 16:32 -0800, Luben Tuikov wrote: --- On Sun, 2/3/08, James Bottomley [EMAIL PROTECTED] wrote: The enclosure misc device is really just a library providing sysfs support for physical enclosure devices and their components. Who is the target audience/user of those facilities? a) The kernel itself needing to read/write SES pages? That depends on the enclosure integration, but right at the moment, it doesn't b) A user space application using sysfs to read/write SES pages? Not an application so much as a user. The idea of sysfs is to allow users to get and set the information in addition to applications. At the moment SES device management is done via an application (user-space) and a user-space library used by the application and /dev/sgX to send SCSI commands to the SES device. I must have missed that when I was looking for implementations; what's the URL? But, if we have non-scsi enclosures to integrate, that makes it harder for a user application because it has to know all the implementations. A sysfs framework on the other hand is a universal known thing for the user applications. One could have a very good argument to not bloat the kernel with this but leave it to a user-space application and a library to do all this and communicate with the SES device via the kernel's /dev/sgX. The same thing goes for other esoteric SCSI infrastructure pieces like cd changers. On the whole, given that ATA is asking for enclosure management in kernel, it makes sense to consolidate the infrastructure and a ses ULD is a very good test bed. James - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RESEND number 2] libata: eliminate the home grown dma padding in favour of that provided by the block layer
James Bottomley wrote: No, it doesn't. Drain needs to extend the sg table too The patches do extend the sg table. Hmm... Where? It's the block component; it's already in git head with this patch: commit fa0ccd837e3dddb44c7db2f128a8bb7e4eabc21a Author: James Bottomley [EMAIL PROTECTED] Date: Thu Jan 10 11:30:36 2008 -0600 block: implement drain buffers The description is pretty reasonable. The way it works is by making block add the drain buffer to the sg table as the last element. The way it works is if you simply use the length block gives you, you find yourself doing DMA to every element in the sg table except the last one. If you expand the length to anything up to blk_rq_drain_size() you then use the last element for the drain. Oops, I was talking about padding. Sorry about that. Adjusting qc-nbytes isn't enough. libata will have to trim at the end of sglist. With both draining and padding, it means libata will have to trim one sg entry and a few more bytes. Block layer is behaving differently and in a very noticeable way, it's sending inconsistent values in data_len and sg list. No, that's the point. With all the space available, you don't need to trim at all ... you simply don't use it. The rationale is the way the sg element stuff works. If you program a DMA table (PRD table in IDE speak) into a controller, it doesn't care if you don't use the last few pieces. With the default length set, the DMA controller won't use the last programmed element, but it's their if you expand the length or tell it to for an overrun. Not all controllers have a place to set 'default length'. They just run till sg list terminates. Okay, I see your point. The biggest problem is that by doing that blk layer breaks a pretty important rule - keeping data len consistent with sg list and the bugs it's gonna cause will be very subtle and dangerous. This is definitely where we can spend four extra bytes. The lengths don't have to be consistent (otherwise we wouldn't need to calculate them separately) the request reported length just has to be less than the sg table actual length. We even make use of this property in SCSI when we can't fit a request into a single command ... we are allowed to chunk up the request as we complete it. I'm not saying it's best practice, but it is acceptable behaviour. libata definitely isn't ready for that and I don't think expanding the behavior to whole block layer is a wise thing to do when the best practice can be bought at the cost of 4 bytes per request. No? -- tejun - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
ide-tape redux (was: Re:)
Hi Borislav,
On Monday 04 February 2008, Borislav Petkov wrote:
Hi Bart,
here are the pending ide-tape patches reworked which incorporate all review
points raised so far. Several new patches are appended to the original series
which i thought would be reasonable to sumbit along with the others. Also,
i've applied ide-tape: dump gcw fields on error in idetape_identify_device()
which is #12 and which you can simply ignore. Furthermore, #32 from the
original
series got split up into the different logical changes it dealt with, as you
requested.
Thanks! [ Reviewing was so much easier. ]
Documentation/feature-removal-schedule.txt | 14 +-
drivers/ide/ide-tape.c | 2764
+---
2 files changed, 1325 insertions(+), 1453 deletions(-)
applied #1-7, #9-10, #13-22 (+queued all of them for 2.6.25)
w.r.t. #8 I'm waiting for Jens to comment on blk_{get,put}_request() approach
w.r.t. #11 ide-tape uses char devices and supports DSC so it is not as obvious
as in ide-floppy case that all atomic bitops can be just removed (extra audit
and some time -mm are required) so please resync/resubmit
#12 is already in Linus' tree
Bart
-
To unsubscribe from this list: send the line unsubscribe linux-ide in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] ppc: fix #ifdef-s in mediabay driver (take 2)
* Replace incorrect CONFIG_BLK_DEV_IDE #ifdef in
check_media_bay() by CONFIG_MAC_FLOPPY one.
* Replace incorrect CONFIG_BLK_DEV_IDE #ifdef-s by
CONFIG_BLK_DEV_IDE_PMAC ones.
* check_media_bay() is used only by drivers/block/swim3.c
so make this function available only if CONFIG_MAC_FLOPPY
is defined.
* check_media_bay_by_base() and media_bay_set_ide_infos()
are used only by drivers/ide/ppc/pmac.c so so make these
functions available only if CONFIG_MAC_FLOPPY is defined.
v2:
* Remove ifdefs from function prototypes. (Andrew Morton)
Cc: Benjamin Herrenschmidt [EMAIL PROTECTED]
Cc: Andrew Morton [EMAIL PROTECTED]
Signed-off-by: Bartlomiej Zolnierkiewicz [EMAIL PROTECTED]
---
drivers/macintosh/mediabay.c | 46 ++---
include/asm-powerpc/mediabay.h |8 +++
2 files changed, 25 insertions(+), 29 deletions(-)
Index: b/drivers/macintosh/mediabay.c
===
--- a/drivers/macintosh/mediabay.c
+++ b/drivers/macintosh/mediabay.c
@@ -78,12 +78,14 @@ struct media_bay_info {
int cached_gpio;
int sleeping;
struct semaphorelock;
-#ifdef CONFIG_BLK_DEV_IDE
+#ifdef CONFIG_BLK_DEV_IDE_PMAC
void __iomem*cd_base;
- int cd_index;
int cd_irq;
int cd_retry;
#endif
+#if defined(CONFIG_BLK_DEV_IDE_PMAC) || defined(CONFIG_MAC_FLOPPY)
+ int cd_index;
+#endif
};
#define MAX_BAYS 2
@@ -91,7 +93,7 @@ struct media_bay_info {
static struct media_bay_info media_bays[MAX_BAYS];
int media_bay_count = 0;
-#ifdef CONFIG_BLK_DEV_IDE
+#ifdef CONFIG_BLK_DEV_IDE_PMAC
/* check the busy bit in the media-bay ide interface
(assumes the media-bay contains an ide device) */
#define MB_IDE_READY(i)((readb(media_bays[i].cd_base + 0x70) 0x80)
== 0)
@@ -401,7 +403,7 @@ static void poll_media_bay(struct media_
set_mb_power(bay, id != MB_NO);
bay-content_id = id;
if (id == MB_NO) {
-#ifdef CONFIG_BLK_DEV_IDE
+#ifdef CONFIG_BLK_DEV_IDE_PMAC
bay-cd_retry = 0;
#endif
printk(KERN_INFO media bay %d is
empty\n, bay-index);
@@ -414,9 +416,9 @@ static void poll_media_bay(struct media_
}
}
+#ifdef CONFIG_MAC_FLOPPY
int check_media_bay(struct device_node *which_bay, int what)
{
-#ifdef CONFIG_BLK_DEV_IDE
int i;
for (i=0; imedia_bay_count; i++)
@@ -426,14 +428,14 @@ int check_media_bay(struct device_node *
media_bays[i].cd_index = -1;
return -EINVAL;
}
-#endif /* CONFIG_BLK_DEV_IDE */
return -ENODEV;
}
EXPORT_SYMBOL(check_media_bay);
+#endif /* CONFIG_MAC_FLOPPY */
+#ifdef CONFIG_BLK_DEV_IDE_PMAC
int check_media_bay_by_base(unsigned long base, int what)
{
-#ifdef CONFIG_BLK_DEV_IDE
int i;
for (i=0; imedia_bay_count; i++)
@@ -443,15 +445,13 @@ int check_media_bay_by_base(unsigned lon
media_bays[i].cd_index = -1;
return -EINVAL;
}
-#endif
-
+
return -ENODEV;
}
int media_bay_set_ide_infos(struct device_node* which_bay, unsigned long base,
- int irq, int index)
+ int irq, int index)
{
-#ifdef CONFIG_BLK_DEV_IDE
int i;
for (i=0; imedia_bay_count; i++) {
@@ -483,10 +483,10 @@ int media_bay_set_ide_infos(struct devic
return -ENODEV;
}
}
-#endif /* CONFIG_BLK_DEV_IDE */
-
+
return -ENODEV;
}
+#endif /* CONFIG_BLK_DEV_IDE_PMAC */
static void media_bay_step(int i)
{
@@ -521,14 +521,13 @@ static void media_bay_step(int i)
bay-state = mb_resetting;
MBDBG(mediabay%d: waiting reset (kind:%d)\n, i,
bay-content_id);
break;
-
case mb_resetting:
if (bay-content_id != MB_CD) {
MBDBG(mediabay%d: bay is up (kind:%d)\n, i,
bay-content_id);
bay-state = mb_up;
break;
}
-#ifdef CONFIG_BLK_DEV_IDE
+#ifdef CONFIG_BLK_DEV_IDE_PMAC
MBDBG(mediabay%d: waiting IDE reset (kind:%d)\n, i,
bay-content_id);
bay-ops-un_reset_ide(bay);
bay-timer = msecs_to_jiffies(MB_IDE_WAIT);
@@ -536,16 +535,14 @@ static void media_bay_step(int i)
#else
printk(KERN_DEBUG media-bay %d is ide (not compiled in
kernel)\n, i);
set_mb_power(bay, 0);
-#endif /* CONFIG_BLK_DEV_IDE */
+#endif /* CONFIG_BLK_DEV_IDE_PMAC */
break;
-
Re: [PATCH 02/22] ide-tape: remove struct idetape_read_position_result_t
On Monday 04 February 2008, Borislav Petkov wrote:
There should be no functional changes resulting from this patch.
Signed-off-by: Borislav Petkov [EMAIL PROTECTED]
---
drivers/ide/ide-tape.c | 49 +--
1 files changed, 18 insertions(+), 31 deletions(-)
diff --git a/drivers/ide/ide-tape.c b/drivers/ide/ide-tape.c
index 442d71c..c8c57ab 100644
--- a/drivers/ide/ide-tape.c
+++ b/drivers/ide/ide-tape.c
[...]
if (!tape-pc-error) {
- result = (idetape_read_position_result_t *) tape-pc-buffer;
- debug_log(DBG_SENSE, BOP - %s\n, result-bop ? Yes : No);
- debug_log(DBG_SENSE, EOP - %s\n, result-eop ? Yes : No);
+ debug_log(DBG_SENSE, BOP - %s\n,
+ !!(readpos[0] 0x80) ? Yes : No);
+ debug_log(DBG_SENSE, EOP - %s\n,
+ !!(readpos[0] 0x40) ? Yes : No);
+
+ if (!!(readpos[0] 0x4)) {
+ printk(KERN_INFO ide-tape: Block location is unknown
+ to the tape\n);
I removed needless !! while merging the patch.
-
To unsubscribe from this list: send the line unsubscribe linux-ide in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] sata_nv: fix ATAPI issues with memory over 4GB (v7)
This fixes some problems with ATAPI devices on nForce4 controllers in ADMA mode
on systems with memory located above 4GB. We need to delay setting the 64-bit
DMA mask until the PRD table and padding buffer are allocated so that they don't
get allocated above 4GB and break legacy mode (which is needed for ATAPI
devices). Also, if either port is in ATAPI mode we need to set the DMA mask
for the PCI device to 32-bit to ensure that the IOMMU code properly bounces
requests above 4GB, as it appears setting the bounce limit does not guarantee
that we will not try to map requests above this point.
Reported to fix https://bugzilla.redhat.com/show_bug.cgi?id=351451
Signed-off-by: Robert Hancock [EMAIL PROTECTED]
--- linux-2.6.24/drivers/ata/sata_nv.c 2008-01-24 16:58:37.0 -0600
+++ linux-2.6.24edit/drivers/ata/sata_nv.c 2008-01-29 18:39:37.0
-0600
@@ -247,6 +247,7 @@ struct nv_adma_port_priv {
void __iomem*ctl_block;
void __iomem*gen_block;
void __iomem*notifier_clear_block;
+ u64 adma_dma_mask;
u8 flags;
int last_issue_ncq;
};
@@ -715,9 +716,10 @@ static int nv_adma_slave_config(struct s
{
struct ata_port *ap = ata_shost_to_port(sdev-host);
struct nv_adma_port_priv *pp = ap-private_data;
+ struct nv_adma_port_priv *port0, *port1;
+ struct scsi_device *sdev0, *sdev1;
struct pci_dev *pdev = to_pci_dev(ap-host-dev);
- u64 bounce_limit;
- unsigned long segment_boundary;
+ unsigned long segment_boundary, flags;
unsigned short sg_tablesize;
int rc;
int adma_enable;
@@ -729,6 +731,8 @@ static int nv_adma_slave_config(struct s
/* Not a proper libata device, ignore */
return rc;
+ spin_lock_irqsave(ap-lock, flags);
+
if (ap-link.device[sdev-id].class == ATA_DEV_ATAPI) {
/*
* NVIDIA reports that ADMA mode does not support ATAPI
commands.
@@ -737,7 +741,6 @@ static int nv_adma_slave_config(struct s
* Restrict DMA parameters as required by the legacy interface
* when an ATAPI device is connected.
*/
- bounce_limit = ATA_DMA_MASK;
segment_boundary = ATA_DMA_BOUNDARY;
/* Subtract 1 since an extra entry may be needed for padding,
see
libata-scsi.c */
@@ -748,7 +751,6 @@ static int nv_adma_slave_config(struct s
adma_enable = 0;
nv_adma_register_mode(ap);
} else {
- bounce_limit = *ap-dev-dma_mask;
segment_boundary = NV_ADMA_DMA_BOUNDARY;
sg_tablesize = NV_ADMA_SGTBL_TOTAL_LEN;
adma_enable = 1;
@@ -774,12 +776,49 @@ static int nv_adma_slave_config(struct s
if (current_reg != new_reg)
pci_write_config_dword(pdev, NV_MCP_SATA_CFG_20, new_reg);
- blk_queue_bounce_limit(sdev-request_queue, bounce_limit);
+ port0 = ap-host-ports[0]-private_data;
+ port1 = ap-host-ports[1]-private_data;
+ sdev0 = ap-host-ports[0]-link.device[0].sdev;
+ sdev1 = ap-host-ports[1]-link.device[0].sdev;
+ if ((port0-flags NV_ADMA_ATAPI_SETUP_COMPLETE) ||
+ (port1-flags NV_ADMA_ATAPI_SETUP_COMPLETE)) {
+ /** We have to set the DMA mask to 32-bit if either port is in
+ ATAPI mode, since they are on the same PCI device which is
+ used for DMA mapping. If we set the mask we also need to set
+ the bounce limit on both ports to ensure that the block
+ layer doesn't feed addresses that cause DMA mapping to
+ choke. If either SCSI device is not allocated yet, it's OK
+ since that port will discover its correct setting when it
+ does get allocated.
+ Note: Setting 32-bit mask should not fail. */
+ if (sdev0)
+ blk_queue_bounce_limit(sdev0-request_queue,
+ ATA_DMA_MASK);
+ if (sdev1)
+ blk_queue_bounce_limit(sdev1-request_queue,
+ ATA_DMA_MASK);
+
+ pci_set_dma_mask(pdev, ATA_DMA_MASK);
+ } else {
+ /** This shouldn't fail as it was set to this value before */
+ pci_set_dma_mask(pdev, pp-adma_dma_mask);
+ if (sdev0)
+ blk_queue_bounce_limit(sdev0-request_queue,
+ pp-adma_dma_mask);
+ if (sdev1)
+ blk_queue_bounce_limit(sdev1-request_queue,
+ pp-adma_dma_mask);
+ }
+
blk_queue_segment_boundary(sdev-request_queue,
Re: [PATCH] enclosure: add support for enclosure services
--- On Mon, 2/4/08, James Bottomley [EMAIL PROTECTED] wrote: The enclosure misc device is really just a library providing sysfs support for physical enclosure devices and their components. Who is the target audience/user of those facilities? a) The kernel itself needing to read/write SES pages? That depends on the enclosure integration, but right at the moment, it doesn't Yes, I didn't suspect so. b) A user space application using sysfs to read/write SES pages? Not an application so much as a user. The idea of sysfs is to allow users to get and set the information in addition to applications. Exactly the same argument stands for a user-space application with a user-space library. This is the classical case of where it is better to do this in user-space as opposed to the kernel. The kernel provides capability to access the SES device. The user space application and library provide interpretation and control. Thus if the enclosure were upgraded, one doesn't need to upgrade their kernel in order to utilize the new capabilities of the SES device. Plus upgrading a user-space application is a lot easier than the kernel (and no reboot necessary). Consider another thing: vendors would really like unprecedented access to the SES device in the enclosure so as your ses/enclosure code keeps state it would get out of sync when vendor user-space enclosure applications access (and modify) the SES device's pages. You can test this yourself: submit a patch that removes SES /dev/sgX support; advertise your ses/class solution and watch the fun. At the moment SES device management is done via an application (user-space) and a user-space library used by the application and /dev/sgX to send SCSI commands to the SES device. I must have missed that when I was looking for implementations; what's the URL? I'm not aware of any GPLed ones. That doesn't necessarily mean that the best course of action is to bloat the kernel. You can move your ses/enclosure stuff to a user space application library and thus start a GPLed one. But, if we have non-scsi enclosures to integrate, that makes it harder for a user application because it has to know all the implementations. So does the kernel. And as I pointed out above, it is a lot easier to upgrade a user-space application and library than it is to upgrade a new kernel and having to reboot the computer to run the new kernel. A sysfs framework on the other hand is a universal known thing for the user applications. So would a user-space ses library, a la libses.so. One could have a very good argument to not bloat the kernel with this but leave it to a user-space application and a library to do all this and communicate with the SES device via the kernel's /dev/sgX. The same thing goes for other esoteric SCSI infrastructure pieces like cd changers. On the whole, given that ATA is asking for enclosure management in kernel, it makes sense to consolidate the infrastructure and a ses ULD is a very good test bed. What is wrong with exporting the SES device as /dev/sgX and having a user-space application and library to do all this? Luben - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] enclosure: add support for enclosure services
On Mon, 2008-02-04 at 18:01 -0800, Luben Tuikov wrote: --- On Mon, 2/4/08, James Bottomley [EMAIL PROTECTED] wrote: The enclosure misc device is really just a library providing sysfs support for physical enclosure devices and their components. Who is the target audience/user of those facilities? a) The kernel itself needing to read/write SES pages? That depends on the enclosure integration, but right at the moment, it doesn't Yes, I didn't suspect so. b) A user space application using sysfs to read/write SES pages? Not an application so much as a user. The idea of sysfs is to allow users to get and set the information in addition to applications. Exactly the same argument stands for a user-space application with a user-space library. This is the classical case of where it is better to do this in user-space as opposed to the kernel. The kernel provides capability to access the SES device. The user space application and library provide interpretation and control. Thus if the enclosure were upgraded, one doesn't need to upgrade their kernel in order to utilize the new capabilities of the SES device. Plus upgrading a user-space application is a lot easier than the kernel (and no reboot necessary). The implementation is modular, so it's remove and insert ... Consider another thing: vendors would really like unprecedented access to the SES device in the enclosure so as your ses/enclosure code keeps state it would get out of sync when vendor user-space enclosure applications access (and modify) the SES device's pages. The state model doesn't assume nothing else will alter the state. You can test this yourself: submit a patch that removes SES /dev/sgX support; advertise your ses/class solution and watch the fun. At the moment SES device management is done via an application (user-space) and a user-space library used by the application and /dev/sgX to send SCSI commands to the SES device. I must have missed that when I was looking for implementations; what's the URL? I'm not aware of any GPLed ones. That doesn't necessarily mean that the best course of action is to bloat the kernel. You can move your ses/enclosure stuff to a user space application library and thus start a GPLed one. Certainly ... patches welcome. But, if we have non-scsi enclosures to integrate, that makes it harder for a user application because it has to know all the implementations. So does the kernel. And as I pointed out above, it is a lot easier to upgrade a user-space application and library than it is to upgrade a new kernel and having to reboot the computer to run the new kernel. No, think again ... it's easy for SES based enclosures because they have a SCSI transport. We have no transport for SGPIO based enclosures nor for any of the other more esoteric ones. That's not to say it can't be done, but it does mean that it can't be completely userspace. A sysfs framework on the other hand is a universal known thing for the user applications. So would a user-space ses library, a la libses.so. One could have a very good argument to not bloat the kernel with this but leave it to a user-space application and a library to do all this and communicate with the SES device via the kernel's /dev/sgX. The same thing goes for other esoteric SCSI infrastructure pieces like cd changers. On the whole, given that ATA is asking for enclosure management in kernel, it makes sense to consolidate the infrastructure and a ses ULD is a very good test bed. What is wrong with exporting the SES device as /dev/sgX and having a user-space application and library to do all this? How do you transport the enclosure commands over /dev/sgX? Only SES has SCSI command encapsulation ... the rest won't even be SCSI targets ... James - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] enclosure: add support for enclosure services
--- On Mon, 2/4/08, James Bottomley [EMAIL PROTECTED] wrote: On Mon, 2008-02-04 at 18:01 -0800, Luben Tuikov wrote: --- On Mon, 2/4/08, James Bottomley [EMAIL PROTECTED] wrote: The enclosure misc device is really just a library providing sysfs support for physical enclosure devices and their components. Who is the target audience/user of those facilities? a) The kernel itself needing to read/write SES pages? That depends on the enclosure integration, but right at the moment, it doesn't Yes, I didn't suspect so. b) A user space application using sysfs to read/write SES pages? Not an application so much as a user. The idea of sysfs is to allow users to get and set the information in addition to applications. Exactly the same argument stands for a user-space application with a user-space library. This is the classical case of where it is better to do this in user-space as opposed to the kernel. The kernel provides capability to access the SES device. The user space application and library provide interpretation and control. Thus if the enclosure were upgraded, one doesn't need to upgrade their kernel in order to utilize the new capabilities of the SES device. Plus upgrading a user-space application is a lot easier than the kernel (and no reboot necessary). The implementation is modular, so it's remove and insert ... I guess the same could be said for STGT and SCST, right? LOL, no seriously, this is unnecessary kernel bloat, or rather at the wrong place (see below). Consider another thing: vendors would really like unprecedented access to the SES device in the enclosure so as your ses/enclosure code keeps state it would get out of sync when vendor user-space enclosure applications access (and modify) the SES device's pages. The state model doesn't assume nothing else will alter the state. But it would be trivial exercise to show that an inconsistent state can be had by modifying pages of the SES device directly from userspace bypassing your implementation. You can test this yourself: submit a patch that removes SES /dev/sgX support; advertise your ses/class solution and watch the fun. At the moment SES device management is done via an application (user-space) and a user-space library used by the application and /dev/sgX to send SCSI commands to the SES device. I must have missed that when I was looking for implementations; what's the URL? I'm not aware of any GPLed ones. That doesn't necessarily mean that the best course of action is to bloat the kernel. You can move your ses/enclosure stuff to a user space application library and thus start a GPLed one. Certainly ... patches welcome. I've non at the moment, plus I don't think you'd be the point of contact for a user-space SES library. Unless of course you've already started something up on sourceforge. Really, such an effort already exists: it is called sg_ses(8). But, if we have non-scsi enclosures to integrate, that makes it harder for a user application because it has to know all the implementations. So does the kernel. And as I pointed out above, it is a lot easier to upgrade a user-space application and library than it is to upgrade a new kernel and having to reboot the computer to run the new kernel. No, think again ... it's easy for SES based enclosures because they have a SCSI transport. We have no transport for SGPIO based enclosures nor for any of the other more esoteric ones. Yes, for which the transport layer, implements the scsi device node for the SES device. It doesn't really matter if the SCSI commands sent to the SES device go over SGPIO or FC or SAS or Bluetooth or I2C, etc, the transport layer can implement that and present the /dev/sgX node. Case in point: the protocol FW running on the ASIC provides this capability so really the LLDD would only see a the pure SCSI SES or processor device and register that with the kernel. At which point no new kernel bloat is required. Your code doesn't quite do that at the moment as it actually goes further in to read and present SES pages. Ideally it would simply provide capability for transport layers to register a SCSI device of type SES, or processor. Architecturally, the LLDD/transport layer would register the SGPIO device on one end with the SGPIO layer and on the other end as a SCSI SES/processpr device. After that sg_ses(8) or sglib, fits the bill for user space applications. That's not to say it can't be done, but it does mean that it can't be completely userspace. See previous paragraph. A sysfs framework on the other hand is a universal known thing for the user applications. So would a user-space ses library, a la libses.so. One could have a very good argument to not bloat the kernel with
Re: [PATCH] enclosure: add support for enclosure services
On Mon, 2008-02-04 at 19:28 -0800, Luben Tuikov wrote: --- On Mon, 2/4/08, James Bottomley [EMAIL PROTECTED] wrote: On Mon, 2008-02-04 at 18:01 -0800, Luben Tuikov wrote: --- On Mon, 2/4/08, James Bottomley [EMAIL PROTECTED] wrote: The enclosure misc device is really just a library providing sysfs support for physical enclosure devices and their components. Who is the target audience/user of those facilities? a) The kernel itself needing to read/write SES pages? That depends on the enclosure integration, but right at the moment, it doesn't Yes, I didn't suspect so. b) A user space application using sysfs to read/write SES pages? Not an application so much as a user. The idea of sysfs is to allow users to get and set the information in addition to applications. Exactly the same argument stands for a user-space application with a user-space library. This is the classical case of where it is better to do this in user-space as opposed to the kernel. The kernel provides capability to access the SES device. The user space application and library provide interpretation and control. Thus if the enclosure were upgraded, one doesn't need to upgrade their kernel in order to utilize the new capabilities of the SES device. Plus upgrading a user-space application is a lot easier than the kernel (and no reboot necessary). The implementation is modular, so it's remove and insert ... I guess the same could be said for STGT and SCST, right? You mean both of their kernel pieces are modular? That's correct. LOL, no seriously, this is unnecessary kernel bloat, or rather at the wrong place (see below). Consider another thing: vendors would really like unprecedented access to the SES device in the enclosure so as your ses/enclosure code keeps state it would get out of sync when vendor user-space enclosure applications access (and modify) the SES device's pages. The state model doesn't assume nothing else will alter the state. But it would be trivial exercise to show that an inconsistent state can be had by modifying pages of the SES device directly from userspace bypassing your implementation. I don't think so ... if you actually look at the code, you'll see it doesn't really have persistent state for the enclosure. You can test this yourself: submit a patch that removes SES /dev/sgX support; advertise your ses/class solution and watch the fun. At the moment SES device management is done via an application (user-space) and a user-space library used by the application and /dev/sgX to send SCSI commands to the SES device. I must have missed that when I was looking for implementations; what's the URL? I'm not aware of any GPLed ones. That doesn't necessarily mean that the best course of action is to bloat the kernel. You can move your ses/enclosure stuff to a user space application library and thus start a GPLed one. Certainly ... patches welcome. I've non at the moment, plus I don't think you'd be the point of contact for a user-space SES library. Unless of course you've already started something up on sourceforge. Really, such an effort already exists: it is called sg_ses(8). But, if we have non-scsi enclosures to integrate, that makes it harder for a user application because it has to know all the implementations. So does the kernel. And as I pointed out above, it is a lot easier to upgrade a user-space application and library than it is to upgrade a new kernel and having to reboot the computer to run the new kernel. No, think again ... it's easy for SES based enclosures because they have a SCSI transport. We have no transport for SGPIO based enclosures nor for any of the other more esoteric ones. Yes, for which the transport layer, implements the scsi device node for the SES device. It doesn't really matter if the SCSI commands sent to the SES device go over SGPIO or FC or SAS or Bluetooth or I2C, etc, the transport layer can implement that and present the /dev/sgX node. But it does matter if the enclosure device doesn't speak SCSI. SGPIO isn't a SCSI protocol ... it's a general purpose serial bus protocol. It's pretty simple and register based, but it might (or might not) be accessible via a SCSI bridge. Case in point: the protocol FW running on the ASIC provides this capability so really the LLDD would only see a the pure SCSI SES or processor device and register that with the kernel. At which point no new kernel bloat is required. Your code doesn't quite do that at the moment as it actually goes further in to read and present SES pages. Ideally it would simply provide capability for transport layers to register a
Re: [BUG] 2.6.24 refuses to boot - ATA problem?
On Monday 04 February 2008, Mark Lord wrote: Gene Heskett wrote: On Sunday 03 February 2008, Ingo Molnar wrote: * Gene Heskett [EMAIL PROTECTED] wrote: I believe its the same, but lemme paste it for sure, yes: [ 26.339926] ENABLING IO-APIC IRQs [ 26.340119] ..TIMER: vector=0x31 apic1=0 pin1=0 apic2=-1 pin2=-1 [ 26.350129] ..MP-BIOS bug: 8254 timer not connected to IO-APIC [ 26.350182] ...trying to set up timer (IRQ0) through the 8259A ... failed. [ 26.350185] ...trying to set up timer as Virtual Wire IRQ... failed. [ 26.360186] ...trying to set up timer as ExtINT IRQ... works. The third line is the only line that makes it to the screen during the boot trace. Now, what does this tell us? the question would be: - if you remove the acpi_use_timer_override boot flag - and if you boot a kernel with this hack applied = do those weird PATA failures come back? If the failues do _not_ come back then the problem is somehow affected/worked-around by the IO-APIC code that generates the above 4 lines. If the failures are still the same then the above 4 lines are really just an uninteresting side-effect of the acpi_use_timer_override flag - and the real side-effects (that fixes PATA on your box) are to be found elsewhere. Sadly, the latter variant is the expected answer. Ingo And at this point, I can't tell. This reboot was from a cold start, without the argument, and cold by long enough to make the rounds about the house and pick up a beer, but not take my evening pillbox. A minute cold, maybe 2 max. The log is clean since except for a kudzu nag of some sort: .. Just to muddy your observations: it is quite possible that a cold (power-off) reboot may be required to properly observe what happens here. Precisely why I've now done that twice, without using the extra argument. No recurrence dammit. Cheers -- Cheers, Gene There are four boxes to be used in defense of liberty: soap, ballot, jury, and ammo. Please use in that order. -Ed Howdershelt (Author) He who makes a beast of himself gets rid of the pain of being a man. -- Dr. Johnson - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 07/22] ide-tape: struct idetape_tape_t: shorten member names v2
On Tue, Feb 05, 2008 at 02:23:21AM +0100, Bartlomiej Zolnierkiewicz wrote: On Monday 04 February 2008, Borislav Petkov wrote: Shorten some member names not too aggressively since this driver might be gone anyway soon. Signed-off-by: Borislav Petkov [EMAIL PROTECTED] --- drivers/ide/ide-tape.c | 210 ++-- 1 files changed, 113 insertions(+), 97 deletions(-) diff --git a/drivers/ide/ide-tape.c b/drivers/ide/ide-tape.c index 126e8a9..0b5ccce 100644 --- a/drivers/ide/ide-tape.c +++ b/drivers/ide/ide-tape.c [...] @@ -1583,7 +1579,8 @@ static void idetape_create_read_cmd(idetape_tape_t *tape, idetape_pc_t *pc, unsi pc-bh = bh; atomic_set(bh-b_count, 0); pc-buffer = NULL; - pc-request_transfer = pc-buffer_size = length * tape-tape_block_size; + pc-buffer_size = length * tape-blk_size; + pc-request_transfer= length * tape-blk_size; if (pc-request_transfer == tape-stage_size) set_bit(PC_DMA_RECOMMENDED, pc-flags); } @@ -1621,7 +1618,8 @@ static void idetape_create_write_cmd(idetape_tape_t *tape, idetape_pc_t *pc, uns pc-b_data = bh-b_data; pc-b_count = atomic_read(bh-b_count); pc-buffer = NULL; - pc-request_transfer = pc-buffer_size = length * tape-tape_block_size; + pc-request_transfer= length * tape-blk_size; + pc-buffer_size = length * tape-blk_size; if (pc-request_transfer == tape-stage_size) set_bit(PC_DMA_RECOMMENDED, pc-flags); } for some reason gcc doesn't seem to optimize the new code as well as the old one (= driver size goes up instead of staying unchanged) interdiff between original patch and merged version: diff -u b/drivers/ide/ide-tape.c b/drivers/ide/ide-tape.c --- b/drivers/ide/ide-tape.c +++ b/drivers/ide/ide-tape.c @@ -324,7 +324,7 @@ /* Current character device data transfer direction */ u8 chrdev_dir; - /* tape block size, usu. 512 or 1024 bytes */ + /* tape block size, usually 512 or 1024 bytes */ unsigned short blk_size; int user_bs_factor; @@ -1580,8 +1580,8 @@ pc-bh = bh; atomic_set(bh-b_count, 0); pc-buffer = NULL; - pc-buffer_size = length * tape-blk_size; - pc-request_transfer= length * tape-blk_size; + pc-buffer_size = length * tape-blk_size; + pc-request_transfer = pc-buffer_size; if (pc-request_transfer == tape-stage_size) set_bit(PC_DMA_RECOMMENDED, pc-flags); } @@ -1619,8 +1619,8 @@ pc-b_data = bh-b_data; pc-b_count = atomic_read(bh-b_count); pc-buffer = NULL; - pc-request_transfer= length * tape-blk_size; - pc-buffer_size = length * tape-blk_size; + pc-buffer_size = length * tape-blk_size; + pc-request_transfer = pc-buffer_size; if (pc-request_transfer == tape-stage_size) set_bit(PC_DMA_RECOMMENDED, pc-flags); } Yeah, i did that only because checkpatch.pl complained that multiple assignments should be avoided. Now it looks kinda dumb that way besides improving readability so converting it to the best form w.r.t generating smaller binary would be a reason good enough to ignore checkpatch.pl in that case. -- Regards/Gruß, Boris. - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RESEND number 2] libata: eliminate the home grown dma padding in favour of that provided by the block layer
On Tue, 2008-02-05 at 10:07 +0900, Tejun Heo wrote: James Bottomley wrote: No, it doesn't. Drain needs to extend the sg table too The patches do extend the sg table. Hmm... Where? It's the block component; it's already in git head with this patch: commit fa0ccd837e3dddb44c7db2f128a8bb7e4eabc21a Author: James Bottomley [EMAIL PROTECTED] Date: Thu Jan 10 11:30:36 2008 -0600 block: implement drain buffers The description is pretty reasonable. The way it works is by making block add the drain buffer to the sg table as the last element. The way it works is if you simply use the length block gives you, you find yourself doing DMA to every element in the sg table except the last one. If you expand the length to anything up to blk_rq_drain_size() you then use the last element for the drain. Oops, I was talking about padding. Sorry about that. Oh, OK ... the I thought the changelog was pretty explicit. However, it works because the dma_aligment parameter of the block layer ensures that all elements of the sg list begin and end on an address that conforms to the dma_alignment. It does this basically by forcing a copy of user incoming commands if they don't conform. It does nothing with kernel based commands, but you can always assume for direct DMA that they begin and end on the cache line boundary ... and if they don't, there's always a spare allocation to expand them. Adjusting qc-nbytes isn't enough. libata will have to trim at the end of sglist. With both draining and padding, it means libata will have to trim one sg entry and a few more bytes. Block layer is behaving differently and in a very noticeable way, it's sending inconsistent values in data_len and sg list. No, that's the point. With all the space available, you don't need to trim at all ... you simply don't use it. The rationale is the way the sg element stuff works. If you program a DMA table (PRD table in IDE speak) into a controller, it doesn't care if you don't use the last few pieces. With the default length set, the DMA controller won't use the last programmed element, but it's their if you expand the length or tell it to for an overrun. Not all controllers have a place to set 'default length'. They just run till sg list terminates. True ... but the traversal of the sg list terminates when the data transfer finishes ... this is the basis of your drain patch (and mine). Basically we don't require that anything transfer into the drain, but it's there just in case the transfer overruns and it needs to be used. Okay, I see your point. The biggest problem is that by doing that blk layer breaks a pretty important rule - keeping data len consistent with sg list and the bugs it's gonna cause will be very subtle and dangerous. This is definitely where we can spend four extra bytes. The lengths don't have to be consistent (otherwise we wouldn't need to calculate them separately) the request reported length just has to be less than the sg table actual length. We even make use of this property in SCSI when we can't fit a request into a single command ... we are allowed to chunk up the request as we complete it. I'm not saying it's best practice, but it is acceptable behaviour. libata definitely isn't ready for that and I don't think expanding the behavior to whole block layer is a wise thing to do when the best practice can be bought at the cost of 4 bytes per request. No? but the whole basis of both patches is inconsistent lengths: we're expecting a transfer of X, but we program the element lists for Y (X) and see if the transfer actually needs X (but hopefully Y). James - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
libata: kill now unused n_iter and fix sata_fsl
From: James Bottomley [EMAIL PROTECTED]
qc-n_iter was used for libata's own sg walking before sg chaining
replaced it. During conversion, the field and its usage in sata_fsl
were left behind. Kill the filed and update sata_fsl.
tj: This was part of James's libata-use-block-layer-padding patch.
Separated out by me.
Signed-off-by: James Bottomley [EMAIL PROTECTED]
Signed-off-by: Tejun Heo [EMAIL PROTECTED]
Cc: Li Yang [EMAIL PROTECTED]
---
drivers/ata/sata_fsl.c |4 ++--
include/linux/libata.h |2 --
2 files changed, 2 insertions(+), 4 deletions(-)
Index: work/drivers/ata/sata_fsl.c
===
--- work.orig/drivers/ata/sata_fsl.c
+++ work/drivers/ata/sata_fsl.c
@@ -355,8 +355,8 @@ static unsigned int sata_fsl_fill_sg(str
ata_port_printk(qc-ap, KERN_ERR,
s/g len unaligned : 0x%x\n, sg_len);
- if ((num_prde == (SATA_FSL_MAX_PRD_DIRECT - 1))
- (qc-n_iter + 1 != qc-n_elem)) {
+ if (num_prde == (SATA_FSL_MAX_PRD_DIRECT - 1)
+ sg_next(sg) != NULL) {
VPRINTK(setting indirect prde\n);
prd_ptr_to_indirect_ext = prd;
prd-dba = cpu_to_le32(indirect_ext_segment_paddr);
Index: work/include/linux/libata.h
===
--- work.orig/include/linux/libata.h
+++ work/include/linux/libata.h
@@ -457,7 +457,6 @@ struct ata_queued_cmd {
unsigned long flags; /* ATA_QCFLAG_xxx */
unsigned inttag;
unsigned intn_elem;
- unsigned intn_iter;
unsigned intmapped_n_elem;
int dma_dir;
@@ -1367,7 +1366,6 @@ static inline void ata_qc_reinit(struct
qc-nbytes = qc-raw_nbytes = qc-curbytes = 0;
qc-n_elem = 0;
qc-mapped_n_elem = 0;
- qc-n_iter = 0;
qc-err_mask = 0;
qc-pad_len = 0;
qc-last_sg = NULL;
-
To unsubscribe from this list: send the line unsubscribe linux-ide in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RESEND number 2] libata: eliminate the home grown dma padding in favour of that provided by the block layer
James Bottomley wrote:
Oops, I was talking about padding. Sorry about that.
Oh, OK ... the I thought the changelog was pretty explicit. However, it
works because the dma_aligment parameter of the block layer ensures that
all elements of the sg list begin and end on an address that conforms to
the dma_alignment. It does this basically by forcing a copy of user
incoming commands if they don't conform. It does nothing with kernel
based commands, but you can always assume for direct DMA that they begin
and end on the cache line boundary ... and if they don't, there's always
a spare allocation to expand them.
Yeah, the area is expanded but the bio list isn't updated accordingly.
In turn the sg list won't be. I found where you worked around this.
The following chunk is added to ata_sg_setup().
+ /* needs padding? */
+ if (pad) {
+ struct scatterlist *sg = sg_last(qc-sg, n_elem);
+ qc-nbytes += ATA_DMA_PAD_SZ - pad;
+ sg-length += ATA_DMA_PAD_SZ - pad;
+ }
So, you add the area way up in the stack and adjust sg way down the
stack. This is too fragile. Actually, it's already broken and
demonstrates very well why block layer shouldn't do half of the job and
defer the rest to lower layer.
With draining, the above code ends up extending the sg entry for drain
page by pad bytes and there's no extra byte allocated after the drain
page. The controller will end up contaminating pad bytes of the next
page and it will be extremely difficult to reproduce and track down.
Not all controllers have a place to set 'default length'. They just run
till sg list terminates.
True ... but the traversal of the sg list terminates when the data
transfer finishes ... this is the basis of your drain patch (and mine).
Basically we don't require that anything transfer into the drain, but
it's there just in case the transfer overruns and it needs to be used.
The thing is there is no way to detect data overrun if sg list doesn't
terminate. For data transfer commands, sg list must terminate precisely
at the request boundary. If 4k drain page is automatically appended,
the controller will happily overflow into that 4k without notifying
anyone. Or worse, the device can suck in few extra blocks without
triggering error. sgl should never contain extra entries for any
command which can affect data integrity.
libata definitely isn't ready for that and I don't think expanding the
behavior to whole block layer is a wise thing to do when the best
practice can be bought at the cost of 4 bytes per request. No?
but the whole basis of both patches is inconsistent lengths: we're
expecting a transfer of X, but we program the element lists for Y (X)
and see if the transfer actually needs X (but hopefully Y).
Yes, it's dual-sizing. There's the size upper layer requested and
there's slightly different size lower layer wants to see (most of the
time). Block layer sits inbetween and matches the impedance by
extending the request necessary but from either side the picture is
still consistent. Lower layer (mostly) just sees -data_len and
matching sgl. Upper layer sends and receives the data length it requested.
The difference is that with your approach what lower layer sees becomes
inconsistent in itself and half of the impedance matching is left to
lower layer and it will cause nasty problems down the road.
Thanks.
--
tejun
-
To unsubscribe from this list: send the line unsubscribe linux-ide in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] enclosure: add support for enclosure services
--- On Mon, 2/4/08, James Bottomley [EMAIL PROTECTED] wrote: --- On Mon, 2/4/08, James Bottomley [EMAIL PROTECTED] wrote: On Mon, 2008-02-04 at 18:01 -0800, Luben Tuikov wrote: --- On Mon, 2/4/08, James Bottomley [EMAIL PROTECTED] wrote: The enclosure misc device is really just a library providing sysfs support for physical enclosure devices and their components. Who is the target audience/user of those facilities? a) The kernel itself needing to read/write SES pages? That depends on the enclosure integration, but right at the moment, it doesn't Yes, I didn't suspect so. b) A user space application using sysfs to read/write SES pages? Not an application so much as a user. The idea of sysfs is to allow users to get and set the information in addition to applications. Exactly the same argument stands for a user-space application with a user-space library. This is the classical case of where it is better to do this in user-space as opposed to the kernel. The kernel provides capability to access the SES device. The user space application and library provide interpretation and control. Thus if the enclosure were upgraded, one doesn't need to upgrade their kernel in order to utilize the new capabilities of the SES device. Plus upgrading a user-space application is a lot easier than the kernel (and no reboot necessary). The implementation is modular, so it's remove and insert ... I guess the same could be said for STGT and SCST, right? You mean both of their kernel pieces are modular? That's correct. No, you know very well what I mean. By the same logic you're preaching to include your solution part of the kernel, you can also apply to SCST. LOL, no seriously, this is unnecessary kernel bloat, or rather at the wrong place (see below). Consider another thing: vendors would really like unprecedented access to the SES device in the enclosure so as your ses/enclosure code keeps state it would get out of sync when vendor user-space enclosure applications access (and modify) the SES device's pages. The state model doesn't assume nothing else will alter the state. But it would be trivial exercise to show that an inconsistent state can be had by modifying pages of the SES device directly from userspace bypassing your implementation. I don't think so ... if you actually look at the code, you'll see it doesn't really have persistent state for the enclosure. You can test this yourself: submit a patch that removes SES /dev/sgX support; advertise your ses/class solution and watch the fun. At the moment SES device management is done via an application (user-space) and a user-space library used by the application and /dev/sgX to send SCSI commands to the SES device. I must have missed that when I was looking for implementations; what's the URL? I'm not aware of any GPLed ones. That doesn't necessarily mean that the best course of action is to bloat the kernel. You can move your ses/enclosure stuff to a user space application library and thus start a GPLed one. Certainly ... patches welcome. I've non at the moment, plus I don't think you'd be the point of contact for a user-space SES library. Unless of course you've already started something up on sourceforge. Really, such an effort already exists: it is called sg_ses(8). But, if we have non-scsi enclosures to integrate, that makes it harder for a user application because it has to know all the implementations. So does the kernel. And as I pointed out above, it is a lot easier to upgrade a user-space application and library than it is to upgrade a new kernel and having to reboot the computer to run the new kernel. No, think again ... it's easy for SES based enclosures because they have a SCSI transport. We have no transport for SGPIO based enclosures nor for any of the other more esoteric ones. Yes, for which the transport layer, implements the scsi device node for the SES device. It doesn't really matter if the SCSI commands sent to the SES device go over SGPIO or FC or SAS or Bluetooth or I2C, etc, the transport layer can implement that and present the /dev/sgX node. But it does matter if the enclosure device doesn't speak SCSI. Enclosure management isn't as simple as you're portraying it here. The enclosure management device speaks either SES or SAF-TE. The transport protocol to access it could be SGPIO or I2C or... SGPIO isn't a SCSI protocol ... it's a general purpose serial bus
[PATCH 3/5] block: implement request_queue-dma_drain_needed
Draining shouldn't be done for commands where overflow may indicate
data integrity issues. Add dma_drain_needed callback to
request_queue. Drain buffer is appened iff this function returns
non-zero.
Signed-off-by: Tejun Heo [EMAIL PROTECTED]
Cc: James Bottomley [EMAIL PROTECTED]
---
block/blk-merge.c |2 +-
block/blk-settings.c |7 +--
include/linux/blkdev.h |7 +--
3 files changed, 11 insertions(+), 5 deletions(-)
diff --git a/block/blk-merge.c b/block/blk-merge.c
index 480d2bc..d50cfc8 100644
--- a/block/blk-merge.c
+++ b/block/blk-merge.c
@@ -220,7 +220,7 @@ new_segment:
bvprv = bvec;
} /* segments in rq */
- if (q-dma_drain_size) {
+ if (q-dma_drain_size q-dma_drain_needed(rq)) {
sg-page_link = ~0x02;
sg = sg_next(sg);
sg_set_page(sg, virt_to_page(q-dma_drain_buffer),
diff --git a/block/blk-settings.c b/block/blk-settings.c
index c8d0c57..0a0b3a4 100644
--- a/block/blk-settings.c
+++ b/block/blk-settings.c
@@ -296,6 +296,7 @@ EXPORT_SYMBOL(blk_queue_stack_limits);
* blk_queue_dma_drain - Set up a drain buffer for excess dma.
*
* @q: the request queue for the device
+ * @dma_drain_needed: fn which returns non-zero if drain is necessary
* @buf: physically contiguous buffer
* @size: size of the buffer in bytes
*
@@ -315,14 +316,16 @@ EXPORT_SYMBOL(blk_queue_stack_limits);
* device can support otherwise there won't be room for the drain
* buffer.
*/
-int blk_queue_dma_drain(struct request_queue *q, void *buf,
- unsigned int size)
+extern int blk_queue_dma_drain(struct request_queue *q,
+ dma_drain_needed_fn *dma_drain_needed,
+ void *buf, unsigned int size)
{
if (q-max_hw_segments 2 || q-max_phys_segments 2)
return -EINVAL;
/* make room for appending the drain */
--q-max_hw_segments;
--q-max_phys_segments;
+ q-dma_drain_needed = dma_drain_needed;
q-dma_drain_buffer = buf;
q-dma_drain_size = size;
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index ee0b021..3912c5d 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -257,6 +257,7 @@ struct bio_vec;
typedef int (merge_bvec_fn) (struct request_queue *, struct bio *, struct
bio_vec *);
typedef void (prepare_flush_fn) (struct request_queue *, struct request *);
typedef void (softirq_done_fn)(struct request *);
+typedef int (dma_drain_needed_fn)(struct request *);
enum blk_queue_state {
Queue_down,
@@ -293,6 +294,7 @@ struct request_queue
merge_bvec_fn *merge_bvec_fn;
prepare_flush_fn*prepare_flush_fn;
softirq_done_fn *softirq_done_fn;
+ dma_drain_needed_fn *dma_drain_needed;
/*
* Dispatch queue sorting
@@ -697,8 +699,9 @@ extern void blk_queue_max_hw_segments(struct request_queue
*, unsigned short);
extern void blk_queue_max_segment_size(struct request_queue *, unsigned int);
extern void blk_queue_hardsect_size(struct request_queue *, unsigned short);
extern void blk_queue_stack_limits(struct request_queue *t, struct
request_queue *b);
-extern int blk_queue_dma_drain(struct request_queue *q, void *buf,
- unsigned int size);
+extern int blk_queue_dma_drain(struct request_queue *q,
+ dma_drain_needed_fn *dma_drain_needed,
+ void *buf, unsigned int size);
extern void blk_queue_segment_boundary(struct request_queue *, unsigned long);
extern void blk_queue_prep_rq(struct request_queue *, prep_rq_fn *pfn);
extern void blk_queue_merge_bvec(struct request_queue *, merge_bvec_fn *);
--
1.5.2.4
-
To unsubscribe from this list: send the line unsubscribe linux-ide in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] ide: another possible ide panic fix for blk-end-request
On Mon, Feb 04, 2008 at 02:53:12PM -0500, Kiyoshi Ueda wrote:
Hi Jens, Bart, Boris,
I have reviewed all blk-end-request patches again to confirm whether
there are any similar problems with the last week's ide-cd panic:
http://lkml.org/lkml/2008/1/29/140
And I found a possible similar bug in ide-io change:
ide_end_drive_cmd() could be called for blk_pc_request() which could
have bios.
You mean ide_abort() and ide_error(), right? Because ide{-tape,-floppy,-scsi}
do call already ide_end_request() for non-special rq's
(!blk_special_request()), except ide-scsi filters also on !blk_sense_request().
To complete such requests correctly, we need to pass
the actual size of the request.
Otherwise, __blk_end_request() returns 1 because the request still has
bios, and the system will BUG() unnecessarily.
The following patch fixes the bug and should be applied on top of
Linus' git.
Please review and apply.
Cc: Bartlomiej Zolnierkiewicz [EMAIL PROTECTED]
Cc: Borislav Petkov [EMAIL PROTECTED]
Signed-off-by: Kiyoshi Ueda [EMAIL PROTECTED]
Signed-off-by: Jun'ichi Nomura [EMAIL PROTECTED]
---
drivers/ide/ide-io.c |3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
--- a/drivers/ide/ide-io.c2008-02-01 18:20:02.0 -0500
+++ b/drivers/ide/ide-io.c2008-02-01 18:21:50.0 -0500
@@ -388,7 +388,8 @@ void ide_end_drive_cmd (ide_drive_t *dri
spin_lock_irqsave(ide_lock, flags);
HWGROUP(drive)-rq = NULL;
rq-errors = err;
- if (__blk_end_request(rq, (rq-errors ? -EIO : 0), 0))
+ if (unlikely(__blk_end_request(rq, (rq-errors ? -EIO : 0),
+blk_rq_bytes(rq
BUG();
spin_unlock_irqrestore(ide_lock, flags);
}
Thanks,
Kiyoshi Ueda
--
Regards/Gruß,
Boris.
-
To unsubscribe from this list: send the line unsubscribe linux-ide in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch 1/1] ata: drivers/ata/sata_mv.c needs dmapool.h
From: Andrew Morton [EMAIL PROTECTED] mips: drivers/ata/sata_mv.c: In function `mv_port_free_dma_mem': drivers/ata/sata_mv.c:1080: error: implicit declaration of function `dma_pool_free' Cc: Jeff Garzik [EMAIL PROTECTED] Signed-off-by: Andrew Morton [EMAIL PROTECTED] --- drivers/ata/sata_mv.c |1 + 1 file changed, 1 insertion(+) diff -puN drivers/ata/sata_mv.c~ata-drivers-ata-sata_mvc-needs-dmapoolh drivers/ata/sata_mv.c --- a/drivers/ata/sata_mv.c~ata-drivers-ata-sata_mvc-needs-dmapoolh +++ a/drivers/ata/sata_mv.c @@ -69,6 +69,7 @@ #include linux/blkdev.h #include linux/delay.h #include linux/interrupt.h +#include linux/dmapool.h #include linux/dma-mapping.h #include linux/device.h #include scsi/scsi_host.h _ - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: AHCI driver preferring nr_ports over port map
Yes, we can be more smart if necessary. I don't know. The hardware is clearly violating the spec which requires those two values to agree. So are you saying the ESB2 spec is violating a higher level spec? I know almost nothing about AHCI, so please forgive that question... What status values are you seeing? Hardware vendors usually don't get n_ports wrong from the start, they probably have forgotten to decrement it by one when one of the ports is plugged for some reason. I bet the silicon for the port is there and reporting offline PHY, right? This is output from our SLE10SP2 kernel, the output is similar for others: 6scsi2 : ahci 6ata3: SATA link down (SStatus 0 SControl 300) 6scsi3 : ahci 6ata4: SATA link down (SStatus 0 SControl 300) 6scsi4 : ahci 6ata5: SATA link down (SStatus 4 SControl 300) 6scsi5 : ahci 6ata6: SATA link down (SStatus 0 SControl 0) Even the message relating to ata5 seems a little dubious to me, as it's not in sync with what the other unused ports say (and also not in sync with what I see on other boxes - SStatus is always 0 for such ports). Jan - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/5] block: update bio according to DMA alignment padding
DMA start address and transfer size alignment for PC requests are
achieved using bio_copy_user() instead of bio_map_user(). This works
because bio_copy_user() always uses full pages and block DMA alignment
isn't allowed to go over PAGE_SIZE.
However, the implementation didn't update the last bio of the request
to make this padding visible to lower layers. This patch makes
blk_rq_map_user() extend the last bio such that it includes the
padding area and the size of area pointed to by the request is
properly aligned.
Signed-off-by: Tejun Heo [EMAIL PROTECTED]
Cc: James Bottomley [EMAIL PROTECTED]
---
block/blk-map.c | 17 +
1 files changed, 17 insertions(+), 0 deletions(-)
diff --git a/block/blk-map.c b/block/blk-map.c
index 955d75c..103b1df 100644
--- a/block/blk-map.c
+++ b/block/blk-map.c
@@ -139,6 +139,23 @@ int blk_rq_map_user(struct request_queue *q, struct
request *rq,
ubuf += ret;
}
+ /*
+* __blk_rq_map_user() copies the buffers if starting address
+* or length isn't aligned. As the copied buffer is always
+* page aligned, we know that there's enough room for padding.
+* Extend the last bio and update rq-data_len accordingly.
+*
+* On unmap, bio_uncopy_user() will use unmodified
+* bio_map_data pointed to by bio-bi_private.
+*/
+ if (len queue_dma_alignment(q)) {
+ unsigned int pad_len = (queue_dma_alignment(q) ~len) + 1;
+ struct bio *bio = rq-biotail;
+
+ bio-bi_io_vec[bio-bi_vcnt - 1].bv_len += pad_len;
+ bio-bi_size += pad_len;
+ }
+
rq-buffer = rq-data = NULL;
return 0;
unmap_rq:
--
1.5.2.4
-
To unsubscribe from this list: send the line unsubscribe linux-ide in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCHSET #upstream] block/libata: update and use block layer padding and draining
This patchset updates block layer padding and draining support and make libata use it. It's based on James Bottomley's initial work and, of the five, the last two patches are from James with some modifications. Please read the following thread for more info. http://thread.gmane.org/gmane.linux.scsi/37185 This patchset is on top of upstream (a6af42fc9a12165136d82206ad52f18c5955ce87) + kill-n_iter-and-fix-fsl patch [1] block/blk-core.c |2 block/blk-map.c | 19 + block/blk-merge.c |3 block/blk-settings.c |7 +- block/bsg.c |8 +- block/scsi_ioctl.c|3 drivers/ata/ahci.c|5 - drivers/ata/libata-core.c | 145 ++ drivers/ata/libata-scsi.c | 54 ++- drivers/ata/pata_icside.c |8 -- drivers/ata/sata_fsl.c| 13 --- drivers/ata/sata_mv.c |6 - drivers/ata/sata_sil24.c |5 - drivers/scsi/ipr.c|4 - drivers/scsi/libsas/sas_ata.c |4 - drivers/scsi/scsi_lib.c |8 +- include/linux/blkdev.h|8 +- include/linux/libata.h| 28 18 files changed, 95 insertions(+), 235 deletions(-) Thanks. -- tejun [1] http://article.gmane.org/gmane.linux.ide/28038 - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/5] block: add request-raw_data_len
With padding and draining moved into it, block layer now may extend
requests as directed by queue parameters, so now a request has two
sizes - the original request size and the extended size which matches
the size of area pointed to by bios and later by sgs. The latter size
is what lower layers are primarily interested in when allocating,
filling up DMA tables and setting up the controller.
Both padding and draining extend the data area to accomodate
controller characteristics. As any controller which speaks SCSI can
handle underflows, feeding larger data area is safe.
So, this patch makes the primary data length field, request-data_len,
indicate the size of full data area and add a separate length field,
request-raw_data_len, for the unmodified request size. The latter is
used to report to higher layer (userland) and where the original
request size should be fed to the controller or device.
Signed-off-by: Tejun Heo [EMAIL PROTECTED]
Cc: James Bottomley [EMAIL PROTECTED]
---
block/blk-core.c|2 ++
block/blk-map.c |2 ++
block/blk-merge.c |1 +
block/bsg.c |8
block/scsi_ioctl.c |3 ++-
drivers/scsi/scsi_lib.c |8
include/linux/blkdev.h |1 +
7 files changed, 16 insertions(+), 9 deletions(-)
diff --git a/block/blk-core.c b/block/blk-core.c
index 4afb39c..8b004e1 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -116,6 +116,7 @@ void rq_init(struct request_queue *q, struct request *rq)
rq-ref_count = 1;
rq-q = q;
rq-special = NULL;
+ rq-raw_data_len = 0;
rq-data_len = 0;
rq-data = NULL;
rq-nr_phys_segments = 0;
@@ -1982,6 +1983,7 @@ void blk_rq_bio_prep(struct request_queue *q, struct
request *rq,
rq-hard_cur_sectors = rq-current_nr_sectors;
rq-hard_nr_sectors = rq-nr_sectors = bio_sectors(bio);
rq-buffer = bio_data(bio);
+ rq-raw_data_len = bio-bi_size;
rq-data_len = bio-bi_size;
rq-bio = rq-biotail = bio;
diff --git a/block/blk-map.c b/block/blk-map.c
index 103b1df..1588ea3 100644
--- a/block/blk-map.c
+++ b/block/blk-map.c
@@ -19,6 +19,7 @@ int blk_rq_append_bio(struct request_queue *q, struct request
*rq,
rq-biotail-bi_next = bio;
rq-biotail = bio;
+ rq-raw_data_len += bio-bi_size;
rq-data_len += bio-bi_size;
}
return 0;
@@ -154,6 +155,7 @@ int blk_rq_map_user(struct request_queue *q, struct request
*rq,
bio-bi_io_vec[bio-bi_vcnt - 1].bv_len += pad_len;
bio-bi_size += pad_len;
+ rq-data_len += pad_len;
}
rq-buffer = rq-data = NULL;
diff --git a/block/blk-merge.c b/block/blk-merge.c
index 845ef81..480d2bc 100644
--- a/block/blk-merge.c
+++ b/block/blk-merge.c
@@ -228,6 +228,7 @@ new_segment:
((unsigned long)q-dma_drain_buffer)
(PAGE_SIZE - 1));
nsegs++;
+ rq-data_len += q-dma_drain_size;
}
if (sg)
diff --git a/block/bsg.c b/block/bsg.c
index 8917c51..7f3c095 100644
--- a/block/bsg.c
+++ b/block/bsg.c
@@ -437,14 +437,14 @@ static int blk_complete_sgv4_hdr_rq(struct request *rq,
struct sg_io_v4 *hdr,
}
if (rq-next_rq) {
- hdr-dout_resid = rq-data_len;
- hdr-din_resid = rq-next_rq-data_len;
+ hdr-dout_resid = rq-raw_data_len;
+ hdr-din_resid = rq-next_rq-raw_data_len;
blk_rq_unmap_user(bidi_bio);
blk_put_request(rq-next_rq);
} else if (rq_data_dir(rq) == READ)
- hdr-din_resid = rq-data_len;
+ hdr-din_resid = rq-raw_data_len;
else
- hdr-dout_resid = rq-data_len;
+ hdr-dout_resid = rq-raw_data_len;
/*
* If the request generated a negative error number, return it
diff --git a/block/scsi_ioctl.c b/block/scsi_ioctl.c
index 9675b34..e993cac 100644
--- a/block/scsi_ioctl.c
+++ b/block/scsi_ioctl.c
@@ -266,7 +266,7 @@ static int blk_complete_sghdr_rq(struct request *rq, struct
sg_io_hdr *hdr,
hdr-info = 0;
if (hdr-masked_status || hdr-host_status || hdr-driver_status)
hdr-info |= SG_INFO_CHECK;
- hdr-resid = rq-data_len;
+ hdr-resid = rq-raw_data_len;
hdr-sb_len_wr = 0;
if (rq-sense_len hdr-sbp) {
@@ -528,6 +528,7 @@ static int __blk_send_generic(struct request_queue *q,
struct gendisk *bd_disk,
rq = blk_get_request(q, WRITE, __GFP_WAIT);
rq-cmd_type = REQ_TYPE_BLOCK_PC;
rq-data = NULL;
+ rq-raw_data_len = 0;
rq-data_len = 0;
rq-timeout = BLK_DEFAULT_SG_TIMEOUT;
memset(rq-cmd, 0, sizeof(rq-cmd));
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index b12fb31..810f87d 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@
[PATCH 4/5] libata: eliminate the home grown dma padding in favour of that provided by the block layer
From: James Bottomley [EMAIL PROTECTED]
From: James Bottomley [EMAIL PROTECTED]
ATA requires that all DMA transfers begin and end on word boundaries.
Because of this, a large amount of machinery grew up in ide to adjust
scatterlists on this basis. However, as of 2.5, the block layer has a
dma_alignment variable which ensures both the beginning and length of a
DMA transfer are aligned on the dma_alignment boundary. Although the
block layer does adjust the beginning of the transfer to ensure this
happens, it doesn't actually adjust the length, it merely makes sure
that space is allocated for transfers beyond the declared length. The
upshot of this is that scatterlists may be padded to any size between
the actual length and the length adjusted to the dma_alignment safely
knowing that memory is allocated in this region.
Right at the moment, SCSI takes the default dma_aligment which is on a
512 byte boundary. Note that this aligment only applies to transfers
coming in from user space. However, since all kernel allocations are
automatically aligned on a minimum of 32 byte boundaries, it is safe to
adjust them in this manner as well.
tj: * Adjusting sg after padding is done in block layer. Make libata
set queue alignment correctly for ATAPI devices and drop broken
sg mangling from ata_sg_setup().
* Use request-raw_data_len for ATAPI transfer chunk size.
* Killed qc-raw_nbytes.
* Separated out killing qc-n_iter.
Signed-off-by: James Bottomley [EMAIL PROTECTED]
Signed-off-by: Tejun Heo [EMAIL PROTECTED]
---
drivers/ata/ahci.c|5 --
drivers/ata/libata-core.c | 145 +++--
drivers/ata/libata-scsi.c | 23 ++-
drivers/ata/pata_icside.c |8 --
drivers/ata/sata_fsl.c| 13
drivers/ata/sata_mv.c |6 +--
drivers/ata/sata_sil24.c |5 --
drivers/scsi/ipr.c|4 +-
drivers/scsi/libsas/sas_ata.c |4 +-
include/linux/libata.h| 28 +
10 files changed, 21 insertions(+), 220 deletions(-)
diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c
index 27c8d56..e75966b 100644
--- a/drivers/ata/ahci.c
+++ b/drivers/ata/ahci.c
@@ -1979,16 +1979,11 @@ static int ahci_port_start(struct ata_port *ap)
struct ahci_port_priv *pp;
void *mem;
dma_addr_t mem_dma;
- int rc;
pp = devm_kzalloc(dev, sizeof(*pp), GFP_KERNEL);
if (!pp)
return -ENOMEM;
- rc = ata_pad_alloc(ap, dev);
- if (rc)
- return rc;
-
mem = dmam_alloc_coherent(dev, AHCI_PORT_PRIV_DMA_SZ, mem_dma,
GFP_KERNEL);
if (!mem)
diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index bdbd55a..490e8d4 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -4476,30 +4476,13 @@ void ata_sg_clean(struct ata_queued_cmd *qc)
struct ata_port *ap = qc-ap;
struct scatterlist *sg = qc-sg;
int dir = qc-dma_dir;
- void *pad_buf = NULL;
WARN_ON(sg == NULL);
- VPRINTK(unmapping %u sg elements\n, qc-mapped_n_elem);
+ VPRINTK(unmapping %u sg elements\n, qc-n_elem);
- /* if we padded the buffer out to 32-bit bound, and data
-* xfer direction is from-device, we must copy from the
-* pad buffer back into the supplied buffer
-*/
- if (qc-pad_len !(qc-tf.flags ATA_TFLAG_WRITE))
- pad_buf = ap-pad + (qc-tag * ATA_DMA_PAD_SZ);
-
- if (qc-mapped_n_elem)
- dma_unmap_sg(ap-dev, sg, qc-mapped_n_elem, dir);
- /* restore last sg */
- if (qc-last_sg)
- *qc-last_sg = qc-saved_last_sg;
- if (pad_buf) {
- struct scatterlist *psg = qc-extra_sg[1];
- void *addr = kmap_atomic(sg_page(psg), KM_IRQ0);
- memcpy(addr + psg-offset, pad_buf, qc-pad_len);
- kunmap_atomic(addr, KM_IRQ0);
- }
+ if (qc-n_elem)
+ dma_unmap_sg(ap-dev, sg, qc-n_elem, dir);
qc-flags = ~ATA_QCFLAG_DMAMAP;
qc-sg = NULL;
@@ -4765,97 +4748,6 @@ void ata_sg_init(struct ata_queued_cmd *qc, struct
scatterlist *sg,
qc-cursg = qc-sg;
}
-static unsigned int ata_sg_setup_extra(struct ata_queued_cmd *qc,
- unsigned int *n_elem_extra,
- unsigned int *nbytes_extra)
-{
- struct ata_port *ap = qc-ap;
- unsigned int n_elem = qc-n_elem;
- struct scatterlist *lsg, *copy_lsg = NULL, *tsg = NULL, *esg = NULL;
-
- *n_elem_extra = 0;
- *nbytes_extra = 0;
-
- /* needs padding? */
- qc-pad_len = qc-nbytes 3;
-
- if (likely(!qc-pad_len))
- return n_elem;
-
- /* locate last sg and save it */
- lsg = sg_last(qc-sg, n_elem);
- qc-last_sg = lsg;
- qc-saved_last_sg = *lsg;
-
- sg_init_table(qc-extra_sg,
[PATCH 5/5] libata: implement drain buffers
From: James Bottomley [EMAIL PROTECTED]
From: James Bottomley [EMAIL PROTECTED]
This just updates the libata slave configure routine to take advantage
of the block layer drain buffers. It also adjusts the size lengths in
the atapi code to add the drain buffer to the DMA length so the driver
knows it can rely on it.
I suspect I should also be checking for AHCI as well as ATA_DEV_ATAPI,
but I couldn't see how to do that easily.
tj: * atapi_drain_needed() added such that draining is applied to only
misc ATAPI commands.
* q-bounce_gfp used when allocating drain buffer.
* ata_dev_printk() used instead of sdev_printk().
Signed-off-by: James Bottomley [EMAIL PROTECTED]
Signed-off-by: Tejun Heo [EMAIL PROTECTED]
---
drivers/ata/libata-scsi.c | 41 +++--
1 files changed, 35 insertions(+), 6 deletions(-)
diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index e54ee6e..185d699 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -826,17 +826,38 @@ static void ata_scsi_sdev_config(struct scsi_device *sdev)
sdev-max_device_blocked = 1;
}
-static void ata_scsi_dev_config(struct scsi_device *sdev,
- struct ata_device *dev)
+static int atapi_drain_needed(struct request *rq)
+{
+ if (likely(!blk_pc_request(rq)))
+ return 0;
+
+ return atapi_cmd_type(rq-cmd[0]) == ATAPI_MISC;
+}
+
+static int ata_scsi_dev_config(struct scsi_device *sdev,
+ struct ata_device *dev)
{
/* configure max sectors */
blk_queue_max_sectors(sdev-request_queue, dev-max_sectors);
- if (dev-class == ATA_DEV_ATAPI)
+ if (dev-class == ATA_DEV_ATAPI) {
+ struct request_queue *q = sdev-request_queue;
+ void *buf;
+
/* set the min alignment */
blk_queue_update_dma_alignment(sdev-request_queue,
ATA_DMA_PAD_SZ - 1);
- else {
+
+ /* configure draining */
+ buf = kmalloc(ATAPI_MAX_DRAIN, q-bounce_gfp | GFP_KERNEL);
+ if (!buf) {
+ ata_dev_printk(dev, KERN_ERR,
+ drain buffer allocation failed\n);
+ return -ENOMEM;
+ }
+
+ blk_queue_dma_drain(q, atapi_drain_needed, buf,
ATAPI_MAX_DRAIN);
+ } else {
/* ATA devices must be sector aligned */
blk_queue_update_dma_alignment(sdev-request_queue,
ATA_SECT_SIZE - 1);
@@ -853,6 +874,8 @@ static void ata_scsi_dev_config(struct scsi_device *sdev,
depth = min(ATA_MAX_QUEUE - 1, depth);
scsi_adjust_queue_depth(sdev, MSG_SIMPLE_TAG, depth);
}
+
+ return 0;
}
/**
@@ -871,13 +894,14 @@ int ata_scsi_slave_config(struct scsi_device *sdev)
{
struct ata_port *ap = ata_shost_to_port(sdev-host);
struct ata_device *dev = __ata_scsi_find_dev(ap, sdev);
+ int rc = 0;
ata_scsi_sdev_config(sdev);
if (dev)
- ata_scsi_dev_config(sdev, dev);
+ rc = ata_scsi_dev_config(sdev, dev);
- return 0;
+ return rc;
}
/**
@@ -897,6 +921,7 @@ int ata_scsi_slave_config(struct scsi_device *sdev)
void ata_scsi_slave_destroy(struct scsi_device *sdev)
{
struct ata_port *ap = ata_shost_to_port(sdev-host);
+ struct request_queue *q = sdev-request_queue;
unsigned long flags;
struct ata_device *dev;
@@ -912,6 +937,10 @@ void ata_scsi_slave_destroy(struct scsi_device *sdev)
ata_port_schedule_eh(ap);
}
spin_unlock_irqrestore(ap-lock, flags);
+
+ kfree(q-dma_drain_buffer);
+ q-dma_drain_buffer = NULL;
+ q-dma_drain_size = 0;
}
/**
--
1.5.2.4
-
To unsubscribe from this list: send the line unsubscribe linux-ide in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
