On Thu, 2014-11-27 at 01:11 +0900, Akinobu Mita wrote:
> 2014-11-27 0:51 GMT+09:00 James Bottomley
> <James.Bottomley@xxxxxxxxxxxxxxxxxxxxx>:
> > On Thu, 2014-11-27 at 00:37 +0900, Akinobu Mita wrote:
> >> In ufshcd_parse_clock_info(), temporary memory allocated by
> >> devm_kzalloc() is released by kfree(). The devres mechanism cannot
> >> track the release and it causes trouble when removing the driver.
> >>
> >> Fix it by allocating it with plain kcalloc() instead of devm_kzalloc()
> >> as the temporary memory is always freed in that function.
> >>
> >> Signed-off-by: Akinobu Mita <mita@xxxxxxxxxxxx>
> >> Cc: Vinayak Holikatti <vinholikatti@xxxxxxxxx>
> >> Cc: Dolev Raviv <draviv@xxxxxxxxxxxxxx>
> >> Cc: Subhash Jadavani <subhashj@xxxxxxxxxxxxxx>
> >> Cc: Yaniv Gardi <ygardi@xxxxxxxxxxxxxx>
> >> Cc: Sujit Reddy Thumma <sthumma@xxxxxxxxxxxxxx>
> >> Cc: Maya Erez <merez@xxxxxxxxxxxxxx>
> >> Cc: Sahitya Tummala <stummala@xxxxxxxxxxxxxx>
> >> Cc: Christoph Hellwig <hch@xxxxxx>
> >> Cc: "James E.J. Bottomley" <JBottomley@xxxxxxxxxxxxx>
> >> Cc: linux-scsi@xxxxxxxxxxxxxxx
> >> ---
> >> drivers/scsi/ufs/ufshcd-pltfrm.c | 3 +--
> >> 1 file changed, 1 insertion(+), 2 deletions(-)
> >>
> >> diff --git a/drivers/scsi/ufs/ufshcd-pltfrm.c b/drivers/scsi/ufs/ufshcd-pltfrm.c
> >> index 0c030ad..67b1f6c 100644
> >> --- a/drivers/scsi/ufs/ufshcd-pltfrm.c
> >> +++ b/drivers/scsi/ufs/ufshcd-pltfrm.c
> >> @@ -99,8 +99,7 @@ static int ufshcd_parse_clock_info(struct ufs_hba *hba)
> >> goto out;
> >> }
> >>
> >> - clkfreq = devm_kzalloc(dev, sz * sizeof(*clkfreq),
> >> - GFP_KERNEL);
> >> + clkfreq = kcalloc(sz, sizeof(*clkfreq), GFP_KERNEL);
> >
> > I agree this doesn't need to be devm memory since it's copied into clki,
> > but where is it freed? I think you need a kfree(clkfreq) at the bottom
> > of the routine.
>
> Oops. I was working on the older code, and blindly applied the -next just
> before sending this patch. The problem I said in this commit log has been
> fixed by the commit e8cb64db81e8c88c5c824ca74c2e57b4c6919ca6 ("scsi: ufs:
> fix static checker warning in ufshcd_parse_clock_info").
>
> So I withdraw this patch for now. I will try with another title next time.
I don't necessarily disagree with the patch if you correct it. In the
current state, clkfreq is only used as a temporary in the routine as you
say, but it persists the entire lifetime of the driver ... it's a tiny
amount of memory and the devm_ allocation ensures it eventually gets
cleaned up, so it's probably not that important in the grand scheme of
things.
James
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
