[v3,0/4] Add xxhash and zstd modules
diff mbox

Message ID 20170722113503.nf227bdrzx6637wl@angband.pl
State New
Headers show

Commit Message

Adam Borowski July 22, 2017, 11:35 a.m. UTC
On Fri, Jul 21, 2017 at 11:56:21AM -0400, Austin S. Hemmelgarn wrote:
> On 2017-07-20 17:27, Nick Terrell wrote:
> > This patch set adds xxhash, zstd compression, and zstd decompression
> > modules. It also adds zstd support to BtrFS and SquashFS.
> > 
> > Each patch has relevant summaries, benchmarks, and tests.
> 
> For patches 2-3, I've compile tested and had runtime testing running for
> about 18 hours now with no issues, so you can add:
> 
> Tested-by: Austin S. Hemmelgarn <ahferroin7@gmail.com>

I assume you haven't tried it on arm64, right?

I had no time to get 'round to it before, and just got the following build
failure:

  CC      fs/btrfs/zstd.o
In file included from fs/btrfs/zstd.c:28:0:
fs/btrfs/compression.h:39:2: error: unknown type name ‘refcount_t’
  refcount_t pending_bios;
  ^~~~~~~~~~
scripts/Makefile.build:302: recipe for target 'fs/btrfs/zstd.o' failed

It's trivially fixably by:
after which it works fine, although half an hour of testing isn't exactly
exhaustive.


Alas, the armhf machine I ran stress tests (Debian archive rebuilds) on
doesn't boot with 4.13-rc1 due to some unrelated regression, bisecting that
would be quite painful so I did not try yet.  I guess re-testing your patch
set on 4.12, even with btrfs-for-4.13 (which it had for a while), wouldn't
be of much help.  So far, previous versions have been running for weeks,
with no issue since you fixed workspace flickering.


On amd64 all is fine.


I haven't tested SquashFS at all.


Meow!

Comments

Austin S. Hemmelgarn July 24, 2017, 1:44 p.m. UTC | #1
On 2017-07-22 07:35, Adam Borowski wrote:
> On Fri, Jul 21, 2017 at 11:56:21AM -0400, Austin S. Hemmelgarn wrote:
>> On 2017-07-20 17:27, Nick Terrell wrote:
>>> This patch set adds xxhash, zstd compression, and zstd decompression
>>> modules. It also adds zstd support to BtrFS and SquashFS.
>>>
>>> Each patch has relevant summaries, benchmarks, and tests.
>>
>> For patches 2-3, I've compile tested and had runtime testing running for
>> about 18 hours now with no issues, so you can add:
>>
>> Tested-by: Austin S. Hemmelgarn <ahferroin7@gmail.com>
> 
> I assume you haven't tried it on arm64, right?
> 
> I had no time to get 'round to it before, and just got the following build
> failure:
> 
>    CC      fs/btrfs/zstd.o
> In file included from fs/btrfs/zstd.c:28:0:
> fs/btrfs/compression.h:39:2: error: unknown type name ‘refcount_t’
>    refcount_t pending_bios;
>    ^~~~~~~~~~
> scripts/Makefile.build:302: recipe for target 'fs/btrfs/zstd.o' failed
> 
> It's trivially fixably by:
> --- a/fs/btrfs/zstd.c
> +++ b/fs/btrfs/zstd.c
> @@ -24,6 +24,7 @@
>   #include <linux/sched.h>
>   #include <linux/pagemap.h>
>   #include <linux/bio.h>
> +#include <linux/refcount.h>
>   #include <linux/zstd.h>
>   #include "compression.h"
> 
> after which it works fine, although half an hour of testing isn't exactly
> exhaustive.
I did, and didn't hit this somehow...

Off to go verify my tool-chain and scripts then...
> 
> 
> Alas, the armhf machine I ran stress tests (Debian archive rebuilds) on
> doesn't boot with 4.13-rc1 due to some unrelated regression, bisecting that
> would be quite painful so I did not try yet.  I guess re-testing your patch
> set on 4.12, even with btrfs-for-4.13 (which it had for a while), wouldn't
> be of much help.  So far, previous versions have been running for weeks,
> with no issue since you fixed workspace flickering.
I also didn't see this, but I test on some seriously bare-bones 
configurations for both the 32-bit ARM tests I run.  On further 
inspection, it looks like my scripts decided to use btrfs-for-4.13 as 
the base, not 4.13-rc1 like I thought they did, so I don't know anymore 
how helpful my testing may have been.
> 
> 
> On amd64 all is fine.
> 
> 
> I haven't tested SquashFS at all.
> 
> 
> Meow!
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch
diff mbox

--- a/fs/btrfs/zstd.c
+++ b/fs/btrfs/zstd.c
@@ -24,6 +24,7 @@ 
 #include <linux/sched.h>
 #include <linux/pagemap.h>
 #include <linux/bio.h>
+#include <linux/refcount.h>
 #include <linux/zstd.h>
 #include "compression.h"