linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3] staging: r8188eu: Use kzalloc() with GFP_ATOMIC in atomic context
@ 2021-11-01 19:18 Fabio M. De Francesco
  2021-11-05 13:25 ` Dan Carpenter
  2021-11-07 11:43 ` Fabio M. De Francesco
  0 siblings, 2 replies; 12+ messages in thread
From: Fabio M. De Francesco @ 2021-11-01 19:18 UTC (permalink / raw)
  To: Larry Finger, Phillip Potter, Greg Kroah-Hartman, linux-staging,
	linux-kernel
  Cc: Fabio M. De Francesco

Use the GFP_ATOMIC flag of kzalloc() with two memory allocation in
report_del_sta_event(). This function is called while holding spinlocks,
therefore it is not allowed to sleep. With the GFP_ATOMIC type flag, the
allocation is high priority and must not sleep.

This issue is detected by Smatch which emits the following warning:
"drivers/staging/r8188eu/core/rtw_mlme_ext.c:6848 report_del_sta_event()
warn: sleeping in atomic context".

After the change, the post-commit hook output the following message:
"CHECK: Prefer kzalloc(sizeof(*pcmd_obj)...) over
kzalloc(sizeof(struct cmd_obj)...)".

According to the above "CHECK", use the preferred style in the first
kzalloc().

Fixes: 79f712ea994d ("staging: r8188eu: Remove wrappers for kalloc() and kzalloc()")
Signed-off-by: Fabio M. De Francesco <fmdefrancesco@gmail.com>
---

v2->v3: Add the "Fixes:" tag, as requested by Greg Kroah-Hartman.

v1->v2: Fix an error that I introduced with an incorrect copy-paste
        of the sizeof() operator.

 drivers/staging/r8188eu/core/rtw_mlme_ext.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/r8188eu/core/rtw_mlme_ext.c b/drivers/staging/r8188eu/core/rtw_mlme_ext.c
index 55c3d4a6faeb..315902682292 100644
--- a/drivers/staging/r8188eu/core/rtw_mlme_ext.c
+++ b/drivers/staging/r8188eu/core/rtw_mlme_ext.c
@@ -6845,12 +6845,12 @@ void report_del_sta_event(struct adapter *padapter, unsigned char *MacAddr, unsi
 	struct mlme_ext_priv		*pmlmeext = &padapter->mlmeextpriv;
 	struct cmd_priv *pcmdpriv = &padapter->cmdpriv;
 
-	pcmd_obj = kzalloc(sizeof(struct cmd_obj), GFP_KERNEL);
+	pcmd_obj = kzalloc(sizeof(*pcmd_obj), GFP_ATOMIC);
 	if (!pcmd_obj)
 		return;
 
 	cmdsz = (sizeof(struct stadel_event) + sizeof(struct C2HEvent_Header));
-	pevtcmd = kzalloc(cmdsz, GFP_KERNEL);
+	pevtcmd = kzalloc(cmdsz, GFP_ATOMIC);
 	if (!pevtcmd) {
 		kfree(pcmd_obj);
 		return;
-- 
2.33.1


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

* Re: [PATCH v3] staging: r8188eu: Use kzalloc() with GFP_ATOMIC in atomic context
  2021-11-01 19:18 [PATCH v3] staging: r8188eu: Use kzalloc() with GFP_ATOMIC in atomic context Fabio M. De Francesco
@ 2021-11-05 13:25 ` Dan Carpenter
  2021-11-05 15:18   ` Fabio M. De Francesco
  2021-11-07 11:43 ` Fabio M. De Francesco
  1 sibling, 1 reply; 12+ messages in thread
From: Dan Carpenter @ 2021-11-05 13:25 UTC (permalink / raw)
  To: Fabio M. De Francesco
  Cc: Larry Finger, Phillip Potter, Greg Kroah-Hartman, linux-staging,
	linux-kernel

On Mon, Nov 01, 2021 at 08:18:47PM +0100, Fabio M. De Francesco wrote:
> Use the GFP_ATOMIC flag of kzalloc() with two memory allocation in
> report_del_sta_event(). This function is called while holding spinlocks,
> therefore it is not allowed to sleep. With the GFP_ATOMIC type flag, the
> allocation is high priority and must not sleep.
> 
> This issue is detected by Smatch which emits the following warning:
> "drivers/staging/r8188eu/core/rtw_mlme_ext.c:6848 report_del_sta_event()
> warn: sleeping in atomic context".
> 
> After the change, the post-commit hook output the following message:
> "CHECK: Prefer kzalloc(sizeof(*pcmd_obj)...) over
> kzalloc(sizeof(struct cmd_obj)...)".
> 
> According to the above "CHECK", use the preferred style in the first
> kzalloc().
> 
> Fixes: 79f712ea994d ("staging: r8188eu: Remove wrappers for kalloc() and kzalloc()")

This is not the correct Fixes tag.  The original allocation wrappers
checked in_interrupt() they did not check in_atomic() so they had same
bug.  The correct tag is:

Fixes: 15865124feed ("staging: r8188eu: introduce new core dir for RTL8188eu driver")

regards,
dan carpenter


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

* Re: [PATCH v3] staging: r8188eu: Use kzalloc() with GFP_ATOMIC in atomic context
  2021-11-05 13:25 ` Dan Carpenter
@ 2021-11-05 15:18   ` Fabio M. De Francesco
  2021-11-05 15:36     ` Dan Carpenter
  0 siblings, 1 reply; 12+ messages in thread
From: Fabio M. De Francesco @ 2021-11-05 15:18 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: Larry Finger, Phillip Potter, Greg Kroah-Hartman, linux-staging,
	linux-kernel

On Friday, November 5, 2021 2:25:52 PM CET Dan Carpenter wrote:
> On Mon, Nov 01, 2021 at 08:18:47PM +0100, Fabio M. De Francesco wrote:
> > Use the GFP_ATOMIC flag of kzalloc() with two memory allocation in
> > report_del_sta_event(). This function is called while holding spinlocks,
> > therefore it is not allowed to sleep. With the GFP_ATOMIC type flag, the
> > allocation is high priority and must not sleep.
> > 
> > This issue is detected by Smatch which emits the following warning:
> > "drivers/staging/r8188eu/core/rtw_mlme_ext.c:6848 report_del_sta_event()
> > warn: sleeping in atomic context".
> > 
> > After the change, the post-commit hook output the following message:
> > "CHECK: Prefer kzalloc(sizeof(*pcmd_obj)...) over
> > kzalloc(sizeof(struct cmd_obj)...)".
> > 
> > According to the above "CHECK", use the preferred style in the first
> > kzalloc().
> > 
> > Fixes: 79f712ea994d ("staging: r8188eu: Remove wrappers for kalloc() and 
kzalloc()")
> 
> This is not the correct Fixes tag.  The original allocation wrappers
> checked in_interrupt() they did not check in_atomic() so they had same
> bug.  The correct tag is:
> 
> Fixes: 15865124feed ("staging: r8188eu: introduce new core dir for 
RTL8188eu driver")
> 
> regards,
> dan carpenter

Hello Dan,

I'm sorry but I surely missing something, therefore, before making changes I 
need to understand this subject a little better. Let me explain what I am 
missing...

The two kzalloc() in report_del_sta_event() are called while spinlocks are 
held and bottom halves are disabled by spin_lock_bh(). If I remember it 
correctly spin_lock_bh() finally calls __local_bh_disable_ip() to disable 
bottom halves on local CPU before actually acquiring the lock.

This is the code and inline documentation of in_interrupt():

/* in_interrupt() - We're in NMI,IRQ,SoftIRQ context or have BH disabled" */
#define irq_count()	(nmi_count() | hardirq_count() | softirq_count())
#define in_interrupt()		(irq_count())

And this is the code and inline documentation of in_atomic():

"/*
 * Are we running in atomic context?  WARNING: this macro cannot
 * always detect atomic context; in particular, it cannot know about
 * held spinlocks in non-preemptible kernels.  Thus it should not be
 * used in the general case to determine whether sleeping is possible.
 * Do not use in_atomic() in driver code.
 */
#define in_atomic()	(preempt_count() != 0)

To summarize, I think that using in_interrupt() in the old wrappers was the 
wiser choice. Therefore this patch fixes 79f712ea994d ("staging: r8188eu: 
Remove wrappers for kalloc() and kzalloc()").

I know that I have so little experience that I shouldn't even discuss this 
topics. However, I would appreciate if you may explain with some more details 
why in_atomic() should have been preferred over in_interrupt() in the old 
wrappers that were removed with commit 79f712ea994d.

Thank you very much in advance,

Fabio M. De Francesco



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

* Re: [PATCH v3] staging: r8188eu: Use kzalloc() with GFP_ATOMIC in atomic context
  2021-11-05 15:18   ` Fabio M. De Francesco
@ 2021-11-05 15:36     ` Dan Carpenter
  2021-11-05 16:05       ` Fabio M. De Francesco
  0 siblings, 1 reply; 12+ messages in thread
From: Dan Carpenter @ 2021-11-05 15:36 UTC (permalink / raw)
  To: Fabio M. De Francesco
  Cc: Larry Finger, Phillip Potter, Greg Kroah-Hartman, linux-staging,
	linux-kernel

Oh yeah, you're right.  It never *just* does spinlocks (as stated in the
commit message btw), it does spin_lock_bh() which bumps the soft IRQ
count.

> To summarize, I think that using in_interrupt() in the old wrappers was the 
> wiser choice.

"Wiser" is not the right word.  The wrappers were always stupid, but I
guess they did work in this case so the fixes tag is correct.

regards,
dan carpenter


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

* Re: [PATCH v3] staging: r8188eu: Use kzalloc() with GFP_ATOMIC in atomic context
  2021-11-05 15:36     ` Dan Carpenter
@ 2021-11-05 16:05       ` Fabio M. De Francesco
  0 siblings, 0 replies; 12+ messages in thread
From: Fabio M. De Francesco @ 2021-11-05 16:05 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: Larry Finger, Phillip Potter, Greg Kroah-Hartman, linux-staging,
	linux-kernel

On Friday, November 5, 2021 4:36:33 PM CET Dan Carpenter wrote:
> Oh yeah, you're right.  It never *just* does spinlocks (as stated in the
> commit message btw), it does spin_lock_bh() which bumps the soft IRQ
> count.

Thank you very much for checking and confirming.
 
> > To summarize, I think that using in_interrupt() in the old wrappers was 
the 
> > wiser choice.
> 
> "Wiser" is not the right word.  The wrappers were always stupid, but I
> guess they did work in this case so the fixes tag is correct.

Ah, sorry. I was not able to express my thought properly :(

I agree with you that the wrappers were a not a good idea and Larry did well 
in removing them. Furthermore, I think that delegating the choice to use 
GFP_KERNEL vs. GFP_ATOMIC depending on the return from in_interrupt() is very 
bad design and it adds sensible overhead. 

I used "wiser" is a stricter sense. I meant that, if wrappers were needed 
(but they were not), in_interrupt() is "wiser" than "in_atomic()".

Regards,

Fabio M. De Francesco   




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

* Re: [PATCH v3] staging: r8188eu: Use kzalloc() with GFP_ATOMIC in atomic context
  2021-11-01 19:18 [PATCH v3] staging: r8188eu: Use kzalloc() with GFP_ATOMIC in atomic context Fabio M. De Francesco
  2021-11-05 13:25 ` Dan Carpenter
@ 2021-11-07 11:43 ` Fabio M. De Francesco
  2021-11-07 12:38   ` Greg Kroah-Hartman
  1 sibling, 1 reply; 12+ messages in thread
From: Fabio M. De Francesco @ 2021-11-07 11:43 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Larry Finger, Phillip Potter, linux-staging, linux-kernel

On Monday, November 1, 2021 8:18:47 PM CET Fabio M. De Francesco wrote:
> Use the GFP_ATOMIC flag of kzalloc() with two memory allocation in
> report_del_sta_event(). This function is called while holding spinlocks,
> therefore it is not allowed to sleep. With the GFP_ATOMIC type flag, the
> allocation is high priority and must not sleep.
> 
> This issue is detected by Smatch which emits the following warning:
> "drivers/staging/r8188eu/core/rtw_mlme_ext.c:6848 report_del_sta_event()
> warn: sleeping in atomic context".
> 
> After the change, the post-commit hook output the following message:
> "CHECK: Prefer kzalloc(sizeof(*pcmd_obj)...) over
> kzalloc(sizeof(struct cmd_obj)...)".
> 
> According to the above "CHECK", use the preferred style in the first
> kzalloc().
> 
> Fixes: 79f712ea994d ("staging: r8188eu: Remove wrappers for kalloc() and 
kzalloc()")
> Signed-off-by: Fabio M. De Francesco <fmdefrancesco@gmail.com>
> ---
> 
> v2->v3: Add the "Fixes:" tag, as requested by Greg Kroah-Hartman.
> 
> v1->v2: Fix an error that I introduced with an incorrect copy-paste
>         of the sizeof() operator.
> 
>  drivers/staging/r8188eu/core/rtw_mlme_ext.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

Hello Greg,

I've noticed that you have already applied recent changes to drivers/staging  
up to the patches of November 6th, but my patch is not among them. 

This patch has already been acked by Larry and I'm not sure if I should send 
a v4 with his "Acked-by" tag or if you can add it by yourself when applying 
to your tree.

Please let me know if there is something that prevents this patch to be 
applied. I have no problem in changing / adding whatever it is needed.

Thanks,

Fabio M. De Francesco



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

* Re: [PATCH v3] staging: r8188eu: Use kzalloc() with GFP_ATOMIC in atomic context
  2021-11-07 11:43 ` Fabio M. De Francesco
@ 2021-11-07 12:38   ` Greg Kroah-Hartman
  2021-11-07 13:15     ` Fabio M. De Francesco
  0 siblings, 1 reply; 12+ messages in thread
From: Greg Kroah-Hartman @ 2021-11-07 12:38 UTC (permalink / raw)
  To: Fabio M. De Francesco
  Cc: Larry Finger, Phillip Potter, linux-staging, linux-kernel

On Sun, Nov 07, 2021 at 12:43:51PM +0100, Fabio M. De Francesco wrote:
> On Monday, November 1, 2021 8:18:47 PM CET Fabio M. De Francesco wrote:
> > Use the GFP_ATOMIC flag of kzalloc() with two memory allocation in
> > report_del_sta_event(). This function is called while holding spinlocks,
> > therefore it is not allowed to sleep. With the GFP_ATOMIC type flag, the
> > allocation is high priority and must not sleep.
> > 
> > This issue is detected by Smatch which emits the following warning:
> > "drivers/staging/r8188eu/core/rtw_mlme_ext.c:6848 report_del_sta_event()
> > warn: sleeping in atomic context".
> > 
> > After the change, the post-commit hook output the following message:
> > "CHECK: Prefer kzalloc(sizeof(*pcmd_obj)...) over
> > kzalloc(sizeof(struct cmd_obj)...)".
> > 
> > According to the above "CHECK", use the preferred style in the first
> > kzalloc().
> > 
> > Fixes: 79f712ea994d ("staging: r8188eu: Remove wrappers for kalloc() and 
> kzalloc()")
> > Signed-off-by: Fabio M. De Francesco <fmdefrancesco@gmail.com>
> > ---
> > 
> > v2->v3: Add the "Fixes:" tag, as requested by Greg Kroah-Hartman.
> > 
> > v1->v2: Fix an error that I introduced with an incorrect copy-paste
> >         of the sizeof() operator.
> > 
> >  drivers/staging/r8188eu/core/rtw_mlme_ext.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> Hello Greg,
> 
> I've noticed that you have already applied recent changes to drivers/staging  
> up to the patches of November 6th, but my patch is not among them. 

I have applied patches that are not targeted for 5.16-final, yes.

> This patch has already been acked by Larry and I'm not sure if I should send 
> a v4 with his "Acked-by" tag or if you can add it by yourself when applying 
> to your tree.
> 
> Please let me know if there is something that prevents this patch to be 
> applied. I have no problem in changing / adding whatever it is needed.

Nothing needs to be done, I am waiting for 5.16-rc1 to be released
before I pick up this patch, and others that will be targeted for
5.16-final.  Only then will I queue them up, as the automated email you
should have gotten when you submitted the patch said would happen.

Just relax, there is no rush here :)

thanks,

greg k-h

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

* Re: [PATCH v3] staging: r8188eu: Use kzalloc() with GFP_ATOMIC in atomic context
  2021-11-07 12:38   ` Greg Kroah-Hartman
@ 2021-11-07 13:15     ` Fabio M. De Francesco
  2021-11-07 13:29       ` Greg Kroah-Hartman
  0 siblings, 1 reply; 12+ messages in thread
From: Fabio M. De Francesco @ 2021-11-07 13:15 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Larry Finger, Phillip Potter, linux-staging, linux-kernel

On Sunday, November 7, 2021 1:38:35 PM CET Greg Kroah-Hartman wrote:
> On Sun, Nov 07, 2021 at 12:43:51PM +0100, Fabio M. De Francesco wrote:
> > On Monday, November 1, 2021 8:18:47 PM CET Fabio M. De Francesco wrote:
> > > Use the GFP_ATOMIC flag of kzalloc() with two memory allocation in
> > > report_del_sta_event(). This function is called while holding 
spinlocks,
> > > therefore it is not allowed to sleep. With the GFP_ATOMIC type flag, 
the
> > > allocation is high priority and must not sleep.
> > > 
> > > This issue is detected by Smatch which emits the following warning:
> > > "drivers/staging/r8188eu/core/rtw_mlme_ext.c:6848 
report_del_sta_event()
> > > warn: sleeping in atomic context".
> > > 
> > > After the change, the post-commit hook output the following message:
> > > "CHECK: Prefer kzalloc(sizeof(*pcmd_obj)...) over
> > > kzalloc(sizeof(struct cmd_obj)...)".
> > > 
> > > According to the above "CHECK", use the preferred style in the first
> > > kzalloc().
> > > 
> > > Fixes: 79f712ea994d ("staging: r8188eu: Remove wrappers for kalloc() 
and 
> > kzalloc()")
> > > Signed-off-by: Fabio M. De Francesco <fmdefrancesco@gmail.com>
> > > ---

> > > [...]

> > Please let me know if there is something that prevents this patch to be 
> > applied. I have no problem in changing / adding whatever it is needed.
> 
> Nothing needs to be done, I am waiting for 5.16-rc1 to be released
> before I pick up this patch, and others that will be targeted for
> 5.16-final.  Only then will I queue them up, as the automated email you
> should have gotten when you submitted the patch said would happen.
> 
> Just relax, there is no rush here :)
> 

Oh, sorry Greg. There must be something that I haven't understand about the 
development process... :(

Obviously I agree that there is no rush here :)

As I said, this morning I read git log and saw patches that seemed more 
recent; thus I thought that was the case to ask. I just (wrongly) thought 
that the v3 of the patch got unnoticed or dropped  because of some requests  
that I had missed. 

Thanks for the explanation,

Fabio

> thanks,
> 
> greg k-h
> 





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

* Re: [PATCH v3] staging: r8188eu: Use kzalloc() with GFP_ATOMIC in atomic context
  2021-11-07 13:15     ` Fabio M. De Francesco
@ 2021-11-07 13:29       ` Greg Kroah-Hartman
  2021-11-07 14:03         ` Fabio M. De Francesco
  0 siblings, 1 reply; 12+ messages in thread
From: Greg Kroah-Hartman @ 2021-11-07 13:29 UTC (permalink / raw)
  To: Fabio M. De Francesco
  Cc: Larry Finger, Phillip Potter, linux-staging, linux-kernel

On Sun, Nov 07, 2021 at 02:15:59PM +0100, Fabio M. De Francesco wrote:
> On Sunday, November 7, 2021 1:38:35 PM CET Greg Kroah-Hartman wrote:
> > On Sun, Nov 07, 2021 at 12:43:51PM +0100, Fabio M. De Francesco wrote:
> > > On Monday, November 1, 2021 8:18:47 PM CET Fabio M. De Francesco wrote:
> > > > Use the GFP_ATOMIC flag of kzalloc() with two memory allocation in
> > > > report_del_sta_event(). This function is called while holding 
> spinlocks,
> > > > therefore it is not allowed to sleep. With the GFP_ATOMIC type flag, 
> the
> > > > allocation is high priority and must not sleep.
> > > > 
> > > > This issue is detected by Smatch which emits the following warning:
> > > > "drivers/staging/r8188eu/core/rtw_mlme_ext.c:6848 
> report_del_sta_event()
> > > > warn: sleeping in atomic context".
> > > > 
> > > > After the change, the post-commit hook output the following message:
> > > > "CHECK: Prefer kzalloc(sizeof(*pcmd_obj)...) over
> > > > kzalloc(sizeof(struct cmd_obj)...)".
> > > > 
> > > > According to the above "CHECK", use the preferred style in the first
> > > > kzalloc().
> > > > 
> > > > Fixes: 79f712ea994d ("staging: r8188eu: Remove wrappers for kalloc() 
> and 
> > > kzalloc()")
> > > > Signed-off-by: Fabio M. De Francesco <fmdefrancesco@gmail.com>
> > > > ---
> 
> > > > [...]
> 
> > > Please let me know if there is something that prevents this patch to be 
> > > applied. I have no problem in changing / adding whatever it is needed.
> > 
> > Nothing needs to be done, I am waiting for 5.16-rc1 to be released
> > before I pick up this patch, and others that will be targeted for
> > 5.16-final.  Only then will I queue them up, as the automated email you
> > should have gotten when you submitted the patch said would happen.
> > 
> > Just relax, there is no rush here :)
> > 
> 
> Oh, sorry Greg. There must be something that I haven't understand about the 
> development process... :(
> 
> Obviously I agree that there is no rush here :)
> 
> As I said, this morning I read git log and saw patches that seemed more 
> recent; thus I thought that was the case to ask. I just (wrongly) thought 
> that the v3 of the patch got unnoticed or dropped  because of some requests  
> that I had missed. 

Be sure to notice what branch commits are being applied to.  There are
different branches for a reason :)

thanks,

greg k-h

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

* Re: [PATCH v3] staging: r8188eu: Use kzalloc() with GFP_ATOMIC in atomic context
  2021-11-07 13:29       ` Greg Kroah-Hartman
@ 2021-11-07 14:03         ` Fabio M. De Francesco
  2021-11-07 14:17           ` Greg Kroah-Hartman
  0 siblings, 1 reply; 12+ messages in thread
From: Fabio M. De Francesco @ 2021-11-07 14:03 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Larry Finger, Phillip Potter, linux-staging, linux-kernel

On Sunday, November 7, 2021 2:29:47 PM CET Greg Kroah-Hartman wrote:
> On Sun, Nov 07, 2021 at 02:15:59PM +0100, Fabio M. De Francesco wrote:
> > On Sunday, November 7, 2021 1:38:35 PM CET Greg Kroah-Hartman wrote:
> > > On Sun, Nov 07, 2021 at 12:43:51PM +0100, Fabio M. De Francesco wrote:
> > > > On Monday, November 1, 2021 8:18:47 PM CET Fabio M. De Francesco 
wrote:
> > > > > Use the GFP_ATOMIC flag of kzalloc() with two memory allocation in
> > > > > report_del_sta_event(). This function is called while holding 
> > spinlocks,
> > > > > therefore it is not allowed to sleep. With the GFP_ATOMIC type 
flag, 
> > the
> > > > > allocation is high priority and must not sleep.
> > > > > 
> > > > > This issue is detected by Smatch which emits the following warning:
> > > > > "drivers/staging/r8188eu/core/rtw_mlme_ext.c:6848 
> > report_del_sta_event()
> > > > > warn: sleeping in atomic context".
> > > > > 
> > > > > After the change, the post-commit hook output the following 
message:
> > > > > "CHECK: Prefer kzalloc(sizeof(*pcmd_obj)...) over
> > > > > kzalloc(sizeof(struct cmd_obj)...)".
> > > > > 
> > > > > According to the above "CHECK", use the preferred style in the 
first
> > > > > kzalloc().
> > > > > 
> > > > > Fixes: 79f712ea994d ("staging: r8188eu: Remove wrappers for 
kalloc() 
> > and 
> > > > kzalloc()")
> > > > > Signed-off-by: Fabio M. De Francesco <fmdefrancesco@gmail.com>
> > > > > ---
> > 
> > > > > [...]
> > 
> > > > Please let me know if there is something that prevents this patch to 
be 
> > > > applied. I have no problem in changing / adding whatever it is 
needed.
> > > 
> > > Nothing needs to be done, I am waiting for 5.16-rc1 to be released
> > > before I pick up this patch, and others that will be targeted for
> > > 5.16-final.  Only then will I queue them up, as the automated email you
> > > should have gotten when you submitted the patch said would happen.
> > > 
> > > Just relax, there is no rush here :)
> > > 
> > 
> > Oh, sorry Greg. There must be something that I haven't understand about 
the 
> > development process... :(
> > 
> > Obviously I agree that there is no rush here :)
> > 
> > As I said, this morning I read git log and saw patches that seemed more 
> > recent; thus I thought that was the case to ask. I just (wrongly) thought 
> > that the v3 of the patch got unnoticed or dropped  because of some 
requests  
> > that I had missed. 
> 
> Be sure to notice what branch commits are being applied to.  There are
> different branches for a reason :)
> 
This is what confuses me:

--- git-log output ---

commit 8a893759d0075ea9556abcf86a4826d9865ba4bf (origin/staging-testing)
Author: Phillip Potter <phil@philpotter.co.uk>
Date:   Sat Nov 6 23:16:36 2021 +0000

    staging: r8188eu: remove MSG_88E macro

--- end of git-log output ---

Aside from the "Date" field, I know that this patch has been sent to the list 
during the last night and that it goes to the same branch (staging-testing) 
to which my patch should go. I know I'm still missing something, but I cannot 
understand what it is... :(

Anyway, never mind. I don't want to bother you with this silly questions :)

Again, thanks,

Fabio



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

* Re: [PATCH v3] staging: r8188eu: Use kzalloc() with GFP_ATOMIC in atomic context
  2021-11-07 14:03         ` Fabio M. De Francesco
@ 2021-11-07 14:17           ` Greg Kroah-Hartman
  2021-11-07 14:30             ` Fabio M. De Francesco
  0 siblings, 1 reply; 12+ messages in thread
From: Greg Kroah-Hartman @ 2021-11-07 14:17 UTC (permalink / raw)
  To: Fabio M. De Francesco
  Cc: Larry Finger, Phillip Potter, linux-staging, linux-kernel

On Sun, Nov 07, 2021 at 03:03:18PM +0100, Fabio M. De Francesco wrote:
> On Sunday, November 7, 2021 2:29:47 PM CET Greg Kroah-Hartman wrote:
> > On Sun, Nov 07, 2021 at 02:15:59PM +0100, Fabio M. De Francesco wrote:
> > > On Sunday, November 7, 2021 1:38:35 PM CET Greg Kroah-Hartman wrote:
> > > > On Sun, Nov 07, 2021 at 12:43:51PM +0100, Fabio M. De Francesco wrote:
> > > > > On Monday, November 1, 2021 8:18:47 PM CET Fabio M. De Francesco 
> wrote:
> > > > > > Use the GFP_ATOMIC flag of kzalloc() with two memory allocation in
> > > > > > report_del_sta_event(). This function is called while holding 
> > > spinlocks,
> > > > > > therefore it is not allowed to sleep. With the GFP_ATOMIC type 
> flag, 
> > > the
> > > > > > allocation is high priority and must not sleep.
> > > > > > 
> > > > > > This issue is detected by Smatch which emits the following warning:
> > > > > > "drivers/staging/r8188eu/core/rtw_mlme_ext.c:6848 
> > > report_del_sta_event()
> > > > > > warn: sleeping in atomic context".
> > > > > > 
> > > > > > After the change, the post-commit hook output the following 
> message:
> > > > > > "CHECK: Prefer kzalloc(sizeof(*pcmd_obj)...) over
> > > > > > kzalloc(sizeof(struct cmd_obj)...)".
> > > > > > 
> > > > > > According to the above "CHECK", use the preferred style in the 
> first
> > > > > > kzalloc().
> > > > > > 
> > > > > > Fixes: 79f712ea994d ("staging: r8188eu: Remove wrappers for 
> kalloc() 
> > > and 
> > > > > kzalloc()")
> > > > > > Signed-off-by: Fabio M. De Francesco <fmdefrancesco@gmail.com>
> > > > > > ---
> > > 
> > > > > > [...]
> > > 
> > > > > Please let me know if there is something that prevents this patch to 
> be 
> > > > > applied. I have no problem in changing / adding whatever it is 
> needed.
> > > > 
> > > > Nothing needs to be done, I am waiting for 5.16-rc1 to be released
> > > > before I pick up this patch, and others that will be targeted for
> > > > 5.16-final.  Only then will I queue them up, as the automated email you
> > > > should have gotten when you submitted the patch said would happen.
> > > > 
> > > > Just relax, there is no rush here :)
> > > > 
> > > 
> > > Oh, sorry Greg. There must be something that I haven't understand about 
> the 
> > > development process... :(
> > > 
> > > Obviously I agree that there is no rush here :)
> > > 
> > > As I said, this morning I read git log and saw patches that seemed more 
> > > recent; thus I thought that was the case to ask. I just (wrongly) thought 
> > > that the v3 of the patch got unnoticed or dropped  because of some 
> requests  
> > > that I had missed. 
> > 
> > Be sure to notice what branch commits are being applied to.  There are
> > different branches for a reason :)
> > 
> This is what confuses me:
> 
> --- git-log output ---
> 
> commit 8a893759d0075ea9556abcf86a4826d9865ba4bf (origin/staging-testing)
> Author: Phillip Potter <phil@philpotter.co.uk>
> Date:   Sat Nov 6 23:16:36 2021 +0000
> 
>     staging: r8188eu: remove MSG_88E macro
> 
> --- end of git-log output ---
> 
> Aside from the "Date" field, I know that this patch has been sent to the list 
> during the last night and that it goes to the same branch (staging-testing) 
> to which my patch should go. I know I'm still missing something, but I cannot 
> understand what it is... :(

No, your change will go to the staging-linus branch, as it needs to go
into 5.16-final and get sent to Linus much sooner than 5.17-rc1, which
is where things are being queued up in the staging-testing branch at the
moment.

hope this helps,

greg k-h

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

* Re: [PATCH v3] staging: r8188eu: Use kzalloc() with GFP_ATOMIC in atomic context
  2021-11-07 14:17           ` Greg Kroah-Hartman
@ 2021-11-07 14:30             ` Fabio M. De Francesco
  0 siblings, 0 replies; 12+ messages in thread
From: Fabio M. De Francesco @ 2021-11-07 14:30 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Larry Finger, Phillip Potter, linux-staging, linux-kernel

On Sunday, November 7, 2021 3:17:19 PM CET Greg Kroah-Hartman wrote:

> No, your change will go to the staging-linus branch, as it needs to go
> into 5.16-final and get sent to Linus much sooner than 5.17-rc1, which
> is where things are being queued up in the staging-testing branch at the
> moment.

Oh, I didn't even remotely guess that this kinds of patches usually go to the 
staging-linus branch so they get sent to Linus much sooner.

Thank you so much for your patience and for taking the time to explain the 
workflow to me.

Fabio

> 
> hope this helps,
> 
> greg k-h
> 





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

end of thread, other threads:[~2021-11-07 14:30 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-01 19:18 [PATCH v3] staging: r8188eu: Use kzalloc() with GFP_ATOMIC in atomic context Fabio M. De Francesco
2021-11-05 13:25 ` Dan Carpenter
2021-11-05 15:18   ` Fabio M. De Francesco
2021-11-05 15:36     ` Dan Carpenter
2021-11-05 16:05       ` Fabio M. De Francesco
2021-11-07 11:43 ` Fabio M. De Francesco
2021-11-07 12:38   ` Greg Kroah-Hartman
2021-11-07 13:15     ` Fabio M. De Francesco
2021-11-07 13:29       ` Greg Kroah-Hartman
2021-11-07 14:03         ` Fabio M. De Francesco
2021-11-07 14:17           ` Greg Kroah-Hartman
2021-11-07 14:30             ` Fabio M. De Francesco

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).