public inbox for linux-arm-kernel@lists.infradead.org
 help / color / mirror / Atom feed
From: mark.rutland@arm.com (Mark Rutland)
To: linux-arm-kernel@lists.infradead.org
Subject: [bug report] perf: xgene: Add APM X-Gene SoC Performance Monitoring Unit driver
Date: Wed, 12 Oct 2016 16:23:26 +0100	[thread overview]
Message-ID: <20161012152326.GB21489@remoulade> (raw)
In-Reply-To: <20161012113229.GA16447@mwanda>

Hi Dan,

Thanks for the report.

On Wed, Oct 12, 2016 at 02:32:29PM +0300, Dan Carpenter wrote:
> The patch 832c927d119b: "perf: xgene: Add APM X-Gene SoC Performance
> Monitoring Unit driver" from Jul 15, 2016, leads to the following
> static checker warning:
> 
> 	drivers/perf/xgene_pmu.c:1014 acpi_get_pmu_hw_inf()
> 	warn: '&res' isn't an ERR_PTR
> 
> drivers/perf/xgene_pmu.c
>    992  static struct
>    993  xgene_pmu_dev_ctx *acpi_get_pmu_hw_inf(struct xgene_pmu *xgene_pmu,
>    994                                         struct acpi_device *adev, u32 type)
>    995  {
>    996          struct device *dev = xgene_pmu->dev;
>    997          struct list_head resource_list;
>    998          struct xgene_pmu_dev_ctx *ctx;
>    999          const union acpi_object *obj;
>   1000          struct hw_pmu_info *inf;
>   1001          void __iomem *dev_csr;
>   1002          struct resource res;
>   1003          int enable_bit;
>   1004          int rc;
>   1005  
>   1006          ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL);
>   1007          if (!ctx)
>   1008                  return NULL;
>   1009  
>   1010          INIT_LIST_HEAD(&resource_list);
>   1011          rc = acpi_dev_get_resources(adev, &resource_list,
>   1012                                      acpi_pmu_dev_add_resource, &res);
>   1013          acpi_dev_free_resource_list(&resource_list);
>   1014          if (rc < 0 || IS_ERR(&res)) {
>                                      ^^^^
> Obviously this is a stack address and not an error pointer.  I'm not
> sure what was intended here.

Urrgh; I should have caught that in review. It's been this way since v1 [1], so
I can't reverse-engineer the intent. However, I don't believe that it's
necessary to check anything other than the rc value here.

This should always evaluate as false, and shouldn't result in a functional
problem, but it is completely bogus.

Tai, can you plase send a patch deleting The IS_ERR() check here?

Thanks,
Mark.

[1] http://lists.infradead.org/pipermail/linux-arm-kernel/2016-March/419173.html

  parent reply	other threads:[~2016-10-12 15:23 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-10-12 11:32 [bug report] perf: xgene: Add APM X-Gene SoC Performance Monitoring Unit driver Dan Carpenter
2016-10-12 14:55 ` Mason
2016-10-12 15:23 ` Mark Rutland [this message]
2016-10-12 15:28   ` Tai Tri Nguyen
  -- strict thread matches above, loose matches on Subject: below --
2017-07-11 11:43 Dan Carpenter
2017-07-11 22:48 ` Tai Tri Nguyen
2017-07-12  9:34   ` Mark Rutland

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20161012152326.GB21489@remoulade \
    --to=mark.rutland@arm.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox