* [PATCH 01/10] ide-atapi: allow ->pc_callback() to change rq->data_len
2009-03-25 14:17 [RFC PATCHSET pata-2.6] ide: clean up ide-tape Tejun Heo
@ 2009-03-25 14:17 ` Tejun Heo
2009-03-25 14:17 ` [PATCH 02/10] ide-tape: use single continuous buffer Tejun Heo
` (9 subsequent siblings)
10 siblings, 0 replies; 19+ messages in thread
From: Tejun Heo @ 2009-03-25 14:17 UTC (permalink / raw)
To: bzolnier, linux-kernel, axboe, linux-ide; +Cc: Tejun Heo, Jens Axboe
Impact: allow residual count implementation in ->pc_callback()
rq->data_len has two duties - carrying the number of input bytes on
issue and carrying residual count back to the issuer on completion.
ide-atapi completion callback ->pc_callback() is the right place to do
this but currently ide-atapi depends on rq->data_len carrying the
original request size after calling ->pc_callback() to complete the pc
request.
This patch makes ide_pc_intr() cache length to complete before calling
->pc_callback() so that it can modify rq->data_len as necessary.
Note: As using rq->data_len for two purposes can make cases like this
incorrect in subtle ways, future changes will introduce separate
field for residual count.
Signed-off-by: Tejun Heo <tj@kernel.org>
Cc: Jens Axboe <jens.axboe@oracle.com>
---
drivers/ide/ide-atapi.c | 19 +++++++++++--------
1 files changed, 11 insertions(+), 8 deletions(-)
diff --git a/drivers/ide/ide-atapi.c b/drivers/ide/ide-atapi.c
index bc7dfcf..2b456c4 100644
--- a/drivers/ide/ide-atapi.c
+++ b/drivers/ide/ide-atapi.c
@@ -328,6 +328,7 @@ static ide_startstop_t ide_pc_intr(ide_drive_t *drive)
/* No more interrupts */
if ((stat & ATA_DRQ) == 0) {
+ unsigned int done;
int uptodate;
debug_log("Packet command completed, %d bytes transferred\n",
@@ -367,6 +368,15 @@ static ide_startstop_t ide_pc_intr(ide_drive_t *drive)
if ((pc->flags & PC_FLAG_WAIT_FOR_DSC) && (stat & ATA_DSC) == 0)
dsc = 1;
+ /*
+ * ->pc_callback() might change rq->data_len for
+ * residual count, cache total length.
+ */
+ if (drive->media == ide_tape)
+ done = ide_rq_bytes(rq); /* FIXME */
+ else
+ done = blk_rq_bytes(rq);
+
/* Command finished - Call the callback function */
uptodate = drive->pc_callback(drive, dsc);
@@ -375,20 +385,13 @@ static ide_startstop_t ide_pc_intr(ide_drive_t *drive)
if (blk_special_request(rq)) {
rq->errors = 0;
- ide_complete_rq(drive, 0, blk_rq_bytes(rq));
+ ide_complete_rq(drive, 0, done);
} else {
- unsigned int done;
-
if (blk_fs_request(rq) == 0 && uptodate <= 0) {
if (rq->errors == 0)
rq->errors = -EIO;
}
- if (drive->media == ide_tape)
- done = ide_rq_bytes(rq); /* FIXME */
- else
- done = blk_rq_bytes(rq);
-
ide_complete_rq(drive, uptodate ? 0 : -EIO, done);
}
--
1.6.0.2
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH 02/10] ide-tape: use single continuous buffer
2009-03-25 14:17 [RFC PATCHSET pata-2.6] ide: clean up ide-tape Tejun Heo
2009-03-25 14:17 ` [PATCH 01/10] ide-atapi: allow ->pc_callback() to change rq->data_len Tejun Heo
@ 2009-03-25 14:17 ` Tejun Heo
2009-03-25 16:04 ` Grant Grundler
2009-03-25 14:17 ` [PATCH 03/10] ide-tape-convert-to-bio Tejun Heo
` (8 subsequent siblings)
10 siblings, 1 reply; 19+ messages in thread
From: Tejun Heo @ 2009-03-25 14:17 UTC (permalink / raw)
To: bzolnier, linux-kernel, axboe, linux-ide; +Cc: Tejun Heo
Impact: simpler buffer allocation and handling, fix DMA transfers
ide-tape has its own multiple buffer mechanism using struct
idetape_bh. It allocates buffer with decreasing order-of-two
allocations so that it results in minimum number of segments.
However, the implementation is quite complex and works in a way that
no other block or ide driver works necessitating a lot of special case
handling.
The benefit this complex allocation scheme brings is questionable as
PIO or DMA the number of segments (16 maximum) doesn't make any
noticeable difference and it also doesn't negate the need for multiple
order allocation which can fail under memory pressure or high
fragmentation although it does lower the highest order necessary by
one when the buffer size isn't power of two.
As the first step to remove the custom buffer management, this patch
makes ide-tape allocate single continous buffer. The maximum order is
four. I doubt the change would cause any trouble but if it ever
matters, it should be converted to regular sg mechanism like everyone
else and even in that case dropping custom buffer handling and moving
to standard mechanism first make sense as an intermediate step.
This patch makes the first bh to contain the whole buffer and drops
multi bh handling code. Following patches will make further changes.
This patch has the side effect of fixing DMA transfers. Previously,
commands were passed to DMA engine without DMA-mapping all the
segments.
Signed-off-by: Tejun Heo <tj@kernel.org>
---
drivers/ide/ide-tape.c | 258 +++++++++++-------------------------------------
1 files changed, 59 insertions(+), 199 deletions(-)
diff --git a/drivers/ide/ide-tape.c b/drivers/ide/ide-tape.c
index d52f8b3..3a7bd98 100644
--- a/drivers/ide/ide-tape.c
+++ b/drivers/ide/ide-tape.c
@@ -134,7 +134,6 @@ enum {
struct idetape_bh {
u32 b_size;
atomic_t b_count;
- struct idetape_bh *b_reqnext;
char *b_data;
};
@@ -228,10 +227,6 @@ typedef struct ide_tape_obj {
char *b_data;
int b_count;
- int pages_per_buffer;
- /* Wasted space in each stage */
- int excess_bh_size;
-
/* Measures average tape speed */
unsigned long avg_time;
int avg_size;
@@ -303,9 +298,7 @@ static int idetape_input_buffers(ide_drive_t *drive, struct ide_atapi_pc *pc,
struct idetape_bh *bh = pc->bh;
int count;
- while (bcount) {
- if (bh == NULL)
- break;
+ if (bcount && bh) {
count = min(
(unsigned int)(bh->b_size - atomic_read(&bh->b_count)),
bcount);
@@ -313,15 +306,10 @@ static int idetape_input_buffers(ide_drive_t *drive, struct ide_atapi_pc *pc,
atomic_read(&bh->b_count), count);
bcount -= count;
atomic_add(count, &bh->b_count);
- if (atomic_read(&bh->b_count) == bh->b_size) {
- bh = bh->b_reqnext;
- if (bh)
- atomic_set(&bh->b_count, 0);
- }
+ if (atomic_read(&bh->b_count) == bh->b_size)
+ pc->bh = NULL;
}
- pc->bh = bh;
-
return bcount;
}
@@ -331,22 +319,14 @@ static int idetape_output_buffers(ide_drive_t *drive, struct ide_atapi_pc *pc,
struct idetape_bh *bh = pc->bh;
int count;
- while (bcount) {
- if (bh == NULL)
- break;
+ if (bcount && bh) {
count = min((unsigned int)pc->b_count, (unsigned int)bcount);
drive->hwif->tp_ops->output_data(drive, NULL, pc->b_data, count);
bcount -= count;
pc->b_data += count;
pc->b_count -= count;
- if (!pc->b_count) {
- bh = bh->b_reqnext;
- pc->bh = bh;
- if (bh) {
- pc->b_data = bh->b_data;
- pc->b_count = atomic_read(&bh->b_count);
- }
- }
+ if (!pc->b_count)
+ pc->bh = NULL;
}
return bcount;
@@ -355,24 +335,20 @@ static int idetape_output_buffers(ide_drive_t *drive, struct ide_atapi_pc *pc,
static void idetape_update_buffers(ide_drive_t *drive, struct ide_atapi_pc *pc)
{
struct idetape_bh *bh = pc->bh;
- int count;
unsigned int bcount = pc->xferred;
if (pc->flags & PC_FLAG_WRITING)
return;
- while (bcount) {
- if (bh == NULL) {
+ if (bcount) {
+ if (bh == NULL || bcount > bh->b_size) {
printk(KERN_ERR "ide-tape: bh == NULL in %s\n",
__func__);
return;
}
- count = min((unsigned int)bh->b_size, (unsigned int)bcount);
- atomic_set(&bh->b_count, count);
+ atomic_set(&bh->b_count, bcount);
if (atomic_read(&bh->b_count) == bh->b_size)
- bh = bh->b_reqnext;
- bcount -= count;
+ pc->bh = NULL;
}
- pc->bh = bh;
}
/*
@@ -439,24 +415,10 @@ static void idetape_analyze_error(ide_drive_t *drive, u8 *sense)
/* Free data buffers completely. */
static void ide_tape_kfree_buffer(idetape_tape_t *tape)
{
- struct idetape_bh *prev_bh, *bh = tape->merge_bh;
-
- while (bh) {
- u32 size = bh->b_size;
-
- while (size) {
- unsigned int order = fls(size >> PAGE_SHIFT)-1;
-
- if (bh->b_data)
- free_pages((unsigned long)bh->b_data, order);
+ struct idetape_bh *bh = tape->merge_bh;
- size &= (order-1);
- bh->b_data += (1 << order) * PAGE_SIZE;
- }
- prev_bh = bh;
- bh = bh->b_reqnext;
- kfree(prev_bh);
- }
+ kfree(bh->b_data);
+ kfree(bh);
}
static void ide_tape_handle_dsc(ide_drive_t *);
@@ -859,117 +821,50 @@ out:
}
/*
- * The function below uses __get_free_pages to allocate a data buffer of size
- * tape->buffer_size (or a bit more). We attempt to combine sequential pages as
- * much as possible.
- *
- * It returns a pointer to the newly allocated buffer, or NULL in case of
- * failure.
+ * It returns a pointer to the newly allocated buffer, or NULL in case
+ * of failure.
*/
static struct idetape_bh *ide_tape_kmalloc_buffer(idetape_tape_t *tape,
- int full, int clear)
-{
- struct idetape_bh *prev_bh, *bh, *merge_bh;
- int pages = tape->pages_per_buffer;
- unsigned int order, b_allocd;
- char *b_data = NULL;
-
- merge_bh = kmalloc(sizeof(struct idetape_bh), GFP_KERNEL);
- bh = merge_bh;
- if (bh == NULL)
- goto abort;
-
- order = fls(pages) - 1;
- bh->b_data = (char *) __get_free_pages(GFP_KERNEL, order);
- if (!bh->b_data)
- goto abort;
- b_allocd = (1 << order) * PAGE_SIZE;
- pages &= (order-1);
-
- if (clear)
- memset(bh->b_data, 0, b_allocd);
- bh->b_reqnext = NULL;
- bh->b_size = b_allocd;
- atomic_set(&bh->b_count, full ? bh->b_size : 0);
+ int full)
+{
+ struct idetape_bh *bh;
- while (pages) {
- order = fls(pages) - 1;
- b_data = (char *) __get_free_pages(GFP_KERNEL, order);
- if (!b_data)
- goto abort;
- b_allocd = (1 << order) * PAGE_SIZE;
-
- if (clear)
- memset(b_data, 0, b_allocd);
-
- /* newly allocated page frames below buffer header or ...*/
- if (bh->b_data == b_data + b_allocd) {
- bh->b_size += b_allocd;
- bh->b_data -= b_allocd;
- if (full)
- atomic_add(b_allocd, &bh->b_count);
- continue;
- }
- /* they are above the header */
- if (b_data == bh->b_data + bh->b_size) {
- bh->b_size += b_allocd;
- if (full)
- atomic_add(b_allocd, &bh->b_count);
- continue;
- }
- prev_bh = bh;
- bh = kmalloc(sizeof(struct idetape_bh), GFP_KERNEL);
- if (!bh) {
- free_pages((unsigned long) b_data, order);
- goto abort;
- }
- bh->b_reqnext = NULL;
- bh->b_data = b_data;
- bh->b_size = b_allocd;
- atomic_set(&bh->b_count, full ? bh->b_size : 0);
- prev_bh->b_reqnext = bh;
+ bh = kmalloc(sizeof(struct idetape_bh), GFP_KERNEL);
+ if (!bh)
+ return NULL;
- pages &= (order-1);
+ bh->b_data = kmalloc(tape->buffer_size, GFP_KERNEL);
+ if (!bh->b_data) {
+ kfree(bh);
+ return NULL;
}
- bh->b_size -= tape->excess_bh_size;
- if (full)
- atomic_sub(tape->excess_bh_size, &bh->b_count);
- return merge_bh;
-abort:
- ide_tape_kfree_buffer(tape);
- return NULL;
+ bh->b_size = tape->buffer_size;
+ atomic_set(&bh->b_count, full ? bh->b_size : 0);
+
+ return bh;
}
static int idetape_copy_stage_from_user(idetape_tape_t *tape,
const char __user *buf, int n)
{
struct idetape_bh *bh = tape->bh;
- int count;
int ret = 0;
- while (n) {
- if (bh == NULL) {
+ if (n) {
+ if (bh == NULL || n > bh->b_size - atomic_read(&bh->b_count)) {
printk(KERN_ERR "ide-tape: bh == NULL in %s\n",
__func__);
return 1;
}
- count = min((unsigned int)
- (bh->b_size - atomic_read(&bh->b_count)),
- (unsigned int)n);
if (copy_from_user(bh->b_data + atomic_read(&bh->b_count), buf,
- count))
+ n))
ret = 1;
- n -= count;
- atomic_add(count, &bh->b_count);
- buf += count;
- if (atomic_read(&bh->b_count) == bh->b_size) {
- bh = bh->b_reqnext;
- if (bh)
- atomic_set(&bh->b_count, 0);
- }
+ atomic_add(n, &bh->b_count);
+ if (atomic_read(&bh->b_count) == bh->b_size)
+ tape->bh = NULL;
}
- tape->bh = bh;
+
return ret;
}
@@ -977,30 +872,20 @@ static int idetape_copy_stage_to_user(idetape_tape_t *tape, char __user *buf,
int n)
{
struct idetape_bh *bh = tape->bh;
- int count;
int ret = 0;
- while (n) {
- if (bh == NULL) {
+ if (n) {
+ if (bh == NULL || n > tape->b_count) {
printk(KERN_ERR "ide-tape: bh == NULL in %s\n",
__func__);
return 1;
}
- count = min(tape->b_count, n);
- if (copy_to_user(buf, tape->b_data, count))
+ if (copy_to_user(buf, tape->b_data, n))
ret = 1;
- n -= count;
- tape->b_data += count;
- tape->b_count -= count;
- buf += count;
- if (!tape->b_count) {
- bh = bh->b_reqnext;
- tape->bh = bh;
- if (bh) {
- tape->b_data = bh->b_data;
- tape->b_count = atomic_read(&bh->b_count);
- }
- }
+ tape->b_data += n;
+ tape->b_count -= n;
+ if (!tape->b_count)
+ tape->bh = NULL;
}
return ret;
}
@@ -1252,7 +1137,7 @@ static int idetape_add_chrdev_write_request(ide_drive_t *drive, int blocks)
static void ide_tape_flush_merge_buffer(ide_drive_t *drive)
{
idetape_tape_t *tape = drive->driver_data;
- int blocks, min;
+ int blocks;
struct idetape_bh *bh;
if (tape->chrdev_dir != IDETAPE_DIR_WRITE) {
@@ -1267,31 +1152,16 @@ static void ide_tape_flush_merge_buffer(ide_drive_t *drive)
if (tape->merge_bh_size) {
blocks = tape->merge_bh_size / tape->blk_size;
if (tape->merge_bh_size % tape->blk_size) {
- unsigned int i;
-
+ unsigned int i = tape->blk_size -
+ tape->merge_bh_size % tape->blk_size;
blocks++;
- i = tape->blk_size - tape->merge_bh_size %
- tape->blk_size;
- bh = tape->bh->b_reqnext;
- while (bh) {
- atomic_set(&bh->b_count, 0);
- bh = bh->b_reqnext;
- }
bh = tape->bh;
- while (i) {
- if (bh == NULL) {
- printk(KERN_INFO "ide-tape: bug,"
- " bh NULL\n");
- break;
- }
- min = min(i, (unsigned int)(bh->b_size -
- atomic_read(&bh->b_count)));
+ if (bh) {
memset(bh->b_data + atomic_read(&bh->b_count),
- 0, min);
- atomic_add(min, &bh->b_count);
- i -= min;
- bh = bh->b_reqnext;
- }
+ 0, i);
+ atomic_add(i, &bh->b_count);
+ } else
+ printk(KERN_INFO "ide-tape: bug, bh NULL\n");
}
(void) idetape_add_chrdev_write_request(drive, blocks);
tape->merge_bh_size = 0;
@@ -1319,7 +1189,7 @@ static int idetape_init_read(ide_drive_t *drive)
" 0 now\n");
tape->merge_bh_size = 0;
}
- tape->merge_bh = ide_tape_kmalloc_buffer(tape, 0, 0);
+ tape->merge_bh = ide_tape_kmalloc_buffer(tape, 0);
if (!tape->merge_bh)
return -ENOMEM;
tape->chrdev_dir = IDETAPE_DIR_READ;
@@ -1366,23 +1236,18 @@ static int idetape_add_chrdev_read_request(ide_drive_t *drive, int blocks)
static void idetape_pad_zeros(ide_drive_t *drive, int bcount)
{
idetape_tape_t *tape = drive->driver_data;
- struct idetape_bh *bh;
+ struct idetape_bh *bh = tape->merge_bh;
int blocks;
while (bcount) {
unsigned int count;
- bh = tape->merge_bh;
count = min(tape->buffer_size, bcount);
bcount -= count;
blocks = count / tape->blk_size;
- while (count) {
- atomic_set(&bh->b_count,
- min(count, (unsigned int)bh->b_size));
- memset(bh->b_data, 0, atomic_read(&bh->b_count));
- count -= atomic_read(&bh->b_count);
- bh = bh->b_reqnext;
- }
+ atomic_set(&bh->b_count, count);
+ memset(bh->b_data, 0, atomic_read(&bh->b_count));
+
idetape_queue_rw_tail(drive, REQ_IDETAPE_WRITE, blocks,
tape->merge_bh);
}
@@ -1594,7 +1459,7 @@ static ssize_t idetape_chrdev_write(struct file *file, const char __user *buf,
"should be 0 now\n");
tape->merge_bh_size = 0;
}
- tape->merge_bh = ide_tape_kmalloc_buffer(tape, 0, 0);
+ tape->merge_bh = ide_tape_kmalloc_buffer(tape, 0);
if (!tape->merge_bh)
return -ENOMEM;
tape->chrdev_dir = IDETAPE_DIR_WRITE;
@@ -1968,7 +1833,7 @@ static void idetape_write_release(ide_drive_t *drive, unsigned int minor)
idetape_tape_t *tape = drive->driver_data;
ide_tape_flush_merge_buffer(drive);
- tape->merge_bh = ide_tape_kmalloc_buffer(tape, 1, 0);
+ tape->merge_bh = ide_tape_kmalloc_buffer(tape, 1);
if (tape->merge_bh != NULL) {
idetape_pad_zeros(drive, tape->blk_size *
(tape->user_bs_factor - 1));
@@ -2199,11 +2064,6 @@ static void idetape_setup(ide_drive_t *drive, idetape_tape_t *tape, int minor)
tape->buffer_size = *ctl * tape->blk_size;
}
buffer_size = tape->buffer_size;
- tape->pages_per_buffer = buffer_size / PAGE_SIZE;
- if (buffer_size % PAGE_SIZE) {
- tape->pages_per_buffer++;
- tape->excess_bh_size = PAGE_SIZE - buffer_size % PAGE_SIZE;
- }
/* select the "best" DSC read/write polling freq */
speed = max(*(u16 *)&tape->caps[14], *(u16 *)&tape->caps[8]);
--
1.6.0.2
^ permalink raw reply related [flat|nested] 19+ messages in thread* Re: [PATCH 02/10] ide-tape: use single continuous buffer
2009-03-25 14:17 ` [PATCH 02/10] ide-tape: use single continuous buffer Tejun Heo
@ 2009-03-25 16:04 ` Grant Grundler
2009-03-25 16:13 ` Tejun Heo
0 siblings, 1 reply; 19+ messages in thread
From: Grant Grundler @ 2009-03-25 16:04 UTC (permalink / raw)
To: Tejun Heo; +Cc: bzolnier, linux-kernel, axboe, linux-ide
On Wed, Mar 25, 2009 at 7:17 AM, Tejun Heo <tj@kernel.org> wrote:
> Impact: simpler buffer allocation and handling, fix DMA transfers
...
> + atomic_set(&bh->b_count, bcount);
> if (atomic_read(&bh->b_count) == bh->b_size)
...
I'm failing to see why bh->b_count is an atomic_t.
I always assumed tapes were exclusive access devices
and would be serialized at a higher level.
> /*
> - * The function below uses __get_free_pages to allocate a data buffer of size
> - * tape->buffer_size (or a bit more). We attempt to combine sequential pages as
> - * much as possible.
> - *
> - * It returns a pointer to the newly allocated buffer, or NULL in case of
> - * failure.
> + * It returns a pointer to the newly allocated buffer, or NULL in case
> + * of failure.
> */
> static struct idetape_bh *ide_tape_kmalloc_buffer(idetape_tape_t *tape,
> - int full, int clear)
> -{
...
> -abort:
> - ide_tape_kfree_buffer(tape);
> - return NULL;
> + bh->b_size = tape->buffer_size;
> + atomic_set(&bh->b_count, full ? bh->b_size : 0);
No one else could possibly be referencing bh->count at this
point...I like that it's consistent though.
The use of atomic won't hurt correctness and this patch looks fine to me.
Please add "Reviewed-by: Grant Grundler <grundler@google.com>"
thanks,
grant
...
> @@ -977,30 +872,20 @@ static int idetape_copy_stage_to_user(idetape_tape_t *tape, char __user *buf,
> int n)
> {
> struct idetape_bh *bh = tape->bh;
> - int count;
> int ret = 0;
>
> - while (n) {
> - if (bh == NULL) {
> + if (n) {
> + if (bh == NULL || n > tape->b_count) {
> printk(KERN_ERR "ide-tape: bh == NULL in %s\n",
> __func__);
> return 1;
> }
> - count = min(tape->b_count, n);
> - if (copy_to_user(buf, tape->b_data, count))
> + if (copy_to_user(buf, tape->b_data, n))
> ret = 1;
> - n -= count;
> - tape->b_data += count;
> - tape->b_count -= count;
> - buf += count;
> - if (!tape->b_count) {
> - bh = bh->b_reqnext;
> - tape->bh = bh;
> - if (bh) {
> - tape->b_data = bh->b_data;
> - tape->b_count = atomic_read(&bh->b_count);
> - }
> - }
> + tape->b_data += n;
> + tape->b_count -= n;
> + if (!tape->b_count)
> + tape->bh = NULL;
> }
> return ret;
> }
> @@ -1252,7 +1137,7 @@ static int idetape_add_chrdev_write_request(ide_drive_t *drive, int blocks)
> static void ide_tape_flush_merge_buffer(ide_drive_t *drive)
> {
> idetape_tape_t *tape = drive->driver_data;
> - int blocks, min;
> + int blocks;
> struct idetape_bh *bh;
>
> if (tape->chrdev_dir != IDETAPE_DIR_WRITE) {
> @@ -1267,31 +1152,16 @@ static void ide_tape_flush_merge_buffer(ide_drive_t *drive)
> if (tape->merge_bh_size) {
> blocks = tape->merge_bh_size / tape->blk_size;
> if (tape->merge_bh_size % tape->blk_size) {
> - unsigned int i;
> -
> + unsigned int i = tape->blk_size -
> + tape->merge_bh_size % tape->blk_size;
> blocks++;
> - i = tape->blk_size - tape->merge_bh_size %
> - tape->blk_size;
> - bh = tape->bh->b_reqnext;
> - while (bh) {
> - atomic_set(&bh->b_count, 0);
> - bh = bh->b_reqnext;
> - }
> bh = tape->bh;
> - while (i) {
> - if (bh == NULL) {
> - printk(KERN_INFO "ide-tape: bug,"
> - " bh NULL\n");
> - break;
> - }
> - min = min(i, (unsigned int)(bh->b_size -
> - atomic_read(&bh->b_count)));
> + if (bh) {
> memset(bh->b_data + atomic_read(&bh->b_count),
> - 0, min);
> - atomic_add(min, &bh->b_count);
> - i -= min;
> - bh = bh->b_reqnext;
> - }
> + 0, i);
> + atomic_add(i, &bh->b_count);
> + } else
> + printk(KERN_INFO "ide-tape: bug, bh NULL\n");
> }
> (void) idetape_add_chrdev_write_request(drive, blocks);
> tape->merge_bh_size = 0;
> @@ -1319,7 +1189,7 @@ static int idetape_init_read(ide_drive_t *drive)
> " 0 now\n");
> tape->merge_bh_size = 0;
> }
> - tape->merge_bh = ide_tape_kmalloc_buffer(tape, 0, 0);
> + tape->merge_bh = ide_tape_kmalloc_buffer(tape, 0);
> if (!tape->merge_bh)
> return -ENOMEM;
> tape->chrdev_dir = IDETAPE_DIR_READ;
> @@ -1366,23 +1236,18 @@ static int idetape_add_chrdev_read_request(ide_drive_t *drive, int blocks)
> static void idetape_pad_zeros(ide_drive_t *drive, int bcount)
> {
> idetape_tape_t *tape = drive->driver_data;
> - struct idetape_bh *bh;
> + struct idetape_bh *bh = tape->merge_bh;
> int blocks;
>
> while (bcount) {
> unsigned int count;
>
> - bh = tape->merge_bh;
> count = min(tape->buffer_size, bcount);
> bcount -= count;
> blocks = count / tape->blk_size;
> - while (count) {
> - atomic_set(&bh->b_count,
> - min(count, (unsigned int)bh->b_size));
> - memset(bh->b_data, 0, atomic_read(&bh->b_count));
> - count -= atomic_read(&bh->b_count);
> - bh = bh->b_reqnext;
> - }
> + atomic_set(&bh->b_count, count);
> + memset(bh->b_data, 0, atomic_read(&bh->b_count));
> +
> idetape_queue_rw_tail(drive, REQ_IDETAPE_WRITE, blocks,
> tape->merge_bh);
> }
> @@ -1594,7 +1459,7 @@ static ssize_t idetape_chrdev_write(struct file *file, const char __user *buf,
> "should be 0 now\n");
> tape->merge_bh_size = 0;
> }
> - tape->merge_bh = ide_tape_kmalloc_buffer(tape, 0, 0);
> + tape->merge_bh = ide_tape_kmalloc_buffer(tape, 0);
> if (!tape->merge_bh)
> return -ENOMEM;
> tape->chrdev_dir = IDETAPE_DIR_WRITE;
> @@ -1968,7 +1833,7 @@ static void idetape_write_release(ide_drive_t *drive, unsigned int minor)
> idetape_tape_t *tape = drive->driver_data;
>
> ide_tape_flush_merge_buffer(drive);
> - tape->merge_bh = ide_tape_kmalloc_buffer(tape, 1, 0);
> + tape->merge_bh = ide_tape_kmalloc_buffer(tape, 1);
> if (tape->merge_bh != NULL) {
> idetape_pad_zeros(drive, tape->blk_size *
> (tape->user_bs_factor - 1));
> @@ -2199,11 +2064,6 @@ static void idetape_setup(ide_drive_t *drive, idetape_tape_t *tape, int minor)
> tape->buffer_size = *ctl * tape->blk_size;
> }
> buffer_size = tape->buffer_size;
> - tape->pages_per_buffer = buffer_size / PAGE_SIZE;
> - if (buffer_size % PAGE_SIZE) {
> - tape->pages_per_buffer++;
> - tape->excess_bh_size = PAGE_SIZE - buffer_size % PAGE_SIZE;
> - }
>
> /* select the "best" DSC read/write polling freq */
> speed = max(*(u16 *)&tape->caps[14], *(u16 *)&tape->caps[8]);
> --
> 1.6.0.2
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-ide" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH 02/10] ide-tape: use single continuous buffer
2009-03-25 16:04 ` Grant Grundler
@ 2009-03-25 16:13 ` Tejun Heo
2009-03-25 16:38 ` Borislav Petkov
0 siblings, 1 reply; 19+ messages in thread
From: Tejun Heo @ 2009-03-25 16:13 UTC (permalink / raw)
To: Grant Grundler; +Cc: bzolnier, linux-kernel, axboe, linux-ide
Hello, Grant.
Grant Grundler wrote:
> On Wed, Mar 25, 2009 at 7:17 AM, Tejun Heo <tj@kernel.org> wrote:
>> Impact: simpler buffer allocation and handling, fix DMA transfers
> ...
>> + atomic_set(&bh->b_count, bcount);
>> if (atomic_read(&bh->b_count) == bh->b_size)
> ...
>
> I'm failing to see why bh->b_count is an atomic_t.
> I always assumed tapes were exclusive access devices
> and would be serialized at a higher level.
Beats me. I don't know. The code is generally pretty over-engineered
but, well, it's an ancient piece of code with (probably too) rich
history.
>> - ide_tape_kfree_buffer(tape);
>> - return NULL;
>> + bh->b_size = tape->buffer_size;
>> + atomic_set(&bh->b_count, full ? bh->b_size : 0);
>
> No one else could possibly be referencing bh->count at this
> point...I like that it's consistent though.
Yeah, this patch is just one of logical steps to remove bh, so it
doesn't make any other changes than described. The whole bh stuff
will be removed later in the series.
> The use of atomic won't hurt correctness and this patch looks fine to me.
> Please add "Reviewed-by: Grant Grundler <grundler@google.com>"
Thanks.
--
tejun
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 02/10] ide-tape: use single continuous buffer
2009-03-25 16:13 ` Tejun Heo
@ 2009-03-25 16:38 ` Borislav Petkov
0 siblings, 0 replies; 19+ messages in thread
From: Borislav Petkov @ 2009-03-25 16:38 UTC (permalink / raw)
To: Tejun Heo; +Cc: Grant Grundler, bzolnier, linux-kernel, axboe, linux-ide
Hi,
On Wed, Mar 25, 2009 at 5:13 PM, Tejun Heo <tj@kernel.org> wrote:
> Hello, Grant.
>
> Grant Grundler wrote:
>> On Wed, Mar 25, 2009 at 7:17 AM, Tejun Heo <tj@kernel.org> wrote:
>>> Impact: simpler buffer allocation and handling, fix DMA transfers
>> ...
>>> + atomic_set(&bh->b_count, bcount);
>>> if (atomic_read(&bh->b_count) == bh->b_size)
>> ...
>>
>> I'm failing to see why bh->b_count is an atomic_t.
>> I always assumed tapes were exclusive access devices
>> and would be serialized at a higher level.
>
> Beats me. I don't know. The code is generally pretty over-engineered
> but, well, it's an ancient piece of code with (probably too) rich
> history.
Yep, actually the driver was even bigger and it implemented a bunch of fun stuff
like command pipelining and those atomics had to protect the pipeline from
simultaneous ->open()s through its chrdev interface, IIRC.
--
Regards/Gruss,
Boris
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 03/10] ide-tape-convert-to-bio
2009-03-25 14:17 [RFC PATCHSET pata-2.6] ide: clean up ide-tape Tejun Heo
2009-03-25 14:17 ` [PATCH 01/10] ide-atapi: allow ->pc_callback() to change rq->data_len Tejun Heo
2009-03-25 14:17 ` [PATCH 02/10] ide-tape: use single continuous buffer Tejun Heo
@ 2009-03-25 14:17 ` Tejun Heo
2009-03-25 14:24 ` [PATCH 03/10 REPOST] ide-tape: use bio to carry data area Tejun Heo
2009-03-25 14:17 ` [PATCH 04/10] ide-tape: use standard data transfer mechanism Tejun Heo
` (7 subsequent siblings)
10 siblings, 1 reply; 19+ messages in thread
From: Tejun Heo @ 2009-03-25 14:17 UTC (permalink / raw)
To: bzolnier, linux-kernel, axboe, linux-ide; +Cc: Tejun Heo
---
drivers/ide/ide-atapi.c | 5 +----
drivers/ide/ide-tape.c | 39 ++++++++++++++++++++++-----------------
2 files changed, 23 insertions(+), 21 deletions(-)
diff --git a/drivers/ide/ide-atapi.c b/drivers/ide/ide-atapi.c
index 2b456c4..53e811d 100644
--- a/drivers/ide/ide-atapi.c
+++ b/drivers/ide/ide-atapi.c
@@ -372,10 +372,7 @@ static ide_startstop_t ide_pc_intr(ide_drive_t *drive)
* ->pc_callback() might change rq->data_len for
* residual count, cache total length.
*/
- if (drive->media == ide_tape)
- done = ide_rq_bytes(rq); /* FIXME */
- else
- done = blk_rq_bytes(rq);
+ done = blk_rq_bytes(rq);
/* Command finished - Call the callback function */
uptodate = drive->pc_callback(drive, dsc);
diff --git a/drivers/ide/ide-tape.c b/drivers/ide/ide-tape.c
index 3a7bd98..bd0e839 100644
--- a/drivers/ide/ide-tape.c
+++ b/drivers/ide/ide-tape.c
@@ -458,7 +458,7 @@ static int ide_tape_callback(ide_drive_t *drive, int dsc)
}
tape->first_frame += blocks;
- rq->current_nr_sectors -= blocks;
+ rq->data_len -= blocks * tape->blk_size;
if (pc->error) {
uptodate = 0;
@@ -799,10 +799,6 @@ static ide_startstop_t idetape_do_request(ide_drive_t *drive,
pc = (struct ide_atapi_pc *)rq->special;
rq->cmd[13] &= ~(REQ_IDETAPE_PC1);
rq->cmd[13] |= REQ_IDETAPE_PC2;
- if (pc->req_xfer) {
- ide_init_sg_cmd(&cmd, pc->req_xfer);
- ide_map_sg(drive, &cmd);
- }
goto out;
}
if (rq->cmd[13] & REQ_IDETAPE_PC2) {
@@ -812,6 +808,10 @@ static ide_startstop_t idetape_do_request(ide_drive_t *drive,
BUG();
out:
+ if (pc->req_xfer) {
+ ide_init_sg_cmd(&cmd, pc->req_xfer);
+ ide_map_sg(drive, &cmd);
+ }
if (rq_data_dir(rq))
cmd.tf_flags |= IDE_TFLAG_WRITE;
@@ -1061,32 +1061,37 @@ static int idetape_queue_rw_tail(ide_drive_t *drive, int cmd, int blocks,
struct idetape_bh *bh)
{
idetape_tape_t *tape = drive->driver_data;
+ size_t size = blocks * tape->blk_size;
struct request *rq;
- int ret, errors;
+ int ret;
debug_log(DBG_SENSE, "%s: cmd=%d\n", __func__, cmd);
+ BUG_ON(cmd != REQ_IDETAPE_READ && cmd != REQ_IDETAPE_WRITE);
rq = blk_get_request(drive->queue, READ, __GFP_WAIT);
rq->cmd_type = REQ_TYPE_SPECIAL;
rq->cmd[13] = cmd;
rq->rq_disk = tape->disk;
- rq->special = (void *)bh;
rq->sector = tape->first_frame;
- rq->nr_sectors = blocks;
- rq->current_nr_sectors = blocks;
- blk_execute_rq(drive->queue, tape->disk, rq, 0);
- errors = rq->errors;
- ret = tape->blk_size * (blocks - rq->current_nr_sectors);
- blk_put_request(rq);
+ if (size) {
+ ret = blk_rq_map_kern(drive->queue, rq, bh->b_data, size,
+ __GFP_WAIT);
+ if (ret)
+ goto out_put;
+ }
- if ((cmd & (REQ_IDETAPE_READ | REQ_IDETAPE_WRITE)) == 0)
- return 0;
+ blk_execute_rq(drive->queue, tape->disk, rq, 0);
+
+ size -= rq->data_len;
+ ret = size;
+ if (rq->errors == IDE_DRV_ERROR_GENERAL)
+ ret = -EIO;
if (tape->merge_bh)
idetape_init_merge_buffer(tape);
- if (errors == IDE_DRV_ERROR_GENERAL)
- return -EIO;
+out_put:
+ blk_put_request(rq);
return ret;
}
--
1.6.0.2
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH 03/10 REPOST] ide-tape: use bio to carry data area
2009-03-25 14:17 ` [PATCH 03/10] ide-tape-convert-to-bio Tejun Heo
@ 2009-03-25 14:24 ` Tejun Heo
0 siblings, 0 replies; 19+ messages in thread
From: Tejun Heo @ 2009-03-25 14:24 UTC (permalink / raw)
To: bzolnier, linux-kernel, axboe, linux-ide
Impact: use standard way to carry data area in request
ide-tape uses rq in an interesting way. For r/w requests, rq->special
is used to carry a private buffer management structure idetape_bh and
rq->nr_sectors and current_nr_sectors are initialized to the number of
idetape blocks which isn't necessary 512 bytes. Also,
rq->current_nr_sectors is used to report back the residual count in
units of idetape blocks.
This peculiarity taxes both block layer and ide. ide-atapi has
different paths and hooks to accomodate it and what a rq means becomes
quite confusing and making changes at the block layer becomes quite
difficult and error-prone.
This patch makes ide-tape use bio instead. With the previous patch,
ide-tape currently is using single contiguos buffer so replacing it is
easy. All that's necessary is doing blk_rq_map_kern() on issue, map
bio to sg in idetape_do_request() and use rq->data_len for residual
count instead of rq->current_nr_sectors.
This change also nicely removes the FIXME in ide_pc_intr() where
ide-tape rqs need to be completed using ide_rq_bytes() instead of
blk_rq_bytes() (although this didn't really matter as the request
didn't have bio).
Note that this patch only changes how the data areas are carried in
requests. Data transfer is still done using idetape_bh.
Signed-off-by: Tejun Heo <tj@kernel.org>
Cc: Jens Axboe <jens.axboe@oracle.com>
---
Sorry, wrote description to the wrong file. git tree updated as well.
drivers/ide/ide-atapi.c | 5 +----
drivers/ide/ide-tape.c | 39 ++++++++++++++++++++++-----------------
2 files changed, 23 insertions(+), 21 deletions(-)
diff --git a/drivers/ide/ide-atapi.c b/drivers/ide/ide-atapi.c
index 2b456c4..53e811d 100644
--- a/drivers/ide/ide-atapi.c
+++ b/drivers/ide/ide-atapi.c
@@ -372,10 +372,7 @@ static ide_startstop_t ide_pc_intr(ide_drive_t *drive)
* ->pc_callback() might change rq->data_len for
* residual count, cache total length.
*/
- if (drive->media == ide_tape)
- done = ide_rq_bytes(rq); /* FIXME */
- else
- done = blk_rq_bytes(rq);
+ done = blk_rq_bytes(rq);
/* Command finished - Call the callback function */
uptodate = drive->pc_callback(drive, dsc);
diff --git a/drivers/ide/ide-tape.c b/drivers/ide/ide-tape.c
index 3a7bd98..bd0e839 100644
--- a/drivers/ide/ide-tape.c
+++ b/drivers/ide/ide-tape.c
@@ -458,7 +458,7 @@ static int ide_tape_callback(ide_drive_t *drive, int dsc)
}
tape->first_frame += blocks;
- rq->current_nr_sectors -= blocks;
+ rq->data_len -= blocks * tape->blk_size;
if (pc->error) {
uptodate = 0;
@@ -799,10 +799,6 @@ static ide_startstop_t idetape_do_request(ide_drive_t *drive,
pc = (struct ide_atapi_pc *)rq->special;
rq->cmd[13] &= ~(REQ_IDETAPE_PC1);
rq->cmd[13] |= REQ_IDETAPE_PC2;
- if (pc->req_xfer) {
- ide_init_sg_cmd(&cmd, pc->req_xfer);
- ide_map_sg(drive, &cmd);
- }
goto out;
}
if (rq->cmd[13] & REQ_IDETAPE_PC2) {
@@ -812,6 +808,10 @@ static ide_startstop_t idetape_do_request(ide_drive_t *drive,
BUG();
out:
+ if (pc->req_xfer) {
+ ide_init_sg_cmd(&cmd, pc->req_xfer);
+ ide_map_sg(drive, &cmd);
+ }
if (rq_data_dir(rq))
cmd.tf_flags |= IDE_TFLAG_WRITE;
@@ -1061,32 +1061,37 @@ static int idetape_queue_rw_tail(ide_drive_t *drive, int cmd, int blocks,
struct idetape_bh *bh)
{
idetape_tape_t *tape = drive->driver_data;
+ size_t size = blocks * tape->blk_size;
struct request *rq;
- int ret, errors;
+ int ret;
debug_log(DBG_SENSE, "%s: cmd=%d\n", __func__, cmd);
+ BUG_ON(cmd != REQ_IDETAPE_READ && cmd != REQ_IDETAPE_WRITE);
rq = blk_get_request(drive->queue, READ, __GFP_WAIT);
rq->cmd_type = REQ_TYPE_SPECIAL;
rq->cmd[13] = cmd;
rq->rq_disk = tape->disk;
- rq->special = (void *)bh;
rq->sector = tape->first_frame;
- rq->nr_sectors = blocks;
- rq->current_nr_sectors = blocks;
- blk_execute_rq(drive->queue, tape->disk, rq, 0);
- errors = rq->errors;
- ret = tape->blk_size * (blocks - rq->current_nr_sectors);
- blk_put_request(rq);
+ if (size) {
+ ret = blk_rq_map_kern(drive->queue, rq, bh->b_data, size,
+ __GFP_WAIT);
+ if (ret)
+ goto out_put;
+ }
- if ((cmd & (REQ_IDETAPE_READ | REQ_IDETAPE_WRITE)) == 0)
- return 0;
+ blk_execute_rq(drive->queue, tape->disk, rq, 0);
+
+ size -= rq->data_len;
+ ret = size;
+ if (rq->errors == IDE_DRV_ERROR_GENERAL)
+ ret = -EIO;
if (tape->merge_bh)
idetape_init_merge_buffer(tape);
- if (errors == IDE_DRV_ERROR_GENERAL)
- return -EIO;
+out_put:
+ blk_put_request(rq);
return ret;
}
--
1.6.0.2
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH 04/10] ide-tape: use standard data transfer mechanism
2009-03-25 14:17 [RFC PATCHSET pata-2.6] ide: clean up ide-tape Tejun Heo
` (2 preceding siblings ...)
2009-03-25 14:17 ` [PATCH 03/10] ide-tape-convert-to-bio Tejun Heo
@ 2009-03-25 14:17 ` Tejun Heo
2009-03-25 15:12 ` Borislav Petkov
2009-03-25 14:17 ` [PATCH 05/10] ide-tape: kill idetape_bh Tejun Heo
` (6 subsequent siblings)
10 siblings, 1 reply; 19+ messages in thread
From: Tejun Heo @ 2009-03-25 14:17 UTC (permalink / raw)
To: bzolnier, linux-kernel, axboe, linux-ide; +Cc: Tejun Heo
Impact: use standard way to transfer data
Now that data area is represented with bio, there is no need to use
custom data transfer methods. Drop idetape_io_buffers() and
idetape_update_buffers(). pc->bh is set to null to tell ide-atapi to
use standard data transfer mechanism and idetape_bh byte counts are
updated by the issuer on completion using the residual count.
Signed-off-by: Tejun Heo <tj@kernel.org>
---
drivers/ide/ide-tape.c | 84 +++--------------------------------------------
1 files changed, 6 insertions(+), 78 deletions(-)
diff --git a/drivers/ide/ide-tape.c b/drivers/ide/ide-tape.c
index bd0e839..dd37fd7 100644
--- a/drivers/ide/ide-tape.c
+++ b/drivers/ide/ide-tape.c
@@ -292,65 +292,6 @@ static struct ide_tape_obj *ide_tape_chrdev_get(unsigned int i)
return tape;
}
-static int idetape_input_buffers(ide_drive_t *drive, struct ide_atapi_pc *pc,
- unsigned int bcount)
-{
- struct idetape_bh *bh = pc->bh;
- int count;
-
- if (bcount && bh) {
- count = min(
- (unsigned int)(bh->b_size - atomic_read(&bh->b_count)),
- bcount);
- drive->hwif->tp_ops->input_data(drive, NULL, bh->b_data +
- atomic_read(&bh->b_count), count);
- bcount -= count;
- atomic_add(count, &bh->b_count);
- if (atomic_read(&bh->b_count) == bh->b_size)
- pc->bh = NULL;
- }
-
- return bcount;
-}
-
-static int idetape_output_buffers(ide_drive_t *drive, struct ide_atapi_pc *pc,
- unsigned int bcount)
-{
- struct idetape_bh *bh = pc->bh;
- int count;
-
- if (bcount && bh) {
- count = min((unsigned int)pc->b_count, (unsigned int)bcount);
- drive->hwif->tp_ops->output_data(drive, NULL, pc->b_data, count);
- bcount -= count;
- pc->b_data += count;
- pc->b_count -= count;
- if (!pc->b_count)
- pc->bh = NULL;
- }
-
- return bcount;
-}
-
-static void idetape_update_buffers(ide_drive_t *drive, struct ide_atapi_pc *pc)
-{
- struct idetape_bh *bh = pc->bh;
- unsigned int bcount = pc->xferred;
-
- if (pc->flags & PC_FLAG_WRITING)
- return;
- if (bcount) {
- if (bh == NULL || bcount > bh->b_size) {
- printk(KERN_ERR "ide-tape: bh == NULL in %s\n",
- __func__);
- return;
- }
- atomic_set(&bh->b_count, bcount);
- if (atomic_read(&bh->b_count) == bh->b_size)
- pc->bh = NULL;
- }
-}
-
/*
* called on each failed packet command retry to analyze the request sense. We
* currently do not utilize this information.
@@ -368,12 +309,10 @@ static void idetape_analyze_error(ide_drive_t *drive, u8 *sense)
pc->c[0], tape->sense_key, tape->asc, tape->ascq);
/* Correct pc->xferred by asking the tape. */
- if (pc->flags & PC_FLAG_DMA_ERROR) {
+ if (pc->flags & PC_FLAG_DMA_ERROR)
pc->xferred = pc->req_xfer -
tape->blk_size *
get_unaligned_be32(&sense[3]);
- idetape_update_buffers(drive, pc);
- }
/*
* If error was the result of a zero-length read or write command,
@@ -520,19 +459,6 @@ static void ide_tape_handle_dsc(ide_drive_t *drive)
idetape_postpone_request(drive);
}
-static int ide_tape_io_buffers(ide_drive_t *drive, struct ide_atapi_pc *pc,
- unsigned int bcount, int write)
-{
- unsigned int bleft;
-
- if (write)
- bleft = idetape_output_buffers(drive, pc, bcount);
- else
- bleft = idetape_input_buffers(drive, pc, bcount);
-
- return bcount - bleft;
-}
-
/*
* Packet Command Interface
*
@@ -685,7 +611,7 @@ static void ide_tape_create_rw_cmd(idetape_tape_t *tape,
ide_init_pc(pc);
put_unaligned(cpu_to_be32(length), (unsigned int *) &pc->c[1]);
pc->c[1] = 1;
- pc->bh = bh;
+ pc->bh = NULL;
pc->buf = NULL;
pc->buf_size = length * tape->blk_size;
pc->req_xfer = pc->buf_size;
@@ -1083,7 +1009,11 @@ static int idetape_queue_rw_tail(ide_drive_t *drive, int cmd, int blocks,
blk_execute_rq(drive->queue, tape->disk, rq, 0);
+ /* calculate the number of transferred bytes and update bh */
size -= rq->data_len;
+ if (cmd == REQ_IDETAPE_READ)
+ atomic_add(size, &bh->b_count);
+
ret = size;
if (rq->errors == IDE_DRV_ERROR_GENERAL)
ret = -EIO;
@@ -2037,8 +1967,6 @@ static void idetape_setup(ide_drive_t *drive, idetape_tape_t *tape, int minor)
u16 *ctl = (u16 *)&tape->caps[12];
drive->pc_callback = ide_tape_callback;
- drive->pc_update_buffers = idetape_update_buffers;
- drive->pc_io_buffers = ide_tape_io_buffers;
drive->dev_flags |= IDE_DFLAG_DSC_OVERLAP;
--
1.6.0.2
^ permalink raw reply related [flat|nested] 19+ messages in thread* Re: [PATCH 04/10] ide-tape: use standard data transfer mechanism
2009-03-25 14:17 ` [PATCH 04/10] ide-tape: use standard data transfer mechanism Tejun Heo
@ 2009-03-25 15:12 ` Borislav Petkov
2009-03-25 15:20 ` Borislav Petkov
0 siblings, 1 reply; 19+ messages in thread
From: Borislav Petkov @ 2009-03-25 15:12 UTC (permalink / raw)
To: Tejun Heo; +Cc: bzolnier, linux-kernel, axboe, linux-ide
On Wed, Mar 25, 2009 at 3:17 PM, Tejun Heo <tj@kernel.org> wrote:
> Impact: use standard way to transfer data
>
> Now that data area is represented with bio, there is no need to use
> custom data transfer methods. Drop idetape_io_buffers() and
> idetape_update_buffers(). pc->bh is set to null to tell ide-atapi to
> use standard data transfer mechanism and idetape_bh byte counts are
> updated by the issuer on completion using the residual count.
>
> Signed-off-by: Tejun Heo <tj@kernel.org>
> ---
> drivers/ide/ide-tape.c | 84 +++--------------------------------------------
> 1 files changed, 6 insertions(+), 78 deletions(-)
>
> diff --git a/drivers/ide/ide-tape.c b/drivers/ide/ide-tape.c
> index bd0e839..dd37fd7 100644
> --- a/drivers/ide/ide-tape.c
> +++ b/drivers/ide/ide-tape.c
> @@ -292,65 +292,6 @@ static struct ide_tape_obj *ide_tape_chrdev_get(unsigned int i)
> return tape;
> }
>
> -static int idetape_input_buffers(ide_drive_t *drive, struct ide_atapi_pc *pc,
> - unsigned int bcount)
> -{
> - struct idetape_bh *bh = pc->bh;
> - int count;
> -
> - if (bcount && bh) {
> - count = min(
> - (unsigned int)(bh->b_size - atomic_read(&bh->b_count)),
> - bcount);
> - drive->hwif->tp_ops->input_data(drive, NULL, bh->b_data +
> - atomic_read(&bh->b_count), count);
> - bcount -= count;
> - atomic_add(count, &bh->b_count);
> - if (atomic_read(&bh->b_count) == bh->b_size)
> - pc->bh = NULL;
> - }
> -
> - return bcount;
> -}
> -
> -static int idetape_output_buffers(ide_drive_t *drive, struct ide_atapi_pc *pc,
> - unsigned int bcount)
> -{
> - struct idetape_bh *bh = pc->bh;
> - int count;
> -
> - if (bcount && bh) {
> - count = min((unsigned int)pc->b_count, (unsigned int)bcount);
> - drive->hwif->tp_ops->output_data(drive, NULL, pc->b_data, count);
> - bcount -= count;
> - pc->b_data += count;
> - pc->b_count -= count;
> - if (!pc->b_count)
> - pc->bh = NULL;
> - }
> -
> - return bcount;
> -}
> -
> -static void idetape_update_buffers(ide_drive_t *drive, struct ide_atapi_pc *pc)
> -{
> - struct idetape_bh *bh = pc->bh;
> - unsigned int bcount = pc->xferred;
> -
> - if (pc->flags & PC_FLAG_WRITING)
> - return;
> - if (bcount) {
> - if (bh == NULL || bcount > bh->b_size) {
> - printk(KERN_ERR "ide-tape: bh == NULL in %s\n",
> - __func__);
> - return;
> - }
> - atomic_set(&bh->b_count, bcount);
> - if (atomic_read(&bh->b_count) == bh->b_size)
> - pc->bh = NULL;
> - }
> -}
> -
> /*
> * called on each failed packet command retry to analyze the request sense. We
> * currently do not utilize this information.
> @@ -368,12 +309,10 @@ static void idetape_analyze_error(ide_drive_t *drive, u8 *sense)
> pc->c[0], tape->sense_key, tape->asc, tape->ascq);
>
> /* Correct pc->xferred by asking the tape. */
> - if (pc->flags & PC_FLAG_DMA_ERROR) {
> + if (pc->flags & PC_FLAG_DMA_ERROR)
> pc->xferred = pc->req_xfer -
> tape->blk_size *
> get_unaligned_be32(&sense[3]);
> - idetape_update_buffers(drive, pc);
> - }
>
> /*
> * If error was the result of a zero-length read or write command,
> @@ -520,19 +459,6 @@ static void ide_tape_handle_dsc(ide_drive_t *drive)
> idetape_postpone_request(drive);
> }
>
> -static int ide_tape_io_buffers(ide_drive_t *drive, struct ide_atapi_pc *pc,
> - unsigned int bcount, int write)
> -{
> - unsigned int bleft;
> -
> - if (write)
> - bleft = idetape_output_buffers(drive, pc, bcount);
> - else
> - bleft = idetape_input_buffers(drive, pc, bcount);
> -
> - return bcount - bleft;
> -}
> -
> /*
> * Packet Command Interface
> *
> @@ -685,7 +611,7 @@ static void ide_tape_create_rw_cmd(idetape_tape_t *tape,
> ide_init_pc(pc);
> put_unaligned(cpu_to_be32(length), (unsigned int *) &pc->c[1]);
> pc->c[1] = 1;
> - pc->bh = bh;
> + pc->bh = NULL;
> pc->buf = NULL;
> pc->buf_size = length * tape->blk_size;
> pc->req_xfer = pc->buf_size;
> @@ -1083,7 +1009,11 @@ static int idetape_queue_rw_tail(ide_drive_t *drive, int cmd, int blocks,
>
> blk_execute_rq(drive->queue, tape->disk, rq, 0);
>
> + /* calculate the number of transferred bytes and update bh */
> size -= rq->data_len;
> + if (cmd == REQ_IDETAPE_READ)
> + atomic_add(size, &bh->b_count);
> +
> ret = size;
> if (rq->errors == IDE_DRV_ERROR_GENERAL)
> ret = -EIO;
> @@ -2037,8 +1967,6 @@ static void idetape_setup(ide_drive_t *drive, idetape_tape_t *tape, int minor)
> u16 *ctl = (u16 *)&tape->caps[12];
>
> drive->pc_callback = ide_tape_callback;
> - drive->pc_update_buffers = idetape_update_buffers;
> - drive->pc_io_buffers = ide_tape_io_buffers;
You might just as well remove those two ide_drive_t members since
they're used only by ide-tape.
--
Regards/Gruss,
Boris
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 05/10] ide-tape: kill idetape_bh
2009-03-25 14:17 [RFC PATCHSET pata-2.6] ide: clean up ide-tape Tejun Heo
` (3 preceding siblings ...)
2009-03-25 14:17 ` [PATCH 04/10] ide-tape: use standard data transfer mechanism Tejun Heo
@ 2009-03-25 14:17 ` Tejun Heo
2009-03-25 14:17 ` [PATCH 06/10] ide-tape: unify r/w init paths Tejun Heo
` (5 subsequent siblings)
10 siblings, 0 replies; 19+ messages in thread
From: Tejun Heo @ 2009-03-25 14:17 UTC (permalink / raw)
To: bzolnier, linux-kernel, axboe, linux-ide; +Cc: Tejun Heo
Impact: kill now unnecessary idetape_bh
With everything using standard mechanisms, there is no need for
idetape_bh anymore. Kill it and use tape->buf, cur and valid to
describe data buffer instead.
Changes worth mentioning are...
* idetape_queue_rq_tail() now always queue tape->buf and and adjusts
buffer state properly before completion.
* idetape_pad_zeros() clears the buffer only once.
Signed-off-by: Tejun Heo <tj@kernel.org>
---
drivers/ide/ide-tape.c | 296 +++++++++++++-----------------------------------
1 files changed, 81 insertions(+), 215 deletions(-)
diff --git a/drivers/ide/ide-tape.c b/drivers/ide/ide-tape.c
index dd37fd7..e7f351b 100644
--- a/drivers/ide/ide-tape.c
+++ b/drivers/ide/ide-tape.c
@@ -131,12 +131,6 @@ enum {
IDETAPE_DIR_WRITE = (1 << 2),
};
-struct idetape_bh {
- u32 b_size;
- atomic_t b_count;
- char *b_data;
-};
-
/* Tape door status */
#define DOOR_UNLOCKED 0
#define DOOR_LOCKED 1
@@ -218,14 +212,12 @@ typedef struct ide_tape_obj {
/* Data buffer size chosen based on the tape's recommendation */
int buffer_size;
- /* merge buffer */
- struct idetape_bh *merge_bh;
- /* size of the merge buffer */
- int merge_bh_size;
- /* pointer to current buffer head within the merge buffer */
- struct idetape_bh *bh;
- char *b_data;
- int b_count;
+ /* Staging buffer of buffer_size bytes */
+ void *buf;
+ /* The read/write cursor */
+ void *cur;
+ /* The number of valid bytes in buf */
+ size_t valid;
/* Measures average tape speed */
unsigned long avg_time;
@@ -351,15 +343,6 @@ static void idetape_analyze_error(ide_drive_t *drive, u8 *sense)
}
}
-/* Free data buffers completely. */
-static void ide_tape_kfree_buffer(idetape_tape_t *tape)
-{
- struct idetape_bh *bh = tape->merge_bh;
-
- kfree(bh->b_data);
- kfree(bh);
-}
-
static void ide_tape_handle_dsc(ide_drive_t *);
static int ide_tape_callback(ide_drive_t *drive, int dsc)
@@ -605,7 +588,6 @@ static void ide_tape_create_rw_cmd(idetape_tape_t *tape,
struct ide_atapi_pc *pc, struct request *rq,
u8 opcode)
{
- struct idetape_bh *bh = (struct idetape_bh *)rq->special;
unsigned int length = rq->current_nr_sectors;
ide_init_pc(pc);
@@ -618,14 +600,11 @@ static void ide_tape_create_rw_cmd(idetape_tape_t *tape,
if (pc->req_xfer == tape->buffer_size)
pc->flags |= PC_FLAG_DMA_OK;
- if (opcode == READ_6) {
+ if (opcode == READ_6)
pc->c[0] = READ_6;
- atomic_set(&bh->b_count, 0);
- } else if (opcode == WRITE_6) {
+ else if (opcode == WRITE_6) {
pc->c[0] = WRITE_6;
pc->flags |= PC_FLAG_WRITING;
- pc->b_data = bh->b_data;
- pc->b_count = atomic_read(&bh->b_count);
}
memcpy(rq->cmd, pc->c, 12);
@@ -747,89 +726,6 @@ out:
}
/*
- * It returns a pointer to the newly allocated buffer, or NULL in case
- * of failure.
- */
-static struct idetape_bh *ide_tape_kmalloc_buffer(idetape_tape_t *tape,
- int full)
-{
- struct idetape_bh *bh;
-
- bh = kmalloc(sizeof(struct idetape_bh), GFP_KERNEL);
- if (!bh)
- return NULL;
-
- bh->b_data = kmalloc(tape->buffer_size, GFP_KERNEL);
- if (!bh->b_data) {
- kfree(bh);
- return NULL;
- }
-
- bh->b_size = tape->buffer_size;
- atomic_set(&bh->b_count, full ? bh->b_size : 0);
-
- return bh;
-}
-
-static int idetape_copy_stage_from_user(idetape_tape_t *tape,
- const char __user *buf, int n)
-{
- struct idetape_bh *bh = tape->bh;
- int ret = 0;
-
- if (n) {
- if (bh == NULL || n > bh->b_size - atomic_read(&bh->b_count)) {
- printk(KERN_ERR "ide-tape: bh == NULL in %s\n",
- __func__);
- return 1;
- }
- if (copy_from_user(bh->b_data + atomic_read(&bh->b_count), buf,
- n))
- ret = 1;
- atomic_add(n, &bh->b_count);
- if (atomic_read(&bh->b_count) == bh->b_size)
- tape->bh = NULL;
- }
-
- return ret;
-}
-
-static int idetape_copy_stage_to_user(idetape_tape_t *tape, char __user *buf,
- int n)
-{
- struct idetape_bh *bh = tape->bh;
- int ret = 0;
-
- if (n) {
- if (bh == NULL || n > tape->b_count) {
- printk(KERN_ERR "ide-tape: bh == NULL in %s\n",
- __func__);
- return 1;
- }
- if (copy_to_user(buf, tape->b_data, n))
- ret = 1;
- tape->b_data += n;
- tape->b_count -= n;
- if (!tape->b_count)
- tape->bh = NULL;
- }
- return ret;
-}
-
-static void idetape_init_merge_buffer(idetape_tape_t *tape)
-{
- struct idetape_bh *bh = tape->merge_bh;
- tape->bh = tape->merge_bh;
-
- if (tape->chrdev_dir == IDETAPE_DIR_WRITE)
- atomic_set(&bh->b_count, 0);
- else {
- tape->b_data = bh->b_data;
- tape->b_count = atomic_read(&bh->b_count);
- }
-}
-
-/*
* Write a filemark if write_filemark=1. Flush the device buffers without
* writing a filemark otherwise.
*/
@@ -926,10 +822,10 @@ static void __ide_tape_discard_merge_buffer(ide_drive_t *drive)
return;
clear_bit(IDE_AFLAG_FILEMARK, &drive->atapi_flags);
- tape->merge_bh_size = 0;
- if (tape->merge_bh != NULL) {
- ide_tape_kfree_buffer(tape);
- tape->merge_bh = NULL;
+ tape->valid = 0;
+ if (tape->buf != NULL) {
+ kfree(tape->buf);
+ tape->buf = NULL;
}
tape->chrdev_dir = IDETAPE_DIR_NONE;
@@ -983,8 +879,7 @@ static void ide_tape_discard_merge_buffer(ide_drive_t *drive,
* Generate a read/write request for the block device interface and wait for it
* to be serviced.
*/
-static int idetape_queue_rw_tail(ide_drive_t *drive, int cmd, int blocks,
- struct idetape_bh *bh)
+static int idetape_queue_rw_tail(ide_drive_t *drive, int cmd, int blocks)
{
idetape_tape_t *tape = drive->driver_data;
size_t size = blocks * tape->blk_size;
@@ -1001,7 +896,7 @@ static int idetape_queue_rw_tail(ide_drive_t *drive, int cmd, int blocks,
rq->sector = tape->first_frame;
if (size) {
- ret = blk_rq_map_kern(drive->queue, rq, bh->b_data, size,
+ ret = blk_rq_map_kern(drive->queue, rq, tape->buf, size,
__GFP_WAIT);
if (ret)
goto out_put;
@@ -1009,17 +904,17 @@ static int idetape_queue_rw_tail(ide_drive_t *drive, int cmd, int blocks,
blk_execute_rq(drive->queue, tape->disk, rq, 0);
- /* calculate the number of transferred bytes and update bh */
+ /* calculate the number of transferred bytes and update buffer state */
size -= rq->data_len;
+ tape->cur = tape->buf;
if (cmd == REQ_IDETAPE_READ)
- atomic_add(size, &bh->b_count);
+ tape->valid = size;
+ else
+ tape->valid = 0;
ret = size;
if (rq->errors == IDE_DRV_ERROR_GENERAL)
ret = -EIO;
-
- if (tape->merge_bh)
- idetape_init_merge_buffer(tape);
out_put:
blk_put_request(rq);
return ret;
@@ -1061,49 +956,33 @@ static void idetape_create_space_cmd(struct ide_atapi_pc *pc, int count, u8 cmd)
/* Queue up a character device originated write request. */
static int idetape_add_chrdev_write_request(ide_drive_t *drive, int blocks)
{
- idetape_tape_t *tape = drive->driver_data;
-
debug_log(DBG_CHRDEV, "Enter %s\n", __func__);
- return idetape_queue_rw_tail(drive, REQ_IDETAPE_WRITE,
- blocks, tape->merge_bh);
+ return idetape_queue_rw_tail(drive, REQ_IDETAPE_WRITE, blocks);
}
static void ide_tape_flush_merge_buffer(ide_drive_t *drive)
{
idetape_tape_t *tape = drive->driver_data;
int blocks;
- struct idetape_bh *bh;
if (tape->chrdev_dir != IDETAPE_DIR_WRITE) {
printk(KERN_ERR "ide-tape: bug: Trying to empty merge buffer"
" but we are not writing.\n");
return;
}
- if (tape->merge_bh_size > tape->buffer_size) {
- printk(KERN_ERR "ide-tape: bug: merge_buffer too big\n");
- tape->merge_bh_size = tape->buffer_size;
- }
- if (tape->merge_bh_size) {
- blocks = tape->merge_bh_size / tape->blk_size;
- if (tape->merge_bh_size % tape->blk_size) {
- unsigned int i = tape->blk_size -
- tape->merge_bh_size % tape->blk_size;
+ if (tape->buf) {
+ blocks = tape->valid / tape->blk_size;
+ if (tape->valid % tape->blk_size) {
blocks++;
- bh = tape->bh;
- if (bh) {
- memset(bh->b_data + atomic_read(&bh->b_count),
- 0, i);
- atomic_add(i, &bh->b_count);
- } else
- printk(KERN_INFO "ide-tape: bug, bh NULL\n");
+ memset(tape->buf + tape->valid, 0,
+ tape->blk_size - tape->valid % tape->blk_size);
}
(void) idetape_add_chrdev_write_request(drive, blocks);
- tape->merge_bh_size = 0;
}
- if (tape->merge_bh != NULL) {
- ide_tape_kfree_buffer(tape);
- tape->merge_bh = NULL;
+ if (tape->buf != NULL) {
+ kfree(tape->buf);
+ tape->buf = NULL;
}
tape->chrdev_dir = IDETAPE_DIR_NONE;
}
@@ -1119,15 +998,15 @@ static int idetape_init_read(ide_drive_t *drive)
ide_tape_flush_merge_buffer(drive);
idetape_flush_tape_buffers(drive);
}
- if (tape->merge_bh || tape->merge_bh_size) {
- printk(KERN_ERR "ide-tape: merge_bh_size should be"
- " 0 now\n");
- tape->merge_bh_size = 0;
+ if (tape->buf || tape->valid) {
+ printk(KERN_ERR "ide-tape: valid should be 0 now\n");
+ tape->valid = 0;
}
- tape->merge_bh = ide_tape_kmalloc_buffer(tape, 0);
- if (!tape->merge_bh)
+ tape->buf = kmalloc(tape->buffer_size, GFP_KERNEL);
+ if (!tape->buf)
return -ENOMEM;
tape->chrdev_dir = IDETAPE_DIR_READ;
+ tape->cur = tape->buf;
/*
* Issue a read 0 command to ensure that DSC handshake is
@@ -1137,11 +1016,10 @@ static int idetape_init_read(ide_drive_t *drive)
*/
if (drive->dev_flags & IDE_DFLAG_DSC_OVERLAP) {
bytes_read = idetape_queue_rw_tail(drive,
- REQ_IDETAPE_READ, 0,
- tape->merge_bh);
+ REQ_IDETAPE_READ, 0);
if (bytes_read < 0) {
- ide_tape_kfree_buffer(tape);
- tape->merge_bh = NULL;
+ kfree(tape->buf);
+ tape->buf = NULL;
tape->chrdev_dir = IDETAPE_DIR_NONE;
return bytes_read;
}
@@ -1154,8 +1032,6 @@ static int idetape_init_read(ide_drive_t *drive)
/* called from idetape_chrdev_read() to service a chrdev read request. */
static int idetape_add_chrdev_read_request(ide_drive_t *drive, int blocks)
{
- idetape_tape_t *tape = drive->driver_data;
-
debug_log(DBG_PROCS, "Enter %s, %d blocks\n", __func__, blocks);
/* If we are at a filemark, return a read length of 0 */
@@ -1164,27 +1040,21 @@ static int idetape_add_chrdev_read_request(ide_drive_t *drive, int blocks)
idetape_init_read(drive);
- return idetape_queue_rw_tail(drive, REQ_IDETAPE_READ, blocks,
- tape->merge_bh);
+ return idetape_queue_rw_tail(drive, REQ_IDETAPE_READ, blocks);
}
static void idetape_pad_zeros(ide_drive_t *drive, int bcount)
{
idetape_tape_t *tape = drive->driver_data;
- struct idetape_bh *bh = tape->merge_bh;
- int blocks;
+
+ memset(tape->buf, 0, tape->buffer_size);
while (bcount) {
- unsigned int count;
+ unsigned int count = min(tape->buffer_size, bcount);
- count = min(tape->buffer_size, bcount);
+ idetape_queue_rw_tail(drive, REQ_IDETAPE_WRITE,
+ count / tape->blk_size);
bcount -= count;
- blocks = count / tape->blk_size;
- atomic_set(&bh->b_count, count);
- memset(bh->b_data, 0, atomic_read(&bh->b_count));
-
- idetape_queue_rw_tail(drive, REQ_IDETAPE_WRITE, blocks,
- tape->merge_bh);
}
}
@@ -1264,7 +1134,7 @@ static int idetape_space_over_filemarks(ide_drive_t *drive, short mt_op,
}
if (tape->chrdev_dir == IDETAPE_DIR_READ) {
- tape->merge_bh_size = 0;
+ tape->valid = 0;
if (test_and_clear_bit(IDE_AFLAG_FILEMARK, &drive->atapi_flags))
++count;
ide_tape_discard_merge_buffer(drive, 0);
@@ -1330,20 +1200,20 @@ static ssize_t idetape_chrdev_read(struct file *file, char __user *buf,
return rc;
if (count == 0)
return (0);
- if (tape->merge_bh_size) {
- actually_read = min((unsigned int)(tape->merge_bh_size),
- (unsigned int)count);
- if (idetape_copy_stage_to_user(tape, buf, actually_read))
+ if (tape->valid) {
+ actually_read = min_t(unsigned int, tape->valid, count);
+ if (copy_to_user(buf, tape->cur, actually_read))
ret = -EFAULT;
buf += actually_read;
- tape->merge_bh_size -= actually_read;
count -= actually_read;
+ tape->cur += actually_read;
+ tape->valid -= actually_read;
}
while (count >= tape->buffer_size) {
bytes_read = idetape_add_chrdev_read_request(drive, ctl);
if (bytes_read <= 0)
goto finish;
- if (idetape_copy_stage_to_user(tape, buf, bytes_read))
+ if (copy_to_user(buf, tape->cur, bytes_read))
ret = -EFAULT;
buf += bytes_read;
count -= bytes_read;
@@ -1354,10 +1224,11 @@ static ssize_t idetape_chrdev_read(struct file *file, char __user *buf,
if (bytes_read <= 0)
goto finish;
temp = min((unsigned long)count, (unsigned long)bytes_read);
- if (idetape_copy_stage_to_user(tape, buf, temp))
+ if (copy_to_user(buf, tape->cur, temp))
ret = -EFAULT;
actually_read += temp;
- tape->merge_bh_size = bytes_read-temp;
+ tape->cur += temp;
+ tape->valid -= temp;
}
finish:
if (!actually_read && test_bit(IDE_AFLAG_FILEMARK, &drive->atapi_flags)) {
@@ -1389,16 +1260,15 @@ static ssize_t idetape_chrdev_write(struct file *file, const char __user *buf,
if (tape->chrdev_dir != IDETAPE_DIR_WRITE) {
if (tape->chrdev_dir == IDETAPE_DIR_READ)
ide_tape_discard_merge_buffer(drive, 1);
- if (tape->merge_bh || tape->merge_bh_size) {
- printk(KERN_ERR "ide-tape: merge_bh_size "
- "should be 0 now\n");
- tape->merge_bh_size = 0;
+ if (tape->buf || tape->valid) {
+ printk(KERN_ERR "ide-tape: valid should be 0 now\n");
+ tape->valid = 0;
}
- tape->merge_bh = ide_tape_kmalloc_buffer(tape, 0);
- if (!tape->merge_bh)
+ tape->buf = kmalloc(tape->buffer_size, GFP_KERNEL);
+ if (!tape->buf)
return -ENOMEM;
tape->chrdev_dir = IDETAPE_DIR_WRITE;
- idetape_init_merge_buffer(tape);
+ tape->cur = tape->buf;
/*
* Issue a write 0 command to ensure that DSC handshake is
@@ -1408,11 +1278,10 @@ static ssize_t idetape_chrdev_write(struct file *file, const char __user *buf,
*/
if (drive->dev_flags & IDE_DFLAG_DSC_OVERLAP) {
ssize_t retval = idetape_queue_rw_tail(drive,
- REQ_IDETAPE_WRITE, 0,
- tape->merge_bh);
+ REQ_IDETAPE_WRITE, 0);
if (retval < 0) {
- ide_tape_kfree_buffer(tape);
- tape->merge_bh = NULL;
+ kfree(tape->buf);
+ tape->buf = NULL;
tape->chrdev_dir = IDETAPE_DIR_NONE;
return retval;
}
@@ -1420,23 +1289,19 @@ static ssize_t idetape_chrdev_write(struct file *file, const char __user *buf,
}
if (count == 0)
return (0);
- if (tape->merge_bh_size) {
- if (tape->merge_bh_size >= tape->buffer_size) {
- printk(KERN_ERR "ide-tape: bug: merge buf too big\n");
- tape->merge_bh_size = 0;
- }
- actually_written = min((unsigned int)
- (tape->buffer_size - tape->merge_bh_size),
- (unsigned int)count);
- if (idetape_copy_stage_from_user(tape, buf, actually_written))
- ret = -EFAULT;
+ if (tape->valid < tape->buffer_size) {
+ actually_written = min_t(unsigned int,
+ tape->buffer_size - tape->valid,
+ count);
+ if (copy_from_user(tape->cur, buf, actually_written))
+ ret = -EFAULT;
buf += actually_written;
- tape->merge_bh_size += actually_written;
count -= actually_written;
+ tape->cur += actually_written;
+ tape->valid += actually_written;
- if (tape->merge_bh_size == tape->buffer_size) {
+ if (tape->valid == tape->buffer_size) {
ssize_t retval;
- tape->merge_bh_size = 0;
retval = idetape_add_chrdev_write_request(drive, ctl);
if (retval <= 0)
return (retval);
@@ -1444,7 +1309,7 @@ static ssize_t idetape_chrdev_write(struct file *file, const char __user *buf,
}
while (count >= tape->buffer_size) {
ssize_t retval;
- if (idetape_copy_stage_from_user(tape, buf, tape->buffer_size))
+ if (copy_from_user(tape->cur, buf, tape->buffer_size))
ret = -EFAULT;
buf += tape->buffer_size;
count -= tape->buffer_size;
@@ -1455,9 +1320,10 @@ static ssize_t idetape_chrdev_write(struct file *file, const char __user *buf,
}
if (count) {
actually_written += count;
- if (idetape_copy_stage_from_user(tape, buf, count))
+ if (copy_from_user(tape->cur, buf, count))
ret = -EFAULT;
- tape->merge_bh_size += count;
+ tape->cur += count;
+ tape->valid += count;
}
return ret ? ret : actually_written;
}
@@ -1620,7 +1486,7 @@ static int idetape_chrdev_ioctl(struct inode *inode, struct file *file,
idetape_flush_tape_buffers(drive);
}
if (cmd == MTIOCGET || cmd == MTIOCPOS) {
- block_offset = tape->merge_bh_size /
+ block_offset = tape->valid /
(tape->blk_size * tape->user_bs_factor);
position = idetape_read_position(drive);
if (position < 0)
@@ -1768,12 +1634,12 @@ static void idetape_write_release(ide_drive_t *drive, unsigned int minor)
idetape_tape_t *tape = drive->driver_data;
ide_tape_flush_merge_buffer(drive);
- tape->merge_bh = ide_tape_kmalloc_buffer(tape, 1);
- if (tape->merge_bh != NULL) {
+ tape->buf = kmalloc(tape->buffer_size, GFP_KERNEL);
+ if (tape->buf != NULL) {
idetape_pad_zeros(drive, tape->blk_size *
(tape->user_bs_factor - 1));
- ide_tape_kfree_buffer(tape);
- tape->merge_bh = NULL;
+ kfree(tape->buf);
+ tape->buf = NULL;
}
idetape_write_filemark(drive);
idetape_flush_tape_buffers(drive);
@@ -2039,7 +1905,7 @@ static void ide_tape_release(struct device *dev)
ide_drive_t *drive = tape->drive;
struct gendisk *g = tape->disk;
- BUG_ON(tape->merge_bh_size);
+ BUG_ON(tape->valid);
drive->dev_flags &= ~IDE_DFLAG_DSC_OVERLAP;
drive->driver_data = NULL;
--
1.6.0.2
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH 06/10] ide-tape: unify r/w init paths
2009-03-25 14:17 [RFC PATCHSET pata-2.6] ide: clean up ide-tape Tejun Heo
` (4 preceding siblings ...)
2009-03-25 14:17 ` [PATCH 05/10] ide-tape: kill idetape_bh Tejun Heo
@ 2009-03-25 14:17 ` Tejun Heo
2009-03-25 14:17 ` [PATCH 07/10] ide-tape: use byte size instead of sectors on rw issue functions Tejun Heo
` (4 subsequent siblings)
10 siblings, 0 replies; 19+ messages in thread
From: Tejun Heo @ 2009-03-25 14:17 UTC (permalink / raw)
To: bzolnier, linux-kernel, axboe, linux-ide; +Cc: Tejun Heo
Impact: cleanup
Read and write init paths are almost identical. Unify them into
idetape_init_rw().
Signed-off-by: Tejun Heo <tj@kernel.org>
---
drivers/ide/ide-tape.c | 106 +++++++++++++++++++-----------------------------
1 files changed, 42 insertions(+), 64 deletions(-)
diff --git a/drivers/ide/ide-tape.c b/drivers/ide/ide-tape.c
index e7f351b..c013954 100644
--- a/drivers/ide/ide-tape.c
+++ b/drivers/ide/ide-tape.c
@@ -987,42 +987,46 @@ static void ide_tape_flush_merge_buffer(ide_drive_t *drive)
tape->chrdev_dir = IDETAPE_DIR_NONE;
}
-static int idetape_init_read(ide_drive_t *drive)
+static int idetape_init_rw(ide_drive_t *drive, int dir)
{
idetape_tape_t *tape = drive->driver_data;
- int bytes_read;
- /* Initialize read operation */
- if (tape->chrdev_dir != IDETAPE_DIR_READ) {
- if (tape->chrdev_dir == IDETAPE_DIR_WRITE) {
- ide_tape_flush_merge_buffer(drive);
- idetape_flush_tape_buffers(drive);
- }
- if (tape->buf || tape->valid) {
- printk(KERN_ERR "ide-tape: valid should be 0 now\n");
- tape->valid = 0;
- }
- tape->buf = kmalloc(tape->buffer_size, GFP_KERNEL);
- if (!tape->buf)
- return -ENOMEM;
- tape->chrdev_dir = IDETAPE_DIR_READ;
- tape->cur = tape->buf;
+ if (tape->chrdev_dir == dir)
+ return 0;
- /*
- * Issue a read 0 command to ensure that DSC handshake is
- * switched from completion mode to buffer available mode.
- * No point in issuing this if DSC overlap isn't supported, some
- * drives (Seagate STT3401A) will return an error.
- */
- if (drive->dev_flags & IDE_DFLAG_DSC_OVERLAP) {
- bytes_read = idetape_queue_rw_tail(drive,
- REQ_IDETAPE_READ, 0);
- if (bytes_read < 0) {
- kfree(tape->buf);
- tape->buf = NULL;
- tape->chrdev_dir = IDETAPE_DIR_NONE;
- return bytes_read;
- }
+ if (tape->chrdev_dir == IDETAPE_DIR_READ)
+ ide_tape_discard_merge_buffer(drive, 1);
+ else if (tape->chrdev_dir == IDETAPE_DIR_WRITE) {
+ ide_tape_flush_merge_buffer(drive);
+ idetape_flush_tape_buffers(drive);
+ }
+
+ if (tape->buf || tape->valid) {
+ printk(KERN_ERR "ide-tape: valid should be 0 now\n");
+ tape->valid = 0;
+ }
+
+ tape->buf = kmalloc(tape->buffer_size, GFP_KERNEL);
+ if (!tape->buf)
+ return -ENOMEM;
+ tape->chrdev_dir = dir;
+ tape->cur = tape->buf;
+
+ /*
+ * Issue a 0 rw command to ensure that DSC handshake is
+ * switched from completion mode to buffer available mode. No
+ * point in issuing this if DSC overlap isn't supported, some
+ * drives (Seagate STT3401A) will return an error.
+ */
+ if (drive->dev_flags & IDE_DFLAG_DSC_OVERLAP) {
+ int rc;
+
+ rc = idetape_queue_rw_tail(drive, dir, 0);
+ if (rc < 0) {
+ kfree(tape->buf);
+ tape->buf = NULL;
+ tape->chrdev_dir = IDETAPE_DIR_NONE;
+ return rc;
}
}
@@ -1038,7 +1042,7 @@ static int idetape_add_chrdev_read_request(ide_drive_t *drive, int blocks)
if (test_bit(IDE_AFLAG_FILEMARK, &drive->atapi_flags))
return 0;
- idetape_init_read(drive);
+ idetape_init_rw(drive, IDETAPE_DIR_READ);
return idetape_queue_rw_tail(drive, REQ_IDETAPE_READ, blocks);
}
@@ -1195,7 +1199,7 @@ static ssize_t idetape_chrdev_read(struct file *file, char __user *buf,
(count % tape->blk_size) == 0)
tape->user_bs_factor = count / tape->blk_size;
}
- rc = idetape_init_read(drive);
+ rc = idetape_init_rw(drive, IDETAPE_DIR_READ);
if (rc < 0)
return rc;
if (count == 0)
@@ -1249,6 +1253,7 @@ static ssize_t idetape_chrdev_write(struct file *file, const char __user *buf,
ssize_t actually_written = 0;
ssize_t ret = 0;
u16 ctl = *(u16 *)&tape->caps[12];
+ int rc;
/* The drive is write protected. */
if (tape->write_prot)
@@ -1257,36 +1262,9 @@ static ssize_t idetape_chrdev_write(struct file *file, const char __user *buf,
debug_log(DBG_CHRDEV, "Enter %s, count %Zd\n", __func__, count);
/* Initialize write operation */
- if (tape->chrdev_dir != IDETAPE_DIR_WRITE) {
- if (tape->chrdev_dir == IDETAPE_DIR_READ)
- ide_tape_discard_merge_buffer(drive, 1);
- if (tape->buf || tape->valid) {
- printk(KERN_ERR "ide-tape: valid should be 0 now\n");
- tape->valid = 0;
- }
- tape->buf = kmalloc(tape->buffer_size, GFP_KERNEL);
- if (!tape->buf)
- return -ENOMEM;
- tape->chrdev_dir = IDETAPE_DIR_WRITE;
- tape->cur = tape->buf;
-
- /*
- * Issue a write 0 command to ensure that DSC handshake is
- * switched from completion mode to buffer available mode. No
- * point in issuing this if DSC overlap isn't supported, some
- * drives (Seagate STT3401A) will return an error.
- */
- if (drive->dev_flags & IDE_DFLAG_DSC_OVERLAP) {
- ssize_t retval = idetape_queue_rw_tail(drive,
- REQ_IDETAPE_WRITE, 0);
- if (retval < 0) {
- kfree(tape->buf);
- tape->buf = NULL;
- tape->chrdev_dir = IDETAPE_DIR_NONE;
- return retval;
- }
- }
- }
+ rc = idetape_init_rw(drive, IDETAPE_DIR_WRITE);
+ if (rc < 0)
+ return rc;
if (count == 0)
return (0);
if (tape->valid < tape->buffer_size) {
--
1.6.0.2
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH 07/10] ide-tape: use byte size instead of sectors on rw issue functions
2009-03-25 14:17 [RFC PATCHSET pata-2.6] ide: clean up ide-tape Tejun Heo
` (5 preceding siblings ...)
2009-03-25 14:17 ` [PATCH 06/10] ide-tape: unify r/w init paths Tejun Heo
@ 2009-03-25 14:17 ` Tejun Heo
2009-03-25 14:17 ` [PATCH 08/10] ide-tape: simplify read/write functions Tejun Heo
` (3 subsequent siblings)
10 siblings, 0 replies; 19+ messages in thread
From: Tejun Heo @ 2009-03-25 14:17 UTC (permalink / raw)
To: bzolnier, linux-kernel, axboe, linux-ide; +Cc: Tejun Heo
Impact: cleanup
Byte size is what most issue functions deal with, make
idetape_queue_rw_tail() and its wrappers take byte size instead of
sector counts. idetape_chrdev_read() and write() functions are
converted to use tape->buffer_size instead of ctl from tape->cap.
This cleans up code a little bit and will ease the next r/w
reimplementation.
Signed-off-by: Tejun Heo <tj@kernel.org>
---
drivers/ide/ide-tape.c | 45 ++++++++++++++++++++-------------------------
1 files changed, 20 insertions(+), 25 deletions(-)
diff --git a/drivers/ide/ide-tape.c b/drivers/ide/ide-tape.c
index c013954..5fc3282 100644
--- a/drivers/ide/ide-tape.c
+++ b/drivers/ide/ide-tape.c
@@ -879,15 +879,15 @@ static void ide_tape_discard_merge_buffer(ide_drive_t *drive,
* Generate a read/write request for the block device interface and wait for it
* to be serviced.
*/
-static int idetape_queue_rw_tail(ide_drive_t *drive, int cmd, int blocks)
+static int idetape_queue_rw_tail(ide_drive_t *drive, int cmd, int size)
{
idetape_tape_t *tape = drive->driver_data;
- size_t size = blocks * tape->blk_size;
struct request *rq;
int ret;
debug_log(DBG_SENSE, "%s: cmd=%d\n", __func__, cmd);
BUG_ON(cmd != REQ_IDETAPE_READ && cmd != REQ_IDETAPE_WRITE);
+ BUG_ON(size < 0 || size % tape->blk_size);
rq = blk_get_request(drive->queue, READ, __GFP_WAIT);
rq->cmd_type = REQ_TYPE_SPECIAL;
@@ -954,17 +954,16 @@ static void idetape_create_space_cmd(struct ide_atapi_pc *pc, int count, u8 cmd)
}
/* Queue up a character device originated write request. */
-static int idetape_add_chrdev_write_request(ide_drive_t *drive, int blocks)
+static int idetape_add_chrdev_write_request(ide_drive_t *drive, int size)
{
debug_log(DBG_CHRDEV, "Enter %s\n", __func__);
- return idetape_queue_rw_tail(drive, REQ_IDETAPE_WRITE, blocks);
+ return idetape_queue_rw_tail(drive, REQ_IDETAPE_WRITE, size);
}
static void ide_tape_flush_merge_buffer(ide_drive_t *drive)
{
idetape_tape_t *tape = drive->driver_data;
- int blocks;
if (tape->chrdev_dir != IDETAPE_DIR_WRITE) {
printk(KERN_ERR "ide-tape: bug: Trying to empty merge buffer"
@@ -972,15 +971,10 @@ static void ide_tape_flush_merge_buffer(ide_drive_t *drive)
return;
}
if (tape->buf) {
- blocks = tape->valid / tape->blk_size;
- if (tape->valid % tape->blk_size) {
- blocks++;
- memset(tape->buf + tape->valid, 0,
- tape->blk_size - tape->valid % tape->blk_size);
- }
- (void) idetape_add_chrdev_write_request(drive, blocks);
- }
- if (tape->buf != NULL) {
+ size_t aligned = roundup(tape->valid, tape->blk_size);
+
+ memset(tape->cur + tape->valid, 0, aligned - tape->valid);
+ idetape_add_chrdev_write_request(drive, aligned);
kfree(tape->buf);
tape->buf = NULL;
}
@@ -1034,9 +1028,9 @@ static int idetape_init_rw(ide_drive_t *drive, int dir)
}
/* called from idetape_chrdev_read() to service a chrdev read request. */
-static int idetape_add_chrdev_read_request(ide_drive_t *drive, int blocks)
+static int idetape_add_chrdev_read_request(ide_drive_t *drive, int size)
{
- debug_log(DBG_PROCS, "Enter %s, %d blocks\n", __func__, blocks);
+ debug_log(DBG_PROCS, "Enter %s, %d bytes\n", __func__, size);
/* If we are at a filemark, return a read length of 0 */
if (test_bit(IDE_AFLAG_FILEMARK, &drive->atapi_flags))
@@ -1044,7 +1038,7 @@ static int idetape_add_chrdev_read_request(ide_drive_t *drive, int blocks)
idetape_init_rw(drive, IDETAPE_DIR_READ);
- return idetape_queue_rw_tail(drive, REQ_IDETAPE_READ, blocks);
+ return idetape_queue_rw_tail(drive, REQ_IDETAPE_READ, size);
}
static void idetape_pad_zeros(ide_drive_t *drive, int bcount)
@@ -1056,8 +1050,7 @@ static void idetape_pad_zeros(ide_drive_t *drive, int bcount)
while (bcount) {
unsigned int count = min(tape->buffer_size, bcount);
- idetape_queue_rw_tail(drive, REQ_IDETAPE_WRITE,
- count / tape->blk_size);
+ idetape_queue_rw_tail(drive, REQ_IDETAPE_WRITE, count);
bcount -= count;
}
}
@@ -1189,7 +1182,6 @@ static ssize_t idetape_chrdev_read(struct file *file, char __user *buf,
ide_drive_t *drive = tape->drive;
ssize_t bytes_read, temp, actually_read = 0, rc;
ssize_t ret = 0;
- u16 ctl = *(u16 *)&tape->caps[12];
debug_log(DBG_CHRDEV, "Enter %s, count %Zd\n", __func__, count);
@@ -1214,7 +1206,8 @@ static ssize_t idetape_chrdev_read(struct file *file, char __user *buf,
tape->valid -= actually_read;
}
while (count >= tape->buffer_size) {
- bytes_read = idetape_add_chrdev_read_request(drive, ctl);
+ bytes_read = idetape_add_chrdev_read_request(drive,
+ tape->buffer_size);
if (bytes_read <= 0)
goto finish;
if (copy_to_user(buf, tape->cur, bytes_read))
@@ -1224,7 +1217,8 @@ static ssize_t idetape_chrdev_read(struct file *file, char __user *buf,
actually_read += bytes_read;
}
if (count) {
- bytes_read = idetape_add_chrdev_read_request(drive, ctl);
+ bytes_read = idetape_add_chrdev_read_request(drive,
+ tape->buffer_size);
if (bytes_read <= 0)
goto finish;
temp = min((unsigned long)count, (unsigned long)bytes_read);
@@ -1252,7 +1246,6 @@ static ssize_t idetape_chrdev_write(struct file *file, const char __user *buf,
ide_drive_t *drive = tape->drive;
ssize_t actually_written = 0;
ssize_t ret = 0;
- u16 ctl = *(u16 *)&tape->caps[12];
int rc;
/* The drive is write protected. */
@@ -1280,7 +1273,8 @@ static ssize_t idetape_chrdev_write(struct file *file, const char __user *buf,
if (tape->valid == tape->buffer_size) {
ssize_t retval;
- retval = idetape_add_chrdev_write_request(drive, ctl);
+ retval = idetape_add_chrdev_write_request(drive,
+ tape->buffer_size);
if (retval <= 0)
return (retval);
}
@@ -1291,7 +1285,8 @@ static ssize_t idetape_chrdev_write(struct file *file, const char __user *buf,
ret = -EFAULT;
buf += tape->buffer_size;
count -= tape->buffer_size;
- retval = idetape_add_chrdev_write_request(drive, ctl);
+ retval = idetape_add_chrdev_write_request(drive,
+ tape->buffer_size);
actually_written += tape->buffer_size;
if (retval <= 0)
return (retval);
--
1.6.0.2
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH 08/10] ide-tape: simplify read/write functions
2009-03-25 14:17 [RFC PATCHSET pata-2.6] ide: clean up ide-tape Tejun Heo
` (6 preceding siblings ...)
2009-03-25 14:17 ` [PATCH 07/10] ide-tape: use byte size instead of sectors on rw issue functions Tejun Heo
@ 2009-03-25 14:17 ` Tejun Heo
2009-03-25 14:17 ` [PATCH 09/10] ide-atapi: kill unused fields and callbacks Tejun Heo
` (2 subsequent siblings)
10 siblings, 0 replies; 19+ messages in thread
From: Tejun Heo @ 2009-03-25 14:17 UTC (permalink / raw)
To: bzolnier, linux-kernel, axboe, linux-ide; +Cc: Tejun Heo
Impact: cleanup
idetape_chrdev_read/write() functions are unnecessarily complex when
everything can be handled in a single loop. Collapse
idetape_add_chrdev_read/write_request() into the rw functions and
simplify the implementation.
Signed-off-by: Tejun Heo <tj@kernel.org>
---
drivers/ide/ide-tape.c | 149 ++++++++++++++++--------------------------------
1 files changed, 50 insertions(+), 99 deletions(-)
diff --git a/drivers/ide/ide-tape.c b/drivers/ide/ide-tape.c
index 5fc3282..95e9131 100644
--- a/drivers/ide/ide-tape.c
+++ b/drivers/ide/ide-tape.c
@@ -953,14 +953,6 @@ static void idetape_create_space_cmd(struct ide_atapi_pc *pc, int count, u8 cmd)
pc->flags |= PC_FLAG_WAIT_FOR_DSC;
}
-/* Queue up a character device originated write request. */
-static int idetape_add_chrdev_write_request(ide_drive_t *drive, int size)
-{
- debug_log(DBG_CHRDEV, "Enter %s\n", __func__);
-
- return idetape_queue_rw_tail(drive, REQ_IDETAPE_WRITE, size);
-}
-
static void ide_tape_flush_merge_buffer(ide_drive_t *drive)
{
idetape_tape_t *tape = drive->driver_data;
@@ -974,7 +966,7 @@ static void ide_tape_flush_merge_buffer(ide_drive_t *drive)
size_t aligned = roundup(tape->valid, tape->blk_size);
memset(tape->cur + tape->valid, 0, aligned - tape->valid);
- idetape_add_chrdev_write_request(drive, aligned);
+ idetape_queue_rw_tail(drive, REQ_IDETAPE_WRITE, aligned);
kfree(tape->buf);
tape->buf = NULL;
}
@@ -1027,20 +1019,6 @@ static int idetape_init_rw(ide_drive_t *drive, int dir)
return 0;
}
-/* called from idetape_chrdev_read() to service a chrdev read request. */
-static int idetape_add_chrdev_read_request(ide_drive_t *drive, int size)
-{
- debug_log(DBG_PROCS, "Enter %s, %d bytes\n", __func__, size);
-
- /* If we are at a filemark, return a read length of 0 */
- if (test_bit(IDE_AFLAG_FILEMARK, &drive->atapi_flags))
- return 0;
-
- idetape_init_rw(drive, IDETAPE_DIR_READ);
-
- return idetape_queue_rw_tail(drive, REQ_IDETAPE_READ, size);
-}
-
static void idetape_pad_zeros(ide_drive_t *drive, int bcount)
{
idetape_tape_t *tape = drive->driver_data;
@@ -1180,8 +1158,9 @@ static ssize_t idetape_chrdev_read(struct file *file, char __user *buf,
{
struct ide_tape_obj *tape = file->private_data;
ide_drive_t *drive = tape->drive;
- ssize_t bytes_read, temp, actually_read = 0, rc;
+ size_t done = 0;
ssize_t ret = 0;
+ int rc;
debug_log(DBG_CHRDEV, "Enter %s, count %Zd\n", __func__, count);
@@ -1191,52 +1170,43 @@ static ssize_t idetape_chrdev_read(struct file *file, char __user *buf,
(count % tape->blk_size) == 0)
tape->user_bs_factor = count / tape->blk_size;
}
+
rc = idetape_init_rw(drive, IDETAPE_DIR_READ);
if (rc < 0)
return rc;
- if (count == 0)
- return (0);
- if (tape->valid) {
- actually_read = min_t(unsigned int, tape->valid, count);
- if (copy_to_user(buf, tape->cur, actually_read))
- ret = -EFAULT;
- buf += actually_read;
- count -= actually_read;
- tape->cur += actually_read;
- tape->valid -= actually_read;
- }
- while (count >= tape->buffer_size) {
- bytes_read = idetape_add_chrdev_read_request(drive,
- tape->buffer_size);
- if (bytes_read <= 0)
- goto finish;
- if (copy_to_user(buf, tape->cur, bytes_read))
- ret = -EFAULT;
- buf += bytes_read;
- count -= bytes_read;
- actually_read += bytes_read;
- }
- if (count) {
- bytes_read = idetape_add_chrdev_read_request(drive,
- tape->buffer_size);
- if (bytes_read <= 0)
- goto finish;
- temp = min((unsigned long)count, (unsigned long)bytes_read);
- if (copy_to_user(buf, tape->cur, temp))
+
+ while (done < count) {
+ size_t todo;
+
+ /* refill if staging buffer is empty */
+ if (!tape->valid) {
+ /* If we are at a filemark, nothing more to read */
+ if (test_bit(IDE_AFLAG_FILEMARK, &drive->atapi_flags))
+ break;
+ /* read */
+ if (idetape_queue_rw_tail(drive, REQ_IDETAPE_READ,
+ tape->buffer_size) <= 0)
+ break;
+ }
+
+ /* copy out */
+ todo = min_t(size_t, count - done, tape->valid);
+ if (copy_to_user(buf + done, tape->cur, todo))
ret = -EFAULT;
- actually_read += temp;
- tape->cur += temp;
- tape->valid -= temp;
+
+ tape->cur += todo;
+ tape->valid -= todo;
+ done += todo;
}
-finish:
- if (!actually_read && test_bit(IDE_AFLAG_FILEMARK, &drive->atapi_flags)) {
+
+ if (!done && test_bit(IDE_AFLAG_FILEMARK, &drive->atapi_flags)) {
debug_log(DBG_SENSE, "%s: spacing over filemark\n", tape->name);
idetape_space_over_filemarks(drive, MTFSF, 1);
return 0;
}
- return ret ? ret : actually_read;
+ return ret ? ret : done;
}
static ssize_t idetape_chrdev_write(struct file *file, const char __user *buf,
@@ -1244,7 +1214,7 @@ static ssize_t idetape_chrdev_write(struct file *file, const char __user *buf,
{
struct ide_tape_obj *tape = file->private_data;
ide_drive_t *drive = tape->drive;
- ssize_t actually_written = 0;
+ size_t done = 0;
ssize_t ret = 0;
int rc;
@@ -1258,47 +1228,28 @@ static ssize_t idetape_chrdev_write(struct file *file, const char __user *buf,
rc = idetape_init_rw(drive, IDETAPE_DIR_WRITE);
if (rc < 0)
return rc;
- if (count == 0)
- return (0);
- if (tape->valid < tape->buffer_size) {
- actually_written = min_t(unsigned int,
- tape->buffer_size - tape->valid,
- count);
- if (copy_from_user(tape->cur, buf, actually_written))
- ret = -EFAULT;
- buf += actually_written;
- count -= actually_written;
- tape->cur += actually_written;
- tape->valid += actually_written;
-
- if (tape->valid == tape->buffer_size) {
- ssize_t retval;
- retval = idetape_add_chrdev_write_request(drive,
- tape->buffer_size);
- if (retval <= 0)
- return (retval);
- }
- }
- while (count >= tape->buffer_size) {
- ssize_t retval;
- if (copy_from_user(tape->cur, buf, tape->buffer_size))
- ret = -EFAULT;
- buf += tape->buffer_size;
- count -= tape->buffer_size;
- retval = idetape_add_chrdev_write_request(drive,
- tape->buffer_size);
- actually_written += tape->buffer_size;
- if (retval <= 0)
- return (retval);
- }
- if (count) {
- actually_written += count;
- if (copy_from_user(tape->cur, buf, count))
+
+ while (done < count) {
+ size_t todo;
+
+ /* flush if staging buffer is full */
+ if (tape->valid == tape->buffer_size &&
+ idetape_queue_rw_tail(drive, REQ_IDETAPE_WRITE,
+ tape->buffer_size) <= 0)
+ return rc;
+
+ /* copy in */
+ todo = min_t(size_t, count - done,
+ tape->buffer_size - tape->valid);
+ if (copy_from_user(tape->cur, buf + done, todo))
ret = -EFAULT;
- tape->cur += count;
- tape->valid += count;
+
+ tape->cur += todo;
+ tape->valid += todo;
+ done += todo;
}
- return ret ? ret : actually_written;
+
+ return ret ? ret : done;
}
static int idetape_write_filemark(ide_drive_t *drive)
--
1.6.0.2
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH 09/10] ide-atapi: kill unused fields and callbacks
2009-03-25 14:17 [RFC PATCHSET pata-2.6] ide: clean up ide-tape Tejun Heo
` (7 preceding siblings ...)
2009-03-25 14:17 ` [PATCH 08/10] ide-tape: simplify read/write functions Tejun Heo
@ 2009-03-25 14:17 ` Tejun Heo
2009-03-25 14:17 ` [PATCH 10/10] ide: drop rq->data handling from ide_map_sg() Tejun Heo
2009-03-31 7:48 ` [RFC PATCHSET pata-2.6] ide: clean up ide-tape Borislav Petkov
10 siblings, 0 replies; 19+ messages in thread
From: Tejun Heo @ 2009-03-25 14:17 UTC (permalink / raw)
To: bzolnier, linux-kernel, axboe, linux-ide; +Cc: Tejun Heo
Impact: remove fields code paths which are no longer necessary
Now that ide-tape uses standard mechanisms to transfer data, special
case handling for bh handling can be dropped from ide-atapi. Drop the
followings.
* pc->cur_pos, b_count, bh and b_data
* drive->pc_update_buffers() and pc_io_buffers().
Signed-off-by: Tejun Heo <tj@kernel.org>
---
drivers/ide/ide-atapi.c | 15 +++------------
drivers/ide/ide-tape.c | 1 -
include/linux/ide.h | 12 ------------
3 files changed, 3 insertions(+), 25 deletions(-)
diff --git a/drivers/ide/ide-atapi.c b/drivers/ide/ide-atapi.c
index 53e811d..bbb8da1 100644
--- a/drivers/ide/ide-atapi.c
+++ b/drivers/ide/ide-atapi.c
@@ -317,9 +317,6 @@ static ide_startstop_t ide_pc_intr(ide_drive_t *drive)
pc->flags |= PC_FLAG_DMA_ERROR;
} else {
pc->xferred = pc->req_xfer;
- if (drive->pc_update_buffers)
- drive->pc_update_buffers(drive, pc);
-
if (drive->media == ide_floppy)
ide_complete_rq(drive, 0, blk_rq_bytes(rq));
}
@@ -420,16 +417,11 @@ static ide_startstop_t ide_pc_intr(ide_drive_t *drive)
return ide_do_reset(drive);
}
- if (drive->media == ide_tape && pc->bh)
- done = drive->pc_io_buffers(drive, pc, bcount, write);
- else {
- done = min_t(unsigned int, bcount, cmd->nleft);
- ide_pio_bytes(drive, cmd, write, done);
- }
+ done = min_t(unsigned int, bcount, cmd->nleft);
+ ide_pio_bytes(drive, cmd, write, done);
- /* Update the current position */
+ /* Update transferred byte count */
pc->xferred += done;
- pc->cur_pos += done;
bcount -= done;
@@ -610,7 +602,6 @@ ide_startstop_t ide_issue_pc(ide_drive_t *drive, struct ide_cmd *cmd)
/* We haven't transferred any data yet */
pc->xferred = 0;
- pc->cur_pos = pc->buf;
tf_flags = IDE_TFLAG_OUT_DEVICE;
bcount = ((drive->media == ide_tape) ?
diff --git a/drivers/ide/ide-tape.c b/drivers/ide/ide-tape.c
index 95e9131..557f073 100644
--- a/drivers/ide/ide-tape.c
+++ b/drivers/ide/ide-tape.c
@@ -593,7 +593,6 @@ static void ide_tape_create_rw_cmd(idetape_tape_t *tape,
ide_init_pc(pc);
put_unaligned(cpu_to_be32(length), (unsigned int *) &pc->c[1]);
pc->c[1] = 1;
- pc->bh = NULL;
pc->buf = NULL;
pc->buf_size = length * tape->blk_size;
pc->req_xfer = pc->buf_size;
diff --git a/include/linux/ide.h b/include/linux/ide.h
index 8d3bb27..996adae 100644
--- a/include/linux/ide.h
+++ b/include/linux/ide.h
@@ -397,11 +397,7 @@ struct ide_atapi_pc {
/* data buffer */
u8 *buf;
- /* current buffer position */
- u8 *cur_pos;
int buf_size;
- /* missing/available data on the current buffer */
- int b_count;
/* the corresponding request */
struct request *rq;
@@ -414,10 +410,6 @@ struct ide_atapi_pc {
*/
u8 pc_buf[IDE_PC_BUFFER_SIZE];
- /* idetape only */
- struct idetape_bh *bh;
- char *b_data;
-
unsigned long timeout;
};
@@ -630,10 +622,6 @@ struct ide_drive_s {
/* callback for packet commands */
int (*pc_callback)(struct ide_drive_s *, int);
- void (*pc_update_buffers)(struct ide_drive_s *, struct ide_atapi_pc *);
- int (*pc_io_buffers)(struct ide_drive_s *, struct ide_atapi_pc *,
- unsigned int, int);
-
ide_startstop_t (*irq_handler)(struct ide_drive_s *);
unsigned long atapi_flags;
--
1.6.0.2
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH 10/10] ide: drop rq->data handling from ide_map_sg()
2009-03-25 14:17 [RFC PATCHSET pata-2.6] ide: clean up ide-tape Tejun Heo
` (8 preceding siblings ...)
2009-03-25 14:17 ` [PATCH 09/10] ide-atapi: kill unused fields and callbacks Tejun Heo
@ 2009-03-25 14:17 ` Tejun Heo
2009-03-31 7:48 ` [RFC PATCHSET pata-2.6] ide: clean up ide-tape Borislav Petkov
10 siblings, 0 replies; 19+ messages in thread
From: Tejun Heo @ 2009-03-25 14:17 UTC (permalink / raw)
To: bzolnier, linux-kernel, axboe, linux-ide; +Cc: Tejun Heo
Impact: remove code path which is no longer necessary
All IDE data transfers now use rq->bio. Simplify ide_map_sg()
accordingly.
Signed-off-by: Tejun Heo <tj@kernel.org>
Cc: Jens Axboe <axboe@kernel.dk>
---
drivers/ide/ide-io.c | 6 +-----
1 files changed, 1 insertions(+), 5 deletions(-)
diff --git a/drivers/ide/ide-io.c b/drivers/ide/ide-io.c
index 9ad9bc1..6746171 100644
--- a/drivers/ide/ide-io.c
+++ b/drivers/ide/ide-io.c
@@ -244,11 +244,7 @@ void ide_map_sg(ide_drive_t *drive, struct ide_cmd *cmd)
struct scatterlist *sg = hwif->sg_table;
struct request *rq = cmd->rq;
- if (!rq->bio) {
- sg_init_one(sg, rq->data, rq->data_len);
- cmd->sg_nents = 1;
- } else
- cmd->sg_nents = blk_rq_map_sg(drive->queue, rq, sg);
+ cmd->sg_nents = blk_rq_map_sg(drive->queue, rq, sg);
}
EXPORT_SYMBOL_GPL(ide_map_sg);
--
1.6.0.2
^ permalink raw reply related [flat|nested] 19+ messages in thread* Re: [RFC PATCHSET pata-2.6] ide: clean up ide-tape
2009-03-25 14:17 [RFC PATCHSET pata-2.6] ide: clean up ide-tape Tejun Heo
` (9 preceding siblings ...)
2009-03-25 14:17 ` [PATCH 10/10] ide: drop rq->data handling from ide_map_sg() Tejun Heo
@ 2009-03-31 7:48 ` Borislav Petkov
10 siblings, 0 replies; 19+ messages in thread
From: Borislav Petkov @ 2009-03-31 7:48 UTC (permalink / raw)
To: Tejun Heo; +Cc: bzolnier, linux-kernel, axboe, linux-ide
On Wed, Mar 25, 2009 at 11:17:43PM +0900, Tejun Heo wrote:
> Hi, Bartlomiej, Jens.
>
> This RFC patchset cleans up ide-tape. Note that the patchset
> currently is only compile tested, so it's likely to be broken, so the
> RFC status. Albert Lee is sending me some IDE tape drives, so I'll
> test these patches as soon as I receive them and repost with whatever
> necessary fixes.
By the way,
we may have another potential tester for ide-tape if we ask him nicely
enough :)
See http://bugzilla.kernel.org/show_bug.cgi?id=12874.
--
Regards/Gruss,
Boris.
^ permalink raw reply [flat|nested] 19+ messages in thread