* [PATCH] staging: sm750fb: always take the lock
@ 2017-06-25 21:39 AbdAllah-MEZITI
2017-06-26 4:55 ` Greg Kroah-Hartman
2017-06-26 6:25 ` Frans Klaver
0 siblings, 2 replies; 8+ messages in thread
From: AbdAllah-MEZITI @ 2017-06-25 21:39 UTC (permalink / raw)
To: sudipm.mukherjee
Cc: AbdAllah MEZITI, Teddy Wang, Greg Kroah-Hartman, linux-fbdev,
devel, linux-kernel
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
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] staging: sm750fb: always take the lock
2017-06-25 21:39 [PATCH] staging: sm750fb: always take the lock AbdAllah-MEZITI
@ 2017-06-26 4:55 ` Greg Kroah-Hartman
2017-06-26 6:25 ` Frans Klaver
1 sibling, 0 replies; 8+ messages in thread
From: Greg Kroah-Hartman @ 2017-06-26 4:55 UTC (permalink / raw)
To: AbdAllah-MEZITI
Cc: sudipm.mukherjee, devel, linux-fbdev, Teddy Wang, linux-kernel
On Sun, Jun 25, 2017 at 11:39:20PM +0200, AbdAllah-MEZITI wrote:
> Signed-off-by: AbdAllah MEZITI <abdallah.meziti.pro@gmail.com>
I can't take patches without any changelog text, sorry.
greg k-h
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] staging: sm750fb: always take the lock
2017-06-25 21:39 [PATCH] staging: sm750fb: always take the lock AbdAllah-MEZITI
2017-06-26 4:55 ` Greg Kroah-Hartman
@ 2017-06-26 6:25 ` Frans Klaver
1 sibling, 0 replies; 8+ messages in thread
From: Frans Klaver @ 2017-06-26 6:25 UTC (permalink / raw)
To: AbdAllah-MEZITI
Cc: Sudip Mukherjee, Teddy Wang, Greg Kroah-Hartman, linux-fbdev,
devel, linux-kernel@vger.kernel.org
On Sun, Jun 25, 2017 at 11:39 PM, AbdAllah-MEZITI
<abdallah.meziti.pro@gmail.com> wrote:
> Subject: [PATCH] staging: sm750fb: always take the lock
When sending a new version of your patch, include a version number:
Subject: [PATCH V2] staging: ...
Frans
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH] staging: sm750fb: always take the lock
@ 2017-06-26 5:45 AbdAllah-MEZITI
2017-06-26 6:33 ` Frans Klaver
2017-06-26 9:11 ` Geert Uytterhoeven
0 siblings, 2 replies; 8+ messages in thread
From: AbdAllah-MEZITI @ 2017-06-26 5:45 UTC (permalink / raw)
To: sudipm.mukherjee
Cc: AbdAllah MEZITI, Teddy Wang, Greg Kroah-Hartman, linux-fbdev,
devel, linux-kernel
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
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] staging: sm750fb: always take the lock
2017-06-26 5:45 AbdAllah-MEZITI
@ 2017-06-26 6:33 ` Frans Klaver
2017-06-26 9:11 ` Geert Uytterhoeven
1 sibling, 0 replies; 8+ messages in thread
From: Frans Klaver @ 2017-06-26 6:33 UTC (permalink / raw)
To: AbdAllah-MEZITI
Cc: Sudip Mukherjee, Teddy Wang, Greg Kroah-Hartman, linux-fbdev,
devel, linux-kernel@vger.kernel.org
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
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] staging: sm750fb: always take the lock
2017-06-26 5:45 AbdAllah-MEZITI
2017-06-26 6:33 ` Frans Klaver
@ 2017-06-26 9:11 ` Geert Uytterhoeven
2017-06-26 9:16 ` Frans Klaver
1 sibling, 1 reply; 8+ messages in thread
From: Geert Uytterhoeven @ 2017-06-26 9:11 UTC (permalink / raw)
To: AbdAllah-MEZITI
Cc: Sudip Mukherjee, Teddy Wang, Greg Kroah-Hartman,
Linux Fbdev development list, driverdevel,
linux-kernel@vger.kernel.org
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
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] staging: sm750fb: always take the lock
2017-06-26 9:11 ` Geert Uytterhoeven
@ 2017-06-26 9:16 ` Frans Klaver
2017-06-26 9:19 ` Geert Uytterhoeven
0 siblings, 1 reply; 8+ messages in thread
From: Frans Klaver @ 2017-06-26 9:16 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: AbdAllah-MEZITI, Sudip Mukherjee, Teddy Wang, Greg Kroah-Hartman,
Linux Fbdev development list, driverdevel,
linux-kernel@vger.kernel.org
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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] staging: sm750fb: always take the lock
2017-06-26 9:16 ` Frans Klaver
@ 2017-06-26 9:19 ` Geert Uytterhoeven
0 siblings, 0 replies; 8+ messages in thread
From: Geert Uytterhoeven @ 2017-06-26 9:19 UTC (permalink / raw)
To: Frans Klaver
Cc: AbdAllah-MEZITI, Sudip Mukherjee, Teddy Wang, Greg Kroah-Hartman,
Linux Fbdev development list, driverdevel,
linux-kernel@vger.kernel.org
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
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2017-06-26 9:19 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-06-25 21:39 [PATCH] staging: sm750fb: always take the lock AbdAllah-MEZITI
2017-06-26 4:55 ` Greg Kroah-Hartman
2017-06-26 6:25 ` Frans Klaver
-- strict thread matches above, loose matches on Subject: below --
2017-06-26 5:45 AbdAllah-MEZITI
2017-06-26 6:33 ` Frans Klaver
2017-06-26 9:11 ` Geert Uytterhoeven
2017-06-26 9:16 ` Frans Klaver
2017-06-26 9:19 ` Geert Uytterhoeven
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox