[PATCH] drivers/scsi/lpfc/lpfc_init.c: adjust code alignment

[Julia Lawall]

From: Julia Lawall julia.law...@lip6.fr

Adjust code alignment so that each statement is lined up with its neighbor.

In the second case, it could be desirable to put the call to
lpfc_destroy_vport_work_array under the if.  The call, is, however, safe
in case vports is NULL, so the patch just adjusts the indentation to
reflect the current semantics.

Signed-off-by: Julia Lawall julia.law...@lip6.fr

---

 drivers/scsi/lpfc/lpfc_init.c |4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/lpfc/lpfc_init.c b/drivers/scsi/lpfc/lpfc_init.c
index e0b20fa..f870421 100644
--- a/drivers/scsi/lpfc/lpfc_init.c
+++ b/drivers/scsi/lpfc/lpfc_init.c
@@ -692,7 +692,7 @@ lpfc_hba_init_link_fc_topology(struct lpfc_hba *phba, 
uint32_t fc_topology,
1302 Invalid speed for this board:%d 
Reset link speed to auto.\n,
phba-cfg_link_speed);
-   phba-cfg_link_speed = LPFC_USER_LINK_SPEED_AUTO;
+   phba-cfg_link_speed = LPFC_USER_LINK_SPEED_AUTO;
}
lpfc_init_link(phba, pmb, fc_topology, phba-cfg_link_speed);
pmb-mbox_cmpl = lpfc_sli_def_mbox_cmpl;
@@ -2702,7 +2702,7 @@ lpfc_online(struct lpfc_hba *phba)
}
spin_unlock_irq(shost-host_lock);
}
-   lpfc_destroy_vport_work_array(phba, vports);
+   lpfc_destroy_vport_work_array(phba, vports);
 
lpfc_unblock_mgmt_io(phba);
return 0;

--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 14/29] drivers/scsi/ufs/ufshcd-pltfrm.c: simplify use of devm_ioremap_resource

[Julia Lawall]

From: Julia Lawall julia.law...@lip6.fr

Remove unneeded error handling on the result of a call to
platform_get_resource when the value is passed to devm_ioremap_resource.

A debugging statement in the error-handling code is removed as well, as it
doesn't seem to give any more information than devm_ioremap_resource
already gives.

A simplified version of the semantic patch that makes this change is as
follows: (http://coccinelle.lip6.fr/)

// smpl
@@
expression pdev,res,n,e,e1;
expression ret != 0;
identifier l;
@@

- res = platform_get_resource(pdev, IORESOURCE_MEM, n);
  ... when != res
- if (res == NULL) { ... \(goto l;\|return ret;\) }
  ... when != res
+ res = platform_get_resource(pdev, IORESOURCE_MEM, n);
  e = devm_ioremap_resource(e1, res);
// /smpl

Signed-off-by: Julia Lawall julia.law...@lip6.fr

---
 drivers/scsi/ufs/ufshcd-pltfrm.c |7 ---
 1 file changed, 7 deletions(-)

diff --git a/drivers/scsi/ufs/ufshcd-pltfrm.c b/drivers/scsi/ufs/ufshcd-pltfrm.c
index c42db40..ea699b9 100644
--- a/drivers/scsi/ufs/ufshcd-pltfrm.c
+++ b/drivers/scsi/ufs/ufshcd-pltfrm.c
@@ -102,15 +102,8 @@ static int ufshcd_pltfrm_probe(struct platform_device 
*pdev)
struct device *dev = pdev-dev;
 
mem_res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-   if (!mem_res) {
-   dev_err(dev, Memory resource not available\n);
-   err = -ENODEV;
-   goto out;
-   }
-
mmio_base = devm_ioremap_resource(dev, mem_res);
if (IS_ERR(mmio_base)) {
-   dev_err(dev, memory map failed\n);
err = PTR_ERR(mmio_base);
goto out;
}

--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 0/29] simplify use of devm_ioremap_resource

[Julia Lawall]

devm_ioremap_resource often uses the result of a call to
platform_get_resource as its last argument.  devm_ioremap_resource does
appropriate error handling on this argument, so error handling can be
removed from the call site.  To make the connection between the call to
platform_get_resource and the call to devm_ioremap_resource more clear, the
former is also moved down to be adjacent to the latter.

In many cases, this patch changes the specific error value that is
returned on failure of platform_get_resource.

The semantic patch that makes this change is as follows:
(http://coccinelle.lip6.fr/)

// smpl
@@
expression pdev,res,n,e,e1;
expression ret != 0;
identifier l;
@@

(
  res = platform_get_resource(pdev, IORESOURCE_MEM, n);
- if (res == NULL) { ... \(goto l;\|return ret;\) }
  e = devm_ioremap_resource(e1, res);
|
- res = platform_get_resource(pdev, IORESOURCE_MEM, n);
  ... when != res
- if (res == NULL) { ... \(goto l;\|return ret;\) }
  ... when != res
+ res = platform_get_resource(pdev, IORESOURCE_MEM, n);
  e = devm_ioremap_resource(e1, res);
)
// /smpl

--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/10] use safer test on the result of find_first_zero_bit

[Julia Lawall]

On Wed, 4 Jun 2014, Geert Uytterhoeven wrote:

 Hi Julia,

 On Wed, Jun 4, 2014 at 11:07 AM, Julia Lawall julia.law...@lip6.fr 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_bit() instead?

OK, I could do that as well.  Most of the callers currently test with =.
Should they be left as is, or changed to use ==?

julia
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH 0/10] use safer test on the result of find_first_zero_bit

[Julia Lawall]

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 julia.law...@lip6.fr 
   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_bit() instead?
 
  OK, I could do that as well.  Most of the callers currently test with =.
  Should they be left as is, or changed to use ==?

 Do we want to add an extra test to find_first_zero_bit() and effectively
 slow down all the calls - especially those where the length is a
 multiple of 8 (probably the most common).

Currently, most of the calls test with =, and most of the others seem to
need to (either the size value did not look like a multiple of anything in
particular, or it was eg read from a device).

Note that it is BITS_PER_LONG, so it seems like it is typically 32 or 64,
not 8.

 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.

thanks,
julia
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/10] use safer test on the result of find_first_zero_bit

[Julia Lawall]

On Wed, 4 Jun 2014, Geert Uytterhoeven wrote:

 Hi Julia,

 On Wed, Jun 4, 2014 at 11:52 AM, Julia Lawall julia.law...@lip6.fr 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 first cleared bit in a memory region
 |  * @addr: The address to start the search at
 |  * @size: The maximum number of bits to search
 |  *
 |  * Returns the bit number of the first cleared bit.
 |  * If no bits are zero, returns @size.

 If no bits are zero, returns @size or a number larger than @size.

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;
if (tmp == ~0UL)/* Are any bits zero? */
return result + size;   /* Nope. */

In the first return, it would seem that result == size.  Could the second
one be changed to just return size?  It should not hurt performance.

julia


 |  */
 | extern unsigned long find_first_zero_bit(const unsigned long *addr,
 |  unsigned long size);

 Gr{oetje,eeting}s,

 Geert

 --
 Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- 
 ge...@linux-m68k.org

 In personal conversations with technical people, I call myself a hacker. But
 when I'm talking to journalists I just say programmer or something like 
 that.
 -- Linus Torvalds

--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/10] use safer test on the result of find_first_zero_bit

[Julia Lawall]

On Wed, 4 Jun 2014, Geert Uytterhoeven wrote:

 Hi Julia,

 On Wed, Jun 4, 2014 at 1:00 PM, Julia Lawall julia.law...@lip6.fr 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;
  if (tmp == ~0UL)/* Are any bits zero? */
  return result + size;   /* Nope. */
 
  In the first return, it would seem that result == size.  Could the second
  one be changed to just return size?  It should not hurt performance.

 size may have been changed between function entry and this line.
 So you have to store it in a temporary.

Sorry, after reflection it seems that indeed size + result is always the
original size, so it is actually all of the code that uses = that is
doing something unnecessary.  == for the failure test is fine.

julia
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 6/10] hpsa: use safer test on the result of find_first_zero_bit

[Julia Lawall]

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 julia.law...@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.
 
  The semantic match that finds this problem is as follows:
  (http://coccinelle.lip6.fr/)
 
  // smpl
  @@
  expression e1,e2,e3;
  statement S1,S2;
  @@
 
  e1 = find_first_zero_bit(e2,e3)
  ...
  if (e1
  - ==
  + =
e3)
  S1 else S2
  // /smpl
 
  Signed-off-by: Julia Lawall julia.law...@lip6.fr
 
  ---
   drivers/scsi/hpsa.c |2 +-
   1 file changed, 1 insertion(+), 1 deletion(-)
 
  diff -u -p a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c
  --- a/drivers/scsi/hpsa.c
  +++ b/drivers/scsi/hpsa.c
  @@ -4703,7 +4703,7 @@ static struct CommandList *cmd_alloc(str
  spin_lock_irqsave(h-lock, flags);
  do {
  i = find_first_zero_bit(h-cmd_pool_bits, h-nr_cmds);
  -   if (i == h-nr_cmds) {
  +   if (i = h-nr_cmds) {
  spin_unlock_irqrestore(h-lock, flags);
  return NULL;
  }

 Thanks, Ack.

 You can add

 Reviewed-by: Stephen M. Cameron scame...@beardog.cce.hp.com

 to this patch if you want.

 You might also consider adding Cc: sta...@vger.kernel.org to the sign-off 
 area.

Actually, it seems that the function can never overshoot the specified
limit.  So the change is not needed.

julia
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH] qla4xxx: Return -ENOMEM on memory allocation failure

[Julia Lawall]

On Fri, 4 Jul 2014, Elliott, Robert (Server Storage) wrote:

 
 
  -Original Message-
  From: linux-scsi-ow...@vger.kernel.org [mailto:linux-scsi-
  ow...@vger.kernel.org] On Behalf Of Himangi Saraogi
  Sent: Friday, 04 July, 2014 1:28 PM
  To: Vikas Chaudhary; iscsi-dri...@qlogic.com; James E.J. Bottomley; linux-
  s...@vger.kernel.org; linux-ker...@vger.kernel.org
  Cc: julia.law...@lip6.fr
  Subject: [PATCH] qla4xxx: Return -ENOMEM on memory allocation failure
  
  In this code, 0 is returned on memory allocation failure, even though
  other failures return -ENOMEM or other similar values.
  
  A simplified version of the Coccinelle semantic match that finds this
  problem is as follows:
  
  // smpl
  @@
  expression ret;
  expression x,e1,e2,e3;
  identifier alloc;
  @@
  
  ret = 0
  ... when != ret = e1
  *x = alloc(...)
  ... when != ret = e2
  if (x == NULL) { ... when != ret = e3
return ret;
  }
  // /smpl
  
  Signed-off-by: Himangi Saraogi himangi...@gmail.com
  Acked-by: Julia Lawall julia.law...@lip6.fr
  ---
   drivers/scsi/qla4xxx/ql4_os.c | 1 +
   1 file changed, 1 insertion(+)
  
  diff --git a/drivers/scsi/qla4xxx/ql4_os.c b/drivers/scsi/qla4xxx/ql4_os.c
  index c5d9564..72ba671 100644
  --- a/drivers/scsi/qla4xxx/ql4_os.c
  +++ b/drivers/scsi/qla4xxx/ql4_os.c
  @@ -1050,6 +1050,7 @@ static int qla4xxx_get_host_stats(struct Scsi_Host
  *shost, char *buf, int len)
  if (!ql_iscsi_stats) {
  ql4_printk(KERN_ERR, ha,
 Unable to allocate memory for iscsi stats\n);
  +   ret = -ENOMEM;
  goto exit_host_stats;
  }
  
 
 Also, the only caller of this function doesn't use the return 
 value - it's overwritten by another function call:
 
 drivers/scsi/scsi_transport_iscsi.c:
 err = transport-get_host_stats(shost, buf, host_stats_size);
 
 actual_size = nlmsg_total_size(sizeof(*ev) + host_stats_size);
 skb_trim(skbhost_stats, NLMSG_ALIGN(actual_size));
 nlhhost_stats-nlmsg_len = actual_size;
 
 err = iscsi_multicast_skb(skbhost_stats, ISCSI_NL_GRP_ISCSID,
   GFP_KERNEL);

Maybe that is intentional?  If get_host_stats fails, eg because of a lack 
of memory, the worst that will happen is that a region of the buffer will 
be 0.  If the loop is done again, there seems to be a risk of an infinite 
loop.

julia
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH] qla4xxx: Return -ENOMEM on memory allocation failure

[Julia Lawall]

On Fri, 4 Jul 2014, Julia Lawall wrote:

 
 
 On Fri, 4 Jul 2014, Elliott, Robert (Server Storage) wrote:
 
  
  
   -Original Message-
   From: linux-scsi-ow...@vger.kernel.org [mailto:linux-scsi-
   ow...@vger.kernel.org] On Behalf Of Himangi Saraogi
   Sent: Friday, 04 July, 2014 1:28 PM
   To: Vikas Chaudhary; iscsi-dri...@qlogic.com; James E.J. Bottomley; linux-
   s...@vger.kernel.org; linux-ker...@vger.kernel.org
   Cc: julia.law...@lip6.fr
   Subject: [PATCH] qla4xxx: Return -ENOMEM on memory allocation failure
   
   In this code, 0 is returned on memory allocation failure, even though
   other failures return -ENOMEM or other similar values.
   
   A simplified version of the Coccinelle semantic match that finds this
   problem is as follows:
   
   // smpl
   @@
   expression ret;
   expression x,e1,e2,e3;
   identifier alloc;
   @@
   
   ret = 0
   ... when != ret = e1
   *x = alloc(...)
   ... when != ret = e2
   if (x == NULL) { ... when != ret = e3
 return ret;
   }
   // /smpl
   
   Signed-off-by: Himangi Saraogi himangi...@gmail.com
   Acked-by: Julia Lawall julia.law...@lip6.fr
   ---
drivers/scsi/qla4xxx/ql4_os.c | 1 +
1 file changed, 1 insertion(+)
   
   diff --git a/drivers/scsi/qla4xxx/ql4_os.c b/drivers/scsi/qla4xxx/ql4_os.c
   index c5d9564..72ba671 100644
   --- a/drivers/scsi/qla4xxx/ql4_os.c
   +++ b/drivers/scsi/qla4xxx/ql4_os.c
   @@ -1050,6 +1050,7 @@ static int qla4xxx_get_host_stats(struct Scsi_Host
   *shost, char *buf, int len)
 if (!ql_iscsi_stats) {
 ql4_printk(KERN_ERR, ha,
Unable to allocate memory for iscsi stats\n);
   + ret = -ENOMEM;
 goto exit_host_stats;
 }
   
  
  Also, the only caller of this function doesn't use the return 
  value - it's overwritten by another function call:
  
  drivers/scsi/scsi_transport_iscsi.c:
  err = transport-get_host_stats(shost, buf, 
  host_stats_size);
  
  actual_size = nlmsg_total_size(sizeof(*ev) + 
  host_stats_size);
  skb_trim(skbhost_stats, NLMSG_ALIGN(actual_size));
  nlhhost_stats-nlmsg_len = actual_size;
  
  err = iscsi_multicast_skb(skbhost_stats, 
  ISCSI_NL_GRP_ISCSID,
GFP_KERNEL);
 
 Maybe that is intentional?  If get_host_stats fails, eg because of a lack 
 of memory, the worst that will happen is that a region of the buffer will 
 be 0.  If the loop is done again, there seems to be a risk of an infinite 
 loop.

Or one could jump out of the function completely, as on failure of 
alloc_skb.

julia
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 5/9] scsi: use correct structure type name in sizeof

[Julia Lawall]

From: Julia Lawall julia.law...@lip6.fr

Correct typo in the name of the type given to sizeof.  Because it is the
size of a pointer that is wanted, the typo has no impact on compilation or
execution.

This problem was found using Coccinelle (http://coccinelle.lip6.fr/).  The
semantic patch used can be found in message 0 of this patch series.

Signed-off-by: Julia Lawall julia.law...@lip6.fr

---
 drivers/scsi/eata.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/eata.c b/drivers/scsi/eata.c
index 813dd5c..4caf221 100644
--- a/drivers/scsi/eata.c
+++ b/drivers/scsi/eata.c
@@ -811,7 +811,7 @@ struct mscp {
struct sg_list *sglist; /* pointer to the allocated SG list */
 };
 
-#define CP_TAIL_SIZE (sizeof(struct sglist *) + sizeof(dma_addr_t))
+#define CP_TAIL_SIZE (sizeof(struct sg_list *) + sizeof(dma_addr_t))
 
 struct hostdata {
struct mscp cp[MAX_MAILBOXES];  /* Mailboxes for this board */

--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 7/9] scsi: use correct structure type name in sizeof

[Julia Lawall]

From: Julia Lawall julia.law...@lip6.fr

Correct typo in the name of the type given to sizeof.  Because it is the
size of a pointer that is wanted, the typo has no impact on compilation or
execution.

This problem was found using Coccinelle (http://coccinelle.lip6.fr/).  The
semantic patch used can be found in message 0 of this patch series.

Signed-off-by: Julia Lawall julia.law...@lip6.fr

---
 drivers/scsi/u14-34f.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/u14-34f.c b/drivers/scsi/u14-34f.c
index 4e76fe8..617d72a 100644
--- a/drivers/scsi/u14-34f.c
+++ b/drivers/scsi/u14-34f.c
@@ -585,7 +585,7 @@ struct mscp {
struct sg_list *sglist; /* pointer to the allocated SG list */
};
 
-#define CP_TAIL_SIZE (sizeof(struct sglist *) + sizeof(dma_addr_t))
+#define CP_TAIL_SIZE (sizeof(struct sg_list *) + sizeof(dma_addr_t))
 
 struct hostdata {
struct mscp cp[MAX_MAILBOXES];   /* Mailboxes for this board */

--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 0/9] use correct structure type name in sizeof

[Julia Lawall]

These patches fix typos in the name of a type referenced in a sizeof
command.  These problems are not caught by the compiler, because they have
no impact on execution - the size of a pointer is independent of the size
of the pointed value.

The semantic patch that finds these problems is shown below
(http://coccinelle.lip6.fr/).  This semantic patch distinguishes between
structures that are defined in C files, and thus are expected to be visible
in only that file, and structures that are defined in header files, which
could potential be visible everywhere.  This distinction seems to be
unnecessary in practice, though.

smpl
virtual after_start

@initialize:ocaml@
@@

type fl = C of string | H

let structures = Hashtbl.create 101
let restarted = ref false

let add_if_not_present _ =
  if not !restarted
  then
begin
  restarted := true;
  let it = new iteration() in
  it#add_virtual_rule After_start;
  it#register()
end

let hashadd str file =
  let cell =
try Hashtbl.find structures str
with Not_found -
  let cell = ref [] in
  Hashtbl.add structures str cell;
  cell in
  if not (List.mem file !cell) then cell := file :: !cell

let get_file fl =
  if Filename.check_suffix fl .c
  then C fl
  else H

@script:ocaml depends on !after_start@
@@
add_if_not_present()

@r depends on !after_start@
identifier nm;
position p;
@@

struct nm@p { ... };

@script:ocaml@
nm  r.nm;
p  r.p;
@@

hashadd nm (get_file (List.hd p).file)

// -

@sz depends on after_start@
identifier nm;
position p;
@@

sizeof(struct nm@p *)

@script:ocaml@
nm  sz.nm;
p  sz.p;
@@

try
  let allowed = !(Hashtbl.find structures nm) in
  if List.mem H allowed or List.mem (get_file (List.hd p).file) allowed
  then ()
  else print_main nm p
with Not_found - print_main nm p
/smpl

--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/1] dpt_i2o: delete unnecessary null test on array

[Julia Lawall]

From: Julia Lawall julia.law...@lip6.fr

Delete NULL test on array (always false).

A simplified version of the semantic match that finds this problem is as
follows: (http://coccinelle.lip6.fr/)

// smpl
@r@
type T;
T [] e;
position p;
@@
e ==@p NULL

@ disable fld_to_ptr@
expression e;
identifier f;
position r.p;
@@
* e.f ==@p NULL
// /smpl

Signed-off-by: Julia Lawall julia.law...@lip6.fr

---
I don't know if this is the correct change, or if some other test was
intended.  But the code has been this way since at least 2.4.20, so it
would seem that no one has been bothered by the lack of whatever this was
supposed to test for.

 drivers/scsi/dpt_i2o.c |5 -
 1 file changed, 5 deletions(-)

diff --git a/drivers/scsi/dpt_i2o.c b/drivers/scsi/dpt_i2o.c
index 67283ef..62e276b 100644
--- a/drivers/scsi/dpt_i2o.c
+++ b/drivers/scsi/dpt_i2o.c
@@ -1169,11 +1169,6 @@ static struct adpt_device* adpt_find_device(adpt_hba* 
pHba, u32 chan, u32 id, u6
if(chan  0 || chan = MAX_CHANNEL)
return NULL;

-   if( pHba-channel[chan].device == NULL){
-   printk(KERN_DEBUGAdaptec I2O RAID: Trying to find device 
before they are allocated\n);
-   return NULL;
-   }
-
d = pHba-channel[chan].device[id];
if(!d || d-tid == 0) {
return NULL;

--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 0/1] delete unnecessary null test on array

[Julia Lawall]

Delete NULL test on array.  The complete semantic patch that finds this
problem is as follows: (http://coccinelle.lip6.fr/)

// smpl
@r@
type T;
T [] e;
position p;
@@

(
 e ==@p NULL
|
 e !=@p NULL
|
 !@p e
)

@ disable fld_to_ptr@
expression e;
identifier f;
position r.p;
@@

(
* (e.f) ==@p NULL
|
* (e.f) !=@p NULL
|
* !@p(e.f)
)
// /smpl

For best results, this semantic patch requires lots of type information,
and should be used with the options --recursive-includes and
--relax-include-path.  This may take a long time to run.

--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/1] dpt_i2o: delete unnecessary null test on array

[Julia Lawall]

  diff --git a/drivers/scsi/dpt_i2o.c b/drivers/scsi/dpt_i2o.c
  index 67283ef..62e276b 100644
  --- a/drivers/scsi/dpt_i2o.c
  +++ b/drivers/scsi/dpt_i2o.c
  @@ -1169,11 +1169,6 @@ static struct adpt_device* 
  adpt_find_device(adpt_hba* pHba, u32 chan, u32 id, u6
  if(chan  0 || chan = MAX_CHANNEL)
  return NULL;
 
  -   if( pHba-channel[chan].device == NULL){
  -   printk(KERN_DEBUGAdaptec I2O RAID: Trying to find device 
  before they are allocated\n);
  -   return NULL;
  -   }

 dpt_i2o is always weirdly fun, but I think, based on the message, this
 check is supposed to be

 if( pHba-channel[chan].device[id] == NULL){

 Since device is an array of device pointers which are allocated by
 parsing data.

That seems to be already checked immediately below:

d = pHba-channel[chan].device[id];
if(!d || d-tid == 0) {
return NULL;
}

julia
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/1] dpt_i2o: delete unnecessary null test on array

[Julia Lawall]

On Fri, 8 Aug 2014, James Bottomley wrote:

 On Fri, 2014-08-08 at 19:03 +0200, Julia Lawall wrote:
diff --git a/drivers/scsi/dpt_i2o.c b/drivers/scsi/dpt_i2o.c
index 67283ef..62e276b 100644
--- a/drivers/scsi/dpt_i2o.c
+++ b/drivers/scsi/dpt_i2o.c
@@ -1169,11 +1169,6 @@ static struct adpt_device* 
adpt_find_device(adpt_hba* pHba, u32 chan, u32 id, u6
if(chan  0 || chan = MAX_CHANNEL)
return NULL;
   
-   if( pHba-channel[chan].device == NULL){
-   printk(KERN_DEBUGAdaptec I2O RAID: Trying to find 
device before they are allocated\n);
-   return NULL;
-   }
  
   dpt_i2o is always weirdly fun, but I think, based on the message, this
   check is supposed to be
  
   if( pHba-channel[chan].device[id] == NULL){
  
   Since device is an array of device pointers which are allocated by
   parsing data.
 
  That seems to be already checked immediately below:
 
  d = pHba-channel[chan].device[id];
  if(!d || d-tid == 0) {
  return NULL;

 Yes, I know, but no message is emitted.  The message seems to be for a
 violation of the state machine which device[id] = NULL implies.

OK, if the message seems helpful, then I can split up the tests.

julia
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/1] dpt_i2o: delete unnecessary null test on array

[Julia Lawall]

From nobody Sat Aug  9 08:17:15 CEST 2014
From: Julia Lawall julia.law...@lip6.fr
To: Adaptec OEM Raid Solutions aacr...@adaptec.com
Cc: James E.J. Bottomley 
jbottom...@parallels.com,linux-scsi@vger.kernel.org,linux-ker...@vger.kernel.org
Subject: [PATCH] dpt_i2o: delete unnecessary null test on array

From: Julia Lawall julia.law...@lip6.fr

Convert a NULL test on an array to a NULL test on its element.  Delete a
redundant test and clean up the code a little.

A simplified version of the semantic match that finds this problem is as
follows: (http://coccinelle.lip6.fr/)

// smpl
@r@
type T;
T [] e;
position p;
@@
e ==@p NULL

@ disable fld_to_ptr@
expression e;
identifier f;
position r.p;
@@
* e.f ==@p NULL
// /smpl

Signed-off-by: Julia Lawall julia.law...@lip6.fr

---
v2: Test instead the array element, and clean up the code a bit.

 drivers/scsi/dpt_i2o.c |9 -
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/scsi/dpt_i2o.c b/drivers/scsi/dpt_i2o.c
index 67283ef..4647c93 100644
--- a/drivers/scsi/dpt_i2o.c
+++ b/drivers/scsi/dpt_i2o.c
@@ -1169,15 +1169,14 @@ static struct adpt_device* adpt_find_device(adpt_hba* 
pHba, u32 chan, u32 id, u6
if(chan  0 || chan = MAX_CHANNEL)
return NULL;

-   if( pHba-channel[chan].device == NULL){
-   printk(KERN_DEBUGAdaptec I2O RAID: Trying to find device 
before they are allocated\n);
+   d = pHba-channel[chan].device[id];
+   if (!d) {
+   printk(KERN_DEBUG Adaptec I2O RAID: Trying to find device 
before they are allocated\n);
return NULL;
}
 
-   d = pHba-channel[chan].device[id];
-   if(!d || d-tid == 0) {
+   if (d-tid == 0)
return NULL;
-   }
 
/* If it is the only lun at that address then this should match*/
if(d-scsi_lun == lun){

--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 0/5] fix error return code

[Julia Lawall]

The complate semantic patch that finds this problem is as follows:
(http://coccinelle.lip6.fr/)

// smpl
@ok exists@
identifier f,ret,i;
expression e;
constant c;
@@

// identify a function that returns a negative return value at least once.
f(...) {
... when any
(
return -c@i;
|
ret = -c@i;
... when != ret = e
return ret;
|
if (ret  0) { ... return ret; }
)
... when any
}

@r exists@
identifier ret,ok.f,fn;
expression e1,e2,e3,e4,e5,e6,x;
statement S,S1;
position p1,p2,p3;
@@

// identify a case where the return variable is set to a non-negative value
// and then returned in error-handling code
f(...) {
... when any
(
if@p1 (\(ret  0\|ret != 0\))
 { ... return ret; }
|
ret@p1 = 0
)
... when != \(ret = e1\|ret++\|ret--\|ret+=e1\|ret-=e1\)
when != ret
when any
(
 if (+... ret = e5 ...+) S1
|
 if (+... ret ...+) S1
|
if@p2(+...x = fn(...)...+)
 {
  ... when != ret = e6
  when forall
 return@p3 ret;
}
|
break;
|
x = fn(...)
... when != \(ret = e4\|ret++\|ret--\|ret+=e4\|ret-=e4\)
when != ret
(
 if (+... ret = e3 ...+) S
|
 if (+... ret ...+) S
|
if@p2(+...\(x != 0\|x  0\|x == NULL\|IS_ERR(x)\)...+)
 {
  ... when != ret = e2
  when forall
 return@p3 ret;
}
)
)
... when any
}

@printer depends on r@
position p;
identifier ok.f,pr;
constant char [] c;
@@

f(...) { ...pr@p(...,c,...)... }

@bad0 exists@
identifier r.ret,ok.f,g != {ERR_PTR,IS_ERR};
position p != printer.p;
@@

f(...) { ... when any
g@p(...,ret,...)
... when any
 }

@bad depends on !bad0 exists@
position r.p1,r.p2;
statement S1,S2;
identifier r.ret;
expression e1;
@@

// ignore the above if there is some path where the variable is set to
// something else
(
if@p1 (\(ret  0\|ret != 0\)) S1
|
ret@p1 = 0
)
... when any
 \(ret = e1\|ret++\|ret--\|ret+=e1\|ret-=e1\|ret\)
... when any
if@p2(...) S2

@bad1 depends on !bad0  !bad exists@
position r.p2;
statement S2;
identifier r.ret;
expression e1;
constant c;
@@

ret = -c
... when != \(ret = e1\|ret++\|ret--\|ret+=e1\|ret-=e1\)
when != ret
when any
if@p2(...) S2

@bad2 depends on !bad0  !bad  !bad1 exists@
position r.p1,r.p2;
identifier r.ret;
expression e1;
statement S2;
constant c;
@@

// likewise ignore it if there has been an intervening return
ret@p1 = 0
... when != if (...) { ... ret = e1 ... return ret; }
when != if (...) { ... return -c; }
when any
if@p2(...) S2


@script:python depends on !bad0  !bad  !bad1  !bad2@
p1  r.p1;
p2  r.p2;
p3  r.p3;
@@

cocci.print_main(,p1)
cocci.print_secs(,p2)
cocci.print_secs(,p3)

// /smpl

--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/5] mptfusion: fix error return code

[Julia Lawall]

From: Julia Lawall julia.law...@lip6.fr

Return a negative error code on failure.

A simplified version of the semantic match that finds this problem is as
follows: (http://coccinelle.lip6.fr/)

// smpl
@@
identifier ret; expression e1,e2;
@@
(
if (\(ret  0\|ret != 0\))
 { ... return ret; }
|
ret = 0
)
... when != ret = e1
when != ret
*if(...)
{
  ... when != ret = e2
  when forall
 return ret;
}
// /smpl

Signed-off-by: Julia Lawall julia.law...@lip6.fr

---
 drivers/message/fusion/mptfc.c |4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/message/fusion/mptfc.c b/drivers/message/fusion/mptfc.c
index d8bf84a..629df6d 100644
--- a/drivers/message/fusion/mptfc.c
+++ b/drivers/message/fusion/mptfc.c
@@ -1325,8 +1325,10 @@ mptfc_probe(struct pci_dev *pdev, const struct 
pci_device_id *id)
 mptfc_wq_%d, sh-host_no);
ioc-fc_rescan_work_q =
create_singlethread_workqueue(ioc-fc_rescan_work_q_name);
-   if (!ioc-fc_rescan_work_q)
+   if (!ioc-fc_rescan_work_q) {
+   error = -ENOMEM;
goto out_mptfc_probe;
+   }
 
/*
 *  Pre-fetch FC port WWN and stuff...

--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/4] drivers/scsi/bfa/bfa_svc.c: remove unneeded structure

[Julia Lawall]

From: Julia Lawall julia.law...@lip6.fr

Delete a local structure that is only used to be initialized by memset.

A semantic patch that makes this change is as follows:
(http://coccinelle.lip6.fr/)

// smpl
@@
identifier x,i;
@@

{
... when any
-struct i x;
+... when != x
- memset(x,...);
...+
}
// /smpl

Signed-off-by: Julia Lawall julia.law...@lip6.fr

---
 drivers/scsi/bfa/bfa_svc.c |6 --
 1 file changed, 6 deletions(-)

diff --git a/drivers/scsi/bfa/bfa_svc.c b/drivers/scsi/bfa/bfa_svc.c
index 625225f..cf46683 100644
--- a/drivers/scsi/bfa/bfa_svc.c
+++ b/drivers/scsi/bfa/bfa_svc.c
@@ -404,13 +404,10 @@ bfa_plog_fchdr(struct bfa_plog_s *plog, enum bfa_plog_mid 
mid,
enum bfa_plog_eid event,
u16 misc, struct fchs_s *fchdr)
 {
-   struct bfa_plog_rec_s  lp;
u32 *tmp_int = (u32 *) fchdr;
u32 ints[BFA_PL_INT_LOG_SZ];
 
if (plog-plog_enabled) {
-   memset(lp, 0, sizeof(struct bfa_plog_rec_s));
-
ints[0] = tmp_int[0];
ints[1] = tmp_int[1];
ints[2] = tmp_int[4];
@@ -424,13 +421,10 @@ bfa_plog_fchdr_and_pl(struct bfa_plog_s *plog, enum 
bfa_plog_mid mid,
  enum bfa_plog_eid event, u16 misc, struct fchs_s *fchdr,
  u32 pld_w0)
 {
-   struct bfa_plog_rec_s  lp;
u32 *tmp_int = (u32 *) fchdr;
u32 ints[BFA_PL_INT_LOG_SZ];
 
if (plog-plog_enabled) {
-   memset(lp, 0, sizeof(struct bfa_plog_rec_s));
-
ints[0] = tmp_int[0];
ints[1] = tmp_int[1];
ints[2] = tmp_int[4];

--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 0/4] remove unneeded array

[Julia Lawall]

Remove an array or structure that only serves as the first argument to
memset.  The complete semantic patch that makes this change is as follows:
(http://coccinelle.lip6.fr/)

// smpl
@@
identifier x;
type T;
@@

{
... when any
-T x[...];
+... when != x
- memset(x,...);
...+
}

@@
identifier x,i;
@@

{
... when any
-struct i x;
+... when != x
- memset(x,...);
...+
}
// /smpl

--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/4] target: remove unneeded array

[Julia Lawall]

From: Julia Lawall julia.law...@lip6.fr

Delete a local array that is only used to be initialized by memset.

A semantic patch that makes this change is as follows:
(http://coccinelle.lip6.fr/)

// smpl
@@
identifier x;
type T;
@@

{
... when any
-T x[...];
+... when != x
- memset(x,...);
...+
}
// /smpl

Signed-off-by: Julia Lawall julia.law...@lip6.fr

---
 drivers/target/target_core_pr.c |7 ++-
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/target/target_core_pr.c b/drivers/target/target_core_pr.c
index 4c261c3..a0f850b 100644
--- a/drivers/target/target_core_pr.c
+++ b/drivers/target/target_core_pr.c
@@ -1429,14 +1429,12 @@ core_scsi3_decode_spec_i_port(
struct target_core_fabric_ops *tmp_tf_ops;
unsigned char *buf;
unsigned char *ptr, *i_str = NULL, proto_ident, tmp_proto_ident;
-   char *iport_ptr = NULL, dest_iport[64], i_buf[PR_REG_ISID_ID_LEN];
+   char *iport_ptr = NULL, i_buf[PR_REG_ISID_ID_LEN];
sense_reason_t ret;
u32 tpdl, tid_len = 0;
int dest_local_nexus;
u32 dest_rtpi = 0;
 
-   memset(dest_iport, 0, 64);
-
local_se_deve = se_sess-se_node_acl-device_list[cmd-orig_fe_lun];
/*
 * Allocate a struct pr_transport_id_holder and setup the
@@ -3059,7 +3057,7 @@ core_scsi3_emulate_pro_register_and_move(struct se_cmd 
*cmd, u64 res_key,
struct t10_reservation *pr_tmpl = dev-t10_pr;
unsigned char *buf;
unsigned char *initiator_str;
-   char *iport_ptr = NULL, dest_iport[64], i_buf[PR_REG_ISID_ID_LEN];
+   char *iport_ptr = NULL, i_buf[PR_REG_ISID_ID_LEN];
u32 tid_len, tmp_tid_len;
int new_reg = 0, type, scope, matching_iname;
sense_reason_t ret;
@@ -3071,7 +3069,6 @@ core_scsi3_emulate_pro_register_and_move(struct se_cmd 
*cmd, u64 res_key,
return TCM_LOGICAL_UNIT_COMMUNICATION_FAILURE;
}
 
-   memset(dest_iport, 0, 64);
memset(i_buf, 0, PR_REG_ISID_ID_LEN);
se_tpg = se_sess-se_tpg;
tf_ops = se_tpg-se_tpg_tfo;

--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [patch] mpt2sas: issue reset is not uninitialized

[Julia Lawall]

On Wed, 3 Dec 2014, Dan Carpenter wrote:

 The issue_reset variables were never set to zero.  This bug was found
 with static analysis.

 Signed-off-by: Dan Carpenter dan.carpen...@oracle.com

 diff --git a/drivers/scsi/mpt2sas/mpt2sas_base.c 
 b/drivers/scsi/mpt2sas/mpt2sas_base.c
 index 58e4521..98bd546 100644
 --- a/drivers/scsi/mpt2sas/mpt2sas_base.c
 +++ b/drivers/scsi/mpt2sas/mpt2sas_base.c
 @@ -3236,7 +3236,7 @@ mpt2sas_base_sas_iounit_control(struct MPT2SAS_ADAPTER 
 *ioc,
   u16 smid;
   u32 ioc_state;
   unsigned long timeleft;
 - u8 issue_reset;
 + bool issue_reset = 0;

Why not false if it is going to be a boolean?

julia

   int rc;
   void *request;
   u16 wait_state_count;
 @@ -3341,7 +3341,7 @@ mpt2sas_base_scsi_enclosure_processor(struct 
 MPT2SAS_ADAPTER *ioc,
   u16 smid;
   u32 ioc_state;
   unsigned long timeleft;
 - u8 issue_reset;
 + bool issue_reset = 0;
   int rc;
   void *request;
   u16 wait_state_count;
 --
 To unsubscribe from this list: send the line unsubscribe kernel-janitors in
 the body of a message to majord...@vger.kernel.org
 More majordomo info at  http://vger.kernel.org/majordomo-info.html

--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 3/4] drivers/scsi/lpfc: Use time_before, time_before_eq, etc.

[Julia Lawall]

From: Julia Lawall [EMAIL PROTECTED]

The functions time_before, time_before_eq, time_after, and time_after_eq
are more robust for comparing jiffies against other values.

A simplified version of the semantic patch making this change is as follows:
(http://www.emn.fr/x-info/coccinelle/)

// smpl
@ change_compare_np @
expression E;
@@

(
- jiffies = E
+ time_before_eq(jiffies,E)
|
- jiffies = E
+ time_after_eq(jiffies,E)
|
- jiffies  E
+ time_before(jiffies,E)
|
- jiffies  E
+ time_after(jiffies,E)
)

@ include depends on change_compare_np @
@@

#include linux/jiffies.h

@ no_include depends on !include  change_compare_np @
@@

  #include linux/...
+ #include linux/jiffies.h
// /smpl

Signed-off-by: Julia Lawall [EMAIL PROTECTED]
---

diff -u -p linux-2.6/drivers/scsi/lpfc/lpfc_scsi.c 
linuxcopy/drivers/scsi/lpfc/lpfc_scsi.c
--- linux-2.6/drivers/scsi/lpfc/lpfc_scsi.c 2007-11-08 08:00:52.0 
+0100
+++ linuxcopy/drivers/scsi/lpfc/lpfc_scsi.c 2007-12-25 20:53:54.0 
+0100
@@ -22,6 +22,7 @@
 #include linux/pci.h
 #include linux/interrupt.h
 #include linux/delay.h
+#include linux/jiffies.h

 #include scsi/scsi.h
 #include scsi/scsi_device.h
@@ -55,7 +56,8 @@ lpfc_adjust_queue_depth(struct lpfc_hba
atomic_inc(phba-num_rsrc_err);
phba-last_rsrc_error_time = jiffies;

-   if ((phba-last_ramp_down_time + QUEUE_RAMP_DOWN_INTERVAL)  jiffies) {
+   if (time_before(jiffies,
+   phba-last_ramp_down_time+QUEUE_RAMP_DOWN_INTERVAL)) {
spin_unlock_irqrestore(phba-hbalock, flags);
return;
}
@@ -94,8 +96,10 @@ lpfc_rampup_queue_depth(struct lpfc_vpor
if (vport-cfg_lun_queue_depth = sdev-queue_depth)
return;
spin_lock_irqsave(phba-hbalock, flags);
-   if (((phba-last_ramp_up_time + QUEUE_RAMP_UP_INTERVAL)  jiffies) ||
-((phba-last_rsrc_error_time + QUEUE_RAMP_UP_INTERVAL )  jiffies)) {
+   if ((time_before(jiffies,
+phba-last_ramp_up_time + QUEUE_RAMP_UP_INTERVAL)) ||
+(time_before(jiffies,
+ phba-last_rsrc_error_time + QUEUE_RAMP_UP_INTERVAL))) {
spin_unlock_irqrestore(phba-hbalock, flags);
return;
}
@@ -617,10 +621,12 @@ lpfc_scsi_cmd_iocb_cmpl(struct lpfc_hba
lpfc_rampup_queue_depth(vport, sdev);

if (!result  pnode != NULL 
-  ((jiffies - pnode-last_ramp_up_time) 
-   LPFC_Q_RAMP_UP_INTERVAL * HZ) 
-  ((jiffies - pnode-last_q_full_time) 
-   LPFC_Q_RAMP_UP_INTERVAL * HZ) 
+  (time_after(jiffies,
+  pnode-last_ramp_up_time +
+  LPFC_Q_RAMP_UP_INTERVAL * HZ)) 
+  (time_after(jiffies,
+  pnode-last_q_full_time +
+  LPFC_Q_RAMP_UP_INTERVAL * HZ)) 
   (vport-cfg_lun_queue_depth  sdev-queue_depth)) {
shost_for_each_device(tmp_sdev, sdev-host) {
if (vport-cfg_lun_queue_depth  tmp_sdev-queue_depth){
-
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 4/4] drivers/scsi/u14-34f.c: Use time_before, time_before_eq, etc.

[Julia Lawall]

From: Julia Lawall [EMAIL PROTECTED]

The functions time_before, time_before_eq, time_after, and time_after_eq
are more robust for comparing jiffies against other values.

A simplified version of the semantic patch making this change is as follows:
(http://www.emn.fr/x-info/coccinelle/)

// smpl
@ change_compare_np @
expression E;
@@

(
- jiffies = E
+ time_before_eq(jiffies,E)
|
- jiffies = E
+ time_after_eq(jiffies,E)
|
- jiffies  E
+ time_before(jiffies,E)
|
- jiffies  E
+ time_after(jiffies,E)
)

@ include depends on change_compare_np @
@@

#include linux/jiffies.h

@ no_include depends on !include  change_compare_np @
@@

  #include linux/...
+ #include linux/jiffies.h
// /smpl

Signed-off-by: Julia Lawall [EMAIL PROTECTED]
---

diff -u -p linux-2.6/drivers/scsi/u14-34f.c linuxcopy/drivers/scsi/u14-34f.c
--- linux-2.6/drivers/scsi/u14-34f.c2007-10-22 11:25:24.0 +0200
+++ linuxcopy/drivers/scsi/u14-34f.c2007-12-26 16:09:12.0 +0100
@@ -420,6 +420,7 @@
 #include linux/init.h
 #include linux/ctype.h
 #include linux/spinlock.h
+#include linux/jiffies.h
 #include asm/dma.h
 #include asm/irq.h

@@ -745,7 +746,8 @@ static int wait_on_busy(unsigned long io
 static int board_inquiry(unsigned int j) {
struct mscp *cpp;
dma_addr_t id_dma_addr;
-   unsigned int time, limit = 0;
+   unsigned long time;
+   unsigned int limit = 0;

id_dma_addr = pci_map_single(HD(j)-pdev, HD(j)-board_id,
 sizeof(HD(j)-board_id), PCI_DMA_BIDIRECTIONAL);
@@ -778,7 +780,7 @@ static int board_inquiry(unsigned int j)

spin_unlock_irq(driver_lock);
time = jiffies;
-   while ((jiffies - time)  HZ  limit++  2) udelay(100L);
+   while (time_before(jiffies, time + HZ)  limit++  2) udelay(100L);
spin_lock_irq(driver_lock);

if (cpp-adapter_status || HD(j)-cp_stat[0] != FREE) {
@@ -1393,7 +1395,8 @@ static int u14_34f_eh_abort(struct scsi_
 }

 static int u14_34f_eh_host_reset(struct scsi_cmnd *SCarg) {
-   unsigned int i, j, time, k, c, limit = 0;
+   unsigned long time;
+   unsigned int i, j, k, c, limit = 0;
int arg_done = FALSE;
struct scsi_cmnd *SCpnt;

@@ -1479,7 +1482,8 @@ static int u14_34f_eh_host_reset(struct

spin_unlock_irq(sh[j]-host_lock);
time = jiffies;
-   while ((jiffies - time)  (10 * HZ)  limit++  20) udelay(100L);
+   while (time_before(jiffies, time + 10 * HZ)  limit++  20)
+   udelay(100L);
spin_lock_irq(sh[j]-host_lock);

printk(%s: reset, interrupts disabled, loops %d.\n, BN(j), limit);
-
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 3/3] drivers/scsi: Use offsetof

[Julia Lawall]

From: Julia Lawall [EMAIL PROTECTED]

In the patch cc154ac64aa8d3396b187f64cef01ce67f433324, Al Viro observed
that the proper way to compute the distance between two structure fields is
to use offsetof() or a cast to a pointer to character.  The same change can
be applied to a few more files.

The change was made using the following semantic patch
(http://www.emn.fr/x-info/coccinelle/)

// smpl
@r3 disable ptr_to_array@
type T;
T *s;
type T1, T2;
identifier fld1;
@@

(
  (char *)s-fld1 - (char *)s
|
  (uint8_t *)s-fld1 - (uint8_t *)s
|
  (u8 *)s-fld1 - (u8 *)s
|
- (T1)s-fld1 - (T2)s
+ offsetof(T,fld1)
)
// /smpl

Signed-off-by: Julia Lawall [EMAIL PROTECTED]
---

diff -u -p a/drivers/scsi/BusLogic.c b/drivers/scsi/BusLogic.c
--- a/drivers/scsi/BusLogic.c   2007-10-22 11:25:20.0 +0200
+++ b/drivers/scsi/BusLogic.c   2007-12-26 16:33:42.0 +0100
@@ -2856,7 +2856,10 @@ static int BusLogic_QueueCommand(struct
CCB-Opcode = BusLogic_InitiatorCCB_ScatterGather;
CCB-DataLength = Count * sizeof(struct 
BusLogic_ScatterGatherSegment);
if (BusLogic_MultiMasterHostAdapterP(HostAdapter))
-   CCB-DataPointer = (unsigned int) CCB-DMA_Handle + 
((unsigned long) CCB-ScatterGatherList - (unsigned long) CCB);
+   CCB-DataPointer =
+   (unsigned int) CCB-DMA_Handle +
+   offsetof(struct BusLogic_CCB,
+ScatterGatherList);
else
CCB-DataPointer = 
Virtual_to_32Bit_Virtual(CCB-ScatterGatherList);
-
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 5/16] drivers/scsi: use WARN

[Julia Lawall]

From: Julia Lawall julia.law...@lip6.fr

Use WARN rather than printk followed by WARN_ON(1), for conciseness.

A simplified version of the semantic patch that makes this transformation
is as follows: (http://coccinelle.lip6.fr/)

// smpl
@@
expression list es;
@@

-printk(
+WARN(1,
  es);
-WARN_ON(1);
// /smpl

Signed-off-by: Julia Lawall julia.law...@lip6.fr

---
 drivers/scsi/initio.c   |3 +--
 drivers/scsi/scsi_lib.c |3 +--
 2 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/scsi/initio.c b/drivers/scsi/initio.c
index dd741bc..1572860 100644
--- a/drivers/scsi/initio.c
+++ b/drivers/scsi/initio.c
@@ -2774,8 +2774,7 @@ static void i91uSCBPost(u8 * host_mem, u8 * cblk_mem)
host = (struct initio_host *) host_mem;
cblk = (struct scsi_ctrl_blk *) cblk_mem;
if ((cmnd = cblk-srb) == NULL) {
-   printk(KERN_ERR i91uSCBPost: SRB pointer is empty\n);
-   WARN_ON(1);
+   WARN(1, KERN_ERR i91uSCBPost: SRB pointer is empty\n);
initio_release_scb(host, cblk); /* Release SCB for current 
channel */
return;
}
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index da36a3a..e5fdcae 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -2573,10 +2573,9 @@ void *scsi_kmap_atomic_sg(struct scatterlist *sgl, int 
sg_count,
}
 
if (unlikely(i == sg_count)) {
-   printk(KERN_ERR %s: Bytes in sg: %zu, requested offset %zu, 
+   WARN(1, KERN_ERR %s: Bytes in sg: %zu, requested offset %zu, 
elements %d\n,
   __func__, sg_len, *offset, sg_count);
-   WARN_ON(1);
return NULL;
}
 

--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 7/16] drivers/scsi/gdth.c: use WARN

[Julia Lawall]

From: Julia Lawall julia.law...@lip6.fr

Use WARN rather than printk followed by WARN_ON(1), for conciseness.

If (count) is also merged into WARN, for further conciseness.

A simplified version of the semantic patch that makes part of this
transformation is as follows: (http://coccinelle.lip6.fr/)

// smpl
@@
expression list es;
@@

-printk(
+WARN(1,
  es);
-WARN_ON(1);
// /smpl

Signed-off-by: Julia Lawall julia.law...@lip6.fr

---
 drivers/scsi/gdth.c |7 +++
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/scsi/gdth.c b/drivers/scsi/gdth.c
index 5d72274..0dbcb27 100644
--- a/drivers/scsi/gdth.c
+++ b/drivers/scsi/gdth.c
@@ -2318,11 +2318,10 @@ static void gdth_copy_internal_data(gdth_ha_str *ha, 
Scsi_Cmnd *scp,
 break;
 buffer += cpnow;
 }
-} else if (count) {
-printk(GDT-HA %d: SCSI command with no buffers but data transfer 
expected!\n,
-   ha-hanum);
-WARN_ON(1);
 }
+   else
+   WARN(count, GDT-HA %d: SCSI command with no buffers but data 
transfer expected!\n,
+ha-hanum);
 }
 
 static int gdth_internal_cache_cmd(gdth_ha_str *ha, Scsi_Cmnd *scp)
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/8] drivers/scsi/bfa/bfa_svc.c: drop if around WARN_ON

[Julia Lawall]

From: Julia Lawall julia.law...@lip6.fr

Just use WARN_ON rather than an if containing only WARN_ON(1).

A simplified version of the semantic patch that makes this transformation
is as follows: (http://coccinelle.lip6.fr/)

// smpl
@@
expression e;
@@
- if (e) WARN_ON(1);
+ WARN_ON(e);
// /smpl

Signed-off-by: Julia Lawall julia.law...@lip6.fr

---
 drivers/scsi/bfa/bfa_svc.c |3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/scsi/bfa/bfa_svc.c b/drivers/scsi/bfa/bfa_svc.c
index 299c1c8..765b8d0 100644
--- a/drivers/scsi/bfa/bfa_svc.c
+++ b/drivers/scsi/bfa/bfa_svc.c
@@ -626,8 +626,7 @@ bfa_fcxp_init_reqrsp(struct bfa_fcxp_s *fcxp,
/*
 * alloc required sgpgs
 */
-   if (n_sgles  BFI_SGE_INLINE)
-   WARN_ON(1);
+   WARN_ON(n_sgles  BFI_SGE_INLINE);
}
 
 }

--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 7/8] drivers/scsi/scsi_transport_fc.c: drop if around WARN_ON

[Julia Lawall]

From: Julia Lawall julia.law...@lip6.fr

Just use WARN_ON rather than an if containing only WARN_ON(1).

A simplified version of the semantic patch that makes this transformation
is as follows: (http://coccinelle.lip6.fr/)

// smpl
@@
expression e;
@@
- if (e) WARN_ON(1);
+ WARN_ON(e);
// /smpl

Signed-off-by: Julia Lawall julia.law...@lip6.fr

---
 drivers/scsi/scsi_transport_fc.c |5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/scsi/scsi_transport_fc.c b/drivers/scsi/scsi_transport_fc.c
index e894ca7..f54c945 100644
--- a/drivers/scsi/scsi_transport_fc.c
+++ b/drivers/scsi/scsi_transport_fc.c
@@ -1699,9 +1699,8 @@ fc_stat_show(const struct device *dev, char *buf, 
unsigned long offset)
struct fc_host_statistics *stats;
ssize_t ret = -ENOENT;
 
-   if (offset  sizeof(struct fc_host_statistics) ||
-   offset % sizeof(u64) != 0)
-   WARN_ON(1);
+   WARN_ON(offset  sizeof(struct fc_host_statistics) ||
+   offset % sizeof(u64) != 0);
 
if (i-f-get_fc_host_stats) {
stats = (i-f-get_fc_host_stats)(shost);

--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 5/8] drivers/scsi/qla2xxx/qla_nx.c: drop if around WARN_ON

[Julia Lawall]

From: Julia Lawall julia.law...@lip6.fr

Just use WARN_ON rather than an if containing only WARN_ON(1).

A simplified version of the semantic patch that makes this transformation
is as follows: (http://coccinelle.lip6.fr/)

// smpl
@@
expression e;
@@
- if (e) WARN_ON(1);
+ WARN_ON(e);
// /smpl

Signed-off-by: Julia Lawall julia.law...@lip6.fr

---
 drivers/scsi/qla2xxx/qla_nx.c |3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/scsi/qla2xxx/qla_nx.c b/drivers/scsi/qla2xxx/qla_nx.c
index f5e297c..4c62a5d 100644
--- a/drivers/scsi/qla2xxx/qla_nx.c
+++ b/drivers/scsi/qla2xxx/qla_nx.c
@@ -388,8 +388,7 @@ qla82xx_pci_set_crbwindow(struct qla_hw_data *ha, u64 off)
 
if ((off = QLA82XX_CRB_PCIX_HOST)  (off  QLA82XX_CRB_PCIX_HOST2)) {
/* We are in first CRB window */
-   if (ha-curr_window != 0)
-   WARN_ON(1);
+   WARN_ON(ha-curr_window != 0);
return off;
}
 

--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

merging printk and WARN

[Julia Lawall]

It looks like these patches were not a good idea, because in each case the 
printk provides an error level, and WARN then provides another one.


Sorry for the noise.

julia
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 5/15] drivers/scsi/bnx2fc/bnx2fc_io.c: adjust duplicate test

[Julia Lawall]

From: Julia Lawall julia.law...@lip6.fr

Delete successive tests to the same location.  The code tested the result
of a previous allocation, that itself was already tested.  It is changed to
test the result of the most recent allocation.

A simplified version of the semantic match that finds this problem is as
follows: (http://coccinelle.lip6.fr/)

// smpl
@s exists@
local idexpression y;
expression x,e;
@@

*if ( \(x == NULL\|IS_ERR(x)\|y != 0\) )
 { ... when forall
   return ...; }
... when != \(y = e\|y += e\|y -= e\|y |= e\|y = e\|y++\|y--\|y\)
when != \(XT_GETPAGE(...,y)\|WMI_CMD_BUF(...)\)
*if ( \(x == NULL\|IS_ERR(x)\|y != 0\) )
 { ... when forall
   return ...; }
// /smpl

Signed-off-by: Julia Lawall julia.law...@lip6.fr

---
 drivers/scsi/bnx2fc/bnx2fc_io.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/bnx2fc/bnx2fc_io.c b/drivers/scsi/bnx2fc/bnx2fc_io.c
index 8d4626c..8bea53d 100644
--- a/drivers/scsi/bnx2fc/bnx2fc_io.c
+++ b/drivers/scsi/bnx2fc/bnx2fc_io.c
@@ -654,7 +654,7 @@ int bnx2fc_init_mp_req(struct bnx2fc_cmd *io_req)
mp_req-mp_resp_bd = dma_alloc_coherent(hba-pcidev-dev, sz,
 mp_req-mp_resp_bd_dma,
 GFP_ATOMIC);
-   if (!mp_req-mp_req_bd) {
+   if (!mp_req-mp_resp_bd) {
printk(KERN_ERR PFX unable to alloc MP resp bd\n);
bnx2fc_free_mp_resc(io_req);
return FAILED;

--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 10/15] drivers/scsi/csiostor/csio_lnode.c: adjust duplicate test

[Julia Lawall]

From: Julia Lawall julia.law...@lip6.fr

Delete successive tests to the same location.  The current value of ln has
been tested already, and is clearly not NULL, given the enclosing if
condition.  Furthermore, the result of csio_lnode_alloc is currently
ignored.

A simplified version of the semantic match that finds this problem is as
follows: (http://coccinelle.lip6.fr/)

// smpl
@s exists@
local idexpression y;
expression x,e;
@@

*if ( \(x == NULL\|IS_ERR(x)\|y != 0\) )
 { ... when forall
   return ...; }
... when != \(y = e\|y += e\|y -= e\|y |= e\|y = e\|y++\|y--\|y\)
when != \(XT_GETPAGE(...,y)\|WMI_CMD_BUF(...)\)
*if ( \(x == NULL\|IS_ERR(x)\|y != 0\) )
 { ... when forall
   return ...; }
// /smpl

Signed-off-by: Julia Lawall julia.law...@lip6.fr

---
Not tested.  I don't know if this is the right fix.

 drivers/scsi/csiostor/csio_lnode.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/csiostor/csio_lnode.c 
b/drivers/scsi/csiostor/csio_lnode.c
index ffe9be0..d956f97 100644
--- a/drivers/scsi/csiostor/csio_lnode.c
+++ b/drivers/scsi/csiostor/csio_lnode.c
@@ -873,7 +873,7 @@ csio_handle_link_up(struct csio_hw *hw, uint8_t portid, 
uint32_t fcfi,
if (ln-vnp_flowid != CSIO_INVALID_IDX) {
/* New VN-Port */
spin_unlock_irq(hw-lock);
-   csio_lnode_alloc(hw);
+   ln = csio_lnode_alloc(hw);
spin_lock_irq(hw-lock);
if (!ln) {
csio_err(hw,
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/25] fix error return code

[Julia Lawall]

From: Julia Lawall julia.law...@lip6.fr

Set the return variable to an error code as done elsewhere in the function.

Additionally, in each case there is no need to initialize ret to 0, since
its value is immediately overwritten.

A simplified version of the semantic match that finds the first problem is as
follows: (http://coccinelle.lip6.fr/)

// smpl
(
if@p1 (\(ret  0\|ret != 0\))
 { ... return ret; }
|
ret@p1 = 0
)
... when != ret = e1
when != ret
*if(...)
{
  ... when != ret = e2
  when forall
 return ret;
}

// /smpl

Signed-off-by: Julia Lawall julia.law...@lip6.fr

---
Not tested.

 drivers/scsi/be2iscsi/be_main.c |   10 +++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/scsi/be2iscsi/be_main.c b/drivers/scsi/be2iscsi/be_main.c
index 1f37505..82f4587 100644
--- a/drivers/scsi/be2iscsi/be_main.c
+++ b/drivers/scsi/be2iscsi/be_main.c
@@ -4326,7 +4326,7 @@ static int beiscsi_get_boot_info(struct beiscsi_hba *phba)
struct be_dma_mem nonemb_cmd;
unsigned int tag;
unsigned int s_handle;
-   int ret = -ENOMEM;
+   int ret;
 
/* Get the session handle of the boot target */
ret = be_mgmt_get_boot_shandle(phba, s_handle);
@@ -4356,6 +4356,7 @@ static int beiscsi_get_boot_info(struct beiscsi_hba *phba)
BEISCSI_LOG_INIT | BEISCSI_LOG_CONFIG,
BM_%d : beiscsi_get_session_info
 Failed\n);
+   ret = -ENOMEM;
 
goto boot_freemem;
}
@@ -4455,7 +4456,8 @@ static int beiscsi_init_port(struct beiscsi_hba *phba)
goto do_cleanup_ctrlr;
}
 
-   if (hba_setup_cid_tbls(phba)) {
+   ret = hba_setup_cid_tbls(phba);
+   if (ret  0) {
beiscsi_log(phba, KERN_ERR, BEISCSI_LOG_INIT,
BM_%d : Failed in hba_setup_cid_tbls\n);
kfree(phba-io_sgl_hndl_base);
@@ -5479,7 +5481,7 @@ static int beiscsi_dev_probe(struct pci_dev *pcidev,
struct hwi_controller *phwi_ctrlr;
struct hwi_context_memory *phwi_context;
struct be_eq_obj *pbe_eq;
-   int ret = 0, i;
+   int ret, i;
 
ret = beiscsi_enable_pci(pcidev);
if (ret  0) {
@@ -5492,6 +5494,7 @@ static int beiscsi_dev_probe(struct pci_dev *pcidev,
if (!phba) {
dev_err(pcidev-dev,
beiscsi_dev_probe - Failed in beiscsi_hba_alloc\n);
+   ret = -ENOMEM;
goto disable_pci;
}
 
@@ -5586,6 +5589,7 @@ static int beiscsi_dev_probe(struct pci_dev *pcidev,
beiscsi_log(phba, KERN_ERR, BEISCSI_LOG_INIT,
BM_%d : beiscsi_dev_probe-
Failed in beiscsi_init_port\n);
+   ret = -ENOMEM;
goto free_port;
}
 

--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 19/25] fix error return code

[Julia Lawall]

From: Julia Lawall julia.law...@lip6.fr

Set the return variable to an error code as done elsewhere in the function.

A simplified version of the semantic match that finds this problem is as
follows: (http://coccinelle.lip6.fr/)

// smpl
(
if@p1 (\(ret  0\|ret != 0\))
 { ... return ret; }
|
ret@p1 = 0
)
... when != ret = e1
when != ret
*if(...)
{
  ... when != ret = e2
  when forall
 return ret;
}

// /smpl

Signed-off-by: Julia Lawall julia.law...@lip6.fr

---
Not tested.

 drivers/scsi/ps3rom.c |1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/scsi/ps3rom.c b/drivers/scsi/ps3rom.c
index e6e2a30..04d77ce 100644
--- a/drivers/scsi/ps3rom.c
+++ b/drivers/scsi/ps3rom.c
@@ -387,6 +387,7 @@ static int ps3rom_probe(struct ps3_system_bus_device *_dev)
if (!host) {
dev_err(dev-sbd.core, %s:%u: scsi_host_alloc failed\n,
__func__, __LINE__);
+   error = -ENOMEM;
goto fail_teardown;
}
 

--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 9/25] fix error return code

[Julia Lawall]

From: Julia Lawall julia.law...@lip6.fr

Set the return variable to an error code as done elsewhere in the function.

A simplified version of the semantic match that finds this problem is as
follows: (http://coccinelle.lip6.fr/)

// smpl
(
if@p1 (\(ret  0\|ret != 0\))
 { ... return ret; }
|
ret@p1 = 0
)
... when != ret = e1
when != ret
*if(...)
{
  ... when != ret = e2
  when forall
 return ret;
}

// /smpl

Signed-off-by: Julia Lawall julia.law...@lip6.fr

---
Not tested.

 drivers/scsi/fnic/fnic_main.c |   12 +---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/scsi/fnic/fnic_main.c b/drivers/scsi/fnic/fnic_main.c
index 33e4ec2..a3472a1 100644
--- a/drivers/scsi/fnic/fnic_main.c
+++ b/drivers/scsi/fnic/fnic_main.c
@@ -731,17 +731,23 @@ static int fnic_probe(struct pci_dev *pdev, const struct 
pci_device_id *ent)
spin_lock_init(fnic-io_req_lock[i]);
 
fnic-io_req_pool = mempool_create_slab_pool(2, fnic_io_req_cache);
-   if (!fnic-io_req_pool)
+   if (!fnic-io_req_pool) {
+   err = -ENOMEM;
goto err_out_free_resources;
+   }
 
pool = mempool_create_slab_pool(2, fnic_sgl_cache[FNIC_SGL_CACHE_DFLT]);
-   if (!pool)
+   if (!pool) {
+   err = -ENOMEM;
goto err_out_free_ioreq_pool;
+   }
fnic-io_sgl_pool[FNIC_SGL_CACHE_DFLT] = pool;
 
pool = mempool_create_slab_pool(2, fnic_sgl_cache[FNIC_SGL_CACHE_MAX]);
-   if (!pool)
+   if (!pool) {
+   err = -ENOMEM;
goto err_out_free_dflt_pool;
+   }
fnic-io_sgl_pool[FNIC_SGL_CACHE_MAX] = pool;
 
/* setup vlan config, hw inserts vlan header */

--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 24/25] fix error return code

[Julia Lawall]

From: Julia Lawall julia.law...@lip6.fr

Set the return variable to an error code as done elsewhere in the function.

A simplified version of the semantic match that finds this problem is as
follows: (http://coccinelle.lip6.fr/)

// smpl
(
if@p1 (\(ret  0\|ret != 0\))
 { ... return ret; }
|
ret@p1 = 0
)
... when != ret = e1
when != ret
*if(...)
{
  ... when != ret = e2
  when forall
 return ret;
}

// /smpl

Signed-off-by: Julia Lawall julia.law...@lip6.fr

---
Not tested.

 drivers/scsi/bnx2fc/bnx2fc_fcoe.c |1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/scsi/bnx2fc/bnx2fc_fcoe.c 
b/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
index 9b94850..d8cb194 100644
--- a/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
+++ b/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
@@ -2199,6 +2199,7 @@ static int _bnx2fc_create(struct net_device *netdev,
interface = bnx2fc_interface_create(hba, netdev, fip_mode);
if (!interface) {
printk(KERN_ERR PFX bnx2fc_interface_create failed\n);
+   rc = -ENOMEM;
goto ifput_err;
}
 

--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 0/25] fix error return code

[Julia Lawall]

These patches fix cases where the return variable is not set to an error
code in an error case.

--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 0/13] make return of 0 explicit

[Julia Lawall]

Sometimes a local variable is used as a return value in a case where the
return value is always 0.  The result is more apparent if this variable is
just replaced by 0.  This is done by the following semantic patch, although
some cleanups may be needed. (http://coccinelle.lip6.fr/)

// smpl
@r exists@
local idexpression ret;
identifier reti;
expression e;
position p;
@@

ret@reti = 0;
... when != \(ret = e\|ret = e\|ret |= e\|ret += e\|ret -= e\|ret *= e\|ret ^= 
e\)
when != \(++ret\|--ret\|ret++\|ret--\)
when != ret
return ret;@p

@bad1 exists@
expression e != 0;
local idexpression r.ret;
position r.p;
@@

 \(ret = e\|ret = e\|ret |= e\|ret += e\|ret -= e\|ret *= e\|ret ^= 
e\|++ret\|--ret\|ret++\|ret--\|ret\)
 ...
 return ret;@p

@bad2 exists@
identifier r.reti;
position r.p;
identifier f;
type T;
@@

f(...,T reti,...) {
 ...
 return reti;@p
}

@change depends on !bad1  !bad2@
position r.p;
local idexpression r.ret;
identifier f;
@@

f(...) { +...
return 
-ret
+0
;@p
...+ }

@rewrite@
local idexpression r.ret;
expression e;
position p;
identifier change.f;
@@

f(...) { +...
 \(ret@p = e\|ret@p\)
...+ }

@depends on change@
local idexpression r.ret;
position p != rewrite.p;
identifier change.f;
@@

f(...) { +...
(
break;
|
ret = 0;
... when exists
ret@p
... when any
|
- ret = 0;
)
...+ }

@depends on change@
identifier r.reti;
type T;
constant C;
identifier change.f;
@@

f(...) { ... when any
-T reti = C;
 ... when != reti
 when strict
 }

@depends on change@
identifier r.reti;
type T;
identifier change.f;
@@

f(...) { ... when any
-T reti;
 ... when != reti
 when strict
 }
// /smpl

The first four rules detect cases where only 0 reaches a return.  The
remaining rules clean up any variable initializations or declarations that
are made unnecessary by this change.

--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/13 v2] [SCSI] qla2xxx: make return of 0 explicit

[Julia Lawall]

From: Julia Lawall julia.law...@lip6.fr

Delete unnecessary use of a local variable to immediately return 0.

A simplified version of the semantic patch that fixes this problem is as
follows: (http://coccinelle.lip6.fr/)

// smpl
@r exists@
local idexpression ret;
expression e;
position p;
@@

-ret = 0;
... when != ret = e
return 
- ret
+ 0
  ;
// /smpl

Signed-off-by: Julia Lawall julia.law...@lip6.fr

---
Changed subject line, which was not appreciated by some spam filters.

 drivers/scsi/qla2xxx/qla_init.c |3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/scsi/qla2xxx/qla_init.c b/drivers/scsi/qla2xxx/qla_init.c
index 38aeb54..a63f9b6 100644
--- a/drivers/scsi/qla2xxx/qla_init.c
+++ b/drivers/scsi/qla2xxx/qla_init.c
@@ -4593,8 +4593,7 @@ qla2x00_abort_isp(scsi_qla_host_t *vha)
if (unlikely(pci_channel_offline(ha-pdev) 
ha-flags.pci_channel_io_perm_failure)) {
clear_bit(ISP_ABORT_RETRY, vha-dpc_flags);
-   status = 0;
-   return status;
+   return 0;
}
 
ha-isp_ops-get_flash_version(vha, req-ring);

--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/13 v2] [SCSI] qla2xxx: make return of 0 explicit

[Julia Lawall]

On Mon, 19 May 2014, Dan Carpenter wrote:

 On Mon, May 19, 2014 at 04:07:52PM +, Saurav Kashyap wrote:
  Hi Julia,
 
  Status is already set to 0 at the beginning of the function, I think
  we should just return status here to be consistent with the rest of
  the function.

 return 0; is more clear than return status;.

 Consistency is great so long as it makes the code easier to read.  Don't
 lose track of the real goal.

If status were an informative word, there might be a reason for it.  But
integer typed functions almost always return their status, so there is no
real information.

julia
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/5] drivers/scsi/bnx2fc/bnx2fc_io.c: Remove potential NULL dereference

[Julia Lawall]

From: Julia Lawall julia.law...@lip6.fr

If the NULL test is necessary, the initialization involving a dereference of
the tested value should be moved after the NULL test.

The sematic patch that fixes this problem is as follows:
(http://coccinelle.lip6.fr/)

// smpl
@@
type T;
expression E;
identifier i,fld;
statement S;
@@

- T i = E-fld;
+ T i;
  ... when != E
  when != i
  if (E == NULL) S
+ i = E-fld;
// /smpl

Signed-off-by: Julia Lawall julia.law...@lip6.fr

---
 drivers/scsi/bnx2fc/bnx2fc_io.c |3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/scsi/bnx2fc/bnx2fc_io.c b/drivers/scsi/bnx2fc/bnx2fc_io.c
index 73f231c..1dd82db 100644
--- a/drivers/scsi/bnx2fc/bnx2fc_io.c
+++ b/drivers/scsi/bnx2fc/bnx2fc_io.c
@@ -686,7 +686,7 @@ static int bnx2fc_initiate_tmf(struct scsi_cmnd *sc_cmd, u8 
tm_flags)
 {
struct fc_lport *lport;
struct fc_rport *rport = starget_to_rport(scsi_target(sc_cmd-device));
-   struct fc_rport_libfc_priv *rp = rport-dd_data;
+   struct fc_rport_libfc_priv *rp;
struct fcoe_port *port;
struct bnx2fc_interface *interface;
struct bnx2fc_rport *tgt;
@@ -712,6 +712,7 @@ static int bnx2fc_initiate_tmf(struct scsi_cmnd *sc_cmd, u8 
tm_flags)
rc = FAILED;
goto tmf_err;
}
+   rp = rport-dd_data;
 
rc = fc_block_scsi_eh(sc_cmd);
if (rc)

--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 9/20] drivers/scsi: fix misspelling of current function in string

[Julia Lawall]

Replace a misspelled function name by %s and then __func__.

This was done using Coccinelle, including the use of Levenshtein distance,
as proposed by Rasmus Villemoes.

Signed-off-by: Julia Lawall julia.law...@lip6.fr

---
The semantic patch is difficult to summarize, but is available in the cover
letter of this patch series.

 drivers/scsi/atp870u.c   |   10 +-
 drivers/scsi/imm.c   |2 +-
 drivers/scsi/initio.c|2 +-
 drivers/scsi/ncr53c8xx.c |3 ++-
 drivers/scsi/ppa.c   |2 +-
 5 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/drivers/scsi/initio.c b/drivers/scsi/initio.c
index e5dae7b..85fd163 100644
--- a/drivers/scsi/initio.c
+++ b/drivers/scsi/initio.c
@@ -1034,7 +1034,7 @@ static int initio_bad_seq(struct initio_host * host)
 {
struct scsi_ctrl_blk *scb;
 
-   printk(initio_bad_seg c=%d\n, host-index);
+   printk(%s c=%d\n, __func__, host-index);
 
if ((scb = host-active) != NULL) {
initio_unlink_busy_scb(host, scb);
diff --git a/drivers/scsi/ncr53c8xx.c b/drivers/scsi/ncr53c8xx.c
index 5b93ed8..347fb49 100644
--- a/drivers/scsi/ncr53c8xx.c
+++ b/drivers/scsi/ncr53c8xx.c
@@ -772,7 +772,8 @@ static int __init sym53c8xx__setup(char *str)
break;
 #endif
default:
-   printk(sym53c8xx_setup: unexpected boot option '%.*s' 
ignored\n, (int)(pc-cur+1), cur);
+   printk(%s: unexpected boot option '%.*s' ignored\n,
+  __func__, (int)(pc-cur+1), cur);
break;
}
 
diff --git a/drivers/scsi/ppa.c b/drivers/scsi/ppa.c
index 1db8b26..4c1f08e 100644
--- a/drivers/scsi/ppa.c
+++ b/drivers/scsi/ppa.c
@@ -365,7 +365,7 @@ static int ppa_in(ppa_struct *dev, char *buffer, int len)
break;
 
default:
-   printk(KERN_ERR PPA: bug in ppa_ins()\n);
+   printk(KERN_ERR PPA: bug in %s()\n, __func__);
r = 0;
break;
}
diff --git a/drivers/scsi/atp870u.c b/drivers/scsi/atp870u.c
index a795d81..ebb4556 100644
--- a/drivers/scsi/atp870u.c
+++ b/drivers/scsi/atp870u.c
@@ -3016,12 +3016,12 @@ flash_ok_885:
goto scsi_add_fail;
scsi_scan_host(shpnt);
 #ifdef ED_DBGP 
-   printk(atp870u_prob : exit\n);
+   printk(%s : exit\n, __func__);
 #endif 
return 0;
 
 scsi_add_fail:
-   printk(atp870u_prob:scsi_add_fail\n);
+   printk(%s:scsi_add_fail\n, __func__);
if(ent-device==ATP885_DEVID) {
release_region(base_io, 0xff);
} else if((ent-device==ATP880_DEVID1)||(ent-device==ATP880_DEVID2)) {
@@ -3030,13 +3030,13 @@ scsi_add_fail:
release_region(base_io, 0x40);
}
 request_io_fail:
-   printk(atp870u_prob:request_io_fail\n);
+   printk(%s:request_io_fail\n, __func__);
free_irq(pdev-irq, shpnt);
 free_tables:
-   printk(atp870u_prob:free_table\n);
+   printk(%s:free_table\n, __func__);
atp870u_free_tables(shpnt);
 unregister:
-   printk(atp870u_prob:unregister\n);
+   printk(%s:unregister\n, __func__);
scsi_host_put(shpnt);
return -1;  
 err_eio:
diff --git a/drivers/scsi/imm.c b/drivers/scsi/imm.c
index 89a8266..1f2a668 100644
--- a/drivers/scsi/imm.c
+++ b/drivers/scsi/imm.c
@@ -439,7 +439,7 @@ static int imm_in(imm_struct *dev, char *buffer, int len)
break;
 
default:
-   printk(IMM: bug in imm_ins()\n);
+   printk(IMM: bug in %s()\n, __func__);
r = 0;
break;
}

--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 0/20] fix misspelling of current function in string

[Julia Lawall]

These patches replace what appears to be a reference to the name of the
current function but is misspelled in some way by either the name of the
function itself, or by %s and then __func__ in an argument list.

// smpl
// sudo apt-get install python-pip
// sudo pip install python-Levenshtein
// spatch requires the argument --in-place

virtual after_start

@initialize:ocaml@
@@

let extensible_functions = ref ([] : string list)
let restarted = ref false

let restart _ =
  restarted := true;
  let it = new iteration() in
  it#add_virtual_rule After_start;
  Printf.eprintf restarting\n;
  it#register()

@initialize:python@
@@
import re
from Levenshtein import distance
mindist = 1 // 1 to find only misspellings
maxdist = 2
ignore_leading = True

// -

@r0@
constant char [] c;
identifier f;
@@

f(...,c,...)

@script:ocaml@
c  r0.c;
f  r0.f;
@@

(if not !restarted then restart());
match Str.split_delim (Str.regexp %) c with
  _::_::_ -
if not (List.mem f !extensible_functions)
then extensible_functions := f :: !extensible_functions
| _ - ()

// -

@r depends on after_start@
constant char [] c;
position p;
identifier f;
@@

f(...,c@p,...)

@script:python flt@
c  r.c;
p  r.p;
matched;
@@

func = p[0].current_element
wpattern = [a-zA-Z_][a-zA-Z0-9_]*
if ignore_leading:
   func = func.strip(_)
   wpattern = [a-zA-Z][a-zA-Z0-9_]*
lf = len(func)
cocci.include_match(False)
// ignore extremely short function names
if lf  3:
   words = [w for w in re.findall(wpattern, c) if abs(len(w) - lf) = maxdist]
   for w in words:
   d = distance(w, func) 
   if mindist = d and d = maxdist:
  coccinelle.matched = w
  cocci.include_match(True)
  //print %s:%d:%s():%d: %s % (p[0].file, int(p[0].line), func, d, c)
  break

@script:ocaml r2@
c  r.c;
f  r.f;
matched  flt.matched;
fixed;
@@

let pieces = Str.split_delim (Str.regexp_string matched) c in
match pieces with
  [before;after] -
let preceeding =
  List.length(Str.split (Str.regexp_string %) before)  1 in
if preceeding
then Coccilib.include_match false
else
  if List.mem f !extensible_functions
  then fixed := before ^ %s ^ after
  else Coccilib.include_match false
| _ - Coccilib.include_match false

@changed1@
constant char [] r.c;
identifier r2.fixed;
position r.p;
identifier r.f;
@@

f(...,
-c@p
+fixed,__func__
 ,...)

// ---

@s depends on after_start@
constant char [] c;
position p;
identifier f;
@@

f(...,c@p,...)

@script:python flt2@
c  s.c;
p  s.p;
matched;
@@

func = p[0].current_element
wpattern = [a-zA-Z_][a-zA-Z0-9_]*
if ignore_leading:
   func = func.strip(_)
   wpattern = [a-zA-Z][a-zA-Z0-9_]*
lf = len(func)
cocci.include_match(False)
// ignore extremely short function names
if lf  3:
   words = [w for w in re.findall(wpattern, c) if abs(len(w) - lf) = maxdist]
   for w in words:
   d = distance(w, func) 
   if mindist = d and d = maxdist:
  coccinelle.matched = w
  cocci.include_match(True)
  //print %s:%d:%s():%d: %s % (p[0].file, int(p[0].line), func, d, c)
  break

@script:ocaml s2@
c  s.c;
f  s.f;
p  s.p;
matched  flt2.matched;
fixed;
@@

let ce = (List.hd p).current_element in
let pieces = Str.split_delim (Str.regexp_string matched) c in
match pieces with
  [before;after] -
let preceeding =
  List.length(Str.split (Str.regexp_string %) before)  1 in
if preceeding
then Coccilib.include_match false
else
  if List.mem f !extensible_functions
  then Coccilib.include_match false
  else fixed := before ^ ce ^ after
| _ - Coccilib.include_match false

@changed2@
constant char [] s.c;
identifier s2.fixed;
position s.p;
identifier s.f;
@@

f(...,
-c@p
+fixed
 ,...)
// /smpl

--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/20] fix misspelling of current function in string

[Julia Lawall]

On Mon, 8 Dec 2014, Julian Calaby wrote:

 Hi Julia,
 
 On Mon, Dec 8, 2014 at 6:20 AM, Julia Lawall julia.law...@lip6.fr wrote:
  These patches replace what appears to be a reference to the name of the
  current function but is misspelled in some way by either the name of the
  function itself, or by %s and then __func__ in an argument list.
 
 Would there be any value in doing this for _all_ cases where the
 function name is written in a format string?

Probably.  But there are a lot of them.  Even for the misspellings, I have 
only don about 1/3 of the cases.

On the other hand, the misspelling have to be checked carefully, because a 
misspelling of one thing could be the correct spelling of the thing thst 
was actually intended.

Joe, however, points out that a lot of these prints are just for function 
tracing, and could be removed.  I worked on another semantic patch that 
tries to do that.  It might be better to remove those prints completely, 
rather than sending one patch to transform them and then one patch to 
remove them after that.  That is why for this series I did only the ones 
where there was actually a problem.

julia
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/4] SCSI-QLA4...: Less function calls in qla4xxx_is_session_exists() after error detection

[Julia Lawall]

On Sat, 7 Feb 2015, Dan Carpenter wrote:

 On Fri, Feb 06, 2015 at 11:15:13PM +0100, SF Markus Elfring wrote:
  diff --git a/drivers/scsi/qla4xxx/ql4_os.c b/drivers/scsi/qla4xxx/ql4_os.c
  index 2a00fd3..a7ca479 100644
  --- a/drivers/scsi/qla4xxx/ql4_os.c
  +++ b/drivers/scsi/qla4xxx/ql4_os.c
  @@ -6327,17 +6327,15 @@ static int qla4xxx_is_session_exists(struct 
  scsi_qla_host *ha,
   uint32_t *index)
   {
  struct ddb_entry *ddb_entry;
  -   struct ql4_tuple_ddb *fw_tddb = NULL;
  -   struct ql4_tuple_ddb *tmp_tddb = NULL;
  int idx;
  int ret = QLA_ERROR;
  +   struct ql4_tuple_ddb *tmp_tddb;
  +   struct ql4_tuple_ddb *fw_tddb = vzalloc(sizeof(*fw_tddb));
   
 
 Don't do allocations in the initializers.  Same for patches 3 and 4 as
 well.

Why not?  I can think of some reasons, but I am wondering what is the 
precise one.

thanks,
julia
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 0/15] don't export static symbol

[Julia Lawall]

These patches remove EXPORT_SYMBOL or EXPORT_SYMBOL_GPL declarations on
static functions.

This was done using the following semantic patch:
(http://coccinelle.lip6.fr/)

// smpl
@r@
type T;
identifier f;
@@

static T f (...) { ... }

@@
identifier r.f;
declarer name EXPORT_SYMBOL;
@@

-EXPORT_SYMBOL(f);

@@
identifier r.f;
declarer name EXPORT_SYMBOL_GPL;
@@

-EXPORT_SYMBOL_GPL(f);
// /smpl

--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 10/15] scsi: don't export static symbol

[Julia Lawall]

From: Julia Lawall julia.law...@lip6.fr

The semantic patch that fixes this problem is as follows:
(http://coccinelle.lip6.fr/)

// smpl
@r@
type T;
identifier f;
@@

static T f (...) { ... }

@@
identifier r.f;
declarer name EXPORT_SYMBOL;
@@

-EXPORT_SYMBOL(f);
// /smpl

Signed-off-by: Julia Lawall julia.law...@lip6.fr

---
 drivers/scsi/libsas/sas_scsi_host.c |1 -
 1 file changed, 1 deletion(-)

diff -u -p a/drivers/scsi/libsas/sas_scsi_host.c 
b/drivers/scsi/libsas/sas_scsi_host.c
--- a/drivers/scsi/libsas/sas_scsi_host.c
+++ b/drivers/scsi/libsas/sas_scsi_host.c
@@ -440,7 +440,6 @@ static void sas_wait_eh(struct domain_de
goto retry;
}
 }
-EXPORT_SYMBOL(sas_wait_eh);
 
 static int sas_queue_reset(struct domain_device *dev, int reset_type,
   u64 lun, int wait)

--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 15/15] iscsi-target: don't export static symbol

[Julia Lawall]

From: Julia Lawall julia.law...@lip6.fr

The semantic patch that fixes this problem is as follows:
(http://coccinelle.lip6.fr/)

// smpl
@r@
type T;
identifier f;
@@

static T f (...) { ... }

@@
identifier r.f;
declarer name EXPORT_SYMBOL;
@@

-EXPORT_SYMBOL(f);
// /smpl

Signed-off-by: Julia Lawall julia.law...@lip6.fr

---
 drivers/target/iscsi/iscsi_target.c |1 -
 1 file changed, 1 deletion(-)

diff -u -p a/drivers/target/iscsi/iscsi_target.c 
b/drivers/target/iscsi/iscsi_target.c
--- a/drivers/target/iscsi/iscsi_target.c
+++ b/drivers/target/iscsi/iscsi_target.c
@@ -2155,7 +2155,6 @@ reject:
cmd-text_in_ptr = NULL;
return iscsit_reject_cmd(cmd, ISCSI_REASON_PROTOCOL_ERROR, buf);
 }
-EXPORT_SYMBOL(iscsit_handle_text_cmd);
 
 int iscsit_logout_closesession(struct iscsi_cmd *cmd, struct iscsi_conn *conn)
 {

--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 5/16] ufs: fix error return code

[Julia Lawall]

Return a negative error code on failure.

A simplified version of the semantic match that finds this problem is as
follows: (http://coccinelle.lip6.fr/)

// smpl
@@
identifier ret; expression e1,e2;
@@
(
if (\(ret  0\|ret != 0\))
 { ... return ret; }
|
ret = 0
)
... when != ret = e1
when != ret
*if(...)
{
  ... when != ret = e2
  when forall
 return ret;
}
// /smpl

This patch also changes the subsequent dev_err call to use err rather
than recomputing PTR_ERR(hba-devfreq).  The format of the error value
is changed from %ld to %d to reflect the type or err.

Signed-off-by: Julia Lawall julia.law...@lip6.fr

---
 drivers/scsi/ufs/ufshcd.c |5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 2aa85e3..8b802d1 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -5506,8 +5506,9 @@ int ufshcd_init(struct ufs_hba *hba, void __iomem 
*mmio_base, unsigned int irq)
hba-devfreq = devfreq_add_device(dev, ufs_devfreq_profile,
   simple_ondemand, NULL);
if (IS_ERR(hba-devfreq)) {
-   dev_err(hba-dev, Unable to register with devfreq 
%ld\n,
-   PTR_ERR(hba-devfreq));
+   err = PTR_ERR(hba-devfreq);
+   dev_err(hba-dev, Unable to register with devfreq 
%d\n,
+   err);
goto out_remove_scsi_host;
}
/* Suspend devfreq until the UFS device is detected */

--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 0/16] fix error return code

[Julia Lawall]

The complate semantic patch that finds this problem is as follows:
(http://coccinelle.lip6.fr/)

// smpl
@ok exists@
identifier f,ret,i;
expression e;
constant c;
@@

// identify a function that returns a negative return value at least once.
f(...) {
... when any
(
return -c@i;
|
ret = -c@i;
... when != ret = e
return ret;
|
if (ret  0) { ... return ret; }
)
... when any
}

@r exists@
identifier ret,ok.f,fn;
expression e1,e2,e3,e4,e5,e6,x;
statement S,S1;
position p1,p2,p3;
@@

// identify a case where the return variable is set to a non-negative value
// and then returned in error-handling code
f(...) {
... when any
(
if@p1 (\(ret  0\|ret != 0\))
 { ... return ret; }
|
ret@p1 = 0
)
... when != \(ret = e1\|ret++\|ret--\|ret+=e1\|ret-=e1\)
when != ret
when any
(
 if (+... ret = e5 ...+) S1
|
 if (+... ret ...+) S1
|
if@p2(+...x = fn(...)...+)
 {
  ... when != ret = e6
  when forall
 return@p3 ret;
}
|
break;
|
x = fn(...)
... when != \(ret = e4\|ret++\|ret--\|ret+=e4\|ret-=e4\)
when != ret
(
 if (+... ret = e3 ...+) S
|
 if (+... ret ...+) S
|
if@p2(+...\(x != 0\|x  0\|x == NULL\|IS_ERR(x)\)...+)
 {
  ... when != ret = e2
  when forall
 return@p3 ret;
}
)
)
... when any
}

@printer depends on r@
position p;
identifier ok.f,pr;
constant char [] c;
@@

f(...) { ...pr@p(...,c,...)... }

@bad0 exists@
identifier r.ret,ok.f,g != {ERR_PTR,IS_ERR};
position p != printer.p;
@@

f(...) { ... when any
g@p(...,ret,...)
... when any
 }

@bad depends on !bad0 exists@
position r.p1,r.p2;
statement S1,S2;
identifier r.ret;
expression e1;
@@

// ignore the above if there is some path where the variable is set to
// something else
(
if@p1 (\(ret  0\|ret != 0\)) S1
|
ret@p1 = 0
)
... when any
 \(ret = e1\|ret++\|ret--\|ret+=e1\|ret-=e1\|ret\)
... when any
if@p2(...) S2

@bad1 depends on !bad0  !bad exists@
position r.p2;
statement S2;
identifier r.ret;
expression e1;
constant c;
@@

ret = -c
... when != \(ret = e1\|ret++\|ret--\|ret+=e1\|ret-=e1\)
when != ret
when any
if@p2(...) S2

@bad2 depends on !bad0  !bad  !bad1 exists@
position r.p1,r.p2;
identifier r.ret;
expression e1;
statement S2;
constant c;
@@

// likewise ignore it if there has been an intervening return
ret@p1 = 0
... when != if (...) { ... ret = e1 ... return ret; }
when != if (...) { ... return -c; }
when any
if@p2(...) S2

@script:python depends on !bad0  !bad  !bad1  !bad2@
p1  r.p1;
p2  r.p2;
p3  r.p3;
@@

cocci.print_main(,p1)
cocci.print_secs(,p2)
cocci.print_secs(,p3)
// /smpl

--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 0/9] constify pci_error_handlers structures

[Julia Lawall]

Constify never-modified pci_error_handlers structures.

---

 drivers/crypto/qat/qat_common/adf_aer.c |2 +-
 drivers/misc/genwqe/card_base.c |2 +-
 drivers/net/ethernet/cavium/liquidio/lio_main.c |2 +-
 drivers/net/ethernet/sfc/efx.c  |2 +-
 drivers/scsi/be2iscsi/be_main.c |2 +-
 drivers/scsi/bfa/bfad.c |2 +-
 drivers/scsi/csiostor/csio_init.c   |2 +-
 drivers/scsi/mpt3sas/mpt3sas_scsih.c|2 +-
 drivers/vfio/pci/vfio_pci.c |2 +-
 9 files changed, 9 insertions(+), 9 deletions(-)
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 3/9] cxgb4/cxgb4vf/csiostor: constify pci_error_handlers structures

[Julia Lawall]

This pci_error_handlers structure is never modified, like all the other
pci_error_handlers structures, so declare it as const.

Done with the help of Coccinelle.

Signed-off-by: Julia Lawall <julia.law...@lip6.fr>

---
There are no dependencies between these patches.

 drivers/scsi/csiostor/csio_init.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/csiostor/csio_init.c 
b/drivers/scsi/csiostor/csio_init.c
index dbe416f..33996f2 100644
--- a/drivers/scsi/csiostor/csio_init.c
+++ b/drivers/scsi/csiostor/csio_init.c
@@ -1162,7 +1162,7 @@ err_resume_exit:
dev_err(>dev, "resume of device failed: %d\n", rv);
 }
 
-static struct pci_error_handlers csio_err_handler = {
+static const struct pci_error_handlers csio_err_handler = {
.error_detected = csio_pci_error_detected,
.slot_reset = csio_pci_slot_reset,
.resume = csio_pci_resume,

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 6/9] be2iscsi: constify pci_error_handlers structures

[Julia Lawall]

This pci_error_handlers structure is never modified, like all the other
pci_error_handlers structures, so declare it as const.

Done with the help of Coccinelle.

Signed-off-by: Julia Lawall <julia.law...@lip6.fr>

---
There are no dependencies between these patches.

 drivers/scsi/be2iscsi/be_main.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/be2iscsi/be_main.c b/drivers/scsi/be2iscsi/be_main.c
index 2e6abe7..0b8f873 100644
--- a/drivers/scsi/be2iscsi/be_main.c
+++ b/drivers/scsi/be2iscsi/be_main.c
@@ -5786,7 +5786,7 @@ disable_pci:
return ret;
 }
 
-static struct pci_error_handlers beiscsi_eeh_handlers = {
+static const struct pci_error_handlers beiscsi_eeh_handlers = {
.error_detected = beiscsi_eeh_err_detected,
.slot_reset = beiscsi_eeh_reset,
.resume = beiscsi_eeh_resume,

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 8/9] mpt3sas: constify pci_error_handlers structures

[Julia Lawall]

This pci_error_handlers structure is never modified, like all the other
pci_error_handlers structures, so declare it as const.

Done with the help of Coccinelle.

Signed-off-by: Julia Lawall <julia.law...@lip6.fr>

---
There are no dependencies between these patches.

 drivers/scsi/mpt3sas/mpt3sas_scsih.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/mpt3sas/mpt3sas_scsih.c 
b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
index 9e68432..39678e7 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_scsih.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
@@ -8557,7 +8557,7 @@ static struct raid_function_template 
mpt3sas_raid_functions = {
.get_state  = _scsih_get_state,
 };
 
-static struct pci_error_handlers _scsih_err_handler = {
+static const struct pci_error_handlers _scsih_err_handler = {
.error_detected = _scsih_pci_error_detected,
.mmio_enabled = _scsih_pci_mmio_enabled,
.slot_reset =   _scsih_pci_slot_reset,

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 4/9] bfa: constify pci_error_handlers structures

[Julia Lawall]

This pci_error_handlers structure is never modified, like all the other
pci_error_handlers structures, so declare it as const.

Done with the help of Coccinelle.

Signed-off-by: Julia Lawall <julia.law...@lip6.fr>

---
There are no dependencies between these patches.

 drivers/scsi/bfa/bfad.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/bfa/bfad.c b/drivers/scsi/bfa/bfad.c
index cc3b9d3..8df346a 100644
--- a/drivers/scsi/bfa/bfad.c
+++ b/drivers/scsi/bfa/bfad.c
@@ -1687,7 +1687,7 @@ MODULE_DEVICE_TABLE(pci, bfad_id_table);
 /*
  * PCI error recovery handlers.
  */
-static struct pci_error_handlers bfad_err_handler = {
+static const struct pci_error_handlers bfad_err_handler = {
.error_detected = bfad_pci_error_detected,
.slot_reset = bfad_pci_slot_reset,
.mmio_enabled = bfad_pci_mmio_enabled,

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] drivers/message/fusion: constify mpt_pci_driver structures

[Julia Lawall]

The mpt_pci_driver structures are never modified, so declare them as const.

Done with the help of Coccinelle.

Signed-off-by: Julia Lawall <julia.law...@lip6.fr>

---
 drivers/message/fusion/mptbase.c |8 +---
 drivers/message/fusion/mptbase.h |3 ++-
 drivers/message/fusion/mptctl.c  |2 +-
 drivers/message/fusion/mptlan.c  |2 +-
 4 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/drivers/message/fusion/mptbase.c b/drivers/message/fusion/mptbase.c
index 5dcc031..451e73c 100644
--- a/drivers/message/fusion/mptbase.c
+++ b/drivers/message/fusion/mptbase.c
@@ -140,7 +140,9 @@ static int   
MptDriverClass[MPT_MAX_PROTOCOL_DRIVERS];
 static MPT_EVHANDLERMptEvHandlers[MPT_MAX_PROTOCOL_DRIVERS];
/* Reset handler lookup table */
 static MPT_RESETHANDLER 
MptResetHandlers[MPT_MAX_PROTOCOL_DRIVERS];
-static struct mpt_pci_driver   
*MptDeviceDriverHandlers[MPT_MAX_PROTOCOL_DRIVERS];
+
+static const
+struct mpt_pci_driver *MptDeviceDriverHandlers[MPT_MAX_PROTOCOL_DRIVERS];
 
 #ifdef CONFIG_PROC_FS
 static struct proc_dir_entry   *mpt_proc_root_dir;
@@ -826,7 +828,7 @@ mpt_reset_deregister(u8 cb_idx)
  * @cb_idx: MPT protocol driver index
  */
 int
-mpt_device_driver_register(struct mpt_pci_driver * dd_cbfunc, u8 cb_idx)
+mpt_device_driver_register(const struct mpt_pci_driver *dd_cbfunc, u8 cb_idx)
 {
MPT_ADAPTER *ioc;
const struct pci_device_id *id;
@@ -855,7 +857,7 @@ mpt_device_driver_register(struct mpt_pci_driver * 
dd_cbfunc, u8 cb_idx)
 void
 mpt_device_driver_deregister(u8 cb_idx)
 {
-   struct mpt_pci_driver *dd_cbfunc;
+   const struct mpt_pci_driver *dd_cbfunc;
MPT_ADAPTER *ioc;
 
if (!cb_idx || cb_idx >= MPT_MAX_PROTOCOL_DRIVERS)
diff --git a/drivers/message/fusion/mptbase.h b/drivers/message/fusion/mptbase.h
index 813d463..e29e4be 100644
--- a/drivers/message/fusion/mptbase.h
+++ b/drivers/message/fusion/mptbase.h
@@ -920,7 +920,8 @@ extern int   mpt_event_register(u8 cb_idx, MPT_EVHANDLER 
ev_cbfunc);
 extern void mpt_event_deregister(u8 cb_idx);
 extern int  mpt_reset_register(u8 cb_idx, MPT_RESETHANDLER reset_func);
 extern void mpt_reset_deregister(u8 cb_idx);
-extern int  mpt_device_driver_register(struct mpt_pci_driver * dd_cbfunc, 
u8 cb_idx);
+extern int  mpt_device_driver_register(const struct mpt_pci_driver 
*dd_cbfunc,
+ u8 cb_idx);
 extern void mpt_device_driver_deregister(u8 cb_idx);
 extern MPT_FRAME_HDR   *mpt_get_msg_frame(u8 cb_idx, MPT_ADAPTER *ioc);
 extern void mpt_free_msg_frame(MPT_ADAPTER *ioc, MPT_FRAME_HDR *mf);
diff --git a/drivers/message/fusion/mptctl.c b/drivers/message/fusion/mptctl.c
index 02b5f69..7d051af 100644
--- a/drivers/message/fusion/mptctl.c
+++ b/drivers/message/fusion/mptctl.c
@@ -2990,7 +2990,7 @@ mptctl_remove(struct pci_dev *pdev)
 {
 }
 
-static struct mpt_pci_driver mptctl_driver = {
+static const struct mpt_pci_driver mptctl_driver = {
   .probe   = mptctl_probe,
   .remove  = mptctl_remove,
 };
diff --git a/drivers/message/fusion/mptlan.c b/drivers/message/fusion/mptlan.c
index cbe9607..4b20af4 100644
--- a/drivers/message/fusion/mptlan.c
+++ b/drivers/message/fusion/mptlan.c
@@ -1443,7 +1443,7 @@ mptlan_remove(struct pci_dev *pdev)
}
 }
 
-static struct mpt_pci_driver mptlan_driver = {
+static const struct mpt_pci_driver mptlan_driver = {
.probe  = mptlan_probe,
.remove = mptlan_remove,
 };

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [patch RESEND] atp870u: 64 bit bug in atp885_init()

[Julia Lawall]

On Wed, 9 Dec 2015, Dan Carpenter wrote:

> We should add a tag to indicate that we are sending a patch for a crappy
> driver.
>
> IMHO-this-driver-is-garbage: Your Name 
>
> If it got 10 votes of no confidence it would be moved to staging and
> then deleted.

Forgive my ignorance, but what is the exact procedure?  For example, the
following file: drivers/pcmcia/vrc4173_cardu.c contains the following
code: INIT_WORK(>tq_work, cardu_bh, socket);.  The last time
INIT_WORK took three arguments was Linux 2.6.19, so I think no one has
been compiling this code recently.  There would be the .c file and the
associated .h file to move to staging, but it's less clear to me eg what
to do with the Kconfig entry and the Makefile entry.  And is there
anything else to take into account?

thanks,
julia
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 13/20] qla2xxx: Remove dependency on hardware_lock to reduce lock contention.

[Julia Lawall]

Looks suspicious.  Please check.

julia

On Tue, 8 Dec 2015, kbuild test robot wrote:

> CC: kbuild-...@01.org
> In-Reply-To: <1449535747-2850-14-git-send-email-himanshu.madh...@qlogic.com>
> TO: Himanshu Madhani 
> CC: target-de...@vger.kernel.org, n...@linux-iscsi.org
> CC: giridhar.malav...@qlogic.com, linux-scsi@vger.kernel.org, 
> himanshu.madh...@qlogic.com
>
> Hi Quinn,
>
> [auto build test WARNING on target/master]
> [also build test WARNING on v4.4-rc4 next-20151207]
> [cannot apply to scsi/for-next]
>
> url:
> https://github.com/0day-ci/linux/commits/Himanshu-Madhani/qla2xxx-Patches-for-target-pending-branch/20151208-093328
> base:   
> https://git.kernel.org/pub/scm/linux/kernel/git/nab/target-pending.git master
> :: branch date: 47 minutes ago
> :: commit date: 47 minutes ago
>
> >> drivers/scsi/qla2xxx/qla_target.c:1091:2-8: preceding lock on line 1074
>
> git remote add linux-review https://github.com/0day-ci/linux
> git remote update linux-review
> git checkout 13bfc932c19778c08e1fc8908c4f0825fd70583f
> vim +1091 drivers/scsi/qla2xxx/qla_target.c
>
> 2d70c103 Nicholas Bellinger 2012-05-15  1068  if 
> (!vha->hw->tgt.tgt_ops)
> 2d70c103 Nicholas Bellinger 2012-05-15  1069  return;
> 2d70c103 Nicholas Bellinger 2012-05-15  1070
> b2032fd5 Roland Dreier  2015-07-14  1071  if (!tgt)
> 2d70c103 Nicholas Bellinger 2012-05-15  1072  return;
> 2d70c103 Nicholas Bellinger 2012-05-15  1073
> 13bfc932 Quinn Tran 2015-12-07 @1074  
> spin_lock_irqsave(>hw->tgt.sess_lock, flags);
> 2d70c103 Nicholas Bellinger 2012-05-15  1075  if (tgt->tgt_stop) {
> 13bfc932 Quinn Tran 2015-12-07  1076  
> spin_unlock_irqrestore(>hw->tgt.sess_lock, flags);
> 2d70c103 Nicholas Bellinger 2012-05-15  1077  return;
> 2d70c103 Nicholas Bellinger 2012-05-15  1078  }
> 2d70c103 Nicholas Bellinger 2012-05-15  1079  sess = 
> qlt_find_sess_by_port_name(tgt, fcport->port_name);
> 2d70c103 Nicholas Bellinger 2012-05-15  1080  if (!sess) {
> 13bfc932 Quinn Tran 2015-12-07  1081  
> spin_unlock_irqrestore(>hw->tgt.sess_lock, flags);
> 2d70c103 Nicholas Bellinger 2012-05-15  1082  return;
> 2d70c103 Nicholas Bellinger 2012-05-15  1083  }
> 2d70c103 Nicholas Bellinger 2012-05-15  1084
> df673274 Alexei Potashnik   2015-07-14  1085  if (max_gen - 
> sess->generation < 0) {
> df673274 Alexei Potashnik   2015-07-14  1086  
> ql_dbg(ql_dbg_tgt_mgt, vha, 0xf092,
> df673274 Alexei Potashnik   2015-07-14  1087  "Ignoring 
> stale deletion request for se_sess %p / sess %p"
> df673274 Alexei Potashnik   2015-07-14  1088  " for port 
> %8phC, req_gen %d, sess_gen %d\n",
> df673274 Alexei Potashnik   2015-07-14  1089  
> sess->se_sess, sess, sess->port_name, max_gen,
> df673274 Alexei Potashnik   2015-07-14  1090  
> sess->generation);
> df673274 Alexei Potashnik   2015-07-14 @1091  return;
> df673274 Alexei Potashnik   2015-07-14  1092  }
> df673274 Alexei Potashnik   2015-07-14  1093
> 2d70c103 Nicholas Bellinger 2012-05-15  1094  ql_dbg(ql_dbg_tgt_mgt, 
> vha, 0xf008, "qla_tgt_fc_port_deleted %p", sess);
>
> :: The code at line 1091 was first introduced by commit
> :: df673274fa4896f25f0bf348d2a3535d74b4cbec qla2xxx: added sess 
> generations to detect RSCN update races
>
> :: TO: Alexei Potashnik 
> :: CC: Nicholas Bellinger 
>
> ---
> 0-DAY kernel test infrastructureOpen Source Technology Center
> https://lists.01.org/pipermail/kbuild-all   Intel Corporation
>
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 4/7] iscsi-target: Make a variable initialisation a bit more obvious in iscsi_create_default_params()

[Julia Lawall]

On Sat, 12 Dec 2015, SF Markus Elfring wrote:

> From: Markus Elfring 
> Date: Sat, 12 Dec 2015 13:44:06 +0100
>
> The variable "pl" was declared and immediately assigned a return value
> from a function call in a separate statement.
>
> * Let us express the desired variable initialisation directly.
>
> * Avoid the repetition of the data type specification for the
>   involved memory allocation according to the Linux coding
>   style convention.
>
> Signed-off-by: Markus Elfring 
> ---
>  drivers/target/iscsi/iscsi_target_parameters.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/target/iscsi/iscsi_target_parameters.c 
> b/drivers/target/iscsi/iscsi_target_parameters.c
> index e0b173d..3f3842f 100644
> --- a/drivers/target/iscsi/iscsi_target_parameters.c
> +++ b/drivers/target/iscsi/iscsi_target_parameters.c
> @@ -200,9 +200,8 @@ free_param:
>  int iscsi_create_default_params(struct iscsi_param_list **param_list_ptr)
>  {
>   struct iscsi_param *param;
> - struct iscsi_param_list *pl;
> + struct iscsi_param_list *pl = kzalloc(sizeof(*pl), GFP_KERNEL);
>
> - pl = kzalloc(sizeof(struct iscsi_param_list), GFP_KERNEL);

I don't see the benefit of this change, and the pattern assignment ->
failure test becomes more obscure.

julia

>   if (!pl) {
>   pr_err("Unable to allocate memory for"
>   " struct iscsi_param_list.\n");
> --
> 2.6.3
>
> --
> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/7] fix typo

[Julia Lawall]

On Tue, 17 May 2016, Kalle Valo wrote:

> Julia Lawall <julia.law...@lip6.fr> writes:
>
> > firmare -> firmware
> >
> > ---
> >
> >  drivers/media/dvb-frontends/mn88473.c   |2 +-
> >  drivers/net/wireless/ath/ath6kl/core.h  |2 +-
> >  drivers/net/wireless/marvell/mwifiex/pcie.c |2 +-
>
> It would be good to know in advance what tree you are planning to submit
> these for. For example, should I take ath6kl and mwifiex patches or
> someone else?

I have no preference.  They are all independent in any case.

julia
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 0/7] fix typo

[Julia Lawall]

firmare -> firmware

---

 drivers/media/dvb-frontends/mn88473.c   |2 +-
 drivers/net/wireless/ath/ath6kl/core.h  |2 +-
 drivers/net/wireless/marvell/mwifiex/pcie.c |2 +-
 drivers/scsi/pm8001/pm8001_init.c   |2 +-
 drivers/scsi/snic/snic_fwint.h  |2 +-
 drivers/staging/media/mn88472/mn88472.c |2 +-
 drivers/staging/wilc1000/linux_wlan.c   |2 +-
 7 files changed, 7 insertions(+), 7 deletions(-)
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 5/7] pm8001: fix typo

[Julia Lawall]

firmare -> firmware

Signed-off-by: Julia Lawall <julia.law...@lip6.fr>

---
 drivers/scsi/pm8001/pm8001_init.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/pm8001/pm8001_init.c 
b/drivers/scsi/pm8001/pm8001_init.c
index 6bd7bf4..fdbee8b4 100644
--- a/drivers/scsi/pm8001/pm8001_init.c
+++ b/drivers/scsi/pm8001/pm8001_init.c
@@ -1249,7 +1249,7 @@ static int pm8001_pci_resume(struct pci_dev *pdev)
 
/* Chip documentation for the 8070 and 8072 SPCv*/
/* states that a 500ms minimum delay is required*/
-   /* before issuing commands.  Otherwise, the firmare */
+   /* before issuing commands. Otherwise, the firmware */
/* will enter an unrecoverable state.   */
 
if (pm8001_ha->chip_id == chip_8070 ||

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/7] snic: fix typo

[Julia Lawall]

firmare -> firmware

Signed-off-by: Julia Lawall <julia.law...@lip6.fr>

---
 drivers/scsi/snic/snic_fwint.h |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/snic/snic_fwint.h b/drivers/scsi/snic/snic_fwint.h
index c5f9e19..2a045a5 100644
--- a/drivers/scsi/snic/snic_fwint.h
+++ b/drivers/scsi/snic/snic_fwint.h
@@ -92,7 +92,7 @@ enum snic_io_status {
 }; /* end of enum snic_io_status */
 
 /*
- * snic_io_hdr : host <--> firmare
+ * snic_io_hdr : host <--> firmware
  *
  * for any other message that will be queued to firmware should
  *  have the following request header

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/2] be2iscsi: Fix some error messages

[Julia Lawall]

On Fri, 12 Aug 2016, Christophe JAILLET wrote:

> This fixes:
>- missing spaces in string split on several lines
>- extra spaces after ':'
>- missing '\n' at the end of some messages
>- too long lines

I think that the strings should be concatenated, even if they go past 80
chars.  I'm surprised checkpatch didn't complain.

julia

>
> Signed-off-by: Christophe JAILLET 
> ---
>  drivers/scsi/be2iscsi/be_main.c | 83 
> +
>  1 file changed, 43 insertions(+), 40 deletions(-)
>
> diff --git a/drivers/scsi/be2iscsi/be_main.c b/drivers/scsi/be2iscsi/be_main.c
> index 89ae6390b697..415c21ec6a13 100644
> --- a/drivers/scsi/be2iscsi/be_main.c
> +++ b/drivers/scsi/be2iscsi/be_main.c
> @@ -268,7 +268,7 @@ static int beiscsi_eh_abort(struct scsi_cmnd *sc)
>   _cmd.dma);
>   if (nonemb_cmd.va == NULL) {
>   beiscsi_log(phba, KERN_ERR, BEISCSI_LOG_EH,
> - "BM_%d : Failed to allocate memory for"
> + "BM_%d : Failed to allocate memory for "
>   "mgmt_invalidate_icds\n");
>   return FAILED;
>   }
> @@ -278,7 +278,7 @@ static int beiscsi_eh_abort(struct scsi_cmnd *sc)
>  cid, _cmd);
>   if (!tag) {
>   beiscsi_log(phba, KERN_WARNING, BEISCSI_LOG_EH,
> - "BM_%d : mgmt_invalidate_icds could not be"
> + "BM_%d : mgmt_invalidate_icds could not be "
>   "submitted\n");
>   pci_free_consistent(phba->ctrl.pdev, nonemb_cmd.size,
>   nonemb_cmd.va, nonemb_cmd.dma);
> @@ -350,7 +350,7 @@ static int beiscsi_eh_device_reset(struct scsi_cmnd *sc)
>   _cmd.dma);
>   if (nonemb_cmd.va == NULL) {
>   beiscsi_log(phba, KERN_ERR, BEISCSI_LOG_EH,
> - "BM_%d : Failed to allocate memory for"
> + "BM_%d : Failed to allocate memory for "
>   "mgmt_invalidate_icds\n");
>   return FAILED;
>   }
> @@ -1010,7 +1010,7 @@ static int beiscsi_init_irqs(struct beiscsi_hba *phba)
> _context->be_eq[i]);
>   if (ret) {
>   beiscsi_log(phba, KERN_ERR, BEISCSI_LOG_INIT,
> - "BM_%d : beiscsi_init_irqs-Failed 
> to"
> + "BM_%d : beiscsi_init_irqs-Failed 
> to "
>   "register msix for i = %d\n",
>   i);
>   kfree(phba->msi_name[i]);
> @@ -1168,7 +1168,7 @@ free_io_sgl_handle(struct beiscsi_hba *phba, struct 
> sgl_handle *psgl_handle)
>* failed in xmit_task or alloc_pdu.
>*/
>beiscsi_log(phba, KERN_INFO, BEISCSI_LOG_IO,
> -  "BM_%d : Double Free in IO SGL 
> io_sgl_free_index=%d,"
> +  "BM_%d : Double Free in IO SGL 
> io_sgl_free_index=%d, "
>"value there=%p\n", phba->io_sgl_free_index,
>phba->io_sgl_hndl_base
>[phba->io_sgl_free_index]);
> @@ -1256,7 +1256,7 @@ free_wrb_handle(struct beiscsi_hba *phba, struct 
> hwi_wrb_context *pwrb_context,
>  phba->params.wrbs_per_cxn);
>   beiscsi_log(phba, KERN_INFO,
>   BEISCSI_LOG_IO | BEISCSI_LOG_CONFIG,
> - "BM_%d : FREE WRB: pwrb_handle=%p free_index=0x%x"
> + "BM_%d : FREE WRB: pwrb_handle=%p free_index=0x%x "
>   "wrb_handles_available=%d\n",
>   pwrb_handle, pwrb_context->free_index,
>   pwrb_context->wrb_handles_available);
> @@ -1293,7 +1293,7 @@ free_mgmt_sgl_handle(struct beiscsi_hba *phba, struct 
> sgl_handle *psgl_handle)
>  {
>   spin_lock_bh(>mgmt_sgl_lock);
>   beiscsi_log(phba, KERN_INFO, BEISCSI_LOG_CONFIG,
> - "BM_%d : In  free_mgmt_sgl_handle,"
> + "BM_%d : In  free_mgmt_sgl_handle, "
>   "eh_sgl_free_index=%d\n",
>   phba->eh_sgl_free_index);
>
> @@ -1303,7 +1303,7 @@ free_mgmt_sgl_handle(struct beiscsi_hba *phba, struct 
> sgl_handle *psgl_handle)
>* failed in xmit_task or alloc_pdu.
>*/
>   beiscsi_log(phba, KERN_WARNING, BEISCSI_LOG_CONFIG,
> - "BM_%d : Double Free in eh SGL ,"
> + "BM_%d : Double Free in eh SGL, "
>   "eh_sgl_free_index=%d\n",
>   phba->eh_sgl_free_index);
>   spin_unlock_bh(>mgmt_sgl_lock);
> @@ -1604,7 +1604,7 @@ static void hwi_complete_cmd(struct beiscsi_conn 
> *beiscsi_conn,
> 

Re: [PATCH 2/2 v2] be2iscsi: Fix some error messages

[Julia Lawall]

On Sat, 13 Aug 2016, Christophe JAILLET wrote:

> This fixes:
>- missing spaces in string split on several lines
>- extra spaces after ':'
>- missing '\n' at the end of some messages
>- turn a \\n to \n in 1 message (v2)
>- concatenate strings on the same line to fix checkpatch warnings (v2)

v2 information should be under the ---.  It's meaningless when the change
is actually committed.

julia

>
> Signed-off-by: Christophe JAILLET 
> ---
>  drivers/scsi/be2iscsi/be_main.c | 79 
> -
>  1 file changed, 39 insertions(+), 40 deletions(-)
>
> diff --git a/drivers/scsi/be2iscsi/be_main.c b/drivers/scsi/be2iscsi/be_main.c
> index 89ae6390b697..826c61b3ffcc 100644
> --- a/drivers/scsi/be2iscsi/be_main.c
> +++ b/drivers/scsi/be2iscsi/be_main.c
> @@ -268,7 +268,7 @@ static int beiscsi_eh_abort(struct scsi_cmnd *sc)
>   _cmd.dma);
>   if (nonemb_cmd.va == NULL) {
>   beiscsi_log(phba, KERN_ERR, BEISCSI_LOG_EH,
> - "BM_%d : Failed to allocate memory for"
> + "BM_%d : Failed to allocate memory for "
>   "mgmt_invalidate_icds\n");
>   return FAILED;
>   }
> @@ -278,7 +278,7 @@ static int beiscsi_eh_abort(struct scsi_cmnd *sc)
>  cid, _cmd);
>   if (!tag) {
>   beiscsi_log(phba, KERN_WARNING, BEISCSI_LOG_EH,
> - "BM_%d : mgmt_invalidate_icds could not be"
> + "BM_%d : mgmt_invalidate_icds could not be "
>   "submitted\n");
>   pci_free_consistent(phba->ctrl.pdev, nonemb_cmd.size,
>   nonemb_cmd.va, nonemb_cmd.dma);
> @@ -350,7 +350,7 @@ static int beiscsi_eh_device_reset(struct scsi_cmnd *sc)
>   _cmd.dma);
>   if (nonemb_cmd.va == NULL) {
>   beiscsi_log(phba, KERN_ERR, BEISCSI_LOG_EH,
> - "BM_%d : Failed to allocate memory for"
> + "BM_%d : Failed to allocate memory for "
>   "mgmt_invalidate_icds\n");
>   return FAILED;
>   }
> @@ -1010,7 +1010,7 @@ static int beiscsi_init_irqs(struct beiscsi_hba *phba)
> _context->be_eq[i]);
>   if (ret) {
>   beiscsi_log(phba, KERN_ERR, BEISCSI_LOG_INIT,
> - "BM_%d : beiscsi_init_irqs-Failed 
> to"
> + "BM_%d : beiscsi_init_irqs-Failed 
> to "
>   "register msix for i = %d\n",
>   i);
>   kfree(phba->msi_name[i]);
> @@ -1040,8 +1040,7 @@ static int beiscsi_init_irqs(struct beiscsi_hba *phba)
> "beiscsi", phba);
>   if (ret) {
>   beiscsi_log(phba, KERN_ERR, BEISCSI_LOG_INIT,
> - "BM_%d : beiscsi_init_irqs-"
> - "Failed to register irq\\n");
> + "BM_%d : beiscsi_init_irqs-Failed to 
> register irq\n");
>   return ret;
>   }
>   }
> @@ -1168,7 +1167,7 @@ free_io_sgl_handle(struct beiscsi_hba *phba, struct 
> sgl_handle *psgl_handle)
>* failed in xmit_task or alloc_pdu.
>*/
>beiscsi_log(phba, KERN_INFO, BEISCSI_LOG_IO,
> -  "BM_%d : Double Free in IO SGL 
> io_sgl_free_index=%d,"
> +  "BM_%d : Double Free in IO SGL 
> io_sgl_free_index=%d, "
>"value there=%p\n", phba->io_sgl_free_index,
>phba->io_sgl_hndl_base
>[phba->io_sgl_free_index]);
> @@ -1256,7 +1255,7 @@ free_wrb_handle(struct beiscsi_hba *phba, struct 
> hwi_wrb_context *pwrb_context,
>  phba->params.wrbs_per_cxn);
>   beiscsi_log(phba, KERN_INFO,
>   BEISCSI_LOG_IO | BEISCSI_LOG_CONFIG,
> - "BM_%d : FREE WRB: pwrb_handle=%p free_index=0x%x"
> + "BM_%d : FREE WRB: pwrb_handle=%p free_index=0x%x "
>   "wrb_handles_available=%d\n",
>   pwrb_handle, pwrb_context->free_index,
>   pwrb_context->wrb_handles_available);
> @@ -1293,7 +1292,7 @@ free_mgmt_sgl_handle(struct beiscsi_hba *phba, struct 
> sgl_handle *psgl_handle)
>  {
>   spin_lock_bh(>mgmt_sgl_lock);
>   beiscsi_log(phba, KERN_INFO, BEISCSI_LOG_CONFIG,
> - "BM_%d : In  free_mgmt_sgl_handle,"
> + "BM_%d : In  free_mgmt_sgl_handle, "
>   "eh_sgl_free_index=%d\n",
>   phba->eh_sgl_free_index);
>
> @@ -1303,7 +1302,7 @@ 

Re: [PATCH v3 08/12] qla2xxx: Add framework for Async fabric discovery.

[Julia Lawall]

Hello,

I don't have the compete context, but I have the impression that there
should be a goto done at which sp can be NULL.

julia

-- Forwarded message --
Date: Thu, 19 Jan 2017 12:38:31 +0800
From: kbuild test robot <fengguang...@intel.com>
To: kbu...@01.org
Cc: Julia Lawall <julia.law...@lip6.fr>
Subject: Re: [PATCH v3 08/12] qla2xxx: Add framework for Async fabric discovery.

In-Reply-To: <1484781585-27252-9-git-send-email-himanshu.madh...@cavium.com>

Hi Quinn,

[auto build test WARNING on next-20170118]
[cannot apply to scsi/for-next v4.9-rc8 v4.9-rc7 v4.9-rc6 v4.10-rc4]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Himanshu-Madhani/qla2xxx-Feature-updates-for-target/20170119-105115
:: branch date: 2 hours ago
:: commit date: 2 hours ago

>> drivers/scsi/qla2xxx/qla_init.c:269:5-11: ERROR: sp is NULL but dereferenced.

git remote add linux-review https://github.com/0day-ci/linux
git remote update linux-review
git checkout 8cf2e24938dd5d941b2edc9786f8e1ad8c80d9af
vim +269 drivers/scsi/qla2xxx/qla_init.c

3822263eb Madhuranath Iyengar 2010-05-04  253   lio->timeout = 
qla2x00_async_iocb_timeout;
9ba56b95a Giridhar Malavali   2012-02-09  254   sp->done = 
qla2x00_async_logout_sp_done;
ac280b670 Andrew Vasquez  2009-08-20  255   rval = qla2x00_start_sp(sp);
ac280b670 Andrew Vasquez  2009-08-20  256   if (rval != QLA_SUCCESS)
ac280b670 Andrew Vasquez  2009-08-20  257   goto done_free_sp;
ac280b670 Andrew Vasquez  2009-08-20  258
7c3df1320 Saurav Kashyap  2011-07-14  259   ql_dbg(ql_dbg_disc, vha, 0x2070,
8cf2e2493 Quinn Tran  2017-01-18  260   "Async-logout - hdl=%x 
loop-id=%x portid=%02x%02x%02x %8phC.\n",
cfb0919c1 Chad Dupuis 2011-11-18  261   sp->handle, 
fcport->loop_id, fcport->d_id.b.domain,
8cf2e2493 Quinn Tran  2017-01-18  262   fcport->d_id.b.area, 
fcport->d_id.b.al_pa,
8cf2e2493 Quinn Tran  2017-01-18  263   fcport->port_name);
ac280b670 Andrew Vasquez  2009-08-20  264   return rval;
ac280b670 Andrew Vasquez  2009-08-20  265
ac280b670 Andrew Vasquez  2009-08-20  266  done_free_sp:
9ba56b95a Giridhar Malavali   2012-02-09  267   sp->free(fcport->vha, sp);
ac280b670 Andrew Vasquez  2009-08-20  268  done:
8cf2e2493 Quinn Tran  2017-01-18 @269   sp->fcport->flags &= 
~FCF_ASYNC_SENT;
ac280b670 Andrew Vasquez  2009-08-20  270   return rval;
ac280b670 Andrew Vasquez  2009-08-20  271  }
ac280b670 Andrew Vasquez  2009-08-20  272
5ff1d5841 Andrew Vasquez  2010-05-04  273  static void
9ba56b95a Giridhar Malavali   2012-02-09  274  qla2x00_async_adisc_sp_done(void 
*data, void *ptr, int res)
5ff1d5841 Andrew Vasquez  2010-05-04  275  {
9ba56b95a Giridhar Malavali   2012-02-09  276   srb_t *sp = (srb_t *)ptr;
9ba56b95a Giridhar Malavali   2012-02-09  277   struct srb_iocb *lio = 
>u.iocb_cmd;

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 15/29] be2iscsi: Fix to make boot discovery non-blocking

[Julia Lawall]

Please check the return on line 1517.  It looks suspicious that it does
not release the lock, unlike the return on line 1508.

julia

---

Hi Jitendra,

[auto build test WARNING on scsi/for-next]
[also build test WARNING on v4.8-rc3 next-20160822]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]
[Suggest to use git(>=2.9.0) format-patch --base= (or --base=auto for 
convenience) to record what (public, well-known) commit your patch series was 
built on]
[Check https://git-scm.com/docs/git-format-patch for more information]

url:
https://github.com/0day-ci/linux/commits/Jitendra-Bhivare/be2iscsi-driver-update-11-2-0-0/20160819-175550
base:   https://git.kernel.org/pub/scm/linux/kernel/git/jejb/scsi.git for-next
:: branch date: 4 days ago
:: commit date: 4 days ago

>> drivers/scsi/be2iscsi/be_mgmt.c:1517:2-8: preceding lock on line 1504

git remote add linux-review https://github.com/0day-ci/linux
git remote update linux-review
git checkout b9f267a2cf5f152fb5b5b907b715e0854f337ccd
vim +1517 drivers/scsi/be2iscsi/be_mgmt.c

b9f267a2 Jitendra Bhivare 2016-08-19  1498  struct be_cmd_get_session_req 
*req;
b9f267a2 Jitendra Bhivare 2016-08-19  1499  struct be_dma_mem *nonemb_cmd;
b9f267a2 Jitendra Bhivare 2016-08-19  1500  struct be_mcc_wrb *wrb;
b9f267a2 Jitendra Bhivare 2016-08-19  1501  struct be_sge *sge;
b9f267a2 Jitendra Bhivare 2016-08-19  1502  unsigned int tag;
b9f267a2 Jitendra Bhivare 2016-08-19  1503
b9f267a2 Jitendra Bhivare 2016-08-19 @1504  mutex_lock(>mbox_lock);
b9f267a2 Jitendra Bhivare 2016-08-19  1505  wrb = alloc_mcc_wrb(phba, );
b9f267a2 Jitendra Bhivare 2016-08-19  1506  if (!wrb) {
b9f267a2 Jitendra Bhivare 2016-08-19  1507  
mutex_unlock(>mbox_lock);
b9f267a2 Jitendra Bhivare 2016-08-19  1508  return 0;
b9f267a2 Jitendra Bhivare 2016-08-19  1509  }
b9f267a2 Jitendra Bhivare 2016-08-19  1510
b9f267a2 Jitendra Bhivare 2016-08-19  1511  nonemb_cmd = 
>boot_struct.nonemb_cmd;
b9f267a2 Jitendra Bhivare 2016-08-19  1512  nonemb_cmd->size = 
sizeof(*resp);
b9f267a2 Jitendra Bhivare 2016-08-19  1513  nonemb_cmd->va = 
pci_alloc_consistent(phba->ctrl.pdev,
b9f267a2 Jitendra Bhivare 2016-08-19  1514  
  sizeof(nonemb_cmd->size),
b9f267a2 Jitendra Bhivare 2016-08-19  1515  
  _cmd->dma);
b9f267a2 Jitendra Bhivare 2016-08-19  1516  if (!nonemb_cmd->va)
b9f267a2 Jitendra Bhivare 2016-08-19 @1517  return 0;
b9f267a2 Jitendra Bhivare 2016-08-19  1518
b9f267a2 Jitendra Bhivare 2016-08-19  1519  req = nonemb_cmd->va;
b9f267a2 Jitendra Bhivare 2016-08-19  1520  memset(req, 0, sizeof(*req));

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 12/26] [SCSI] hptiop: constify local structures

[Julia Lawall]

For structure types defined in the same file or local header files, find
top-level static structure declarations that have the following
properties:
1. Never reassigned.
2. Address never taken
3. Not passed to a top-level macro call
4. No pointer or array-typed field passed to a function or stored in a
variable.
Declare structures having all of these properties as const.

Done using Coccinelle.
Based on a suggestion by Joe Perches <j...@perches.com>.

Signed-off-by: Julia Lawall <julia.law...@lip6.fr>

---
The semantic patch seems too long for a commit log, but is in the cover
letter.

 drivers/scsi/hptiop.c |6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/scsi/hptiop.c b/drivers/scsi/hptiop.c
index a83f705..358732d 100644
--- a/drivers/scsi/hptiop.c
+++ b/drivers/scsi/hptiop.c
@@ -1590,7 +1590,7 @@ static void hptiop_remove(struct pci_dev *pcidev)
scsi_host_put(host);
 }
 
-static struct hptiop_adapter_ops hptiop_itl_ops = {
+static const struct hptiop_adapter_ops hptiop_itl_ops = {
.family= INTEL_BASED_IOP,
.iop_wait_ready= iop_wait_ready_itl,
.internal_memalloc = hptiop_internal_memalloc_itl,
@@ -1609,7 +1609,7 @@ static struct hptiop_adapter_ops hptiop_itl_ops = {
.host_phy_flag = cpu_to_le64(0),
 };
 
-static struct hptiop_adapter_ops hptiop_mv_ops = {
+static const struct hptiop_adapter_ops hptiop_mv_ops = {
.family= MV_BASED_IOP,
.iop_wait_ready= iop_wait_ready_mv,
.internal_memalloc = hptiop_internal_memalloc_mv,
@@ -1628,7 +1628,7 @@ static struct hptiop_adapter_ops hptiop_mv_ops = {
.host_phy_flag = cpu_to_le64(0),
 };
 
-static struct hptiop_adapter_ops hptiop_mvfrey_ops = {
+static const struct hptiop_adapter_ops hptiop_mvfrey_ops = {
.family= MVFREY_BASED_IOP,
.iop_wait_ready= iop_wait_ready_mvfrey,
.internal_memalloc = hptiop_internal_memalloc_mvfrey,

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 00/26] constify local structures

[Julia Lawall]

On Sun, 11 Sep 2016, Joe Perches wrote:

> On Sun, 2016-09-11 at 15:05 +0200, Julia Lawall wrote:
> > Constify local structures.
>
> Thanks Julia.
>
> A few suggestions & questions:
>
> Perhaps the script should go into scripts/coccinelle/
> so that future cases could be caught by the robot
> and commit message referenced by the patch instances.

OK.

> Can you please compile the files modified using the
> appropriate defconfig/allyesconfig and show the

I currently send patches for this issue only for files that compile using
the x86 allyesconfig.

> movement from data to const by using
>   $ size .new/old
> and include that in the changelogs (maybe next time)?

OK, thanks for the suggestion.

> Is it possible for a rule to trace the instances where
> an address of a struct or struct member is taken by
> locally defined and declared function call where the
> callee does not modify any dereferenced object?
>
> ie:
>
> struct foo {
>   int bar;
>   char *baz;
> };
>
> struct foo qux[] = {
>   { 1, "description 1" },
>   { 2, "dewcription 2" },
>   [ n, "etc" ]...,
> };
>
> void message(struct foo *msg)
> {
>   printk("%d %s\n", msg->bar, msg->baz);
> }
>
> where some code uses
>
>   message(qux[index]);
>
> So could a coccinelle script change:
>
> struct foo qux[] = { to const struct foo quz[] = {
>
> and
>
> void message(struct foo *msg) to void message(const struct foo *msg)

Yes, this could be possible too.

Thanks for the feedback.

julia
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 22/26] esas2r: constify local structures

[Julia Lawall]

For structure types defined in the same file or local header files, find
top-level static structure declarations that have the following
properties:
1. Never reassigned.
2. Address never taken
3. Not passed to a top-level macro call
4. No pointer or array-typed field passed to a function or stored in a
variable.
Declare structures having all of these properties as const.

Done using Coccinelle.
Based on a suggestion by Joe Perches <j...@perches.com>.

Signed-off-by: Julia Lawall <julia.law...@lip6.fr>

---
The semantic patch seems too long for a commit log, but is in the cover
letter.

 drivers/scsi/esas2r/esas2r_flash.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/esas2r/esas2r_flash.c 
b/drivers/scsi/esas2r/esas2r_flash.c
index 7bd376d..f8414b5 100644
--- a/drivers/scsi/esas2r/esas2r_flash.c
+++ b/drivers/scsi/esas2r/esas2r_flash.c
@@ -54,7 +54,7 @@
 
 #define ESAS2R_FS_DRVR_VER 2
 
-static struct esas2r_sas_nvram default_sas_nvram = {
+static const struct esas2r_sas_nvram default_sas_nvram = {
{ 'E',  'S',  'A',  'S'  }, /* signature  */
SASNVR_VERSION, /* version*/
0,  /* checksum   */

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 00/26] constify local structures

[Julia Lawall]

Constify local structures.

The semantic patch that makes this change is as follows:
(http://coccinelle.lip6.fr/)

// 
// The first rule ignores some cases that posed problems
@r disable optional_qualifier@
identifier s != {peri_clk_data,threshold_attr,tracer_flags,tracer};
identifier i != {s5k5baf_cis_rect,smtcfb_fix};
position p;
@@
static struct s i@p = { ... };

@lstruct@
identifier r.s;
@@
struct s { ... };

@used depends on lstruct@
identifier r.i;
@@
i

@bad1@
expression e;
identifier r.i;
assignment operator a;
@@
 (<+...i...+>) a e

@bad2@
identifier r.i;
@@
 &(<+...i...+>)

@bad3@
identifier r.i;
declarer d;
@@
 d(...,<+...i...+>,...);

@bad4@
identifier r.i;
type T;
T[] e;
identifier f;
position p;
@@

f@p(...,
(
  (<+...i...+>)
&
  e
)
,...)

@bad4a@
identifier r.i;
type T;
T *e;
identifier f;
position p;
@@

f@p(...,
(
  (<+...i...+>)
&
  e
)
,...)

@ok5@
expression *e;
identifier r.i;
position p;
@@
e =@p i

@bad5@
expression *e;
identifier r.i;
position p != ok5.p;
@@
e =@p (<+...i...+>)

@rr depends on used && !bad1 && !bad2 && !bad3 && !bad4 && !bad4a && !bad5@
identifier s,r.i;
position r.p;
@@

static
+const
 struct s i@p = { ... };

@depends on used && !bad1 && !bad2 && !bad3 && !bad4 && !bad4a && !bad5
 disable optional_qualifier@
identifier rr.s,r.i;
@@

static
+const
 struct s i;
// 

---

 drivers/acpi/acpi_apd.c  |8 +++---
 drivers/char/tpm/tpm-interface.c |   10 
 drivers/char/tpm/tpm-sysfs.c |2 -
 drivers/cpufreq/intel_pstate.c   |8 +++---
 drivers/infiniband/hw/i40iw/i40iw_uk.c   |6 ++---
 drivers/media/i2c/tvp514x.c  |2 -
 drivers/media/pci/ddbridge/ddbridge-core.c   |   18 +++
 drivers/media/pci/ngene/ngene-cards.c|   14 ++--
 drivers/media/pci/smipcie/smipcie-main.c |8 +++---
 drivers/misc/sgi-xp/xpc_uv.c |2 -
 drivers/net/arcnet/com20020-pci.c|   10 
 drivers/net/can/c_can/c_can_pci.c|4 +--
 drivers/net/can/sja1000/plx_pci.c|   20 -
 drivers/net/ethernet/mellanox/mlx4/main.c|4 +--
 drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c |2 -
 drivers/net/ethernet/renesas/sh_eth.c|   14 ++--
 drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c |2 -
 drivers/net/wireless/ath/dfs_pattern_detector.c  |2 -
 drivers/net/wireless/intel/iwlegacy/3945.c   |4 +--
 drivers/net/wireless/realtek/rtlwifi/rtl8188ee/sw.c  |2 -
 drivers/net/wireless/realtek/rtlwifi/rtl8192ce/sw.c  |2 -
 drivers/net/wireless/realtek/rtlwifi/rtl8192de/sw.c  |2 -
 drivers/net/wireless/realtek/rtlwifi/rtl8192ee/sw.c  |2 -
 drivers/net/wireless/realtek/rtlwifi/rtl8192se/sw.c  |2 -
 drivers/net/wireless/realtek/rtlwifi/rtl8723ae/sw.c  |2 -
 drivers/net/wireless/realtek/rtlwifi/rtl8723be/sw.c  |2 -
 drivers/net/wireless/realtek/rtlwifi/rtl8821ae/sw.c  |2 -
 drivers/platform/chrome/chromeos_laptop.c|   22 +--
 drivers/platform/x86/intel_scu_ipc.c |6 ++---
 drivers/platform/x86/intel_telemetry_debugfs.c   |2 -
 drivers/scsi/esas2r/esas2r_flash.c   |2 -
 drivers/scsi/hptiop.c|6 ++---
 drivers/spi/spi-dw-pci.c |4 +--
 drivers/staging/rtl8192e/rtl8192e/rtl_core.c |2 -
 drivers/usb/misc/ezusb.c |2 -
 drivers/video/fbdev/matrox/matroxfb_g450.c   |2 -
 lib/crc64_ecma.c |2 -
 sound/pci/ctxfi/ctatc.c  |2 -
 sound/pci/hda/patch_ca0132.c |   10 
 sound/pci/riptide/riptide.c  |2 -
 40 files changed, 110 insertions(+), 110 deletions(-)
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 00/26] constify local structures

[Julia Lawall]

On Sun, 11 Sep 2016, Jarkko Sakkinen wrote:

> On Sun, Sep 11, 2016 at 03:05:42PM +0200, Julia Lawall wrote:
> > Constify local structures.
> >
> > The semantic patch that makes this change is as follows:
> > (http://coccinelle.lip6.fr/)
>
> Just my two cents but:
>
> 1. You *can* use a static analysis too to find bugs or other issues.
> 2. However, you should manually do the commits and proper commit
>messages to subsystems based on your findings. And I generally think
>that if one contributes code one should also at least smoke test changes
>somehow.
>
> I don't know if I'm alone with my opinion. I just think that one should
> also do the analysis part and not blindly create and submit patches.

All of the patches are compile tested.  And the individual patches are
submitted to the relevant maintainers.  The individual commit messages
give a more detailed explanation of the strategy used to decide that the
structure was constifiable.  It seemed redundant to put that in the cover
letter, which will not be committed anyway.

julia

>
> Anyway, I'll apply the TPM change at some point. As I said they were
> for better. Thanks.
>
> /Jarkko
>
> > // 
> > // The first rule ignores some cases that posed problems
> > @r disable optional_qualifier@
> > identifier s != {peri_clk_data,threshold_attr,tracer_flags,tracer};
> > identifier i != {s5k5baf_cis_rect,smtcfb_fix};
> > position p;
> > @@
> > static struct s i@p = { ... };
> >
> > @lstruct@
> > identifier r.s;
> > @@
> > struct s { ... };
> >
> > @used depends on lstruct@
> > identifier r.i;
> > @@
> > i
> >
> > @bad1@
> > expression e;
> > identifier r.i;
> > assignment operator a;
> > @@
> >  (<+...i...+>) a e
> >
> > @bad2@
> > identifier r.i;
> > @@
> >  &(<+...i...+>)
> >
> > @bad3@
> > identifier r.i;
> > declarer d;
> > @@
> >  d(...,<+...i...+>,...);
> >
> > @bad4@
> > identifier r.i;
> > type T;
> > T[] e;
> > identifier f;
> > position p;
> > @@
> >
> > f@p(...,
> > (
> >   (<+...i...+>)
> > &
> >   e
> > )
> > ,...)
> >
> > @bad4a@
> > identifier r.i;
> > type T;
> > T *e;
> > identifier f;
> > position p;
> > @@
> >
> > f@p(...,
> > (
> >   (<+...i...+>)
> > &
> >   e
> > )
> > ,...)
> >
> > @ok5@
> > expression *e;
> > identifier r.i;
> > position p;
> > @@
> > e =@p i
> >
> > @bad5@
> > expression *e;
> > identifier r.i;
> > position p != ok5.p;
> > @@
> > e =@p (<+...i...+>)
> >
> > @rr depends on used && !bad1 && !bad2 && !bad3 && !bad4 && !bad4a && !bad5@
> > identifier s,r.i;
> > position r.p;
> > @@
> >
> > static
> > +const
> >  struct s i@p = { ... };
> >
> > @depends on used && !bad1 && !bad2 && !bad3 && !bad4 && !bad4a && !bad5
> >  disable optional_qualifier@
> > identifier rr.s,r.i;
> > @@
> >
> > static
> > +const
> >  struct s i;
> > // 
> >
> > ---
> >
> >  drivers/acpi/acpi_apd.c  |8 +++---
> >  drivers/char/tpm/tpm-interface.c |   10 
> >  drivers/char/tpm/tpm-sysfs.c |2 -
> >  drivers/cpufreq/intel_pstate.c   |8 +++---
> >  drivers/infiniband/hw/i40iw/i40iw_uk.c   |6 ++---
> >  drivers/media/i2c/tvp514x.c  |2 -
> >  drivers/media/pci/ddbridge/ddbridge-core.c   |   18 +++
> >  drivers/media/pci/ngene/ngene-cards.c|   14 ++--
> >  drivers/media/pci/smipcie/smipcie-main.c |8 +++---
> >  drivers/misc/sgi-xp/xpc_uv.c |2 -
> >  drivers/net/arcnet/com20020-pci.c|   10 
> >  drivers/net/can/c_can/c_can_pci.c|4 +--
> >  drivers/net/can/sja1000/plx_pci.c|   20 
> > -
> >  drivers/net/ethernet/mellanox/mlx4/main.c|4 +--
> >  drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c |2 -
> >  drivers/net/ethernet/renesas/sh_eth.c|   14 ++--
> >  drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c |2 -
> > 

Re: [PATCH 12/26] [SCSI] hptiop: constify local structures

[Julia Lawall]

On Sun, 11 Sep 2016, Julia Lawall wrote:

> For structure types defined in the same file or local header files, find
> top-level static structure declarations that have the following
> properties:
> 1. Never reassigned.
> 2. Address never taken
> 3. Not passed to a top-level macro call
> 4. No pointer or array-typed field passed to a function or stored in a
> variable.
> Declare structures having all of these properties as const.

Actually, this patch should be dropped.  Coccinelle did not recognize
kernel_ulong_t as a type, so it considered eg

(kernel_ulong_t)_itl_ops

as a bit and operation.

julia


> Done using Coccinelle.
> Based on a suggestion by Joe Perches <j...@perches.com>.
>
> Signed-off-by: Julia Lawall <julia.law...@lip6.fr>
>
> ---
> The semantic patch seems too long for a commit log, but is in the cover
> letter.
>
>  drivers/scsi/hptiop.c |6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/scsi/hptiop.c b/drivers/scsi/hptiop.c
> index a83f705..358732d 100644
> --- a/drivers/scsi/hptiop.c
> +++ b/drivers/scsi/hptiop.c
> @@ -1590,7 +1590,7 @@ static void hptiop_remove(struct pci_dev *pcidev)
>   scsi_host_put(host);
>  }
>
> -static struct hptiop_adapter_ops hptiop_itl_ops = {
> +static const struct hptiop_adapter_ops hptiop_itl_ops = {
>   .family= INTEL_BASED_IOP,
>   .iop_wait_ready= iop_wait_ready_itl,
>   .internal_memalloc = hptiop_internal_memalloc_itl,
> @@ -1609,7 +1609,7 @@ static struct hptiop_adapter_ops hptiop_itl_ops = {
>   .host_phy_flag = cpu_to_le64(0),
>  };
>
> -static struct hptiop_adapter_ops hptiop_mv_ops = {
> +static const struct hptiop_adapter_ops hptiop_mv_ops = {
>   .family= MV_BASED_IOP,
>   .iop_wait_ready= iop_wait_ready_mv,
>   .internal_memalloc = hptiop_internal_memalloc_mv,
> @@ -1628,7 +1628,7 @@ static struct hptiop_adapter_ops hptiop_mv_ops = {
>   .host_phy_flag = cpu_to_le64(0),
>  };
>
> -static struct hptiop_adapter_ops hptiop_mvfrey_ops = {
> +static const struct hptiop_adapter_ops hptiop_mvfrey_ops = {
>   .family= MVFREY_BASED_IOP,
>   .iop_wait_ready= iop_wait_ready_mvfrey,
>   .internal_memalloc = hptiop_internal_memalloc_mvfrey,
>
> --
> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 00/26] constify local structures

[Julia Lawall]

On Mon, 12 Sep 2016, Jarkko Sakkinen wrote:

> On Mon, Sep 12, 2016 at 10:54:07AM +0200, Julia Lawall wrote:
> >
> >
> > On Sun, 11 Sep 2016, Jarkko Sakkinen wrote:
> >
> > > On Sun, Sep 11, 2016 at 03:05:42PM +0200, Julia Lawall wrote:
> > > > Constify local structures.
> > > >
> > > > The semantic patch that makes this change is as follows:
> > > > (http://coccinelle.lip6.fr/)
> > >
> > > Just my two cents but:
> > >
> > > 1. You *can* use a static analysis too to find bugs or other issues.
> > > 2. However, you should manually do the commits and proper commit
> > >messages to subsystems based on your findings. And I generally think
> > >that if one contributes code one should also at least smoke test 
> > > changes
> > >somehow.
> > >
> > > I don't know if I'm alone with my opinion. I just think that one should
> > > also do the analysis part and not blindly create and submit patches.
> >
> > All of the patches are compile tested.  And the individual patches are
>
> Compile-testing is not testing. If you are not able to test a commit,
> you should explain why.
>
> > submitted to the relevant maintainers.  The individual commit messages
> > give a more detailed explanation of the strategy used to decide that the
> > structure was constifiable.  It seemed redundant to put that in the cover
> > letter, which will not be committed anyway.
>
> I don't mean to be harsh but I do not care about your thought process
> *that much* when I review a commit (sometimes it might make sense to
> explain that but it depends on the context).
>
> I mostly only care why a particular change makes sense for this
> particular subsystem. The report given by a static analysis tool can
> be a starting point for making a commit but it's not sufficient.
> Based on the report you should look subsystems as individuals.

OK, thanks for the feedback.

julia
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 00/26] constify local structures

[Julia Lawall]

On Mon, 12 Sep 2016, Felipe Balbi wrote:

>
> Hi,
>
> Jarkko Sakkinen <jarkko.sakki...@linux.intel.com> writes:
> > On Mon, Sep 12, 2016 at 10:54:07AM +0200, Julia Lawall wrote:
> >>
> >>
> >> On Sun, 11 Sep 2016, Jarkko Sakkinen wrote:
> >>
> >> > On Sun, Sep 11, 2016 at 03:05:42PM +0200, Julia Lawall wrote:
> >> > > Constify local structures.
> >> > >
> >> > > The semantic patch that makes this change is as follows:
> >> > > (http://coccinelle.lip6.fr/)
> >> >
> >> > Just my two cents but:
> >> >
> >> > 1. You *can* use a static analysis too to find bugs or other issues.
> >> > 2. However, you should manually do the commits and proper commit
> >> >messages to subsystems based on your findings. And I generally think
> >> >that if one contributes code one should also at least smoke test 
> >> > changes
> >> >somehow.
> >> >
> >> > I don't know if I'm alone with my opinion. I just think that one should
> >> > also do the analysis part and not blindly create and submit patches.
> >>
> >> All of the patches are compile tested.  And the individual patches are
> >
> > Compile-testing is not testing. If you are not able to test a commit,
> > you should explain why.
>
> Dude, Julia has been doing semantic patching for years already and
> nobody has raised any concerns so far. There's already an expectation
> that Coccinelle *works* and Julia's sematic patches are sound.
>
> Besides, adding 'const' is something that causes virtually no functional
> changes to the point that build-testing is really all you need. Any
> problems caused by adding 'const' to a definition will be seen by build
> errors or warnings.
>
> Really, just stop with the pointless discussion and go read a bit about
> Coccinelle and what semantic patches are giving you. The work done by
> Julia and her peers are INRIA have measurable benefits.
>
> You're really making a thunderstorm in a glass of water.

Thanks for the defense, but since a lot of these patches torned out to be
wrong, due to an incorrect parse by Coccinelle, combined with an
unpleasantly lax compiler, Jarkko does have a point that I should have
looked at the patches more carefully.  In any case, I have written to the
maintainers relevant to the patches that turned out to be incorrect.

julia
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 00/26] constify local structures

[Julia Lawall]

On Mon, 12 Sep 2016, Jarkko Sakkinen wrote:

> On Mon, Sep 12, 2016 at 04:43:58PM +0300, Felipe Balbi wrote:
> >
> > Hi,
> >
> > Jarkko Sakkinen <jarkko.sakki...@linux.intel.com> writes:
> > > On Mon, Sep 12, 2016 at 10:54:07AM +0200, Julia Lawall wrote:
> > >>
> > >>
> > >> On Sun, 11 Sep 2016, Jarkko Sakkinen wrote:
> > >>
> > >> > On Sun, Sep 11, 2016 at 03:05:42PM +0200, Julia Lawall wrote:
> > >> > > Constify local structures.
> > >> > >
> > >> > > The semantic patch that makes this change is as follows:
> > >> > > (http://coccinelle.lip6.fr/)
> > >> >
> > >> > Just my two cents but:
> > >> >
> > >> > 1. You *can* use a static analysis too to find bugs or other issues.
> > >> > 2. However, you should manually do the commits and proper commit
> > >> >messages to subsystems based on your findings. And I generally think
> > >> >that if one contributes code one should also at least smoke test 
> > >> > changes
> > >> >somehow.
> > >> >
> > >> > I don't know if I'm alone with my opinion. I just think that one should
> > >> > also do the analysis part and not blindly create and submit patches.
> > >>
> > >> All of the patches are compile tested.  And the individual patches are
> > >
> > > Compile-testing is not testing. If you are not able to test a commit,
> > > you should explain why.
> >
> > Dude, Julia has been doing semantic patching for years already and
> > nobody has raised any concerns so far. There's already an expectation
> > that Coccinelle *works* and Julia's sematic patches are sound.
> >
> > Besides, adding 'const' is something that causes virtually no functional
> > changes to the point that build-testing is really all you need. Any
> > problems caused by adding 'const' to a definition will be seen by build
> > errors or warnings.
> >
> > Really, just stop with the pointless discussion and go read a bit about
> > Coccinelle and what semantic patches are giving you. The work done by
> > Julia and her peers are INRIA have measurable benefits.
> >
> > You're really making a thunderstorm in a glass of water.
>
> Hmm... I've been using coccinelle in cyclic basis for some time now.
> My comment was oversized but I didn't mean it to be impolite or attack
> of any kind for that matter.

No problem :)  Thanks for the feedback.

julia
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] scsi: constify sr_pm_ops structure

[Julia Lawall]

sr_pm_ops, of type struct dev_pm_ops, is never modified, so declare it as
const.

Done with the help of Coccinelle.

Signed-off-by: Julia Lawall <julia.law...@lip6.fr>

---
 drivers/scsi/sr.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/sr.c b/drivers/scsi/sr.c
index ed17934..bed2bbd 100644
--- a/drivers/scsi/sr.c
+++ b/drivers/scsi/sr.c
@@ -83,7 +83,7 @@ static int sr_init_command(struct scsi_cmnd *SCpnt);
 static int sr_done(struct scsi_cmnd *);
 static int sr_runtime_suspend(struct device *dev);
 
-static struct dev_pm_ops sr_pm_ops = {
+static const struct dev_pm_ops sr_pm_ops = {
.runtime_suspend= sr_runtime_suspend,
 };
 

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V4 04/11] megaraid_sas: SAS3.5 Generic Megaraid Controllers Stream Detection and IO Coalescing (fwd)

[Julia Lawall]

The code on lines 1744 and 1749 seems to need to be indented more.  1744
should be lined up with the inside of the first ( and the comment and the
continue should be indented.

julia



-- Forwarded message --
Date: Wed, 7 Dec 2016 12:14:15 +0800
From: kbuild test robot <fengguang...@intel.com>
To: kbu...@01.org
Cc: Julia Lawall <julia.law...@lip6.fr>
Subject: Re: [PATCH V4 04/11] megaraid_sas: SAS3.5 Generic Megaraid Controllers
Stream Detection and IO Coalescing

In-Reply-To: <1481065220-18431-5-git-send-email-sasikumar...@broadcom.com>

Hi Sasikumar,

[auto build test WARNING on scsi/for-next]
[also build test WARNING on next-20161206]
[cannot apply to v4.9-rc8]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Sasikumar-Chandrasekaran/megaraid_sas-Updates-for-scsi-next/20161207-102153
base:   https://git.kernel.org/pub/scm/linux/kernel/git/jejb/scsi.git for-next
:: branch date: 2 hours ago
:: commit date: 2 hours ago

>> drivers/scsi/megaraid/megaraid_sas_fusion.c:1749:3-12: code aligned with 
>> following code on line 1750

git remote add linux-review https://github.com/0day-ci/linux
git remote update linux-review
git checkout c919127dc789eec06ff5e34e06ebb05ef30dbb5e
vim +1749 drivers/scsi/megaraid/megaraid_sas_fusion.c

c919127d Sasikumar Chandrasekaran 2016-12-06  1743  if 
((io_info->ldStartBlock != current_sd->next_seq_lba) &&
c919127d Sasikumar Chandrasekaran 2016-12-06  1744  
((!io_info->isRead) || (!is_read_ahead)))
c919127d Sasikumar Chandrasekaran 2016-12-06  1745  /*
c919127d Sasikumar Chandrasekaran 2016-12-06  1746  * Once 
the API availible we need to change this.
c919127d Sasikumar Chandrasekaran 2016-12-06  1747  * At 
this point we are not allowing any gap
c919127d Sasikumar Chandrasekaran 2016-12-06  1748  */
c919127d Sasikumar Chandrasekaran 2016-12-06 @1749  
continue;
c919127d Sasikumar Chandrasekaran 2016-12-06 @1750  
cmd->io_request->RaidContext.raid_context_g35.stream_detected
c919127d Sasikumar Chandrasekaran 2016-12-06  1751  
= true;
c919127d Sasikumar Chandrasekaran 2016-12-06  1752  
current_sd->next_seq_lba =
c919127d Sasikumar Chandrasekaran 2016-12-06  1753  
io_info->ldStartBlock + io_info->numBlocks;

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 04/22] target: Make use of the new sg_map function at 16 call sites (fwd)

[Julia Lawall]

It looks like >cmdr_lock should be released at line 512 if it has
not been released otherwise.  The lock was taken at line 438.

julia

-- Forwarded message --
Date: Fri, 14 Apr 2017 22:21:44 +0800
From: kbuild test robot <fengguang...@intel.com>
To: kbu...@01.org
Cc: Julia Lawall <julia.law...@lip6.fr>
Subject: Re: [PATCH 04/22] target: Make use of the new sg_map function at 16
call sites

Hi Logan,

[auto build test WARNING on scsi/for-next]
[also build test WARNING on v4.11-rc6]
[cannot apply to next-20170413]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Logan-Gunthorpe/Introduce-common-scatterlist-map-function/20170414-142518
base:   https://git.kernel.org/pub/scm/linux/kernel/git/jejb/scsi.git for-next
:: branch date: 8 hours ago
:: commit date: 8 hours ago

>> drivers/target/target_core_user.c:512:2-8: preceding lock on line 438
   drivers/target/target_core_user.c:512:2-8: preceding lock on line 471

git remote add linux-review https://github.com/0day-ci/linux
git remote update linux-review
git checkout 78082134e7afdc59d744eb8d2def5c588e89c378
vim +512 drivers/target/target_core_user.c

7c9e7a6f Andy Grover  2014-10-01  432   
sizeof(struct tcmu_cmd_entry));
7c9e7a6f Andy Grover  2014-10-01  433   command_size = base_command_size
7c9e7a6f Andy Grover  2014-10-01  434   + 
round_up(scsi_command_size(se_cmd->t_task_cdb), TCMU_OP_ALIGN_SIZE);
7c9e7a6f Andy Grover  2014-10-01  435
7c9e7a6f Andy Grover  2014-10-01  436   WARN_ON(command_size & 
(TCMU_OP_ALIGN_SIZE-1));
7c9e7a6f Andy Grover  2014-10-01  437
7c9e7a6f Andy Grover  2014-10-01 @438   spin_lock_irq(>cmdr_lock);
7c9e7a6f Andy Grover  2014-10-01  439
7c9e7a6f Andy Grover  2014-10-01  440   mb = udev->mb_addr;
7c9e7a6f Andy Grover  2014-10-01  441   cmd_head = mb->cmd_head % 
udev->cmdr_size; /* UAM */
26418649 Sheng Yang   2016-02-26  442   data_length = 
se_cmd->data_length;
26418649 Sheng Yang   2016-02-26  443   if (se_cmd->se_cmd_flags & 
SCF_BIDI) {
26418649 Sheng Yang   2016-02-26  444   
BUG_ON(!(se_cmd->t_bidi_data_sg && se_cmd->t_bidi_data_nents));
26418649 Sheng Yang   2016-02-26  445   data_length += 
se_cmd->t_bidi_data_sg->length;
26418649 Sheng Yang   2016-02-26  446   }
554617b2 Andy Grover  2016-08-25  447   if ((command_size > 
(udev->cmdr_size / 2)) ||
554617b2 Andy Grover  2016-08-25  448   data_length > 
udev->data_size) {
554617b2 Andy Grover  2016-08-25  449   pr_warn("TCMU: Request 
of size %zu/%zu is too big for %u/%zu "
3d9b9555 Andy Grover  2016-08-25  450   "cmd ring/data 
area\n", command_size, data_length,
7c9e7a6f Andy Grover  2014-10-01  451   
udev->cmdr_size, udev->data_size);
554617b2 Andy Grover  2016-08-25  452   
spin_unlock_irq(>cmdr_lock);
554617b2 Andy Grover  2016-08-25  453   return 
TCM_INVALID_CDB_FIELD;
554617b2 Andy Grover  2016-08-25  454   }
7c9e7a6f Andy Grover  2014-10-01  455
26418649 Sheng Yang   2016-02-26  456   while 
(!is_ring_space_avail(udev, command_size, data_length)) {
7c9e7a6f Andy Grover  2014-10-01  457   int ret;
7c9e7a6f Andy Grover  2014-10-01  458   DEFINE_WAIT(__wait);
7c9e7a6f Andy Grover  2014-10-01  459
7c9e7a6f Andy Grover  2014-10-01  460   
prepare_to_wait(>wait_cmdr, &__wait, TASK_INTERRUPTIBLE);
7c9e7a6f Andy Grover  2014-10-01  461
7c9e7a6f Andy Grover  2014-10-01  462   pr_debug("sleeping for 
ring space\n");
7c9e7a6f Andy Grover  2014-10-01  463   
spin_unlock_irq(>cmdr_lock);
7c9e7a6f Andy Grover  2014-10-01  464   ret = 
schedule_timeout(msecs_to_jiffies(TCMU_TIME_OUT));
7c9e7a6f Andy Grover  2014-10-01  465   
finish_wait(>wait_cmdr, &__wait);
7c9e7a6f Andy Grover  2014-10-01  466   if (!ret) {
7c9e7a6f Andy Grover  2014-10-01  467   pr_warn("tcmu: 
command timed out\n");
02eb924f Andy Grover  2016-10-06  468   return 
TCM_LOGICAL_UNIT_COMMUNICATION_FAILURE;
7c9e7a6f Andy Grover  2014-10-01  469   }
7c9e7a6f Andy Grover  2014-10-01  470
7c9e7a6f Andy Grover  2014-10-01  471   
spin_lock_irq(>cmdr_lock);
7c9e7a6f Andy Grover  2014-10-01  472
7c9e7a6f Andy Grover  2014-10-01  473   /* We dropped 
cmdr_lock, cmd_head is stale */
7c9e7a6f Andy Grover  2014-10-01  474   cmd_head = mb->cmd_head 
% udev->cmdr_size; /* UAM */
7c9e7a6f Andy Grover

[PATCH 1/6] scsi: mpt3sas: constify pci_error_handlers structures

[Julia Lawall]

These pci_error_handlers structures are only stored in the err_handler
field of a pci_driver structure, and this field is declared as const.  Thus
the pci_error_handlers structures can be const too.

Done with the help of Coccinelle.

Signed-off-by: Julia Lawall <julia.law...@lip6.fr>

---
 drivers/scsi/mpt3sas/mpt3sas_scsih.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/mpt3sas/mpt3sas_scsih.c 
b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
index 22998cb..52c163f 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_scsih.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
@@ -9277,7 +9277,7 @@ bool scsih_ncq_prio_supp(struct scsi_device *sdev)
 };
 MODULE_DEVICE_TABLE(pci, mpt3sas_pci_table);
 
-static struct pci_error_handlers _mpt3sas_err_handler = {
+static const struct pci_error_handlers _mpt3sas_err_handler = {
.error_detected = scsih_pci_error_detected,
.mmio_enabled   = scsih_pci_mmio_enabled,
.slot_reset = scsih_pci_slot_reset,

[PATCH 4/6] scsi: be2iscsi: constify pci_error_handlers structures

[Julia Lawall]

These pci_error_handlers structures are only stored in the err_handler
field of a pci_driver structure, and this field is declared as const.  Thus
the pci_error_handlers structures can be const too.

Done with the help of Coccinelle.

Signed-off-by: Julia Lawall <julia.law...@lip6.fr>

---
 drivers/scsi/be2iscsi/be_main.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/be2iscsi/be_main.c b/drivers/scsi/be2iscsi/be_main.c
index b4542e7..511732a 100644
--- a/drivers/scsi/be2iscsi/be_main.c
+++ b/drivers/scsi/be2iscsi/be_main.c
@@ -5786,7 +5786,7 @@ static void beiscsi_remove(struct pci_dev *pcidev)
 }
 
 
-static struct pci_error_handlers beiscsi_eeh_handlers = {
+static const struct pci_error_handlers beiscsi_eeh_handlers = {
.error_detected = beiscsi_eeh_err_detected,
.slot_reset = beiscsi_eeh_reset,
.resume = beiscsi_eeh_resume,

[PATCH 3/6] scsi: aacraid: constify pci_error_handlers structures

[Julia Lawall]

These pci_error_handlers structures are only stored in the err_handler
field of a pci_driver structure, and this field is declared as const.  Thus
the pci_error_handlers structures can be const too.

Done with the help of Coccinelle.

Signed-off-by: Julia Lawall <julia.law...@lip6.fr>

---
 drivers/scsi/aacraid/linit.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/aacraid/linit.c b/drivers/scsi/aacraid/linit.c
index a8dedc3..2b978d8 100644
--- a/drivers/scsi/aacraid/linit.c
+++ b/drivers/scsi/aacraid/linit.c
@@ -2070,7 +2070,7 @@ static void aac_pci_resume(struct pci_dev *pdev)
dev_err(>dev, "aacraid: PCI error - resume\n");
 }
 
-static struct pci_error_handlers aac_pci_err_handler = {
+static const struct pci_error_handlers aac_pci_err_handler = {
.error_detected = aac_pci_error_detected,
.mmio_enabled   = aac_pci_mmio_enabled,
.slot_reset = aac_pci_slot_reset,

[PATCH 0/6] constify pci_error_handlers structures

[Julia Lawall]

These pci_error_handlers structures are only stored in the err_handler
field of a pci_driver structure, and this field is declared as const.  Thus
the pci_error_handlers structures can be const too.

Done with the help of Coccinelle.

---

 drivers/misc/genwqe/card_base.c  |2 +-
 drivers/scsi/aacraid/linit.c |2 +-
 drivers/scsi/be2iscsi/be_main.c  |2 +-
 drivers/scsi/bfa/bfad.c  |2 +-
 drivers/scsi/csiostor/csio_init.c|2 +-
 drivers/scsi/mpt3sas/mpt3sas_scsih.c |2 +-
 6 files changed, 6 insertions(+), 6 deletions(-)

[PATCH 6/6] bfa: constify pci_error_handlers structures

[Julia Lawall]

These pci_error_handlers structures are only stored in the err_handler
field of a pci_driver structure, and this field is declared as const.  Thus
the pci_error_handlers structures can be const too.

Done with the help of Coccinelle.

Signed-off-by: Julia Lawall <julia.law...@lip6.fr>

---
 drivers/scsi/bfa/bfad.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/bfa/bfad.c b/drivers/scsi/bfa/bfad.c
index 5caf5f3..2861694 100644
--- a/drivers/scsi/bfa/bfad.c
+++ b/drivers/scsi/bfa/bfad.c
@@ -1683,7 +1683,7 @@ struct pci_device_id bfad_id_table[] = {
 /*
  * PCI error recovery handlers.
  */
-static struct pci_error_handlers bfad_err_handler = {
+static const struct pci_error_handlers bfad_err_handler = {
.error_detected = bfad_pci_error_detected,
.slot_reset = bfad_pci_slot_reset,
.mmio_enabled = bfad_pci_mmio_enabled,

[PATCH 5/6] [SCSI] csiostor: constify pci_error_handlers structures

[Julia Lawall]

These pci_error_handlers structures are only stored in the err_handler
field of a pci_driver structure, and this field is declared as const.  Thus
the pci_error_handlers structures can be const too.

Done with the help of Coccinelle.

Signed-off-by: Julia Lawall <julia.law...@lip6.fr>

---
 drivers/scsi/csiostor/csio_init.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/csiostor/csio_init.c 
b/drivers/scsi/csiostor/csio_init.c
index 28a9c7d..ab4fbcf 100644
--- a/drivers/scsi/csiostor/csio_init.c
+++ b/drivers/scsi/csiostor/csio_init.c
@@ -1168,7 +1168,7 @@ static void csio_remove_one(struct pci_dev *pdev)
dev_err(>dev, "resume of device failed: %d\n", rv);
 }
 
-static struct pci_error_handlers csio_err_handler = {
+static const struct pci_error_handlers csio_err_handler = {
.error_detected = csio_pci_error_detected,
.slot_reset = csio_pci_slot_reset,
.resume = csio_pci_resume,

Re: [PATCH 0/6] constify pci_error_handlers structures

[Julia Lawall]

On Sat, 12 Aug 2017, Christoph Hellwig wrote:

> On Sat, Aug 12, 2017 at 09:52:28AM +0200, Julia Lawall wrote:
> > OK, sure.  So to be precise, you want the fields error_detected,
> > mmio_enabled, etc to be added as new fields to the pci_driver structure?
>
> Yes.
>
> > They both have a resume field, though.  What should the pci_error_handlers
> > resume function be renamed to?  Would resume_after_error be too much?
>
> error_resume maybe?

OK

>
> FYI, I already killed it for the PCIe port drivers a while ago:
>
> https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git/commit/?h=pci/aer=c5dc3c69f17a7e77359f10c342d1816390bc8846

Thanks for the pointer.

julia

Re: [PATCH 0/6] constify pci_error_handlers structures

[Julia Lawall]

On Sat, 12 Aug 2017, Christoph Hellwig wrote:

> On Sat, Aug 12, 2017 at 07:44:28AM +0200, Julia Lawall wrote:
> > These pci_error_handlers structures are only stored in the err_handler
> > field of a pci_driver structure, and this field is declared as const.  Thus
> > the pci_error_handlers structures can be const too.
> >
> > Done with the help of Coccinelle.
>
> If you're doing a scripted conversion of the pci_error_handlers
> structured I'd much rather see that structure killed off and folded
> into the pci_driver one.

OK, sure.  So to be precise, you want the fields error_detected,
mmio_enabled, etc to be added as new fields to the pci_driver structure?

They both have a resume field, though.  What should the pci_error_handlers
resume function be renamed to?  Would resume_after_error be too much?

julia

Re: [PATCH 0/6] constify pci_error_handlers structures

[Julia Lawall]

Another issue arises in the files drivers/infiniband/hw/hfi1/pcie.c and
drivers/infiniband/hw/qib/qib_pcie.c, where the pci_error_handlers
structure is defined in one file and used in another file.  The structure
definition references various functions that are static in the same file.
Should I try to move those functions to the file containing the pci_driver
structure?  Or leave the functions where they are and remove the static
annotation?

thanks,
julia

Re: [PATCH] qla2xxx: Protect access to qpair members with qpair->qp_lock (fwd)

[Julia Lawall]

Please check on whether an unlock is neeed before line 1965.

julia

-- Forwarded message --
Date: Fri, 23 Jun 2017 15:23:00 +0800
From: kbuild test robot <fengguang...@intel.com>
To: kbu...@01.org
Cc: Julia Lawall <julia.law...@lip6.fr>
Subject: Re: [PATCH] qla2xxx: Protect access to qpair members with
qpair->qp_lock

CC: kbuild-...@01.org
In-Reply-To: <20170622134325.26931-1-jthumsh...@suse.de>

Hi Johannes,

[auto build test WARNING on scsi/for-next]
[also build test WARNING on v4.12-rc6 next-20170622]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Johannes-Thumshirn/qla2xxx-Protect-access-to-qpair-members-with-qpair-qp_lock/20170623-123844
base:   https://git.kernel.org/pub/scm/linux/kernel/git/jejb/scsi.git for-next
:: branch date: 3 hours ago
:: commit date: 3 hours ago

>> drivers/scsi/qla2xxx/qla_iocb.c:1965:3-9: preceding lock on line 1952

git remote add linux-review https://github.com/0day-ci/linux
git remote update linux-review
git checkout 4a35d720268dbe9ac016957a3c4fc644398d68ba
vim +1965 drivers/scsi/qla2xxx/qla_iocb.c

d74595278 Michael Hernandez  2016-12-12  1946   /* Only process protection or 
>16 cdb in this routine */
d74595278 Michael Hernandez  2016-12-12  1947   if (scsi_get_prot_op(cmd) == 
SCSI_PROT_NORMAL) {
d74595278 Michael Hernandez  2016-12-12  1948   if (cmd->cmd_len <= 16)
d74595278 Michael Hernandez  2016-12-12  1949   return 
qla2xxx_start_scsi_mq(sp);
d74595278 Michael Hernandez  2016-12-12  1950   }
d74595278 Michael Hernandez  2016-12-12  1951
4a35d7202 Johannes Thumshirn 2017-06-22 @1952   
spin_lock_irqsave(>qp_lock, flags);
4a35d7202 Johannes Thumshirn 2017-06-22  1953
d74595278 Michael Hernandez  2016-12-12  1954   /* Setup qpair pointers */
d74595278 Michael Hernandez  2016-12-12  1955   rsp = qpair->rsp;
d74595278 Michael Hernandez  2016-12-12  1956   req = qpair->req;
d74595278 Michael Hernandez  2016-12-12  1957
d74595278 Michael Hernandez  2016-12-12  1958   /* So we know we haven't 
pci_map'ed anything yet */
d74595278 Michael Hernandez  2016-12-12  1959   tot_dsds = 0;
d74595278 Michael Hernandez  2016-12-12  1960
d74595278 Michael Hernandez  2016-12-12  1961   /* Send marker if required */
d74595278 Michael Hernandez  2016-12-12  1962   if (vha->marker_needed != 0) {
d74595278 Michael Hernandez  2016-12-12  1963   if (qla2x00_marker(vha, 
req, rsp, 0, 0, MK_SYNC_ALL) !=
d74595278 Michael Hernandez  2016-12-12  1964   QLA_SUCCESS)
d74595278 Michael Hernandez  2016-12-12 @1965   return 
QLA_FUNCTION_FAILED;
d74595278 Michael Hernandez  2016-12-12  1966   vha->marker_needed = 0;
d74595278 Michael Hernandez  2016-12-12  1967   }
d74595278 Michael Hernandez  2016-12-12  1968

:: The code at line 1965 was first introduced by commit
:: d74595278f4ab192af66d9e60a9087464638beee scsi: qla2xxx: Add multiple 
queue pair functionality.

:: TO: Michael Hernandez <michael.hernan...@cavium.com>
:: CC: Martin K. Petersen <martin.peter...@oracle.com>

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation

Re: [PATCH 34/35] scsi: Move eh_device_reset_handler() to use scsi_device as argument (fwd)

[Julia Lawall]

Hello,

It looks like the goto on line 2374 could result in the io_lock not being
released.

julia

-- Forwarded message --
Date: Sat, 24 Jun 2017 13:27:41 +0800
From: kbuild test robot <fengguang...@intel.com>
To: kbu...@01.org
Cc: Julia Lawall <julia.law...@lip6.fr>
Subject: Re: [PATCH 34/35] scsi: Move eh_device_reset_handler() to use
scsi_device as argument

Hi Hannes,

[auto build test WARNING on mkp-scsi/for-next]
[also build test WARNING on next-20170623]
[cannot apply to v4.12-rc6]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Hannes-Reinecke/SCSI-EH-argument-reshuffling/20170624-071433
base:   https://git.kernel.org/pub/scm/linux/kernel/git/mkp/scsi.git for-next
:: branch date: 6 hours ago
:: commit date: 6 hours ago

>> drivers/scsi/fnic/fnic_scsi.c:2546:1-7: preceding lock on line 2369

git remote add linux-review https://github.com/0day-ci/linux
git remote update linux-review
git checkout fb100e8dae35309fce9f6f27ae70b8498ebb479a
vim +2546 drivers/scsi/fnic/fnic_scsi.c

67125b028 Hiral Patel   2013-09-12  2363
atomic64_inc(_stats->misc_stats.rport_not_ready);
5df6d737d Abhijeet Joglekar 2009-04-17  2364goto 
fnic_device_reset_end;
67125b028 Hiral Patel   2013-09-12  2365}
fb100e8da Hannes Reinecke   2017-06-23  2366/* The last tag is reserved for 
device reset */
fb100e8da Hannes Reinecke   2017-06-23  2367sc = 
scsi_host_find_tag(sdev->host, fnic->fnic_max_tag_id - 1);
5df6d737d Abhijeet Joglekar 2009-04-17  2368io_lock = 
fnic_io_lock_hash(fnic, sc);
5df6d737d Abhijeet Joglekar 2009-04-17 @2369spin_lock_irqsave(io_lock, 
flags);
fb100e8da Hannes Reinecke   2017-06-23  2370if (CMD_SP(sc)) {
5df6d737d Abhijeet Joglekar 2009-04-17  2371/*
fb100e8da Hannes Reinecke   2017-06-23  2372 * Reset tag busy
5df6d737d Abhijeet Joglekar 2009-04-17  2373 */
fb100e8da Hannes Reinecke   2017-06-23  2374goto 
fnic_device_reset_end;
fb100e8da Hannes Reinecke   2017-06-23  2375}
fb100e8da Hannes Reinecke   2017-06-23  2376CMD_FLAGS(sc) = 
FNIC_DEVICE_RESET;
5df6d737d Abhijeet Joglekar 2009-04-17  2377io_req = 
mempool_alloc(fnic->io_req_pool, GFP_ATOMIC);
5df6d737d Abhijeet Joglekar 2009-04-17  2378if (!io_req) {
5df6d737d Abhijeet Joglekar 2009-04-17  2379
spin_unlock_irqrestore(io_lock, flags);
5df6d737d Abhijeet Joglekar 2009-04-17  2380goto 
fnic_device_reset_end;
5df6d737d Abhijeet Joglekar 2009-04-17  2381}
5df6d737d Abhijeet Joglekar 2009-04-17  2382memset(io_req, 0, 
sizeof(*io_req));
5df6d737d Abhijeet Joglekar 2009-04-17  2383io_req->port_id = 
rport->port_id;
5df6d737d Abhijeet Joglekar 2009-04-17  2384CMD_SP(sc) = (char *)io_req;
5df6d737d Abhijeet Joglekar 2009-04-17  2385io_req->dr_done = _done;
fb100e8da Hannes Reinecke   2017-06-23  2386
5df6d737d Abhijeet Joglekar 2009-04-17  2387CMD_STATE(sc) = 
FNIC_IOREQ_CMD_PENDING;
5df6d737d Abhijeet Joglekar 2009-04-17  2388CMD_LR_STATUS(sc) = 
FCPIO_INVALID_CODE;
5df6d737d Abhijeet Joglekar 2009-04-17  2389spin_unlock_irqrestore(io_lock, 
flags);
5df6d737d Abhijeet Joglekar 2009-04-17  2390
03298552c Hiral Patel   2013-02-12  2391FNIC_SCSI_DBG(KERN_DEBUG, 
fnic->lport->host, "TAG %x\n", tag);
5df6d737d Abhijeet Joglekar 2009-04-17  2392
5df6d737d Abhijeet Joglekar 2009-04-17  2393/*
5df6d737d Abhijeet Joglekar 2009-04-17  2394 * issue the device reset, if 
enqueue failed, clean up the ioreq
5df6d737d Abhijeet Joglekar 2009-04-17  2395 * and break assoc with scsi cmd
5df6d737d Abhijeet Joglekar 2009-04-17  2396 */
5df6d737d Abhijeet Joglekar 2009-04-17  2397if (fnic_queue_dr_io_req(fnic, 
sc, io_req)) {
5df6d737d Abhijeet Joglekar 2009-04-17  2398
spin_lock_irqsave(io_lock, flags);
5df6d737d Abhijeet Joglekar 2009-04-17  2399io_req = (struct 
fnic_io_req *)CMD_SP(sc);
5df6d737d Abhijeet Joglekar 2009-04-17  2400if (io_req)
5df6d737d Abhijeet Joglekar 2009-04-17  2401io_req->dr_done 
= NULL;
5df6d737d Abhijeet Joglekar 2009-04-17  2402goto 
fnic_device_reset_clean;
5df6d737d Abhijeet Joglekar 2009-04-17  2403}
03298552c Hiral Patel   2013-02-12  2404spin_lock_irqsave(io_lock, 
flags);
14eb5d905 Hiral Patel   2013-02-12  2405CMD_FLAGS(sc) |= 
FNIC_DEV_RST_ISSUED;
03298552c Hiral Patel   2013-02-12  2406spin_unlock_irqrestore(io_lock, 
flags);
5df6d737d Abhijeet Joglekar 2009-04-17  2407
5df6d737d Abhijeet Joglekar 2009-04-17  2408/*
5df6d737d Abhijeet Joglekar 2009-04-17  2409 * Wait on the local completion 
for LUN reset.  The io_req may be
5df6d737d Abhijeet Joglekar 2009-04-17  2410 * freed while we wait since we 
hold no lock.
5df6d737d Abhijeet Joglekar 2009

Re: [PATCH] Eliminate extra 'out_free' label from fcoe_init function

[Julia Lawall]

On Fri, 2 Jun 2017, Milan P. Gandhi wrote:

> On 06/01/2017 08:32 PM, Dan Carpenter wrote:
> > On Thu, Jun 01, 2017 at 05:41:06PM +0530, Milan P. Gandhi wrote:
> >> Simplify the check for return code of fcoe_if_init routine
> >> in fcoe_init function such that we could eliminate need for
> >> extra 'out_free' label.
> >>
> >> Signed-off-by: Milan P. Gandhi 
> >> ---
> >>  drivers/scsi/fcoe/fcoe.c | 10 --
> >>  1 file changed, 4 insertions(+), 6 deletions(-)
> >>
> >> diff --git a/drivers/scsi/fcoe/fcoe.c b/drivers/scsi/fcoe/fcoe.c
> >> index ea21e7b..fb2a4c9 100644
> >> --- a/drivers/scsi/fcoe/fcoe.c
> >> +++ b/drivers/scsi/fcoe/fcoe.c
> >> @@ -2523,13 +2523,11 @@ static int __init fcoe_init(void)
> >>fcoe_dev_setup();
> >>
> >>rc = fcoe_if_init();
> >> -  if (rc)
> >> -  goto out_free;
> >> -
> >> -  mutex_unlock(_config_mutex);
> >> -  return 0;
> >> +  if (rc == 0) {
> >> +  mutex_unlock(_config_mutex);
> >> +  return 0;
> >> +  }
> >>
> >> -out_free:
> >>mutex_unlock(_config_mutex);
> >
> > Gar...  Stop!  No1  Don't do this.
> >
> > Do failure handling, not success handling.
> >
> > People always think they should get creative with the last if statement
> > in a function.  This leads to spaghetti code and it's confusing.  Please
> > never do this again.
> >
> > The original is correct and the new code is bad rubbish code.
> >
> > regards,
> > dan carpenter
> >
> >
>
> Oops, my bad sir. Will keep this in mind.

Still, does the mutex_unlock really need to be duplicated?

julia

>
> Thanks,
> Milan.
>
>
> Thanks,
> Milan.
> --
> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

Re: [PATCH RESEND] Eliminate extra 'out_free' label from fcoe_init function

[Julia Lawall]

On Fri, 2 Jun 2017, walter harms wrote:

>
>
> Am 02.06.2017 14:39, schrieb Milan P. Gandhi:
> > Simplify the check for return code of fcoe_if_init routine
> > in fcoe_init function such that we could eliminate need for
> > extra 'out_free' label and duplicate mutex_unlock statement.
> >
> > Signed-off-by: Milan P. Gandhi 
> > ---
> >  drivers/scsi/fcoe/fcoe.c | 7 +++
> >  1 file changed, 3 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/scsi/fcoe/fcoe.c b/drivers/scsi/fcoe/fcoe.c
> > index ea21e7b..a2cf3d0 100644
> > --- a/drivers/scsi/fcoe/fcoe.c
> > +++ b/drivers/scsi/fcoe/fcoe.c
> > @@ -2523,14 +2523,13 @@ static int __init fcoe_init(void)
> > fcoe_dev_setup();
> >
> > rc = fcoe_if_init();
> > +   mutex_unlock(_config_mutex);
> > +
> > if (rc)
> > -   goto out_free;
> > +   goto out_destroy;
> >
> > -   mutex_unlock(_config_mutex);
> > return 0;
> >
> if you do that, why not
> if (!rc) return 0;

I agree with Dan.  If's should be for failures.

julia

>
> re,
>  wh
>
>
>
> > -out_free:
> > -   mutex_unlock(_config_mutex);
> >  out_destroy:
> > destroy_workqueue(fcoe_wq);
> > return rc;
> > --
> > To unsubscribe from this list: send the line "unsubscribe kernel-janitors" 
> > in
> > the body of a message to majord...@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> >
> --
> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

Re: [PATCH RESEND] Eliminate extra 'out_free' label from fcoe_init function

[Julia Lawall]

On Fri, 2 Jun 2017, Milan P. Gandhi wrote:

> Simplify the check for return code of fcoe_if_init routine
> in fcoe_init function such that we could eliminate need for
> extra 'out_free' label and duplicate mutex_unlock statement.
>
> Signed-off-by: Milan P. Gandhi 
> ---
>  drivers/scsi/fcoe/fcoe.c | 7 +++
>  1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/scsi/fcoe/fcoe.c b/drivers/scsi/fcoe/fcoe.c
> index ea21e7b..a2cf3d0 100644
> --- a/drivers/scsi/fcoe/fcoe.c
> +++ b/drivers/scsi/fcoe/fcoe.c
> @@ -2523,14 +2523,13 @@ static int __init fcoe_init(void)
>   fcoe_dev_setup();
>
>   rc = fcoe_if_init();
> + mutex_unlock(_config_mutex);
> +
>   if (rc)
> - goto out_free;
> + goto out_destroy;
>
> - mutex_unlock(_config_mutex);

That's what I was thinking of, but it's not a RESEND, but rather a v2.
You need to explain under the --- what is the change since the original
submission.

julia

>   return 0;
>
> -out_free:
> - mutex_unlock(_config_mutex);
>  out_destroy:
>   destroy_workqueue(fcoe_wq);
>   return rc;
>

Re: [PATCH] Remove white spaces from fcoe.h

[Julia Lawall]

On Mon, 5 Jun 2017, Milan P. Gandhi wrote:

> Remove the white spaces in do/while loop used for checking fcoe
> debug logging

Is it converting spaces to tabs?  If so, saying that would be clearer.

julia

>
> Signed-off-by: Milan P. Gandhi 
> ---
>  drivers/scsi/fcoe/fcoe.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/scsi/fcoe/fcoe.h b/drivers/scsi/fcoe/fcoe.h
> index 6aa4820..7406376 100644
> --- a/drivers/scsi/fcoe/fcoe.h
> +++ b/drivers/scsi/fcoe/fcoe.h
> @@ -46,7 +46,7 @@ extern unsigned int fcoe_debug_logging;
>  #define FCOE_NETDEV_LOGGING 0x02 /* Netdevice logging */
>
>  #define FCOE_CHECK_LOGGING(LEVEL, CMD)   
> \
> -do { \
> +do { \
>   if (unlikely(fcoe_debug_logging & LEVEL))   \
>   do {\
>   CMD;\
> --
> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

Re: [PATCH v2 00/15] make structure field, function arguments and structures const

[Julia Lawall]

On Tue, 17 Oct 2017, Greg KH wrote:

> On Mon, Oct 16, 2017 at 05:18:39PM +0200, Bhumika Goyal wrote:
> > Make the ci_type field and some function arguments as const. After this
> > change, make config_item_type structures as const.
> >
> > * Changes in v2- Combine all the followup patches and the constification
> > patches into a series.
>
> Who do you want to take these patches?  If you want, I can take them
> through my driver-core tree, which has done other configfs stuff like
> this in the past.

Christoph Hellwig proposed to take care of it.

julia



>
> thanks,
>
> greg k-h
>

Re: [PATCH] scsi: Use vzalloc instead of vmalloc/memset

[Julia Lawall]

On Wed, 8 Nov 2017, Himanshu Jha wrote:

> On Tue, Nov 07, 2017 at 08:51:36PM +0100, Luis R. Rodriguez wrote:
> > On Sun, Nov 05, 2017 at 03:26:26AM +0530, Himanshu Jha wrote:
> > > Use vzalloc instead of vmalloc/memset to allocate memory filled with 0
> > > value.
> > >
> > > Done using Coccinelle.
> > > Semantic patch used :
> > >
> > > @@
> > > expression x,a;
> > > statement S;
> > > @@
> > >
> > > - x = vmalloc(a);
> > > + x = vzalloc(a);
> > >   if (x == NULL || ...) S
> > > - memset(x, 0, a);
> >
> > How many false positives do you get? Have you identified any?
> > If not you should consider adding this SmPL rule to:
> >
> > scripts/coccinelle/api/
> >
> > Some maintainers may ask you for the SmPL rule to be upstream first,
> > not all though. So its good practice for you to strive for this.
> > Another reason for it to go upstream is then other maintainers
> > can / should be running coccicheck against their subsystem to avoid
> > stupid regressions.
> >
> > You may want to explain for patches like these that they have been
> > tested by 0-day without any issues found.
> >
> > Also add the tag:
> >
> > Generated-by: Coccinelle SmPL
> >
> > > Signed-off-by: Himanshu Jha 
> > > ---
> > >  drivers/scsi/bfa/bfad.c| 3 +--
> > >  drivers/scsi/bfa/bfad_debugfs.c| 8 ++--
> > >  drivers/scsi/qla2xxx/qla_bsg.c | 3 +--
> > >  drivers/scsi/qla2xxx/tcm_qla2xxx.c | 5 +
> > >  drivers/scsi/scsi_debug.c  | 6 ++
> > >  drivers/scsi/snic/snic_trc.c   | 3 +--
> > >  6 files changed, 8 insertions(+), 20 deletions(-)
> >
> > Split this up per driver, and resend by using ./scripts/get_maintainer.pl
> > foo.patch and ensuring the right folks get the email.  Right now you
> > just spammed tons of people and the changes may be preferred to go
> > upstream atomically per driver, always assume this first.

Depending on the subsystem, you may get similar pushback if you send one
patch per file - "why send so many patches for such a small change when
they are all going through my tree".  So consider grouping the patches by
set of maintainers.

julia

> >
> > Other than this, feel free to add to each of the patches you created:
> >
>
> Thanks for the feeedback! I will resend the patch with the necessary
> changes.
>
> > Acked-by: Luis R. Rodriguez 
>
>
> Thanks
> Himanshu Jha
>

[target:for-next 17/33] drivers/target/target_core_user.c:891:2-8: preceding lock on line 791 (fwd)

[Julia Lawall]

Please check whether an unlock is needed before the return on line 891.

julia

-- Forwarded message --
Date: Wed, 8 Nov 2017 15:05:33 +0800
From: kbuild test robot <fengguang...@intel.com>
To: kbu...@01.org
Cc: Julia Lawall <julia.law...@lip6.fr>
Subject: [target:for-next 17/33] drivers/target/target_core_user.c:891:2-8:
preceding lock on line 791

CC: kbuild-...@01.org
CC: linux-r...@vger.kernel.org
CC: target-de...@vger.kernel.org
CC: linux-scsi@vger.kernel.org
CC: Nicholas Bellinger <n...@linux-iscsi.org>

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/nab/target-pending.git 
for-next
head:   3fc9fb13a4b2576aeab86c62fd64eb29ab68659c
commit: 0d44374c1aaec7c81b470d3b5f955bc270711f9c [17/33] tcmu: fix double 
se_cmd completion
:: branch date: 3 hours ago
:: commit date: 3 days ago

>> drivers/target/target_core_user.c:891:2-8: preceding lock on line 791
   drivers/target/target_core_user.c:891:2-8: preceding lock on line 823

# 
https://git.kernel.org/pub/scm/linux/kernel/git/nab/target-pending.git/commit/?id=0d44374c1aaec7c81b470d3b5f955bc270711f9c
git remote add target 
https://git.kernel.org/pub/scm/linux/kernel/git/nab/target-pending.git
git remote update target
git checkout 0d44374c1aaec7c81b470d3b5f955bc270711f9c
vim +891 drivers/target/target_core_user.c

0d44374c Mike Christie2017-10-25  757
02eb924f Andy Grover  2016-10-06  758  static sense_reason_t
02eb924f Andy Grover  2016-10-06  759  tcmu_queue_cmd_ring(struct tcmu_cmd 
*tcmu_cmd)
7c9e7a6f Andy Grover  2014-10-01  760  {
7c9e7a6f Andy Grover  2014-10-01  761   struct tcmu_dev *udev = 
tcmu_cmd->tcmu_dev;
7c9e7a6f Andy Grover  2014-10-01  762   struct se_cmd *se_cmd = 
tcmu_cmd->se_cmd;
7c9e7a6f Andy Grover  2014-10-01  763   size_t base_command_size, 
command_size;
7c9e7a6f Andy Grover  2014-10-01  764   struct tcmu_mailbox *mb;
7c9e7a6f Andy Grover  2014-10-01  765   struct tcmu_cmd_entry *entry;
7c9e7a6f Andy Grover  2014-10-01  766   struct iovec *iov;
141685a3 Xiubo Li 2017-05-02  767   int iov_cnt, ret;
7c9e7a6f Andy Grover  2014-10-01  768   uint32_t cmd_head;
7c9e7a6f Andy Grover  2014-10-01  769   uint64_t cdb_off;
f97ec7db Ilias Tsitsimpis 2015-04-23  770   bool copy_to_data_area;
ab22d260 Xiubo Li 2017-03-27  771   size_t data_length = 
tcmu_cmd_get_data_length(tcmu_cmd);
7c9e7a6f Andy Grover  2014-10-01  772
7c9e7a6f Andy Grover  2014-10-01  773   if 
(test_bit(TCMU_DEV_BIT_BROKEN, >flags))
02eb924f Andy Grover  2016-10-06  774   return 
TCM_LOGICAL_UNIT_COMMUNICATION_FAILURE;
7c9e7a6f Andy Grover  2014-10-01  775
7c9e7a6f Andy Grover  2014-10-01  776   /*
7c9e7a6f Andy Grover  2014-10-01  777* Must be a certain minimum 
size for response sense info, but
7c9e7a6f Andy Grover  2014-10-01  778* also may be larger if the 
iov array is large.
7c9e7a6f Andy Grover  2014-10-01  779*
fe25cc34 Xiubo Li 2017-05-02  780* We prepare as many iovs as 
possbile for potential uses here,
fe25cc34 Xiubo Li 2017-05-02  781* because it's expensive to 
tell how many regions are freed in
fe25cc34 Xiubo Li 2017-05-02  782* the bitmap & global data 
pool, as the size calculated here
fe25cc34 Xiubo Li 2017-05-02  783* will only be used to do the 
checks.
fe25cc34 Xiubo Li 2017-05-02  784*
fe25cc34 Xiubo Li 2017-05-02  785* The size will be 
recalculated later as actually needed to save
fe25cc34 Xiubo Li 2017-05-02  786* cmd area memories.
7c9e7a6f Andy Grover  2014-10-01  787*/
fe25cc34 Xiubo Li 2017-05-02  788   base_command_size = 
tcmu_cmd_get_base_cmd_size(tcmu_cmd->dbi_cnt);
fe25cc34 Xiubo Li 2017-05-02  789   command_size = 
tcmu_cmd_get_cmd_size(tcmu_cmd, base_command_size);
7c9e7a6f Andy Grover  2014-10-01  790
b6df4b79 Xiubo Li 2017-05-02 @791   mutex_lock(>cmdr_lock);
7c9e7a6f Andy Grover  2014-10-01  792
7c9e7a6f Andy Grover  2014-10-01  793   mb = udev->mb_addr;
7c9e7a6f Andy Grover  2014-10-01  794   cmd_head = mb->cmd_head % 
udev->cmdr_size; /* UAM */
554617b2 Andy Grover  2016-08-25  795   if ((command_size > 
(udev->cmdr_size / 2)) ||
554617b2 Andy Grover  2016-08-25  796   data_length > 
udev->data_size) {
554617b2 Andy Grover  2016-08-25  797   pr_warn("TCMU: Request 
of size %zu/%zu is too big for %u/%zu "
3d9b9555 Andy Grover  2016-08-25  798   "cmd ring/data 
area\n", command_size, data_length,
7c9e7a6f Andy Grover  2014-10-01  799   
udev->cmdr_size, udev->data_size);
b6df4b79 Xiubo Li 2017-05-02  800   
mutex_unlock(>cmdr_lock);
554617b