Re: [PATCH] z3fold: use %z modifier for format string

[Dan Streetman]

On Fri, Nov 25, 2016 at 10:51 AM, Vitaly Wool <vitalywool@xxxxxxxxx> wrote:
> On Fri, Nov 25, 2016 at 9:41 AM, Arnd Bergmann <arnd@xxxxxxxx> 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