All of lore.kernel.org
 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 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.