02 Aug, 2012

1 commit

  • Pull second vfs pile from Al Viro:
    "The stuff in there: fsfreeze deadlock fixes by Jan (essentially, the
    deadlock reproduced by xfstests 068), symlink and hardlink restriction
    patches, plus assorted cleanups and fixes.

    Note that another fsfreeze deadlock (emergency thaw one) is *not*
    dealt with - the series by Fernando conflicts a lot with Jan's, breaks
    userland ABI (FIFREEZE semantics gets changed) and trades the deadlock
    for massive vfsmount leak; this is going to be handled next cycle.
    There probably will be another pull request, but that stuff won't be
    in it."

    Fix up trivial conflicts due to unrelated changes next to each other in
    drivers/{staging/gdm72xx/usb_boot.c, usb/gadget/storage_common.c}

    * 'for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/viro/vfs: (54 commits)
    delousing target_core_file a bit
    Documentation: Correct s_umount state for freeze_fs/unfreeze_fs
    fs: Remove old freezing mechanism
    ext2: Implement freezing
    btrfs: Convert to new freezing mechanism
    nilfs2: Convert to new freezing mechanism
    ntfs: Convert to new freezing mechanism
    fuse: Convert to new freezing mechanism
    gfs2: Convert to new freezing mechanism
    ocfs2: Convert to new freezing mechanism
    xfs: Convert to new freezing code
    ext4: Convert to new freezing mechanism
    fs: Protect write paths by sb_start_write - sb_end_write
    fs: Skip atime update on frozen filesystem
    fs: Add freezing handling to mnt_want_write() / mnt_drop_write()
    fs: Improve filesystem freezing handling
    switch the protection of percpu_counter list to spinlock
    nfsd: Push mnt_want_write() outside of i_mutex
    btrfs: Push mnt_want_write() outside of i_mutex
    fat: Push mnt_want_write() outside of i_mutex
    ...

    Linus Torvalds
     

01 Aug, 2012

1 commit

  • * set_fs(KERNEL_DS) + getname() is probably the weirdest implementation
    of strdup() I've seen. Especially since they don't to copy it at all...
    * filp_open() never returns NULL; it's ERR_PTR(-E...) on failure.
    * file->f_dentry is never going to be NULL, TYVM.
    * match_strdup() + snprintf() + kfree() is a bloody weird way to spell
    match_strlcpy().

    Pox on cargo-cult programmers...

    Signed-off-by: Al Viro

    Al Viro
     

21 Jul, 2012

2 commits

  • From Al Viro:

    BTW, speaking of struct file treatment related to sockets -
    there's this piece of code in iscsi:
    /*
    * The SCTP stack needs struct socket->file.
    */
    if ((np->np_network_transport == ISCSI_SCTP_TCP) ||
    (np->np_network_transport == ISCSI_SCTP_UDP)) {
    if (!new_sock->file) {
    new_sock->file = kzalloc(
    sizeof(struct file), GFP_KERNEL);

    For one thing, as far as I can see it'not true - sctp does *not* depend on
    socket->file being non-NULL; it does, in one place, check socket->file->f_flags
    for O_NONBLOCK, but there it treats NULL socket->file as "flag not set".
    Which is the case here anyway - the fake struct file created in
    __iscsi_target_login_thread() (and in iscsi_target_setup_login_socket(), with
    the same excuse) do *not* get that flag set.

    Moreover, it's a bloody serious violation of a bunch of asserts in VFS;
    all struct file instances should come from filp_cachep, via get_empty_filp()
    (or alloc_file(), which is a wrapper for it). FWIW, I'm very tempted to
    do this and be done with the entire mess:

    Signed-off-by: Al Viro
    Cc: Andy Grover
    Cc: Hannes Reinecke
    Cc: Christoph Hellwig
    Cc: stable@vger.kernel.org
    Signed-off-by: Nicholas Bellinger

    Al Viro
     
  • During a failure in transport_add_device_to_core_hba() code, we called
    destroy_workqueue(dev->tmr_wq) before ->tmr_wq was allocated which leads
    to an oops.

    This fixes a regression introduced in with:

    commit af8772926f019b7bddd7477b8de5f3b0f12bad21
    Author: Christoph Hellwig
    Date: Sun Jul 8 15:58:49 2012 -0400

    target: replace the processing thread with a TMR work queue

    Signed-off-by: Dan Carpenter
    Cc: Christoph Hellwig
    Signed-off-by: Nicholas Bellinger

    Dan Carpenter
     

18 Jul, 2012

1 commit

  • We want it to be possible for target_submit_cmd() to return errors up
    to its fabric module callers. For now just update the prototype to
    return an int, and update all callers to handle non-zero return values
    as an error.

    This is immediately useful for tcm_qla2xxx to fix a long-standing active
    I/O session shutdown race, but tcm_fc, usb-gadget, and sbp-target the
    fabric maintainers need to check + ACK that handling a target_submit_cmd()
    failure due to session shutdown does not introduce regressions

    (nab: Respin against for-next after initial NACK + update docbook comment +
    fix double se_cmd init in exception path for usb-gadget)

    Cc: Chad Dupuis
    Cc: Arun Easi
    Cc: Chris Boot
    Cc: Stefan Richter
    Cc: Mark Rustad
    Cc: Sebastian Andrzej Siewior
    Cc: Felipe Balbi
    Cc: Andy Grover
    Signed-off-by: Roland Dreier
    Signed-off-by: Nicholas Bellinger

    Roland Dreier
     

17 Jul, 2012

35 commits

  • Fail UNMAP commands that have more than our reported limit on unmap
    descriptors.

    Signed-off-by: Roland Dreier
    Cc: stable@vger.kernel.org
    Signed-off-by: Nicholas Bellinger

    Roland Dreier
     
  • It's possible for an initiator to send us an UNMAP command with a
    descriptor that is less than 8 bytes; in that case it's really bad for
    us to set an unsigned int to that value, subtract 8 from it, and then
    use that as a limit for our loop (since the value will wrap around to
    a huge positive value).

    Fix this by making size be signed and only looping if size >= 16 (ie
    if we have at least a full descriptor available).

    Also remove offset as an obfuscated name for the constant 8.

    Signed-off-by: Roland Dreier
    Cc: stable@vger.kernel.org
    Signed-off-by: Nicholas Bellinger

    Roland Dreier
     
  • The UNMAP DATA LENGTH and UNMAP BLOCK DESCRIPTOR DATA LENGTH fields
    are in the unmap descriptor (the payload transferred to our data out
    buffer), not in the CDB itself. Read them from the correct place in
    target_emulated_unmap.

    Signed-off-by: Roland Dreier
    Cc: stable@vger.kernel.org
    Signed-off-by: Nicholas Bellinger

    Roland Dreier
     
  • When processing an UNMAP command, we need to make sure that the number
    of blocks we're asked to UNMAP does not exceed our reported maximum
    number of blocks per UNMAP, and that the range of blocks we're
    unmapping doesn't go past the end of the device.

    Signed-off-by: Roland Dreier
    Cc: stable@vger.kernel.org
    Signed-off-by: Nicholas Bellinger

    Roland Dreier
     
  • Many SCSI commands are defined to return a CHECK CONDITION / ILLEGAL
    REQUEST with ASC set to LOGICAL BLOCK ADDRESS OUT OF RANGE if the
    initiator sends a command that accesses a too-big LBA. Add an enum
    value and case entries so that target code can return this status.

    Signed-off-by: Roland Dreier
    Cc: stable@vger.kernel.org
    Signed-off-by: Nicholas Bellinger

    Roland Dreier
     
  • Signed-off-by: Roland Dreier
    Signed-off-by: Nicholas Bellinger

    Roland Dreier
     
  • Since we set se_session.sess_tearing_down and stop new commands from
    being added to se_session.sess_cmd_list before we wait for commands to
    finish when freeing a session, there's no need for a separate
    sess_wait_list -- if we let new commands be added to sess_cmd_list
    after setting sess_tearing_down, that would be a bug that breaks the
    logic of waiting in-flight commands.

    Also rename target_splice_sess_cmd_list() to
    target_sess_cmd_list_set_waiting(), since we are no longer splicing
    onto a separate list.

    Signed-off-by: Roland Dreier
    Signed-off-by: Nicholas Bellinger

    Roland Dreier
     
  • Target core code assumes that target_splice_sess_cmd_list() has set
    sess_tearing_down and moved the list of pending commands to
    sess_wait_list, no more commands will be added to the session; if any
    are added, nothing keeps the se_session from being freed while the
    command is still in flight, which e.g. leads to use-after-free of
    se_cmd->se_sess in target_release_cmd_kref().

    To enforce this invariant, put a check of sess_tearing_down inside of
    sess_cmd_lock in target_get_sess_cmd(); any checks before this are
    racy and can lead to the use-after-free described above. For example,
    the qla_target check in qlt_do_work() checks sess_tearing_down from
    work thread context but then drops all locks before calling
    target_submit_cmd() (as it must, since that is a sleeping function).

    However, since no locks are held, anything can happen with respect to
    the session it has looked up -- although it does correctly get
    sess_kref within its lock, so the memory won't be freed while
    target_submit_cmd() is actually running, nothing stops eg an ACL from
    being dropped and calling ->shutdown_session() (which calls into
    target_splice_sess_cmd_list()) before we get to target_get_sess_cmd().
    Once this happens, the se_session memory can be freed as soon as
    target_submit_cmd() returns and qlt_do_work() drops its reference,
    even though we've just added a command to sess_cmd_list.

    To prevent this use-after-free, check sess_tearing_down inside of
    sess_cmd_lock right before target_get_sess_cmd() adds a command to
    sess_cmd_list; this is synchronized with target_splice_sess_cmd_list()
    so that every command is either waited for or not added to the queue.

    (nab: Keep target_submit_cmd() returning void for now..)

    Signed-off-by: Roland Dreier
    Signed-off-by: Nicholas Bellinger

    Roland Dreier
     
  • Cc: Chris Boot
    Cc: Stefan Richter
    Signed-off-by: Roland Dreier

    Roland Dreier
     
  • There are no in-tree users of target_get_sess_cmd() outside of
    target_core_transport.c. Any new code should use the higher-level
    target_submit_cmd() interface. So let's un-export target_get_sess_cmd()
    and make it static to the one file where it's actually used.

    (nab: Fix up minor fuzz to for-next)

    Signed-off-by: Roland Dreier
    Signed-off-by: Nicholas Bellinger

    Roland Dreier
     
  • So after kicking around commit 547ac4c9c90 around a bit more, a tcm_qla2xxx LUN
    unlink OP has generated the following warning:

    [ 50.386625] qla2xxx [0000:07:00.0]-00af:0: Performing ISP error recovery - ha=ffff880263774000.
    [ 70.572988] qla2xxx [0000:07:00.0]-8038:0: Cable is unplugged...
    [ 126.527531] ------------[ cut here ]------------
    [ 126.532677] WARNING: at kernel/softirq.c:159 local_bh_enable_ip+0x41/0x8c()
    [ 126.540433] Hardware name: S5520HC
    [ 126.544248] Modules linked in: tcm_vhost ib_srpt ib_cm ib_sa ib_mad ib_core tcm_qla2xxx tcm_loop tcm_fc libfc iscsi_target_mod target_core_pscsi target_core_file target_core_iblock target_core_mod configfs ipv6 iscsi_tcp libiscsi_tcp libiscsi scsi_transport_iscsi dm_round_robin dm_multipath scsi_dh loop i2c_i801 kvm_intel kvm crc32c_intel i2c_core microcode joydev button iomemory_vsl(O) pcspkr ext3 jbd uhci_hcd lpfc ata_piix libata ehci_hcd qla2xxx mlx4_core scsi_transport_fc scsi_tgt igb [last unloaded: scsi_wait_scan]
    [ 126.595567] Pid: 3283, comm: unlink Tainted: G O 3.5.0-rc2+ #33
    [ 126.603128] Call Trace:
    [ 126.605853] [] ? warn_slowpath_common+0x78/0x8c
    [ 126.612737] [] ? local_bh_enable_ip+0x41/0x8c
    [ 126.619433] [] ? core_disable_device_list_for_node+0x70/0xe3 [target_core_mod]
    [ 126.629323] [] ? core_clear_lun_from_tpg+0x88/0xeb [target_core_mod]
    [ 126.638244] [] ? core_tpg_post_dellun+0x17/0x48 [target_core_mod]
    [ 126.646873] [] ? core_dev_del_lun+0x26/0x8c [target_core_mod]
    [ 126.655114] [] ? dput+0x27/0x154
    [ 126.660549] [] ? target_fabric_port_unlink+0x3b/0x41 [target_core_mod]
    [ 126.669661] [] ? configfs_unlink+0xfc/0x14a [configfs]
    [ 126.677224] [] ? vfs_unlink+0x58/0xb7
    [ 126.683141] [] ? do_unlinkat+0xbb/0x142
    [ 126.689253] [] ? page_fault+0x25/0x30
    [ 126.695170] [] ? system_call_fastpath+0x16/0x1b
    [ 126.702053] ---[ end trace 2f8e5b0a9ec797ef ]---
    [ 126.756336] qla2xxx [0000:07:00.0]-00af:0: Performing ISP error recovery - ha=ffff880263774000.
    [ 146.942414] qla2xxx [0000:07:00.0]-8038:0: Cable is unplugged...

    So this warning triggered because device_list disable logic is now
    holding nacl->device_list_lock w/ spin_lock_irqsave before obtaining
    port->sep_alua_lock with only spin_lock_bh..

    The original disable logic obtains *deve ahead of dropping the entry
    from deve->alua_port_list and then obtains ->device_list_lock to do the
    remaining work. Also, I'm pretty sure this particular warning is being
    generated by a demo-mode session in tcm_qla2xxx, and not by explicit
    NodeACL MappedLUNs. The Initiator MappedLUNs are already protected by a
    seperate configfs symlink reference back se_lun->lun_group, and the
    demo-mode se_node_acl (and associated ->device_list[]) is released
    during se_portal_group->tpg_group shutdown.

    The following patch drops the extra functional change to disable logic
    in commit 547ac4c9c90

    Cc: Andy Grover
    Signed-off-by: Nicholas Bellinger

    Nicholas Bellinger
     
  • Code was almost entirely divided based on value of bool param "enable".

    Split it into two functions.

    Signed-off-by: Andy Grover
    Signed-off-by: Nicholas Bellinger

    Andy Grover
     
  • Signed-off-by: Andy Grover
    Signed-off-by: Nicholas Bellinger

    Andy Grover
     
  • Bubble-up retval from iscsi_update_param_value() and
    iscsit_ta_authentication().

    Other very small retval cleanups.

    Signed-off-by: Andy Grover
    Signed-off-by: Nicholas Bellinger

    Andy Grover
     
  • Only used in a debugprint, and function signature is cleaner now.

    Signed-off-by: Andy Grover
    Signed-off-by: Nicholas Bellinger

    Andy Grover
     
  • Signed-off-by: Andy Grover
    Signed-off-by: Nicholas Bellinger

    Andy Grover
     
  • The last functionality of the target processing thread is offloading possibly
    long running task management requests from the submitter context. To keep
    TMR semantics the same we need a single threaded ordered queue, which can
    be provided by a per-device workqueue with the right flags.

    Signed-off-by: Christoph Hellwig
    Signed-off-by: Nicholas Bellinger

    Christoph Hellwig
     
  • Remove this command submission path which is not used by any in-tree driver.
    This also removes the now unused new_cmd_map fabtric method, which a few
    drivers implemented despite never calling transport_generic_handle_cdb_map.

    Signed-off-by: Christoph Hellwig
    Signed-off-by: Nicholas Bellinger

    Christoph Hellwig
     
  • There is no need to schedule the delayed processing in a workqueue that
    offloads it to the target processing thread. Instead execute it directly
    from the workqueue. There will be a lot of future work in this area,
    which I'd likfe to defer for now as it is not nessecary for getting rid
    of the target processing thread.

    Signed-off-by: Christoph Hellwig
    Signed-off-by: Nicholas Bellinger

    Christoph Hellwig
     
  • Signed-off-by: Christoph Hellwig
    Signed-off-by: Nicholas Bellinger

    Christoph Hellwig
     
  • Defer the write processing to the internal to be able to use
    target_execute_cmd. I'm not even entirely sure the calling code requires
    this due to the convoluted structure in libfc, but let's be safe for now.

    Signed-off-by: Christoph Hellwig
    Cc: Mark Rustad
    Cc: Kiran Patil
    Signed-off-by: Nicholas Bellinger

    Christoph Hellwig
     
  • All three callers of transport_generic_handle_data are from user context
    and can use target_execute_cmd directly to handle the backend I/O submission
    of WRITE I/O.

    Signed-off-by: Christoph Hellwig
    Cc: Andy Grover
    Signed-off-by: Nicholas Bellinger

    Christoph Hellwig
     
  • Signed-off-by: Christoph Hellwig
    Signed-off-by: Nicholas Bellinger

    Christoph Hellwig
     
  • When we call target_execute_cmd for write commands the command has been
    on the state list before an abort might have come in before
    target_execute_cmd. Call transport_check_aborted_status to deal with
    this case.

    Signed-off-by: Christoph Hellwig
    Signed-off-by: Nicholas Bellinger

    Christoph Hellwig
     
  • Just call target_execute_cmd directly. Also, convert loopback, sbp,
    usb-gadget to use the newly exported target_execute_cmd().

    Signed-off-by: Christoph Hellwig
    Signed-off-by: Nicholas Bellinger

    Christoph Hellwig
     
  • Inline the transport_off == 0 case into target_execute_cmd to simplify
    the function for the remaining cases.

    Signed-off-by: Christoph Hellwig
    Signed-off-by: Nicholas Bellinger

    Christoph Hellwig
     
  • Existing lio_dump.py code expects this to be in place for /iscsi.

    Revert for now to avoid userspace breakage in lio-utils

    This reverts commit fd88a785f9ac5d6be437c528571ccd85cdf2d493.

    Signed-off-by: Nicholas Bellinger

    Nicholas Bellinger
     
  • Having all the unmap payload parsing in the backed is a bit ugly, but until
    more drivers support it and we can find a good interface for all of them
    that seems the way to go.

    Signed-off-by: Christoph Hellwig
    Signed-off-by: Nicholas Bellinger

    Christoph Hellwig
     
  • Add spc_ops->execute_write_same() caller for ->execute_cmd() setup,
    and update IBLOCK backends to use it.

    (nab: add export of spc_get_write_same_sectors symbol)
    (roland: Carry forward: Fix range calculation in WRITE SAME emulation
    when num blocks == 0)

    Signed-off-by: Christoph Hellwig
    Signed-off-by: Nicholas Bellinger

    Christoph Hellwig
     
  • Add spc_ops->execute_sync_cache() caller for ->execute_cmd() setup,
    and update IBLOCK + FILEIO backends to use it.

    Signed-off-by: Christoph Hellwig
    Signed-off-by: Nicholas Bellinger

    Christoph Hellwig
     
  • Remove the execute_cmd method in struct se_subsystem_api, and always use the
    one directly in struct se_cmd. To make life simpler for SBC virtual backends
    a struct spc_ops that is passed to sbc_parse_cmd is added. For now it
    only contains an execute_rw member, but more will follow with the subsequent
    commits.

    Signed-off-by: Christoph Hellwig
    Signed-off-by: Nicholas Bellinger

    Christoph Hellwig
     
  • Remove the dead SCF_SE_ALLOW_EOO and SCF_DELAYED_CMD_FROM_SAM_ATTR
    from se_cmd_flags_table.

    Signed-off-by: Christoph Hellwig
    Signed-off-by: Nicholas Bellinger

    Christoph Hellwig
     
  • It's got no callers...

    Signed-off-by: Roland Dreier
    Signed-off-by: Nicholas Bellinger

    Roland Dreier
     
  • see https://bugzilla.redhat.com/show_bug.cgi?id=818855

    Adds a parameter so read-only block devices may be registered as
    LIO backstores.

    Signed-off-by: Andy Grover
    Signed-off-by: Nicholas Bellinger

    Andy Grover
     
  • These modules, along with other fabrics, should be loaded as-needed by
    the LIO userspace tools.

    Signed-off-by: Andy Grover
    Signed-off-by: Nicholas Bellinger

    Andy Grover