Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1710755 > unrolled thread
| Started by | SF Markus Elfring <elfring@users.sourceforge.net> |
|---|---|
| First post | 2017-08-14 11:40 +0200 |
| Last post | 2017-08-14 16:50 +0200 |
| Articles | 7 — 3 participants |
Back to article view | Back to linux.kernel
[PATCH 0/2] kmemleak: Adjustments for three function implementations SF Markus Elfring <elfring@users.sourceforge.net> - 2017-08-14 11:40 +0200
[PATCH 1/2] kmemleak: Delete an error message for a failed memory allocation in two functions SF Markus Elfring <elfring@users.sourceforge.net> - 2017-08-14 11:40 +0200
Re: [PATCH 1/2] kmemleak: Delete an error message for a failed memory allocation in two functions Catalin Marinas <catalin.marinas@arm.com> - 2017-08-14 13:20 +0200
Re: [PATCH 1/2] kmemleak: Delete an error message for a failed memory allocation in two functions SF Markus Elfring <elfring@users.sourceforge.net> - 2017-08-14 13:50 +0200
Re: [PATCH 1/2] kmemleak: Delete an error message for a failed memory allocation in two functions Dan Carpenter <dan.carpenter@oracle.com> - 2017-08-14 15:10 +0200
Re: [PATCH 1/2] kmemleak: Delete an error message for a failed memory allocation in two functions Catalin Marinas <catalin.marinas@arm.com> - 2017-08-14 16:40 +0200
Re: [PATCH 1/2] kmemleak: Delete an error message for a failed memory allocation in two functions Dan Carpenter <dan.carpenter@oracle.com> - 2017-08-14 16:50 +0200
| From | SF Markus Elfring <elfring@users.sourceforge.net> |
|---|---|
| Date | 2017-08-14 11:40 +0200 |
| Subject | [PATCH 0/2] kmemleak: Adjustments for three function implementations |
| Message-ID | <uek0N-2qo-23@gated-at.bofh.it> |
From: Markus Elfring <elfring@users.sourceforge.net> Date: Mon, 14 Aug 2017 11:30:22 +0200 Two update suggestions were taken into account from static source code analysis. Markus Elfring (2): Delete an error message for a failed memory allocation in two functions Use seq_puts() in print_unreferenced() mm/kmemleak.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) -- 2.14.0
[toc] | [next] | [standalone]
| From | SF Markus Elfring <elfring@users.sourceforge.net> |
|---|---|
| Date | 2017-08-14 11:40 +0200 |
| Subject | [PATCH 1/2] kmemleak: Delete an error message for a failed memory allocation in two functions |
| Message-ID | <uek0O-2qo-39@gated-at.bofh.it> |
| In reply to | #1710755 |
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Mon, 14 Aug 2017 10:50:22 +0200
Omit an extra message for a memory allocation failure in these functions.
This issue was detected by using the Coccinelle software.
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
mm/kmemleak.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/mm/kmemleak.c b/mm/kmemleak.c
index 7780cd83a495..c6c798d90b2e 100644
--- a/mm/kmemleak.c
+++ b/mm/kmemleak.c
@@ -555,7 +555,6 @@ static struct kmemleak_object *create_object(unsigned long ptr, size_t size,
object = kmem_cache_alloc(object_cache, gfp_kmemleak_mask(gfp));
if (!object) {
- pr_warn("Cannot allocate a kmemleak_object structure\n");
kmemleak_disable();
return NULL;
}
@@ -775,10 +774,8 @@ static void add_scan_area(unsigned long ptr, size_t size, gfp_t gfp)
}
area = kmem_cache_alloc(scan_area_cache, gfp_kmemleak_mask(gfp));
- if (!area) {
- pr_warn("Cannot allocate a scan area\n");
+ if (!area)
goto out;
- }
spin_lock_irqsave(&object->lock, flags);
if (size == SIZE_MAX) {
--
2.14.0
[toc] | [prev] | [next] | [standalone]
| From | Catalin Marinas <catalin.marinas@arm.com> |
|---|---|
| Date | 2017-08-14 13:20 +0200 |
| Subject | Re: [PATCH 1/2] kmemleak: Delete an error message for a failed memory allocation in two functions |
| Message-ID | <uelzA-3tE-7@gated-at.bofh.it> |
| In reply to | #1710757 |
On Mon, Aug 14, 2017 at 11:35:02AM +0200, SF Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Mon, 14 Aug 2017 10:50:22 +0200
>
> Omit an extra message for a memory allocation failure in these functions.
>
> This issue was detected by using the Coccinelle software.
>
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> ---
> mm/kmemleak.c | 5 +----
> 1 file changed, 1 insertion(+), 4 deletions(-)
>
> diff --git a/mm/kmemleak.c b/mm/kmemleak.c
> index 7780cd83a495..c6c798d90b2e 100644
> --- a/mm/kmemleak.c
> +++ b/mm/kmemleak.c
> @@ -555,7 +555,6 @@ static struct kmemleak_object *create_object(unsigned long ptr, size_t size,
>
> object = kmem_cache_alloc(object_cache, gfp_kmemleak_mask(gfp));
> if (!object) {
> - pr_warn("Cannot allocate a kmemleak_object structure\n");
> kmemleak_disable();
I don't really get what this patch is trying to achieve. Given that
kmemleak will be disabled after this, I'd rather know why it happened.
--
Catalin
[toc] | [prev] | [next] | [standalone]
| From | SF Markus Elfring <elfring@users.sourceforge.net> |
|---|---|
| Date | 2017-08-14 13:50 +0200 |
| Subject | Re: [PATCH 1/2] kmemleak: Delete an error message for a failed memory allocation in two functions |
| Message-ID | <uem2C-3DE-29@gated-at.bofh.it> |
| In reply to | #1710835 |
>> +++ b/mm/kmemleak.c
>> @@ -555,7 +555,6 @@ static struct kmemleak_object *create_object(unsigned long ptr, size_t size,
>>
>> object = kmem_cache_alloc(object_cache, gfp_kmemleak_mask(gfp));
>> if (!object) {
>> - pr_warn("Cannot allocate a kmemleak_object structure\n");
>> kmemleak_disable();
>
> I don't really get what this patch is trying to achieve.
I suggest to reduce the code size a bit.
> Given that kmemleak will be disabled after this,
I have got difficulties to interpret this information.
> I'd rather know why it happened.
Do you find the default allocation failure report sufficient?
Regards,
Markus
[toc] | [prev] | [next] | [standalone]
| From | Dan Carpenter <dan.carpenter@oracle.com> |
|---|---|
| Date | 2017-08-14 15:10 +0200 |
| Subject | Re: [PATCH 1/2] kmemleak: Delete an error message for a failed memory allocation in two functions |
| Message-ID | <ueni2-4zq-17@gated-at.bofh.it> |
| In reply to | #1710835 |
On Mon, Aug 14, 2017 at 12:14:32PM +0100, Catalin Marinas wrote:
> On Mon, Aug 14, 2017 at 11:35:02AM +0200, SF Markus Elfring wrote:
> > From: Markus Elfring <elfring@users.sourceforge.net>
> > Date: Mon, 14 Aug 2017 10:50:22 +0200
> >
> > Omit an extra message for a memory allocation failure in these functions.
> >
> > This issue was detected by using the Coccinelle software.
> >
> > Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> > ---
> > mm/kmemleak.c | 5 +----
> > 1 file changed, 1 insertion(+), 4 deletions(-)
> >
> > diff --git a/mm/kmemleak.c b/mm/kmemleak.c
> > index 7780cd83a495..c6c798d90b2e 100644
> > --- a/mm/kmemleak.c
> > +++ b/mm/kmemleak.c
> > @@ -555,7 +555,6 @@ static struct kmemleak_object *create_object(unsigned long ptr, size_t size,
> >
> > object = kmem_cache_alloc(object_cache, gfp_kmemleak_mask(gfp));
> > if (!object) {
> > - pr_warn("Cannot allocate a kmemleak_object structure\n");
> > kmemleak_disable();
>
> I don't really get what this patch is trying to achieve. Given that
> kmemleak will be disabled after this, I'd rather know why it happened.
kmem_cache_alloc() will generate a stack trace and a bunch of more
useful information if it fails. The allocation isn't likely to fail,
but if it does you will know. The extra message is just wasting RAM.
regards,
dan carpenter
[toc] | [prev] | [next] | [standalone]
| From | Catalin Marinas <catalin.marinas@arm.com> |
|---|---|
| Date | 2017-08-14 16:40 +0200 |
| Subject | Re: [PATCH 1/2] kmemleak: Delete an error message for a failed memory allocation in two functions |
| Message-ID | <ueoH8-5ij-13@gated-at.bofh.it> |
| In reply to | #1710909 |
On Mon, Aug 14, 2017 at 04:02:21PM +0300, Dan Carpenter wrote:
> On Mon, Aug 14, 2017 at 12:14:32PM +0100, Catalin Marinas wrote:
> > On Mon, Aug 14, 2017 at 11:35:02AM +0200, SF Markus Elfring wrote:
> > > From: Markus Elfring <elfring@users.sourceforge.net>
> > > Date: Mon, 14 Aug 2017 10:50:22 +0200
> > >
> > > Omit an extra message for a memory allocation failure in these functions.
> > >
> > > This issue was detected by using the Coccinelle software.
> > >
> > > Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> > > ---
> > > mm/kmemleak.c | 5 +----
> > > 1 file changed, 1 insertion(+), 4 deletions(-)
> > >
> > > diff --git a/mm/kmemleak.c b/mm/kmemleak.c
> > > index 7780cd83a495..c6c798d90b2e 100644
> > > --- a/mm/kmemleak.c
> > > +++ b/mm/kmemleak.c
> > > @@ -555,7 +555,6 @@ static struct kmemleak_object *create_object(unsigned long ptr, size_t size,
> > >
> > > object = kmem_cache_alloc(object_cache, gfp_kmemleak_mask(gfp));
> > > if (!object) {
> > > - pr_warn("Cannot allocate a kmemleak_object structure\n");
> > > kmemleak_disable();
> >
> > I don't really get what this patch is trying to achieve. Given that
> > kmemleak will be disabled after this, I'd rather know why it happened.
>
> kmem_cache_alloc() will generate a stack trace and a bunch of more
> useful information if it fails. The allocation isn't likely to fail,
> but if it does you will know. The extra message is just wasting RAM.
Currently kmemleak uses __GFP_NOWARN for its own metadata allocation, so
we wouldn't see the sl*b warnings. I don't fully remember why I went for
this gfp flag, probably not to interfere with other messages printed by
the allocator (kmemleak_alloc is called from within sl*b).
I'm fine to drop __GFP_NOWARN and remove those extra messages.
--
Catalin
[toc] | [prev] | [next] | [standalone]
| From | Dan Carpenter <dan.carpenter@oracle.com> |
|---|---|
| Date | 2017-08-14 16:50 +0200 |
| Subject | Re: [PATCH 1/2] kmemleak: Delete an error message for a failed memory allocation in two functions |
| Message-ID | <ueoQO-5lI-9@gated-at.bofh.it> |
| In reply to | #1710975 |
On Mon, Aug 14, 2017 at 03:38:04PM +0100, Catalin Marinas wrote:
> On Mon, Aug 14, 2017 at 04:02:21PM +0300, Dan Carpenter wrote:
> > On Mon, Aug 14, 2017 at 12:14:32PM +0100, Catalin Marinas wrote:
> > > On Mon, Aug 14, 2017 at 11:35:02AM +0200, SF Markus Elfring wrote:
> > > > From: Markus Elfring <elfring@users.sourceforge.net>
> > > > Date: Mon, 14 Aug 2017 10:50:22 +0200
> > > >
> > > > Omit an extra message for a memory allocation failure in these functions.
> > > >
> > > > This issue was detected by using the Coccinelle software.
> > > >
> > > > Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> > > > ---
> > > > mm/kmemleak.c | 5 +----
> > > > 1 file changed, 1 insertion(+), 4 deletions(-)
> > > >
> > > > diff --git a/mm/kmemleak.c b/mm/kmemleak.c
> > > > index 7780cd83a495..c6c798d90b2e 100644
> > > > --- a/mm/kmemleak.c
> > > > +++ b/mm/kmemleak.c
> > > > @@ -555,7 +555,6 @@ static struct kmemleak_object *create_object(unsigned long ptr, size_t size,
> > > >
> > > > object = kmem_cache_alloc(object_cache, gfp_kmemleak_mask(gfp));
> > > > if (!object) {
> > > > - pr_warn("Cannot allocate a kmemleak_object structure\n");
> > > > kmemleak_disable();
> > >
> > > I don't really get what this patch is trying to achieve. Given that
> > > kmemleak will be disabled after this, I'd rather know why it happened.
> >
> > kmem_cache_alloc() will generate a stack trace and a bunch of more
> > useful information if it fails. The allocation isn't likely to fail,
> > but if it does you will know. The extra message is just wasting RAM.
>
> Currently kmemleak uses __GFP_NOWARN for its own metadata allocation, so
> we wouldn't see the sl*b warnings. I don't fully remember why I went for
> this gfp flag, probably not to interfere with other messages printed by
> the allocator (kmemleak_alloc is called from within sl*b).
>
> I'm fine to drop __GFP_NOWARN and remove those extra messages.
>
Ah. I considered that, but didn't see a NOWARN in the diff and was too
lazy to check the code. Probably I would just leave it as-is.
regards,
dan carpenter
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web