This patch fixes a regression bug introduced in v3.9-rc1 where if the
underlying struct block_device for a IBLOCK backend is configured with
WCE=1 + DPOFUA=1 settings, the rw = WRITE assignment no longer occurs
in iblock_execute_rw(), and rw = 0 is passed to iblock_submit_bios()
in effect causing a READ bio operation to occur.
The offending commit is:
commit d0c8b259f8
Author: Nicholas Bellinger <nab@linux-iscsi.org>
Date: Tue Jan 29 22:10:06 2013 -0800
target/iblock: Use backend REQ_FLUSH hint for WriteCacheEnabled status
Note the WCE=1 + DPOFUA=0, WCE=0 + DPOFUA=1, and WCE=0 + DPOFUA=0 cases
are not affected by this regression bug.
Reported-by: Chris Boot <bootc@bootc.net>
Tested-by: Chris Boot <bootc@bootc.net>
Reported-by: Hannes Reinecke <hare@suse.de>
Cc: <stable@vger.kernel.org>
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
iblock_execute_unmap() and fd_execute_unmap share a lot of code.
Add sbc_execute_unmap() helper to remove duplicated code for
iblock_execute_unmap() and fd_execute_unmap().
Cc: Christoph Hellwig <hch@lst.de>
Cc: Martin K. Petersen <martin.petersen@oracle.com>
Cc: Nicholas Bellinger <nab@linux-iscsi.org>
Signed-off-by: Asias He <asias@redhat.com>
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
Add helper iblock_do_unmap() to remove duplicated code in
iblock_execute_write_same_unmap() and iblock_execute_unmap().
Cc: Christoph Hellwig <hch@lst.de>
Cc: Martin K. Petersen <martin.petersen@oracle.com>
Cc: Nicholas Bellinger <nab@linux-iscsi.org>
Signed-off-by: Asias He <asias@redhat.com>
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
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 <asias@redhat.com>
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
Trivial, but WRITE SAME is an SBC command so it seems strange for a
related function (defined in target_core_sbc.c) to be in the spc_
namespace.
Signed-off-by: Roland Dreier <roland@purestorage.com>
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
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 <roland@purestorage.com>
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
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 <majianpeng@gmail.com>
Cc: majianpeng <majianpeng@gmail.com>
Cc: Christoph Hellwig <hch@infradead.org>
Cc: Jens Axboe <axboe@kernel.dk>
Cc: James Bottomley <JBottomley@Parallels.com>
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
We already expect TFO->get_blocks() to return sector_t for zero value case
when doing WRITE_SAME to the end of the backend device, so go ahead and return
sector_t from spc_get_write_same_sectors() to handle this case properly.
Also, update the single iblock_execute_write_same() caller of this code.
Cc: Christoph Hellwig <hch@lst.de>
Cc: Martin K. Petersen <martin.petersen@oracle.com>
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
Go ahead and forward declare the handful of helper functions required
for bio submission code in order to avoid the extra function prototypes.
Reported-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
v2: Use correct target_core_stat.c 2006 copyright year
v3: Drop extra unnessary legal verbage from header (hch)
Cc: Christoph Hellwig <hch@lst.de>
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
This patch adds support for emulation of WRITE_SAME w/ UNMAP=0 within
iblock_execute_write_same() backend code.
The emulation uses a bio_add_page() call for each sector, and by default
enforces a limit of max_write_same_len=0xFFFF (65536) sectors following
what scsi_debug reports per default for MAXIMUM WRITE SAME LENGTH.
It also sets max_write_same_len to the operational default at setup ->
iblock_configure_device() time.
(hch: Move unmap logic into iblock_execute_write_same_unmap + add
check for single sector SGLs in iblock_execute_write_same)
(mkp: Update comment for 0xFFFF magic constant)
(nab: drop left-over max_write_same_len check in iblock_execute_write_same)
Cc: Christoph Hellwig <hch@lst.de>
Cc: Martin K. Petersen <martin.petersen@oracle.com>
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
Pass the sense reason as an explicit return value from the I/O submission
path instead of storing it in struct se_cmd and using negative return
values. This cleans up a lot of the code pathes, and with the sparse
annotations for the new sense_reason_t type allows for much better
error checking.
(nab: Convert spc_emulate_modesense + spc_emulate_modeselect to use
sense_reason_t with Roland's MODE SELECT changes)
Signed-off-by: Christoph Hellwig <hch@lst.de>
Cc: Roland Dreier <roland@purestorage.com>
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
Now that the reservations and ALUA code have been cleaned up there is no need
for the get_device_rev method, as we only need the standards revision in the
inquiry data, where we can hardcode it.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
These really are sbc_ops, so name them correctly.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
Simplify the code a lot by killing the superflous struct se_subsystem_dev.
Instead se_device is allocated early on by the backend driver, which allocates
it as part of its own per-device structure, borrowing the scheme that is for
example used for inode allocation.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
Pull block IO update from Jens Axboe:
"Core block IO bits for 3.7. Not a huge round this time, it contains:
- First series from Kent cleaning up and generalizing bio allocation
and freeing.
- WRITE_SAME support from Martin.
- Mikulas patches to prevent O_DIRECT crashes when someone changes
the block size of a device.
- Make bio_split() work on data-less bio's (like trim/discards).
- A few other minor fixups."
Fixed up silent semantic mis-merge as per Mikulas Patocka and Andrew
Morton. It is due to the VM no longer using a prio-tree (see commit
6b2dbba8b6ac: "mm: replace vma prio_tree with an interval tree").
So make set_blocksize() use mapping_mapped() instead of open-coding the
internal VM knowledge that has changed.
* 'for-3.7/core' of git://git.kernel.dk/linux-block: (26 commits)
block: makes bio_split support bio without data
scatterlist: refactor the sg_nents
scatterlist: add sg_nents
fs: fix include/percpu-rwsem.h export error
percpu-rw-semaphore: fix documentation typos
fs/block_dev.c:1644:5: sparse: symbol 'blkdev_mmap' was not declared
blockdev: turn a rw semaphore into a percpu rw semaphore
Fix a crash when block device is read and block size is changed at the same time
block: fix request_queue->flags initialization
block: lift the initial queue bypass mode on blk_register_queue() instead of blk_init_allocated_queue()
block: ioctl to zero block ranges
block: Make blkdev_issue_zeroout use WRITE SAME
block: Implement support for WRITE SAME
block: Consolidate command flag and queue limit checks for merges
block: Clean up special command handling logic
block/blk-tag.c: Remove useless kfree
block: remove the duplicated setting for congestion_threshold
block: reject invalid queue attribute values
block: Add bio_clone_bioset(), bio_clone_kmalloc()
block: Consolidate bio_alloc_bioset(), bio_kmalloc()
...
scsi_setup_fs_cmnd does not like to receive requests with no
bios attached to it. Special-case zero-length reads and writes,
by not submitting any bio.
Testcase: sg_raw /dev/sdb 28 00 00 00 00 00 00 00 00 00
should not fail
panics with the rest of the series but not this patch
behaves correctly without or with this series
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
This patch drops se_subsystem_api->[write_cache,fua_write]_emulated flags
set by viritual FILEIO/IBLOCK/RD_MCP backend drivers in favor of explict
TRANSPORT_PLUGIN_PHBA_PDEV checks to know when to fail if userspace is
attempting to set virtual emulation bits for an pSCSI (passthrough)
backend device.
Reported-by: Christoph Hellwig <hch@lst.de>
Cc: Christoph Hellwig <hch@lst.de>
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
Following commit dbc6e0222 from Al Viro for fileio, go ahead and make
Opt_udev_path within iblock_set_configfs_dev_params use match_strlcpy
instead of the match_strdup -> snprintf -> kfree equivalent.
Reported-by: Al Viro <viro@zeniv.linux.org.uk>
Cc: Al Viro <viro@zeniv.linux.org.uk>
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
With the old code, when you allocate a bio from a bio pool you have to
implement your own destructor that knows how to find the bio pool the
bio was originally allocated from.
This adds a new field to struct bio (bi_pool) and changes
bio_alloc_bioset() to use it. This makes various bio destructors
unnecessary, so they're then deleted.
v6: Explain the temporary if statement in bio_put
Signed-off-by: Kent Overstreet <koverstreet@google.com>
CC: Jens Axboe <axboe@kernel.dk>
CC: NeilBrown <neilb@suse.de>
CC: Alasdair Kergon <agk@redhat.com>
CC: Nicholas Bellinger <nab@linux-iscsi.org>
CC: Lars Ellenberg <lars.ellenberg@linbit.com>
Acked-by: Tejun Heo <tj@kernel.org>
Acked-by: Nicholas Bellinger <nab@linux-iscsi.org>
Signed-off-by: Jens Axboe <axboe@kernel.dk>
Several places were not checking that the parameter list length
was large enough, and thus accessing invalid memory. Zero-length
parameter lists are just a special case of this.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
Fail UNMAP commands that have more than our reported limit on unmap
descriptors.
Signed-off-by: Roland Dreier <roland@purestorage.com>
Cc: stable@vger.kernel.org
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
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 <roland@purestorage.com>
Cc: stable@vger.kernel.org
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
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 <roland@purestorage.com>
Cc: stable@vger.kernel.org
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
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 <roland@purestorage.com>
Cc: stable@vger.kernel.org
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
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 <hch@lst.de>
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
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 <hch@lst.de>
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
Add spc_ops->execute_sync_cache() caller for ->execute_cmd() setup,
and update IBLOCK + FILEIO backends to use it.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
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 <hch@lst.de>
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
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 <agrover@redhat.com>
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
Instead of trying to handle all SCSI command sets in one function
(transport_generic_cmd_sequencer) call out to the backend driver to perform
this functionality. For pSCSI a copy of the existing code is used, but for
all virtual backends we can use a new parse_sbc_cdb helper is used to
provide a simple SBC emulation.
For now this setups means a fair amount of duplication between pSCSI and the
SBC library, but patches later in this series will sort out that problem.
(nab: Fix up build failure in target_core_pscsi.c)
Signed-off-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
We can use struct se_cmd for everything it did. Make sure to pass the S/G
list and data direction to the execution function to ease adding back BIDI
support later on.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
Make CDB emulation work on commands instead of tasks again as a preparation
of removing tasks completely.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
Now that we don't split commands the size field in the task is always
equivalent to the one in the CDB, even in cases where we have two tasks
due to a BIDI transfer. Just refer the the size in the command instead
of duplicating it in the task.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
Now that we don't split commands the lba field in the task is always
equivalent to the one in the CDB, even in cases where we have two tasks
due to a BIDI transfer. Just refer the the lba in the command instead
of duplicating it in the task.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
There is no real limit for task sizes in the iblock driver given that we
can chain bios. Increase the maximum size to UINT_MAX, and change the
code to submit bios in a smaller batch size to avoid deadlocks when
having more bios in flight than the pool supports. Also increase the
pool size to always allow multiple tasks to be in flight.
I also had to change the task refcounting to include one reference for
the submission task, which is a standard practice in this kind of code
in Linux (e.g. XFS I/O submission). This was wrong before, but couldn't
be hit easily.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
There is no reason to allocate a struct just to store the host number for
a debug printk in the detach path. I've simply removed the verbose
debugging given that the calling code thinks the number passed in is
something different from a host ID anyway.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
Requesting to many bvecs upsets bio_alloc_bioset, so limit the number we ask
for to the amount it can handle.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
The block layer keeps q->limits.discard_granularity in bytes, but iblock
(and the SCSI Block Limits VPD page) keep unmap_granularity in blocks.
Report the correct value when exporting block devices by dividing to
convert bytes to blocks.
Signed-off-by: Roland Dreier <roland@purestorage.com>
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
We never embedd the bio into a structure, so there is no need to allocate
64 bytes of headroom per bio.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
A reader should spend an extra moment whenever noticing a cast,
because either something special is going on that deserves extra
attention or, as is all too often the case, the code is wrong.
These casts, afaics, have all been useless. They cast a foo* to a
foo*, cast a void* to the assigned type, cast a foo* to void*, before
assigning it to a void* variable, etc.
In a few cases I also removed an additional &...[0], which is equally
useless.
Lastly I added three FIXMEs where, to the best of my judgement, the
code appears to have a bug. It would be good if someone could check
these.
Signed-off-by: Joern Engel <joern@logfs.org>
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
The command
| echo rd_pages=32768 > ramdisk/control
Does not work because it writes "rd_pages=32768\n" and the parser which
matches for "rd_pages=%d" does not recognize it due to the \n. One way
of fixing this would be using "echo -n" instead.
This patch adds \n to the list of separators so we don't have to use the
-n argument which I find is more convinient.
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
This reorganized the headers under include/target into:
- target_core_base.h stays as is with all target-wide data stuctures and defines
- target_core_backend.h contains the whole interface to I/O backends
- target_core_fabric.h contains the whole interface to fabric modules
Except for those only the various configfs macro headers stay around.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
This patch removes legacy usage of PYX_TRANSPORT_* return codes in a number
of locations and addresses cases where transport_generic_request_failure()
was returning the incorrect sense upon CHECK_CONDITION status after the
v3.1 converson to use errno return codes.
This includes the conversion of transport_generic_request_failure() to
process cmd->scsi_sense_reason and handle extra TCM_RESERVATION_CONFLICT
before calling transport_send_check_condition_and_sense() to queue up
response status. It also drops PYX_TRANSPORT_OUT_OF_MEMORY_RESOURCES legacy
usgae, and returns TCM_LOGICAL_UNIT_COMMUNICATION_FAILURE w/ a response
for these cases.
transport_generic_allocate_tasks(), transport_generic_new_cmd(), backend
SCF_SCSI_DATA_SG_IO_CDB ->do_task(), and emulated ->execute_task() have
all been updated to set se_cmd->scsi_sense_reason and return errno codes
universally upon failure. This includes cmd->scsi_sense_reason assignment
in target_core_alua.c, target_core_pr.c and target_core_cdb.c emulation code.
Finally it updates fabric modules to remove the legacy usage, and for
TFO->new_cmd_map() callers forwards return values outside of fabric code.
iscsi-target has also been updated to remove a handful of special cases
related to the cleanup and signaling QUEUE_FULL handling w/ ft_write_pending()
(v2: Drop extra SCF_SCSI_CDB_EXCEPTION check during failure from
transport_generic_new_cmd, and re-add missing task->task_error_status
assignment in transport_complete_task)
Cc: Christoph Hellwig <hch@lst.de>
Cc: stable@kernel.org
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
Remove the dpo_emulated, fua_write_emulated, fua_read_emulated and
write_cache_emulated methods, and replace them with a simple bitfields in
se_subsystem_api in those cases where they ever returned one.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
Do not block the submitting thread when handling a SYNCHRONIZE CACHE command,
but implement it asynchronously by sending the FLUSH command ourself and
calling transport_complete_sync_cache from the completion handler.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
The most commonly used file, iblock and rd backends have no use for
a per-task CDB and thus don't need a method to copy it into their
otherwise unused CDB fields.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
Nicholas Bellinger