On 06/04/14 20:08, Rickard Strandqvist wrote:
> Minimized the use of snprintf()
> And removed a variable that was only used for the temporary storage.
>
> This was partly found using a static code analysis program called cppcheck.
>
> Signed-off-by: Rickard Strandqvist
> ---
> drivers/scsi/bfa/
Take the following scene in guest:
seqA: scsi_done() -> gapX (before taking REQ_ATOM_COMPLETE)
seqB: scmd_eh_abort_handler()-> ...-> ibmvscsi_eh_abort_handler()->
...->scsi_put_command(scmd)
If seqA is scheduled at gapX, and seqB reclaims scmd. Then when seqA
comes back, it tries to access t
On 6/4/14, Paolo Bonzini wrote:
> Il 04/06/2014 19:29, Venkatesh Srinivas ha scritto:
>> Do you really want to poll the request VQs for completions if the TMF
>> was rejected?
>
> I wasn't sure, but bugs in this path are hard enough that I preferred
> the safer patch.
Ok. As long as it was delibe
On 06/04/2014 12:24 PM, Arnd Bergmann wrote:
>
> For other timekeeping stuff in the kernel, I agree that using some
> 64-bit representation (nanoseconds, 32/32 unsigned seconds/nanoseconds,
> ...) has advantages, that's exactly the point I was making earlier
> against simply extending the internal
On Wed, 2014-06-04 at 23:36 +0200, Rickard Strandqvist wrote:
> Added a guaranteed null-terminate after call to strncpy.
>
> This was partly found using a static code analysis program called cppcheck.
>
> Signed-off-by: Rickard Strandqvist
> ---
> drivers/scsi/mpt2sas/mpt2sas_base.c |3 ++-
Hi
A little embarrassing, but I actually did not know that there was a
better replacement for strncpy.
Sorry, but I will send a new platch based on strlcpy instead then.
Will investigate cover letter then to.
Best regards
Rickard Strandqvist
2014-06-05 0:01 GMT+02:00 Joe Perches :
> On Wed,
On Wed, 2014-06-04 at 23:36 +0200, Rickard Strandqvist wrote:
> Added a guaranteed null-terminate after call to strncpy.
Next time you submit a patch series like this
can you please use a cover letter?
That way I can reply to the cover letter with
a comment about the series instead of multiple
re
On Wed, 2014-06-04 at 23:32 +0200, Rickard Strandqvist wrote:
> Added a guaranteed null-terminate after call to strncpy.
>
> This was partly found using a static code analysis program called cppcheck.
>
> Signed-off-by: Rickard Strandqvist
> ---
> drivers/scsi/bfa/bfad_attr.c |3 ++-
> 1 fi
Added a guaranteed null-terminate after call to strncpy.
This was partly found using a static code analysis program called cppcheck.
Signed-off-by: Rickard Strandqvist
---
drivers/scsi/mpt3sas/mpt3sas_base.c |3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/scsi/m
Added a guaranteed null-terminate after call to strncpy.
This was partly found using a static code analysis program called cppcheck.
Signed-off-by: Rickard Strandqvist
---
drivers/scsi/mpt2sas/mpt2sas_base.c |3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/scsi/m
Added a guaranteed null-terminate after call to strncpy.
This was partly found using a static code analysis program called cppcheck.
Signed-off-by: Rickard Strandqvist
---
drivers/scsi/ibmvscsi/ibmvscsi.c |4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/scsi/ibm
Added a guaranteed null-terminate after call to strncpy.
This was partly found using a static code analysis program called cppcheck.
Signed-off-by: Rickard Strandqvist
---
drivers/scsi/bfa/bfad_attr.c |3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/scsi/bfa/bfad
On Wednesday 04 June 2014 13:30:32 Nicolas Pitre wrote:
> On Wed, 4 Jun 2014, Arnd Bergmann wrote:
>
> > On Tuesday 03 June 2014, Dave Chinner wrote:
> > > Just ot be pedantic, inodes don't need 96 bit timestamps - some
> > > filesystems can *support up to* 96 bit timestamps. If the kernel
> > > o
Il 04/06/2014 19:29, Venkatesh Srinivas ha scritto:
Do you really want to poll the request VQs for completions if the TMF
was rejected?
I wasn't sure, but bugs in this path are hard enough that I preferred
the safer patch.
TMF ABORT may return FUNCTION REJECTED if the command to abort
compl
Minimized the use of snprintf()
And removed a variable that was only used for the temporary storage.
This was partly found using a static code analysis program called cppcheck.
Signed-off-by: Rickard Strandqvist
---
drivers/scsi/bfa/bfad_attr.c | 14 --
1 file changed, 8 insertion
On Wed, 4 Jun 2014, Arnd Bergmann wrote:
> On Tuesday 03 June 2014, Dave Chinner wrote:
> > Just ot be pedantic, inodes don't need 96 bit timestamps - some
> > filesystems can *support up to* 96 bit timestamps. If the kernel
> > only supports 64 bit timestamps and that's all the kernel can
> > rep
> -Original Message-
> From: James Bottomley [mailto:jbottom...@parallels.com]
> Sent: Wednesday, June 4, 2014 10:02 AM
> To: KY Srinivasan
> Cc: linux-ker...@vger.kernel.org; a...@canonical.com;
> de...@linuxdriverproject.org; h...@infradead.org; linux-
> s...@vger.kernel.org; oher...@su
On 6/4/14, Paolo Bonzini wrote:
> Even though the virtio-scsi spec guarantees that all requests related
> to the TMF will have been completed by the time the TMF itself completes,
> the request queue's callback might not have run yet. This causes requests
> to be completed more than once, and as
On 6/4/14, Paolo Bonzini wrote:
> From: Ming Lei
>
> The spinlock of tgt_lock is only for serializing read and write
> req_vq, one lockless seqcount is enough for the purpose.
>
> On one 16core VM with vhost-scsi backend, the patch can improve
> IOPS with 3% on random read test.
>
> Signed-off-by
On Wed, 2014-06-04 at 09:33 -0700, K. Y. Srinivasan wrote:
> Commit ID: 7e660100d85af860e7ad763202fff717adcdaacd added code to derive the
> FLUSH_TIMEOUT from the basic I/O timeout. However, this patch did not use the
> basic I/O timeout of the device. Fix this bug.
>
> Signed-off-by: K. Y. Sriniv
On Wed, 4 Jun 2014 12:49:46 -0400
Joe Lawrence wrote:
> Fixes the following smatch warning:
>
> drivers/message/fusion/mptctl.c:1369 mptctl_getiocinfo() warn:
> possible info leak 'karg'
>
> Signed-off-by: Joe Lawrence
> Cc: Christoph Hellwig
> Cc: Dan Carpenter
> Cc: Sreekanth Reddy
Fixes the following smatch warnings:
drivers/message/fusion/mptbase.c:652 mptbase_reply() warn: variable
dereferenced before check 'reply' (see line 639)
drivers/message/fusion/mptsas.c:1255 mptsas_taskmgmt_complete() error:
we previously assumed 'pScsiTmReply' could be null (see line
Tack the firmware reply event_data payload to the end of its
corresponding struct fw_event_work allocation. Rework fw_event_work
allocation calculations to include the event_data size where
appropriate.
This clarifies the code a bit and avoids the following smatch warnings:
drivers/message/fus
The struct _MPT_ADAPTER doesn't need a full copy of the product string,
so prod_name can point to the string literal storage that the driver
already provides.
Avoids the following sparse warning:
drivers/message/fusion/mptbase.c:2858 MptDisplayIocCapabilities()
warn: this array is probably
Fixes the following sparse warnings:
drivers/message/fusion/mptbase.c:7011:1: warning: symbol
'mpt_SoftResetHandler' was not declared. Should it be static?
drivers/message/fusion/mptsas.c:1578:23: warning: symbol
'mptsas_refreshing_device_handles' was not declared. Should it be
st
Fixes the following smatch warnings:
drivers/message/fusion/mptfc.c:529 mptfc_target_destroy() info:
redundant null check on starget->hostdata calling kfree()
drivers/message/fusion/mptspi.c:465 mptspi_target_destroy() info:
redundant null check on starget->hostdata calling kfree()
S
Fixes the following smatch warning:
drivers/message/fusion/mptctl.c:1369 mptctl_getiocinfo() warn:
possible info leak 'karg'
Signed-off-by: Joe Lawrence
Cc: Christoph Hellwig
Cc: Dan Carpenter
Cc: Sreekanth Reddy
---
drivers/message/fusion/mptctl.c |2 +-
1 file changed, 1 insertio
The lone caller of initChainBuffers checks the return code for < 0, so
it is safe to appease smatch and return the proper errno value.
Fixes the following smatch warnings:
drivers/message/fusion/mptbase.c:4328 initChainBuffers() warn:
returning -1 instead of -ENOMEM is sloppy
drivers/mes
While reviewing the mpt2/mpt3 static checker fixup patchset, Christoph
inquired about mptfusion. None of the sparse / smatch warnings from the
earlier patchset directly apply to fusion, but there were a few easy to
fix warnings (compile tested only).
The patchset is ordered from the smallest/easi
On Tue, 2014-06-03 at 23:34 +0200, Clément Calmels wrote:
> In init_sd function, if kmem_cache_create or mempool_create_slab_pools
> calls fail, the error will not be correclty reported because
> class_register previously set the value of err to 0.
>
> Signed-off-by: Clément Calmels
> ---
> driv
Commit ID: 7e660100d85af860e7ad763202fff717adcdaacd added code to derive the
FLUSH_TIMEOUT from the basic I/O timeout. However, this patch did not use the
basic I/O timeout of the device. Fix this bug.
Signed-off-by: K. Y. Srinivasan
---
drivers/scsi/sd.c |4 +++-
1 files changed, 3 insertio
On Wed, Jun 04, 2014 at 05:06:44PM +0200, Julia Lawall wrote:
>
>
> On Wed, 4 Jun 2014, scame...@beardog.cce.hp.com wrote:
>
> > On Wed, Jun 04, 2014 at 11:07:56AM +0200, Julia Lawall wrote:
> > > From: Julia Lawall
> > >
> > > Find_first_zero_bit considers BITS_PER_LONG bits at a time, and thu
On Tuesday 03 June 2014, Dave Chinner wrote:
> On Tue, Jun 03, 2014 at 04:22:19PM +0200, Arnd Bergmann wrote:
> > On Monday 02 June 2014 14:57:26 H. Peter Anvin wrote:
> > > On 06/02/2014 12:55 PM, Arnd Bergmann wrote:
> > The possible uses I can see for non-ktime_t types in the kernel are:
> > * i
On Monday 02 June 2014, Joseph S. Myers wrote:
> On Mon, 2 Jun 2014, Arnd Bergmann wrote:
>
> > Ok. Sorry about missing linux-api, I confused it with linux-arch, which
> > may not be as relevant here, except for the one question whether we
> > actually want to have the new ABI on all 32-bit archit
On Wed, 4 Jun 2014, scame...@beardog.cce.hp.com wrote:
> On Wed, Jun 04, 2014 at 11:07:56AM +0200, Julia Lawall wrote:
> > From: Julia Lawall
> >
> > Find_first_zero_bit considers BITS_PER_LONG bits at a time, and thus may
> > return a larger number than the maximum position argument if that po
When the SG_IO ioctl was copied into the block layer and
later into the bsg driver, subtle differences emerged.
One difference is the way injected commands are queued through
the block layer (i.e. this is not SCSI device queueing nor SATA
NCQ). Summarizing:
- SG_IO in the block layer: blk_exec*
On Wed, Jun 04, 2014 at 11:07:56AM +0200, Julia Lawall wrote:
> From: Julia Lawall
>
> Find_first_zero_bit considers BITS_PER_LONG bits at a time, and thus may
> return a larger number than the maximum position argument if that position
> is not a multiple of BITS_PER_LONG.
>
> The semantic matc
https://bugzilla.kernel.org/show_bug.cgi?id=16490
Alain Kalker changed:
What|Removed |Added
CC||a.c.kal...@gmail.com
--- Comment #8 from A
From: Julia Lawall
> On Wed, 4 Jun 2014, Geert Uytterhoeven wrote:
>
> > Hi Julia,
> >
> > On Wed, Jun 4, 2014 at 1:00 PM, Julia Lawall wrote:
> > > OK, thanks. I was only looking at the C code.
> > >
> > > But the C code contains a loop that is followed by:
> > >
> > > if (!size)
> > >
On Wed, 4 Jun 2014, Geert Uytterhoeven wrote:
> Hi Julia,
>
> On Wed, Jun 4, 2014 at 1:00 PM, Julia Lawall wrote:
> > OK, thanks. I was only looking at the C code.
> >
> > But the C code contains a loop that is followed by:
> >
> > if (!size)
> > return result;
> >
As mentioned in the v1 submission, these patches were delayed a bit
by the discussion and testing of patch 5. Debugging the problem also
led to the discovery of a midlayer bug fixed by patch 4.
Paolo
v1->v2: fix all occurrences of "scmd->result |= DID_TIME_OUT << 16"
in patch 4
v2->v3:
Calling the workqueue interface on uninitialized work items isn't a
good idea even if they're zeroed. It's not failing catastrophically only
through happy accidents.
Signed-off-by: Paolo Bonzini
---
drivers/scsi/virtio_scsi.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a
From: Ming Lei
Access to tgt->req_vq is strictly serialized by spin_lock
of tgt->tgt_lock, so the ACCESS_ONCE() isn't necessary.
smp_read_barrier_depends() in virtscsi_req_done was introduced
to order reading req_vq and decreasing tgt->reqs, but it isn't
needed now because req_vq is read from
sc
From: Ulrich Obergfell
After scsi_try_to_abort_cmd returns, the eh_abort_handler may have
already found that the command has completed in the device, causing
the host_byte to be nonzero (e.g. it could be DID_ABORT). When
this happens, ORing DID_TIME_OUT into the host byte will corrupt
the result
From: Ming Lei
The spinlock of tgt_lock is only for serializing read and write
req_vq, one lockless seqcount is enough for the purpose.
On one 16core VM with vhost-scsi backend, the patch can improve
IOPS with 3% on random read test.
Signed-off-by: Ming Lei
[Add initialization in virtscsi_targ
Even though the virtio-scsi spec guarantees that all requests related
to the TMF will have been completed by the time the TMF itself completes,
the request queue's callback might not have run yet. This causes requests
to be completed more than once, and as a result triggers a variety of
BUGs or oo
From: Venkatesh Srinivas
change_queue_depth allows changing per-target queue depth via sysfs.
It also allows the SCSI midlayer to ramp down the number of concurrent
inflight requests in response to a SCSI BUSY status response and allows
the midlayer to ramp the count back up to the device maximu
Il 04/06/2014 13:21, Bart Van Assche ha scritto:
Thanks for the quick respin. However, since you are mentioning that in
v2 all occurrences of "scmd->result |= DID_TIME_OUT << 16" have been
addressed, this made me wonder whether you had noticed the following
code in scsi_decide_disposition() ?
On 06/04/14 12:11, Paolo Bonzini wrote:
> v1->v2: fix all occurrences of "scmd->result |= DID_TIME_OUT << 16"
> in patch 4
Hello Paolo,
Thanks for the quick respin. However, since you are mentioning that in
v2 all occurrences of "scmd->result |= DID_TIME_OUT << 16" have been
addressed, th
Hi Julia,
On Wed, Jun 4, 2014 at 1:00 PM, Julia Lawall wrote:
> OK, thanks. I was only looking at the C code.
>
> But the C code contains a loop that is followed by:
>
> if (!size)
> return result;
> tmp = *p;
>
> found_first:
> tmp |= ~0UL << size;
>
On Wed, 4 Jun 2014, Geert Uytterhoeven wrote:
> Hi Julia,
>
> On Wed, Jun 4, 2014 at 11:52 AM, Julia Lawall wrote:
> >> Maybe the documented return code should be changed to allow for the
> >> existing behaviour.
> >
> > Sorry, I'm not sure to understand what you suggest here.
>
> include/asm-g
Hi Julia,
On Wed, Jun 4, 2014 at 11:52 AM, Julia Lawall wrote:
>> Maybe the documented return code should be changed to allow for the
>> existing behaviour.
>
> Sorry, I'm not sure to understand what you suggest here.
include/asm-generic/bitops/find.h:
| /**
| * find_first_zero_bit - find the
From: Ming Lei
The spinlock of tgt_lock is only for serializing read and write
req_vq, one lockless seqcount is enough for the purpose.
On one 16core VM with vhost-scsi backend, the patch can improve
IOPS with 3% on random read test.
Signed-off-by: Ming Lei
[Add initialization in virtscsi_targ
From: Ming Lei
Access to tgt->req_vq is strictly serialized by spin_lock
of tgt->tgt_lock, so the ACCESS_ONCE() isn't necessary.
smp_read_barrier_depends() in virtscsi_req_done was introduced
to order reading req_vq and decreasing tgt->reqs, but it isn't
needed now because req_vq is read from
sc
From: Venkatesh Srinivas
change_queue_depth allows changing per-target queue depth via sysfs.
It also allows the SCSI midlayer to ramp down the number of concurrent
inflight requests in response to a SCSI BUSY status response and allows
the midlayer to ramp the count back up to the device maximu
Even though the virtio-scsi spec guarantees that all requests related
to the TMF will have been completed by the time the TMF itself completes,
the request queue's callback might not have run yet. This causes requests
to be completed more than once, and as a result triggers a variety of
BUGs or oo
Calling the workqueue interface on uninitialized work items isn't a
good idea even if they're zeroed. It's not failing catastrophically only
through happy accidents.
Signed-off-by: Paolo Bonzini
---
drivers/scsi/virtio_scsi.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a
From: Ulrich Obergfell
After scsi_try_to_abort_cmd returns, the eh_abort_handler may have
already found that the command has completed in the device, causing
the host_byte to be nonzero (e.g. it could be DID_ABORT). When
this happens, ORing DID_TIME_OUT into the host byte will corrupt
the result
As mentioned in the v1 submission, these patches were delayed a bit
by the discussion and testing of patch 5. Debugging the problem also
led to the discovery of a midlayer bug fixed by patch 4.
Paolo
v1->v2: fix all occurrences of "scmd->result |= DID_TIME_OUT << 16"
in patch 4
Ming Lei
On Wed, 4 Jun 2014, David Laight wrote:
> From: Julia Lawall
> > On Wed, 4 Jun 2014, Geert Uytterhoeven wrote:
> >
> > > Hi Julia,
> > >
> > > On Wed, Jun 4, 2014 at 11:07 AM, Julia Lawall
> > > wrote:
> > > > Find_first_zero_bit considers BITS_PER_LONG bits at a time, and thus may
> > > > ret
From: Julia Lawall
> On Wed, 4 Jun 2014, Geert Uytterhoeven wrote:
>
> > Hi Julia,
> >
> > On Wed, Jun 4, 2014 at 11:07 AM, Julia Lawall wrote:
> > > Find_first_zero_bit considers BITS_PER_LONG bits at a time, and thus may
> > > return a larger number than the maximum position argument if that po
On Wed, 4 Jun 2014, Geert Uytterhoeven wrote:
> Hi Julia,
>
> On Wed, Jun 4, 2014 at 11:07 AM, Julia Lawall wrote:
> > Find_first_zero_bit considers BITS_PER_LONG bits at a time, and thus may
> > return a larger number than the maximum position argument if that position
> > is not a multiple of
Hi Julia,
On Wed, Jun 4, 2014 at 11:07 AM, Julia Lawall wrote:
> Find_first_zero_bit considers BITS_PER_LONG bits at a time, and thus may
> return a larger number than the maximum position argument if that position
> is not a multiple of BITS_PER_LONG.
Shouldn't this be fixed in find_first_zero_
From: Julia Lawall
Find_first_zero_bit considers BITS_PER_LONG bits at a time, and thus may
return a larger number than the maximum position argument if that position
is not a multiple of BITS_PER_LONG.
The semantic match that finds this problem is as follows:
(http://coccinelle.lip6.fr/)
//
@
Find_first_zero_bit considers BITS_PER_LONG bits at a time, and thus may
return a larger number than the maximum position argument if that position
is not a multiple of BITS_PER_LONG.
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vge
On Tue, Jun 03, 2014 at 09:36:56PM -0400, Douglas Gilbert wrote:
> When the SG_IO ioctl was copied into the block layer and
> later into the bsg driver, subtle differences emerged.
>
> One difference is the way injected commands are queued through
> the block layer (i.e. this is not SCSI device qu
On Tue, 3 Jun 2014, Bjorn Helgaas wrote:
> >>> > Attached is dmesg output leading to timeouts (that are cured by my
> >>> > original patch in this thread) and lspci.
> >>>
> >>> I opened https://bugzilla.kernel.org/show_bug.cgi?id=64141 for this
> >>> issue and attached your dmesg log and lspci ou
On 06/04/2014 09:51 AM, Maurizio Lombardi wrote:
On Thu, May 15, 2014 at 08:41:10AM +0200, Christoph Hellwig wrote:
We now have a readily available copy of EVPD page 0x83 hanging off
the scsi device, so the alua device handler should use it.
Hannes, how about you handle that if you're doing ma
On Thu, May 15, 2014 at 08:41:10AM +0200, Christoph Hellwig wrote:
>
> We now have a readily available copy of EVPD page 0x83 hanging off
> the scsi device, so the alua device handler should use it.
>
> Hannes, how about you handle that if you're doing major surgery in that
> area anwyay?
Hannes
69 matches
Mail list logo