1. 28 Apr, 2009 40 commits
    • Tejun Heo's avatar
      swim: clean up request completion paths · e138b4e0
      Tejun Heo authored
      swim curiously tries to update request parameters before calling
      __blk_end_request() when __blk_end_request() will do it anyway and
      unnecessarily checks whether current_nr_sectors is zero right after
      fetching.
      
      Drop unnecessary stuff and use standard block layer mechanisms.
      
      [ Impact: cleanup ]
      Signed-off-by: default avatarTejun Heo <tj@kernel.org>
      Cc: Laurent Vivier <Laurent@lvivier.info>
      Signed-off-by: default avatarJens Axboe <jens.axboe@oracle.com>
      e138b4e0
    • Tejun Heo's avatar
      swim3: clean up request completion paths · 467ca759
      Tejun Heo authored
      swim3 curiously tries to update request parameters before calling
      __blk_end_request() when __blk_end_request() will do it anyway, and it
      updates request for partial completion manually instead of using
      blk_update_request().  Also, it does some spurious checks on rq such
      as testing whether rq->sector is negative or current_nr_sectors is
      zero right after fetching.
      
      Drop unnecessary stuff and use standard block layer mechanisms.
      
      [ Impact: cleanup ]
      Signed-off-by: default avatarTejun Heo <tj@kernel.org>
      Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
      Signed-off-by: default avatarJens Axboe <jens.axboe@oracle.com>
      467ca759
    • Tejun Heo's avatar
      hd: clean up request completion paths · e091eb67
      Tejun Heo authored
      hd read/write_intr() functions manually manipulate request to
      incrementally complete it, which block layer already supports.  Simply
      use block layer completion routines instead of manual partial
      completion.
      
      While at it, clear unnecessary elv_next_request() check at the tail of
      read_intr().  This also makes read and write_intr() more consistent.
      
      [ Impact: cleanup ]
      Signed-off-by: default avatarTejun Heo <tj@kernel.org>
      Signed-off-by: default avatarJens Axboe <jens.axboe@oracle.com>
      e091eb67
    • Tejun Heo's avatar
      ubd: drop unnecessary rq->sector manipulation · f81f2f7c
      Tejun Heo authored
      ubd curiously updates rq->sector while issuing the request in multiple
      pieces.  Don't do it and simply use local copy of sector.
      
      [ Impact: cleanup ]
      Signed-off-by: default avatarTejun Heo <tj@kernel.org>
      Cc: Jeff Dike <jdike@linux.intel.com>
      Signed-off-by: default avatarJens Axboe <jens.axboe@oracle.com>
      f81f2f7c
    • Tejun Heo's avatar
      ubd: cleanup completion path · 4d6c84d9
      Tejun Heo authored
      ubd had its own block request partial completion mechanism, which is
      unnecessary as block layer already does it.  Kill ubd_end_request()
      and ubd_finish() and replace them with direct call to
      blk_end_request().
      
      [ Impact: cleanup ]
      Signed-off-by: default avatarTejun Heo <tj@kernel.org>
      Cc: Jeff Dike <jdike@linux.intel.com>
      Signed-off-by: default avatarJens Axboe <jens.axboe@oracle.com>
      4d6c84d9
    • Tejun Heo's avatar
      sunvdc: kill vdc_end_request() · 04420850
      Tejun Heo authored
      vdc_end_request() is a thin silly wrapper on top of
      __blk_end_request().  Kill it.
      
      [ Impact: cleanup ]
      Signed-off-by: default avatarTejun Heo <tj@kernel.org>
      Acked-by: default avatarDavid S. Miller <davem@davemloft.net>
      Signed-off-by: default avatarJens Axboe <jens.axboe@oracle.com>
      04420850
    • Tejun Heo's avatar
      ps3disk: simplify request completion · cd4c34eb
      Tejun Heo authored
      ps3disk_interrupt() always completes requests fully but it uses
      rq->hard_cur_sectors for FLUSH requests for some reason.  Drop them
      and simply use __blk_end_request_all().
      
      [ Impact: cleanup ]
      Signed-off-by: default avatarTejun Heo <tj@kernel.org>
      Signed-off-by: default avatarJens Axboe <jens.axboe@oracle.com>
      cd4c34eb
    • Tejun Heo's avatar
      amiflop,ataflop,xd,mg_disk: clean up unnecessary stuff from block drivers · 5b5c5d12
      Tejun Heo authored
      rq_data_dir() can only be READ or WRITE and rq->sector and nr_sectors
      are always automatically updated after partial request completion.
      Don't worry about rq_data_dir() not being either READ or WRITE or
      manually update sector and nr_sectors.
      
      [ Impact: cleanup ]
      Signed-off-by: default avatarTejun Heo <tj@kernel.org>
      Cc: Jörg Dorchain <joerg@dorchain.net>
      Cc: Geert Uytterhoeven <geert@linux-m68k.org>
      Cc: unsik Kim <donari75@gmail.com>
      Signed-off-by: default avatarJens Axboe <jens.axboe@oracle.com>
      5b5c5d12
    • Tejun Heo's avatar
      block: don't init rq fields unnecessarily · 4c94dece
      Tejun Heo authored
      blk_get_request() always returns properly zeroed requests.  Don't set
      fields to zero/NULL unnecessarily.
      
      [ Impact: cleanup ]
      Signed-off-by: default avatarTejun Heo <tj@kernel.org>
      Signed-off-by: default avatarJens Axboe <jens.axboe@oracle.com>
      4c94dece
    • Tejun Heo's avatar
      block: make blk_end_request_cur() return bool · 9fd8d0e1
      Tejun Heo authored
      In the process of mindlessly copying [__]blk_end_request_all(),
      [__]blk_end_request_cur() ended up returning void even though they're
      partial completion functions.  Fix it.
      
      [ Impact: fix braindead API ]
      Signed-off-by: default avatarTejun Heo <tj@kernel.org>
      Signed-off-by: default avatarJens Axboe <jens.axboe@oracle.com>
      9fd8d0e1
    • Nikanth Karthikesan's avatar
    • Jens Axboe's avatar
      block: include discard requests in IO accounting · c69d4854
      Jens Axboe authored
      We currently don't do merging on discard requests, but we potentially
      could. If we do, then we need to include discard requests in the IO
      accounting, or merging would end up decrementing in_flight IO counters
      for an IO which never incremented them.
      
      So enable accounting for discard requests.
      
      Problem found by Nikanth Karthikesan <knikanth@suse.de>
      Signed-off-by: default avatarJens Axboe <jens.axboe@oracle.com>
      c69d4854
    • Jens Axboe's avatar
      block: make blk_do_io_stat() do the full "is this rq accountable" checks · c2553b58
      Jens Axboe authored
      We currently check for file system requests outside of blk_do_io_stat(rq),
      but we may as well just include it.
      Signed-off-by: default avatarJens Axboe <jens.axboe@oracle.com>
      c2553b58
    • Tejun Heo's avatar
      block: kill rq->data · 731ec497
      Tejun Heo authored
      Now that all block request data transfer is done via bio, rq->data
      isn't used.  Kill it.
      
      While at it, make the roles of rq->special and buffer clear.
      
      [ Impact: drop now unncessary field from struct request ]
      Signed-off-by: default avatarTejun Heo <tj@kernel.org>
      Cc: Boaz Harrosh <bharrosh@panasas.com>
      731ec497
    • Tejun Heo's avatar
      arm-omap: don't abuse rq->data · ec24751a
      Tejun Heo authored
      omap mailbox uses rq->data as the second opaque pointer to carry
      mbox_msg_t and rq->special message argument which is needed only for
      tx.  Add and use omap_msg_tx_data struct for tx and use rq->special
      for mbox_msg_t for rx such that only rq->special is used as opaque
      pointer.
      
      [ Impact: cleanup rq->data usage, extra kmalloc in msg_send ]
      Signed-off-by: default avatarTejun Heo <tj@kernel.org>
      Cc: Russell King <rmk@arm.linux.org.uk>
      ec24751a
    • Tejun Heo's avatar
      block: replace end_request() with [__]blk_end_request_cur() · f06d9a2b
      Tejun Heo authored
      end_request() has been kept around for backward compatibility;
      however, it's about time for it to go away.
      
      * There aren't too many users left.
      
      * Its use of @updtodate is pretty confusing.
      
      * In some cases, newer code ends up using mixture of end_request() and
        [__]blk_end_request[_all](), which is way too confusing.
      
      So, add [__]blk_end_request_cur() and replace end_request() with it.
      Most conversions are straightforward.  Noteworthy ones are...
      
      * paride/pcd: next_request() updated to take 0/-errno instead of 1/0.
      
      * paride/pf: pf_end_request() and next_request() updated to take
        0/-errno instead of 1/0.
      
      * xd: xd_readwrite() updated to return 0/-errno instead of 1/0.
      
      * mtd/mtd_blkdevs: blktrans_discard_request() updated to return
        0/-errno instead of 1/0.  Unnecessary local variable res
        initialization removed from mtd_blktrans_thread().
      
      [ Impact: cleanup ]
      Signed-off-by: default avatarTejun Heo <tj@kernel.org>
      Acked-by: default avatarJoerg Dorchain <joerg@dorchain.net>
      Acked-by: default avatarGeert Uytterhoeven <geert@linux-m68k.org>
      Acked-by: default avatarGrant Likely <grant.likely@secretlab.ca>
      Acked-by: default avatarLaurent Vivier <Laurent@lvivier.info>
      Cc: Tim Waugh <tim@cyberelk.net>
      Cc: Stephen Rothwell <sfr@canb.auug.org.au>
      Cc: Paul Mackerras <paulus@samba.org>
      Cc: Jeremy Fitzhardinge <jeremy@xensource.com>
      Cc: Markus Lidel <Markus.Lidel@shadowconnect.com>
      Cc: David Woodhouse <dwmw2@infradead.org>
      Cc: Pete Zaitcev <zaitcev@redhat.com>
      Cc: unsik Kim <donari75@gmail.com>
      f06d9a2b
    • Tejun Heo's avatar
      block: implement and use [__]blk_end_request_all() · 40cbbb78
      Tejun Heo authored
      There are many [__]blk_end_request() call sites which call it with
      full request length and expect full completion.  Many of them ensure
      that the request actually completes by doing BUG_ON() the return
      value, which is awkward and error-prone.
      
      This patch adds [__]blk_end_request_all() which takes @rq and @error
      and fully completes the request.  BUG_ON() is added to to ensure that
      this actually happens.
      
      Most conversions are simple but there are a few noteworthy ones.
      
      * cdrom/viocd: viocd_end_request() replaced with direct calls to
        __blk_end_request_all().
      
      * s390/block/dasd: dasd_end_request() replaced with direct calls to
        __blk_end_request_all().
      
      * s390/char/tape_block: tapeblock_end_request() replaced with direct
        calls to blk_end_request_all().
      
      [ Impact: cleanup ]
      Signed-off-by: default avatarTejun Heo <tj@kernel.org>
      Cc: Russell King <rmk@arm.linux.org.uk>
      Cc: Stephen Rothwell <sfr@canb.auug.org.au>
      Cc: Mike Miller <mike.miller@hp.com>
      Cc: Martin Schwidefsky <schwidefsky@de.ibm.com>
      Cc: Jeff Garzik <jgarzik@pobox.com>
      Cc: Rusty Russell <rusty@rustcorp.com.au>
      Cc: Jeremy Fitzhardinge <jeremy@xensource.com>
      Cc: Alex Dubov <oakad@yahoo.com>
      Cc: James Bottomley <James.Bottomley@HansenPartnership.com>
      40cbbb78
    • Tejun Heo's avatar
      block: move rq->start_time initialization to blk_rq_init() · b243ddcb
      Tejun Heo authored
      rq->start_time was initialized in init_request_from_bio() so special
      requests didn't have start_time set.  This has been okay as start_time
      has been used only for fs requests; however, there is no indication of
      this actually is the case or not.  Set rq->start_time in blk_rq_init()
      and guarantee that all initialized rq's have its start_time set.  This
      improves consistency at virtually no cost and future changes will make
      use of the timestamp for !bio requests.
      
      [ Impact: rq->start_time is valid for all requests ]
      Signed-off-by: default avatarTejun Heo <tj@kernel.org>
      b243ddcb
    • Tejun Heo's avatar
      block: clean up request completion API · 2e60e022
      Tejun Heo authored
      Request completion has gone through several changes and became a bit
      messy over the time.  Clean it up.
      
      1. end_that_request_data() is a thin wrapper around
         end_that_request_data_first() which checks whether bio is NULL
         before doing anything and handles bidi completion.
         blk_update_request() is a thin wrapper around
         end_that_request_data() which clears nr_sectors on the last
         iteration but doesn't use the bidi completion.
      
         Clean it up by moving the initial bio NULL check and nr_sectors
         clearing on the last iteration into end_that_request_data() and
         renaming it to blk_update_request(), which makes blk_end_io() the
         only user of end_that_request_data().  Collapse
         end_that_request_data() into blk_end_io().
      
      2. There are four visible completion variants - blk_end_request(),
         __blk_end_request(), blk_end_bidi_request() and end_request().
         blk_end_request() and blk_end_bidi_request() uses blk_end_request()
         as the backend but __blk_end_request() and end_request() use
         separate implementation in __blk_end_request() due to different
         locking rules.
      
         blk_end_bidi_request() is identical to blk_end_io().  Collapse
         blk_end_io() into blk_end_bidi_request(), separate out request
         update into internal helper blk_update_bidi_request() and add
         __blk_end_bidi_request().  Redefine [__]blk_end_request() as thin
         inline wrappers around [__]blk_end_bidi_request().
      
      3. As the whole request issue/completion usages are about to be
         modified and audited, it's a good chance to convert completion
         functions return bool which better indicates the intended meaning
         of return values.
      
      4. The function name end_that_request_last() is from the days when it
         was a public interface and slighly confusing.  Give it a proper
         internal name - blk_finish_request().
      
      5. Add description explaning that blk_end_bidi_request() can be safely
         used for uni requests as suggested by Boaz Harrosh.
      
      The only visible behavior change is from #1.  nr_sectors counts are
      cleared after the final iteration no matter which function is used to
      complete the request.  I couldn't find any place where the code
      assumes those nr_sectors counters contain the values for the last
      segment and this change is good as it makes the API much more
      consistent as the end result is now same whether a request is
      completed using [__]blk_end_request() alone or in combination with
      blk_update_request().
      
      API further cleaned up per Christoph's suggestion.
      
      [ Impact: cleanup, rq->*nr_sectors always updated after req completion ]
      Signed-off-by: default avatarTejun Heo <tj@kernel.org>
      Reviewed-by: default avatarBoaz Harrosh <bharrosh@panasas.com>
      Cc: Christoph Hellwig <hch@infradead.org>
      2e60e022
    • Tejun Heo's avatar
      block: kill blk_end_request_callback() · 0b302d5a
      Tejun Heo authored
      With recent IDE updates, blk_end_request_callback() doesn't have any
      user now.  Kill it.
      
      [ Impact: removal of unused convoluted interface ]
      Signed-off-by: default avatarTejun Heo <tj@kernel.org>
      0b302d5a
    • Tejun Heo's avatar
      block: reorganize request fetching functions · 158dbda0
      Tejun Heo authored
      Impact: code reorganization
      
      elv_next_request() and elv_dequeue_request() are public block layer
      interface than actual elevator implementation.  They mostly deal with
      how requests interact with block layer and low level drivers at the
      beginning of rqeuest processing whereas __elv_next_request() is the
      actual eleveator request fetching interface.
      
      Move the two functions to blk-core.c.  This prepares for further
      interface cleanup.
      Signed-off-by: default avatarTejun Heo <tj@kernel.org>
      158dbda0
    • Tejun Heo's avatar
      block: reorder request completion functions · 5efccd17
      Tejun Heo authored
      Reorder request completion functions such that
      
      * All request completion functions are located together.
      
      * Functions which are used by only one caller is put right above the
        caller.
      
      * end_request() is put after other completion functions but before
        blk_update_request().
      
      This change is for completion function cleanup which will follow.
      
      [ Impact: cleanup, code reorganization ]
      Signed-off-by: default avatarTejun Heo <tj@kernel.org>
      5efccd17
    • Tejun Heo's avatar
      block: clean up misc stuff after block layer timeout conversion · 2eef33e4
      Tejun Heo authored
      * In blk_rq_timed_out_timer(), else { if } to else if
      
      * In blk_add_timer(), simplify if/else block
      
      [ Impact: cleanup ]
      Signed-off-by: default avatarTejun Heo <tj@kernel.org>
      2eef33e4
    • Tejun Heo's avatar
      block: cleanup REQ_SOFTBARRIER usages · 10732f56
      Tejun Heo authored
      blk_insert_request() doesn't need to worry about REQ_SOFTBARRIER.
      Don't set it.  Combined with recent ide updates, REQ_SOFTBARRIER is
      now only used in elevator proper and for discard requests.
      
      [ Impact: cleanup ]
      Signed-off-by: default avatarTejun Heo <tj@kernel.org>
      10732f56
    • Tejun Heo's avatar
      block: don't set REQ_NOMERGE unnecessarily · e4025f6c
      Tejun Heo authored
      RQ_NOMERGE_FLAGS already clears defines which REQ flags aren't
      mergeable.  There is no reason to specify it superflously.  It only
      adds to confusion.  Don't set REQ_NOMERGE for barriers and requests
      with specific queueing directive.  REQ_NOMERGE is now exclusively used
      by the merging code.
      
      [ Impact: cleanup ]
      Signed-off-by: default avatarTejun Heo <tj@kernel.org>
      e4025f6c
    • Tejun Heo's avatar
      block: kill blk_start_queueing() · a7f55792
      Tejun Heo authored
      blk_start_queueing() is identical to __blk_run_queue() except that it
      doesn't check for recursion.  None of the current users depends on
      blk_start_queueing() running request_fn directly.  Replace usages of
      blk_start_queueing() with [__]blk_run_queue() and kill it.
      
      [ Impact: removal of mostly duplicate interface function ]
      Signed-off-by: default avatarTejun Heo <tj@kernel.org>
      a7f55792
    • Tejun Heo's avatar
      block: merge blk_invoke_request_fn() into __blk_run_queue() · a538cd03
      Tejun Heo authored
      __blk_run_queue wraps blk_invoke_request_fn() such that it
      additionally removes plug and bails out early if the queue is empty.
      Both extra operations have their own pending mechanisms and don't
      cause any harm correctness-wise when they are done superflously.
      
      The only user of blk_invoke_request_fn() being blk_start_queue(),
      there isn't much reason to keep both functions around.  Merge
      blk_invoke_request_fn() into __blk_run_queue() and make
      blk_start_queue() use __blk_run_queue() instead.
      
      [ Impact: merge two subtly different internal functions ]
      Signed-off-by: default avatarTejun Heo <tj@kernel.org>
      a538cd03
    • Jeff Moyer's avatar
      block: implement blkdev_readpages · db2dbb12
      Jeff Moyer authored
      Doing a proper block dev ->readpages() speeds up the crazy dump(8)
      approach of using interleaved process IO.
      Signed-off-by: default avatarJeff Moyer <jmoyer@redhat.com>
      Signed-off-by: default avatarJens Axboe <jens.axboe@oracle.com>
      db2dbb12
    • Bartlomiej Zolnierkiewicz's avatar
      block: enable by default support for large devices and files on 32-bit archs · db29a6b4
      Bartlomiej Zolnierkiewicz authored
      Enable by default support for large devices and files (CONFIG_LBD):
      
      - With 1TB disks being a commodity hardware it is quite easy to hit 2TB
        limitation while building RAIDs etc. and many distros have been using
        CONFIG_LBD=y by default already (at least Fedora 10 and openSUSE 11.1).
      
      - This should also prevent a subtle ext4 filesystem compatibility issue:
        mke2fs.ext4 defaults to creating filesystems with huge_files feature
        enabled and such filesystems cannot be later mounted read-write on
        machines with CONFIG_LBD=n (it should be quite easy to hit this issue
        when trying to use filesystem created using distro kernel on system
        running the self-build kernel, think about USB disk enclosures & co.).
      
      While at it:
      
      - Clarify config option help text w.r.t. mounting ext4 filesystems
        (they can be mounted with CONFIG_LBD=n but in the read-only mode).
      
      Cc: "Theodore Ts'o" <tytso@mit.edu>
      Signed-off-by: default avatarBartlomiej Zolnierkiewicz <bzolnier@gmail.com>
      Signed-off-by: default avatarJens Axboe <jens.axboe@oracle.com>
      db29a6b4
    • Tejun Heo's avatar
      ide-dma: don't reset request fields on dma_timeout_retry() · 586cf268
      Tejun Heo authored
      Impact: drop unnecessary code
      
      Now that everything uses bio and block operations, there is no need to
      reset request fields manually when retrying a request.  Every field is
      guaranteed to be always valid.  Drop unnecessary request field
      resetting from ide_dma_timeout_retry().
      Signed-off-by: default avatarTejun Heo <tj@kernel.org>
      586cf268
    • Tejun Heo's avatar
      ide: drop rq->data handling from ide_map_sg() · 5ad960fe
      Tejun Heo authored
      Impact: remove code path which is no longer necessary
      
      All IDE data transfers now use rq->bio.  Simplify ide_map_sg()
      accordingly.
      Signed-off-by: default avatarTejun Heo <tj@kernel.org>
      Cc: Jens Axboe <axboe@kernel.dk>
      5ad960fe
    • Tejun Heo's avatar
      ide-atapi: kill unused fields and callbacks · 29d1a437
      Tejun Heo authored
      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: default avatarTejun Heo <tj@kernel.org>
      29d1a437
    • Tejun Heo's avatar
      ide-tape: simplify read/write functions · 4344d07f
      Tejun Heo authored
      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: default avatarTejun Heo <tj@kernel.org>
      4344d07f
    • Tejun Heo's avatar
      ide-tape: use byte size instead of sectors on rw issue functions · 71294cf9
      Tejun Heo authored
      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: default avatarTejun Heo <tj@kernel.org>
      71294cf9
    • Tejun Heo's avatar
      ide-tape: unify r/w init paths · 3596b664
      Tejun Heo authored
      Impact: cleanup
      
      Read and write init paths are almost identical.  Unify them into
      idetape_init_rw().
      Signed-off-by: default avatarTejun Heo <tj@kernel.org>
      3596b664
    • Tejun Heo's avatar
      ide-tape: kill idetape_bh · 6cf3d545
      Tejun Heo authored
      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: default avatarTejun Heo <tj@kernel.org>
      6cf3d545
    • Tejun Heo's avatar
      ide-tape: use standard data transfer mechanism · e998f30b
      Tejun Heo authored
      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: default avatarTejun Heo <tj@kernel.org>
      Cc: Jens Axboe <jens.axboe@oracle.com>
      e998f30b
    • Tejun Heo's avatar
      ide-tape: use single continuous buffer · 7b13354e
      Tejun Heo authored
      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: default avatarTejun Heo <tj@kernel.org>
      7b13354e
    • Tejun Heo's avatar
      ide-atapi,tape,floppy: allow ->pc_callback() to change rq->data_len · eb6a61bb
      Tejun Heo authored
      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: default avatarTejun Heo <tj@kernel.org>
      Cc: Jens Axboe <jens.axboe@oracle.com>
      eb6a61bb
    • Tejun Heo's avatar
      ide-tape,floppy: fix failed command completion after request sense · 08f370f0
      Tejun Heo authored
      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: default avatarTejun Heo <tj@kernel.org>
      08f370f0