1. 22 Sep, 2009 1 commit
  2. 01 Sep, 2009 1 commit
  3. 07 Aug, 2009 3 commits
    • Borislav Petkov's avatar
      ide-tape: fix handling of postponed rqs · 6f3848ac
      Borislav Petkov authored
      ide-tape used to hit
      [   58.614854] ide-tape: ht0: BUG: Two DSC requests queued!
      due to the fact that another rq was being issued while the driver was
      waiting for DSC to get set for the device executing ATAPI commands which
      set the DSC to 1 to indicate completion.
      Here's a sample output of that case:
      issue REZERO_UNIT
      [  143.088505] ide-tape: ide_tape_issue_pc: retry #0, cmd: 0x01
      [  143.095122] ide: Enter ide_pc_intr - interrupt handler
      [  143.096118] ide: Packet command completed, 0 bytes transferred
      [  143.106319] ide-tape: ide_tape_callback: cmd: 0x1, dsc: 1, err: 0
      [  143.112601] ide-tape: idetape_postpone_request: cmd: 0x1, dsc_poll_freq: 2000
      we stall the ide-tape queue here waiting for DSC
      [  143.119936] ide-tape: ide_tape_read_position: enter
      [  145.119019] ide-tape: idetape_do_request: sector: 4294967295, nr_sectors: 0
      and issue the new READ_POSITION rq and hit the check.
      [  145.126247] ide-tape: ht0: BUG: Two DSC requests queued!
      [  145.131748] ide-tape: ide_tape_read_position: BOP - No
      [  145.137059] ide-tape: ide_tape_read_position: EOP - No
      Also, ->postponed_rq used to point to that postponed request. To make
      things worse, in certain circumstances the rq it was pointing to got
      replaced unterneath it by swiftly reusing the same rq from the mempool
      of the block layer practically confusing stuff even more.
      However, we don't need to keep a pointer to that rq but simply wait for
      DSC to be set first before issuing the follow-up request in the drive's
      queue. In order to do that, we make idetape_do_request() first check the
      DSC and if not set, we stall the drive queue giving the other device on
      that IDE channel a chance.
      Signed-off-by: default avatarBorislav Petkov <petkovbb@gmail.com>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
    • Borislav Petkov's avatar
      ide-tape: convert to ide_debug_log macro · e972d702
      Borislav Petkov authored
      Remove tape->debug_mask and use drive->debug_mask instead.
      There should be no functional change resulting from this patch.
      Signed-off-by: default avatarBorislav Petkov <petkovbb@gmail.com>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
    • Mark de Wever's avatar
      ide-tape: fix debug call · 37bbe084
      Mark de Wever authored
      This error only occurs when IDETAPE_DEBUG_LOG is enabled.
      Signed-off-by: default avatarMark de Wever <koraq@xs4all.nl>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
  4. 21 Jul, 2009 1 commit
  5. 15 Jun, 2009 2 commits
  6. 08 Jun, 2009 1 commit
    • Borislav Petkov's avatar
      ide-tape: fix proc warning · 9d01e4cd
      Borislav Petkov authored
      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]  [<c012141d>] warn_slowpath+0x71/0xa0
      [  278.160126]  [<c035f219>] ? _spin_unlock_irqrestore+0x29/0x2c
      [  278.160132]  [<c011c686>] ? try_to_wake_up+0x1b6/0x1c0
      [  278.160141]  [<c011c69b>] ? default_wake_function+0xb/0xd
      [  278.160149]  [<c0177ead>] ? pollwake+0x4a/0x55
      [  278.160156]  [<c035f240>] ? _spin_unlock+0x24/0x26
      [  278.160163]  [<c0165d38>] ? add_partial+0x44/0x49
      [  278.160169]  [<c01669e8>] ? __slab_free+0xba/0x29c
      [  278.160177]  [<c01a13d8>] ? sysfs_delete_inode+0x0/0x3c
      [  278.160184]  [<c019ca92>] remove_proc_entry+0x199/0x1b8
      [  278.160191]  [<c01a297e>] ? remove_dir+0x27/0x2e
      [  278.160199]  [<c025f3ab>] ide_proc_unregister_device+0x40/0x4c
      [  278.160207]  [<c02599cd>] drive_release_dev+0x14/0x47
      [  278.160214]  [<c0250538>] device_release+0x35/0x5a
      [  278.160221]  [<c01f8bed>] kobject_release+0x40/0x50
      [  278.160226]  [<c01f8bad>] ? kobject_release+0x0/0x50
      [  278.160232]  [<c01f96ac>] kref_put+0x3c/0x4a
      [  278.160238]  [<c01f8b29>] kobject_put+0x37/0x3c
      [  278.160243]  [<c025020c>] put_device+0xf/0x11
      [  278.160249]  [<c025789f>] ide_device_put+0x2d/0x30
      [  278.160255]  [<c02658da>] ide_tape_put+0x24/0x32
      [  278.160261]  [<c0266e0c>] idetape_chrdev_release+0x17f/0x18e
      [  278.160269]  [<c016c4f5>] __fput+0xca/0x175
      [  278.160275]  [<c016c5b9>] fput+0x19/0x1b
      [  278.160280]  [<c0169d19>] filp_close+0x51/0x5b
      [  278.160286]  [<c0169d96>] sys_close+0x73/0xad
      [  278.160293]  [<c0102a61>] 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: default avatarBorislav Petkov <petkovbb@gmail.com>
      Signed-off-by: default avatarBartlomiej Zolnierkiewicz <bzolnier@gmail.com>
  7. 07 Jun, 2009 2 commits
  8. 19 May, 2009 1 commit
    • Tejun Heo's avatar
      block: set rq->resid_len to blk_rq_bytes() on issue · 5f49f631
      Tejun Heo authored
      In commit c3a4d78c, 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
      Signed-off-by: default avatarTejun Heo <tj@kernel.org>
      Acked-by: default avatarBorislav Petkov <petkovbb@googlemail.com>
      Cc: Boaz Harrosh <bharrosh@panasas.com>
      Cc: James Bottomley <James.Bottomley@HansenPartnership.com>
      Cc: Pete Zaitcev <zaitcev@redhat.com>
      Cc: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
      Cc: Sergei Shtylyov <sshtylyov@ru.mvista.com>
      Cc: Eric Moore <Eric.Moore@lsi.com>
      Cc: Darrick J. Wong <djwong@us.ibm.com>
      Signed-off-by: default avatarJens Axboe <jens.axboe@oracle.com>
  9. 17 May, 2009 1 commit
  10. 16 May, 2009 1 commit
  11. 14 May, 2009 9 commits
  12. 11 May, 2009 4 commits
    • Tejun Heo's avatar
      ide: cleanup rq->data_len usages · 34b7d2c9
      Tejun Heo authored
      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: default avatarTejun Heo <tj@kernel.org>
      Acked-by: default avatarBartlomiej Zolnierkiewicz <bzolnier@gmail.com>
      Cc: Borislav Petkov <petkovbb@googlemail.com>
      Cc: Sergei Shtylyov <sshtylyov@ru.mvista.com>
      Signed-off-by: default avatarJens Axboe <jens.axboe@oracle.com>
    • Tejun Heo's avatar
      ide: convert to rq pos and nr_sectors accessors · 9780e2dd
      Tejun Heo authored
      ide doesn't manipulate request fields anymore and thus all hard and
      their soft equivalents are always equal.  Convert all references to
      [ Impact: use pos and nr_sectors accessors ]
      Signed-off-by: default avatarTejun Heo <tj@kernel.org>
      Acked-by: default avatarBartlomiej Zolnierkiewicz <bzolnier@gmail.com>
      Cc: Borislav Petkov <petkovbb@googlemail.com>
      Cc: Sergei Shtylyov <sshtylyov@ru.mvista.com>
      Signed-off-by: default avatarJens Axboe <jens.axboe@oracle.com>
    • Tejun Heo's avatar
      block: add rq->resid_len · c3a4d78c
      Tejun Heo authored
      rq->data_len served two purposes - the length of data buffer on issue
      and the residual count on completion.  This duality creates some
      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: default avatarTejun Heo <tj@kernel.org>
      Cc: James Bottomley <James.Bottomley@HansenPartnership.com>
      Cc: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
      Cc: Borislav Petkov <petkovbb@googlemail.com>
      Cc: Sergei Shtylyov <sshtylyov@ru.mvista.com>
      Cc: Mike Miller <mike.miller@hp.com>
      Cc: Eric Moore <Eric.Moore@lsi.com>
      Cc: Alan Stern <stern@rowland.harvard.edu>
      Cc: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
      Cc: Doug Gilbert <dgilbert@interlog.com>
      Cc: Mike Miller <mike.miller@hp.com>
      Cc: Eric Moore <Eric.Moore@lsi.com>
      Cc: Darrick J. Wong <djwong@us.ibm.com>
      Cc: Pete Zaitcev <zaitcev@redhat.com>
      Cc: Boaz Harrosh <bharrosh@panasas.com>
      Signed-off-by: default avatarJens Axboe <jens.axboe@oracle.com>
    • Tejun Heo's avatar
      ide-tape: don't initialize rq->sector for rw requests · 9720aef2
      Tejun Heo authored
      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
      [ Impact: cleanup ]
      Signed-off-by: default avatarTejun Heo <tj@kernel.org>
      Acked-by: default avatarBorislav Petkov <petkovbb@gmail.com>
      Acked-by: default avatarBartlomiej Zolnierkiewicz <bzolnier@gmail.com>
      Signed-off-by: default avatarJens Axboe <jens.axboe@oracle.com>
  13. 27 Apr, 2009 13 commits
    • 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
      * 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>
    • 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>
    • 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
      Signed-off-by: default avatarTejun Heo <tj@kernel.org>
    • 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
      Signed-off-by: default avatarTejun Heo <tj@kernel.org>
    • 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>
    • 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
      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>
    • 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
      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>
    • 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
      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>
    • 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>
    • Tejun Heo's avatar
      ide-cd,atapi: use bio for internal commands · 02e7cf8f
      Tejun Heo authored
      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: default avatarTejun Heo <tj@kernel.org>
    • Borislav Petkov's avatar
      ide-atapi: convert ide-{floppy,tape} to using preallocated sense buffer · 06875320
      Borislav Petkov authored
      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
      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_retry_pc() no longer takes @disk.
      CC: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
      CC: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
      Signed-off-by: default avatarBorislav Petkov <petkovbb@gmail.com>
      Signed-off-by: default avatarTejun Heo <tj@kernel.org>
    • Tejun Heo's avatar
      ide-atapi: don't abuse rq->buffer · ac0b0113
      Tejun Heo authored
      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
      Signed-off-by: default avatarTejun Heo <tj@kernel.org>
      Cc: Jens Axboe <axboe@kernel.dk>
    • Tejun Heo's avatar
      ide-tape: remove back-to-back REQUEST_SENSE detection · 0de57fb9
      Tejun Heo authored
      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: default avatarTejun Heo <tj@kernel.org>