linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Rui Salvaterra <rsalvaterra@gmail.com>
To: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
Cc: Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com>,
	Greg KH <gregkh@linuxfoundation.org>,
	linux-kernel@vger.kernel.org, eunb.song@samsung.com,
	minchan@kernel.org, linux-mm@kvack.org,
	Chanho Min <chanho.min@lge.com>,
	Kyungsik Lee <kyungsik.lee@lge.com>
Subject: Re: [BUG] lib: zram lz4 compression/decompression still broken on big endian
Date: Fri, 8 Apr 2016 15:53:30 +0100	[thread overview]
Message-ID: <CALjTZvYx+zYV0SHWR0=C+jhQ0M9BbeU0TRRuPDks_B4ZkZpVaA@mail.gmail.com> (raw)
In-Reply-To: <20160407140702.GB464@swordfish>

2016-04-07 15:07 GMT+01:00 Sergey Senozhatsky <sergey.senozhatsky@gmail.com>:
> On (04/07/16 13:33), Rui Salvaterra wrote:
> [..]
>> Hi again, Sergey
>
> Hello,
>
>> Thanks for the patch, I'll test it as soon as possible. I agree with
>> your second option, usually one selects lz4 when (especially
>> decompression) speed is paramount, so it needs all the help it can
>> get.
>
> thanks!
>
>> Speaking of fishy, the 64-bit detection code also looks suspiciously
>> bogus. Some of the identifiers don't even exist anywhere in the kernel
>> (__ppc64__, por example, after grepping all .c and .h files).
>> Shouldn't we instead check for CONFIG_64BIT or BITS_PER_LONG == 64?
>
> definitely a good question. personally, I'd prefer to test for
> CONFIG_64BIT only, looking at this hairy
>
>   /* Detects 64 bits mode */
>   #if (defined(__x86_64__) || defined(__x86_64) || defined(__amd64__) \
>          || defined(__ppc64__) || defined(__LP64__))
>
> and remove/rewrite a bunch of other stuff. but the thing with cleanups
> is that they don't fix anything, while potentially can introduce bugs.
> it's more risky to touch the stable code. /* well, removing those 'ghost'
> identifiers is sort of OK to me */. but that's just my opinion, I'll
> leave it to you and Greg.
>
>         -ss

Hi again, Sergey

I finally was able to test your patch but, as I suspected, it wasn't
enough. However, based on it, I was able to write a (hopefully)
correct one, which I'll send soon (tested on ppc64, with no
regressions on x86_64).

Thanks,

Rui

--
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>

      reply	other threads:[~2016-04-08 14:53 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-04-05 14:07 Rui Salvaterra
2016-04-05 15:34 ` Greg KH
2016-04-05 16:02   ` Rui Salvaterra
2016-04-06  5:33     ` Sergey Senozhatsky
2016-04-06  9:39       ` Rui Salvaterra
2016-04-06 13:09         ` Sergey Senozhatsky
2016-04-07 12:33           ` Rui Salvaterra
2016-04-07 14:07             ` Sergey Senozhatsky
2016-04-08 14:53               ` Rui Salvaterra [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CALjTZvYx+zYV0SHWR0=C+jhQ0M9BbeU0TRRuPDks_B4ZkZpVaA@mail.gmail.com' \
    --to=rsalvaterra@gmail.com \
    --cc=chanho.min@lge.com \
    --cc=eunb.song@samsung.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=kyungsik.lee@lge.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=minchan@kernel.org \
    --cc=sergey.senozhatsky.work@gmail.com \
    --cc=sergey.senozhatsky@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox