Printing a size_t requires the %zd format rather than %d:
mm/z3fold.c: In function ‘init_z3fold’:
include/linux/kern_levels.h:4:18: error: format ‘%d’ expects argument of type ‘int’, but argument 2 has type ‘long unsigned int’ [-Werror=format=]
Fixes: 50a50d2676c4 ("z3fold: don't fail kernel build if z3fold_header is too big")
Signed-off-by: Arnd Bergmann <[email protected]>
---
mm/z3fold.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/mm/z3fold.c b/mm/z3fold.c
index e282ba073e77..66ac7a7dc934 100644
--- a/mm/z3fold.c
+++ b/mm/z3fold.c
@@ -884,7 +884,7 @@ static int __init init_z3fold(void)
{
/* Fail the initialization if z3fold header won't fit in one chunk */
if (sizeof(struct z3fold_header) > ZHDR_SIZE_ALIGNED) {
- pr_err("z3fold: z3fold_header size (%d) is bigger than "
+ pr_err("z3fold: z3fold_header size (%zd) is bigger than "
"the chunk size (%d), can't proceed\n",
sizeof(struct z3fold_header) , ZHDR_SIZE_ALIGNED);
return -E2BIG;
--
2.9.0
On Thu, 2016-11-24 at 17:31 +0100, Arnd Bergmann wrote:
> Printing a size_t requires the %zd format rather than %d:
>
> mm/z3fold.c: In function ‘init_z3fold’:
> include/linux/kern_levels.h:4:18: error: format ‘%d’ expects argument of type ‘int’, but argument 2 has type ‘long unsigned int’ [-Werror=format=]
>
> Fixes: 50a50d2676c4 ("z3fold: don't fail kernel build if z3fold_header is too big")
> Signed-off-by: Arnd Bergmann <[email protected]>
> ---
> mm/z3fold.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/mm/z3fold.c b/mm/z3fold.c
> index e282ba073e77..66ac7a7dc934 100644
> --- a/mm/z3fold.c
> +++ b/mm/z3fold.c
> @@ -884,7 +884,7 @@ static int __init init_z3fold(void)
> {
> /* Fail the initialization if z3fold header won't fit in one chunk */
> if (sizeof(struct z3fold_header) > ZHDR_SIZE_ALIGNED) {
> - pr_err("z3fold: z3fold_header size (%d) is bigger than "
> + pr_err("z3fold: z3fold_header size (%zd) is bigger than "
> "the chunk size (%d), can't proceed\n",
> sizeof(struct z3fold_header) , ZHDR_SIZE_ALIGNED);
> return -E2BIG;
The embedded "z3fold: " prefix here should be removed
as there's a pr_fmt that also adds it.
The test looks like it should be a BUILD_BUG_ON rather
than any runtime test too.
Hi Arnd,
On Thu, Nov 24, 2016 at 5:31 PM, Arnd Bergmann <[email protected]> wrote:
> Printing a size_t requires the %zd format rather than %d:
>
> mm/z3fold.c: In function ‘init_z3fold’:
> include/linux/kern_levels.h:4:18: error: format ‘%d’ expects argument of type ‘int’, but argument 2 has type ‘long unsigned int’ [-Werror=format=]
>
> Fixes: 50a50d2676c4 ("z3fold: don't fail kernel build if z3fold_header is too big")
> Signed-off-by: Arnd Bergmann <[email protected]>
Acked-by: Vitaly Wool <[email protected]>
And thanks :)
~vitaly
> ---
> mm/z3fold.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/mm/z3fold.c b/mm/z3fold.c
> index e282ba073e77..66ac7a7dc934 100644
> --- a/mm/z3fold.c
> +++ b/mm/z3fold.c
> @@ -884,7 +884,7 @@ static int __init init_z3fold(void)
> {
> /* Fail the initialization if z3fold header won't fit in one chunk */
> if (sizeof(struct z3fold_header) > ZHDR_SIZE_ALIGNED) {
> - pr_err("z3fold: z3fold_header size (%d) is bigger than "
> + pr_err("z3fold: z3fold_header size (%zd) is bigger than "
> "the chunk size (%d), can't proceed\n",
> sizeof(struct z3fold_header) , ZHDR_SIZE_ALIGNED);
> return -E2BIG;
> --
> 2.9.0
>
Hi Joe,
On Thu, Nov 24, 2016 at 6:08 PM, Joe Perches <[email protected]> wrote:
> On Thu, 2016-11-24 at 17:31 +0100, Arnd Bergmann wrote:
>> Printing a size_t requires the %zd format rather than %d:
>>
>> mm/z3fold.c: In function ‘init_z3fold’:
>> include/linux/kern_levels.h:4:18: error: format ‘%d’ expects argument of type ‘int’, but argument 2 has type ‘long unsigned int’ [-Werror=format=]
>>
>> Fixes: 50a50d2676c4 ("z3fold: don't fail kernel build if z3fold_header is too big")
>> Signed-off-by: Arnd Bergmann <[email protected]>
>> ---
>> mm/z3fold.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/mm/z3fold.c b/mm/z3fold.c
>> index e282ba073e77..66ac7a7dc934 100644
>> --- a/mm/z3fold.c
>> +++ b/mm/z3fold.c
>> @@ -884,7 +884,7 @@ static int __init init_z3fold(void)
>> {
>> /* Fail the initialization if z3fold header won't fit in one chunk */
>> if (sizeof(struct z3fold_header) > ZHDR_SIZE_ALIGNED) {
>> - pr_err("z3fold: z3fold_header size (%d) is bigger than "
>> + pr_err("z3fold: z3fold_header size (%zd) is bigger than "
>> "the chunk size (%d), can't proceed\n",
>> sizeof(struct z3fold_header) , ZHDR_SIZE_ALIGNED);
>> return -E2BIG;
>
> The embedded "z3fold: " prefix here should be removed
> as there's a pr_fmt that also adds it.
>
> The test looks like it should be a BUILD_BUG_ON rather
> than any runtime test too.
It used to be BUILD_BUG_ON but we deliberately changed that because
sizeof(spinlock_t) gets bloated in debug builds, so it just won't
build with default CHUNK_SIZE.
~vitaly
On Friday, November 25, 2016 8:38:25 AM CET Vitaly Wool wrote:
> >> diff --git a/mm/z3fold.c b/mm/z3fold.c
> >> index e282ba073e77..66ac7a7dc934 100644
> >> --- a/mm/z3fold.c
> >> +++ b/mm/z3fold.c
> >> @@ -884,7 +884,7 @@ static int __init init_z3fold(void)
> >> {
> >> /* Fail the initialization if z3fold header won't fit in one chunk */
> >> if (sizeof(struct z3fold_header) > ZHDR_SIZE_ALIGNED) {
> >> - pr_err("z3fold: z3fold_header size (%d) is bigger than "
> >> + pr_err("z3fold: z3fold_header size (%zd) is bigger than "
> >> "the chunk size (%d), can't proceed\n",
> >> sizeof(struct z3fold_header) , ZHDR_SIZE_ALIGNED);
> >> return -E2BIG;
> >
> > The embedded "z3fold: " prefix here should be removed
> > as there's a pr_fmt that also adds it.
> >
> > The test looks like it should be a BUILD_BUG_ON rather
> > than any runtime test too.
>
> It used to be BUILD_BUG_ON but we deliberately changed that because
> sizeof(spinlock_t) gets bloated in debug builds, so it just won't
> build with default CHUNK_SIZE.
Could this be improved by making the CHUNK_SIZE bigger depending on
the debug options?
Alternatively, how about using a bit_spin_lock instead of raw_spin_lock?
That would guarantee a fixed size for the lock and make z3fold_header
always 24 bytes (on 32-bit architectures) or 40 bytes
(on 64-bit architectures). You could even play some tricks with the
first_num field to make it fit in the same word as the lock and make the
structure fit into 32 bytes if you care about that.
Arnd
On Fri, Nov 25, 2016 at 9:41 AM, Arnd Bergmann <[email protected]> wrote:
> On Friday, November 25, 2016 8:38:25 AM CET Vitaly Wool wrote:
>> >> diff --git a/mm/z3fold.c b/mm/z3fold.c
>> >> index e282ba073e77..66ac7a7dc934 100644
>> >> --- a/mm/z3fold.c
>> >> +++ b/mm/z3fold.c
>> >> @@ -884,7 +884,7 @@ static int __init init_z3fold(void)
>> >> {
>> >> /* Fail the initialization if z3fold header won't fit in one chunk */
>> >> if (sizeof(struct z3fold_header) > ZHDR_SIZE_ALIGNED) {
>> >> - pr_err("z3fold: z3fold_header size (%d) is bigger than "
>> >> + pr_err("z3fold: z3fold_header size (%zd) is bigger than "
>> >> "the chunk size (%d), can't proceed\n",
>> >> sizeof(struct z3fold_header) , ZHDR_SIZE_ALIGNED);
>> >> return -E2BIG;
>> >
>> > The embedded "z3fold: " prefix here should be removed
>> > as there's a pr_fmt that also adds it.
>> >
>> > The test looks like it should be a BUILD_BUG_ON rather
>> > than any runtime test too.
>>
>> It used to be BUILD_BUG_ON but we deliberately changed that because
>> sizeof(spinlock_t) gets bloated in debug builds, so it just won't
>> build with default CHUNK_SIZE.
>
> Could this be improved by making the CHUNK_SIZE bigger depending on
> the debug options?
I don't see how silently enforcing a suboptimal configuration is
better than failing the initialization (so that you can adjust
CHUNK_SIZE yourself). I can add something descriptive to
Documentation/vm/z3fold.txt for that matter.
> Alternatively, how about using a bit_spin_lock instead of raw_spin_lock?
> That would guarantee a fixed size for the lock and make z3fold_header
> always 24 bytes (on 32-bit architectures) or 40 bytes
> (on 64-bit architectures). You could even play some tricks with the
> first_num field to make it fit in the same word as the lock and make the
> structure fit into 32 bytes if you care about that.
That is interesting. Actually I can have that bit in page->private and
then I don't need to handle headless pages in a special way, that
sounds appealing. However, there is a warning about bit_spin_lock
performance penalty. Do you know how big it is?
Best regards,
Vitaly
On Friday, November 25, 2016 4:51:03 PM CET Vitaly Wool wrote:
> On Fri, Nov 25, 2016 at 9:41 AM, Arnd Bergmann <[email protected]> wrote:
> > On Friday, November 25, 2016 8:38:25 AM CET Vitaly Wool wrote:
> >> >> diff --git a/mm/z3fold.c b/mm/z3fold.c
> >> >> index e282ba073e77..66ac7a7dc934 100644
> >> >> --- a/mm/z3fold.c
> >> >> +++ b/mm/z3fold.c
> >> >> @@ -884,7 +884,7 @@ static int __init init_z3fold(void)
> >> >> {
> >> >> /* Fail the initialization if z3fold header won't fit in one chunk */
> >> >> if (sizeof(struct z3fold_header) > ZHDR_SIZE_ALIGNED) {
> >> >> - pr_err("z3fold: z3fold_header size (%d) is bigger than "
> >> >> + pr_err("z3fold: z3fold_header size (%zd) is bigger than "
> >> >> "the chunk size (%d), can't proceed\n",
> >> >> sizeof(struct z3fold_header) , ZHDR_SIZE_ALIGNED);
> >> >> return -E2BIG;
> >> >
> >> > The embedded "z3fold: " prefix here should be removed
> >> > as there's a pr_fmt that also adds it.
> >> >
> >> > The test looks like it should be a BUILD_BUG_ON rather
> >> > than any runtime test too.
> >>
> >> It used to be BUILD_BUG_ON but we deliberately changed that because
> >> sizeof(spinlock_t) gets bloated in debug builds, so it just won't
> >> build with default CHUNK_SIZE.
> >
> > Could this be improved by making the CHUNK_SIZE bigger depending on
> > the debug options?
>
> I don't see how silently enforcing a suboptimal configuration is
> better than failing the initialization (so that you can adjust
> CHUNK_SIZE yourself). I can add something descriptive to
> Documentation/vm/z3fold.txt for that matter.
Failing at runtime when you know it's broken at compile-time
seems wrong, too. If you can't use z3fold with spinlock debugging,
you may as well hide the option in Kconfig based on the other ones.
Printing a runtime warning for the suboptimal configuration but
making it work anyway is probably better than just failing.
> > Alternatively, how about using a bit_spin_lock instead of raw_spin_lock?
> > That would guarantee a fixed size for the lock and make z3fold_header
> > always 24 bytes (on 32-bit architectures) or 40 bytes
> > (on 64-bit architectures). You could even play some tricks with the
> > first_num field to make it fit in the same word as the lock and make the
> > structure fit into 32 bytes if you care about that.
>
> That is interesting. Actually I can have that bit in page->private and
> then I don't need to handle headless pages in a special way, that
> sounds appealing. However, there is a warning about bit_spin_lock
> performance penalty. Do you know how big it is?
No idea, sorry. On x86, test_and_set_bit() seems to be only
one instruction to test/set the bit, followed by a conditional
branch, compared to a cmpxchg() for the raw_spin_lock(), so the
fast path seems pretty much the same.
Arnd
On Fri, Nov 25, 2016 at 10:51 AM, Vitaly Wool <[email protected]> wrote:
> On Fri, Nov 25, 2016 at 9:41 AM, Arnd Bergmann <[email protected]> wrote:
>> On Friday, November 25, 2016 8:38:25 AM CET Vitaly Wool wrote:
>>> >> diff --git a/mm/z3fold.c b/mm/z3fold.c
>>> >> index e282ba073e77..66ac7a7dc934 100644
>>> >> --- a/mm/z3fold.c
>>> >> +++ b/mm/z3fold.c
>>> >> @@ -884,7 +884,7 @@ static int __init init_z3fold(void)
>>> >> {
>>> >> /* Fail the initialization if z3fold header won't fit in one chunk */
>>> >> if (sizeof(struct z3fold_header) > ZHDR_SIZE_ALIGNED) {
>>> >> - pr_err("z3fold: z3fold_header size (%d) is bigger than "
>>> >> + pr_err("z3fold: z3fold_header size (%zd) is bigger than "
>>> >> "the chunk size (%d), can't proceed\n",
>>> >> sizeof(struct z3fold_header) , ZHDR_SIZE_ALIGNED);
>>> >> return -E2BIG;
>>> >
>>> > The embedded "z3fold: " prefix here should be removed
>>> > as there's a pr_fmt that also adds it.
>>> >
>>> > The test looks like it should be a BUILD_BUG_ON rather
>>> > than any runtime test too.
>>>
>>> It used to be BUILD_BUG_ON but we deliberately changed that because
>>> sizeof(spinlock_t) gets bloated in debug builds, so it just won't
>>> build with default CHUNK_SIZE.
>>
>> Could this be improved by making the CHUNK_SIZE bigger depending on
>> the debug options?
>
> I don't see how silently enforcing a suboptimal configuration is
> better than failing the initialization (so that you can adjust
> CHUNK_SIZE yourself). I can add something descriptive to
> Documentation/vm/z3fold.txt for that matter.
>
>> Alternatively, how about using a bit_spin_lock instead of raw_spin_lock?
>> That would guarantee a fixed size for the lock and make z3fold_header
>> always 24 bytes (on 32-bit architectures) or 40 bytes
>> (on 64-bit architectures). You could even play some tricks with the
>> first_num field to make it fit in the same word as the lock and make the
>> structure fit into 32 bytes if you care about that.
>
> That is interesting. Actually I can have that bit in page->private and
> then I don't need to handle headless pages in a special way, that
> sounds appealing. However, there is a warning about bit_spin_lock
> performance penalty. Do you know how big it is?
all these patches you're sending are to improve performance...why
would we then use bit spinlocks that degrade performance? let's just
calculate the zhdr size correctly instead of assuming it's always <
chunk size.
>
> Best regards,
> Vitaly