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 <tj@kernel.org>

1 files changed, 84 insertions, 221 deletions

diff --git a/drivers/ide/ide-tape.c b/drivers/ide/ide-tape.c
index b373ac876352..d2e9c09b5215 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. */
@@ -749,89 +726,6 @@ out:
 }
 
 /*
- * It returns a pointer to the newly allocated buffer, or NULL in case
- * of failure.
- */
-static struct idetape_bh *ide_tape_kmalloc_buffer(idetape_tape_t *tape,
-                                                  int full)
-{
-        struct idetape_bh *bh;
-
-        bh = kmalloc(sizeof(struct idetape_bh), GFP_KERNEL);
-        if (!bh)
-                return NULL;
-
-        bh->b_data = kmalloc(tape->buffer_size, GFP_KERNEL);
-        if (!bh->b_data) {
-                kfree(bh);
-                return NULL;
-        }
-
-        bh->b_size = tape->buffer_size;
-        atomic_set(&bh->b_count, full ? bh->b_size : 0);
-
-        return bh;
-}
-
-static int idetape_copy_stage_from_user(idetape_tape_t *tape,
-                                        const char __user *buf, int n)
-{
-        struct idetape_bh *bh = tape->bh;
-        int ret = 0;
-
-        if (n) {
-                if (bh == NULL || n > bh->b_size - atomic_read(&bh->b_count)) {
-                        printk(KERN_ERR "ide-tape: bh == NULL in %s\n",
-                                        __func__);
-                        return 1;
-                }
-                if (copy_from_user(bh->b_data + atomic_read(&bh->b_count), buf,
-                                   n))
-                        ret = 1;
-                atomic_add(n, &bh->b_count);
-                if (atomic_read(&bh->b_count) == bh->b_size)
-                        tape->bh = NULL;
-        }
-
-        return ret;
-}
-
-static int idetape_copy_stage_to_user(idetape_tape_t *tape, char __user *buf,
-                                      int n)
-{
-        struct idetape_bh *bh = tape->bh;
-        int ret = 0;
-
-        if (n) {
-                if (bh == NULL || n > tape->b_count) {
-                        printk(KERN_ERR "ide-tape: bh == NULL in %s\n",
-                                        __func__);
-                        return 1;
-                }
-                if (copy_to_user(buf, tape->b_data, n))
-                        ret = 1;
-                tape->b_data += n;
-                tape->b_count -= n;
-                if (!tape->b_count)
-                        tape->bh = NULL;
-        }
-        return ret;
-}
-
-static void idetape_init_merge_buffer(idetape_tape_t *tape)
-{
-        struct idetape_bh *bh = tape->merge_bh;
-        tape->bh = tape->merge_bh;
-
-        if (tape->chrdev_dir == IDETAPE_DIR_WRITE)
-                atomic_set(&bh->b_count, 0);
-        else {
-                tape->b_data = bh->b_data;
-                tape->b_count = atomic_read(&bh->b_count);
-        }
-}
-
-/*
  * Write a filemark if write_filemark=1. Flush the device buffers without
  * writing a filemark otherwise.
  */
@@ -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;