[Cluster-devel] GFS2: Combine functions gfs2_glock_wait and wait_on_holder

[Cluster-devel] GFS2: Combine functions gfs2_glock_wait and wait_on_holder

[Dan Carpenter]

Hello Bob Peterson,

This is probably a false positive but I thought it would be
interesting to ask.

The patch 07a790494260: "GFS2: Combine functions gfs2_glock_wait and 
wait_on_holder" from Aug 9, 2012, leads to the following warning on
my not yet pushed version of Smatch:

"fs/gfs2/inode.c:217 gfs2_lookup_by_inum()
	 error: passing non neg 13 to ERR_PTR"

   884  /**
   885   * gfs2_glock_wait - wait on a glock acquisition
   886   * @gh: the glock holder
   887   *
   888   * Returns: 0 on success
   889   */
   890  
   891  int gfs2_glock_wait(struct gfs2_holder *gh)
   892  {
   893          unsigned long time1 = jiffies;
   894  
   895          might_sleep();
   896          wait_on_bit(&gh->gh_iflags, HIF_WAIT, gfs2_glock_holder_wait, TASK_UNINTERRUPTIBLE);
   897          if (time_after(jiffies, time1 + HZ)) /* have we waited > a second? */
   898                  /* Lengthen the minimum hold time. */
   899                  gh->gh_gl->gl_hold_time = min(gh->gh_gl->gl_hold_time +
   900                                                GL_GLOCK_HOLD_INCR,
   901                                                GL_GLOCK_MAX_HOLD);
   902          return gh->gh_error;
                       ^^^^^^^^^^^^
Can this be GLR_TRYFAILED at this point?  If it is then it would
cause a problem later on.

   903  }

regards,
dan carpenter

[Cluster-devel] GFS2: Combine functions gfs2_glock_wait and wait_on_holder

[Steven Whitehouse]

Hi,

That is a false positive, because gfs2_lookup_by_inum does not call the
gfs2_glock_nq_num() function with arguments which specify a try lock.
There are few uses of the try lock left in GFS2 and over time we are
eliminating them gradually,

Steve.

On Tue, 2013-02-26 at 09:13 +0300, Dan Carpenter wrote:
> Hello Bob Peterson,
> 
> This is probably a false positive but I thought it would be
> interesting to ask.
> 
> The patch 07a790494260: "GFS2: Combine functions gfs2_glock_wait and 
> wait_on_holder" from Aug 9, 2012, leads to the following warning on
> my not yet pushed version of Smatch:
> 
> "fs/gfs2/inode.c:217 gfs2_lookup_by_inum()
> 	 error: passing non neg 13 to ERR_PTR"
> 
>    884  /**
>    885   * gfs2_glock_wait - wait on a glock acquisition
>    886   * @gh: the glock holder
>    887   *
>    888   * Returns: 0 on success
>    889   */
>    890  
>    891  int gfs2_glock_wait(struct gfs2_holder *gh)
>    892  {
>    893          unsigned long time1 = jiffies;
>    894  
>    895          might_sleep();
>    896          wait_on_bit(&gh->gh_iflags, HIF_WAIT, gfs2_glock_holder_wait, TASK_UNINTERRUPTIBLE);
>    897          if (time_after(jiffies, time1 + HZ)) /* have we waited > a second? */
>    898                  /* Lengthen the minimum hold time. */
>    899                  gh->gh_gl->gl_hold_time = min(gh->gh_gl->gl_hold_time +
>    900                                                GL_GLOCK_HOLD_INCR,
>    901                                                GL_GLOCK_MAX_HOLD);
>    902          return gh->gh_error;
>                        ^^^^^^^^^^^^
> Can this be GLR_TRYFAILED at this point?  If it is then it would
> cause a problem later on.
> 
>    903  }
> 
> regards,
> dan carpenter
>