Re:[PATCH v3] drm/panfrost: fix -Wmissing-prototypes warnings
Hi Steve,
Thanks a lot for your time and patience :)
> On 25/10/2019 02:30, Yi Wang wrote:
> > We get these warnings when build kernel W=1:
> > drivers/gpu/drm/panfrost/panfrost_perfcnt.c:35:6: warning: no previous
> > prototype for ‘panfrost_perfcnt_clean_cache_done’ [-Wmissing-prototypes]
> > drivers/gpu/drm/panfrost/panfrost_perfcnt.c:40:6: warning: no previous
> > prototype for ‘panfrost_perfcnt_sample_done’ [-Wmissing-prototypes]
> > drivers/gpu/drm/panfrost/panfrost_perfcnt.c:190:5: warning: no previous
> > prototype for ‘panfrost_ioctl_perfcnt_enable’ [-Wmissing-prototypes]
> > drivers/gpu/drm/panfrost/panfrost_perfcnt.c:218:5: warning: no previous
> > prototype for ‘panfrost_ioctl_perfcnt_dump’ [-Wmissing-prototypes]
> > drivers/gpu/drm/panfrost/panfrost_perfcnt.c:250:6: warning: no previous
> > prototype for ‘panfrost_perfcnt_close’ [-Wmissing-prototypes]
> > drivers/gpu/drm/panfrost/panfrost_perfcnt.c:264:5: warning: no previous
> > prototype for ‘panfrost_perfcnt_init’ [-Wmissing-prototypes]
> > drivers/gpu/drm/panfrost/panfrost_perfcnt.c:320:6: warning: no previous
> > prototype for ‘panfrost_perfcnt_fini’ [-Wmissing-prototypes]
> > drivers/gpu/drm/panfrost/panfrost_mmu.c:227:6: warning: no previous
> > prototype for ‘panfrost_mmu_flush_range’ [-Wmissing-prototypes]
> > drivers/gpu/drm/panfrost/panfrost_mmu.c:435:5: warning: no previous
> > prototype for ‘panfrost_mmu_map_fault_addr’ [-Wmissing-prototypes]
> >
> > For file panfrost_mmu.c, make functions static to fix this.
> > For file panfrost_perfcnt.c, include header file can fix this.
> >
> > Signed-off-by: Yi Wang
> > Reviewed-by: Steven Price
> > ---
> >
> > v3: using tab size of 8 other than 4.
> >
> > v2: align parameter line and modify comment. Thanks to Steve.
> > ---
> > drivers/gpu/drm/panfrost/panfrost_mmu.c | 9 +
> > drivers/gpu/drm/panfrost/panfrost_perfcnt.c | 1 +
> > 2 files changed, 6 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/panfrost/panfrost_mmu.c
> > b/drivers/gpu/drm/panfrost/panfrost_mmu.c
> > index bdd9905..871574c 100644
> > --- a/drivers/gpu/drm/panfrost/panfrost_mmu.c
> > +++ b/drivers/gpu/drm/panfrost/panfrost_mmu.c
> > @@ -224,9 +224,9 @@ static size_t get_pgsize(u64 addr, size_t size)
> > return SZ_2M;
> > }
> >
> > -void panfrost_mmu_flush_range(struct panfrost_device *pfdev,
> > - struct panfrost_mmu *mmu,
> > - u64 iova, size_t size)
> > +static void panfrost_mmu_flush_range(struct panfrost_device *pfdev,
> > +struct panfrost_mmu *mmu,
> > +u64 iova, size_t size)
>
> Ok, I'll admit I wouldn't have spotted this unless I'd double checked by
> applying the patch, but you still seem to have something misconfigured
> in your editor. This is out by one character:
>
> static void panfrost_mmu_flush_range(struct panfrost_device *pfdev,
> >--->--->--->---struct panfrost_mmu *mmu,
> >--->--->--->---u64 iova, size_t size)
>
> There should be an extra space to align correctly.
According to [Linux kernel coding
style](https://www.kernel.org/doc/html/v4.10/process/coding-style.html):
> Outside of comments, documentation and except in Kconfig, spaces are
> never used for indentation, and the above example is deliberately broken.
If I understand corretly, the tab is enough for indentation :-)
>
> > {
> > if (mmu->as < 0)
> > return;
> > @@ -432,7 +432,8 @@ void panfrost_mmu_pgtable_free(struct
> > panfrost_file_priv *priv)
> >
> > #define NUM_FAULT_PAGES (SZ_2M / PAGE_SIZE)
> >
> > -int panfrost_mmu_map_fault_addr(struct panfrost_device *pfdev, int as, u64
> > addr)
> > +static int panfrost_mmu_map_fault_addr(struct panfrost_device *pfdev, int
> > as,
> > +u64 addr)
>
> Here you're off-by-one in the other direction - you need to replace the
> final tab with 7 spaces:
>
> static int panfrost_mmu_map_fault_addr(struct panfrost_device *pfdev, int as,
> >--->--->--->--->---u64 addr)
>
> Sorry to nit-pick over this, but it's good to get your editor setup
> correctly to ensure your formatting is correct.
Yeah, it worth time on the editor setup, and thanks again.
>
> Thanks,
>
> Steve
---
Best wishes
Yi Wang___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re:[PATCH v3] drm/panfrost: fix -Wmissing-prototypes warnings
> On Mon, Oct 28, 2019 at 09:27:33AM +, wang.y...@zte.com.cn wrote:
> > Hi Steve,
> >
> > Thanks a lot for your time and patience :)
> >
> > > On 25/10/2019 02:30, Yi Wang wrote:
> > > > We get these warnings when build kernel W=1:
> > > > drivers/gpu/drm/panfrost/panfrost_perfcnt.c:35:6: warning: no previous
> > > > prototype for ‘panfrost_perfcnt_clean_cache_done’ [-Wmissing-prototypes]
> > > > drivers/gpu/drm/panfrost/panfrost_perfcnt.c:40:6: warning: no previous
> > > > prototype for ‘panfrost_perfcnt_sample_done’ [-Wmissing-prototypes]
> > > > drivers/gpu/drm/panfrost/panfrost_perfcnt.c:190:5: warning: no previous
> > > > prototype for ‘panfrost_ioctl_perfcnt_enable’ [-Wmissing-prototypes]
> > > > drivers/gpu/drm/panfrost/panfrost_perfcnt.c:218:5: warning: no previous
> > > > prototype for ‘panfrost_ioctl_perfcnt_dump’ [-Wmissing-prototypes]
> > > > drivers/gpu/drm/panfrost/panfrost_perfcnt.c:250:6: warning: no previous
> > > > prototype for ‘panfrost_perfcnt_close’ [-Wmissing-prototypes]
> > > > drivers/gpu/drm/panfrost/panfrost_perfcnt.c:264:5: warning: no previous
> > > > prototype for ‘panfrost_perfcnt_init’ [-Wmissing-prototypes]
> > > > drivers/gpu/drm/panfrost/panfrost_perfcnt.c:320:6: warning: no previous
> > > > prototype for ‘panfrost_perfcnt_fini’ [-Wmissing-prototypes]
> > > > drivers/gpu/drm/panfrost/panfrost_mmu.c:227:6: warning: no previous
> > > > prototype for ‘panfrost_mmu_flush_range’ [-Wmissing-prototypes]
> > > > drivers/gpu/drm/panfrost/panfrost_mmu.c:435:5: warning: no previous
> > > > prototype for ‘panfrost_mmu_map_fault_addr’ [-Wmissing-prototypes]
> > > >
> > > > For file panfrost_mmu.c, make functions static to fix this.
> > > > For file panfrost_perfcnt.c, include header file can fix this.
> > > >
> > > > Signed-off-by: Yi Wang
> > > > Reviewed-by: Steven Price
> > > > ---
> > > >
> > > > v3: using tab size of 8 other than 4.
> > > >
> > > > v2: align parameter line and modify comment. Thanks to Steve.
> > > > ---
> > > > drivers/gpu/drm/panfrost/panfrost_mmu.c | 9 +
> > > > drivers/gpu/drm/panfrost/panfrost_perfcnt.c | 1 +
> > > > 2 files changed, 6 insertions(+), 4 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/panfrost/panfrost_mmu.c
> > > > b/drivers/gpu/drm/panfrost/panfrost_mmu.c
> > > > index bdd9905..871574c 100644
> > > > --- a/drivers/gpu/drm/panfrost/panfrost_mmu.c
> > > > +++ b/drivers/gpu/drm/panfrost/panfrost_mmu.c
> > > > @@ -224,9 +224,9 @@ static size_t get_pgsize(u64 addr, size_t size)
> > > > return SZ_2M;
> > > > }
> > > >
> > > > -void panfrost_mmu_flush_range(struct panfrost_device *pfdev,
> > > > - struct panfrost_mmu *mmu,
> > > > - u64 iova, size_t size)
> > > > +static void panfrost_mmu_flush_range(struct panfrost_device *pfdev,
> > > > +struct panfrost_mmu *mmu,
> > > > +u64 iova, size_t size)
> > >
> > > Ok, I'll admit I wouldn't have spotted this unless I'd double checked by
> > > applying the patch, but you still seem to have something misconfigured
> > > in your editor. This is out by one character:
> > >
> > > static void panfrost_mmu_flush_range(struct panfrost_device *pfdev,
> > > >--->--->--->---struct panfrost_mmu *mmu,
> > > >--->--->--->---u64 iova, size_t size)
> > >
> > > There should be an extra space to align correctly.
> >
> > According to [Linux kernel coding
> > style](https://www.kernel.org/doc/html/v4.10/process/coding-style.html):
> > > Outside of comments, documentation and except in Kconfig, spaces are
> > > never used for indentation, and the above example is deliberately broken.
> >
> > If I understand corretly, the tab is enough for indentation :-)
>
> There's a (subtle) difference between indentation and alignment.
> Indentation is where the code is moved over because it is contained
> within an outer statement (e.g. an 'if' or 'while'). The Linux coding
> style is that this should be done with only tabs as you quote.
Ok, I got it.
>
> However when the line is a continuation of the previous line, like in
> the example here, then we often need a combination of tabs/spaces to
> align correctly. The desire here is that the continuation lines all
> start straight after the "(" on the first line. Since that is 37
> characters from the start we need 4 tabs + 5 spaces (4*8+5=37), whereas
> you had 4 tabs + 4 spaces. My guess is this is because you've got the
> following vim configuration:
>
> tabstop = 8
> softtabstop = 4
> noexpandtab
>
> This means that pressing moves you along only 4 columns (i.e.
> inserts 4 spaces), a second will then replace the spaces with a
> real tab character (i.e. move another 4 columns for a total of 8).
>
> It's probably best to set "softtabstop = 0" so that always inserts
> a real tab character. You might also find it useful to make tabs
> visible:
>
> set list
> set listchars=tab:>-
>
> That way you can easily see where you h
Re:[PATCH] drm/panfrost: fix -Wmissing-prototypes warnings
Hi Steve, Thanks a lot for your review, and I will send a v2 patch ASAP. > On 19/10/2019 08:28, Yi Wang wrote: > > We get these warnings when build kernel W=1: > > drivers/gpu/drm/panfrost/panfrost_perfcnt.c:35:6: warning: no previous > > prototype for ‘panfrost_perfcnt_clean_cache_done’ [-Wmissing-prototypes] > > drivers/gpu/drm/panfrost/panfrost_perfcnt.c:40:6: warning: no previous > > prototype for ‘panfrost_perfcnt_sample_done’ [-Wmissing-prototypes] > > drivers/gpu/drm/panfrost/panfrost_perfcnt.c:190:5: warning: no previous > > prototype for ‘panfrost_ioctl_perfcnt_enable’ [-Wmissing-prototypes] > > drivers/gpu/drm/panfrost/panfrost_perfcnt.c:218:5: warning: no previous > > prototype for ‘panfrost_ioctl_perfcnt_dump’ [-Wmissing-prototypes] > > drivers/gpu/drm/panfrost/panfrost_perfcnt.c:250:6: warning: no previous > > prototype for ‘panfrost_perfcnt_close’ [-Wmissing-prototypes] > > drivers/gpu/drm/panfrost/panfrost_perfcnt.c:264:5: warning: no previous > > prototype for ‘panfrost_perfcnt_init’ [-Wmissing-prototypes] > > drivers/gpu/drm/panfrost/panfrost_perfcnt.c:320:6: warning: no previous > > prototype for ‘panfrost_perfcnt_fini’ [-Wmissing-prototypes] > > drivers/gpu/drm/panfrost/panfrost_mmu.c:227:6: warning: no previous > > prototype for ‘panfrost_mmu_flush_range’ [-Wmissing-prototypes] > > drivers/gpu/drm/panfrost/panfrost_mmu.c:435:5: warning: no previous > > prototype for ‘panfrost_mmu_map_fault_addr’ [-Wmissing-prototypes] > > > > For file panfrost_mmu.c, make functions static to fix this. > > For file panfrost_perfcnt.c, include head file can fix this. > > Nit: s/head/header/ > > > > > Signed-off-by: Yi Wang > > Looks good, just a few minor style issues (below), with those fixed: > > Reviewed-by: Steven Price > > > --- > > drivers/gpu/drm/panfrost/panfrost_mmu.c | 5 +++-- > > drivers/gpu/drm/panfrost/panfrost_perfcnt.c | 1 + > > 2 files changed, 4 insertions(+), 2 deletions(-) > > ... > > #include "panfrost_regs.h" > > +#include "panfrost_perfcnt.h" > > Please keep header includes sorted alphabetically. > > Thanks, > > Steve > > > > > #define COUNTERS_PER_BLOCK64 > > #define BYTES_PER_COUNTER4 > > --- Best wishes Yi Wang___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re:[PATCH v2] drm/panfrost: fix -Wmissing-prototypes warnings
Hi Steve,
How do you find the format problem? I have set noexpandtab in
vim ;-)
I will send a v3 patch with tabstop=8, please review later :)
Thanks a lot.
> On 22/10/2019 04:02, Yi Wang wrote:
> > We get these warnings when build kernel W=1:
> > drivers/gpu/drm/panfrost/panfrost_perfcnt.c:35:6: warning: no previous
> > prototype for ‘panfrost_perfcnt_clean_cache_done’ [-Wmissing-prototypes]
> > drivers/gpu/drm/panfrost/panfrost_perfcnt.c:40:6: warning: no previous
> > prototype for ‘panfrost_perfcnt_sample_done’ [-Wmissing-prototypes]
> > drivers/gpu/drm/panfrost/panfrost_perfcnt.c:190:5: warning: no previous
> > prototype for ‘panfrost_ioctl_perfcnt_enable’ [-Wmissing-prototypes]
> > drivers/gpu/drm/panfrost/panfrost_perfcnt.c:218:5: warning: no previous
> > prototype for ‘panfrost_ioctl_perfcnt_dump’ [-Wmissing-prototypes]
> > drivers/gpu/drm/panfrost/panfrost_perfcnt.c:250:6: warning: no previous
> > prototype for ‘panfrost_perfcnt_close’ [-Wmissing-prototypes]
> > drivers/gpu/drm/panfrost/panfrost_perfcnt.c:264:5: warning: no previous
> > prototype for ‘panfrost_perfcnt_init’ [-Wmissing-prototypes]
> > drivers/gpu/drm/panfrost/panfrost_perfcnt.c:320:6: warning: no previous
> > prototype for ‘panfrost_perfcnt_fini’ [-Wmissing-prototypes]
> > drivers/gpu/drm/panfrost/panfrost_mmu.c:227:6: warning: no previous
> > prototype for ‘panfrost_mmu_flush_range’ [-Wmissing-prototypes]
> > drivers/gpu/drm/panfrost/panfrost_mmu.c:435:5: warning: no previous
> > prototype for ‘panfrost_mmu_map_fault_addr’ [-Wmissing-prototypes]
> >
> > For file panfrost_mmu.c, make functions static to fix this.
> > For file panfrost_perfcnt.c, include header file can fix this.
> >
> > Signed-off-by: Yi Wang
> > Reviewed-by: Steven Price
> > ---
> >
> > v2: align parameter line, modify comment and sort header
> > includes alphabetically. Thanks to Steve.
> > ---
> > drivers/gpu/drm/panfrost/panfrost_mmu.c | 9 +
> > drivers/gpu/drm/panfrost/panfrost_perfcnt.c | 1 +
> > 2 files changed, 6 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/panfrost/panfrost_mmu.c
> > b/drivers/gpu/drm/panfrost/panfrost_mmu.c
> > index bdd9905..1f6cc7d 100644
> > --- a/drivers/gpu/drm/panfrost/panfrost_mmu.c
> > +++ b/drivers/gpu/drm/panfrost/panfrost_mmu.c
> > @@ -224,9 +224,9 @@ static size_t get_pgsize(u64 addr, size_t size)
> > return SZ_2M;
> > }
> >
> > -void panfrost_mmu_flush_range(struct panfrost_device *pfdev,
> > - struct panfrost_mmu *mmu,
> > - u64 iova, size_t size)
> > +static void panfrost_mmu_flush_range(struct panfrost_device *pfdev,
> > + struct panfrost_mmu *mmu,
> > + u64 iova, size_t size)
>
> Whoa, tabs in Linux are 8 spaces. This looks like you're using a tab
> size of 4. The same problem exists below too.
>
> Steve
>
> > {
> > if (mmu->as < 0)
> > return;
> > @@ -432,7 +432,8 @@ void panfrost_mmu_pgtable_free(struct
> > panfrost_file_priv *priv)
> >
> > #define NUM_FAULT_PAGES (SZ_2M / PAGE_SIZE)
> >
> > -int panfrost_mmu_map_fault_addr(struct panfrost_device *pfdev, int as, u64
> > addr)
> > +static int panfrost_mmu_map_fault_addr(struct panfrost_device *pfdev, int
> > as,
> > + u64 addr)
> > {
---
Best wishes
Yi Wang___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/i915/gvt: fix memory leak in intel_vgpu_ioctl()
> On 2018.08.03 08:41:19 +0800, Yi Wang wrote:
> > The 'sparse' variable may leak when return in function
> > intel_vgpu_ioctl(), and this patch fixes this.
> >
> > Signed-off-by: Yi Wang
> > Reviewed-by: Jiang Biao
> > ---
> > drivers/gpu/drm/i915/gvt/kvmgt.c | 3 +++
> > 1 file changed, 3 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c
> > b/drivers/gpu/drm/i915/gvt/kvmgt.c
> > index df4e4a0..6a6f199 100644
> > --- a/drivers/gpu/drm/i915/gvt/kvmgt.c
> > +++ b/drivers/gpu/drm/i915/gvt/kvmgt.c
> > @@ -1200,6 +1200,7 @@ static long intel_vgpu_ioctl(struct mdev_device
> > *mdev, unsigned int cmd,
> > return ret;
> > break;
> > default:
> > +kfree(sparse);
> > return -EINVAL;
> > }
> > }
> > @@ -1215,6 +1216,7 @@ static long intel_vgpu_ioctl(struct mdev_device
> > *mdev, unsigned int cmd,
> >sizeof(info), caps.buf,
> >caps.size)) {
> > kfree(caps.buf);
> > +kfree(sparse);
> > return -EFAULT;
> > }
> > info.cap_offset = sizeof(info);
> > @@ -1223,6 +1225,7 @@ static long intel_vgpu_ioctl(struct mdev_device
> > *mdev, unsigned int cmd,
> > kfree(caps.buf);
> > }
> >
> > +kfree(sparse);
>
> Unfortunately this would cause a double-free error in normal path, as we
> tried to free sparse after use to add caps. So may be better to fix free
> in error path and move normal free of sparse in final point, e.g
Yeah, that's right! Thanks a lot for your advice. I will send a v2 patch.
---
Best wishes
Yi Wang___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
