From 1e75540ec5202cae63cd238c86bd880e3d496546 Mon Sep 17 00:00:00 2001 From: Tejun Heo Date: Sun, 19 Apr 2009 07:00:41 +0900 Subject: ide-tape: remove back-to-back REQUEST_SENSE detection Impact: fix an oops which always triggers ide_tape_issue_pc() assumed drive->pc isn't NULL on invocation when checking for back-to-back request sense issues but drive->pc can be NULL and even when it's not NULL, it's not safe to dereference it once the previous command is complete because pc could have been freed or was on stack. Kill back-to-back REQUEST_SENSE detection. Signed-off-by: Tejun Heo --- drivers/ide/ide-tape.c | 6 ------ 1 file changed, 6 deletions(-) (limited to 'drivers/ide/ide-tape.c') diff --git a/drivers/ide/ide-tape.c b/drivers/ide/ide-tape.c index cb942a9b580..3a53e0834cf 100644 --- a/drivers/ide/ide-tape.c +++ b/drivers/ide/ide-tape.c @@ -614,12 +614,6 @@ static ide_startstop_t ide_tape_issue_pc(ide_drive_t *drive, { idetape_tape_t *tape = drive->driver_data; - if (drive->pc->c[0] == REQUEST_SENSE && - pc->c[0] == REQUEST_SENSE) { - printk(KERN_ERR "ide-tape: possible ide-tape.c bug - " - "Two request sense in serial were issued\n"); - } - if (drive->failed_pc == NULL && pc->c[0] != REQUEST_SENSE) drive->failed_pc = pc; -- cgit v1.2.3 From c267cc1c4db4ccb3406d045a8da8660f0bbfe08d Mon Sep 17 00:00:00 2001 From: Tejun Heo Date: Sun, 19 Apr 2009 07:00:42 +0900 Subject: ide-atapi: don't abuse rq->buffer Impact: rq->buffer usage cleanup ide-atapi uses rq->buffer as private opaque value for internal special requests. rq->special isn't used for these cases (the only case where rq->special is used is for ide-tape rw requests). Use rq->special instead. Signed-off-by: Tejun Heo Cc: Jens Axboe --- drivers/ide/ide-tape.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/ide/ide-tape.c') diff --git a/drivers/ide/ide-tape.c b/drivers/ide/ide-tape.c index 3a53e0834cf..aadf53cfac6 100644 --- a/drivers/ide/ide-tape.c +++ b/drivers/ide/ide-tape.c @@ -828,7 +828,7 @@ static ide_startstop_t idetape_do_request(ide_drive_t *drive, goto out; } if (rq->cmd[13] & REQ_IDETAPE_PC1) { - pc = (struct ide_atapi_pc *) rq->buffer; + pc = (struct ide_atapi_pc *)rq->special; rq->cmd[13] &= ~(REQ_IDETAPE_PC1); rq->cmd[13] |= REQ_IDETAPE_PC2; goto out; -- cgit v1.2.3 From 6b544fcc8cd0a04eb42de9d1ecdd345e979d6ada Mon Sep 17 00:00:00 2001 From: Borislav Petkov Date: Sun, 19 Apr 2009 07:00:42 +0900 Subject: ide-atapi: convert ide-{floppy,tape} to using preallocated sense buffer Since we're issuing REQ_TYPE_SENSE now we need to allow those types of rqs in the ->do_request callbacks. As a future improvement, sense_len assignment might be unified across all ATAPI devices. Borislav to check with specs and test. As a result, get rid of ide_queue_pc_head() and drive->request_sense_rq. tj: * Init request sense ide_atapi_pc from sense request. In the longer timer, it would probably better to fold ide_create_request_sense_cmd() into its only current user - ide_floppy_get_format_progress(). * ide_retry_pc() no longer takes @disk. CC: Bartlomiej Zolnierkiewicz CC: FUJITA Tomonori Signed-off-by: Borislav Petkov Signed-off-by: Tejun Heo --- drivers/ide/ide-tape.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) (limited to 'drivers/ide/ide-tape.c') diff --git a/drivers/ide/ide-tape.c b/drivers/ide/ide-tape.c index aadf53cfac6..8324dfa78a3 100644 --- a/drivers/ide/ide-tape.c +++ b/drivers/ide/ide-tape.c @@ -695,7 +695,7 @@ static ide_startstop_t idetape_media_access_finished(ide_drive_t *drive) printk(KERN_ERR "ide-tape: %s: I/O error, ", tape->name); /* Retry operation */ - ide_retry_pc(drive, tape->disk); + ide_retry_pc(drive); return ide_stopped; } pc->error = 0; @@ -752,7 +752,7 @@ static ide_startstop_t idetape_do_request(ide_drive_t *drive, (unsigned long long)rq->sector, rq->nr_sectors, rq->current_nr_sectors); - if (!blk_special_request(rq)) { + if (!(blk_special_request(rq) || blk_sense_request(rq))) { /* We do not support buffer cache originated requests. */ printk(KERN_NOTICE "ide-tape: %s: Unsupported request in " "request queue (%d)\n", drive->name, rq->cmd_type); @@ -840,6 +840,9 @@ static ide_startstop_t idetape_do_request(ide_drive_t *drive, BUG(); out: + /* prepare sense request for this command */ + ide_prep_sense(drive, rq); + memset(&cmd, 0, sizeof(cmd)); if (rq_data_dir(rq)) -- cgit v1.2.3 From 5c4be57249e2e09136446597d2fe2a967c6ffef0 Mon Sep 17 00:00:00 2001 From: Tejun Heo Date: Sun, 19 Apr 2009 07:00:42 +0900 Subject: ide-cd,atapi: use bio for internal commands Impact: unify request data buffer handling rq->data is used mostly to pass kernel buffer through request queue without using bio. There are only a couple of places which still do this in kernel and converting to bio isn't difficult. This patch converts ide-cd and atapi to use bio instead of rq->data for request sense and internal pc commands. With previous change to unify sense request handling, this is relatively easily achieved by adding blk_rq_map_kern() during sense_rq prep and PC issue. If blk_rq_map_kern() fails for sense, the error is deferred till sense issue and aborts the failed command which triggered the sense. Note that this is a slim possibility as sense prep is done on each command issue, so for the above condition to actually trigger, all preps since the last sense issue till the issue of the request which would require a sense should fail. * do_request functions might sleep now. This should be okay as ide request_fn - do_ide_request() - is invoked only from make_request and plug work. Make sure this is the case by adding might_sleep() to do_ide_request(). * Functions which access the read sense data before the sense request is complete now should access bio_data(sense_rq->bio) as the sense buffer might have been copied during blk_rq_map_kern(). * ide-tape updated to map sg. * cdrom_do_block_pc() now doesn't have to deal with REQ_TYPE_ATA_PC special case. Simplified. * tp_ops->output/input_data path dropped from ide_pc_intr(). Signed-off-by: Tejun Heo --- drivers/ide/ide-tape.c | 3 +++ 1 file changed, 3 insertions(+) (limited to 'drivers/ide/ide-tape.c') diff --git a/drivers/ide/ide-tape.c b/drivers/ide/ide-tape.c index 8324dfa78a3..9b762a2d5d9 100644 --- a/drivers/ide/ide-tape.c +++ b/drivers/ide/ide-tape.c @@ -850,6 +850,9 @@ out: cmd.rq = rq; + ide_init_sg_cmd(&cmd, pc->req_xfer); + ide_map_sg(drive, &cmd); + return ide_tape_issue_pc(drive, &cmd, pc); } -- cgit v1.2.3 From ea7066afcd590e4663e6dc010f93704164050f48 Mon Sep 17 00:00:00 2001 From: Tejun Heo Date: Sun, 19 Apr 2009 08:46:02 +0900 Subject: ide-tape,floppy: fix failed command completion after request sense Impact: fix infinite retry loop After a command failed, ide-tape and floppy inserts REQUEST_SENSE in front of the failed command and according to the result, sets pc->retries, flags and errors. After REQUEST_SENSE is complete, the failed command is again at the front of the queue and if the verdict was to terminate the request, the issue functions tries to complete it directly by calling drive->pc_callback() and returning ide_stopped. However, drive->pc_callback() doesn't complete a request. It only prepares for completion of the request. As a result, this creates an infinite loop where the failed request is retried perpetually. Fix it by actually ending the request by calling ide_complete_rq(). Signed-off-by: Tejun Heo --- drivers/ide/ide-tape.c | 1 + 1 file changed, 1 insertion(+) (limited to 'drivers/ide/ide-tape.c') diff --git a/drivers/ide/ide-tape.c b/drivers/ide/ide-tape.c index 9b762a2d5d9..2b9a13671c5 100644 --- a/drivers/ide/ide-tape.c +++ b/drivers/ide/ide-tape.c @@ -643,6 +643,7 @@ static ide_startstop_t ide_tape_issue_pc(ide_drive_t *drive, } drive->failed_pc = NULL; drive->pc_callback(drive, 0); + ide_complete_rq(drive, -EIO, blk_rq_bytes(drive->hwif->rq)); return ide_stopped; } debug_log(DBG_SENSE, "Retry #%d, cmd = %02X\n", pc->retries, pc->c[0]); -- cgit v1.2.3 From b3071d190d6757b14af002a9d79832f12de61bce Mon Sep 17 00:00:00 2001 From: Tejun Heo Date: Sun, 19 Apr 2009 08:46:02 +0900 Subject: ide-atapi,tape,floppy: allow ->pc_callback() to change rq->data_len 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(), ide_tape_issue_pc() and ide_floppy_issue_pc() 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 Cc: Jens Axboe --- drivers/ide/ide-tape.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) (limited to 'drivers/ide/ide-tape.c') diff --git a/drivers/ide/ide-tape.c b/drivers/ide/ide-tape.c index 2b9a13671c5..8226d52504d 100644 --- a/drivers/ide/ide-tape.c +++ b/drivers/ide/ide-tape.c @@ -622,6 +622,8 @@ static ide_startstop_t ide_tape_issue_pc(ide_drive_t *drive, if (pc->retries > IDETAPE_MAX_PC_RETRIES || (pc->flags & PC_FLAG_ABORT)) { + unsigned int done = blk_rq_bytes(drive->hwif->rq); + /* * We will "abort" retrying a packet command in case legitimate * error code was received (crossing a filemark, or end of the @@ -641,9 +643,10 @@ static ide_startstop_t ide_tape_issue_pc(ide_drive_t *drive, /* Giving up */ pc->error = IDE_DRV_ERROR_GENERAL; } + drive->failed_pc = NULL; drive->pc_callback(drive, 0); - ide_complete_rq(drive, -EIO, blk_rq_bytes(drive->hwif->rq)); + ide_complete_rq(drive, -EIO, done); return ide_stopped; } debug_log(DBG_SENSE, "Retry #%d, cmd = %02X\n", pc->retries, pc->c[0]); -- cgit v1.2.3 From 35ab8d3251833e4052aa64b09b08195e949518c7 Mon Sep 17 00:00:00 2001 From: Tejun Heo Date: Sun, 19 Apr 2009 08:46:02 +0900 Subject: ide-tape: use single continuous buffer Impact: simpler buffer allocation and handling, kills OOM, 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 killing OOM triggered by allocation path and fixing DMA transfers. Previously, bug in alloc path triggered OOM on command issue and commands were passed to DMA engine without DMA-mapping all the segments. Signed-off-by: Tejun Heo --- drivers/ide/ide-tape.c | 256 +++++++++++-------------------------------------- 1 file changed, 58 insertions(+), 198 deletions(-) (limited to 'drivers/ide/ide-tape.c') diff --git a/drivers/ide/ide-tape.c b/drivers/ide/ide-tape.c index 8226d52504d..b2afbc7bcb6 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 *); @@ -861,117 +823,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; } @@ -979,30 +874,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; } @@ -1254,7 +1139,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) { @@ -1269,31 +1154,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; @@ -1321,7 +1191,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; @@ -1368,23 +1238,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); } @@ -1596,7 +1461,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; @@ -1970,7 +1835,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)); @@ -2201,11 +2066,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]); -- cgit v1.2.3 From 21d9c5d227593d15630ae83a336d1519653e9b8a Mon Sep 17 00:00:00 2001 From: Tejun Heo Date: Sun, 19 Apr 2009 08:46:02 +0900 Subject: ide-tape: use standard data transfer mechanism Impact: use standard way to transfer data 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 isn't difficult. Data buffer is mapped into bio using blk_rq_map_kern() in idetape_queue_rw_tail(). idetape_io_buffers() and idetape_update_buffers() are dropped and 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. 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). Signed-off-by: Tejun Heo Cc: Jens Axboe --- drivers/ide/ide-tape.c | 112 ++++++++++--------------------------------------- 1 file changed, 23 insertions(+), 89 deletions(-) (limited to 'drivers/ide/ide-tape.c') diff --git a/drivers/ide/ide-tape.c b/drivers/ide/ide-tape.c index b2afbc7bcb6..b373ac87635 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, @@ -458,7 +397,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; @@ -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 * @@ -683,7 +609,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; @@ -1063,10 +989,12 @@ 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; @@ -1074,21 +1002,29 @@ static int idetape_queue_rw_tail(ide_drive_t *drive, int cmd, int blocks, rq->rq_disk = tape->disk; rq->special = (void *)bh; rq->sector = tape->first_frame; - rq->nr_sectors = blocks; - rq->current_nr_sectors = blocks; + + if (size) { + ret = blk_rq_map_kern(drive->queue, rq, bh->b_data, size, + __GFP_WAIT); + if (ret) + goto out_put; + } + 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); + /* calculate the number of transferred bytes and update bh */ + size -= rq->data_len; + if (cmd == REQ_IDETAPE_READ) + atomic_add(size, &bh->b_count); - if ((cmd & (REQ_IDETAPE_READ | REQ_IDETAPE_WRITE)) == 0) - return 0; + 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; } @@ -2034,8 +1970,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; -- cgit v1.2.3 From 963da55c4b9eeeb2085ca74ba927cf77bce966d4 Mon Sep 17 00:00:00 2001 From: Tejun Heo Date: Sun, 19 Apr 2009 08:46:02 +0900 Subject: ide-tape: kill idetape_bh 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 --- drivers/ide/ide-tape.c | 305 ++++++++++++++----------------------------------- 1 file changed, 84 insertions(+), 221 deletions(-) (limited to 'drivers/ide/ide-tape.c') diff --git a/drivers/ide/ide-tape.c b/drivers/ide/ide-tape.c index b373ac87635..d2e9c09b521 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) @@ -603,8 +586,7 @@ 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; + unsigned int length = rq->nr_sectors; ide_init_pc(pc); put_unaligned(cpu_to_be32(length), (unsigned int *) &pc->c[1]); @@ -616,14 +598,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); @@ -639,10 +618,8 @@ static ide_startstop_t idetape_do_request(ide_drive_t *drive, struct ide_cmd cmd; u8 stat; - debug_log(DBG_SENSE, "sector: %llu, nr_sectors: %lu," - " current_nr_sectors: %u\n", - (unsigned long long)rq->sector, rq->nr_sectors, - rq->current_nr_sectors); + debug_log(DBG_SENSE, "sector: %llu, nr_sectors: %lu\n" + (unsigned long long)rq->sector, rq->nr_sectors); if (!(blk_special_request(rq) || blk_sense_request(rq))) { /* We do not support buffer cache originated requests. */ @@ -748,89 +725,6 @@ out: return ide_tape_issue_pc(drive, &cmd, pc); } -/* - * 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. @@ -928,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; @@ -985,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; @@ -1000,11 +893,10 @@ static int idetape_queue_rw_tail(ide_drive_t *drive, int cmd, int blocks, rq->cmd_type = REQ_TYPE_SPECIAL; rq->cmd[13] = cmd; rq->rq_disk = tape->disk; - rq->special = (void *)bh; 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; @@ -1012,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; @@ -1064,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; } @@ -1122,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 @@ -1140,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; } @@ -1157,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 */ @@ -1167,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); } } @@ -1267,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); @@ -1333,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; @@ -1357,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)) { @@ -1392,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 @@ -1411,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; } @@ -1423,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); @@ -1447,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; @@ -1458,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; } @@ -1623,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) @@ -1771,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); @@ -2042,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; -- cgit v1.2.3 From 88f1b941c5c94016a59144a3c94c9ca31eb16205 Mon Sep 17 00:00:00 2001 From: Tejun Heo Date: Sun, 19 Apr 2009 08:46:02 +0900 Subject: ide-tape: unify r/w init paths Impact: cleanup Read and write init paths are almost identical. Unify them into idetape_init_rw(). Signed-off-by: Tejun Heo --- drivers/ide/ide-tape.c | 110 +++++++++++++++++++++---------------------------- 1 file changed, 46 insertions(+), 64 deletions(-) (limited to 'drivers/ide/ide-tape.c') diff --git a/drivers/ide/ide-tape.c b/drivers/ide/ide-tape.c index d2e9c09b521..4da7fa9bca1 100644 --- a/drivers/ide/ide-tape.c +++ b/drivers/ide/ide-tape.c @@ -987,42 +987,50 @@ 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; + int rc; - /* 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; + BUG_ON(dir != IDETAPE_DIR_READ && dir != IDETAPE_DIR_WRITE); - /* - * 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 == dir) + return 0; + + 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 cmd = dir == IDETAPE_DIR_READ ? REQ_IDETAPE_READ + : REQ_IDETAPE_WRITE; + + rc = idetape_queue_rw_tail(drive, cmd, 0); + if (rc < 0) { + kfree(tape->buf); + tape->buf = NULL; + tape->chrdev_dir = IDETAPE_DIR_NONE; + return rc; } } @@ -1038,7 +1046,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 +1203,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 +1257,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 +1266,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) { -- cgit v1.2.3 From 6bb11dd14f70228f8dab25fd25dabeb9bc74926d Mon Sep 17 00:00:00 2001 From: Tejun Heo Date: Sun, 19 Apr 2009 08:46:03 +0900 Subject: ide-tape: use byte size instead of sectors on rw issue functions 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 --- drivers/ide/ide-tape.c | 45 ++++++++++++++++++++------------------------- 1 file changed, 20 insertions(+), 25 deletions(-) (limited to 'drivers/ide/ide-tape.c') diff --git a/drivers/ide/ide-tape.c b/drivers/ide/ide-tape.c index 4da7fa9bca1..d5e9bb286e3 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, 0, aligned - tape->valid); + idetape_add_chrdev_write_request(drive, aligned); kfree(tape->buf); tape->buf = NULL; } @@ -1038,9 +1032,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)) @@ -1048,7 +1042,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) @@ -1060,8 +1054,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; } } @@ -1193,7 +1186,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); @@ -1218,7 +1210,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)) @@ -1228,7 +1221,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); @@ -1256,7 +1250,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. */ @@ -1284,7 +1277,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); } @@ -1295,7 +1289,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); -- cgit v1.2.3 From 07bd9686c50c2b1f10e48089d4fb836a971f5177 Mon Sep 17 00:00:00 2001 From: Tejun Heo Date: Sun, 19 Apr 2009 08:46:03 +0900 Subject: ide-tape: simplify read/write functions 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 --- drivers/ide/ide-tape.c | 149 +++++++++++++++++-------------------------------- 1 file changed, 50 insertions(+), 99 deletions(-) (limited to 'drivers/ide/ide-tape.c') diff --git a/drivers/ide/ide-tape.c b/drivers/ide/ide-tape.c index d5e9bb286e3..2599579e417 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, 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; } @@ -1031,20 +1023,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; @@ -1184,8 +1162,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); @@ -1195,52 +1174,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, @@ -1248,7 +1218,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; @@ -1262,47 +1232,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) -- cgit v1.2.3 From 6d7003877c2f0578f1c08f66d05c3f72ef4ae596 Mon Sep 17 00:00:00 2001 From: Tejun Heo Date: Sun, 19 Apr 2009 08:46:03 +0900 Subject: ide-atapi: kill unused fields and callbacks Impact: remove fields and 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 --- drivers/ide/ide-tape.c | 1 - 1 file changed, 1 deletion(-) (limited to 'drivers/ide/ide-tape.c') diff --git a/drivers/ide/ide-tape.c b/drivers/ide/ide-tape.c index 2599579e417..8dfc68892d6 100644 --- a/drivers/ide/ide-tape.c +++ b/drivers/ide/ide-tape.c @@ -591,7 +591,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; -- cgit v1.2.3 From 0de57fb93b1daaeaecb658a98b3299ae460c02e9 Mon Sep 17 00:00:00 2001 From: Tejun Heo Date: Sun, 19 Apr 2009 07:00:41 +0900 Subject: ide-tape: remove back-to-back REQUEST_SENSE detection Impact: fix an oops which always triggers ide_tape_issue_pc() assumed drive->pc isn't NULL on invocation when checking for back-to-back request sense issues but drive->pc can be NULL and even when it's not NULL, it's not safe to dereference it once the previous command is complete because pc could have been freed or was on stack. Kill back-to-back REQUEST_SENSE detection. Signed-off-by: Tejun Heo --- drivers/ide/ide-tape.c | 6 ------ 1 file changed, 6 deletions(-) (limited to 'drivers/ide/ide-tape.c') diff --git a/drivers/ide/ide-tape.c b/drivers/ide/ide-tape.c index cb942a9b580..3a53e0834cf 100644 --- a/drivers/ide/ide-tape.c +++ b/drivers/ide/ide-tape.c @@ -614,12 +614,6 @@ static ide_startstop_t ide_tape_issue_pc(ide_drive_t *drive, { idetape_tape_t *tape = drive->driver_data; - if (drive->pc->c[0] == REQUEST_SENSE && - pc->c[0] == REQUEST_SENSE) { - printk(KERN_ERR "ide-tape: possible ide-tape.c bug - " - "Two request sense in serial were issued\n"); - } - if (drive->failed_pc == NULL && pc->c[0] != REQUEST_SENSE) drive->failed_pc = pc; -- cgit v1.2.3 From ac0b0113ddbab3ed2388132d368c97292f9f3c84 Mon Sep 17 00:00:00 2001 From: Tejun Heo Date: Sun, 19 Apr 2009 07:00:42 +0900 Subject: ide-atapi: don't abuse rq->buffer Impact: rq->buffer usage cleanup ide-atapi uses rq->buffer as private opaque value for internal special requests. rq->special isn't used for these cases (the only case where rq->special is used is for ide-tape rw requests). Use rq->special instead. Signed-off-by: Tejun Heo Cc: Jens Axboe --- drivers/ide/ide-tape.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/ide/ide-tape.c') diff --git a/drivers/ide/ide-tape.c b/drivers/ide/ide-tape.c index 3a53e0834cf..aadf53cfac6 100644 --- a/drivers/ide/ide-tape.c +++ b/drivers/ide/ide-tape.c @@ -828,7 +828,7 @@ static ide_startstop_t idetape_do_request(ide_drive_t *drive, goto out; } if (rq->cmd[13] & REQ_IDETAPE_PC1) { - pc = (struct ide_atapi_pc *) rq->buffer; + pc = (struct ide_atapi_pc *)rq->special; rq->cmd[13] &= ~(REQ_IDETAPE_PC1); rq->cmd[13] |= REQ_IDETAPE_PC2; goto out; -- cgit v1.2.3 From 068753203e6cd085664a62e0fc0636e19b148a12 Mon Sep 17 00:00:00 2001 From: Borislav Petkov Date: Sun, 19 Apr 2009 07:00:42 +0900 Subject: ide-atapi: convert ide-{floppy,tape} to using preallocated sense buffer Since we're issuing REQ_TYPE_SENSE now we need to allow those types of rqs in the ->do_request callbacks. As a future improvement, sense_len assignment might be unified across all ATAPI devices. Borislav to check with specs and test. As a result, get rid of ide_queue_pc_head() and drive->request_sense_rq. tj: * Init request sense ide_atapi_pc from sense request. In the longer timer, it would probably better to fold ide_create_request_sense_cmd() into its only current user - ide_floppy_get_format_progress(). * ide_retry_pc() no longer takes @disk. CC: Bartlomiej Zolnierkiewicz CC: FUJITA Tomonori Signed-off-by: Borislav Petkov Signed-off-by: Tejun Heo --- drivers/ide/ide-tape.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) (limited to 'drivers/ide/ide-tape.c') diff --git a/drivers/ide/ide-tape.c b/drivers/ide/ide-tape.c index aadf53cfac6..8324dfa78a3 100644 --- a/drivers/ide/ide-tape.c +++ b/drivers/ide/ide-tape.c @@ -695,7 +695,7 @@ static ide_startstop_t idetape_media_access_finished(ide_drive_t *drive) printk(KERN_ERR "ide-tape: %s: I/O error, ", tape->name); /* Retry operation */ - ide_retry_pc(drive, tape->disk); + ide_retry_pc(drive); return ide_stopped; } pc->error = 0; @@ -752,7 +752,7 @@ static ide_startstop_t idetape_do_request(ide_drive_t *drive, (unsigned long long)rq->sector, rq->nr_sectors, rq->current_nr_sectors); - if (!blk_special_request(rq)) { + if (!(blk_special_request(rq) || blk_sense_request(rq))) { /* We do not support buffer cache originated requests. */ printk(KERN_NOTICE "ide-tape: %s: Unsupported request in " "request queue (%d)\n", drive->name, rq->cmd_type); @@ -840,6 +840,9 @@ static ide_startstop_t idetape_do_request(ide_drive_t *drive, BUG(); out: + /* prepare sense request for this command */ + ide_prep_sense(drive, rq); + memset(&cmd, 0, sizeof(cmd)); if (rq_data_dir(rq)) -- cgit v1.2.3 From 02e7cf8f848841ca21864ccd019e480b73c323b7 Mon Sep 17 00:00:00 2001 From: Tejun Heo Date: Sun, 19 Apr 2009 07:00:42 +0900 Subject: ide-cd,atapi: use bio for internal commands Impact: unify request data buffer handling rq->data is used mostly to pass kernel buffer through request queue without using bio. There are only a couple of places which still do this in kernel and converting to bio isn't difficult. This patch converts ide-cd and atapi to use bio instead of rq->data for request sense and internal pc commands. With previous change to unify sense request handling, this is relatively easily achieved by adding blk_rq_map_kern() during sense_rq prep and PC issue. If blk_rq_map_kern() fails for sense, the error is deferred till sense issue and aborts the failed command which triggered the sense. Note that this is a slim possibility as sense prep is done on each command issue, so for the above condition to actually trigger, all preps since the last sense issue till the issue of the request which would require a sense should fail. * do_request functions might sleep now. This should be okay as ide request_fn - do_ide_request() - is invoked only from make_request and plug work. Make sure this is the case by adding might_sleep() to do_ide_request(). * Functions which access the read sense data before the sense request is complete now should access bio_data(sense_rq->bio) as the sense buffer might have been copied during blk_rq_map_kern(). * ide-tape updated to map sg. * cdrom_do_block_pc() now doesn't have to deal with REQ_TYPE_ATA_PC special case. Simplified. * tp_ops->output/input_data path dropped from ide_pc_intr(). Signed-off-by: Tejun Heo --- drivers/ide/ide-tape.c | 3 +++ 1 file changed, 3 insertions(+) (limited to 'drivers/ide/ide-tape.c') diff --git a/drivers/ide/ide-tape.c b/drivers/ide/ide-tape.c index 8324dfa78a3..9b762a2d5d9 100644 --- a/drivers/ide/ide-tape.c +++ b/drivers/ide/ide-tape.c @@ -850,6 +850,9 @@ out: cmd.rq = rq; + ide_init_sg_cmd(&cmd, pc->req_xfer); + ide_map_sg(drive, &cmd); + return ide_tape_issue_pc(drive, &cmd, pc); } -- cgit v1.2.3 From 08f370f0a2fb223bf48d0bfa2a173be0393c19dc Mon Sep 17 00:00:00 2001 From: Tejun Heo Date: Sun, 19 Apr 2009 08:46:02 +0900 Subject: ide-tape,floppy: fix failed command completion after request sense Impact: fix infinite retry loop After a command failed, ide-tape and floppy inserts REQUEST_SENSE in front of the failed command and according to the result, sets pc->retries, flags and errors. After REQUEST_SENSE is complete, the failed command is again at the front of the queue and if the verdict was to terminate the request, the issue functions tries to complete it directly by calling drive->pc_callback() and returning ide_stopped. However, drive->pc_callback() doesn't complete a request. It only prepares for completion of the request. As a result, this creates an infinite loop where the failed request is retried perpetually. Fix it by actually ending the request by calling ide_complete_rq(). Signed-off-by: Tejun Heo --- drivers/ide/ide-tape.c | 1 + 1 file changed, 1 insertion(+) (limited to 'drivers/ide/ide-tape.c') diff --git a/drivers/ide/ide-tape.c b/drivers/ide/ide-tape.c index 9b762a2d5d9..2b9a13671c5 100644 --- a/drivers/ide/ide-tape.c +++ b/drivers/ide/ide-tape.c @@ -643,6 +643,7 @@ static ide_startstop_t ide_tape_issue_pc(ide_drive_t *drive, } drive->failed_pc = NULL; drive->pc_callback(drive, 0); + ide_complete_rq(drive, -EIO, blk_rq_bytes(drive->hwif->rq)); return ide_stopped; } debug_log(DBG_SENSE, "Retry #%d, cmd = %02X\n", pc->retries, pc->c[0]); -- cgit v1.2.3 From eb6a61bb9543aa54d62595e27206b3d3c0293bcc Mon Sep 17 00:00:00 2001 From: Tejun Heo Date: Sun, 19 Apr 2009 08:46:02 +0900 Subject: ide-atapi,tape,floppy: allow ->pc_callback() to change rq->data_len 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(), ide_tape_issue_pc() and ide_floppy_issue_pc() 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 Cc: Jens Axboe --- drivers/ide/ide-tape.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) (limited to 'drivers/ide/ide-tape.c') diff --git a/drivers/ide/ide-tape.c b/drivers/ide/ide-tape.c index 2b9a13671c5..8226d52504d 100644 --- a/drivers/ide/ide-tape.c +++ b/drivers/ide/ide-tape.c @@ -622,6 +622,8 @@ static ide_startstop_t ide_tape_issue_pc(ide_drive_t *drive, if (pc->retries > IDETAPE_MAX_PC_RETRIES || (pc->flags & PC_FLAG_ABORT)) { + unsigned int done = blk_rq_bytes(drive->hwif->rq); + /* * We will "abort" retrying a packet command in case legitimate * error code was received (crossing a filemark, or end of the @@ -641,9 +643,10 @@ static ide_startstop_t ide_tape_issue_pc(ide_drive_t *drive, /* Giving up */ pc->error = IDE_DRV_ERROR_GENERAL; } + drive->failed_pc = NULL; drive->pc_callback(drive, 0); - ide_complete_rq(drive, -EIO, blk_rq_bytes(drive->hwif->rq)); + ide_complete_rq(drive, -EIO, done); return ide_stopped; } debug_log(DBG_SENSE, "Retry #%d, cmd = %02X\n", pc->retries, pc->c[0]); -- cgit v1.2.3 From 7b13354eeaabaf6283b8c669a7d67d104ce7c638 Mon Sep 17 00:00:00 2001 From: Tejun Heo Date: Sun, 19 Apr 2009 08:46:02 +0900 Subject: ide-tape: use single continuous buffer Impact: simpler buffer allocation and handling, kills OOM, 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 killing OOM triggered by allocation path and fixing DMA transfers. Previously, bug in alloc path triggered OOM on command issue and commands were passed to DMA engine without DMA-mapping all the segments. Signed-off-by: Tejun Heo --- drivers/ide/ide-tape.c | 256 +++++++++++-------------------------------------- 1 file changed, 58 insertions(+), 198 deletions(-) (limited to 'drivers/ide/ide-tape.c') diff --git a/drivers/ide/ide-tape.c b/drivers/ide/ide-tape.c index 8226d52504d..b2afbc7bcb6 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 *); @@ -861,117 +823,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; } @@ -979,30 +874,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; } @@ -1254,7 +1139,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) { @@ -1269,31 +1154,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; @@ -1321,7 +1191,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; @@ -1368,23 +1238,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); } @@ -1596,7 +1461,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; @@ -1970,7 +1835,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)); @@ -2201,11 +2066,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]); -- cgit v1.2.3 From e998f30b45efb99a3c3ce7b5483f76317a17abed Mon Sep 17 00:00:00 2001 From: Tejun Heo Date: Sun, 19 Apr 2009 08:46:02 +0900 Subject: ide-tape: use standard data transfer mechanism Impact: use standard way to transfer data 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 isn't difficult. Data buffer is mapped into bio using blk_rq_map_kern() in idetape_queue_rw_tail(). idetape_io_buffers() and idetape_update_buffers() are dropped and 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. 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). Signed-off-by: Tejun Heo Cc: Jens Axboe --- drivers/ide/ide-tape.c | 112 ++++++++++--------------------------------------- 1 file changed, 23 insertions(+), 89 deletions(-) (limited to 'drivers/ide/ide-tape.c') diff --git a/drivers/ide/ide-tape.c b/drivers/ide/ide-tape.c index b2afbc7bcb6..b373ac87635 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, @@ -458,7 +397,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; @@ -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 * @@ -683,7 +609,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; @@ -1063,10 +989,12 @@ 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; @@ -1074,21 +1002,29 @@ static int idetape_queue_rw_tail(ide_drive_t *drive, int cmd, int blocks, rq->rq_disk = tape->disk; rq->special = (void *)bh; rq->sector = tape->first_frame; - rq->nr_sectors = blocks; - rq->current_nr_sectors = blocks; + + if (size) { + ret = blk_rq_map_kern(drive->queue, rq, bh->b_data, size, + __GFP_WAIT); + if (ret) + goto out_put; + } + 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); + /* calculate the number of transferred bytes and update bh */ + size -= rq->data_len; + if (cmd == REQ_IDETAPE_READ) + atomic_add(size, &bh->b_count); - if ((cmd & (REQ_IDETAPE_READ | REQ_IDETAPE_WRITE)) == 0) - return 0; + 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; } @@ -2034,8 +1970,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; -- cgit v1.2.3 From 6cf3d545f7d71b183e5b89960d4cc850a42c410d Mon Sep 17 00:00:00 2001 From: Tejun Heo Date: Sun, 19 Apr 2009 08:46:02 +0900 Subject: ide-tape: kill idetape_bh 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 --- drivers/ide/ide-tape.c | 305 ++++++++++++++----------------------------------- 1 file changed, 84 insertions(+), 221 deletions(-) (limited to 'drivers/ide/ide-tape.c') diff --git a/drivers/ide/ide-tape.c b/drivers/ide/ide-tape.c index b373ac87635..d2e9c09b521 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) @@ -603,8 +586,7 @@ 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; + unsigned int length = rq->nr_sectors; ide_init_pc(pc); put_unaligned(cpu_to_be32(length), (unsigned int *) &pc->c[1]); @@ -616,14 +598,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); @@ -639,10 +618,8 @@ static ide_startstop_t idetape_do_request(ide_drive_t *drive, struct ide_cmd cmd; u8 stat; - debug_log(DBG_SENSE, "sector: %llu, nr_sectors: %lu," - " current_nr_sectors: %u\n", - (unsigned long long)rq->sector, rq->nr_sectors, - rq->current_nr_sectors); + debug_log(DBG_SENSE, "sector: %llu, nr_sectors: %lu\n" + (unsigned long long)rq->sector, rq->nr_sectors); if (!(blk_special_request(rq) || blk_sense_request(rq))) { /* We do not support buffer cache originated requests. */ @@ -748,89 +725,6 @@ out: return ide_tape_issue_pc(drive, &cmd, pc); } -/* - * 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. @@ -928,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; @@ -985,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; @@ -1000,11 +893,10 @@ static int idetape_queue_rw_tail(ide_drive_t *drive, int cmd, int blocks, rq->cmd_type = REQ_TYPE_SPECIAL; rq->cmd[13] = cmd; rq->rq_disk = tape->disk; - rq->special = (void *)bh; 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; @@ -1012,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; @@ -1064,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; } @@ -1122,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 @@ -1140,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; } @@ -1157,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 */ @@ -1167,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); } } @@ -1267,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); @@ -1333,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; @@ -1357,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)) { @@ -1392,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 @@ -1411,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; } @@ -1423,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); @@ -1447,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; @@ -1458,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; } @@ -1623,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) @@ -1771,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); @@ -2042,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; -- cgit v1.2.3 From 3596b66452491a3cff26256a5e6e6061a66c4142 Mon Sep 17 00:00:00 2001 From: Tejun Heo Date: Sun, 19 Apr 2009 08:46:02 +0900 Subject: ide-tape: unify r/w init paths Impact: cleanup Read and write init paths are almost identical. Unify them into idetape_init_rw(). Signed-off-by: Tejun Heo --- drivers/ide/ide-tape.c | 110 +++++++++++++++++++++---------------------------- 1 file changed, 46 insertions(+), 64 deletions(-) (limited to 'drivers/ide/ide-tape.c') diff --git a/drivers/ide/ide-tape.c b/drivers/ide/ide-tape.c index d2e9c09b521..4da7fa9bca1 100644 --- a/drivers/ide/ide-tape.c +++ b/drivers/ide/ide-tape.c @@ -987,42 +987,50 @@ 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; + int rc; - /* 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; + BUG_ON(dir != IDETAPE_DIR_READ && dir != IDETAPE_DIR_WRITE); - /* - * 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 == dir) + return 0; + + 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 cmd = dir == IDETAPE_DIR_READ ? REQ_IDETAPE_READ + : REQ_IDETAPE_WRITE; + + rc = idetape_queue_rw_tail(drive, cmd, 0); + if (rc < 0) { + kfree(tape->buf); + tape->buf = NULL; + tape->chrdev_dir = IDETAPE_DIR_NONE; + return rc; } } @@ -1038,7 +1046,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 +1203,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 +1257,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 +1266,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) { -- cgit v1.2.3 From 71294cf93d22ceaa75448cc6ebee2c65897be8c2 Mon Sep 17 00:00:00 2001 From: Tejun Heo Date: Sun, 19 Apr 2009 08:46:03 +0900 Subject: ide-tape: use byte size instead of sectors on rw issue functions 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 --- drivers/ide/ide-tape.c | 45 ++++++++++++++++++++------------------------- 1 file changed, 20 insertions(+), 25 deletions(-) (limited to 'drivers/ide/ide-tape.c') diff --git a/drivers/ide/ide-tape.c b/drivers/ide/ide-tape.c index 4da7fa9bca1..d5e9bb286e3 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, 0, aligned - tape->valid); + idetape_add_chrdev_write_request(drive, aligned); kfree(tape->buf); tape->buf = NULL; } @@ -1038,9 +1032,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)) @@ -1048,7 +1042,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) @@ -1060,8 +1054,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; } } @@ -1193,7 +1186,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); @@ -1218,7 +1210,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)) @@ -1228,7 +1221,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); @@ -1256,7 +1250,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. */ @@ -1284,7 +1277,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); } @@ -1295,7 +1289,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); -- cgit v1.2.3 From 4344d07fb8dbf0cbfec1f7d7c1afeccaceaaa120 Mon Sep 17 00:00:00 2001 From: Tejun Heo Date: Sun, 19 Apr 2009 08:46:03 +0900 Subject: ide-tape: simplify read/write functions 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 --- drivers/ide/ide-tape.c | 149 +++++++++++++++++-------------------------------- 1 file changed, 50 insertions(+), 99 deletions(-) (limited to 'drivers/ide/ide-tape.c') diff --git a/drivers/ide/ide-tape.c b/drivers/ide/ide-tape.c index d5e9bb286e3..2599579e417 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, 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; } @@ -1031,20 +1023,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; @@ -1184,8 +1162,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); @@ -1195,52 +1174,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, @@ -1248,7 +1218,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; @@ -1262,47 +1232,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) -- cgit v1.2.3 From 29d1a4371035e01b0d079bc5aa88b50f5af7a566 Mon Sep 17 00:00:00 2001 From: Tejun Heo Date: Sun, 19 Apr 2009 08:46:03 +0900 Subject: ide-atapi: kill unused fields and callbacks Impact: remove fields and 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 --- drivers/ide/ide-tape.c | 1 - 1 file changed, 1 deletion(-) (limited to 'drivers/ide/ide-tape.c') diff --git a/drivers/ide/ide-tape.c b/drivers/ide/ide-tape.c index 2599579e417..8dfc68892d6 100644 --- a/drivers/ide/ide-tape.c +++ b/drivers/ide/ide-tape.c @@ -591,7 +591,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; -- cgit v1.2.3 From 9720aef2539c10e3a872e9a92beec225030d99db Mon Sep 17 00:00:00 2001 From: Tejun Heo Date: Thu, 7 May 2009 22:24:36 +0900 Subject: ide-tape: don't initialize rq->sector for rw requests rq->sector is set to the tape->first_frame but it's never actually used and not even in the correct unit (512 byte sectors). Don't set it. [ Impact: cleanup ] Signed-off-by: Tejun Heo Acked-by: Borislav Petkov Acked-by: Bartlomiej Zolnierkiewicz Signed-off-by: Jens Axboe --- drivers/ide/ide-tape.c | 1 - 1 file changed, 1 deletion(-) (limited to 'drivers/ide/ide-tape.c') diff --git a/drivers/ide/ide-tape.c b/drivers/ide/ide-tape.c index 8dfc68892d6..7149224d1fe 100644 --- a/drivers/ide/ide-tape.c +++ b/drivers/ide/ide-tape.c @@ -892,7 +892,6 @@ static int idetape_queue_rw_tail(ide_drive_t *drive, int cmd, int size) rq->cmd_type = REQ_TYPE_SPECIAL; rq->cmd[13] = cmd; rq->rq_disk = tape->disk; - rq->sector = tape->first_frame; if (size) { ret = blk_rq_map_kern(drive->queue, rq, tape->buf, size, -- cgit v1.2.3 From c3a4d78c580de4edc9ef0f7c59812fb02ceb037f Mon Sep 17 00:00:00 2001 From: Tejun Heo Date: Thu, 7 May 2009 22:24:37 +0900 Subject: block: add rq->resid_len rq->data_len served two purposes - the length of data buffer on issue and the residual count on completion. This duality creates some headaches. First of all, block layer and low level drivers can't really determine what rq->data_len contains while a request is executing. It could be the total request length or it coulde be anything else one of the lower layers is using to keep track of residual count. This complicates things because blk_rq_bytes() and thus [__]blk_end_request_all() relies on rq->data_len for PC commands. Drivers which want to report residual count should first cache the total request length, update rq->data_len and then complete the request with the cached data length. Secondly, it makes requests default to reporting full residual count, ie. reporting that no data transfer occurred. The residual count is an exception not the norm; however, the driver should clear rq->data_len to zero to signify the normal cases while leaving it alone means no data transfer occurred at all. This reverse default behavior complicates code unnecessarily and renders block PC on some drivers (ide-tape/floppy) unuseable. This patch adds rq->resid_len which is used only for residual count. While at it, remove now unnecessasry blk_rq_bytes() caching in ide_pc_intr() as rq->data_len is not changed anymore. Boaz : spotted missing conversion in osd Sergei : spotted too early conversion to blk_rq_bytes() in ide-tape [ Impact: cleanup residual count handling, report 0 resid by default ] Signed-off-by: Tejun Heo Cc: James Bottomley Cc: Bartlomiej Zolnierkiewicz Cc: Borislav Petkov Cc: Sergei Shtylyov Cc: Mike Miller Cc: Eric Moore Cc: Alan Stern Cc: FUJITA Tomonori Cc: Doug Gilbert Cc: Mike Miller Cc: Eric Moore Cc: Darrick J. Wong Cc: Pete Zaitcev Cc: Boaz Harrosh Signed-off-by: Jens Axboe --- drivers/ide/ide-tape.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'drivers/ide/ide-tape.c') diff --git a/drivers/ide/ide-tape.c b/drivers/ide/ide-tape.c index 7149224d1fe..65c5b705883 100644 --- a/drivers/ide/ide-tape.c +++ b/drivers/ide/ide-tape.c @@ -380,7 +380,7 @@ static int ide_tape_callback(ide_drive_t *drive, int dsc) } tape->first_frame += blocks; - rq->data_len -= blocks * tape->blk_size; + rq->resid_len = rq->data_len - blocks * tape->blk_size; if (pc->error) { uptodate = 0; @@ -903,7 +903,7 @@ static int idetape_queue_rw_tail(ide_drive_t *drive, int cmd, int size) blk_execute_rq(drive->queue, tape->disk, rq, 0); /* calculate the number of transferred bytes and update buffer state */ - size -= rq->data_len; + size -= rq->resid_len; tape->cur = tape->buf; if (cmd == REQ_IDETAPE_READ) tape->valid = size; -- cgit v1.2.3 From 9780e2dd8254351f6cbe11304849126b51dbd561 Mon Sep 17 00:00:00 2001 From: Tejun Heo Date: Thu, 7 May 2009 22:24:40 +0900 Subject: ide: convert to rq pos and nr_sectors accessors ide doesn't manipulate request fields anymore and thus all hard and their soft equivalents are always equal. Convert all references to accessors. [ Impact: use pos and nr_sectors accessors ] Signed-off-by: Tejun Heo Acked-by: Bartlomiej Zolnierkiewicz Cc: Borislav Petkov Cc: Sergei Shtylyov Signed-off-by: Jens Axboe --- drivers/ide/ide-tape.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'drivers/ide/ide-tape.c') diff --git a/drivers/ide/ide-tape.c b/drivers/ide/ide-tape.c index 65c5b705883..c6cf3a951f2 100644 --- a/drivers/ide/ide-tape.c +++ b/drivers/ide/ide-tape.c @@ -586,7 +586,7 @@ static void ide_tape_create_rw_cmd(idetape_tape_t *tape, struct ide_atapi_pc *pc, struct request *rq, u8 opcode) { - unsigned int length = rq->nr_sectors; + unsigned int length = blk_rq_sectors(rq); ide_init_pc(pc); put_unaligned(cpu_to_be32(length), (unsigned int *) &pc->c[1]); @@ -617,8 +617,8 @@ static ide_startstop_t idetape_do_request(ide_drive_t *drive, struct ide_cmd cmd; u8 stat; - debug_log(DBG_SENSE, "sector: %llu, nr_sectors: %lu\n" - (unsigned long long)rq->sector, rq->nr_sectors); + debug_log(DBG_SENSE, "sector: %llu, nr_sectors: %u\n" + (unsigned long long)blk_rq_pos(rq), blk_rq_sectors(rq)); if (!(blk_special_request(rq) || blk_sense_request(rq))) { /* We do not support buffer cache originated requests. */ -- cgit v1.2.3 From 34b7d2c957199834c474c9d46739265643f4d9c7 Mon Sep 17 00:00:00 2001 From: Tejun Heo Date: Thu, 7 May 2009 22:24:43 +0900 Subject: ide: cleanup rq->data_len usages With recent unification of fields, it's now guaranteed that rq->data_len always equals blk_rq_bytes(). Convert all direct users to accessors. [ Impact: convert direct rq->data_len usages to blk_rq_bytes() ] Signed-off-by: Tejun Heo Acked-by: Bartlomiej Zolnierkiewicz Cc: Borislav Petkov Cc: Sergei Shtylyov Signed-off-by: Jens Axboe --- drivers/ide/ide-tape.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/ide/ide-tape.c') diff --git a/drivers/ide/ide-tape.c b/drivers/ide/ide-tape.c index c6cf3a951f2..e16604562f2 100644 --- a/drivers/ide/ide-tape.c +++ b/drivers/ide/ide-tape.c @@ -380,7 +380,7 @@ static int ide_tape_callback(ide_drive_t *drive, int dsc) } tape->first_frame += blocks; - rq->resid_len = rq->data_len - blocks * tape->blk_size; + rq->resid_len = blk_rq_bytes(rq) - blocks * tape->blk_size; if (pc->error) { uptodate = 0; -- cgit v1.2.3 From e8e7526c3c0863be25ab03a0871ee0978de5ba50 Mon Sep 17 00:00:00 2001 From: Mark de Wever Date: Sun, 17 May 2009 17:22:53 +0200 Subject: ide-tape: fix debug call This error only occurs when IDETAPE_DEBUG_LOG is enabled. Signed-off-by: Mark de Wever Signed-off-by: Borislav Petkov Signed-off-by: Bartlomiej Zolnierkiewicz --- drivers/ide/ide-tape.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/ide/ide-tape.c') diff --git a/drivers/ide/ide-tape.c b/drivers/ide/ide-tape.c index 8dfc68892d6..203bbeac182 100644 --- a/drivers/ide/ide-tape.c +++ b/drivers/ide/ide-tape.c @@ -617,7 +617,7 @@ static ide_startstop_t idetape_do_request(ide_drive_t *drive, struct ide_cmd cmd; u8 stat; - debug_log(DBG_SENSE, "sector: %llu, nr_sectors: %lu\n" + debug_log(DBG_SENSE, "sector: %llu, nr_sectors: %lu\n", (unsigned long long)rq->sector, rq->nr_sectors); if (!(blk_special_request(rq) || blk_sense_request(rq))) { -- cgit v1.2.3 From 5f49f63178360b07a095bd33b0d850d60edf7590 Mon Sep 17 00:00:00 2001 From: Tejun Heo Date: Tue, 19 May 2009 18:33:05 +0900 Subject: block: set rq->resid_len to blk_rq_bytes() on issue In commit c3a4d78c580de4edc9ef0f7c59812fb02ceb037f, while introducing rq->resid_len, the default value of residue count was changed from full count to zero. The conversion was done under the assumption that when a request fails residue count wasn't defined. However, Boaz and James pointed out that this wasn't true and the residue count should be preserved for failed requests too. This patchset restores the original behavior by setting rq->resid_len to blk_rq_bytes(rq) on request start and restoring explicit clearing in affected drivers. While at it, take advantage of the fact that rq->resid_len is set to full count where applicable. * ide-cd: rq->resid_len cleared on pc success * mptsas: req->resid_len cleared on success * sas_expander: rsp/req->resid_len cleared on success * mpt2sas_transport: req->resid_len cleared on success * ide-cd, ide-tape, mptsas, sas_host_smp, mpt2sas_transport, ub: take advantage of initial full count to simplify code Boaz Harrosh spotted bug in resid_len initialization. Fixed as suggested. Signed-off-by: Tejun Heo Acked-by: Borislav Petkov Cc: Boaz Harrosh Cc: James Bottomley Cc: Pete Zaitcev Cc: Bartlomiej Zolnierkiewicz Cc: Sergei Shtylyov Cc: Eric Moore Cc: Darrick J. Wong Signed-off-by: Jens Axboe --- drivers/ide/ide-tape.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/ide/ide-tape.c') diff --git a/drivers/ide/ide-tape.c b/drivers/ide/ide-tape.c index e16604562f2..683ff37d407 100644 --- a/drivers/ide/ide-tape.c +++ b/drivers/ide/ide-tape.c @@ -380,7 +380,7 @@ static int ide_tape_callback(ide_drive_t *drive, int dsc) } tape->first_frame += blocks; - rq->resid_len = blk_rq_bytes(rq) - blocks * tape->blk_size; + rq->resid_len -= blocks * tape->blk_size; if (pc->error) { uptodate = 0; -- cgit v1.2.3 From 626542ca2277961aaa64855206574f8ca4f360e3 Mon Sep 17 00:00:00 2001 From: Borislav Petkov Date: Sun, 7 Jun 2009 15:37:05 +0200 Subject: ide-tape: change IDE_AFLAG_IGNORE_DSC non-atomically There are two sites where the flag is being changed: ide_retry_pc and idetape_do_request. Both codepaths are protected by hwif->busy (ide_lock_port) and therefore we shouldn't need the atomic accesses. Spotted-by: Jiri Slaby Signed-off-by: Borislav Petkov Signed-off-by: Bartlomiej Zolnierkiewicz --- drivers/ide/ide-tape.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) (limited to 'drivers/ide/ide-tape.c') diff --git a/drivers/ide/ide-tape.c b/drivers/ide/ide-tape.c index 203bbeac182..f1d3c7b2a2b 100644 --- a/drivers/ide/ide-tape.c +++ b/drivers/ide/ide-tape.c @@ -656,15 +656,15 @@ static ide_startstop_t idetape_do_request(ide_drive_t *drive, if ((drive->dev_flags & IDE_DFLAG_DSC_OVERLAP) == 0 && (rq->cmd[13] & REQ_IDETAPE_PC2) == 0) - set_bit(IDE_AFLAG_IGNORE_DSC, &drive->atapi_flags); + drive->atapi_flags |= IDE_AFLAG_IGNORE_DSC; if (drive->dev_flags & IDE_DFLAG_POST_RESET) { - set_bit(IDE_AFLAG_IGNORE_DSC, &drive->atapi_flags); + drive->atapi_flags |= IDE_AFLAG_IGNORE_DSC; drive->dev_flags &= ~IDE_DFLAG_POST_RESET; } - if (!test_and_clear_bit(IDE_AFLAG_IGNORE_DSC, &drive->atapi_flags) && - (stat & ATA_DSC) == 0) { + if (!(drive->atapi_flags & IDE_AFLAG_IGNORE_DSC) && + !(stat & ATA_DSC)) { if (postponed_rq == NULL) { tape->dsc_polling_start = jiffies; tape->dsc_poll_freq = tape->best_dsc_rw_freq; @@ -684,7 +684,9 @@ static ide_startstop_t idetape_do_request(ide_drive_t *drive, tape->dsc_poll_freq = IDETAPE_DSC_MA_SLOW; idetape_postpone_request(drive); return ide_stopped; - } + } else + drive->atapi_flags &= ~IDE_AFLAG_IGNORE_DSC; + if (rq->cmd[13] & REQ_IDETAPE_READ) { pc = &tape->queued_pc; ide_tape_create_rw_cmd(tape, pc, rq, READ_6); -- cgit v1.2.3 From 49d8078ad1c3dca5b11ce18391bf6bd9af9acdf5 Mon Sep 17 00:00:00 2001 From: Borislav Petkov Date: Sun, 7 Jun 2009 15:37:06 +0200 Subject: ide-tape: fix IDE_AFLAG_* atomic accesses These flags used to be bit numbers and now are single bits in the ->atapi_flags vector. Use them properly. Spotted-by: Jiri Slaby Signed-off-by: Borislav Petkov Signed-off-by: Bartlomiej Zolnierkiewicz --- drivers/ide/ide-tape.c | 43 ++++++++++++++++++++++++++----------------- 1 file changed, 26 insertions(+), 17 deletions(-) (limited to 'drivers/ide/ide-tape.c') diff --git a/drivers/ide/ide-tape.c b/drivers/ide/ide-tape.c index f1d3c7b2a2b..055f52e1ea0 100644 --- a/drivers/ide/ide-tape.c +++ b/drivers/ide/ide-tape.c @@ -397,7 +397,8 @@ static int ide_tape_callback(ide_drive_t *drive, int dsc) if (readpos[0] & 0x4) { printk(KERN_INFO "ide-tape: Block location is unknown" "to the tape\n"); - clear_bit(IDE_AFLAG_ADDRESS_VALID, &drive->atapi_flags); + clear_bit(ilog2(IDE_AFLAG_ADDRESS_VALID), + &drive->atapi_flags); uptodate = 0; err = IDE_DRV_ERROR_GENERAL; } else { @@ -406,7 +407,8 @@ static int ide_tape_callback(ide_drive_t *drive, int dsc) tape->partition = readpos[1]; tape->first_frame = be32_to_cpup((__be32 *)&readpos[4]); - set_bit(IDE_AFLAG_ADDRESS_VALID, &drive->atapi_flags); + set_bit(ilog2(IDE_AFLAG_ADDRESS_VALID), + &drive->atapi_flags); } } @@ -746,7 +748,7 @@ static int idetape_wait_ready(ide_drive_t *drive, unsigned long timeout) int load_attempted = 0; /* Wait for the tape to become ready */ - set_bit(IDE_AFLAG_MEDIUM_PRESENT, &drive->atapi_flags); + set_bit(ilog2(IDE_AFLAG_MEDIUM_PRESENT), &drive->atapi_flags); timeout += jiffies; while (time_before(jiffies, timeout)) { if (ide_do_test_unit_ready(drive, disk) == 0) @@ -822,7 +824,7 @@ static void __ide_tape_discard_merge_buffer(ide_drive_t *drive) if (tape->chrdev_dir != IDETAPE_DIR_READ) return; - clear_bit(IDE_AFLAG_FILEMARK, &drive->atapi_flags); + clear_bit(ilog2(IDE_AFLAG_FILEMARK), &drive->atapi_flags); tape->valid = 0; if (tape->buf != NULL) { kfree(tape->buf); @@ -1115,7 +1117,8 @@ static int idetape_space_over_filemarks(ide_drive_t *drive, short mt_op, if (tape->chrdev_dir == IDETAPE_DIR_READ) { tape->valid = 0; - if (test_and_clear_bit(IDE_AFLAG_FILEMARK, &drive->atapi_flags)) + if (test_and_clear_bit(ilog2(IDE_AFLAG_FILEMARK), + &drive->atapi_flags)) ++count; ide_tape_discard_merge_buffer(drive, 0); } @@ -1170,7 +1173,7 @@ static ssize_t idetape_chrdev_read(struct file *file, char __user *buf, debug_log(DBG_CHRDEV, "Enter %s, count %Zd\n", __func__, count); if (tape->chrdev_dir != IDETAPE_DIR_READ) { - if (test_bit(IDE_AFLAG_DETECT_BS, &drive->atapi_flags)) + if (test_bit(ilog2(IDE_AFLAG_DETECT_BS), &drive->atapi_flags)) if (count > tape->blk_size && (count % tape->blk_size) == 0) tape->user_bs_factor = count / tape->blk_size; @@ -1186,7 +1189,8 @@ static ssize_t idetape_chrdev_read(struct file *file, char __user *buf, /* refill if staging buffer is empty */ if (!tape->valid) { /* If we are at a filemark, nothing more to read */ - if (test_bit(IDE_AFLAG_FILEMARK, &drive->atapi_flags)) + if (test_bit(ilog2(IDE_AFLAG_FILEMARK), + &drive->atapi_flags)) break; /* read */ if (idetape_queue_rw_tail(drive, REQ_IDETAPE_READ, @@ -1204,7 +1208,7 @@ static ssize_t idetape_chrdev_read(struct file *file, char __user *buf, done += todo; } - if (!done && test_bit(IDE_AFLAG_FILEMARK, &drive->atapi_flags)) { + if (!done && test_bit(ilog2(IDE_AFLAG_FILEMARK), &drive->atapi_flags)) { debug_log(DBG_SENSE, "%s: spacing over filemark\n", tape->name); idetape_space_over_filemarks(drive, MTFSF, 1); @@ -1338,7 +1342,8 @@ static int idetape_mtioctop(ide_drive_t *drive, short mt_op, int mt_count) ide_tape_discard_merge_buffer(drive, 0); retval = ide_do_start_stop(drive, disk, !IDETAPE_LU_LOAD_MASK); if (!retval) - clear_bit(IDE_AFLAG_MEDIUM_PRESENT, &drive->atapi_flags); + clear_bit(ilog2(IDE_AFLAG_MEDIUM_PRESENT), + &drive->atapi_flags); return retval; case MTNOP: ide_tape_discard_merge_buffer(drive, 0); @@ -1360,9 +1365,11 @@ static int idetape_mtioctop(ide_drive_t *drive, short mt_op, int mt_count) mt_count % tape->blk_size) return -EIO; tape->user_bs_factor = mt_count / tape->blk_size; - clear_bit(IDE_AFLAG_DETECT_BS, &drive->atapi_flags); + clear_bit(ilog2(IDE_AFLAG_DETECT_BS), + &drive->atapi_flags); } else - set_bit(IDE_AFLAG_DETECT_BS, &drive->atapi_flags); + set_bit(ilog2(IDE_AFLAG_DETECT_BS), + &drive->atapi_flags); return 0; case MTSEEK: ide_tape_discard_merge_buffer(drive, 0); @@ -1507,20 +1514,20 @@ static int idetape_chrdev_open(struct inode *inode, struct file *filp) filp->private_data = tape; - if (test_and_set_bit(IDE_AFLAG_BUSY, &drive->atapi_flags)) { + if (test_and_set_bit(ilog2(IDE_AFLAG_BUSY), &drive->atapi_flags)) { retval = -EBUSY; goto out_put_tape; } retval = idetape_wait_ready(drive, 60 * HZ); if (retval) { - clear_bit(IDE_AFLAG_BUSY, &drive->atapi_flags); + clear_bit(ilog2(IDE_AFLAG_BUSY), &drive->atapi_flags); printk(KERN_ERR "ide-tape: %s: drive not ready\n", tape->name); goto out_put_tape; } idetape_read_position(drive); - if (!test_bit(IDE_AFLAG_ADDRESS_VALID, &drive->atapi_flags)) + if (!test_bit(ilog2(IDE_AFLAG_ADDRESS_VALID), &drive->atapi_flags)) (void)idetape_rewind_tape(drive); /* Read block size and write protect status from drive. */ @@ -1536,7 +1543,7 @@ static int idetape_chrdev_open(struct inode *inode, struct file *filp) if (tape->write_prot) { if ((filp->f_flags & O_ACCMODE) == O_WRONLY || (filp->f_flags & O_ACCMODE) == O_RDWR) { - clear_bit(IDE_AFLAG_BUSY, &drive->atapi_flags); + clear_bit(ilog2(IDE_AFLAG_BUSY), &drive->atapi_flags); retval = -EROFS; goto out_put_tape; } @@ -1593,15 +1600,17 @@ static int idetape_chrdev_release(struct inode *inode, struct file *filp) ide_tape_discard_merge_buffer(drive, 1); } - if (minor < 128 && test_bit(IDE_AFLAG_MEDIUM_PRESENT, &drive->atapi_flags)) + if (minor < 128 && test_bit(ilog2(IDE_AFLAG_MEDIUM_PRESENT), + &drive->atapi_flags)) (void) idetape_rewind_tape(drive); + if (tape->chrdev_dir == IDETAPE_DIR_NONE) { if (tape->door_locked == DOOR_LOCKED) { if (!ide_set_media_lock(drive, tape->disk, 0)) tape->door_locked = DOOR_UNLOCKED; } } - clear_bit(IDE_AFLAG_BUSY, &drive->atapi_flags); + clear_bit(ilog2(IDE_AFLAG_BUSY), &drive->atapi_flags); ide_tape_put(tape); unlock_kernel(); return 0; -- cgit v1.2.3 From 9d01e4cd7eb4a70b04cf5a5b4f79c99e8e3e3edc Mon Sep 17 00:00:00 2001 From: Borislav Petkov Date: Mon, 8 Jun 2009 22:03:03 +0200 Subject: ide-tape: fix proc warning ide_tape_chrdev_get() was missing an ide_device_get() refcount increment which lead to the following warning: [ 278.147906] ------------[ cut here ]------------ [ 278.152685] WARNING: at fs/proc/generic.c:847 remove_proc_entry+0x199/0x1b8() [ 278.160070] Hardware name: P4I45PE 1.00 [ 278.160076] remove_proc_entry: removing non-empty directory 'ide0/hdb', leaking at least 'name' [ 278.160080] Modules linked in: rtc intel_agp pcspkr thermal processor thermal_sys parport_pc parport agpgart button [ 278.160100] Pid: 2312, comm: mt Not tainted 2.6.30-rc2 #3 [ 278.160105] Call Trace: [ 278.160117] [] warn_slowpath+0x71/0xa0 [ 278.160126] [] ? _spin_unlock_irqrestore+0x29/0x2c [ 278.160132] [] ? try_to_wake_up+0x1b6/0x1c0 [ 278.160141] [] ? default_wake_function+0xb/0xd [ 278.160149] [] ? pollwake+0x4a/0x55 [ 278.160156] [] ? _spin_unlock+0x24/0x26 [ 278.160163] [] ? add_partial+0x44/0x49 [ 278.160169] [] ? __slab_free+0xba/0x29c [ 278.160177] [] ? sysfs_delete_inode+0x0/0x3c [ 278.160184] [] remove_proc_entry+0x199/0x1b8 [ 278.160191] [] ? remove_dir+0x27/0x2e [ 278.160199] [] ide_proc_unregister_device+0x40/0x4c [ 278.160207] [] drive_release_dev+0x14/0x47 [ 278.160214] [] device_release+0x35/0x5a [ 278.160221] [] kobject_release+0x40/0x50 [ 278.160226] [] ? kobject_release+0x0/0x50 [ 278.160232] [] kref_put+0x3c/0x4a [ 278.160238] [] kobject_put+0x37/0x3c [ 278.160243] [] put_device+0xf/0x11 [ 278.160249] [] ide_device_put+0x2d/0x30 [ 278.160255] [] ide_tape_put+0x24/0x32 [ 278.160261] [] idetape_chrdev_release+0x17f/0x18e [ 278.160269] [] __fput+0xca/0x175 [ 278.160275] [] fput+0x19/0x1b [ 278.160280] [] filp_close+0x51/0x5b [ 278.160286] [] sys_close+0x73/0xad [ 278.160293] [] syscall_call+0x7/0xb [ 278.160298] ---[ end trace f16d907ea1f89336 ]--- Instead of trivially fixing it by adding the missing call, ide_tape_chrdev_get() and ide_tape_get() were merged into one function since both were almost identical. The only difference was that ide_tape_chrdev_get() was accessing the ide-tape reference through the idetape_devs[] array of minors instead of through the gendisk. Accomodate that by adding two additional parameters to ide_tape_get() to annotate the call site and invoke the proper behavior. As a result, remove ide_tape_chrdev_get(). Signed-off-by: Borislav Petkov Signed-off-by: Bartlomiej Zolnierkiewicz --- drivers/ide/ide-tape.c | 35 +++++++++++++---------------------- 1 file changed, 13 insertions(+), 22 deletions(-) (limited to 'drivers/ide/ide-tape.c') diff --git a/drivers/ide/ide-tape.c b/drivers/ide/ide-tape.c index 055f52e1ea0..51ea59e3f6a 100644 --- a/drivers/ide/ide-tape.c +++ b/drivers/ide/ide-tape.c @@ -240,18 +240,27 @@ static struct class *idetape_sysfs_class; static void ide_tape_release(struct device *); -static struct ide_tape_obj *ide_tape_get(struct gendisk *disk) +static struct ide_tape_obj *idetape_devs[MAX_HWIFS * MAX_DRIVES]; + +static struct ide_tape_obj *ide_tape_get(struct gendisk *disk, bool cdev, + unsigned int i) { struct ide_tape_obj *tape = NULL; mutex_lock(&idetape_ref_mutex); - tape = ide_drv_g(disk, ide_tape_obj); + + if (cdev) + tape = idetape_devs[i]; + else + tape = ide_drv_g(disk, ide_tape_obj); + if (tape) { if (ide_device_get(tape->drive)) tape = NULL; else get_device(&tape->dev); } + mutex_unlock(&idetape_ref_mutex); return tape; } @@ -266,24 +275,6 @@ static void ide_tape_put(struct ide_tape_obj *tape) mutex_unlock(&idetape_ref_mutex); } -/* - * The variables below are used for the character device interface. Additional - * state variables are defined in our ide_drive_t structure. - */ -static struct ide_tape_obj *idetape_devs[MAX_HWIFS * MAX_DRIVES]; - -static struct ide_tape_obj *ide_tape_chrdev_get(unsigned int i) -{ - struct ide_tape_obj *tape = NULL; - - mutex_lock(&idetape_ref_mutex); - tape = idetape_devs[i]; - if (tape) - get_device(&tape->dev); - mutex_unlock(&idetape_ref_mutex); - return tape; -} - /* * called on each failed packet command retry to analyze the request sense. We * currently do not utilize this information. @@ -1495,7 +1486,7 @@ static int idetape_chrdev_open(struct inode *inode, struct file *filp) return -ENXIO; lock_kernel(); - tape = ide_tape_chrdev_get(i); + tape = ide_tape_get(NULL, true, i); if (!tape) { unlock_kernel(); return -ENXIO; @@ -1916,7 +1907,7 @@ static const struct file_operations idetape_fops = { static int idetape_open(struct block_device *bdev, fmode_t mode) { - struct ide_tape_obj *tape = ide_tape_get(bdev->bd_disk); + struct ide_tape_obj *tape = ide_tape_get(bdev->bd_disk, false, 0); if (!tape) return -ENXIO; -- cgit v1.2.3