On Fri, Nov 25, 2016 at 10:51 AM, Vitaly Wool [off-list ref] wrote:

On Fri, Nov 25, 2016 at 9:41 AM, Arnd Bergmann [off-list ref] wrote:

quoted

On Friday, November 25, 2016 8:38:25 AM CET Vitaly Wool wrote:

quoted

quoted

quoted

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.

quoted

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

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>