Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1519899

RE: [PATCH 1/2] Staging: fsl-mc: include: mc: Kernel type 's16' preferred over 'int16_t'

Show all headers | View raw

> -----Original Message-----
> From: Dan Carpenter [mailto:dan.carpenter@oracle.com]
> Sent: Friday, November 11, 2016 5:23 AM
> To: Shiva Kerdel <shiva@exdev.nl>
> Cc: Stuart Yoder <stuart.yoder@nxp.com>; devel@driverdev.osuosl.org; German.Rivera@freescale.com;
> gregkh@linuxfoundation.org; Nipun Gupta <nipun.gupta@nxp.com>; linux-kernel@vger.kernel.org; German
> Rivera <german.rivera@nxp.com>; treding@nvidia.com; itai.katz@nxp.com
> Subject: Re: [PATCH 1/2] Staging: fsl-mc: include: mc: Kernel type 's16' preferred over 'int16_t'
> 
> On Fri, Nov 11, 2016 at 12:07:39PM +0100, Shiva Kerdel wrote:
> > Follow the kernel type preferrences of using 's16' over 'int16_t'.
> >
> > Signed-off-by: Shiva Kerdel <shiva@exdev.nl>
> > Acked-by: Stuart Yoder <stuart.yoder@nxp.com>
> > ---
> > Changes for v2:
> >     - corrected an error in the log message, wrote 's32' instead of 's16'.
> > Changes for v3:
> >     - added the missing annotates.
> > Changes for v4:
> >     - corrected patch subject to version 4.
> >
> >  drivers/staging/fsl-mc/include/mc-bus.h | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/staging/fsl-mc/include/mc-bus.h b/drivers/staging/fsl-mc/include/mc-bus.h
> > index e915574..c7cad87 100644
> > --- a/drivers/staging/fsl-mc/include/mc-bus.h
> > +++ b/drivers/staging/fsl-mc/include/mc-bus.h
> > @@ -42,8 +42,8 @@ struct msi_domain_info;
> >   */
> >  struct fsl_mc_resource_pool {
> >  	enum fsl_mc_pool_type type;
> > -	int16_t max_count;
> > -	int16_t free_count;
> > +	s16 max_count;
> 
> My understanding is that this has to be signed because the design of
> this driver is that we keep adding devices until the the counter
> overflows.  After that there are a couple tests for
> "if (WARN_ON(res_pool->max_count < 0)) " which prevent the driver from
> working again.
>
> This all seems pretty horrible.

Can you elaborate?

The resource pools managed by this driver are populated by hardware objects
discovered when the fsl-mc bus probes a DPRC/container.

The number of potential objects discovered of a given type is in the hundreds,
so a signed 16-bit number is order of magnitudes larger than anything we will
ever encounter.

Would you feel better about this if max_count was an int?

The max_count reflects the total number of objects discovered.  If that is
exceeded we display a warning, because something is horribly wrong.  Nothing
stops working, the allocator simply refuses to add anything else to the
free list.

The only reason max_count is there at all is as an internal check against
bugs and resource leaks.  If the driver is being removed and a resource
pool is being freed, max_count must be zero...i.e. all objects should have
been removed.  If not, there is a leak somewhere.  So, it's a sanity check.

Thanks,
Stuart

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

[PATCH 1/2] Staging: fsl-mc: include: mc: Kernel type 's16' preferred over 'int16_t' Shiva Kerdel <shiva@exdev.nl> - 2016-11-11 12:10 +0100
  Re: [PATCH 1/2] Staging: fsl-mc: include: mc: Kernel type 's16'  preferred over 'int16_t' Dan Carpenter <dan.carpenter@oracle.com> - 2016-11-11 12:30 +0100
    RE: [PATCH 1/2] Staging: fsl-mc: include: mc: Kernel type 's16'  preferred over 'int16_t' Stuart Yoder <stuart.yoder@nxp.com> - 2016-11-11 17:30 +0100
      Re: [PATCH 1/2] Staging: fsl-mc: include: mc: Kernel type 's16'  preferred over 'int16_t' Dan Carpenter <dan.carpenter@oracle.com> - 2016-11-14 11:20 +0100
        RE: [PATCH 1/2] Staging: fsl-mc: include: mc: Kernel type 's16'  preferred over 'int16_t' Stuart Yoder <stuart.yoder@nxp.com> - 2016-11-14 16:20 +0100
          Re: [PATCH 1/2] Staging: fsl-mc: include: mc: Kernel type 's16'  preferred over 'int16_t' Shiva Kerdel <shiva@exdev.nl> - 2016-11-15 09:00 +0100

csiph-web