Re: [PATCH v2 01/10] DMA, CMA: clean-up log message
On Thu, Jun 12, 2014 at 11:53:16AM +0200, Michal Nazarewicz wrote: > On Thu, Jun 12 2014, Michal Nazarewicz wrote: > > I used “function(arg1, arg2, …)” at the *beginning* of functions when > > the arguments passed to the function were included in the message. In > > all other cases I left it at just “function:” (or just no additional > > prefix). IMO that's a reasonable strategy. > > At closer inspection, I realised drivers/base/dma-contiguous.c is > Marek's code, but the above I think is still reasonable thing to do, so > I'd rather standardise on having “function(…)” only at the beginning of > a function. Just my 0.02 CHF. Hello, Now, I realize that these changes aren't needed in this patchset, so I simplify this patch just to remove redundant 'CMA' prefix. Other things can be done after merging if we need. Thanks. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 01/10] DMA, CMA: clean-up log message
On Thu, Jun 12, 2014 at 11:53:16AM +0200, Michal Nazarewicz wrote: On Thu, Jun 12 2014, Michal Nazarewicz min...@mina86.com wrote: I used “function(arg1, arg2, …)” at the *beginning* of functions when the arguments passed to the function were included in the message. In all other cases I left it at just “function:” (or just no additional prefix). IMO that's a reasonable strategy. At closer inspection, I realised drivers/base/dma-contiguous.c is Marek's code, but the above I think is still reasonable thing to do, so I'd rather standardise on having “function(…)” only at the beginning of a function. Just my 0.02 CHF. Hello, Now, I realize that these changes aren't needed in this patchset, so I simplify this patch just to remove redundant 'CMA' prefix. Other things can be done after merging if we need. Thanks. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 01/10] DMA, CMA: clean-up log message
On Thu, Jun 12 2014, Michal Nazarewicz wrote: > I used “function(arg1, arg2, …)” at the *beginning* of functions when > the arguments passed to the function were included in the message. In > all other cases I left it at just “function:” (or just no additional > prefix). IMO that's a reasonable strategy. At closer inspection, I realised drivers/base/dma-contiguous.c is Marek's code, but the above I think is still reasonable thing to do, so I'd rather standardise on having “function(…)” only at the beginning of a function. Just my 0.02 CHF. -- Best regards, _ _ .o. | Liege of Serenely Enlightened Majesty of o' \,=./ `o ..o | Computer Science, Michał “mina86” Nazarewicz(o o) ooo +--ooO--(_)--Ooo-- -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 01/10] DMA, CMA: clean-up log message
On Thu, Jun 12 2014, Joonsoo Kim wrote:
> We don't need explicit 'CMA:' prefix, since we already define prefix
> 'cma:' in pr_fmt. So remove it.
>
> And, some logs print function name and others doesn't. This looks
> bad to me, so I unify log format to print function name consistently.
>
> Lastly, I add one more debug log on cma_activate_area().
I don't particularly care what format of logs you choose, so:
Acked-by: Michal Nazarewicz
even though I'd go without empty “()”.
> Signed-off-by: Joonsoo Kim
>
> diff --git a/drivers/base/dma-contiguous.c b/drivers/base/dma-contiguous.c
> index 83969f8..bd0bb81 100644
> --- a/drivers/base/dma-contiguous.c
> +++ b/drivers/base/dma-contiguous.c
> @@ -144,7 +144,7 @@ void __init dma_contiguous_reserve(phys_addr_t limit)
> }
>
> if (selected_size && !dma_contiguous_default_area) {
> - pr_debug("%s: reserving %ld MiB for global area\n", __func__,
> + pr_debug("%s(): reserving %ld MiB for global area\n", __func__,
>(unsigned long)selected_size / SZ_1M);
>
> dma_contiguous_reserve_area(selected_size, selected_base,
> @@ -163,8 +163,9 @@ static int __init cma_activate_area(struct cma *cma)
> unsigned i = cma->count >> pageblock_order;
> struct zone *zone;
>
> - cma->bitmap = kzalloc(bitmap_size, GFP_KERNEL);
> + pr_debug("%s()\n", __func__);
>
> + cma->bitmap = kzalloc(bitmap_size, GFP_KERNEL);
> if (!cma->bitmap)
> return -ENOMEM;
>
> @@ -234,7 +235,8 @@ int __init dma_contiguous_reserve_area(phys_addr_t size,
> phys_addr_t base,
>
> /* Sanity checks */
> if (cma_area_count == ARRAY_SIZE(cma_areas)) {
> - pr_err("Not enough slots for CMA reserved regions!\n");
> + pr_err("%s(): Not enough slots for CMA reserved regions!\n",
> + __func__);
> return -ENOSPC;
> }
>
> @@ -274,14 +276,15 @@ int __init dma_contiguous_reserve_area(phys_addr_t
> size, phys_addr_t base,
> *res_cma = cma;
> cma_area_count++;
>
> - pr_info("CMA: reserved %ld MiB at %08lx\n", (unsigned long)size / SZ_1M,
> - (unsigned long)base);
> + pr_info("%s(): reserved %ld MiB at %08lx\n",
> + __func__, (unsigned long)size / SZ_1M, (unsigned long)base);
>
> /* Architecture specific contiguous memory fixup. */
> dma_contiguous_early_fixup(base, size);
> return 0;
> err:
> - pr_err("CMA: failed to reserve %ld MiB\n", (unsigned long)size / SZ_1M);
> + pr_err("%s(): failed to reserve %ld MiB\n",
> + __func__, (unsigned long)size / SZ_1M);
> return ret;
> }
--
Best regards, _ _
.o. | Liege of Serenely Enlightened Majesty of o' \,=./ `o
..o | Computer Science, Michał “mina86” Nazarewicz(o o)
ooo +--ooO--(_)--Ooo--
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 01/10] DMA, CMA: clean-up log message
>> Joonsoo Kim writes:
>>
>> > We don't need explicit 'CMA:' prefix, since we already define prefix
>> > 'cma:' in pr_fmt. So remove it.
>> >
>> > And, some logs print function name and others doesn't. This looks
>> > bad to me, so I unify log format to print function name consistently.
>> >
>> > Lastly, I add one more debug log on cma_activate_area().
>> >
>> > Signed-off-by: Joonsoo Kim
>> >
>> > diff --git a/drivers/base/dma-contiguous.c b/drivers/base/dma-contiguous.c
>> > index 83969f8..bd0bb81 100644
>> > --- a/drivers/base/dma-contiguous.c
>> > +++ b/drivers/base/dma-contiguous.c
>> > @@ -144,7 +144,7 @@ void __init dma_contiguous_reserve(phys_addr_t limit)
>> >}
>> >
>> >if (selected_size && !dma_contiguous_default_area) {
>> > - pr_debug("%s: reserving %ld MiB for global area\n", __func__,
>> > + pr_debug("%s(): reserving %ld MiB for global area\n", __func__,
>> > (unsigned long)selected_size / SZ_1M);
> On Thu, Jun 12, 2014 at 10:11:19AM +0530, Aneesh Kumar K.V wrote:
>> Do we need to do function(), or just function:. I have seen the later
>> usage in other parts of the kernel.
On Thu, Jun 12 2014, Joonsoo Kim wrote:
> I also haven't seen this format in other kernel code, but, in cma, they use
> this format as following.
>
> function(arg1, arg2, ...): some message
>
> If we all dislike this format, we can change it after merging this
> patchset. Until then, it seems better to me to leave it as is.
I used “function(arg1, arg2, …)” at the *beginning* of functions when
the arguments passed to the function were included in the message. In
all other cases I left it at just “function:” (or just no additional
prefix). IMO that's a reasonable strategy.
--
Best regards, _ _
.o. | Liege of Serenely Enlightened Majesty of o' \,=./ `o
..o | Computer Science, Michał “mina86” Nazarewicz(o o)
ooo +--ooO--(_)--Ooo--
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 01/10] DMA, CMA: clean-up log message
On 06/12/2014 11:21 AM, Joonsoo Kim wrote:
> We don't need explicit 'CMA:' prefix, since we already define prefix
> 'cma:' in pr_fmt. So remove it.
>
> And, some logs print function name and others doesn't. This looks
> bad to me, so I unify log format to print function name consistently.
>
> Lastly, I add one more debug log on cma_activate_area().
>
> Signed-off-by: Joonsoo Kim
Reviewed-by: Zhang Yanfei
>
> diff --git a/drivers/base/dma-contiguous.c b/drivers/base/dma-contiguous.c
> index 83969f8..bd0bb81 100644
> --- a/drivers/base/dma-contiguous.c
> +++ b/drivers/base/dma-contiguous.c
> @@ -144,7 +144,7 @@ void __init dma_contiguous_reserve(phys_addr_t limit)
> }
>
> if (selected_size && !dma_contiguous_default_area) {
> - pr_debug("%s: reserving %ld MiB for global area\n", __func__,
> + pr_debug("%s(): reserving %ld MiB for global area\n", __func__,
>(unsigned long)selected_size / SZ_1M);
>
> dma_contiguous_reserve_area(selected_size, selected_base,
> @@ -163,8 +163,9 @@ static int __init cma_activate_area(struct cma *cma)
> unsigned i = cma->count >> pageblock_order;
> struct zone *zone;
>
> - cma->bitmap = kzalloc(bitmap_size, GFP_KERNEL);
> + pr_debug("%s()\n", __func__);
>
> + cma->bitmap = kzalloc(bitmap_size, GFP_KERNEL);
> if (!cma->bitmap)
> return -ENOMEM;
>
> @@ -234,7 +235,8 @@ int __init dma_contiguous_reserve_area(phys_addr_t size,
> phys_addr_t base,
>
> /* Sanity checks */
> if (cma_area_count == ARRAY_SIZE(cma_areas)) {
> - pr_err("Not enough slots for CMA reserved regions!\n");
> + pr_err("%s(): Not enough slots for CMA reserved regions!\n",
> + __func__);
> return -ENOSPC;
> }
>
> @@ -274,14 +276,15 @@ int __init dma_contiguous_reserve_area(phys_addr_t
> size, phys_addr_t base,
> *res_cma = cma;
> cma_area_count++;
>
> - pr_info("CMA: reserved %ld MiB at %08lx\n", (unsigned long)size / SZ_1M,
> - (unsigned long)base);
> + pr_info("%s(): reserved %ld MiB at %08lx\n",
> + __func__, (unsigned long)size / SZ_1M, (unsigned long)base);
>
> /* Architecture specific contiguous memory fixup. */
> dma_contiguous_early_fixup(base, size);
> return 0;
> err:
> - pr_err("CMA: failed to reserve %ld MiB\n", (unsigned long)size / SZ_1M);
> + pr_err("%s(): failed to reserve %ld MiB\n",
> + __func__, (unsigned long)size / SZ_1M);
> return ret;
> }
>
>
--
Thanks.
Zhang Yanfei
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 01/10] DMA, CMA: clean-up log message
On 06/12/2014 11:21 AM, Joonsoo Kim wrote:
We don't need explicit 'CMA:' prefix, since we already define prefix
'cma:' in pr_fmt. So remove it.
And, some logs print function name and others doesn't. This looks
bad to me, so I unify log format to print function name consistently.
Lastly, I add one more debug log on cma_activate_area().
Signed-off-by: Joonsoo Kim iamjoonsoo@lge.com
Reviewed-by: Zhang Yanfei zhangyan...@cn.fujitsu.com
diff --git a/drivers/base/dma-contiguous.c b/drivers/base/dma-contiguous.c
index 83969f8..bd0bb81 100644
--- a/drivers/base/dma-contiguous.c
+++ b/drivers/base/dma-contiguous.c
@@ -144,7 +144,7 @@ void __init dma_contiguous_reserve(phys_addr_t limit)
}
if (selected_size !dma_contiguous_default_area) {
- pr_debug(%s: reserving %ld MiB for global area\n, __func__,
+ pr_debug(%s(): reserving %ld MiB for global area\n, __func__,
(unsigned long)selected_size / SZ_1M);
dma_contiguous_reserve_area(selected_size, selected_base,
@@ -163,8 +163,9 @@ static int __init cma_activate_area(struct cma *cma)
unsigned i = cma-count pageblock_order;
struct zone *zone;
- cma-bitmap = kzalloc(bitmap_size, GFP_KERNEL);
+ pr_debug(%s()\n, __func__);
+ cma-bitmap = kzalloc(bitmap_size, GFP_KERNEL);
if (!cma-bitmap)
return -ENOMEM;
@@ -234,7 +235,8 @@ int __init dma_contiguous_reserve_area(phys_addr_t size,
phys_addr_t base,
/* Sanity checks */
if (cma_area_count == ARRAY_SIZE(cma_areas)) {
- pr_err(Not enough slots for CMA reserved regions!\n);
+ pr_err(%s(): Not enough slots for CMA reserved regions!\n,
+ __func__);
return -ENOSPC;
}
@@ -274,14 +276,15 @@ int __init dma_contiguous_reserve_area(phys_addr_t
size, phys_addr_t base,
*res_cma = cma;
cma_area_count++;
- pr_info(CMA: reserved %ld MiB at %08lx\n, (unsigned long)size / SZ_1M,
- (unsigned long)base);
+ pr_info(%s(): reserved %ld MiB at %08lx\n,
+ __func__, (unsigned long)size / SZ_1M, (unsigned long)base);
/* Architecture specific contiguous memory fixup. */
dma_contiguous_early_fixup(base, size);
return 0;
err:
- pr_err(CMA: failed to reserve %ld MiB\n, (unsigned long)size / SZ_1M);
+ pr_err(%s(): failed to reserve %ld MiB\n,
+ __func__, (unsigned long)size / SZ_1M);
return ret;
}
--
Thanks.
Zhang Yanfei
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 01/10] DMA, CMA: clean-up log message
Joonsoo Kim iamjoonsoo@lge.com writes:
We don't need explicit 'CMA:' prefix, since we already define prefix
'cma:' in pr_fmt. So remove it.
And, some logs print function name and others doesn't. This looks
bad to me, so I unify log format to print function name consistently.
Lastly, I add one more debug log on cma_activate_area().
Signed-off-by: Joonsoo Kim iamjoonsoo@lge.com
diff --git a/drivers/base/dma-contiguous.c b/drivers/base/dma-contiguous.c
index 83969f8..bd0bb81 100644
--- a/drivers/base/dma-contiguous.c
+++ b/drivers/base/dma-contiguous.c
@@ -144,7 +144,7 @@ void __init dma_contiguous_reserve(phys_addr_t limit)
}
if (selected_size !dma_contiguous_default_area) {
- pr_debug(%s: reserving %ld MiB for global area\n, __func__,
+ pr_debug(%s(): reserving %ld MiB for global area\n, __func__,
(unsigned long)selected_size / SZ_1M);
On Thu, Jun 12, 2014 at 10:11:19AM +0530, Aneesh Kumar K.V wrote:
Do we need to do function(), or just function:. I have seen the later
usage in other parts of the kernel.
On Thu, Jun 12 2014, Joonsoo Kim iamjoonsoo@lge.com wrote:
I also haven't seen this format in other kernel code, but, in cma, they use
this format as following.
function(arg1, arg2, ...): some message
If we all dislike this format, we can change it after merging this
patchset. Until then, it seems better to me to leave it as is.
I used “function(arg1, arg2, …)” at the *beginning* of functions when
the arguments passed to the function were included in the message. In
all other cases I left it at just “function:” (or just no additional
prefix). IMO that's a reasonable strategy.
--
Best regards, _ _
.o. | Liege of Serenely Enlightened Majesty of o' \,=./ `o
..o | Computer Science, Michał “mina86” Nazarewicz(o o)
ooo +--m...@google.com--xmpp:min...@jabber.org--ooO--(_)--Ooo--
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 01/10] DMA, CMA: clean-up log message
On Thu, Jun 12 2014, Joonsoo Kim iamjoonsoo@lge.com wrote:
We don't need explicit 'CMA:' prefix, since we already define prefix
'cma:' in pr_fmt. So remove it.
And, some logs print function name and others doesn't. This looks
bad to me, so I unify log format to print function name consistently.
Lastly, I add one more debug log on cma_activate_area().
I don't particularly care what format of logs you choose, so:
Acked-by: Michal Nazarewicz min...@mina86.com
even though I'd go without empty “()”.
Signed-off-by: Joonsoo Kim iamjoonsoo@lge.com
diff --git a/drivers/base/dma-contiguous.c b/drivers/base/dma-contiguous.c
index 83969f8..bd0bb81 100644
--- a/drivers/base/dma-contiguous.c
+++ b/drivers/base/dma-contiguous.c
@@ -144,7 +144,7 @@ void __init dma_contiguous_reserve(phys_addr_t limit)
}
if (selected_size !dma_contiguous_default_area) {
- pr_debug(%s: reserving %ld MiB for global area\n, __func__,
+ pr_debug(%s(): reserving %ld MiB for global area\n, __func__,
(unsigned long)selected_size / SZ_1M);
dma_contiguous_reserve_area(selected_size, selected_base,
@@ -163,8 +163,9 @@ static int __init cma_activate_area(struct cma *cma)
unsigned i = cma-count pageblock_order;
struct zone *zone;
- cma-bitmap = kzalloc(bitmap_size, GFP_KERNEL);
+ pr_debug(%s()\n, __func__);
+ cma-bitmap = kzalloc(bitmap_size, GFP_KERNEL);
if (!cma-bitmap)
return -ENOMEM;
@@ -234,7 +235,8 @@ int __init dma_contiguous_reserve_area(phys_addr_t size,
phys_addr_t base,
/* Sanity checks */
if (cma_area_count == ARRAY_SIZE(cma_areas)) {
- pr_err(Not enough slots for CMA reserved regions!\n);
+ pr_err(%s(): Not enough slots for CMA reserved regions!\n,
+ __func__);
return -ENOSPC;
}
@@ -274,14 +276,15 @@ int __init dma_contiguous_reserve_area(phys_addr_t
size, phys_addr_t base,
*res_cma = cma;
cma_area_count++;
- pr_info(CMA: reserved %ld MiB at %08lx\n, (unsigned long)size / SZ_1M,
- (unsigned long)base);
+ pr_info(%s(): reserved %ld MiB at %08lx\n,
+ __func__, (unsigned long)size / SZ_1M, (unsigned long)base);
/* Architecture specific contiguous memory fixup. */
dma_contiguous_early_fixup(base, size);
return 0;
err:
- pr_err(CMA: failed to reserve %ld MiB\n, (unsigned long)size / SZ_1M);
+ pr_err(%s(): failed to reserve %ld MiB\n,
+ __func__, (unsigned long)size / SZ_1M);
return ret;
}
--
Best regards, _ _
.o. | Liege of Serenely Enlightened Majesty of o' \,=./ `o
..o | Computer Science, Michał “mina86” Nazarewicz(o o)
ooo +--m...@google.com--xmpp:min...@jabber.org--ooO--(_)--Ooo--
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 01/10] DMA, CMA: clean-up log message
On Thu, Jun 12 2014, Michal Nazarewicz min...@mina86.com wrote: I used “function(arg1, arg2, …)” at the *beginning* of functions when the arguments passed to the function were included in the message. In all other cases I left it at just “function:” (or just no additional prefix). IMO that's a reasonable strategy. At closer inspection, I realised drivers/base/dma-contiguous.c is Marek's code, but the above I think is still reasonable thing to do, so I'd rather standardise on having “function(…)” only at the beginning of a function. Just my 0.02 CHF. -- Best regards, _ _ .o. | Liege of Serenely Enlightened Majesty of o' \,=./ `o ..o | Computer Science, Michał “mina86” Nazarewicz(o o) ooo +--m...@google.com--xmpp:min...@jabber.org--ooO--(_)--Ooo-- -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 01/10] DMA, CMA: clean-up log message
On Thu, Jun 12, 2014 at 02:18:53PM +0900, Minchan Kim wrote: > Hi Joonsoo, > > On Thu, Jun 12, 2014 at 12:21:38PM +0900, Joonsoo Kim wrote: > > We don't need explicit 'CMA:' prefix, since we already define prefix > > 'cma:' in pr_fmt. So remove it. > > > > And, some logs print function name and others doesn't. This looks > > bad to me, so I unify log format to print function name consistently. > > > > Lastly, I add one more debug log on cma_activate_area(). > > When I take a look, it just indicates cma_activate_area was called or not, > without what range for the area was reserved successfully so I couldn't see > the intention for new message. Description should explain it so that everybody > can agree on your claim. > Hello, I paste the answer in other thread. This pr_debug() comes from ppc kvm's kvm_cma_init_reserved_areas(). I want to maintain all log messages as much as possible to reduce confusion with this generalization. If I need to respin this patchset, I will explain more about it. Thanks. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 01/10] DMA, CMA: clean-up log message
On Thu, Jun 12, 2014 at 10:11:19AM +0530, Aneesh Kumar K.V wrote:
> Joonsoo Kim writes:
>
> > We don't need explicit 'CMA:' prefix, since we already define prefix
> > 'cma:' in pr_fmt. So remove it.
> >
> > And, some logs print function name and others doesn't. This looks
> > bad to me, so I unify log format to print function name consistently.
> >
> > Lastly, I add one more debug log on cma_activate_area().
> >
> > Signed-off-by: Joonsoo Kim
> >
> > diff --git a/drivers/base/dma-contiguous.c b/drivers/base/dma-contiguous.c
> > index 83969f8..bd0bb81 100644
> > --- a/drivers/base/dma-contiguous.c
> > +++ b/drivers/base/dma-contiguous.c
> > @@ -144,7 +144,7 @@ void __init dma_contiguous_reserve(phys_addr_t limit)
> > }
> >
> > if (selected_size && !dma_contiguous_default_area) {
> > - pr_debug("%s: reserving %ld MiB for global area\n", __func__,
> > + pr_debug("%s(): reserving %ld MiB for global area\n", __func__,
> > (unsigned long)selected_size / SZ_1M);
>
> Do we need to do function(), or just function:. I have seen the later
> usage in other parts of the kernel.
Hello,
I also haven't seen this format in other kernel code, but, in cma, they use
this format as following.
function(arg1, arg2, ...): some message
If we all dislike this format, we can change it after merging this
patchset. Until then, it seems better to me to leave it as is.
>
> >
> > dma_contiguous_reserve_area(selected_size, selected_base,
> > @@ -163,8 +163,9 @@ static int __init cma_activate_area(struct cma *cma)
> > unsigned i = cma->count >> pageblock_order;
> > struct zone *zone;
> >
> > - cma->bitmap = kzalloc(bitmap_size, GFP_KERNEL);
> > + pr_debug("%s()\n", __func__);
>
> why ?
>
This pr_debug() comes from ppc kvm's kvm_cma_init_reserved_areas().
I want to maintain all log messages as much as possible to reduce confusion
with this generalization.
Thanks.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 01/10] DMA, CMA: clean-up log message
Hi Joonsoo,
On Thu, Jun 12, 2014 at 12:21:38PM +0900, Joonsoo Kim wrote:
> We don't need explicit 'CMA:' prefix, since we already define prefix
> 'cma:' in pr_fmt. So remove it.
>
> And, some logs print function name and others doesn't. This looks
> bad to me, so I unify log format to print function name consistently.
>
> Lastly, I add one more debug log on cma_activate_area().
When I take a look, it just indicates cma_activate_area was called or not,
without what range for the area was reserved successfully so I couldn't see
the intention for new message. Description should explain it so that everybody
can agree on your claim.
>
> Signed-off-by: Joonsoo Kim
>
> diff --git a/drivers/base/dma-contiguous.c b/drivers/base/dma-contiguous.c
> index 83969f8..bd0bb81 100644
> --- a/drivers/base/dma-contiguous.c
> +++ b/drivers/base/dma-contiguous.c
> @@ -144,7 +144,7 @@ void __init dma_contiguous_reserve(phys_addr_t limit)
> }
>
> if (selected_size && !dma_contiguous_default_area) {
> - pr_debug("%s: reserving %ld MiB for global area\n", __func__,
> + pr_debug("%s(): reserving %ld MiB for global area\n", __func__,
>(unsigned long)selected_size / SZ_1M);
>
> dma_contiguous_reserve_area(selected_size, selected_base,
> @@ -163,8 +163,9 @@ static int __init cma_activate_area(struct cma *cma)
> unsigned i = cma->count >> pageblock_order;
> struct zone *zone;
>
> - cma->bitmap = kzalloc(bitmap_size, GFP_KERNEL);
> + pr_debug("%s()\n", __func__);
>
> + cma->bitmap = kzalloc(bitmap_size, GFP_KERNEL);
> if (!cma->bitmap)
> return -ENOMEM;
>
> @@ -234,7 +235,8 @@ int __init dma_contiguous_reserve_area(phys_addr_t size,
> phys_addr_t base,
>
> /* Sanity checks */
> if (cma_area_count == ARRAY_SIZE(cma_areas)) {
> - pr_err("Not enough slots for CMA reserved regions!\n");
> + pr_err("%s(): Not enough slots for CMA reserved regions!\n",
> + __func__);
> return -ENOSPC;
> }
>
> @@ -274,14 +276,15 @@ int __init dma_contiguous_reserve_area(phys_addr_t
> size, phys_addr_t base,
> *res_cma = cma;
> cma_area_count++;
>
> - pr_info("CMA: reserved %ld MiB at %08lx\n", (unsigned long)size / SZ_1M,
> - (unsigned long)base);
> + pr_info("%s(): reserved %ld MiB at %08lx\n",
> + __func__, (unsigned long)size / SZ_1M, (unsigned long)base);
>
> /* Architecture specific contiguous memory fixup. */
> dma_contiguous_early_fixup(base, size);
> return 0;
> err:
> - pr_err("CMA: failed to reserve %ld MiB\n", (unsigned long)size / SZ_1M);
> + pr_err("%s(): failed to reserve %ld MiB\n",
> + __func__, (unsigned long)size / SZ_1M);
> return ret;
> }
>
> --
> 1.7.9.5
--
Kind regards,
Minchan Kim
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 01/10] DMA, CMA: clean-up log message
Joonsoo Kim writes:
> We don't need explicit 'CMA:' prefix, since we already define prefix
> 'cma:' in pr_fmt. So remove it.
>
> And, some logs print function name and others doesn't. This looks
> bad to me, so I unify log format to print function name consistently.
>
> Lastly, I add one more debug log on cma_activate_area().
>
> Signed-off-by: Joonsoo Kim
>
> diff --git a/drivers/base/dma-contiguous.c b/drivers/base/dma-contiguous.c
> index 83969f8..bd0bb81 100644
> --- a/drivers/base/dma-contiguous.c
> +++ b/drivers/base/dma-contiguous.c
> @@ -144,7 +144,7 @@ void __init dma_contiguous_reserve(phys_addr_t limit)
> }
>
> if (selected_size && !dma_contiguous_default_area) {
> - pr_debug("%s: reserving %ld MiB for global area\n", __func__,
> + pr_debug("%s(): reserving %ld MiB for global area\n", __func__,
>(unsigned long)selected_size / SZ_1M);
Do we need to do function(), or just function:. I have seen the later
usage in other parts of the kernel.
>
> dma_contiguous_reserve_area(selected_size, selected_base,
> @@ -163,8 +163,9 @@ static int __init cma_activate_area(struct cma *cma)
> unsigned i = cma->count >> pageblock_order;
> struct zone *zone;
>
> - cma->bitmap = kzalloc(bitmap_size, GFP_KERNEL);
> + pr_debug("%s()\n", __func__);
why ?
>
> + cma->bitmap = kzalloc(bitmap_size, GFP_KERNEL);
> if (!cma->bitmap)
> return -ENOMEM;
>
> @@ -234,7 +235,8 @@ int __init dma_contiguous_reserve_area(phys_addr_t size,
> phys_addr_t base,
>
> /* Sanity checks */
> if (cma_area_count == ARRAY_SIZE(cma_areas)) {
> - pr_err("Not enough slots for CMA reserved regions!\n");
> + pr_err("%s(): Not enough slots for CMA reserved regions!\n",
> + __func__);
> return -ENOSPC;
> }
>
> @@ -274,14 +276,15 @@ int __init dma_contiguous_reserve_area(phys_addr_t
> size, phys_addr_t base,
> *res_cma = cma;
> cma_area_count++;
>
> - pr_info("CMA: reserved %ld MiB at %08lx\n", (unsigned long)size / SZ_1M,
> - (unsigned long)base);
> + pr_info("%s(): reserved %ld MiB at %08lx\n",
> + __func__, (unsigned long)size / SZ_1M, (unsigned long)base);
>
> /* Architecture specific contiguous memory fixup. */
> dma_contiguous_early_fixup(base, size);
> return 0;
> err:
> - pr_err("CMA: failed to reserve %ld MiB\n", (unsigned long)size / SZ_1M);
> + pr_err("%s(): failed to reserve %ld MiB\n",
> + __func__, (unsigned long)size / SZ_1M);
> return ret;
> }
>
> --
> 1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 01/10] DMA, CMA: clean-up log message
Joonsoo Kim iamjoonsoo@lge.com writes:
We don't need explicit 'CMA:' prefix, since we already define prefix
'cma:' in pr_fmt. So remove it.
And, some logs print function name and others doesn't. This looks
bad to me, so I unify log format to print function name consistently.
Lastly, I add one more debug log on cma_activate_area().
Signed-off-by: Joonsoo Kim iamjoonsoo@lge.com
diff --git a/drivers/base/dma-contiguous.c b/drivers/base/dma-contiguous.c
index 83969f8..bd0bb81 100644
--- a/drivers/base/dma-contiguous.c
+++ b/drivers/base/dma-contiguous.c
@@ -144,7 +144,7 @@ void __init dma_contiguous_reserve(phys_addr_t limit)
}
if (selected_size !dma_contiguous_default_area) {
- pr_debug(%s: reserving %ld MiB for global area\n, __func__,
+ pr_debug(%s(): reserving %ld MiB for global area\n, __func__,
(unsigned long)selected_size / SZ_1M);
Do we need to do function(), or just function:. I have seen the later
usage in other parts of the kernel.
dma_contiguous_reserve_area(selected_size, selected_base,
@@ -163,8 +163,9 @@ static int __init cma_activate_area(struct cma *cma)
unsigned i = cma-count pageblock_order;
struct zone *zone;
- cma-bitmap = kzalloc(bitmap_size, GFP_KERNEL);
+ pr_debug(%s()\n, __func__);
why ?
+ cma-bitmap = kzalloc(bitmap_size, GFP_KERNEL);
if (!cma-bitmap)
return -ENOMEM;
@@ -234,7 +235,8 @@ int __init dma_contiguous_reserve_area(phys_addr_t size,
phys_addr_t base,
/* Sanity checks */
if (cma_area_count == ARRAY_SIZE(cma_areas)) {
- pr_err(Not enough slots for CMA reserved regions!\n);
+ pr_err(%s(): Not enough slots for CMA reserved regions!\n,
+ __func__);
return -ENOSPC;
}
@@ -274,14 +276,15 @@ int __init dma_contiguous_reserve_area(phys_addr_t
size, phys_addr_t base,
*res_cma = cma;
cma_area_count++;
- pr_info(CMA: reserved %ld MiB at %08lx\n, (unsigned long)size / SZ_1M,
- (unsigned long)base);
+ pr_info(%s(): reserved %ld MiB at %08lx\n,
+ __func__, (unsigned long)size / SZ_1M, (unsigned long)base);
/* Architecture specific contiguous memory fixup. */
dma_contiguous_early_fixup(base, size);
return 0;
err:
- pr_err(CMA: failed to reserve %ld MiB\n, (unsigned long)size / SZ_1M);
+ pr_err(%s(): failed to reserve %ld MiB\n,
+ __func__, (unsigned long)size / SZ_1M);
return ret;
}
--
1.7.9.5
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 01/10] DMA, CMA: clean-up log message
Hi Joonsoo,
On Thu, Jun 12, 2014 at 12:21:38PM +0900, Joonsoo Kim wrote:
We don't need explicit 'CMA:' prefix, since we already define prefix
'cma:' in pr_fmt. So remove it.
And, some logs print function name and others doesn't. This looks
bad to me, so I unify log format to print function name consistently.
Lastly, I add one more debug log on cma_activate_area().
When I take a look, it just indicates cma_activate_area was called or not,
without what range for the area was reserved successfully so I couldn't see
the intention for new message. Description should explain it so that everybody
can agree on your claim.
Signed-off-by: Joonsoo Kim iamjoonsoo@lge.com
diff --git a/drivers/base/dma-contiguous.c b/drivers/base/dma-contiguous.c
index 83969f8..bd0bb81 100644
--- a/drivers/base/dma-contiguous.c
+++ b/drivers/base/dma-contiguous.c
@@ -144,7 +144,7 @@ void __init dma_contiguous_reserve(phys_addr_t limit)
}
if (selected_size !dma_contiguous_default_area) {
- pr_debug(%s: reserving %ld MiB for global area\n, __func__,
+ pr_debug(%s(): reserving %ld MiB for global area\n, __func__,
(unsigned long)selected_size / SZ_1M);
dma_contiguous_reserve_area(selected_size, selected_base,
@@ -163,8 +163,9 @@ static int __init cma_activate_area(struct cma *cma)
unsigned i = cma-count pageblock_order;
struct zone *zone;
- cma-bitmap = kzalloc(bitmap_size, GFP_KERNEL);
+ pr_debug(%s()\n, __func__);
+ cma-bitmap = kzalloc(bitmap_size, GFP_KERNEL);
if (!cma-bitmap)
return -ENOMEM;
@@ -234,7 +235,8 @@ int __init dma_contiguous_reserve_area(phys_addr_t size,
phys_addr_t base,
/* Sanity checks */
if (cma_area_count == ARRAY_SIZE(cma_areas)) {
- pr_err(Not enough slots for CMA reserved regions!\n);
+ pr_err(%s(): Not enough slots for CMA reserved regions!\n,
+ __func__);
return -ENOSPC;
}
@@ -274,14 +276,15 @@ int __init dma_contiguous_reserve_area(phys_addr_t
size, phys_addr_t base,
*res_cma = cma;
cma_area_count++;
- pr_info(CMA: reserved %ld MiB at %08lx\n, (unsigned long)size / SZ_1M,
- (unsigned long)base);
+ pr_info(%s(): reserved %ld MiB at %08lx\n,
+ __func__, (unsigned long)size / SZ_1M, (unsigned long)base);
/* Architecture specific contiguous memory fixup. */
dma_contiguous_early_fixup(base, size);
return 0;
err:
- pr_err(CMA: failed to reserve %ld MiB\n, (unsigned long)size / SZ_1M);
+ pr_err(%s(): failed to reserve %ld MiB\n,
+ __func__, (unsigned long)size / SZ_1M);
return ret;
}
--
1.7.9.5
--
Kind regards,
Minchan Kim
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 01/10] DMA, CMA: clean-up log message
On Thu, Jun 12, 2014 at 10:11:19AM +0530, Aneesh Kumar K.V wrote:
Joonsoo Kim iamjoonsoo@lge.com writes:
We don't need explicit 'CMA:' prefix, since we already define prefix
'cma:' in pr_fmt. So remove it.
And, some logs print function name and others doesn't. This looks
bad to me, so I unify log format to print function name consistently.
Lastly, I add one more debug log on cma_activate_area().
Signed-off-by: Joonsoo Kim iamjoonsoo@lge.com
diff --git a/drivers/base/dma-contiguous.c b/drivers/base/dma-contiguous.c
index 83969f8..bd0bb81 100644
--- a/drivers/base/dma-contiguous.c
+++ b/drivers/base/dma-contiguous.c
@@ -144,7 +144,7 @@ void __init dma_contiguous_reserve(phys_addr_t limit)
}
if (selected_size !dma_contiguous_default_area) {
- pr_debug(%s: reserving %ld MiB for global area\n, __func__,
+ pr_debug(%s(): reserving %ld MiB for global area\n, __func__,
(unsigned long)selected_size / SZ_1M);
Do we need to do function(), or just function:. I have seen the later
usage in other parts of the kernel.
Hello,
I also haven't seen this format in other kernel code, but, in cma, they use
this format as following.
function(arg1, arg2, ...): some message
If we all dislike this format, we can change it after merging this
patchset. Until then, it seems better to me to leave it as is.
dma_contiguous_reserve_area(selected_size, selected_base,
@@ -163,8 +163,9 @@ static int __init cma_activate_area(struct cma *cma)
unsigned i = cma-count pageblock_order;
struct zone *zone;
- cma-bitmap = kzalloc(bitmap_size, GFP_KERNEL);
+ pr_debug(%s()\n, __func__);
why ?
This pr_debug() comes from ppc kvm's kvm_cma_init_reserved_areas().
I want to maintain all log messages as much as possible to reduce confusion
with this generalization.
Thanks.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 01/10] DMA, CMA: clean-up log message
On Thu, Jun 12, 2014 at 02:18:53PM +0900, Minchan Kim wrote: Hi Joonsoo, On Thu, Jun 12, 2014 at 12:21:38PM +0900, Joonsoo Kim wrote: We don't need explicit 'CMA:' prefix, since we already define prefix 'cma:' in pr_fmt. So remove it. And, some logs print function name and others doesn't. This looks bad to me, so I unify log format to print function name consistently. Lastly, I add one more debug log on cma_activate_area(). When I take a look, it just indicates cma_activate_area was called or not, without what range for the area was reserved successfully so I couldn't see the intention for new message. Description should explain it so that everybody can agree on your claim. Hello, I paste the answer in other thread. This pr_debug() comes from ppc kvm's kvm_cma_init_reserved_areas(). I want to maintain all log messages as much as possible to reduce confusion with this generalization. If I need to respin this patchset, I will explain more about it. Thanks. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
