03 Mar, 2013

1 commit

  • Pull SCSI target patches from Nicholas Bellinger:
    "Here are the remaining target-pending patches for v3.9-rc1.

    The most important one here is the immediate queue starvation
    regression fix for iscsi-target, which addresses a bug that's
    effecting v3.5+ kernels under heavy sustained READ only workloads.
    Thanks alot to Benjamin Estrabaud for helping to track this down!

    Also included is a pSCSI exception bugfix from Asias, along with a
    handful of other minor changes. Both bugfixes are CC'ed to stable."

    * 'for-next' of git://git.kernel.org/pub/scm/linux/kernel/git/nab/target-pending:
    target/pscsi: Rename sg_num to nr_vecs in pscsi_get_bio()
    target/pscsi: Fix page increment
    target/pscsi: Drop unnecessary NULL assignment to bio->bi_next
    target: Add __exit annotation for module_exit functions
    iscsi-target: Fix immediate queue starvation regression with DATAIN

    Linus Torvalds
     

28 Feb, 2013

7 commits

  • It is actually a vector not a sg, so nr_vecs is better than sg_num.

    Signed-off-by: Asias He
    Signed-off-by: Nicholas Bellinger

    Asias He
     
  • The page++ is wrong. It makes bio_add_pc_page() pointing to a wrong page
    address if the 'while (len > 0 && data_len > 0) { ... }' loop is
    executed more than one once.

    Signed-off-by: Asias He
    Cc:
    Signed-off-by: Nicholas Bellinger

    Asias He
     
  • Signed-off-by: Asias He
    Signed-off-by: Nicholas Bellinger

    Asias He
     
  • Inclues sbp_exit, fileio_module_exit, iblock_module_exit and
    pscsi_module_exit.

    Note: rd_module_exit() can not be annotated by __exit, becasue it is
    called by target_core_init_configfs() which is annotated by __init.

    Signed-off-by: Asias He
    Signed-off-by: Nicholas Bellinger

    Asias He
     
  • I'm not sure why, but the hlist for each entry iterators were conceived

    list_for_each_entry(pos, head, member)

    The hlist ones were greedy and wanted an extra parameter:

    hlist_for_each_entry(tpos, pos, head, member)

    Why did they need an extra pos parameter? I'm not quite sure. Not only
    they don't really need it, it also prevents the iterator from looking
    exactly like the list iterator, which is unfortunate.

    Besides the semantic patch, there was some manual work required:

    - Fix up the actual hlist iterators in linux/list.h
    - Fix up the declaration of other iterators based on the hlist ones.
    - A very small amount of places were using the 'node' parameter, this
    was modified to use 'obj->member' instead.
    - Coccinelle didn't handle the hlist_for_each_entry_safe iterator
    properly, so those had to be fixed up manually.

    The semantic patch which is mostly the work of Peter Senna Tschudin is here:

    @@
    iterator name hlist_for_each_entry, hlist_for_each_entry_continue, hlist_for_each_entry_from, hlist_for_each_entry_rcu, hlist_for_each_entry_rcu_bh, hlist_for_each_entry_continue_rcu_bh, for_each_busy_worker, ax25_uid_for_each, ax25_for_each, inet_bind_bucket_for_each, sctp_for_each_hentry, sk_for_each, sk_for_each_rcu, sk_for_each_from, sk_for_each_safe, sk_for_each_bound, hlist_for_each_entry_safe, hlist_for_each_entry_continue_rcu, nr_neigh_for_each, nr_neigh_for_each_safe, nr_node_for_each, nr_node_for_each_safe, for_each_gfn_indirect_valid_sp, for_each_gfn_sp, for_each_host;

    type T;
    expression a,c,d,e;
    identifier b;
    statement S;
    @@

    -T b;

    [akpm@linux-foundation.org: drop bogus change from net/ipv4/raw.c]
    [akpm@linux-foundation.org: drop bogus hunk from net/ipv6/raw.c]
    [akpm@linux-foundation.org: checkpatch fixes]
    [akpm@linux-foundation.org: fix warnings]
    [akpm@linux-foudnation.org: redo intrusive kvm changes]
    Tested-by: Peter Senna Tschudin
    Acked-by: Paul E. McKenney
    Signed-off-by: Sasha Levin
    Cc: Wu Fengguang
    Cc: Marcelo Tosatti
    Cc: Gleb Natapov
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    Sasha Levin
     
  • Convert to the much saner new idr interface.

    Signed-off-by: Tejun Heo
    Cc: Nicholas A. Bellinger
    Cc: James Bottomley
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    Tejun Heo
     
  • This patch addresses a v3.5+ regression in iscsi-target where TX thread
    process context -> handle_response_queue() execution is allowed to run
    unbounded while servicing constant outgoing flow of ISTATE_SEND_DATAIN
    response state.

    This ends up preventing memory release of StatSN acknowledged commands
    in a timely manner when under heavy large block streaming DATAIN
    workloads.

    The regression bug was initially introduced with:

    commit 6f3c0e69a9c20441bdc6d3b2d18b83b244384ec6
    Author: Andy Grover
    Date: Tue Apr 3 15:51:09 2012 -0700

    target/iscsi: Refactor target_tx_thread immediate+response queue loops

    Go ahead and follow original iscsi_target_tx_thread() logic and check
    to break for immediate queue processing after each DataIN Sequence and/or
    Response PDU has been sent.

    Reported-by: Benjamin ESTRABAUD
    Cc: Andy Grover
    Cc:
    Signed-off-by: Nicholas Bellinger

    Nicholas Bellinger
     

27 Feb, 2013

2 commits

  • Pull vfs pile (part one) from Al Viro:
    "Assorted stuff - cleaning namei.c up a bit, fixing ->d_name/->d_parent
    locking violations, etc.

    The most visible changes here are death of FS_REVAL_DOT (replaced with
    "has ->d_weak_revalidate()") and a new helper getting from struct file
    to inode. Some bits of preparation to xattr method interface changes.

    Misc patches by various people sent this cycle *and* ocfs2 fixes from
    several cycles ago that should've been upstream right then.

    PS: the next vfs pile will be xattr stuff."

    * 'for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/viro/vfs: (46 commits)
    saner proc_get_inode() calling conventions
    proc: avoid extra pde_put() in proc_fill_super()
    fs: change return values from -EACCES to -EPERM
    fs/exec.c: make bprm_mm_init() static
    ocfs2/dlm: use GFP_ATOMIC inside a spin_lock
    ocfs2: fix possible use-after-free with AIO
    ocfs2: Fix oops in ocfs2_fast_symlink_readpage() code path
    get_empty_filp()/alloc_file() leave both ->f_pos and ->f_version zero
    target: writev() on single-element vector is pointless
    export kernel_write(), convert open-coded instances
    fs: encode_fh: return FILEID_INVALID if invalid fid_type
    kill f_vfsmnt
    vfs: kill FS_REVAL_DOT by adding a d_weak_revalidate dentry op
    nfsd: handle vfs_getattr errors in acl protocol
    switch vfs_getattr() to struct path
    default SET_PERSONALITY() in linux/elf.h
    ceph: prepopulate inodes only when request is aborted
    d_hash_and_lookup(): export, switch open-coded instances
    9p: switch v9fs_set_create_acl() to inode+fid, do it before d_instantiate()
    9p: split dropping the acls from v9fs_set_create_acl()
    ...

    Linus Torvalds
     
  • Pull scsi target updates from Nicholas Bellinger:
    "The highlights in this series include:

    - Improve sg_table lookup scalability in RAMDISK_MCP (martin)

    - Add device attribute to expose config name for INQUIRY model (tregaron)

    - Convert tcm_vhost to use lock-less list for cmd completion (asias)

    - Add tcm_vhost support for multiple target's per endpoint (asias)

    - Add tcm_vhost support for multiple queues per vhost (asias)

    - Add missing mapped_lun bounds checking during make_mappedlun setup
    in generic fabric configfs code (jan engelhardt + nab)

    - Enforce individual iscsi-target network portal export once per
    TargetName endpoint (grover + nab)

    - Add WRITE_SAME w/ UNMAP=0 emulation to FILEIO backend (nab)

    Things have been mostly quiet this round, with majority of the work
    being done on the iser-target WIP driver + associated iscsi-target
    refactoring patches currently in flight for v3.10 code.

    At this point there is one patch series left outstanding from Asias to
    add support for UNMAP + WRITE_SAME w/ UNMAP=1 to FILEIO awaiting
    feedback from hch & Co, that will likely be included in a post
    v3.9-rc1 PULL request if there are no objections.

    Also, there is a regression bug recently reported off-list that seems
    to be effecting v3.5 and v3.6 kernels with MSFT iSCSI initiators that
    is still being tracked down. No word if this effects >= v3.7 just
    yet, but if so there will likely another PULL request coming your
    way.."

    * 'for-next' of git://git.kernel.org/pub/scm/linux/kernel/git/nab/target-pending: (26 commits)
    target: Rename spc_get_write_same_sectors -> sbc_get_write_same_sectors
    target/file: Add WRITE_SAME w/ UNMAP=0 emulation support
    iscsi-target: Enforce individual network portal export once per TargetName
    iscsi-target: Refactor iscsit_get_np sockaddr matching into iscsit_check_np_match
    target: Add missing mapped_lun bounds checking during make_mappedlun setup
    target: Fix lookup of dynamic NodeACLs during cached demo-mode operation
    target: Fix parameter list length checking in MODE SELECT
    target: Fix error checking for UNMAP commands
    target: Fix sense data for out-of-bounds IO operations
    target_core_rd: break out unterminated loop during copy
    tcm_vhost: Multi-queue support
    tcm_vhost: Multi-target support
    target: Add device attribute to expose config_item_name for INQUIRY model
    target: don't truncate the fail intr address
    target: don't always say "ipv6" as address type
    target/iblock: Use backend REQ_FLUSH hint for WriteCacheEnabled status
    iscsi-target: make some temporary buffers larger
    tcm_vhost: Optimize gup in vhost_scsi_map_to_sgl
    tcm_vhost: Use iov_num_pages to calculate sgl_count
    tcm_vhost: Introduce iov_num_pages
    ...

    Linus Torvalds
     

26 Feb, 2013

1 commit


24 Feb, 2013

1 commit


23 Feb, 2013

1 commit


22 Feb, 2013

1 commit

  • Pull trivial tree from Jiri Kosina:
    "Assorted tiny fixes queued in trivial tree"

    * 'for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/jikos/trivial: (22 commits)
    DocBook: update EXPORT_SYMBOL entry to point at export.h
    Documentation: update top level 00-INDEX file with new additions
    ARM: at91/ide: remove unsused at91-ide Kconfig entry
    percpu_counter.h: comment code for better readability
    x86, efi: fix comment typo in head_32.S
    IB: cxgb3: delay freeing mem untill entirely done with it
    net: mvneta: remove unneeded version.h include
    time: x86: report_lost_ticks doesn't exist any more
    pcmcia: avoid static analysis complaint about use-after-free
    fs/jfs: Fix typo in comment : 'how may' -> 'how many'
    of: add missing documentation for of_platform_populate()
    btrfs: remove unnecessary cur_trans set before goto loop in join_transaction
    sound: soc: Fix typo in sound/codecs
    treewide: Fix typo in various drivers
    btrfs: fix comment typos
    Update ibmvscsi module name in Kconfig.
    powerpc: fix typo (utilties -> utilities)
    of: fix spelling mistake in comment
    h8300: Fix home page URL in h8300/README
    xtensa: Fix home page URL in Kconfig
    ...

    Linus Torvalds
     

21 Feb, 2013

1 commit

  • This patch adds support for emulation of WRITE_SAME w/ UNMAP=0 within
    fd_execute_write_same() backend code.

    The emulation uses vfs_writev() to submit a locally populated buffer
    from the received WRITE_SAME scatterlist block for duplication, and by
    default enforces a limit of max_write_same_len=0x1000 (8192) sectors up
    to the limit of 1024 iovec entries for the single call to vfs_writev().

    It also sets max_write_same_len to the operational default at setup ->
    fd_configure_device() time.

    Tested with 512, 1k, 2k, and 4k block_sizes.

    (asias: convert to vzalloc)

    Cc: Martin K. Petersen
    Cc: Christoph Hellwig
    Cc: Asias He
    Signed-off-by: Nicholas Bellinger

    Nicholas Bellinger
     

20 Feb, 2013

2 commits


19 Feb, 2013

2 commits

  • This patch adds missing bounds checking for the configfs provided
    mapped_lun value during target_fabric_make_mappedlun() setup ahead
    of se_lun_acl initialization.

    This addresses a potential OOPs when using a mapped_lun value that
    exceeds the hardcoded TRANSPORT_MAX_LUNS_PER_TPG-1 value within
    se_node_acl->device_list[].

    Reported-by: Jan Engelhardt
    Cc: Jan Engelhardt
    Cc:
    Signed-off-by: Nicholas Bellinger

    Nicholas Bellinger
     
  • This patch fixes a bug in core_tpg_check_initiator_node_acl() ->
    core_tpg_get_initiator_node_acl() where a dynamically created
    se_node_acl generated during session login would be skipped during
    subsequent lookup due to the '!acl->dynamic_node_acl' check, causing
    a new se_node_acl to be created with a duplicate ->initiatorname.

    This would occur when a fabric endpoint was configured with
    TFO->tpg_check_demo_mode()=1 + TPF->tpg_check_demo_mode_cache()=1
    preventing the release of an existing se_node_acl during se_session
    shutdown.

    Also, drop the unnecessary usage of core_tpg_get_initiator_node_acl()
    within core_dev_init_initiator_node_lun_acl() that originally
    required the extra '!acl->dynamic_node_acl' check, and just pass
    the configfs provided se_node_acl pointer instead.

    Cc:
    Signed-off-by: Nicholas Bellinger

    Nicholas Bellinger
     

14 Feb, 2013

13 commits

  • An empty parameter list (length == 0) is not an error, so succeed MODE
    SELECT in this case. If the parameter list length is too small,
    return the correct sense code of PARAMETER LIST LENGTH ERROR.

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

    Roland Dreier
     
  • SBC-3 (revision 35) says:

    The PARAMETER LIST LENGTH field specifies the length in bytes of the
    UNMAP parameter list that is available to be transferred from the
    Data-Out Buffer. If the parameter list length is greater than zero
    and less than 0008h (i.e., eight), then the device server shall
    terminate the command with CHECK CONDITION status with the sense key
    set to ILLEGAL REQUEST and the additional sense code set to
    PARAMETER LIST LENGTH ERROR. A PARAMETER LIST LENGTH set to zero
    specifies that no data shall be sent.

    so our sense code for too-short descriptors was wrong, and we were
    incorrectly failing commands that didn't transfer any descriptors.

    While we're at it, also handle the UNMAP check:

    If the ANCHOR bit is set to one, and the ANC_SUP bit in the Logical
    Block Provisioning VPD page (see 6.6.4) is set to zero, then the
    device server shall terminate the command with CHECK CONDITION
    status with the sense key set to ILLEGAL REQUEST and the additional
    sense code set to INVALID FIELD IN CDB.

    (chris boot: Fix wrong cut+paste comment in transport_send_check_condition_and_sense)

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

    Roland Dreier
     
  • We're supposed to return LOGICAL BLOCK ADDRESS OUT OF RANGE, not
    INVALID FIELD IN CDB.

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

    Roland Dreier
     
  • The loop in rd_execute_rw() will never terminate if the
    sg element has a zero size. Or it'll spill over into
    outer space if the sg element is larger than the available
    space.
    So we need to add some safety catches here.

    Cc: Nic Bellinger
    Signed-off-by: Hannes Reinecke
    Signed-off-by: Nicholas Bellinger

    Hannes Reinecke
     
  • This patch changes LIO to use the configfs backend device name as the
    model if you echo '1' to an individual device's emulate_model_alias attribute.
    This is a valid operation only on devices with an export count of 0.

    Signed-off-by: Tregaron Bayly
    Signed-off-by: Nicholas Bellinger

    Tregaron Bayly
     
  • The temporary buffer was only 32 characters but ->last_intr_fail_ip_addr
    is a 48 character buffer. We don't need to use a temporary buffer at
    all, we can just print directly to "page".

    Signed-off-by: Dan Carpenter
    Signed-off-by: Nicholas Bellinger

    Dan Carpenter
     
  • "lstat->last_intr_fail_ip_addr" is an array inside the "lstat" struct.
    It's never NULL so we always print "ipv6\n" here. The test should be
    "if (lstat->last_intr_fail_ip_family == AF_INET6)".

    We don't need the temporary buffer either. We could print directly into
    "page".

    Signed-off-by: Dan Carpenter
    Signed-off-by: Nicholas Bellinger

    Dan Carpenter
     
  • This patch allows IBLOCK to check block hints in request_queue->flush_flags
    when reporting current backend device WriteCacheEnabled status to a remote
    SCSI initiator port.

    This is done via a se_subsystem_api->get_write_cache() call instead of a
    backend se_device creation time flag, as we expect REQ_FLUSH bits to possibly
    change from an underlying blk_queue_flush() by the SCSI disk driver, or
    internal raw struct block_device driver usage.

    Also go ahead and update iblock_execute_rw() bio I/O path code to use
    REQ_FLUSH + REQ_FUA hints when determining WRITE_FUA usage, and make SPC
    emulation code use a spc_check_dev_wce() helper to handle both types of
    cases for virtual backend subsystem drivers.

    (asias: Drop unnecessary comparsion operators)

    Reported-by: majianpeng
    Cc: majianpeng
    Cc: Christoph Hellwig
    Cc: Jens Axboe
    Cc: James Bottomley
    Signed-off-by: Nicholas Bellinger

    Nicholas Bellinger
     
  • My static checker complains because we use sprintf() to print some
    unsigned ints into 10 byte buffers. In theory unsigned ints can take 10
    characters and we need another for the terminator.

    Signed-off-by: Dan Carpenter
    Signed-off-by: Nicholas Bellinger

    Dan Carpenter
     
  • "buf" is 128 characters and "vpd->device_identifier" is 256. It makes
    the static checkers complain.

    Also bump VPD_TMP_BUF_SIZE to match INQUIRY_VPD_DEVICE_IDENTIFIER_LEN.

    Signed-off-by: Dan Carpenter
    Signed-off-by: Nicholas Bellinger

    Dan Carpenter
     
  • Sequential scan of rd_dev->sg_table_array in rd_get_sg_table is
    a serious I/O performance bottleneck for large rd LUNs. Fix this
    by computing the sg_table index directly from page offset because
    all sg_tables (except the last one) have the same number of pages.

    Tested with 90 GiB rd_mcp LUN, where the patch improved maximal
    random R/W IOPS by more than 100-150%, depending on actual
    hardware and SAN setup.

    Signed-off-by: Martin Svec
    Signed-off-by: Nicholas Bellinger

    Martin Svec
     
  • We do the same thing no matter which way the test goes, so just remove
    the test and do what we're going to do.

    The debug messages printed the wrong value of CMD_T_ACTIVE and don't
    seem particularly useful, remove them too.

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

    Roland Dreier
     
  • Some target drivers might need to access the inquiry data
    directly, without sending out the actual command.
    So export these functions.

    Signed-off-by: Hannes Reinecke
    Cc: Nicholas Bellinger
    Signed-off-by: Nicholas Bellinger

    Hannes Reinecke
     

01 Feb, 2013

2 commits

  • This patch fixes a possible divide by zero bug when the fabric_max_sectors
    device attribute is written and backend se_device failed to be successfully
    configured -> enabled.

    Go ahead and use block_size=512 within se_dev_set_fabric_max_sectors()
    in the event of a target_configure_device() failure case, as no valid
    dev->dev_attrib.block_size value will have been setup yet.

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

    Nicholas Bellinger
     
  • This patch fixes a v3.8-rc1 regression bug where an unconfigured se_device
    was incorrectly allowed to perform a fabric port-link. This bug was
    introduced in commit:

    commit 0fd97ccf45be26fb01b3a412f1f6c6b5044b2f16
    Author: Christoph Hellwig
    Date: Mon Oct 8 00:03:19 2012 -0400

    target: kill struct se_subsystem_dev

    which ended up dropping the original se_subsystem_dev->se_dev_ptr check
    preventing this from happening with pre commit 0fd97ccf code.

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

    Nicholas Bellinger
     

30 Jan, 2013

3 commits

  • This patch fixes a regression introduced in v3.8-rc1 code where a
    zero-length READ_CAPACITY_16 was no longer returning GOOD status, but
    instead returning TCM_LOGICAL_UNIT_COMMUNICATION_FAILURE to generate
    a CHECK_CONDITION status.

    This regression was introduced with the following commit:

    commit de103c93aff0bed0ae984274e5dc8b95899badab
    Author: Christoph Hellwig
    Date: Tue Nov 6 12:24:09 2012 -0800

    target: pass sense_reason as a return value

    and this patch has been tested with the following zero-length CDB:

    sg_raw /dev/sdd 9e 10 00 00 00 00 00 00 00 00 00 00 00 00 00 00
    SCSI Status: Good

    Sense Information:
    sense buffer empty

    Also, convert sbc_emulate_readcapacity() to follow the same method
    of handling transport_kmap_data_sg() return values, but we never
    expect a zero-length request here.

    Cc: Christoph Hellwig
    Cc: Paolo Bonzini
    Cc: Roland Dreier
    Signed-off-by: Nicholas Bellinger

    Nicholas Bellinger
     
  • This patch fixes a regression introduced in v3.8-rc1 code where
    a zero-length MODE_SENSE was no longer returning GOOD status, but
    instead returning TCM_LOGICAL_UNIT_COMMUNICATION_FAILURE to generate
    a CHECK_CONDITION status.

    This regression was introduced with the following commit:

    commit de103c93aff0bed0ae984274e5dc8b95899badab
    Author: Christoph Hellwig
    Date: Tue Nov 6 12:24:09 2012 -0800

    target: pass sense_reason as a return value

    and this patch has been tested with the following zero-length CDB:

    sg_raw /dev/sdd 5a 00 0a 00 00 00 00 00 00 00
    SCSI Status: Good

    Sense Information:
    sense buffer empty

    Cc: Christoph Hellwig
    Cc: Paolo Bonzini
    Cc: Roland Dreier
    Signed-off-by: Nicholas Bellinger

    Nicholas Bellinger
     
  • This patch fixes a minor regression introduced in v3.8-rc1 code
    where a zero-length INQUIRY was no longer returning the correct
    INVALID FIELD IN CDB additional sense code.

    This regression was introduced with the following commit:

    commit de103c93aff0bed0ae984274e5dc8b95899badab
    Author: Christoph Hellwig
    Date: Tue Nov 6 12:24:09 2012 -0800

    target: pass sense_reason as a return value

    and this patch has been tested with the following zero-length CDB:

    sg_raw /dev/sdd 12 00 83 00 00 00
    SCSI Status: Check Condition

    Sense Information:
    Fixed format, current; Sense key: Illegal Request
    Additional sense: Invalid field in cdb

    Cc: Christoph Hellwig
    Cc: Paolo Bonzini
    Cc: Roland Dreier
    Signed-off-by: Nicholas Bellinger

    Nicholas Bellinger
     

29 Jan, 2013

1 commit


11 Jan, 2013

2 commits

  • Commit 64c13330a389 ("iscsi-target: Fix bug in handling of ExpStatSN
    ACK during u32 wrap-around") introduced a bug where we compare the
    wrong SN against our ExpCmdSN.

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

    Roland Dreier
     
  • When transport_lookup_tmr_lun() fails and we return a task management
    response from target_complete_tmr_failure(), we need to call
    transport_cmd_check_stop_to_fabric() to release the last ref to the
    cmd after calling se_tfo->queue_tm_rsp(), or else we will never remove
    the failed TMR from the session command list (and we'll end up waiting
    forever when trying to tear down the session).

    (nab: Fix minor compile breakage)

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

    Roland Dreier