| Message ID | 21b4d996c8861373ac77d427914ec7882fe0c83e.1314650069.git.joe@perches.com |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
On 08/29/2011 05:17 PM, Joe Perches wrote: > Removing unnecessary messages saves code and text. > > Site specific OOM messages are duplications of a generic MM > out of memory message and aren't really useful, so just > delete them. > > Signed-off-by: Joe Perches <joe@perches.com> Acked-by: Vlad Yasevich <vladislav.yasevich@hp.com> -vlad > --- > net/sctp/protocol.c | 3 --- > 1 files changed, 0 insertions(+), 3 deletions(-) > > diff --git a/net/sctp/protocol.c b/net/sctp/protocol.c > index 91784f4..0801444 100644 > --- a/net/sctp/protocol.c > +++ b/net/sctp/protocol.c > @@ -1326,7 +1326,6 @@ SCTP_STATIC __init int sctp_init(void) > __get_free_pages(GFP_ATOMIC|__GFP_NOWARN, order); > } while (!sctp_assoc_hashtable && --order > 0); > if (!sctp_assoc_hashtable) { > - pr_err("Failed association hash alloc\n"); > status = -ENOMEM; > goto err_ahash_alloc; > } > @@ -1340,7 +1339,6 @@ SCTP_STATIC __init int sctp_init(void) > sctp_ep_hashtable = (struct sctp_hashbucket *) > kmalloc(64 * sizeof(struct sctp_hashbucket), GFP_KERNEL); > if (!sctp_ep_hashtable) { > - pr_err("Failed endpoint_hash alloc\n"); > status = -ENOMEM; > goto err_ehash_alloc; > } > @@ -1359,7 +1357,6 @@ SCTP_STATIC __init int sctp_init(void) > __get_free_pages(GFP_ATOMIC|__GFP_NOWARN, order); > } while (!sctp_port_hashtable && --order > 0); > if (!sctp_port_hashtable) { > - pr_err("Failed bind hash alloc\n"); > status = -ENOMEM; > goto err_bhash_alloc; > } -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Le lundi 29 août 2011 à 14:17 -0700, Joe Perches a écrit : > Removing unnecessary messages saves code and text. > > Site specific OOM messages are duplications of a generic MM > out of memory message and aren't really useful, so just > delete them. > > Signed-off-by: Joe Perches <joe@perches.com> > --- > net/sctp/protocol.c | 3 --- > 1 files changed, 0 insertions(+), 3 deletions(-) > > diff --git a/net/sctp/protocol.c b/net/sctp/protocol.c > index 91784f4..0801444 100644 > --- a/net/sctp/protocol.c > +++ b/net/sctp/protocol.c > @@ -1326,7 +1326,6 @@ SCTP_STATIC __init int sctp_init(void) > __get_free_pages(GFP_ATOMIC|__GFP_NOWARN, order); > } while (!sctp_assoc_hashtable && --order > 0); > if (!sctp_assoc_hashtable) { > - pr_err("Failed association hash alloc\n"); > status = -ENOMEM; > goto err_ahash_alloc; > } > @@ -1340,7 +1339,6 @@ SCTP_STATIC __init int sctp_init(void) > sctp_ep_hashtable = (struct sctp_hashbucket *) > kmalloc(64 * sizeof(struct sctp_hashbucket), GFP_KERNEL); > if (!sctp_ep_hashtable) { > - pr_err("Failed endpoint_hash alloc\n"); > status = -ENOMEM; > goto err_ehash_alloc; > } > @@ -1359,7 +1357,6 @@ SCTP_STATIC __init int sctp_init(void) > __get_free_pages(GFP_ATOMIC|__GFP_NOWARN, order); > } while (!sctp_port_hashtable && --order > 0); > if (!sctp_port_hashtable) { > - pr_err("Failed bind hash alloc\n"); > status = -ENOMEM; > goto err_bhash_alloc; > } It would be nice if you could avoid all these patches, that you dont even read. As I already told you in the past, __GFP_NOWARN dont print generic OOM messages. Its not because I told Wang Shaoyan not adding a useless "pr_err("Out of memory\n");" in last gianfar patch, that you have to remove all messages, with one hundred or more patches. If I remember well, you even disagreed at that time. Furthermore, a failed vmalloc() is not guaranteed to emit an OOM message, is it ? -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Le lundi 29 août 2011 à 23:43 +0200, Eric Dumazet a écrit : > Furthermore, a failed vmalloc() is not guaranteed to emit an OOM > message, is it ? It currently displays a message without context : vmap allocation for size XXXXXX failed: use vmalloc=<size> to increase size. So we dont know which part of the kernel asked this allocation. Please dont remove existing error messages after failed vmalloc() calls. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
From: Eric Dumazet <eric.dumazet@gmail.com> Date: Mon, 29 Aug 2011 23:51:21 +0200 > Le lundi 29 août 2011 à 23:43 +0200, Eric Dumazet a écrit : > >> Furthermore, a failed vmalloc() is not guaranteed to emit an OOM >> message, is it ? > > It currently displays a message without context : > > vmap allocation for size XXXXXX failed: use vmalloc=<size> to increase > size. > > So we dont know which part of the kernel asked this allocation. > > Please dont remove existing error messages after failed vmalloc() calls. Indeed. Joe, these vmalloc() and also the __GFP_NOWARN cases will need to be attended to and this series resubmitted as such. Thanks. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, 2011-08-29 at 23:43 +0200, Eric Dumazet wrote: > Le lundi 29 août 2011 à 14:17 -0700, Joe Perches a écrit : > > Removing unnecessary messages saves code and text. > > Site specific OOM messages are duplications of a generic MM > > out of memory message and aren't really useful, so just > > delete them. > > Signed-off-by: Joe Perches <joe@perches.com> > > --- > > net/sctp/protocol.c | 3 --- > > 1 files changed, 0 insertions(+), 3 deletions(-) > > diff --git a/net/sctp/protocol.c b/net/sctp/protocol.c [] > > @@ -1326,7 +1326,6 @@ SCTP_STATIC __init int sctp_init(void) > > __get_free_pages(GFP_ATOMIC|__GFP_NOWARN, order); > > } while (!sctp_assoc_hashtable && --order > 0); > > if (!sctp_assoc_hashtable) { > > - pr_err("Failed association hash alloc\n"); > > status = -ENOMEM; > > goto err_ahash_alloc; > > } [] > > @@ -1359,7 +1357,6 @@ SCTP_STATIC __init int sctp_init(void) > > __get_free_pages(GFP_ATOMIC|__GFP_NOWARN, order); > > } while (!sctp_port_hashtable && --order > 0); > > if (!sctp_port_hashtable) { > > - pr_err("Failed bind hash alloc\n"); > > status = -ENOMEM; > > goto err_bhash_alloc; > > } > It would be nice if you could avoid all these patches, that you dont > even read. Didn't read is not the same thing as didn't notice. > As I already told you in the past, __GFP_NOWARN dont print generic OOM > messages. I didn't notice those had GFP_NOWARN. > Its not because I told Wang Shaoyan not adding a useless "pr_err("Out of > memory\n");" in last gianfar patch, that you have to remove all > messages, with one hundred or more patches. > If I remember well, you even disagreed at that time. No, what I said was that it'd be better to get agreement to delete them before deleting them. https://lkml.org/lkml/2011/8/9/379 So I submitted an RFC and cc'd you. You did not reply. https://lkml.org/lkml/2011/8/25/580 > Furthermore, a failed vmalloc() is not guaranteed to emit an OOM > message, is it ? Doesn't seem to be, perhaps it should be when __GFP_NOWARN is not set... -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, 2011-08-29 at 18:15 -0400, David Miller wrote: > From: Eric Dumazet <eric.dumazet@gmail.com> > Date: Mon, 29 Aug 2011 23:51:21 +0200 > > Le lundi 29 août 2011 à 23:43 +0200, Eric Dumazet a écrit : > >> Furthermore, a failed vmalloc() is not guaranteed to emit an OOM > >> message, is it ? > > It currently displays a message without context : > > vmap allocation for size XXXXXX failed: use vmalloc=<size> to increase > > size. > > So we dont know which part of the kernel asked this allocation. > > Please dont remove existing error messages after failed vmalloc() calls. > Indeed. > Joe, these vmalloc() and also the __GFP_NOWARN cases will need to be > attended to and this series resubmitted as such. No worries. Andrew Morton picked up a patch I posted that changes vmalloc to be similar to kmalloc when the pointer returned is NULL (OOM). It now uses dump_stack for those cases. https://patchwork.kernel.org/patch/1114682/ I'll keep all the current vmalloc failure messages for now and resubmit in a day or two this series with acks. Not batman or netfilter though as they were picked up by their maintainers. A month or two after the vmalloc patch hits mainline and/or wider testing, and it's deemed acceptable, removing vmalloc site specific OOM messages should be appropriate. Anyone object? I plan on submitting drivers/net OOM removals next week. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/net/sctp/protocol.c b/net/sctp/protocol.c index 91784f4..0801444 100644 --- a/net/sctp/protocol.c +++ b/net/sctp/protocol.c @@ -1326,7 +1326,6 @@ SCTP_STATIC __init int sctp_init(void) __get_free_pages(GFP_ATOMIC|__GFP_NOWARN, order); } while (!sctp_assoc_hashtable && --order > 0); if (!sctp_assoc_hashtable) { - pr_err("Failed association hash alloc\n"); status = -ENOMEM; goto err_ahash_alloc; } @@ -1340,7 +1339,6 @@ SCTP_STATIC __init int sctp_init(void) sctp_ep_hashtable = (struct sctp_hashbucket *) kmalloc(64 * sizeof(struct sctp_hashbucket), GFP_KERNEL); if (!sctp_ep_hashtable) { - pr_err("Failed endpoint_hash alloc\n"); status = -ENOMEM; goto err_ehash_alloc; } @@ -1359,7 +1357,6 @@ SCTP_STATIC __init int sctp_init(void) __get_free_pages(GFP_ATOMIC|__GFP_NOWARN, order); } while (!sctp_port_hashtable && --order > 0); if (!sctp_port_hashtable) { - pr_err("Failed bind hash alloc\n"); status = -ENOMEM; goto err_bhash_alloc; }
Removing unnecessary messages saves code and text. Site specific OOM messages are duplications of a generic MM out of memory message and aren't really useful, so just delete them. Signed-off-by: Joe Perches <joe@perches.com> --- net/sctp/protocol.c | 3 --- 1 files changed, 0 insertions(+), 3 deletions(-)