> -----Original Message----- > From: Dan Carpenter [mailto:dan.carpenter@xxxxxxxxxx] > Sent: Friday, November 11, 2016 5:23 AM > To: Shiva Kerdel <shiva@xxxxxxxx> > Cc: Stuart Yoder <stuart.yoder@xxxxxxx>; devel@xxxxxxxxxxxxxxxxxxxx; German.Rivera@xxxxxxxxxxxxx; > gregkh@xxxxxxxxxxxxxxxxxxx; Nipun Gupta <nipun.gupta@xxxxxxx>; linux-kernel@xxxxxxxxxxxxxxx; German > Rivera <german.rivera@xxxxxxx>; treding@xxxxxxxxxx; itai.katz@xxxxxxx > 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@xxxxxxxx> > > Acked-by: Stuart Yoder <stuart.yoder@xxxxxxx> > > --- > > 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 _______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
