[lustre-devel] [bug report] staging: lustre: clio: Revise read ahead implementation

[lustre-devel] [bug report] staging: lustre: clio: Revise read ahead implementation

[Dan Carpenter]

Hello Jinshan Xiong,

The patch 1e1db2a97be5: "staging: lustre: clio: Revise read ahead
implementation" from Oct 2, 2016, leads to the following static
checker warning:

	drivers/staging/lustre/lustre/lov/lov_io.c:611 lov_io_read_ahead()
	error: 'sub' dereferencing possible ERR_PTR()

drivers/staging/lustre/lustre/lov/lov_io.c
   589  static int lov_io_read_ahead(const struct lu_env *env,
   590                               const struct cl_io_slice *ios,
   591                               pgoff_t start, struct cl_read_ahead *ra)
   592  {
   593          struct lov_io *lio = cl2lov_io(env, ios);
   594          struct lov_object *loo = lio->lis_object;
   595          struct cl_object *obj = lov2cl(loo);
   596          struct lov_layout_raid0 *r0 = lov_r0(loo);
   597          unsigned int pps; /* pages per stripe */
   598          struct lov_io_sub *sub;
   599          pgoff_t ra_end;
   600          loff_t suboff;
   601          int stripe;
   602          int rc;
   603  
   604          stripe = lov_stripe_number(loo->lo_lsm, cl_offset(obj, start));
   605          if (unlikely(!r0->lo_sub[stripe]))
   606                  return -EIO;
   607  
   608          sub = lov_sub_get(env, lio, stripe);
                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Do we need error handling here?

   609  
   610          lov_stripe_offset(loo->lo_lsm, cl_offset(obj, start), stripe, &suboff);
   611          rc = cl_io_read_ahead(sub->sub_env, sub->sub_io,
   612                                cl_index(lovsub2cl(r0->lo_sub[stripe]), suboff),
   613                                ra);
   614          lov_sub_put(sub);
   615  

regards,
dan carpenter

[lustre-devel] [bug report] staging: lustre: clio: Revise read ahead implementation

[Giedrius Statkevičius]

On Thursday, October 20, 2016, Dan Carpenter <dan.carpenter@oracle.com>
wrote:
> Hello Jinshan Xiong,
>
> The patch 1e1db2a97be5: "staging: lustre: clio: Revise read ahead
> implementation" from Oct 2, 2016, leads to the following static
> checker warning:
>
>         drivers/staging/lustre/lustre/lov/lov_io.c:611 lov_io_read_ahead()
>         error: 'sub' dereferencing possible ERR_PTR()
>
> drivers/staging/lustre/lustre/lov/lov_io.c
>    589  static int lov_io_read_ahead(const struct lu_env *env,
>    590                               const struct cl_io_slice *ios,
>    591                               pgoff_t start, struct cl_read_ahead
*ra)
>    592  {
>    593          struct lov_io *lio = cl2lov_io(env, ios);
>    594          struct lov_object *loo = lio->lis_object;
>    595          struct cl_object *obj = lov2cl(loo);
>    596          struct lov_layout_raid0 *r0 = lov_r0(loo);
>    597          unsigned int pps; /* pages per stripe */
>    598          struct lov_io_sub *sub;
>    599          pgoff_t ra_end;
>    600          loff_t suboff;
>    601          int stripe;
>    602          int rc;
>    603
>    604          stripe = lov_stripe_number(loo->lo_lsm, cl_offset(obj,
start));
>    605          if (unlikely(!r0->lo_sub[stripe]))
>    606                  return -EIO;
>    607
>    608          sub = lov_sub_get(env, lio, stripe);
>                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Hey Dan,
May I ask what static checker are you using?
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.lustre.org/pipermail/lustre-devel-lustre.org/attachments/20161020/c759927a/attachment-0001.htm>

[lustre-devel] [bug report] staging: lustre: clio: Revise read ahead implementation

[Xiong, Jinshan]

> On Oct 20, 2016, at 7:17 PM, Dan Carpenter <dan.carpenter@oracle.com> wrote:
> 
> Hello Jinshan Xiong,
> 
> The patch 1e1db2a97be5: "staging: lustre: clio: Revise read ahead
> implementation" from Oct 2, 2016, leads to the following static
> checker warning:
> 
> 	drivers/staging/lustre/lustre/lov/lov_io.c:611 lov_io_read_ahead()
> 	error: 'sub' dereferencing possible ERR_PTR()
> 
> drivers/staging/lustre/lustre/lov/lov_io.c
>   589  static int lov_io_read_ahead(const struct lu_env *env,
>   590                               const struct cl_io_slice *ios,
>   591                               pgoff_t start, struct cl_read_ahead *ra)
>   592  {
>   593          struct lov_io *lio = cl2lov_io(env, ios);
>   594          struct lov_object *loo = lio->lis_object;
>   595          struct cl_object *obj = lov2cl(loo);
>   596          struct lov_layout_raid0 *r0 = lov_r0(loo);
>   597          unsigned int pps; /* pages per stripe */
>   598          struct lov_io_sub *sub;
>   599          pgoff_t ra_end;
>   600          loff_t suboff;
>   601          int stripe;
>   602          int rc;
>   603  
>   604          stripe = lov_stripe_number(loo->lo_lsm, cl_offset(obj, start));
>   605          if (unlikely(!r0->lo_sub[stripe]))
>   606                  return -EIO;
>   607  
>   608          sub = lov_sub_get(env, lio, stripe);
>                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> Do we need error handling here?

Indeed, this needs error handling. Thanks for pointing it out. I will push a patch soon.

Jinshan

> 
>   609  
>   610          lov_stripe_offset(loo->lo_lsm, cl_offset(obj, start), stripe, &suboff);
>   611          rc = cl_io_read_ahead(sub->sub_env, sub->sub_io,
>   612                                cl_index(lovsub2cl(r0->lo_sub[stripe]), suboff),
>   613                                ra);
>   614          lov_sub_put(sub);
>   615  
> 
> regards,
> dan carpenter

[lustre-devel] [bug report] staging: lustre: clio: Revise read ahead implementation

[Dan Carpenter]

This is Smatch stuff.  Some of it requires that the database be built
and some is stuff I'm still working on and haven't released yet.

regards,
dan carpenter