The code is currently riddled with "drop the hardware_lock to avoid a
deadlock" bugs that expose races. One of those races seems to expose a
valid warning in tcm_qla2xxx_clear_nacl_from_fcport_map. Add some
bandaid to it.
Signed-off-by: Joern Engel <joern@logfs.org>
Cc: <stable@vger.kernel.org> # v3.5+
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
If more than one thread is waiting for command ring space that includes
a PAD, then if the first one finishes (inserts a PAD and a CMD at the
start of the cmd ring) then the second one will incorrectly think it still
needs to insert a PAD (i.e. cmdr_space_needed is now wrong.) This will
lead to it asking for more space than it actually needs, and then inserting
a PAD somewhere else than at the end -- not what we want.
This patch moves the pad calculation inside is_ring_space_available() so
in the above scenario the second thread would then ask for space not
including a PAD. The patch also inserts a PAD op based upon an up-to-date
cmd_head, instead of the potentially stale value.
Signed-off-by: Andy Grover <agrover@redhat.com>
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
The SCSI command tag is set to the tag assigned from the block
layer, not the SCSI-II tag message. So we need to convert
it into the correct SCSI-II tag message based on the
device flags, not the tag value itself.
Signed-off-by: Hannes Reinecke <hare@suse.de>
Reviewed-by: Sagi Grimberg <sagig@mellanox.com>
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
Unused return value from down_interruptible
Reported-by: Or Gerlitz <ogerlitz@mellanox.com>
Signed-off-by: Sagi Grimberg <sagig@mellanox.com>
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
This patch fixes up the following unused return smatch warnings:
drivers/target/target_core_user.c:778 tcmu_netlink_event warn: unused return: ret = nla_put_string()
drivers/target/target_core_user.c:780 tcmu_netlink_event warn: unused `return: ret = nla_put_u32()
(Fix up missing semicolon: grover)
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
Add a LIO storage engine that presents commands to userspace for execution.
This would allow more complex backstores to be implemented out-of-kernel,
and also make experimentation a-la FUSE (but at the SCSI level -- "SUSE"?)
possible.
It uses a mmap()able UIO device per LUN to share a command ring and data
area. The commands are raw SCSI CDBs and iovs for in/out data. The command
ring is also reused for returning scsi command status and optional sense
data.
This implementation is based on Shaohua Li's earlier version but heavily
modified. Differences include:
* Shared memory allocated by kernel, not locked-down user pages
* Single ring for command request and response
* Offsets instead of embedded pointers
* Generic SCSI CDB passthrough instead of per-cmd specialization in ring
format.
* Uses UIO device instead of anon_file passed in mailbox.
* Optional in-kernel handling of some commands.
The main reason for these differences is to permit greater resiliency
if the user process dies or hangs.
Things not yet implemented (on purpose):
* Zero copy. The data area is flexible enough to allow page flipping or
backend-allocated pages to be used by fabrics, but it's not clear these
are performance wins. Can come later.
* Out-of-order command completion by userspace. Possible to add by just
allowing userspace to change cmd_id in rsp cmd entries, but currently
not supported.
* No locks between kernel cmd submission and completion routines. Sounds
like it's possible, but this can come later.
* Sparse allocation of mmaped area. Current code vmallocs the whole thing.
If the mapped area was larger and not fully mapped then the driver would
have more freedom to change cmd and data area sizes based on demand.
Current code open issues:
* The use of idrs may be overkill -- we maybe can replace them with a
simple counter to generate cmd_ids, and a hash table to get a cmd_id's
associated pointer.
* Use of a free-running counter for cmd ring instead of explicit modulo
math. This would require power-of-2 cmd ring size.
(Add kconfig depends NET - Randy)
Signed-off-by: Andy Grover <agrover@redhat.com>
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
Describes the driver and its interface to make it possible for user
programs to back a LIO-exported LUN.
Thanks to Richard W. M. Jones for review, and supplementing this doc
with the first two paragraphs.
Signed-off-by: Andy Grover <agrover@redhat.com>
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
In order to prevent a O(n) search of the filesystem to link up its uio
node with its target configuration, TCMU needs to know the minor number
that UIO assigned. Expose the definition of this struct so TCMU can
access this field.
Signed-off-by: Andy Grover <agrover@redhat.com>
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
The check of SCF_SCSI_DATA_CDB seems to be a remnant from before hch's
refactoring of this function. There are no places where that flag is set
that cmd->execute_cmd isn't also set.
Signed-off-by: Andy Grover <agrover@redhat.com>
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
During temporary resource starvation at lower transport layer, command
is placed on queue full retry path, which expose this problem. The TCM
queue full handling of SCF_TRANSPORT_TASK_SENSE currently sends the same
cmd twice to lower layer. The 1st time led to cmd normal free path.
The 2nd time cause Null pointer access.
This regression bug was originally introduced v3.1-rc code in the
following commit:
commit e057f53308
Author: Christoph Hellwig <hch@infradead.org>
Date: Mon Oct 17 13:56:41 2011 -0400
target: remove the transport_qf_callback se_cmd callback
Signed-off-by: Quinn Tran <quinn.tran@qlogic.com>
Signed-off-by: Saurav Kashyap <saurav.kashyap@qlogic.com>
Cc: <stable@vger.kernel.org> # v3.1+
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
On most (non-x86) 64bit platforms this will remove 8 padding bytes
from the structure.
Signed-off-by: Joern Engel <joern@logfs.org>
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
Remove the inline attribute. Modern compilers ignore it and the
function has grown beyond where inline made sense anyway.
Remove the BUG_ON(!cmd->sg_mapped), and instead return if sg_mapped is
not set. Every caller is doing this check, so we might as well have it
in one place instead of four.
Signed-off-by: Joern Engel <joern@logfs.org>
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
Also removes the declarations from the header - including two
declarations without function definitions or callers.
Signed-off-by: Joern Engel <joern@logfs.org>
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
list_for_each_entry_safe is necessary if list objects are deleted from
the list while traversing it. Not the case here, so we can use the base
list_for_each_entry variant.
Signed-off-by: Joern Engel <joern@logfs.org>
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
The target code has a rather generous helping of smp_mb__after_atomic()
throughout the code base. Most atomic operations were followed by one
and none were preceded by smp_mb__before_atomic(), nor accompanied by a
comment explaining the need for a barrier.
Instead of trying to prove for every case whether or not it is needed,
this patch introduces atomic_inc_mb() and atomic_dec_mb(), which
explicitly include the memory barriers before and after the atomic
operation. For now they are defined in a target header, although they
could be of general use.
Most of the existing atomic/mb combinations were replaced by the new
helpers. In a few cases the atomic was sandwiched in
spin_lock/spin_unlock and I simply removed the barrier.
I suspect that in most cases the correct conversion would have been to
drop the barrier. I also suspect that a few cases exist where a) the
barrier was necessary and b) a second barrier before the atomic would
have been necessary and got added by this patch.
Signed-off-by: Joern Engel <joern@logfs.org>
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
atomic_inc_return() already does an implicit memory barrier and the
second case was moved from an atomic to a plain flag operation. If a
barrier were needed in the second case, it would have to be smp_mb(),
not a variant optimized away for x86 and other architectures.
Signed-off-by: Joern Engel <joern@logfs.org>
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
And while at it, do minimal coding style fixes in the area.
Signed-off-by: Joern Engel <joern@logfs.org>
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
Simple and just called from one place.
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Andy Grover <agrover@redhat.com>
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
Remove core_tpg_pre_dellun entirely, since we don't need to get/check
a pointer we already have.
Nothing else can return an error, so core_dev_del_lun can return void.
Rename core_tpg_post_dellun to remove_lun - a clearer name, now that
pre_dellun is gone.
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Andy Grover <agrover@redhat.com>
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
Nothing in it can raise an error.
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Andy Grover <agrover@redhat.com>
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
Clearly a right-shift was meant. Effectively doesn't make a difference,
as add_len is hard-coded to 8 and the high byte will be zero no matter
which way you shift. But I hate leaving bad examples for others to
copy.
Found by coverity.
Signed-off-by: Joern Engel <joern@logfs.org>
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
This patch correctly handles match_int() errors in FILEIO + PSCSI
backend parameter parsing, which can potentially fail due to a
memory allocation failure or invalid argument.
Found by coverity.
Signed-off-by: Joern Engel <joern@logfs.org>
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
Old code in iscsi_parse_pr_out_transport_id() was obviously buggy
and effectively ignored the high byte.
Found by coverity.
Signed-off-by: Joern Engel <joern@logfs.org>
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
Coverity complained that lun_cg has been dereferenced in all paths
leading to NULL check. It didn't mention that only a single path could
lead there and the code can be simplified even further.
Signed-off-by: Joern Engel <joern@logfs.org>
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
This patch fixes a memory leak on error in target_fabric_make_mappedlun(),
where se_lun_acl memory does not get released on exit.
Found by coverity.
Signed-off-by: Joern Engel <joern@logfs.org>
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
Each case of match_strdup could leak memory if the same argument was
present before. I am not too concerned, as it would require a
non-sensical combination like "target_lun=foo target_lun=bar", done
with root privileges and even then leak just a few bytes per instance.
But arg_p is different, as it will always leak memory. Let's plug that
one. And while at it, replace some &args[0] with args.
Found by coverity.
Signed-off-by: Joern Engel <joern@logfs.org>
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
last_intr_fail_name is a fixed-size array and could theoretically
overflow. In reality intrname->value doesn't seem to depend on
untrusted input or be anywhere near 224 characters, so the overflow is
pretty theoretical. But strlcpy is cheap enough.
Found by coverity.
Signed-off-by: Joern Engel <joern@logfs.org>
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
Found by coverity. At this point sock is non-NULL, so the check
to unnecessary.
Signed-off-by: Joern Engel <joern@logfs.org>
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
This patch drops the now duplicate + unnecessary check for -ENODEV from
iscsi_transport->iscsit_accept_np() for jumping to out:, or immediately
returning 1 in __iscsi_target_login_thread() code.
Since commit 81a9c5e72b the jump to out: and returning 1 have the same
effect, and end up hitting the ISCSI_NP_THREAD_SHUTDOWN check regardless
at the top of __iscsi_target_login_thread() during next loop iteration.
So that said, it's safe to go ahead and remove this duplicate check.
Reported-by: Joern Engel <joern@logfs.org>
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
The return statement cannot be reached without either recovery or dump
being set to 1. Therefore the condition always evaluates to true and
recovery and dump are useless variables.
Found by Coverity.
Signed-off-by: Joern Engel <joern@logfs.org>
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
Found by coverity. InitiatorName and InitiatorAlias are static arrays
and therefore always non-NULL. At some point in the past they may have
been dynamically allocated, but for current code the condition is
useless. If the intent was to check InitiatorName[0] instead, I cannot
find a use for that either. Let's get rid of it.
Signed-off-by: Joern Engel <joern@logfs.org>
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
Last user of buf was removed with c6037cc546. While at it,
free_cpumask_var() handles a NULL argument just fine, so remove the
conditionals.
Signed-off-by: Joern Engel <joern@logfs.org>
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
Clearly the file was meant to contain an include guard, but it was
missing the #define part.
Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
The use of "rcu_assign_pointer()" is NULLing out the pointer.
According to RCU_INIT_POINTER()'s block comment:
"1. This use of RCU_INIT_POINTER() is NULLing out the pointer"
it is better to use it instead of rcu_assign_pointer() because it has a
smaller overhead.
The following Coccinelle semantic patch was used:
@@
@@
- rcu_assign_pointer
+ RCU_INIT_POINTER
(..., NULL)
Signed-off-by: Andreea-Cristina Bernat <bernat.ada@gmail.com>
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
This patch removes the null test on lun_cg. lun_cg is initialized
at the beginning of the function to &lun->lun_group. Since lun_cg is
dereferenced prior to the null test, it must be a valid pointer.
The following Coccinelle script is used for detecting the change:
@r@
expression e,f;
identifier g,y;
statement S1,S2;
@@
*e = &f->g
<+...
f->y
...+>
*if (e != NULL || ...)
S1 else S2
Signed-off-by: Himangi Saraogi <himangi774@gmail.com>
Acked-by: Julia Lawall <julia.lawall@lip6.fr>
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
Pull vfs fixes from Al Viro:
"double iput() on failure exit in lustre, racy removal of spliced
dentries from ->s_anon in __d_materialise_dentry() plus a bunch of
assorted RCU pathwalk fixes"
The RCU pathwalk fixes end up fixing a couple of cases where we
incorrectly dropped out of RCU walking, due to incorrect initialization
and testing of the sequence locks in some corner cases. Since dropping
out of RCU walk mode forces the slow locked accesses, those corner cases
slowed down quite dramatically.
* 'for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/viro/vfs:
be careful with nd->inode in path_init() and follow_dotdot_rcu()
don't bugger nd->seq on set_root_rcu() from follow_dotdot_rcu()
fix bogus read_seqretry() checks introduced in b37199e
move the call of __d_drop(anon) into __d_materialise_unique(dentry, anon)
[fix] lustre: d_make_root() does iput() on dentry allocation failure
The performance regression that Josef Bacik reported in the pathname
lookup (see commit 99d263d4c5 "vfs: fix bad hashing of dentries") made
me look at performance stability of the dcache code, just to verify that
the problem was actually fixed. That turned up a few other problems in
this area.
There are a few cases where we exit RCU lookup mode and go to the slow
serializing case when we shouldn't, Al has fixed those and they'll come
in with the next VFS pull.
But my performance verification also shows that link_path_walk() turns
out to have a very unfortunate 32-bit store of the length and hash of
the name we look up, followed by a 64-bit read of the combined hash_len
field. That screws up the processor store to load forwarding, causing
an unnecessary hickup in this critical routine.
It's caused by the ugly calling convention for the "hash_name()"
function, and easily fixed by just making hash_name() fill in the whole
'struct qstr' rather than passing it a pointer to just the hash value.
With that, the profile for this function looks much smoother.
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Pull parisc updates from Helge Deller:
"The most important patch is a new Light Weigth Syscall (LWS) for 8,
16, 32 and 64 bit atomic CAS operations which is required in order to
be able to implement the atomic gcc builtins on our platform.
Other than that, we wire up the seccomp, getrandom and memfd_create
syscalls, fixes a minor off-by-one bug and a wrong printk string"
* 'parisc-3.17-1' of git://git.kernel.org/pub/scm/linux/kernel/git/deller/parisc-linux:
parisc: Implement new LWS CAS supporting 64 bit operations.
parisc: Wire up seccomp, getrandom and memfd_create syscalls
parisc: dino: fix %d confusingly prefixed with 0x in format string
parisc: sys_hpux: NUL terminator is one past the end
in the former we simply check if dentry is still valid after picking
its ->d_inode; in the latter we fetch ->d_inode in the same places
where we fetch dentry and its ->d_seq, under the same checks.
Cc: stable@vger.kernel.org # 2.6.38+
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
return the value instead, and have path_init() do the assignment. Broken by
"vfs: Fix absolute RCU path walk failures due to uninitialized seq number",
which was Cc-stable with 2.6.38+ as destination. This one should go where
it went.
To avoid dummy value returned in case when root is already set (it would do
no harm, actually, since the only caller that doesn't ignore the return value
is guaranteed to have nd->root *not* set, but it's more obvious that way),
lift the check into callers. And do the same to set_root(), to keep them
in sync.
Cc: stable@vger.kernel.org # 2.6.38+
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
MAINTAINERS to reflect new e-mail address.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1
iQIcBAABAgAGBQJUFR7/AAoJEG5mS6x6i9Ij/84P/RniNXGFS4Kh+YuS5E6KdZ+C
6rzpxlUkeEWkL+1Oy8xpLSJdWsCYajOmteP2V3ubQNlqzd0H/zN5W52/i4tIhVc1
W7SXexh1tr18BeeiQZ0sX5IlBGzixJ9QDzsuf0cg5keX9RphVBNbm16d8DvW/Le3
fj2LXyN+K82vy0GPZE0awi9TWfgj1oZobsKswjBp6Kz/z4gq11/rKQBeBvJbxSF/
WttgvDWONHiuUyHuKYJq53KQ+B7Q6ANwtKfIvd2CpFb8dWlsYYMnr1eRmz4ZC5OM
NDj2T4Dn5VrxqivmFJ+6d4EZK1agZXYXly+mkW21N2sMqL2/fq7b6CgbKzy76PLz
sDc1e5WUTJu0AjfrtTQBKG/ZbxXxvpMsarO3rI3C2Oos9DUPnDG8eHsHeOSoZQXi
tyUyIu4IyH4x7BWO//srU7l82YXWrrGN8+YU+GcLsMGDfsOaR4DDURUu9iYNX3cA
RoGfhmb+H41X+zOnzgnioyGMcK6mwVvsPxIzZ4CwsSa64aWlud6vbxoJr+q6K2ap
KEGxT78gy32Jw9wsfYUJKQpa5By7FdpGUDFk/rTt4ZYVU8TR1e+ShCz2l7Ny5Ioh
u13/vZ/VGVG3+MFjZ5In02egYx3fPnbilbavGCNRQM31/GJxHSc5p8xnoPxLmg70
tVREVtRX23KREqt38CBf
=uhqP
-----END PGP SIGNATURE-----
Merge tag 'ntb-3.17' of git://github.com/jonmason/ntb
Pull ntb driver bugfixes from Jon Mason:
"NTB driver fixes for queue spread and buffer alignment. Also, update
to MAINTAINERS to reflect new e-mail address"
* tag 'ntb-3.17' of git://github.com/jonmason/ntb:
ntb: Add alignment check to meet hardware requirement
MAINTAINERS: update NTB info
NTB: correct the spread of queues over mw's
Pull ARM irq chip fixes from Thomas Gleixner:
"Another pile of ARM specific irq chip fixlets:
- off by one bugs in the crossbar driver
- missing annotations
- a bunch of "make it compile" updates
I pulled the lot today from Jason, but it has been in -next for at
least a week"
* 'irq-urgent-for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip:
irqchip: gic-v3: Declare rdist as __percpu pointer to __iomem pointer
irqchip: gic: Make gic_default_routable_irq_domain_ops static
irqchip: exynos-combiner: Fix compilation error on ARM64
irqchip: crossbar: Off by one bugs in init
irqchip: gic-v3: Tag all low level accessors __maybe_unused
irqchip: gic-v3: Only define gic_peek_irq() when building SMP
The NTB translate register must have the value to be BAR size aligned.
This alignment check make sure that the DMA memory allocated has the
proper alignment. Another requirement for NTB to function properly with
memory window BAR size greater or equal to 4M is to use the CMA feature
in 3.16 kernel with the appropriate CONFIG_CMA_ALIGNMENT and
CONFIG_CMA_SIZE_MBYTES set.
Signed-off-by: Dave Jiang <dave.jiang@intel.com>
Signed-off-by: Jon Mason <jdmason@kudzu.us>
Update my contact info to my personal email address and add Dave Jiang.
Signed-off-by: Jon Mason <jon.mason@intel.com>
Signed-off-by: Dave Jiang <dave.jiang@intel.com>
The detection of an uneven number of queues on the given memory windows
was not correct. The mw_num is zero based and the mod should be
division to spread them evenly over the mw's.
Signed-off-by: Jon Mason <jon.mason@intel.com>
Joern Engel