All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/3] Add comment to memory barrier
@ 2019-11-03 16:13 Evan Chime
  2019-11-03 16:14 ` [PATCH v2 1/3] Staging: media: omap4iss: " Evan Chime
                   ` (4 more replies)
  0 siblings, 5 replies; 13+ messages in thread
From: Evan Chime @ 2019-11-03 16:13 UTC (permalink / raw)
  To: gregkh; +Cc: outreachy-kernel, laurent.pinchart, mchehab

Include outreachy mailing list in "cc" list. No change

Evan Chime (3):
  Staging: media: omap4iss: Add comment to memory barrier
  Staging: media: omap4iss: Add comment to memory barrier
  Staging: media: omap4iss: Add comment to memory barrier

 drivers/staging/media/omap4iss/iss.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

-- 
2.17.1



^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH v2 1/3] Staging: media: omap4iss: Add comment to memory barrier
  2019-11-03 16:13 [PATCH v2 0/3] Add comment to memory barrier Evan Chime
@ 2019-11-03 16:14 ` Evan Chime
  2019-11-03 17:07   ` Greg KH
  2019-11-03 16:14 ` [PATCH v2 2/3] " Evan Chime
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 13+ messages in thread
From: Evan Chime @ 2019-11-03 16:14 UTC (permalink / raw)
  To: gregkh; +Cc: outreachy-kernel, laurent.pinchart, mchehab

Fix "WARNING: memory barrier without comment" from checkpath. Add comment
to memory barrier smp_wmb() on line 635

Signed-off-by: Evan Chime <chime.evan.dri.devel@gmail.com>
---
 drivers/staging/media/omap4iss/iss.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/media/omap4iss/iss.c b/drivers/staging/media/omap4iss/iss.c
index 1a966cb2f3a6..5156ad8e4b96 100644
--- a/drivers/staging/media/omap4iss/iss.c
+++ b/drivers/staging/media/omap4iss/iss.c
@@ -632,7 +632,7 @@ int omap4iss_module_sync_idle(struct media_entity *me, wait_queue_head_t *wait,
 	 * scenario. We'll call it here to avoid race conditions.
 	 */
 	atomic_set(stopping, 1);
-	smp_wmb();
+	smp_wmb(); /* Prevents stores from being reordered across the barrier */
 
 	/*
 	 * If module is the last one, it's writing to memory. In this case,
-- 
2.17.1



^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH v2 2/3] Staging: media: omap4iss: Add comment to memory barrier
  2019-11-03 16:13 [PATCH v2 0/3] Add comment to memory barrier Evan Chime
  2019-11-03 16:14 ` [PATCH v2 1/3] Staging: media: omap4iss: " Evan Chime
@ 2019-11-03 16:14 ` Evan Chime
  2019-11-03 16:15 ` [PATCH v2 3/3] " Evan Chime
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 13+ messages in thread
From: Evan Chime @ 2019-11-03 16:14 UTC (permalink / raw)
  To: gregkh; +Cc: outreachy-kernel, laurent.pinchart, mchehab

Fix "WARNING: memory barrier without comment" from checkpath. Add comment
to memory barrier smp_wmb() on line 653

Signed-off-by: Evan Chime <chime.evan.dri.devel@gmail.com>
---
 drivers/staging/media/omap4iss/iss.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/media/omap4iss/iss.c b/drivers/staging/media/omap4iss/iss.c
index 5156ad8e4b96..ce34c19874c7 100644
--- a/drivers/staging/media/omap4iss/iss.c
+++ b/drivers/staging/media/omap4iss/iss.c
@@ -650,7 +650,7 @@ int omap4iss_module_sync_idle(struct media_entity *me, wait_queue_head_t *wait,
 	if (video->dmaqueue_flags & ISS_VIDEO_DMAQUEUE_UNDERRUN) {
 		spin_unlock_irqrestore(&video->qlock, flags);
 		atomic_set(stopping, 0);
-		smp_wmb();
+		smp_wmb(); /* Prevent stores reordering across the barrier */
 		return 0;
 	}
 	spin_unlock_irqrestore(&video->qlock, flags);
-- 
2.17.1



^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH v2 3/3] Staging: media: omap4iss: Add comment to memory barrier
  2019-11-03 16:13 [PATCH v2 0/3] Add comment to memory barrier Evan Chime
  2019-11-03 16:14 ` [PATCH v2 1/3] Staging: media: omap4iss: " Evan Chime
  2019-11-03 16:14 ` [PATCH v2 2/3] " Evan Chime
@ 2019-11-03 16:15 ` Evan Chime
  2019-11-03 16:16   ` [Outreachy kernel] " Julia Lawall
  2019-11-03 16:18 ` [Outreachy kernel] [PATCH v2 0/3] " Julia Lawall
  2019-11-05 16:40 ` ledepiele
  4 siblings, 1 reply; 13+ messages in thread
From: Evan Chime @ 2019-11-03 16:15 UTC (permalink / raw)
  To: gregkh; +Cc: outreachy-kernel, laurent.pinchart, mchehab

Fix "WARNING: memory barrier without comment" from checkpath. Add comment
to memory barrier smp_wmb() on line 660

Signed-off-by: Evan Chime <chime.evan.dri.devel@gmail.com>
---
 drivers/staging/media/omap4iss/iss.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/media/omap4iss/iss.c b/drivers/staging/media/omap4iss/iss.c
index ce34c19874c7..85b0d9cbaa14 100644
--- a/drivers/staging/media/omap4iss/iss.c
+++ b/drivers/staging/media/omap4iss/iss.c
@@ -657,7 +657,7 @@ int omap4iss_module_sync_idle(struct media_entity *me, wait_queue_head_t *wait,
 	if (!wait_event_timeout(*wait, !atomic_read(stopping),
 				msecs_to_jiffies(1000))) {
 		atomic_set(stopping, 0);
-		smp_wmb();
+		smp_wmb(); /* Prevent stores reordering across the barrier */
 		return -ETIMEDOUT;
 	}
 
-- 
2.17.1



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [Outreachy kernel] [PATCH v2 3/3] Staging: media: omap4iss: Add comment to memory barrier
  2019-11-03 16:15 ` [PATCH v2 3/3] " Evan Chime
@ 2019-11-03 16:16   ` Julia Lawall
  2019-11-03 16:23     ` evan chime
  0 siblings, 1 reply; 13+ messages in thread
From: Julia Lawall @ 2019-11-03 16:16 UTC (permalink / raw)
  To: Evan Chime; +Cc: gregkh, outreachy-kernel, laurent.pinchart, mchehab



On Sun, 3 Nov 2019, Evan Chime wrote:

> Fix "WARNING: memory barrier without comment" from checkpath. Add comment
> to memory barrier smp_wmb() on line 660
>
> Signed-off-by: Evan Chime <chime.evan.dri.devel@gmail.com>
> ---
>  drivers/staging/media/omap4iss/iss.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/staging/media/omap4iss/iss.c b/drivers/staging/media/omap4iss/iss.c
> index ce34c19874c7..85b0d9cbaa14 100644
> --- a/drivers/staging/media/omap4iss/iss.c
> +++ b/drivers/staging/media/omap4iss/iss.c
> @@ -657,7 +657,7 @@ int omap4iss_module_sync_idle(struct media_entity *me, wait_queue_head_t *wait,
>  	if (!wait_event_timeout(*wait, !atomic_read(stopping),
>  				msecs_to_jiffies(1000))) {
>  		atomic_set(stopping, 0);
> -		smp_wmb();
> +		smp_wmb(); /* Prevent stores reordering across the barrier */

Which stores?

julia

>  		return -ETIMEDOUT;
>  	}
>
> --
> 2.17.1
>
> --
> You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/62b034d416bc0b0f2fc2a10c225bcf1441b4eecc.1572797322.git.chime.evan.dri.devel%40gmail.com.
>


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [Outreachy kernel] [PATCH v2 0/3] Add comment to memory barrier
  2019-11-03 16:13 [PATCH v2 0/3] Add comment to memory barrier Evan Chime
                   ` (2 preceding siblings ...)
  2019-11-03 16:15 ` [PATCH v2 3/3] " Evan Chime
@ 2019-11-03 16:18 ` Julia Lawall
  2019-11-03 16:27   ` evan chime
  2019-11-05 16:40 ` ledepiele
  4 siblings, 1 reply; 13+ messages in thread
From: Julia Lawall @ 2019-11-03 16:18 UTC (permalink / raw)
  To: Evan Chime; +Cc: gregkh, outreachy-kernel, laurent.pinchart, mchehab



On Sun, 3 Nov 2019, Evan Chime wrote:

> Include outreachy mailing list in "cc" list. No change
>
> Evan Chime (3):
>   Staging: media: omap4iss: Add comment to memory barrier
>   Staging: media: omap4iss: Add comment to memory barrier
>   Staging: media: omap4iss: Add comment to memory barrier

Each patch should have a different subject.  If you end up with patches
with the same subject, you should make the subjects more specific or merge
them.

In this case, though, I think that this is an issue that only someone with
a deep knowledge of the code could really address.  The question is not
what a memory barrier does, but why one is needed at a particular place in
the code.

julia

>
>  drivers/staging/media/omap4iss/iss.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> --
> 2.17.1
>
> --
> You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/cover.1572797322.git.chime.evan.dri.devel%40gmail.com.
>


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [Outreachy kernel] [PATCH v2 3/3] Staging: media: omap4iss: Add comment to memory barrier
  2019-11-03 16:16   ` [Outreachy kernel] " Julia Lawall
@ 2019-11-03 16:23     ` evan chime
  0 siblings, 0 replies; 13+ messages in thread
From: evan chime @ 2019-11-03 16:23 UTC (permalink / raw)
  To: Julia Lawall; +Cc: gregkh, laurent.pinchart, mchehab, outreachy-kernel

[-- Attachment #1: Type: text/plain, Size: 2017 bytes --]

On Sun, 3 Nov 2019 at 16:16, Julia Lawall <julia.lawall@lip6.fr> wrote:

>
>
> On Sun, 3 Nov 2019, Evan Chime wrote:
>
> > Fix "WARNING: memory barrier without comment" from checkpath. Add comment
> > to memory barrier smp_wmb() on line 660
> >
> > Signed-off-by: Evan Chime <chime.evan.dri.devel@gmail.com>
> > ---
> >  drivers/staging/media/omap4iss/iss.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/staging/media/omap4iss/iss.c
> b/drivers/staging/media/omap4iss/iss.c
> > index ce34c19874c7..85b0d9cbaa14 100644
> > --- a/drivers/staging/media/omap4iss/iss.c
> > +++ b/drivers/staging/media/omap4iss/iss.c
> > @@ -657,7 +657,7 @@ int omap4iss_module_sync_idle(struct media_entity
> *me, wait_queue_head_t *wait,
> >       if (!wait_event_timeout(*wait, !atomic_read(stopping),
> >                               msecs_to_jiffies(1000))) {
> >               atomic_set(stopping, 0);
> > -             smp_wmb();
> > +             smp_wmb(); /* Prevent stores reordering across the barrier
> */
>
> Which stores?
>
> julia
>

Memory writes

Kind regards
Evan



> >               return -ETIMEDOUT;
> >       }
> >
> > --
> > 2.17.1
> >
> > --
> > You received this message because you are subscribed to the Google
> Groups "outreachy-kernel" group.
> > To unsubscribe from this group and stop receiving emails from it, send
> an email to outreachy-kernel+unsubscribe@googlegroups.com.
> > To view this discussion on the web visit
> https://groups.google.com/d/msgid/outreachy-kernel/62b034d416bc0b0f2fc2a10c225bcf1441b4eecc.1572797322.git.chime.evan.dri.devel%40gmail.com
> .
> >
>
> --
> You received this message because you are subscribed to the Google Groups
> "outreachy-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to outreachy-kernel+unsubscribe@googlegroups.com.
> To view this discussion on the web visit
> https://groups.google.com/d/msgid/outreachy-kernel/alpine.DEB.2.21.1911031716020.2557%40hadrien
> .
>

[-- Attachment #2: Type: text/html, Size: 3424 bytes --]

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [Outreachy kernel] [PATCH v2 0/3] Add comment to memory barrier
  2019-11-03 16:18 ` [Outreachy kernel] [PATCH v2 0/3] " Julia Lawall
@ 2019-11-03 16:27   ` evan chime
  2019-11-04 13:26     ` evan chime
  0 siblings, 1 reply; 13+ messages in thread
From: evan chime @ 2019-11-03 16:27 UTC (permalink / raw)
  To: Julia Lawall; +Cc: gregkh, laurent.pinchart, mchehab, outreachy-kernel

[-- Attachment #1: Type: text/plain, Size: 1794 bytes --]

On Sun, 3 Nov 2019 at 16:18, Julia Lawall <julia.lawall@lip6.fr> wrote:

>
>
> On Sun, 3 Nov 2019, Evan Chime wrote:
>
> > Include outreachy mailing list in "cc" list. No change
> >
> > Evan Chime (3):
> >   Staging: media: omap4iss: Add comment to memory barrier
> >   Staging: media: omap4iss: Add comment to memory barrier
> >   Staging: media: omap4iss: Add comment to memory barrier
>
> Each patch should have a different subject.  If you end up with patches
> with the same subject, you should make the subjects more specific or merge
> them.
>
> In this case, though, I think that this is an issue that only someone with
> a deep knowledge of the code could really address.  The question is not
> what a memory barrier does, but why one is needed at a particular place in
> the code.
>
> julia


Thanks for the feedback Julia

Kind Regards
Evan



>
> >
> >  drivers/staging/media/omap4iss/iss.c | 6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> >
> > --
> > 2.17.1
> >
> > --
> > You received this message because you are subscribed to the Google
> Groups "outreachy-kernel" group.
> > To unsubscribe from this group and stop receiving emails from it, send
> an email to outreachy-kernel+unsubscribe@googlegroups.com.
> > To view this discussion on the web visit
> https://groups.google.com/d/msgid/outreachy-kernel/cover.1572797322.git.chime.evan.dri.devel%40gmail.com
> .
> >
>
> --
> You received this message because you are subscribed to the Google Groups
> "outreachy-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to outreachy-kernel+unsubscribe@googlegroups.com.
> To view this discussion on the web visit
> https://groups.google.com/d/msgid/outreachy-kernel/alpine.DEB.2.21.1911031716390.2557%40hadrien
> .
>

[-- Attachment #2: Type: text/html, Size: 2991 bytes --]

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 1/3] Staging: media: omap4iss: Add comment to memory barrier
  2019-11-03 16:14 ` [PATCH v2 1/3] Staging: media: omap4iss: " Evan Chime
@ 2019-11-03 17:07   ` Greg KH
  2019-11-03 17:26     ` evan chime
  0 siblings, 1 reply; 13+ messages in thread
From: Greg KH @ 2019-11-03 17:07 UTC (permalink / raw)
  To: Evan Chime; +Cc: outreachy-kernel, laurent.pinchart, mchehab

On Sun, Nov 03, 2019 at 04:14:31PM +0000, Evan Chime wrote:
> Fix "WARNING: memory barrier without comment" from checkpath. Add comment
> to memory barrier smp_wmb() on line 635
> 
> Signed-off-by: Evan Chime <chime.evan.dri.devel@gmail.com>
> ---
>  drivers/staging/media/omap4iss/iss.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/media/omap4iss/iss.c b/drivers/staging/media/omap4iss/iss.c
> index 1a966cb2f3a6..5156ad8e4b96 100644
> --- a/drivers/staging/media/omap4iss/iss.c
> +++ b/drivers/staging/media/omap4iss/iss.c
> @@ -632,7 +632,7 @@ int omap4iss_module_sync_idle(struct media_entity *me, wait_queue_head_t *wait,
>  	 * scenario. We'll call it here to avoid race conditions.
>  	 */
>  	atomic_set(stopping, 1);
> -	smp_wmb();
> +	smp_wmb(); /* Prevents stores from being reordered across the barrier */

We all know what smp_wmb() does, what you need to document is what
exactly it is here for.  What is it preventing from happening to what
data that is being touched elsewhere where?

If you don't know the code, solving something like this might be very
difficult, sorry.

greg k-h


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 1/3] Staging: media: omap4iss: Add comment to memory barrier
  2019-11-03 17:07   ` Greg KH
@ 2019-11-03 17:26     ` evan chime
  0 siblings, 0 replies; 13+ messages in thread
From: evan chime @ 2019-11-03 17:26 UTC (permalink / raw)
  To: Greg KH; +Cc: laurent.pinchart, mchehab, outreachy-kernel

[-- Attachment #1: Type: text/plain, Size: 1383 bytes --]

On Sun, 3 Nov 2019 at 17:07, Greg KH <gregkh@linuxfoundation.org> wrote:

> On Sun, Nov 03, 2019 at 04:14:31PM +0000, Evan Chime wrote:
> > Fix "WARNING: memory barrier without comment" from checkpath. Add comment
> > to memory barrier smp_wmb() on line 635
> >
> > Signed-off-by: Evan Chime <chime.evan.dri.devel@gmail.com>
> > ---
> >  drivers/staging/media/omap4iss/iss.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/staging/media/omap4iss/iss.c
> b/drivers/staging/media/omap4iss/iss.c
> > index 1a966cb2f3a6..5156ad8e4b96 100644
> > --- a/drivers/staging/media/omap4iss/iss.c
> > +++ b/drivers/staging/media/omap4iss/iss.c
> > @@ -632,7 +632,7 @@ int omap4iss_module_sync_idle(struct media_entity
> *me, wait_queue_head_t *wait,
> >        * scenario. We'll call it here to avoid race conditions.
> >        */
> >       atomic_set(stopping, 1);
> > -     smp_wmb();
> > +     smp_wmb(); /* Prevents stores from being reordered across the
> barrier */
>
> We all know what smp_wmb() does, what you need to document is what
> exactly it is here for.  What is it preventing from happening to what
> data that is being touched elsewhere where?
>
> If you don't know the code, solving something like this might be very
> difficult, sorry.
>
> greg k-h


Thanks for your feedback Greg. I will have a look at the code

Kind regards
Evan



>

[-- Attachment #2: Type: text/html, Size: 2187 bytes --]

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [Outreachy kernel] [PATCH v2 0/3] Add comment to memory barrier
  2019-11-03 16:27   ` evan chime
@ 2019-11-04 13:26     ` evan chime
  2019-11-04 18:54       ` evan chime
  0 siblings, 1 reply; 13+ messages in thread
From: evan chime @ 2019-11-04 13:26 UTC (permalink / raw)
  To: Julia Lawall; +Cc: gregkh, laurent.pinchart, mchehab, outreachy-kernel

[-- Attachment #1: Type: text/plain, Size: 2969 bytes --]

On Sun, 3 Nov 2019 at 16:27, evan chime <chime.evan.dri.devel@gmail.com>
wrote:

>
> On Sun, 3 Nov 2019 at 16:18, Julia Lawall <julia.lawall@lip6.fr> wrote:
>
>>
>>
>> On Sun, 3 Nov 2019, Evan Chime wrote:
>>
>> > Include outreachy mailing list in "cc" list. No change
>> >
>> > Evan Chime (3):
>> >   Staging: media: omap4iss: Add comment to memory barrier
>> >   Staging: media: omap4iss: Add comment to memory barrier
>> >   Staging: media: omap4iss: Add comment to memory barrier
>>
>> Each patch should have a different subject.  If you end up with patches
>> with the same subject, you should make the subjects more specific or merge
>> them.
>>
>> In this case, though, I think that this is an issue that only someone with
>> a deep knowledge of the code could really address.  The question is not
>> what a memory barrier does, but why one is needed at a particular place in
>> the code.
>>
>> julia
>
>
> Thanks for the feedback Julia
>
> Kind Regards
> Evan
>

Please I have studied the function. From what I understand, the function
checks if ISS submodule needs to wait for next interrupt. If yes, makes the
caller to sleep while waiting for such event.


The atomic_set(stopping, 1) indicates whether the submodule needs to wait,
where stopping is the flag, and atomic_set(stopping, 0) indicates no need
to wait


According to the comment before the first atomic_set() call and the
smp_wmb() call that follows which says:

/* Atomic set() doesn’t include memory barrier on ARM platform for SMP
scenario. We’ll call it here to avoid race conditions */, the smp_wmb()
call is I think, to avoid reordering of the atomic_set() operation


So can I use one of the following, as the comment


A) prevent reordering of atomic_set()


B) sync module with it’s idle state(from the comment that introduced the
function)


C) ensures *stopping is persistent


D) none of the above :)




>
>
>>
>> >
>> >  drivers/staging/media/omap4iss/iss.c | 6 +++---
>> >  1 file changed, 3 insertions(+), 3 deletions(-)
>> >
>> > --
>> > 2.17.1
>> >
>> > --
>> > You received this message because you are subscribed to the Google
>> Groups "outreachy-kernel" group.
>> > To unsubscribe from this group and stop receiving emails from it, send
>> an email to outreachy-kernel+unsubscribe@googlegroups.com.
>> > To view this discussion on the web visit
>> https://groups.google.com/d/msgid/outreachy-kernel/cover.1572797322.git.chime.evan.dri.devel%40gmail.com
>> .
>> >
>>
>> --
>> You received this message because you are subscribed to the Google Groups
>> "outreachy-kernel" group.
>> To unsubscribe from this group and stop receiving emails from it, send an
>> email to outreachy-kernel+unsubscribe@googlegroups.com.
>> To view this discussion on the web visit
>> https://groups.google.com/d/msgid/outreachy-kernel/alpine.DEB.2.21.1911031716390.2557%40hadrien
>> .
>>
>

[-- Attachment #2: Type: text/html, Size: 8716 bytes --]

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [Outreachy kernel] [PATCH v2 0/3] Add comment to memory barrier
  2019-11-04 13:26     ` evan chime
@ 2019-11-04 18:54       ` evan chime
  0 siblings, 0 replies; 13+ messages in thread
From: evan chime @ 2019-11-04 18:54 UTC (permalink / raw)
  To: Julia Lawall; +Cc: gregkh, laurent.pinchart, mchehab, outreachy-kernel

[-- Attachment #1: Type: text/plain, Size: 4279 bytes --]

On Mon, 4 Nov 2019 at 13:26, evan chime <chime.evan.dri.devel@gmail.com>
wrote:

>
>
> On Sun, 3 Nov 2019 at 16:27, evan chime <chime.evan.dri.devel@gmail.com>
> wrote:
>
>>
>> On Sun, 3 Nov 2019 at 16:18, Julia Lawall <julia.lawall@lip6.fr> wrote:
>>
>>>
>>>
>>> On Sun, 3 Nov 2019, Evan Chime wrote:
>>>
>>> > Include outreachy mailing list in "cc" list. No change
>>> >
>>> > Evan Chime (3):
>>> >   Staging: media: omap4iss: Add comment to memory barrier
>>> >   Staging: media: omap4iss: Add comment to memory barrier
>>> >   Staging: media: omap4iss: Add comment to memory barrier
>>>
>>> Each patch should have a different subject.  If you end up with patches
>>> with the same subject, you should make the subjects more specific or
>>> merge
>>> them.
>>>
>>> In this case, though, I think that this is an issue that only someone
>>> with
>>> a deep knowledge of the code could really address.  The question is not
>>> what a memory barrier does, but why one is needed at a particular place
>>> in
>>> the code.
>>>
>>> julia
>>
>>
>> Thanks for the feedback Julia
>>
>> Kind Regards
>> Evan
>>
>
> Please I have studied the function. From what I understand, the function
> checks if ISS submodule needs to wait for next interrupt. If yes, makes the
> caller to sleep while waiting for such event.
>
>
> The atomic_set(stopping, 1) indicates whether the submodule needs to wait,
> where stopping is the flag, and atomic_set(stopping, 0) indicates no need
> to wait
>
>
> According to the comment before the first atomic_set() call and the
> smp_wmb() call that follows which says:
>
> /* Atomic set() doesn’t include memory barrier on ARM platform for SMP
> scenario. We’ll call it here to avoid race conditions */, the smp_wmb()
> call is I think, to avoid reordering of the atomic_set() operation
>
>
> So can I use one of the following, as the comment
>
>
> A) prevent reordering of atomic_set()
>
>
> B) sync module with it’s idle state(from the comment that introduced the
> function)
>
>
> C) ensures *stopping is persistent
>
>
> D) none of the above :)
>

Please forgive me for the first email, just noticed I didn’t say who it was
coming from at the end.


I have studied the function. From what I understand, the function checks if
ISS submodule needs to wait for next interrupt. If yes, makes the caller to
sleep while waiting for such event.


The atomic_set(stopping, 1) indicates whether the submodule needs to wait,
where stopping is the flag, and atomic_set(stopping, 0) indicates no need
to wait


According to the comment before the first atomic_set() call and the
smp_wmb() call that follows which says:

/* Atomic set() doesn’t include memory barrier on ARM platform for SMP
scenario. We’ll call it here to avoid race conditions */, the smp_wmb()
call is I think, to avoid reordering of the atomic_set() operation


So can I use one of the following, as the comment


A) prevent reordering of atomic_set()


B) sync module with it’s idle state(from the comment that introduced the
function)


C) ensures *stopping is persistent


D) none of the above :)


Kind regards

Evan



>
>
>>
>>
>>>
>>> >
>>> >  drivers/staging/media/omap4iss/iss.c | 6 +++---
>>> >  1 file changed, 3 insertions(+), 3 deletions(-)
>>> >
>>> > --
>>> > 2.17.1
>>> >
>>> > --
>>> > You received this message because you are subscribed to the Google
>>> Groups "outreachy-kernel" group.
>>> > To unsubscribe from this group and stop receiving emails from it, send
>>> an email to outreachy-kernel+unsubscribe@googlegroups.com.
>>> > To view this discussion on the web visit
>>> https://groups.google.com/d/msgid/outreachy-kernel/cover.1572797322.git.chime.evan.dri.devel%40gmail.com
>>> .
>>> >
>>>
>>> --
>>> You received this message because you are subscribed to the Google
>>> Groups "outreachy-kernel" group.
>>> To unsubscribe from this group and stop receiving emails from it, send
>>> an email to outreachy-kernel+unsubscribe@googlegroups.com.
>>> To view this discussion on the web visit
>>> https://groups.google.com/d/msgid/outreachy-kernel/alpine.DEB.2.21.1911031716390.2557%40hadrien
>>> .
>>>
>>

[-- Attachment #2: Type: text/html, Size: 16317 bytes --]

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH v2 0/3] Add comment to memory barrier
  2019-11-03 16:13 [PATCH v2 0/3] Add comment to memory barrier Evan Chime
                   ` (3 preceding siblings ...)
  2019-11-03 16:18 ` [Outreachy kernel] [PATCH v2 0/3] " Julia Lawall
@ 2019-11-05 16:40 ` ledepiele
  4 siblings, 0 replies; 13+ messages in thread
From: ledepiele @ 2019-11-05 16:40 UTC (permalink / raw)
  To: outreachy-kernel

[-- Attachment #1: Type: text/plain, Size: 15 bytes --]

Comment








^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2019-11-06 19:26 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-03 16:13 [PATCH v2 0/3] Add comment to memory barrier Evan Chime
2019-11-03 16:14 ` [PATCH v2 1/3] Staging: media: omap4iss: " Evan Chime
2019-11-03 17:07   ` Greg KH
2019-11-03 17:26     ` evan chime
2019-11-03 16:14 ` [PATCH v2 2/3] " Evan Chime
2019-11-03 16:15 ` [PATCH v2 3/3] " Evan Chime
2019-11-03 16:16   ` [Outreachy kernel] " Julia Lawall
2019-11-03 16:23     ` evan chime
2019-11-03 16:18 ` [Outreachy kernel] [PATCH v2 0/3] " Julia Lawall
2019-11-03 16:27   ` evan chime
2019-11-04 13:26     ` evan chime
2019-11-04 18:54       ` evan chime
2019-11-05 16:40 ` ledepiele

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.