Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1674442 > unrolled thread
| Started by | AbdAllah-MEZITI <abdallah.meziti.pro@gmail.com> |
|---|---|
| First post | 2017-06-26 07:50 +0200 |
| Last post | 2017-06-26 11:30 +0200 |
| Articles | 5 — 3 participants |
Back to article view | Back to linux.kernel
[PATCH] staging: sm750fb: always take the lock AbdAllah-MEZITI <abdallah.meziti.pro@gmail.com> - 2017-06-26 07:50 +0200
Re: [PATCH] staging: sm750fb: always take the lock Frans Klaver <fransklaver@gmail.com> - 2017-06-26 08:40 +0200
Re: [PATCH] staging: sm750fb: always take the lock Geert Uytterhoeven <geert@linux-m68k.org> - 2017-06-26 11:20 +0200
Re: [PATCH] staging: sm750fb: always take the lock Frans Klaver <fransklaver@gmail.com> - 2017-06-26 11:20 +0200
Re: [PATCH] staging: sm750fb: always take the lock Geert Uytterhoeven <geert@linux-m68k.org> - 2017-06-26 11:30 +0200
| From | AbdAllah-MEZITI <abdallah.meziti.pro@gmail.com> |
|---|---|
| Date | 2017-06-26 07:50 +0200 |
| Subject | [PATCH] staging: sm750fb: always take the lock |
| Message-ID | <tWv4l-6Jc-5@gated-at.bofh.it> |
This patch - will always take the lock - fix the sparse warning: drivers/staging/sm750fb/sm750.c:159:13: warning: context imbalance in 'lynxfb_ops_fillrect' - different lock contexts for basic block drivers/staging/sm750fb/sm750.c:231:9: warning: context imbalance in 'lynxfb_ops_copyarea' - different lock contexts for basic block drivers/staging/sm750fb/sm750.c:235:13: warning: context imbalance in 'lynxfb_ops_imageblit' - different lock contexts for basic block Signed-off-by: AbdAllah MEZITI <abdallah.meziti.pro@gmail.com> --- drivers/staging/sm750fb/sm750.c | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/drivers/staging/sm750fb/sm750.c b/drivers/staging/sm750fb/sm750.c index 386d4ad..4a22190 100644 --- a/drivers/staging/sm750fb/sm750.c +++ b/drivers/staging/sm750fb/sm750.c @@ -186,16 +186,14 @@ static void lynxfb_ops_fillrect(struct fb_info *info, * If not use spin_lock,system will die if user load driver * and immediately unload driver frequently (dual) */ - if (sm750_dev->fb_count > 1) - spin_lock(&sm750_dev->slock); + spin_lock(&sm750_dev->slock); sm750_dev->accel.de_fillrect(&sm750_dev->accel, base, pitch, Bpp, region->dx, region->dy, region->width, region->height, color, rop); - if (sm750_dev->fb_count > 1) - spin_unlock(&sm750_dev->slock); + spin_unlock(&sm750_dev->slock); } static void lynxfb_ops_copyarea(struct fb_info *info, @@ -220,16 +218,14 @@ static void lynxfb_ops_copyarea(struct fb_info *info, * If not use spin_lock, system will die if user load driver * and immediately unload driver frequently (dual) */ - if (sm750_dev->fb_count > 1) - spin_lock(&sm750_dev->slock); + spin_lock(&sm750_dev->slock); sm750_dev->accel.de_copyarea(&sm750_dev->accel, base, pitch, region->sx, region->sy, base, pitch, Bpp, region->dx, region->dy, region->width, region->height, HW_ROP2_COPY); - if (sm750_dev->fb_count > 1) - spin_unlock(&sm750_dev->slock); + spin_unlock(&sm750_dev->slock); } static void lynxfb_ops_imageblit(struct fb_info *info, @@ -269,8 +265,7 @@ static void lynxfb_ops_imageblit(struct fb_info *info, * If not use spin_lock, system will die if user load driver * and immediately unload driver frequently (dual) */ - if (sm750_dev->fb_count > 1) - spin_lock(&sm750_dev->slock); + spin_lock(&sm750_dev->slock); sm750_dev->accel.de_imageblit(&sm750_dev->accel, image->data, image->width >> 3, 0, @@ -278,8 +273,7 @@ static void lynxfb_ops_imageblit(struct fb_info *info, image->dx, image->dy, image->width, image->height, fgcol, bgcol, HW_ROP2_COPY); - if (sm750_dev->fb_count > 1) - spin_unlock(&sm750_dev->slock); + spin_unlock(&sm750_dev->slock); } static int lynxfb_ops_pan_display(struct fb_var_screeninfo *var, -- 2.7.4
[toc] | [next] | [standalone]
| From | Frans Klaver <fransklaver@gmail.com> |
|---|---|
| Date | 2017-06-26 08:40 +0200 |
| Message-ID | <tWvQJ-7gm-9@gated-at.bofh.it> |
| In reply to | #1674442 |
There's no version number. Which one is the correct one? On Mon, Jun 26, 2017 at 7:45 AM, AbdAllah-MEZITI <abdallah.meziti.pro@gmail.com> wrote: > This patch > - will always take the lock > - fix the sparse warning: > drivers/staging/sm750fb/sm750.c:159:13: warning: context imbalance in 'lynxfb_ops_fillrect' - different lock contexts for basic block > drivers/staging/sm750fb/sm750.c:231:9: warning: context imbalance in 'lynxfb_ops_copyarea' - different lock contexts for basic block > drivers/staging/sm750fb/sm750.c:235:13: warning: context imbalance in 'lynxfb_ops_imageblit' - different lock contexts for basic block > > Signed-off-by: AbdAllah MEZITI <abdallah.meziti.pro@gmail.com> > --- > drivers/staging/sm750fb/sm750.c | 18 ++++++------------ > 1 file changed, 6 insertions(+), 12 deletions(-) > > diff --git a/drivers/staging/sm750fb/sm750.c b/drivers/staging/sm750fb/sm750.c > index 386d4ad..4a22190 100644 > --- a/drivers/staging/sm750fb/sm750.c > +++ b/drivers/staging/sm750fb/sm750.c > @@ -186,16 +186,14 @@ static void lynxfb_ops_fillrect(struct fb_info *info, > * If not use spin_lock,system will die if user load driver > * and immediately unload driver frequently (dual) > */ > - if (sm750_dev->fb_count > 1) > - spin_lock(&sm750_dev->slock); > + spin_lock(&sm750_dev->slock); > > sm750_dev->accel.de_fillrect(&sm750_dev->accel, > base, pitch, Bpp, > region->dx, region->dy, > region->width, region->height, > color, rop); > - if (sm750_dev->fb_count > 1) > - spin_unlock(&sm750_dev->slock); > + spin_unlock(&sm750_dev->slock); > } > > static void lynxfb_ops_copyarea(struct fb_info *info, > @@ -220,16 +218,14 @@ static void lynxfb_ops_copyarea(struct fb_info *info, > * If not use spin_lock, system will die if user load driver > * and immediately unload driver frequently (dual) > */ > - if (sm750_dev->fb_count > 1) > - spin_lock(&sm750_dev->slock); > + spin_lock(&sm750_dev->slock); > > sm750_dev->accel.de_copyarea(&sm750_dev->accel, > base, pitch, region->sx, region->sy, > base, pitch, Bpp, region->dx, region->dy, > region->width, region->height, > HW_ROP2_COPY); > - if (sm750_dev->fb_count > 1) > - spin_unlock(&sm750_dev->slock); > + spin_unlock(&sm750_dev->slock); > } > > static void lynxfb_ops_imageblit(struct fb_info *info, > @@ -269,8 +265,7 @@ static void lynxfb_ops_imageblit(struct fb_info *info, > * If not use spin_lock, system will die if user load driver > * and immediately unload driver frequently (dual) > */ > - if (sm750_dev->fb_count > 1) > - spin_lock(&sm750_dev->slock); > + spin_lock(&sm750_dev->slock); > > sm750_dev->accel.de_imageblit(&sm750_dev->accel, > image->data, image->width >> 3, 0, > @@ -278,8 +273,7 @@ static void lynxfb_ops_imageblit(struct fb_info *info, > image->dx, image->dy, > image->width, image->height, > fgcol, bgcol, HW_ROP2_COPY); > - if (sm750_dev->fb_count > 1) > - spin_unlock(&sm750_dev->slock); > + spin_unlock(&sm750_dev->slock); > } > > static int lynxfb_ops_pan_display(struct fb_var_screeninfo *var, > -- > 2.7.4 >
[toc] | [prev] | [next] | [standalone]
| From | Geert Uytterhoeven <geert@linux-m68k.org> |
|---|---|
| Date | 2017-06-26 11:20 +0200 |
| Message-ID | <tWylA-tB-5@gated-at.bofh.it> |
| In reply to | #1674442 |
On Mon, Jun 26, 2017 at 7:45 AM, AbdAllah-MEZITI
<abdallah.meziti.pro@gmail.com> wrote:
> This patch
> - will always take the lock
Why?
"The current code only takes the lock if multiple instances are in use.
This is error-prone, and confuses static analyzers.
As taking the lock in case of a single instance is harmful and cheap,
change the code to always take the lock."
> - fix the sparse warning:
> drivers/staging/sm750fb/sm750.c:159:13: warning: context imbalance in 'lynxfb_ops_fillrect' - different lock contexts for basic block
> drivers/staging/sm750fb/sm750.c:231:9: warning: context imbalance in 'lynxfb_ops_copyarea' - different lock contexts for basic block
> drivers/staging/sm750fb/sm750.c:235:13: warning: context imbalance in 'lynxfb_ops_imageblit' - different lock contexts for basic block
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
[toc] | [prev] | [next] | [standalone]
| From | Frans Klaver <fransklaver@gmail.com> |
|---|---|
| Date | 2017-06-26 11:20 +0200 |
| Message-ID | <tWylA-tB-19@gated-at.bofh.it> |
| In reply to | #1674545 |
On Mon, Jun 26, 2017 at 11:11 AM, Geert Uytterhoeven <geert@linux-m68k.org> wrote: > On Mon, Jun 26, 2017 at 7:45 AM, AbdAllah-MEZITI > <abdallah.meziti.pro@gmail.com> wrote: >> This patch >> - will always take the lock > > Why? > > "The current code only takes the lock if multiple instances are in use. > This is error-prone, and confuses static analyzers. > As taking the lock in case of a single instance is harmful and cheap, > change the code to always take the lock." I would argue that it's not harmful, lest people get confused about it. And I agree that this explanation is much more useful than just mentioning the warnings that you saw. Thanks, Frans
[toc] | [prev] | [next] | [standalone]
| From | Geert Uytterhoeven <geert@linux-m68k.org> |
|---|---|
| Date | 2017-06-26 11:30 +0200 |
| Message-ID | <tWyvg-x2-41@gated-at.bofh.it> |
| In reply to | #1674549 |
On Mon, Jun 26, 2017 at 11:16 AM, Frans Klaver <fransklaver@gmail.com> wrote:
> On Mon, Jun 26, 2017 at 11:11 AM, Geert Uytterhoeven
> <geert@linux-m68k.org> wrote:
>> On Mon, Jun 26, 2017 at 7:45 AM, AbdAllah-MEZITI
>> <abdallah.meziti.pro@gmail.com> wrote:
>>> This patch
>>> - will always take the lock
>>
>> Why?
>>
>> "The current code only takes the lock if multiple instances are in use.
>> This is error-prone, and confuses static analyzers.
>> As taking the lock in case of a single instance is harmful and cheap,
>> change the code to always take the lock."
>
> I would argue that it's not harmful, lest people get confused about
Sorry, I meant "harmless". I should start caring as much for commit
messages I write for other people as for those I write for myself ;-)
> it. And I agree that this explanation is much more useful than just
> mentioning the warnings that you saw.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web