Re: Problematic culture around Signed-off-by

[Adam Borowski]

On Sun, Jul 30, 2017 at 08:52:36PM +0200, Pavel Machek wrote:
> > I've been away from kernel development for a bit, but I've returned and
> > I'm troubled by what seems to be an entrenched and widespread (IMO)
> > misuse of the "Signed-off-by:" in commits.
> > 
> > I've now either been asked to sign off RFC quality patches "because its
> > quicker" on more than one occasion in the last week or so, and I've seen
> > others signing off code which clearly has no hope of going anywhere near
> > the kernel. (eg. // commented out lines)
> > 
> > I was of the impression that Signed-off-by: was intended to be used on
> > essentially *finished* commits, indicating both readiness for inclusion
> > upstream and ones ownership of the copyright.
> > 
> > Even if the intent is *purely* a copyright isue, Signing off
> > *everything* surely makes it far too easy for people to get junk into
> > the kernel.
> 
> I normally sign-off everything... because getting patch without
> sign-off is nasty. If maintainer gets unclean, but signed-off patch,
> he can just clean it up, add his sign-off and continue normally.

Yet there are cases with known but unobvious breakage (see below).

> That may or may not be allowed if patch is not signed-off. (We are in
> lawyer teritory now.)
> 
> So I'd recommend signing everything, and if patch is considered "not
> ready", make it clear in some other way.

I think it'd be much better if you could suggest a new marker.  Something
like "Copyright-but-not-Readiness-Signed-off-by:", "RFC-Signed-off-by:",
"WIP-Signed-off-by:", etc.

And having a common, agreed upon marker would avoid confusion.

For example, I'm currently messing with a patch set that appears to give a
nice speed-up for the O_PONIES issue, but it messes with parts of the kernel
I'm really unfamiliar with.  Moreover, I know of data-loss scenarios in my
current version (I have naive ideas for a fix), but these scenarios are
irrelevant for a RFC.  Thus, it's imperative these patches do not get into
any non-experimental tree -- yet it's also likely that, if the concept
proves as good as it seems, that one of kernel devs with more skill would
take my work and beat it into sanity.

Cases like this just scream for a "Here-be-Dragons-but-Signed-off-Copyright-
Wise:" line.


Meow!
-- 
⢀⣴⠾⠻⢶⣦⠀ What Would Jesus Do, MUD/MMORPG edition:
⣾⠁⢰⠒⠀⣿⡁ • multiplay with an admin char to benefit your mortal
⢿⡄⠘⠷⠚⠋⠀ • abuse item cloning bugs (the five fishes + two breads affair)
⠈⠳⣄ • use glitches to walk on water

MikeeUSA warning (Re: Yes you have standing to sue GRSecurity.)

[Adam Borowski]

Note that this is quite clearly yet another of MikeeUSA's sockpuppets.  And
you guys really don't want to be caught in another of his troll threads.

Yeah, GRsecurity is a problem, but don't let our dear Mikee milk it.


-- 
⢀⣴⠾⠻⢶⣦⠀ What Would Jesus Do, MUD/MMORPG edition:
⣾⠁⢰⠒⠀⣿⡁ • multiplay with an admin char to benefit your mortal
⢿⡄⠘⠷⠚⠋⠀ • abuse item cloning bugs (the five fishes + two breads affair)
⠈⠳⣄ • use glitches to walk on water

MikeeUSA warning (Re: Yes you have standing to sue GRSecurity.)

[Adam Borowski]

Note that this is quite clearly yet another of MikeeUSA's sockpuppets.  And
you guys really don't want to be caught in another of his troll threads.

Yeah, GRsecurity is a problem, but don't let our dear Mikee milk it.


-- 
⢀⣴⠾⠻⢶⣦⠀ What Would Jesus Do, MUD/MMORPG edition:
⣾⠁⢰⠒⠀⣿⡁ • multiplay with an admin char to benefit your mortal
⢿⡄⠘⠷⠚⠋⠀ • abuse item cloning bugs (the five fishes + two breads affair)
⠈⠳⣄ • use glitches to walk on water

Re: [PATCH 0/3] Add ethernet0 alias for several A64 boards

[Adam Borowski]

On Tue, Jul 25, 2017 at 11:04:24AM +0800, icen...@aosc.io wrote:
> 在 2017-07-24 15:58，Maxime Ripard 写道：
> > On Sat, Jul 22, 2017 at 10:28:49AM +0800, Icenowy Zheng wrote:
> > > Allwinner A64 SoC has an EMAC which is used to provide Ethernet
> > > function on several boards.
> > > 
> > > The EMAC itself doesn't have a fixed MAC address, but the sunxi
> > > mainline U-Boot have the ability to generate one based on the eFUSE
> > > SID in the chip, and add the generated MAC address to the device
> > > tree when booting.
> > > 
> > > The MAC address setting step is based on the device tree's aliases,
> > > and device tree nodes prefixed "ethernet" will get the MAC address
> > > added. However, in several A64 boards' device tree, the alias is not
> > > set up, so that the U-Boot won't set the MAC address.
> > > 
> > > Add the ethernet0 aliases to these boards.
> > > 
> > > I hope this patchset can be queued in 4.13, otherwise 4.13 kernels
> > > won't get non-volatile MAC addresses, and will use random ones
> > > instead, which is annoying to many users.
> > > 
> > > Icenowy Zheng (3):
> > >   arm64: allwinner: a64: add ethernet0 alias for BPi M64 EMAC node
> > >   arm64: allwinner: a64: add ethernet0 alias for Pine64 EMAC node
> > >   arm64: allwinner: a64: add ethernet0 alias for SoPine EMAC node
> > 
> > Applied all three, thanks!
> 
> Sorry, but could you queue them to 4.13?
> 
> Otherwise 4.13 kernel release will have annoying random MAC problem,
> which heavily affects headless usages.

Perhaps it would be better to reword the commit subject as "fix missing
ethernet0 alias ..."?  That'd convey that the previous behaviour is a defect
that needs these patches as a fix.


Meow!
-- 
// If you believe in so-called "intellectual property", please immediately
// cease using counterfeit alphabets.  Instead, contact the nearest temple
// of Amon, whose priests will provide you with scribal services for all
// your writing needs, for Reasonable and Non-Discriminatory prices.

Re: [PATCH 0/3] Add ethernet0 alias for several A64 boards

[Adam Borowski]

On Tue, Jul 25, 2017 at 11:04:24AM +0800, icen...@aosc.io wrote:
> 在 2017-07-24 15:58，Maxime Ripard 写道：
> > On Sat, Jul 22, 2017 at 10:28:49AM +0800, Icenowy Zheng wrote:
> > > Allwinner A64 SoC has an EMAC which is used to provide Ethernet
> > > function on several boards.
> > > 
> > > The EMAC itself doesn't have a fixed MAC address, but the sunxi
> > > mainline U-Boot have the ability to generate one based on the eFUSE
> > > SID in the chip, and add the generated MAC address to the device
> > > tree when booting.
> > > 
> > > The MAC address setting step is based on the device tree's aliases,
> > > and device tree nodes prefixed "ethernet" will get the MAC address
> > > added. However, in several A64 boards' device tree, the alias is not
> > > set up, so that the U-Boot won't set the MAC address.
> > > 
> > > Add the ethernet0 aliases to these boards.
> > > 
> > > I hope this patchset can be queued in 4.13, otherwise 4.13 kernels
> > > won't get non-volatile MAC addresses, and will use random ones
> > > instead, which is annoying to many users.
> > > 
> > > Icenowy Zheng (3):
> > >   arm64: allwinner: a64: add ethernet0 alias for BPi M64 EMAC node
> > >   arm64: allwinner: a64: add ethernet0 alias for Pine64 EMAC node
> > >   arm64: allwinner: a64: add ethernet0 alias for SoPine EMAC node
> > 
> > Applied all three, thanks!
> 
> Sorry, but could you queue them to 4.13?
> 
> Otherwise 4.13 kernel release will have annoying random MAC problem,
> which heavily affects headless usages.

Perhaps it would be better to reword the commit subject as "fix missing
ethernet0 alias ..."?  That'd convey that the previous behaviour is a defect
that needs these patches as a fix.


Meow!
-- 
// If you believe in so-called "intellectual property", please immediately
// cease using counterfeit alphabets.  Instead, contact the nearest temple
// of Amon, whose priests will provide you with scribal services for all
// your writing needs, for Reasonable and Non-Discriminatory prices.

Re: [PATCH v3 0/4] Add xxhash and zstd modules

[Adam Borowski]

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 

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 
 #include 
 #include 
+#include 
 #include 
 #include "compression.h"

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!
-- 
⢀⣴⠾⠻⢶⣦⠀ 
⣾⠁⢠⠒⠀⣿⡁ A dumb species has no way to open a tuna can.
⢿⡄⠘⠷⠚⠋⠀ A smart species invents a can opener.
⠈⠳⣄ A master species delegates.

Re: [PATCH v3 0/4] Add xxhash and zstd modules

[Adam Borowski]

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 

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 
 #include 
 #include 
+#include 
 #include 
 #include "compression.h"

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!
-- 
⢀⣴⠾⠻⢶⣦⠀ 
⣾⠁⢠⠒⠀⣿⡁ A dumb species has no way to open a tuna can.
⢿⡄⠘⠷⠚⠋⠀ A smart species invents a can opener.
⠈⠳⣄ A master species delegates.

Re: nbd drops connection on most writes

[Adam Borowski]

On Fri, Jul 21, 2017 at 12:22:51PM +, Josef Bacik wrote:
> Oh shit the default timeout is 0 if you don't set it in the client.  Use
> the timeout option with nbd client and it should fix it for you.  I'll
> send something up to make this a sane default.

Confirmed, adding a timeout=XXX argument makes it work.
Great, thanks!

-- 
⢀⣴⠾⠻⢶⣦⠀ 
⣾⠁⢠⠒⠀⣿⡁ A dumb species has no way to open a tuna can.
⢿⡄⠘⠷⠚⠋⠀ A smart species invents a can opener.
⠈⠳⣄ A master species delegates.

Re: nbd drops connection on most writes

[Adam Borowski]

On Fri, Jul 21, 2017 at 12:22:51PM +, Josef Bacik wrote:
> Oh shit the default timeout is 0 if you don't set it in the client.  Use
> the timeout option with nbd client and it should fix it for you.  I'll
> send something up to make this a sane default.

Confirmed, adding a timeout=XXX argument makes it work.
Great, thanks!

-- 
⢀⣴⠾⠻⢶⣦⠀ 
⣾⠁⢠⠒⠀⣿⡁ A dumb species has no way to open a tuna can.
⢿⡄⠘⠷⠚⠋⠀ A smart species invents a can opener.
⠈⠳⣄ A master species delegates.

nbd drops connection on most writes

[Adam Borowski]

Hi!
I'm afraid that 4.13-rc1 nbd aborts connection on writes for me:

[  251.938384] block nbd0: Send data failed (result -11)
[  251.943484] block nbd0: Request send failed trying another connection
[  251.950034] block nbd0: Receive control failed (result -32)
[  251.955676] block nbd0: Attempted send on invalid socket
[  251.961022] print_req_error: I/O error, dev nbd0, sector 2206344
[  251.961025] block nbd0: shutting down sockets

Not all kinds of writes trigger the problem.  For example, you can dd to the
nbd block device, likewise badblocks -w succeeds without a hitch.  Yet at
least btrfs and swap disconnect nearly immediately.  Reads seem to work: for
example, btrfs can usually mount and scrub successfully, yet minor writes
that happen on a filesystem mounted rw even without explicit user-level
writes cause a disconnect in a short time.  "Real" writes to the filesystem
trigger it apparently outright.  Likewise, to use swap you need to write to
it first, thus it fails quickly.

Reproduced on arm64 (Pine64) first.  As this SoC just switched from an
out-of-tree ethernet driver to a completely different new one (dwmac-sun8i),
and such a switch can't be bisected, I assumed that's the culprit and did
not complain while in -next.

However, turns out the same happens on a bog-standard amd64, both on bare
metal and in qemu.

In all of these cases, the server is an amd64 Debian stretch, kernel
4.9.30-2+deb9u2, nbd-server 1:3.15.2-3.

Bisect blames dc88e34d "nbd: set sk->sk_sndtimeo for our sockets", and
indeed, reverting that patch makes everything fine again.


Bisect log:
# bad: [63a86362130f4c17eaa57f3ef5171ec43111a54e] Merge tag 'pm-4.13-rc2' of 
git://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm
# good: [6f7da290413ba713f0cdd9ff1a2a9bb129ef4f6c] Linux 4.12
git bisect start 'linus/master' 'v4.12'
# bad: [55a7b2125cf4739a8478d2d7223310ae7393408c] Merge tag 'arm64-upstream' of 
git://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux
git bisect bad 55a7b2125cf4739a8478d2d7223310ae7393408c
# bad: [1849f800fba32cd5a0b647f824f11426b85310d8] Merge tag 'armsoc-dt' of 
git://git.kernel.org/pub/scm/linux/kernel/git/arm/arm-soc
git bisect bad 1849f800fba32cd5a0b647f824f11426b85310d8
# bad: [cbcd4f08aa637b74f575268770da86a00fabde6d] Merge tag 'staging-4.13-rc1' 
of git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging
git bisect bad cbcd4f08aa637b74f575268770da86a00fabde6d
# bad: [1b044f1cfc65a7d90b209dfabd57e16d98b58c5b] Merge branch 
'timers-core-for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip
git bisect bad 1b044f1cfc65a7d90b209dfabd57e16d98b58c5b
# bad: [892ad5acca0b2ddb514fae63fa4686bf726d2471] Merge branch 
'locking-core-for-linus' of 
git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip
git bisect bad 892ad5acca0b2ddb514fae63fa4686bf726d2471
# bad: [e442cbf910c71fba5926cf757dd7f8fcce22fc5f] pktcdvd: remove the call to 
blk_queue_bounce
git bisect bad e442cbf910c71fba5926cf757dd7f8fcce22fc5f
# bad: [d86c4d8ef31b3d99c681c859cb4e936dafc2d7a4] nvme: move reset workqueue 
handling to common code
git bisect bad d86c4d8ef31b3d99c681c859cb4e936dafc2d7a4
# bad: [fdd050b5b3c96813ae6756ed68157d32ba31b9f2] Merge branch 'uuid-types' of 
bombadil.infradead.org:public_git/uuid into nvme-base
git bisect bad fdd050b5b3c96813ae6756ed68157d32ba31b9f2
# bad: [a104c9f22c7d073d4ae308ca36383ce5cc4631cc] nvme-rdma: fix merge error
git bisect bad a104c9f22c7d073d4ae308ca36383ce5cc4631cc
# good: [b040ad9cf6a169cc000a5324fcada695dfa1f4b3] loop: fix error handling 
regression
git bisect good b040ad9cf6a169cc000a5324fcada695dfa1f4b3
# bad: [36ffc6c1c0e67acdacb53348350d0a37206dbadf] block_dev: propagate 
bio_iov_iter_get_pages error in __blkdev_direct_IO
git bisect bad 36ffc6c1c0e67acdacb53348350d0a37206dbadf
# bad: [f729b66fca43d850d564b264c2033980c00a14b0] gfs2: remove the unused 
sd_log_error field
git bisect bad f729b66fca43d850d564b264c2033980c00a14b0
# bad: [401741547f95c0883fe143ac446d92c772937556] nvme-lightnvm: use 
blk_execute_rq in nvme_nvm_submit_user_cmd
git bisect bad 401741547f95c0883fe143ac446d92c772937556
# bad: [dc88e34d69d87c370deaa9d613dac8e3a0411f59] nbd: set sk->sk_sndtimeo for 
our sockets
git bisect bad dc88e34d69d87c370deaa9d613dac8e3a0411f59
# first bad commit: [dc88e34d69d87c370deaa9d613dac8e3a0411f59] nbd: set 
sk->sk_sndtimeo for our sockets


Meow!
-- 
⢀⣴⠾⠻⢶⣦⠀ 
⣾⠁⢠⠒⠀⣿⡁ A dumb species has no way to open a tuna can.
⢿⡄⠘⠷⠚⠋⠀ A smart species invents a can opener.
⠈⠳⣄ A master species delegates.

nbd drops connection on most writes

[Adam Borowski]

Hi!
I'm afraid that 4.13-rc1 nbd aborts connection on writes for me:

[  251.938384] block nbd0: Send data failed (result -11)
[  251.943484] block nbd0: Request send failed trying another connection
[  251.950034] block nbd0: Receive control failed (result -32)
[  251.955676] block nbd0: Attempted send on invalid socket
[  251.961022] print_req_error: I/O error, dev nbd0, sector 2206344
[  251.961025] block nbd0: shutting down sockets

Not all kinds of writes trigger the problem.  For example, you can dd to the
nbd block device, likewise badblocks -w succeeds without a hitch.  Yet at
least btrfs and swap disconnect nearly immediately.  Reads seem to work: for
example, btrfs can usually mount and scrub successfully, yet minor writes
that happen on a filesystem mounted rw even without explicit user-level
writes cause a disconnect in a short time.  "Real" writes to the filesystem
trigger it apparently outright.  Likewise, to use swap you need to write to
it first, thus it fails quickly.

Reproduced on arm64 (Pine64) first.  As this SoC just switched from an
out-of-tree ethernet driver to a completely different new one (dwmac-sun8i),
and such a switch can't be bisected, I assumed that's the culprit and did
not complain while in -next.

However, turns out the same happens on a bog-standard amd64, both on bare
metal and in qemu.

In all of these cases, the server is an amd64 Debian stretch, kernel
4.9.30-2+deb9u2, nbd-server 1:3.15.2-3.

Bisect blames dc88e34d "nbd: set sk->sk_sndtimeo for our sockets", and
indeed, reverting that patch makes everything fine again.


Bisect log:
# bad: [63a86362130f4c17eaa57f3ef5171ec43111a54e] Merge tag 'pm-4.13-rc2' of 
git://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm
# good: [6f7da290413ba713f0cdd9ff1a2a9bb129ef4f6c] Linux 4.12
git bisect start 'linus/master' 'v4.12'
# bad: [55a7b2125cf4739a8478d2d7223310ae7393408c] Merge tag 'arm64-upstream' of 
git://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux
git bisect bad 55a7b2125cf4739a8478d2d7223310ae7393408c
# bad: [1849f800fba32cd5a0b647f824f11426b85310d8] Merge tag 'armsoc-dt' of 
git://git.kernel.org/pub/scm/linux/kernel/git/arm/arm-soc
git bisect bad 1849f800fba32cd5a0b647f824f11426b85310d8
# bad: [cbcd4f08aa637b74f575268770da86a00fabde6d] Merge tag 'staging-4.13-rc1' 
of git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging
git bisect bad cbcd4f08aa637b74f575268770da86a00fabde6d
# bad: [1b044f1cfc65a7d90b209dfabd57e16d98b58c5b] Merge branch 
'timers-core-for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip
git bisect bad 1b044f1cfc65a7d90b209dfabd57e16d98b58c5b
# bad: [892ad5acca0b2ddb514fae63fa4686bf726d2471] Merge branch 
'locking-core-for-linus' of 
git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip
git bisect bad 892ad5acca0b2ddb514fae63fa4686bf726d2471
# bad: [e442cbf910c71fba5926cf757dd7f8fcce22fc5f] pktcdvd: remove the call to 
blk_queue_bounce
git bisect bad e442cbf910c71fba5926cf757dd7f8fcce22fc5f
# bad: [d86c4d8ef31b3d99c681c859cb4e936dafc2d7a4] nvme: move reset workqueue 
handling to common code
git bisect bad d86c4d8ef31b3d99c681c859cb4e936dafc2d7a4
# bad: [fdd050b5b3c96813ae6756ed68157d32ba31b9f2] Merge branch 'uuid-types' of 
bombadil.infradead.org:public_git/uuid into nvme-base
git bisect bad fdd050b5b3c96813ae6756ed68157d32ba31b9f2
# bad: [a104c9f22c7d073d4ae308ca36383ce5cc4631cc] nvme-rdma: fix merge error
git bisect bad a104c9f22c7d073d4ae308ca36383ce5cc4631cc
# good: [b040ad9cf6a169cc000a5324fcada695dfa1f4b3] loop: fix error handling 
regression
git bisect good b040ad9cf6a169cc000a5324fcada695dfa1f4b3
# bad: [36ffc6c1c0e67acdacb53348350d0a37206dbadf] block_dev: propagate 
bio_iov_iter_get_pages error in __blkdev_direct_IO
git bisect bad 36ffc6c1c0e67acdacb53348350d0a37206dbadf
# bad: [f729b66fca43d850d564b264c2033980c00a14b0] gfs2: remove the unused 
sd_log_error field
git bisect bad f729b66fca43d850d564b264c2033980c00a14b0
# bad: [401741547f95c0883fe143ac446d92c772937556] nvme-lightnvm: use 
blk_execute_rq in nvme_nvm_submit_user_cmd
git bisect bad 401741547f95c0883fe143ac446d92c772937556
# bad: [dc88e34d69d87c370deaa9d613dac8e3a0411f59] nbd: set sk->sk_sndtimeo for 
our sockets
git bisect bad dc88e34d69d87c370deaa9d613dac8e3a0411f59
# first bad commit: [dc88e34d69d87c370deaa9d613dac8e3a0411f59] nbd: set 
sk->sk_sndtimeo for our sockets


Meow!
-- 
⢀⣴⠾⠻⢶⣦⠀ 
⣾⠁⢠⠒⠀⣿⡁ A dumb species has no way to open a tuna can.
⢿⡄⠘⠷⠚⠋⠀ A smart species invents a can opener.
⠈⠳⣄ A master species delegates.

Re: [PATCH v2 3/4] btrfs: Add zstd support

[Adam Borowski]

On Tue, Jul 11, 2017 at 06:01:38AM +, Nick Terrell wrote:
> On 7/10/17, 9:57 PM, "Nick Terrell"  wrote:
> > The problem is caused by a gcc-7 bug [1]. It miscompiles
> > ZSTD_wildcopy(void *dst, void const *src, ptrdiff_t len) when len is 0.
>
> Sorry, my patch still triggered the gcc bug, I used the wrong compiler.
> This patch works, and runs about the same speed as before the patch for
> small inputs, and slightly faster for larger inputs (100+ bytes). I'll
> look for a faster workaround if benchmarks show it matters.

Confirmed, the fix stops the crash for me.  Yay!

> --- a/lib/zstd/zstd_internal.h
> +++ b/lib/zstd/zstd_internal.h
> @@ -139,12 +139,8 @@ static void ZSTD_copy8(void *dst, const void *src) { 
> memcpy(dst, src, 8); }
>  #define WILDCOPY_OVERLENGTH 8
>  ZSTD_STATIC void ZSTD_wildcopy(void *dst, const void *src, ptrdiff_t length)
>  {
> - const BYTE *ip = (const BYTE *)src;
> - BYTE *op = (BYTE *)dst;
> - BYTE *const oend = op + length;
> - do
> - COPY8(op, ip)
> - while (op < oend);
> + if (length > 0)
> + memcpy(dst, src, length);
>  }
>  
>  ZSTD_STATIC void ZSTD_wildcopy_e(void *dst, const void *src, void *dstEnd) 
> /* should be faster for decoding, but strangely, not verified on all platform 
> */

-- 
⢀⣴⠾⠻⢶⣦⠀ 
⣾⠁⢠⠒⠀⣿⡁ A dumb species has no way to open a tuna can.
⢿⡄⠘⠷⠚⠋⠀ A smart species invents a can opener.
⠈⠳⣄ A master species delegates.

Re: [PATCH v2 3/4] btrfs: Add zstd support

[Adam Borowski]

On Tue, Jul 11, 2017 at 06:01:38AM +, Nick Terrell wrote:
> On 7/10/17, 9:57 PM, "Nick Terrell"  wrote:
> > The problem is caused by a gcc-7 bug [1]. It miscompiles
> > ZSTD_wildcopy(void *dst, void const *src, ptrdiff_t len) when len is 0.
>
> Sorry, my patch still triggered the gcc bug, I used the wrong compiler.
> This patch works, and runs about the same speed as before the patch for
> small inputs, and slightly faster for larger inputs (100+ bytes). I'll
> look for a faster workaround if benchmarks show it matters.

Confirmed, the fix stops the crash for me.  Yay!

> --- a/lib/zstd/zstd_internal.h
> +++ b/lib/zstd/zstd_internal.h
> @@ -139,12 +139,8 @@ static void ZSTD_copy8(void *dst, const void *src) { 
> memcpy(dst, src, 8); }
>  #define WILDCOPY_OVERLENGTH 8
>  ZSTD_STATIC void ZSTD_wildcopy(void *dst, const void *src, ptrdiff_t length)
>  {
> - const BYTE *ip = (const BYTE *)src;
> - BYTE *op = (BYTE *)dst;
> - BYTE *const oend = op + length;
> - do
> - COPY8(op, ip)
> - while (op < oend);
> + if (length > 0)
> + memcpy(dst, src, length);
>  }
>  
>  ZSTD_STATIC void ZSTD_wildcopy_e(void *dst, const void *src, void *dstEnd) 
> /* should be faster for decoding, but strangely, not verified on all platform 
> */

-- 
⢀⣴⠾⠻⢶⣦⠀ 
⣾⠁⢠⠒⠀⣿⡁ A dumb species has no way to open a tuna can.
⢿⡄⠘⠷⠚⠋⠀ A smart species invents a can opener.
⠈⠳⣄ A master species delegates.

Re: [PATCH v2 3/4] btrfs: Add zstd support

[Adam Borowski]

On Sat, Jul 08, 2017 at 01:40:18AM +0200, Adam Borowski wrote:
> On Fri, Jul 07, 2017 at 11:17:49PM +, Nick Terrell wrote:
> > On 7/6/17, 9:32 AM, "Adam Borowski" <kilob...@angband.pl> wrote:
> > > Got a reproducible crash on amd64:
>
> > Thanks for the bug report Adam! I'm looking into the failure, and haven't
> > been able to reproduce it yet. I've built my kernel from your tree, and
> > I ran your script with the kernel.tar tarball 100 times, but haven't gotten
> > a failure yet.
> 
> > I have a few questions to guide my debugging.
> > 
> > - How many cores are you running with? I’ve run the script with 1, 2, and 4 
> > cores.
> > - Which version of gcc are you using to compile the kernel? I’m using 
> > gcc-6.2.0-5ubuntu12.
> > - Are the failures always in exactly the same place, and does it fail 100%
> >   of the time or just regularly?
> 
> 6 cores -- all on bare metal.  gcc-7.1.0-9.
> Lemme try with gcc-6, a different config or in a VM.

I've tried the following:
* gcc-6, defconfig (+btrfs obviously)
* gcc-7, defconfig
* gcc-6, my regular config
* gcc-7, my regular config
* gcc-7, debug + UBSAN + etc
* gcc-7, defconfig, qemu-kvm with only 1 core

Every build with gcc-7 reproduces the crash, every with gcc-6 does not.

-- 
⢀⣴⠾⠻⢶⣦⠀ 
⣾⠁⢠⠒⠀⣿⡁ A dumb species has no way to open a tuna can.
⢿⡄⠘⠷⠚⠋⠀ A smart species invents a can opener.
⠈⠳⣄ A master species delegates.

Re: [PATCH v2 3/4] btrfs: Add zstd support

[Adam Borowski]

On Sat, Jul 08, 2017 at 01:40:18AM +0200, Adam Borowski wrote:
> On Fri, Jul 07, 2017 at 11:17:49PM +, Nick Terrell wrote:
> > On 7/6/17, 9:32 AM, "Adam Borowski"  wrote:
> > > Got a reproducible crash on amd64:
>
> > Thanks for the bug report Adam! I'm looking into the failure, and haven't
> > been able to reproduce it yet. I've built my kernel from your tree, and
> > I ran your script with the kernel.tar tarball 100 times, but haven't gotten
> > a failure yet.
> 
> > I have a few questions to guide my debugging.
> > 
> > - How many cores are you running with? I’ve run the script with 1, 2, and 4 
> > cores.
> > - Which version of gcc are you using to compile the kernel? I’m using 
> > gcc-6.2.0-5ubuntu12.
> > - Are the failures always in exactly the same place, and does it fail 100%
> >   of the time or just regularly?
> 
> 6 cores -- all on bare metal.  gcc-7.1.0-9.
> Lemme try with gcc-6, a different config or in a VM.

I've tried the following:
* gcc-6, defconfig (+btrfs obviously)
* gcc-7, defconfig
* gcc-6, my regular config
* gcc-7, my regular config
* gcc-7, debug + UBSAN + etc
* gcc-7, defconfig, qemu-kvm with only 1 core

Every build with gcc-7 reproduces the crash, every with gcc-6 does not.

-- 
⢀⣴⠾⠻⢶⣦⠀ 
⣾⠁⢠⠒⠀⣿⡁ A dumb species has no way to open a tuna can.
⢿⡄⠘⠷⠚⠋⠀ A smart species invents a can opener.
⠈⠳⣄ A master species delegates.

Re: [PATCH v2 3/4] btrfs: Add zstd support

[Adam Borowski]

On Fri, Jul 07, 2017 at 11:17:49PM +, Nick Terrell wrote:
> On 7/6/17, 9:32 AM, "Adam Borowski" <kilob...@angband.pl> wrote:
> > Got a reproducible crash on amd64:
> >
> > [98235.266511] BUG: unable to handle kernel paging request at 
> > c90001251000

> > [98235.314008]  ? ZSTD_compressBlock_fast+0x94b/0xb30
> > [98235.315975]  ? ZSTD_compressContinue_internal+0x1a0/0x580
> > [98235.317938]  ? ZSTD_compressStream_generic+0x248/0x2f0
> > [98235.319877]  ? ZSTD_compressStream+0x41/0x60
> > [98235.321821]  ? zstd_compress_pages+0x236/0x5d0
> > [98235.323724]  ? btrfs_compress_pages+0x5e/0x80
> > [98235.325684]  ? compress_file_range.constprop.79+0x1eb/0x750
> 
> Thanks for the bug report Adam! I'm looking into the failure, and haven't
> been able to reproduce it yet. I've built my kernel from your tree, and
> I ran your script with the kernel.tar tarball 100 times, but haven't gotten
> a failure yet.

Crashed the same way 4 out of 4 tries for me.

> I have a few questions to guide my debugging.
> 
> - How many cores are you running with? I’ve run the script with 1, 2, and 4 
> cores.
> - Which version of gcc are you using to compile the kernel? I’m using 
> gcc-6.2.0-5ubuntu12.
> - Are the failures always in exactly the same place, and does it fail 100%
>   of the time or just regularly?

6 cores -- all on bare metal.  gcc-7.1.0-9.

Lemme try with gcc-6, a different config or in a VM.

-- 
⢀⣴⠾⠻⢶⣦⠀ 
⣾⠁⢠⠒⠀⣿⡁ A dumb species has no way to open a tuna can.
⢿⡄⠘⠷⠚⠋⠀ A smart species invents a can opener.
⠈⠳⣄ A master species delegates.

Re: [PATCH v2 3/4] btrfs: Add zstd support

[Adam Borowski]

On Fri, Jul 07, 2017 at 11:17:49PM +, Nick Terrell wrote:
> On 7/6/17, 9:32 AM, "Adam Borowski"  wrote:
> > Got a reproducible crash on amd64:
> >
> > [98235.266511] BUG: unable to handle kernel paging request at 
> > c90001251000

> > [98235.314008]  ? ZSTD_compressBlock_fast+0x94b/0xb30
> > [98235.315975]  ? ZSTD_compressContinue_internal+0x1a0/0x580
> > [98235.317938]  ? ZSTD_compressStream_generic+0x248/0x2f0
> > [98235.319877]  ? ZSTD_compressStream+0x41/0x60
> > [98235.321821]  ? zstd_compress_pages+0x236/0x5d0
> > [98235.323724]  ? btrfs_compress_pages+0x5e/0x80
> > [98235.325684]  ? compress_file_range.constprop.79+0x1eb/0x750
> 
> Thanks for the bug report Adam! I'm looking into the failure, and haven't
> been able to reproduce it yet. I've built my kernel from your tree, and
> I ran your script with the kernel.tar tarball 100 times, but haven't gotten
> a failure yet.

Crashed the same way 4 out of 4 tries for me.

> I have a few questions to guide my debugging.
> 
> - How many cores are you running with? I’ve run the script with 1, 2, and 4 
> cores.
> - Which version of gcc are you using to compile the kernel? I’m using 
> gcc-6.2.0-5ubuntu12.
> - Are the failures always in exactly the same place, and does it fail 100%
>   of the time or just regularly?

6 cores -- all on bare metal.  gcc-7.1.0-9.

Lemme try with gcc-6, a different config or in a VM.

-- 
⢀⣴⠾⠻⢶⣦⠀ 
⣾⠁⢠⠒⠀⣿⡁ A dumb species has no way to open a tuna can.
⢿⡄⠘⠷⠚⠋⠀ A smart species invents a can opener.
⠈⠳⣄ A master species delegates.

Re: [PATCH v2 3/4] btrfs: Add zstd support

[Adam Borowski]

On Thu, Jun 29, 2017 at 12:41:07PM -0700, Nick Terrell wrote:
> Add zstd compression and decompression support to BtrFS. zstd at its
> fastest level compresses almost as well as zlib, while offering much
> faster compression and decompression, approaching lzo speeds.

Got a reproducible crash on amd64:

[98235.266511] BUG: unable to handle kernel paging request at c90001251000
[98235.267485] IP: ZSTD_storeSeq.constprop.24+0x67/0xe0
[98235.269395] PGD 227034067 
[98235.269397] P4D 227034067 
[98235.271587] PUD 227035067 
[98235.273657] PMD 223323067 
[98235.275744] PTE 0

[98235.281545] Oops: 0002 [#1] SMP
[98235.283353] Modules linked in: loop veth tun fuse arc4 rtl8xxxu mac80211 
cfg80211 cp210x pl2303 rfkill usbserial nouveau video mxm_wmi ttm
[98235.285203] CPU: 0 PID: 10850 Comm: kworker/u12:9 Not tainted 4.12.0+ #1
[98235.287070] Hardware name: System manufacturer System Product Name/M4A77T, 
BIOS 240105/18/2011
[98235.288964] Workqueue: btrfs-delalloc btrfs_delalloc_helper
[98235.290934] task: 880224984140 task.stack: c90007e5c000
[98235.292731] RIP: 0010:ZSTD_storeSeq.constprop.24+0x67/0xe0
[98235.294579] RSP: 0018:c90007e5fa68 EFLAGS: 00010282
[98235.296395] RAX: c90001251001 RBX: 0094 RCX: c9000118f930
[98235.298380] RDX: 0006 RSI: c900011b06b0 RDI: c9000118d1e0
[98235.300321] RBP: 009f R08: 1fffbe58 R09: 
[98235.302282] R10: c9000118f970 R11: 0005 R12: c9000118f878
[98235.304221] R13: 005b R14: c9000118f915 R15: c900011cfe88
[98235.306147] FS:  () GS:88022fc0() 
knlGS:
[98235.308162] CS:  0010 DS:  ES:  CR0: 80050033
[98235.310129] CR2: c90001251000 CR3: 00021018d000 CR4: 06f0
[98235.312095] Call Trace:
[98235.314008]  ? ZSTD_compressBlock_fast+0x94b/0xb30
[98235.315975]  ? ZSTD_compressContinue_internal+0x1a0/0x580
[98235.317938]  ? ZSTD_compressStream_generic+0x248/0x2f0
[98235.319877]  ? ZSTD_compressStream+0x41/0x60
[98235.321821]  ? zstd_compress_pages+0x236/0x5d0
[98235.323724]  ? btrfs_compress_pages+0x5e/0x80
[98235.325684]  ? compress_file_range.constprop.79+0x1eb/0x750
[98235.327668]  ? async_cow_start+0x2e/0x50
[98235.329594]  ? btrfs_worker_helper+0x1b9/0x1d0
[98235.331486]  ? process_one_work+0x158/0x2f0
[98235.61]  ? worker_thread+0x45/0x3a0
[98235.335253]  ? process_one_work+0x2f0/0x2f0
[98235.337189]  ? kthread+0x10e/0x130
[98235.339020]  ? kthread_park+0x60/0x60
[98235.340819]  ? ret_from_fork+0x22/0x30
[98235.342637] Code: 8b 4e d0 4c 89 48 d0 4c 8b 4e d8 4c 89 48 d8 4c 8b 4e e0 
4c 89 48 e0 4c 8b 4e e8 4c 89 48 e8 4c 8b 4e f0 4c 89 48 f0 4c 8b 4e f8 <4c> 89 
48 f8 48 39 f1 75 a2 4e 8d 04 c0 48 8b 31 48 83 c0 08 48 
[98235.346773] RIP: ZSTD_storeSeq.constprop.24+0x67/0xe0 RSP: c90007e5fa68
[98235.348809] CR2: c90001251000
[98235.363216] ---[ end trace 5fb3ad0f2aec0605 ]---
[98235.363218] BUG: unable to handle kernel paging request at c9000393a000
[98235.363239] IP: ZSTD_storeSeq.constprop.24+0x67/0xe0
[98235.363241] PGD 227034067 
[98235.363242] P4D 227034067 
[98235.363243] PUD 227035067 
[98235.363244] PMD 21edec067 
[98235.363245] PTE 0
(More of the above follows.)

My reproducer copies an uncompressed tarball onto a fresh filesystem:
.
#!/bin/sh
set -e

losetup -D; umount /mnt/vol1 ||:
dd if=/dev/zero of=/tmp/disk bs=2048 seek=1048575 count=1
mkfs.btrfs -msingle /tmp/disk
losetup -f /tmp/disk
sleep 1 # yay udev races
mount -onoatime,compress=$1 /dev/loop0 /mnt/vol1
time sh -c 'cp -p ~kilobyte/tmp/kernel.tar /mnt/vol1 && umount /mnt/vol1'
losetup -D
`
(run it with arg of "zstd")

Kernel is 4.12.0 + btrfs-for-4.13 + v4 of Qu's chunk check + some unrelated
stuff + zstd; in case it matters I've pushed my tree to
https://github.com/kilobyte/linux/tree/zstd-crash

The payload is a tarball of the above, but, for debugging compression you
need the exact byte stream.  https://angband.pl/tmp/kernel.tar.xz --
without xz, I compressed it for transport.


Meow!
-- 
⢀⣴⠾⠻⢶⣦⠀ 
⣾⠁⢠⠒⠀⣿⡁ A dumb species has no way to open a tuna can.
⢿⡄⠘⠷⠚⠋⠀ A smart species invents a can opener.
⠈⠳⣄ A master species delegates.

Re: [PATCH v2 3/4] btrfs: Add zstd support

[Adam Borowski]

On Thu, Jun 29, 2017 at 12:41:07PM -0700, Nick Terrell wrote:
> Add zstd compression and decompression support to BtrFS. zstd at its
> fastest level compresses almost as well as zlib, while offering much
> faster compression and decompression, approaching lzo speeds.

Got a reproducible crash on amd64:

[98235.266511] BUG: unable to handle kernel paging request at c90001251000
[98235.267485] IP: ZSTD_storeSeq.constprop.24+0x67/0xe0
[98235.269395] PGD 227034067 
[98235.269397] P4D 227034067 
[98235.271587] PUD 227035067 
[98235.273657] PMD 223323067 
[98235.275744] PTE 0

[98235.281545] Oops: 0002 [#1] SMP
[98235.283353] Modules linked in: loop veth tun fuse arc4 rtl8xxxu mac80211 
cfg80211 cp210x pl2303 rfkill usbserial nouveau video mxm_wmi ttm
[98235.285203] CPU: 0 PID: 10850 Comm: kworker/u12:9 Not tainted 4.12.0+ #1
[98235.287070] Hardware name: System manufacturer System Product Name/M4A77T, 
BIOS 240105/18/2011
[98235.288964] Workqueue: btrfs-delalloc btrfs_delalloc_helper
[98235.290934] task: 880224984140 task.stack: c90007e5c000
[98235.292731] RIP: 0010:ZSTD_storeSeq.constprop.24+0x67/0xe0
[98235.294579] RSP: 0018:c90007e5fa68 EFLAGS: 00010282
[98235.296395] RAX: c90001251001 RBX: 0094 RCX: c9000118f930
[98235.298380] RDX: 0006 RSI: c900011b06b0 RDI: c9000118d1e0
[98235.300321] RBP: 009f R08: 1fffbe58 R09: 
[98235.302282] R10: c9000118f970 R11: 0005 R12: c9000118f878
[98235.304221] R13: 005b R14: c9000118f915 R15: c900011cfe88
[98235.306147] FS:  () GS:88022fc0() 
knlGS:
[98235.308162] CS:  0010 DS:  ES:  CR0: 80050033
[98235.310129] CR2: c90001251000 CR3: 00021018d000 CR4: 06f0
[98235.312095] Call Trace:
[98235.314008]  ? ZSTD_compressBlock_fast+0x94b/0xb30
[98235.315975]  ? ZSTD_compressContinue_internal+0x1a0/0x580
[98235.317938]  ? ZSTD_compressStream_generic+0x248/0x2f0
[98235.319877]  ? ZSTD_compressStream+0x41/0x60
[98235.321821]  ? zstd_compress_pages+0x236/0x5d0
[98235.323724]  ? btrfs_compress_pages+0x5e/0x80
[98235.325684]  ? compress_file_range.constprop.79+0x1eb/0x750
[98235.327668]  ? async_cow_start+0x2e/0x50
[98235.329594]  ? btrfs_worker_helper+0x1b9/0x1d0
[98235.331486]  ? process_one_work+0x158/0x2f0
[98235.61]  ? worker_thread+0x45/0x3a0
[98235.335253]  ? process_one_work+0x2f0/0x2f0
[98235.337189]  ? kthread+0x10e/0x130
[98235.339020]  ? kthread_park+0x60/0x60
[98235.340819]  ? ret_from_fork+0x22/0x30
[98235.342637] Code: 8b 4e d0 4c 89 48 d0 4c 8b 4e d8 4c 89 48 d8 4c 8b 4e e0 
4c 89 48 e0 4c 8b 4e e8 4c 89 48 e8 4c 8b 4e f0 4c 89 48 f0 4c 8b 4e f8 <4c> 89 
48 f8 48 39 f1 75 a2 4e 8d 04 c0 48 8b 31 48 83 c0 08 48 
[98235.346773] RIP: ZSTD_storeSeq.constprop.24+0x67/0xe0 RSP: c90007e5fa68
[98235.348809] CR2: c90001251000
[98235.363216] ---[ end trace 5fb3ad0f2aec0605 ]---
[98235.363218] BUG: unable to handle kernel paging request at c9000393a000
[98235.363239] IP: ZSTD_storeSeq.constprop.24+0x67/0xe0
[98235.363241] PGD 227034067 
[98235.363242] P4D 227034067 
[98235.363243] PUD 227035067 
[98235.363244] PMD 21edec067 
[98235.363245] PTE 0
(More of the above follows.)

My reproducer copies an uncompressed tarball onto a fresh filesystem:
.
#!/bin/sh
set -e

losetup -D; umount /mnt/vol1 ||:
dd if=/dev/zero of=/tmp/disk bs=2048 seek=1048575 count=1
mkfs.btrfs -msingle /tmp/disk
losetup -f /tmp/disk
sleep 1 # yay udev races
mount -onoatime,compress=$1 /dev/loop0 /mnt/vol1
time sh -c 'cp -p ~kilobyte/tmp/kernel.tar /mnt/vol1 && umount /mnt/vol1'
losetup -D
`
(run it with arg of "zstd")

Kernel is 4.12.0 + btrfs-for-4.13 + v4 of Qu's chunk check + some unrelated
stuff + zstd; in case it matters I've pushed my tree to
https://github.com/kilobyte/linux/tree/zstd-crash

The payload is a tarball of the above, but, for debugging compression you
need the exact byte stream.  https://angband.pl/tmp/kernel.tar.xz --
without xz, I compressed it for transport.


Meow!
-- 
⢀⣴⠾⠻⢶⣦⠀ 
⣾⠁⢠⠒⠀⣿⡁ A dumb species has no way to open a tuna can.
⢿⡄⠘⠷⠚⠋⠀ A smart species invents a can opener.
⠈⠳⣄ A master species delegates.

Re: [PATCH v2 3/4] btrfs: Add zstd support

[Adam Borowski]

On Thu, Jun 29, 2017 at 12:41:07PM -0700, Nick Terrell wrote:
> Add zstd compression and decompression support to BtrFS. zstd at its
> fastest level compresses almost as well as zlib, while offering much
> faster compression and decompression, approaching lzo speeds.

> +static int zstd_decompress_bio(struct list_head *ws, struct page **pages_in,
> + u64 disk_start,
> + struct bio *orig_bio,
> + size_t srclen)

Note that this fails to build if e1ddce71 is applied, and it'll be in
mainline in just a few days from now.  Fixing that is obvious, but
to avoid duplicating the effort:


diff --git a/fs/btrfs/zstd.c b/fs/btrfs/zstd.c
index 838741b3732c..e9403e569d8e 100644
--- a/fs/btrfs/zstd.c
+++ b/fs/btrfs/zstd.c
@@ -25,6 +25,7 @@
 #include 
 #include 
 #include 
+#include 
 #include "compression.h"
 
 #define ZSTD_BTRFS_MAX_WINDOWLOG 17
@@ -262,17 +263,18 @@ static int zstd_compress_pages(struct list_head *ws,
return ret;
 }
 
-static int zstd_decompress_bio(struct list_head *ws, struct page **pages_in,
-   u64 disk_start,
-   struct bio *orig_bio,
-   size_t srclen)
+static int zstd_decompress_bio(struct list_head *ws, struct compressed_bio *cb)
 {
struct workspace *workspace = list_entry(ws, struct workspace, list);
ZSTD_DStream *stream;
int ret = 0;
unsigned long page_in_index = 0;
+   size_t srclen = cb->compressed_len;
unsigned long total_pages_in = DIV_ROUND_UP(srclen, PAGE_SIZE);
unsigned long buf_start;
+   struct page **pages_in = cb->compressed_pages;
+   u64 disk_start = cb->start;
+   struct bio *orig_bio = cb->orig_bio;
unsigned long total_out = 0;
ZSTD_inBuffer in_buf = { NULL, 0, 0 };
ZSTD_outBuffer out_buf = { NULL, 0, 0 };


-- 
⢀⣴⠾⠻⢶⣦⠀ 
⣾⠁⢠⠒⠀⣿⡁ A dumb species has no way to open a tuna can.
⢿⡄⠘⠷⠚⠋⠀ A smart species invents a can opener.
⠈⠳⣄ A master species delegates.

Re: [PATCH v2 3/4] btrfs: Add zstd support

[Adam Borowski]

On Thu, Jun 29, 2017 at 12:41:07PM -0700, Nick Terrell wrote:
> Add zstd compression and decompression support to BtrFS. zstd at its
> fastest level compresses almost as well as zlib, while offering much
> faster compression and decompression, approaching lzo speeds.

> +static int zstd_decompress_bio(struct list_head *ws, struct page **pages_in,
> + u64 disk_start,
> + struct bio *orig_bio,
> + size_t srclen)

Note that this fails to build if e1ddce71 is applied, and it'll be in
mainline in just a few days from now.  Fixing that is obvious, but
to avoid duplicating the effort:


diff --git a/fs/btrfs/zstd.c b/fs/btrfs/zstd.c
index 838741b3732c..e9403e569d8e 100644
--- a/fs/btrfs/zstd.c
+++ b/fs/btrfs/zstd.c
@@ -25,6 +25,7 @@
 #include 
 #include 
 #include 
+#include 
 #include "compression.h"
 
 #define ZSTD_BTRFS_MAX_WINDOWLOG 17
@@ -262,17 +263,18 @@ static int zstd_compress_pages(struct list_head *ws,
return ret;
 }
 
-static int zstd_decompress_bio(struct list_head *ws, struct page **pages_in,
-   u64 disk_start,
-   struct bio *orig_bio,
-   size_t srclen)
+static int zstd_decompress_bio(struct list_head *ws, struct compressed_bio *cb)
 {
struct workspace *workspace = list_entry(ws, struct workspace, list);
ZSTD_DStream *stream;
int ret = 0;
unsigned long page_in_index = 0;
+   size_t srclen = cb->compressed_len;
unsigned long total_pages_in = DIV_ROUND_UP(srclen, PAGE_SIZE);
unsigned long buf_start;
+   struct page **pages_in = cb->compressed_pages;
+   u64 disk_start = cb->start;
+   struct bio *orig_bio = cb->orig_bio;
unsigned long total_out = 0;
ZSTD_inBuffer in_buf = { NULL, 0, 0 };
ZSTD_outBuffer out_buf = { NULL, 0, 0 };


-- 
⢀⣴⠾⠻⢶⣦⠀ 
⣾⠁⢠⠒⠀⣿⡁ A dumb species has no way to open a tuna can.
⢿⡄⠘⠷⠚⠋⠀ A smart species invents a can opener.
⠈⠳⣄ A master species delegates.

Re: [PATCH] lib/zstd: use div_u64() to let it build on 32-bit

[Adam Borowski]

On Tue, Jun 27, 2017 at 05:27:51AM +, Nick Terrell wrote:
> Adam, I’ve applied the same patch in my tree. I’ll send out the update [1]
> once it's reviewed, since I also reduced the stack usage of functions
> using over 1 KB of stack space.
> 
> I have userland tests set up mocking the linux kernel headers, and tested
> 32-bit mode there, but neglected to test the kernel on a 32-bit VM, which
> I’ve now corrected. Thanks for testing the patch on your ARM machine!

Is there a version I should be testing?

I got a bunch of those:
[10170.448783] kworker/u8:6: page allocation stalls for 60720ms, order:0, 
mode:0x14000c2(GFP_KERNEL|__GFP_HIGHMEM), nodemask=(null)
[10170.448819] kworker/u8:6 cpuset=/ mems_allowed=0
[10170.448842] CPU: 3 PID: 13430 Comm: kworker/u8:6 Not tainted 
4.12.0-rc7-00034-gdff47ed160bb #1
[10170.448846] Hardware name: SAMSUNG EXYNOS (Flattened Device Tree)
[10170.448872] Workqueue: btrfs-endio btrfs_endio_helper
[10170.448910] [] (unwind_backtrace) from [] 
(show_stack+0x10/0x14)
[10170.448925] [] (show_stack) from [] 
(dump_stack+0x78/0x8c)
[10170.448942] [] (dump_stack) from [] 
(warn_alloc+0xc0/0x170)
[10170.448952] [] (warn_alloc) from [] 
(__alloc_pages_nodemask+0x97c/0xe30)
[10170.448964] [] (__alloc_pages_nodemask) from [] 
(__vmalloc_node_range+0x144/0x27c)
[10170.448976] [] (__vmalloc_node_range) from [] 
(__vmalloc_node.constprop.10+0x48/0x50)
[10170.448982] [] (__vmalloc_node.constprop.10) from [] 
(vmalloc+0x2c/0x34)
[10170.448990] [] (vmalloc) from [] 
(zstd_alloc_workspace+0x6c/0xb8)
[10170.448997] [] (zstd_alloc_workspace) from [] 
(find_workspace+0x120/0x1f4)
[10170.449002] [] (find_workspace) from [] 
(end_compressed_bio_read+0x1d4/0x3b0)
[10170.449016] [] (end_compressed_bio_read) from [] 
(process_one_work+0x1d8/0x3f0)
[10170.449026] [] (process_one_work) from [] 
(worker_thread+0x38/0x558)
[10170.449035] [] (worker_thread) from [] 
(kthread+0x124/0x154)
[10170.449042] [] (kthread) from [] 
(ret_from_fork+0x14/0x3c)

which never happened with compress=lzo, and a 2GB RAM machine that runs 4
threads of various builds runs into memory pressure quite often.  On the
other hand, I used 4.11 for lzo so this needs more testing before I can
blame the zstd code.

Also, I had network problems all day today so the machine was mostly idle
instead of doing further tests -- not quite going to pull sources to build
over a phone connection.

I'm on linus:4.12-rc7 with only a handful of btrfs patches (v3 of Qu's chunk
check, some misc crap) -- I guess I should use at least btrfs-for-4.13.  Or
would you prefer full-blown next?


Meow!
-- 
⢀⣴⠾⠻⢶⣦⠀ 
⣾⠁⢠⠒⠀⣿⡁ A dumb species has no way to open a tuna can.
⢿⡄⠘⠷⠚⠋⠀ A smart species invents a can opener.
⠈⠳⣄ A master species delegates.

Re: [PATCH] lib/zstd: use div_u64() to let it build on 32-bit

[Adam Borowski]

On Tue, Jun 27, 2017 at 05:27:51AM +, Nick Terrell wrote:
> Adam, I’ve applied the same patch in my tree. I’ll send out the update [1]
> once it's reviewed, since I also reduced the stack usage of functions
> using over 1 KB of stack space.
> 
> I have userland tests set up mocking the linux kernel headers, and tested
> 32-bit mode there, but neglected to test the kernel on a 32-bit VM, which
> I’ve now corrected. Thanks for testing the patch on your ARM machine!

Is there a version I should be testing?

I got a bunch of those:
[10170.448783] kworker/u8:6: page allocation stalls for 60720ms, order:0, 
mode:0x14000c2(GFP_KERNEL|__GFP_HIGHMEM), nodemask=(null)
[10170.448819] kworker/u8:6 cpuset=/ mems_allowed=0
[10170.448842] CPU: 3 PID: 13430 Comm: kworker/u8:6 Not tainted 
4.12.0-rc7-00034-gdff47ed160bb #1
[10170.448846] Hardware name: SAMSUNG EXYNOS (Flattened Device Tree)
[10170.448872] Workqueue: btrfs-endio btrfs_endio_helper
[10170.448910] [] (unwind_backtrace) from [] 
(show_stack+0x10/0x14)
[10170.448925] [] (show_stack) from [] 
(dump_stack+0x78/0x8c)
[10170.448942] [] (dump_stack) from [] 
(warn_alloc+0xc0/0x170)
[10170.448952] [] (warn_alloc) from [] 
(__alloc_pages_nodemask+0x97c/0xe30)
[10170.448964] [] (__alloc_pages_nodemask) from [] 
(__vmalloc_node_range+0x144/0x27c)
[10170.448976] [] (__vmalloc_node_range) from [] 
(__vmalloc_node.constprop.10+0x48/0x50)
[10170.448982] [] (__vmalloc_node.constprop.10) from [] 
(vmalloc+0x2c/0x34)
[10170.448990] [] (vmalloc) from [] 
(zstd_alloc_workspace+0x6c/0xb8)
[10170.448997] [] (zstd_alloc_workspace) from [] 
(find_workspace+0x120/0x1f4)
[10170.449002] [] (find_workspace) from [] 
(end_compressed_bio_read+0x1d4/0x3b0)
[10170.449016] [] (end_compressed_bio_read) from [] 
(process_one_work+0x1d8/0x3f0)
[10170.449026] [] (process_one_work) from [] 
(worker_thread+0x38/0x558)
[10170.449035] [] (worker_thread) from [] 
(kthread+0x124/0x154)
[10170.449042] [] (kthread) from [] 
(ret_from_fork+0x14/0x3c)

which never happened with compress=lzo, and a 2GB RAM machine that runs 4
threads of various builds runs into memory pressure quite often.  On the
other hand, I used 4.11 for lzo so this needs more testing before I can
blame the zstd code.

Also, I had network problems all day today so the machine was mostly idle
instead of doing further tests -- not quite going to pull sources to build
over a phone connection.

I'm on linus:4.12-rc7 with only a handful of btrfs patches (v3 of Qu's chunk
check, some misc crap) -- I guess I should use at least btrfs-for-4.13.  Or
would you prefer full-blown next?


Meow!
-- 
⢀⣴⠾⠻⢶⣦⠀ 
⣾⠁⢠⠒⠀⣿⡁ A dumb species has no way to open a tuna can.
⢿⡄⠘⠷⠚⠋⠀ A smart species invents a can opener.
⠈⠳⣄ A master species delegates.

[PATCH] lib/zstd: use div_u64() to let it build on 32-bit

[Adam Borowski]

David Sterba wrote:
> > Thus, you want do_div() instead of /; do check widths and signedness of
> > arguments.
>
> No do_div please, div_u64 or div64_u64.

Good to know, the interface of do_div() is indeed weird.

I guess Nick has found and fixed the offending divisions in his tree
already, but this patch I'm sending is what I'm testing.

One thing to note is that it divides u64 by size_t, so the actual operation
differs on 32 vs 64-bit.  Yet the code fails to handle compressing pieces
bigger than 4GB in other places -- so use of size_t is misleading.  Perhaps
u32 would better convey this limitation?

Anyway, that this code didn't even compile on 32-bit also means it hasn't
been tested.  I just happen to have such an ARM machine doing Debian archive
rebuilds; I've rewritten the chroots with compress=zstd; this should be a
nice non-artificial test.  The load consists of snapshot+dpkg+gcc/etc+
assorted testsuites, two sbuild instances.  Seems to work fine for a whole
hour (yay!) already, let's see if there'll be any explosions.


-- >8  >8  >8  >8  >8  >8  >8  >8  >8 --
Note that "total" is limited to 2³²-1 elsewhere despite being declared
as size_t, so it's ok to use 64/32 -- it's much faster on eg. x86-32
than 64/64.

Signed-off-by: Adam Borowski <kilob...@angband.pl>
---
 lib/zstd/fse_compress.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/lib/zstd/fse_compress.c b/lib/zstd/fse_compress.c
index e016bb177833..f59f9ebfe9c0 100644
--- a/lib/zstd/fse_compress.c
+++ b/lib/zstd/fse_compress.c
@@ -49,6 +49,7 @@
 #include "fse.h"
 #include 
 #include  /* memcpy, memset */
+#include 
 
 /* **
 *  Error Management
@@ -575,7 +576,7 @@ static size_t FSE_normalizeM2(short *norm, U32 tableLog, 
const unsigned *count,
{
U64 const vStepLog = 62 - tableLog;
U64 const mid = (1ULL << (vStepLog - 1)) - 1;
-   U64 const rStep = U64)1 << vStepLog) * ToDistribute) + mid) 
/ total; /* scale on remaining */
+   U64 const rStep = div_u64U64)1 << vStepLog) * ToDistribute) 
+ mid, total); /* scale on remaining */
U64 tmpTotal = mid;
for (s = 0; s <= maxSymbolValue; s++) {
if (norm[s] == NOT_YET_ASSIGNED) {
@@ -609,7 +610,7 @@ size_t FSE_normalizeCount(short *normalizedCounter, 
unsigned tableLog, const uns
{
U32 const rtbTable[] = {0, 473195, 504333, 520860, 55, 
70, 75, 83};
U64 const scale = 62 - tableLog;
-   U64 const step = ((U64)1 << 62) / total; /* <== here, one 
division ! */
+   U64 const step = div_u64((U64)1 << 62, total); /* <== here, one 
division ! */
U64 const vStep = 1ULL << (scale - 20);
int stillToDistribute = 1 << tableLog;
unsigned s;
-- 
2.13.1

[PATCH] lib/zstd: use div_u64() to let it build on 32-bit

[Adam Borowski]

David Sterba wrote:
> > Thus, you want do_div() instead of /; do check widths and signedness of
> > arguments.
>
> No do_div please, div_u64 or div64_u64.

Good to know, the interface of do_div() is indeed weird.

I guess Nick has found and fixed the offending divisions in his tree
already, but this patch I'm sending is what I'm testing.

One thing to note is that it divides u64 by size_t, so the actual operation
differs on 32 vs 64-bit.  Yet the code fails to handle compressing pieces
bigger than 4GB in other places -- so use of size_t is misleading.  Perhaps
u32 would better convey this limitation?

Anyway, that this code didn't even compile on 32-bit also means it hasn't
been tested.  I just happen to have such an ARM machine doing Debian archive
rebuilds; I've rewritten the chroots with compress=zstd; this should be a
nice non-artificial test.  The load consists of snapshot+dpkg+gcc/etc+
assorted testsuites, two sbuild instances.  Seems to work fine for a whole
hour (yay!) already, let's see if there'll be any explosions.


-- >8  >8  >8  >8  >8  >8  >8  >8  >8 --
Note that "total" is limited to 2³²-1 elsewhere despite being declared
as size_t, so it's ok to use 64/32 -- it's much faster on eg. x86-32
than 64/64.

Signed-off-by: Adam Borowski 
---
 lib/zstd/fse_compress.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/lib/zstd/fse_compress.c b/lib/zstd/fse_compress.c
index e016bb177833..f59f9ebfe9c0 100644
--- a/lib/zstd/fse_compress.c
+++ b/lib/zstd/fse_compress.c
@@ -49,6 +49,7 @@
 #include "fse.h"
 #include 
 #include  /* memcpy, memset */
+#include 
 
 /* **
 *  Error Management
@@ -575,7 +576,7 @@ static size_t FSE_normalizeM2(short *norm, U32 tableLog, 
const unsigned *count,
{
U64 const vStepLog = 62 - tableLog;
U64 const mid = (1ULL << (vStepLog - 1)) - 1;
-   U64 const rStep = U64)1 << vStepLog) * ToDistribute) + mid) 
/ total; /* scale on remaining */
+   U64 const rStep = div_u64U64)1 << vStepLog) * ToDistribute) 
+ mid, total); /* scale on remaining */
U64 tmpTotal = mid;
for (s = 0; s <= maxSymbolValue; s++) {
if (norm[s] == NOT_YET_ASSIGNED) {
@@ -609,7 +610,7 @@ size_t FSE_normalizeCount(short *normalizedCounter, 
unsigned tableLog, const uns
{
U32 const rtbTable[] = {0, 473195, 504333, 520860, 55, 
70, 75, 83};
U64 const scale = 62 - tableLog;
-   U64 const step = ((U64)1 << 62) / total; /* <== here, one 
division ! */
+   U64 const step = div_u64((U64)1 << 62, total); /* <== here, one 
division ! */
U64 const vStep = 1ULL << (scale - 20);
int stillToDistribute = 1 << tableLog;
unsigned s;
-- 
2.13.1

Re: [PATCH 3/4] btrfs: Add zstd support

[Adam Borowski]

On Mon, Jun 26, 2017 at 03:03:17AM +0800, kbuild test robot wrote:
> Hi Nick,
> 
> url:
> https://github.com/0day-ci/linux/commits/Nick-Terrell/lib-Add-xxhash-module/20170625-214344
> config: i386-allmodconfig (attached as .config)
> compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
> reproduce:
> # save the attached .config to linux build tree
> make ARCH=i386 
> 
> All errors (new ones prefixed by >>):
> 
> >> ERROR: "__udivdi3" [lib/zstd/zstd_compress.ko] undefined!
>ERROR: "__udivdi3" [fs/ufs/ufs.ko] undefined!

Just to save you time to figure it out:
for division when one or both arguments are longer than the architecture's
word, gcc uses helper functions that are included when compiling in a hosted
environment -- but not in freestanding.

Thus, you want do_div() instead of /; do check widths and signedness of
arguments.


Meow!
-- 
⢀⣴⠾⠻⢶⣦⠀ 
⣾⠁⢠⠒⠀⣿⡁ A dumb species has no way to open a tuna can.
⢿⡄⠘⠷⠚⠋⠀ A smart species invents a can opener.
⠈⠳⣄ A master species delegates.

Re: [PATCH 3/4] btrfs: Add zstd support

[Adam Borowski]

On Mon, Jun 26, 2017 at 03:03:17AM +0800, kbuild test robot wrote:
> Hi Nick,
> 
> url:
> https://github.com/0day-ci/linux/commits/Nick-Terrell/lib-Add-xxhash-module/20170625-214344
> config: i386-allmodconfig (attached as .config)
> compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
> reproduce:
> # save the attached .config to linux build tree
> make ARCH=i386 
> 
> All errors (new ones prefixed by >>):
> 
> >> ERROR: "__udivdi3" [lib/zstd/zstd_compress.ko] undefined!
>ERROR: "__udivdi3" [fs/ufs/ufs.ko] undefined!

Just to save you time to figure it out:
for division when one or both arguments are longer than the architecture's
word, gcc uses helper functions that are included when compiling in a hosted
environment -- but not in freestanding.

Thus, you want do_div() instead of /; do check widths and signedness of
arguments.


Meow!
-- 
⢀⣴⠾⠻⢶⣦⠀ 
⣾⠁⢠⠒⠀⣿⡁ A dumb species has no way to open a tuna can.
⢿⡄⠘⠷⠚⠋⠀ A smart species invents a can opener.
⠈⠳⣄ A master species delegates.

Re: [PATCH] checkpatch: Change format of --color argument to --color[=WHEN]

[Adam Borowski]

On Mon, Jun 05, 2017 at 04:10:30PM -0700, Joe Perches wrote:
> On Mon, 2017-06-05 at 18:27 -0400, John Brooks wrote:
> > The boolean --color argument did not offer the ability to force colourized
> > output even if stdout is not a terminal.
> 
> OK, but why is colorizing output not to terminals desired?

* You may post-process the output somehow.  grep, sed, some highlighter...
* The output may go to less -R, ansi2html, etc.

I've made a tool that does what you want, "pipetty" (Debian package
colorized-logs, in stretch and jessie-backports), but that's a dirty hack. 
Lying about isatty() works for programs that check STDOUT but it's notorious
to instead look at STDIN, which can't be fooled in a reliable way.  Thus,
it's better to standardize on --color={always,auto,never}.


Meow!
-- 
⢀⣴⠾⠻⢶⣦⠀ A tit a day keeps the vet away.
⣾⠁⢰⠒⠀⣿⡁
⢿⡄⠘⠷⠚⠋⠀ (Rejoice as my small-animal-murder-machine got unbroken after
⠈⠳⣄ nearly two years of no catch!)

Re: [PATCH] checkpatch: Change format of --color argument to --color[=WHEN]

[Adam Borowski]

On Mon, Jun 05, 2017 at 04:10:30PM -0700, Joe Perches wrote:
> On Mon, 2017-06-05 at 18:27 -0400, John Brooks wrote:
> > The boolean --color argument did not offer the ability to force colourized
> > output even if stdout is not a terminal.
> 
> OK, but why is colorizing output not to terminals desired?

* You may post-process the output somehow.  grep, sed, some highlighter...
* The output may go to less -R, ansi2html, etc.

I've made a tool that does what you want, "pipetty" (Debian package
colorized-logs, in stretch and jessie-backports), but that's a dirty hack. 
Lying about isatty() works for programs that check STDOUT but it's notorious
to instead look at STDIN, which can't be fooled in a reliable way.  Thus,
it's better to standardize on --color={always,auto,never}.


Meow!
-- 
⢀⣴⠾⠻⢶⣦⠀ A tit a day keeps the vet away.
⣾⠁⢰⠒⠀⣿⡁
⢿⡄⠘⠷⠚⠋⠀ (Rejoice as my small-animal-murder-machine got unbroken after
⠈⠳⣄ nearly two years of no catch!)

[PATCH 3/5] vt: use copy_to_user instead of __put_user in GIO_UNIMAP ioctl

[Adam Borowski]

A nice big linear transfer, no need to flip stac/PAN/etc every half-entry.
Also, yay __put_user() after checking only read.

Signed-off-by: Adam Borowski <kilob...@angband.pl>
---
 drivers/tty/vt/consolemap.c | 14 ++
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/drivers/tty/vt/consolemap.c b/drivers/tty/vt/consolemap.c
index 1361f2a8b832..c6a692f63a9b 100644
--- a/drivers/tty/vt/consolemap.c
+++ b/drivers/tty/vt/consolemap.c
@@ -740,11 +740,11 @@ EXPORT_SYMBOL(con_copy_unimap);
  */
 int con_get_unimap(struct vc_data *vc, ushort ct, ushort __user *uct, struct 
unipair __user *list)
 {
-   int i, j, k;
+   int i, j, k, ret = 0;
ushort ect;
u16 **p1, *p2;
struct uni_pagedir *p;
-   struct unipair *unilist, *plist;
+   struct unipair *unilist;
 
unilist = kmalloc_array(ct, sizeof(struct unipair), GFP_KERNEL);
if (!unilist)
@@ -775,13 +775,11 @@ int con_get_unimap(struct vc_data *vc, ushort ct, ushort 
__user *uct, struct uni
}
}
console_unlock();
-   for (i = min(ect, ct), plist = unilist; i; i--, list++, plist++) {
-   __put_user(plist->unicode, >unicode);
-   __put_user(plist->fontpos, >fontpos);
-   }
-   __put_user(ect, uct);
+   if (copy_to_user(list, unilist, min(ect, ct) * sizeof(struct unipair)))
+   ret = -EFAULT;
+   put_user(ect, uct);
kfree(unilist);
-   return ((ect <= ct) ? 0 : -ENOMEM);
+   return ret ? ret : (ect <= ct) ? 0 : -ENOMEM;
 }
 
 /*
-- 
2.11.0

[PATCH 1/5] vt: use copy_from/to_user instead of __get/put_user for scrnmap ioctls

[Adam Borowski]

Linus wants to get rid of these functions, and these uses are especially
egregious: they copy a big linear array element by element.

Signed-off-by: Adam Borowski <kilob...@angband.pl>
---
 drivers/tty/vt/consolemap.c | 31 +++
 1 file changed, 7 insertions(+), 24 deletions(-)

diff --git a/drivers/tty/vt/consolemap.c b/drivers/tty/vt/consolemap.c
index 1f6e17fc3fb0..1361f2a8b832 100644
--- a/drivers/tty/vt/consolemap.c
+++ b/drivers/tty/vt/consolemap.c
@@ -322,15 +322,13 @@ int con_set_trans_old(unsigned char __user * arg)
 {
int i;
unsigned short inbuf[E_TABSZ];
+   unsigned char ubuf[E_TABSZ];
 
-   if (!access_ok(VERIFY_READ, arg, E_TABSZ))
+   if (copy_from_user(ubuf, arg, E_TABSZ))
return -EFAULT;
 
-   for (i = 0; i < E_TABSZ ; i++) {
-   unsigned char uc;
-   __get_user(uc, arg+i);
-   inbuf[i] = UNI_DIRECT_BASE | uc;
-   }
+   for (i = 0; i < E_TABSZ ; i++)
+   inbuf[i] = UNI_DIRECT_BASE | ubuf[i];
 
console_lock();
memcpy(translations[USER_MAP], inbuf, sizeof(inbuf));
@@ -345,9 +343,6 @@ int con_get_trans_old(unsigned char __user * arg)
unsigned short *p = translations[USER_MAP];
unsigned char outbuf[E_TABSZ];
 
-   if (!access_ok(VERIFY_WRITE, arg, E_TABSZ))
-   return -EFAULT;
-
console_lock();
for (i = 0; i < E_TABSZ ; i++)
{
@@ -356,22 +351,16 @@ int con_get_trans_old(unsigned char __user * arg)
}
console_unlock();
 
-   for (i = 0; i < E_TABSZ ; i++)
-   __put_user(outbuf[i], arg+i);
-   return 0;
+   return copy_to_user(arg, outbuf, sizeof(outbuf)) ? -EFAULT : 0;
 }
 
 int con_set_trans_new(ushort __user * arg)
 {
-   int i;
unsigned short inbuf[E_TABSZ];
 
-   if (!access_ok(VERIFY_READ, arg, E_TABSZ*sizeof(unsigned short)))
+   if (copy_from_user(inbuf, arg, sizeof(inbuf)))
return -EFAULT;
 
-   for (i = 0; i < E_TABSZ ; i++)
-   __get_user(inbuf[i], arg+i);
-
console_lock();
memcpy(translations[USER_MAP], inbuf, sizeof(inbuf));
update_user_maps();
@@ -381,19 +370,13 @@ int con_set_trans_new(ushort __user * arg)
 
 int con_get_trans_new(ushort __user * arg)
 {
-   int i;
unsigned short outbuf[E_TABSZ];
 
-   if (!access_ok(VERIFY_WRITE, arg, E_TABSZ*sizeof(unsigned short)))
-   return -EFAULT;
-
console_lock();
memcpy(outbuf, translations[USER_MAP], sizeof(outbuf));
console_unlock();
 
-   for (i = 0; i < E_TABSZ ; i++)
-   __put_user(outbuf[i], arg+i);
-   return 0;
+   return copy_to_user(arg, outbuf, sizeof(outbuf)) ? -EFAULT : 0;
 }
 
 /*
-- 
2.11.0

[PATCH 3/5] vt: use copy_to_user instead of __put_user in GIO_UNIMAP ioctl

[Adam Borowski]

A nice big linear transfer, no need to flip stac/PAN/etc every half-entry.
Also, yay __put_user() after checking only read.

Signed-off-by: Adam Borowski 
---
 drivers/tty/vt/consolemap.c | 14 ++
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/drivers/tty/vt/consolemap.c b/drivers/tty/vt/consolemap.c
index 1361f2a8b832..c6a692f63a9b 100644
--- a/drivers/tty/vt/consolemap.c
+++ b/drivers/tty/vt/consolemap.c
@@ -740,11 +740,11 @@ EXPORT_SYMBOL(con_copy_unimap);
  */
 int con_get_unimap(struct vc_data *vc, ushort ct, ushort __user *uct, struct 
unipair __user *list)
 {
-   int i, j, k;
+   int i, j, k, ret = 0;
ushort ect;
u16 **p1, *p2;
struct uni_pagedir *p;
-   struct unipair *unilist, *plist;
+   struct unipair *unilist;
 
unilist = kmalloc_array(ct, sizeof(struct unipair), GFP_KERNEL);
if (!unilist)
@@ -775,13 +775,11 @@ int con_get_unimap(struct vc_data *vc, ushort ct, ushort 
__user *uct, struct uni
}
}
console_unlock();
-   for (i = min(ect, ct), plist = unilist; i; i--, list++, plist++) {
-   __put_user(plist->unicode, >unicode);
-   __put_user(plist->fontpos, >fontpos);
-   }
-   __put_user(ect, uct);
+   if (copy_to_user(list, unilist, min(ect, ct) * sizeof(struct unipair)))
+   ret = -EFAULT;
+   put_user(ect, uct);
kfree(unilist);
-   return ((ect <= ct) ? 0 : -ENOMEM);
+   return ret ? ret : (ect <= ct) ? 0 : -ENOMEM;
 }
 
 /*
-- 
2.11.0

[PATCH 1/5] vt: use copy_from/to_user instead of __get/put_user for scrnmap ioctls

[Adam Borowski]

Linus wants to get rid of these functions, and these uses are especially
egregious: they copy a big linear array element by element.

Signed-off-by: Adam Borowski 
---
 drivers/tty/vt/consolemap.c | 31 +++
 1 file changed, 7 insertions(+), 24 deletions(-)

diff --git a/drivers/tty/vt/consolemap.c b/drivers/tty/vt/consolemap.c
index 1f6e17fc3fb0..1361f2a8b832 100644
--- a/drivers/tty/vt/consolemap.c
+++ b/drivers/tty/vt/consolemap.c
@@ -322,15 +322,13 @@ int con_set_trans_old(unsigned char __user * arg)
 {
int i;
unsigned short inbuf[E_TABSZ];
+   unsigned char ubuf[E_TABSZ];
 
-   if (!access_ok(VERIFY_READ, arg, E_TABSZ))
+   if (copy_from_user(ubuf, arg, E_TABSZ))
return -EFAULT;
 
-   for (i = 0; i < E_TABSZ ; i++) {
-   unsigned char uc;
-   __get_user(uc, arg+i);
-   inbuf[i] = UNI_DIRECT_BASE | uc;
-   }
+   for (i = 0; i < E_TABSZ ; i++)
+   inbuf[i] = UNI_DIRECT_BASE | ubuf[i];
 
console_lock();
memcpy(translations[USER_MAP], inbuf, sizeof(inbuf));
@@ -345,9 +343,6 @@ int con_get_trans_old(unsigned char __user * arg)
unsigned short *p = translations[USER_MAP];
unsigned char outbuf[E_TABSZ];
 
-   if (!access_ok(VERIFY_WRITE, arg, E_TABSZ))
-   return -EFAULT;
-
console_lock();
for (i = 0; i < E_TABSZ ; i++)
{
@@ -356,22 +351,16 @@ int con_get_trans_old(unsigned char __user * arg)
}
console_unlock();
 
-   for (i = 0; i < E_TABSZ ; i++)
-   __put_user(outbuf[i], arg+i);
-   return 0;
+   return copy_to_user(arg, outbuf, sizeof(outbuf)) ? -EFAULT : 0;
 }
 
 int con_set_trans_new(ushort __user * arg)
 {
-   int i;
unsigned short inbuf[E_TABSZ];
 
-   if (!access_ok(VERIFY_READ, arg, E_TABSZ*sizeof(unsigned short)))
+   if (copy_from_user(inbuf, arg, sizeof(inbuf)))
return -EFAULT;
 
-   for (i = 0; i < E_TABSZ ; i++)
-   __get_user(inbuf[i], arg+i);
-
console_lock();
memcpy(translations[USER_MAP], inbuf, sizeof(inbuf));
update_user_maps();
@@ -381,19 +370,13 @@ int con_set_trans_new(ushort __user * arg)
 
 int con_get_trans_new(ushort __user * arg)
 {
-   int i;
unsigned short outbuf[E_TABSZ];
 
-   if (!access_ok(VERIFY_WRITE, arg, E_TABSZ*sizeof(unsigned short)))
-   return -EFAULT;
-
console_lock();
memcpy(outbuf, translations[USER_MAP], sizeof(outbuf));
console_unlock();
 
-   for (i = 0; i < E_TABSZ ; i++)
-   __put_user(outbuf[i], arg+i);
-   return 0;
+   return copy_to_user(arg, outbuf, sizeof(outbuf)) ? -EFAULT : 0;
 }
 
 /*
-- 
2.11.0

[PATCH 4/5] vt: use memdup_user in PIO_UNIMAP ioctl

[Adam Borowski]

Again, a nice linear transfer that simplifies the code.

Signed-off-by: Adam Borowski <kilob...@angband.pl>
---
 drivers/tty/vt/consolemap.c | 11 +++
 1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/drivers/tty/vt/consolemap.c b/drivers/tty/vt/consolemap.c
index c6a692f63a9b..a5f88cf0f61d 100644
--- a/drivers/tty/vt/consolemap.c
+++ b/drivers/tty/vt/consolemap.c
@@ -540,14 +540,9 @@ int con_set_unimap(struct vc_data *vc, ushort ct, struct 
unipair __user *list)
if (!ct)
return 0;
 
-   unilist = kmalloc_array(ct, sizeof(struct unipair), GFP_KERNEL);
-   if (!unilist)
-   return -ENOMEM;
-
-   for (i = ct, plist = unilist; i; i--, plist++, list++) {
-   __get_user(plist->unicode, >unicode);
-   __get_user(plist->fontpos, >fontpos);
-   }
+   unilist = memdup_user(list, ct * sizeof(struct unipair));
+   if (IS_ERR(unilist))
+   return PTR_ERR(unilist);
 
console_lock();
 
-- 
2.11.0

[PATCH 4/5] vt: use memdup_user in PIO_UNIMAP ioctl

[Adam Borowski]

Again, a nice linear transfer that simplifies the code.

Signed-off-by: Adam Borowski 
---
 drivers/tty/vt/consolemap.c | 11 +++
 1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/drivers/tty/vt/consolemap.c b/drivers/tty/vt/consolemap.c
index c6a692f63a9b..a5f88cf0f61d 100644
--- a/drivers/tty/vt/consolemap.c
+++ b/drivers/tty/vt/consolemap.c
@@ -540,14 +540,9 @@ int con_set_unimap(struct vc_data *vc, ushort ct, struct 
unipair __user *list)
if (!ct)
return 0;
 
-   unilist = kmalloc_array(ct, sizeof(struct unipair), GFP_KERNEL);
-   if (!unilist)
-   return -ENOMEM;
-
-   for (i = ct, plist = unilist; i; i--, plist++, list++) {
-   __get_user(plist->unicode, >unicode);
-   __get_user(plist->fontpos, >fontpos);
-   }
+   unilist = memdup_user(list, ct * sizeof(struct unipair));
+   if (IS_ERR(unilist))
+   return PTR_ERR(unilist);
 
console_lock();
 
-- 
2.11.0

[PATCH 5/5] vt: drop access_ok() calls in unimap ioctls

[Adam Borowski]

Done by copy_{from,to}_user().

Signed-off-by: Adam Borowski <kilob...@angband.pl>
---
 drivers/tty/vt/vt_ioctl.c | 8 
 1 file changed, 8 deletions(-)

diff --git a/drivers/tty/vt/vt_ioctl.c b/drivers/tty/vt/vt_ioctl.c
index 0cbfe1ff6f6c..96d389cb506c 100644
--- a/drivers/tty/vt/vt_ioctl.c
+++ b/drivers/tty/vt/vt_ioctl.c
@@ -266,10 +266,6 @@ do_unimap_ioctl(int cmd, struct unimapdesc __user 
*user_ud, int perm, struct vc_
 
if (copy_from_user(, user_ud, sizeof tmp))
return -EFAULT;
-   if (tmp.entries)
-   if (!access_ok(VERIFY_WRITE, tmp.entries,
-   tmp.entry_ct*sizeof(struct unipair)))
-   return -EFAULT;
switch (cmd) {
case PIO_UNIMAP:
if (!perm)
@@ -1170,10 +1166,6 @@ compat_unimap_ioctl(unsigned int cmd, struct 
compat_unimapdesc __user *user_ud,
if (copy_from_user(, user_ud, sizeof tmp))
return -EFAULT;
tmp_entries = compat_ptr(tmp.entries);
-   if (tmp_entries)
-   if (!access_ok(VERIFY_WRITE, tmp_entries,
-   tmp.entry_ct*sizeof(struct unipair)))
-   return -EFAULT;
switch (cmd) {
case PIO_UNIMAP:
if (!perm)
-- 
2.11.0

[PATCH 2/5] vt: fix unchecked __put_user() in tioclinux ioctls

[Adam Borowski]

Only read access is checked before this call.

Actually, at the moment this is not an issue, as every in-tree arch does
the same manual checks for VERIFY_READ vs VERIFY_WRITE, relying on the MMU
to tell them apart, but this wasn't the case in the past and may happen
again on some odd arch in the future.

If anyone cares about 3.7 and earlier, this is a security hole (untested)
on real 80386 CPUs.

Signed-off-by: Adam Borowski <kilob...@angband.pl>
CC: sta...@vger.kernel.org # v3.7-
---
 drivers/tty/vt/vt.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c
index 9309c7da660a..2ebaba16f785 100644
--- a/drivers/tty/vt/vt.c
+++ b/drivers/tty/vt/vt.c
@@ -2709,13 +2709,13 @@ int tioclinux(struct tty_struct *tty, unsigned long arg)
 * related to the kernel should not use this.
 */
data = vt_get_shift_state();
-   ret = __put_user(data, p);
+   ret = put_user(data, p);
break;
case TIOCL_GETMOUSEREPORTING:
console_lock(); /* May be overkill */
data = mouse_reporting();
console_unlock();
-   ret = __put_user(data, p);
+   ret = put_user(data, p);
break;
case TIOCL_SETVESABLANK:
console_lock();
@@ -2724,7 +2724,7 @@ int tioclinux(struct tty_struct *tty, unsigned long arg)
break;
case TIOCL_GETKMSGREDIRECT:
data = vt_get_kmsg_redirect();
-   ret = __put_user(data, p);
+   ret = put_user(data, p);
break;
case TIOCL_SETKMSGREDIRECT:
if (!capable(CAP_SYS_ADMIN)) {
-- 
2.11.0

[PATCH 5/5] vt: drop access_ok() calls in unimap ioctls

[Adam Borowski]

Done by copy_{from,to}_user().

Signed-off-by: Adam Borowski 
---
 drivers/tty/vt/vt_ioctl.c | 8 
 1 file changed, 8 deletions(-)

diff --git a/drivers/tty/vt/vt_ioctl.c b/drivers/tty/vt/vt_ioctl.c
index 0cbfe1ff6f6c..96d389cb506c 100644
--- a/drivers/tty/vt/vt_ioctl.c
+++ b/drivers/tty/vt/vt_ioctl.c
@@ -266,10 +266,6 @@ do_unimap_ioctl(int cmd, struct unimapdesc __user 
*user_ud, int perm, struct vc_
 
if (copy_from_user(, user_ud, sizeof tmp))
return -EFAULT;
-   if (tmp.entries)
-   if (!access_ok(VERIFY_WRITE, tmp.entries,
-   tmp.entry_ct*sizeof(struct unipair)))
-   return -EFAULT;
switch (cmd) {
case PIO_UNIMAP:
if (!perm)
@@ -1170,10 +1166,6 @@ compat_unimap_ioctl(unsigned int cmd, struct 
compat_unimapdesc __user *user_ud,
if (copy_from_user(, user_ud, sizeof tmp))
return -EFAULT;
tmp_entries = compat_ptr(tmp.entries);
-   if (tmp_entries)
-   if (!access_ok(VERIFY_WRITE, tmp_entries,
-   tmp.entry_ct*sizeof(struct unipair)))
-   return -EFAULT;
switch (cmd) {
case PIO_UNIMAP:
if (!perm)
-- 
2.11.0

[PATCH 2/5] vt: fix unchecked __put_user() in tioclinux ioctls

[Adam Borowski]

Only read access is checked before this call.

Actually, at the moment this is not an issue, as every in-tree arch does
the same manual checks for VERIFY_READ vs VERIFY_WRITE, relying on the MMU
to tell them apart, but this wasn't the case in the past and may happen
again on some odd arch in the future.

If anyone cares about 3.7 and earlier, this is a security hole (untested)
on real 80386 CPUs.

Signed-off-by: Adam Borowski 
CC: sta...@vger.kernel.org # v3.7-
---
 drivers/tty/vt/vt.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c
index 9309c7da660a..2ebaba16f785 100644
--- a/drivers/tty/vt/vt.c
+++ b/drivers/tty/vt/vt.c
@@ -2709,13 +2709,13 @@ int tioclinux(struct tty_struct *tty, unsigned long arg)
 * related to the kernel should not use this.
 */
data = vt_get_shift_state();
-   ret = __put_user(data, p);
+   ret = put_user(data, p);
break;
case TIOCL_GETMOUSEREPORTING:
console_lock(); /* May be overkill */
data = mouse_reporting();
console_unlock();
-   ret = __put_user(data, p);
+   ret = put_user(data, p);
break;
case TIOCL_SETVESABLANK:
console_lock();
@@ -2724,7 +2724,7 @@ int tioclinux(struct tty_struct *tty, unsigned long arg)
break;
case TIOCL_GETKMSGREDIRECT:
data = vt_get_kmsg_redirect();
-   ret = __put_user(data, p);
+   ret = put_user(data, p);
break;
case TIOCL_SETKMSGREDIRECT:
if (!capable(CAP_SYS_ADMIN)) {
-- 
2.11.0

[PATCH 0/5] vt: get rid of worst cases of __put_user/__get_user

[Adam Borowski]

Hi!
In a recent discussion, Linus and Al Viro said quite a bit of expletives
about __put_user() and __get_user(), that it's a bad interface that's
almost always the wrong thing to use:
https://marc.info/?l=linux-kernel=149463725626316=2
https://marc.info/?l=linux-kernel=149465866929092=2

Here's a few patches applying the lessons from that discussion to vt.
None of the uses is performance-critical, but at least we get a nice bit
of code simplification.  And, it's a start of manual review + conversion
that Al Viro wants.


Adam Borowski (5):
  vt: use copy_from/to_user instead of __get/put_user for scrnmap ioctls
  vt: fix unchecked __put_user() in tioclinux ioctls
  vt: use copy_to_user instead of __put_user in GIO_UNIMAP ioctl
  vt: use memdup_user in PIO_UNIMAP ioctl
  vt: drop access_ok() calls in unimap ioctls

 drivers/tty/vt/consolemap.c | 56 

 drivers/tty/vt/vt.c |  6 +++---
 drivers/tty/vt/vt_ioctl.c   |  8 
 3 files changed, 19 insertions(+), 51 deletions(-)

-- 
⢀⣴⠾⠻⢶⣦⠀ A tit a day keeps the vet away.
⣾⠁⢰⠒⠀⣿⡁
⢿⡄⠘⠷⠚⠋⠀ (Rejoice as my small-animal-murder-machine got unbroken after
⠈⠳⣄ nearly two years of no catch!)

[PATCH 0/5] vt: get rid of worst cases of __put_user/__get_user

[Adam Borowski]

Hi!
In a recent discussion, Linus and Al Viro said quite a bit of expletives
about __put_user() and __get_user(), that it's a bad interface that's
almost always the wrong thing to use:
https://marc.info/?l=linux-kernel=149463725626316=2
https://marc.info/?l=linux-kernel=149465866929092=2

Here's a few patches applying the lessons from that discussion to vt.
None of the uses is performance-critical, but at least we get a nice bit
of code simplification.  And, it's a start of manual review + conversion
that Al Viro wants.


Adam Borowski (5):
  vt: use copy_from/to_user instead of __get/put_user for scrnmap ioctls
  vt: fix unchecked __put_user() in tioclinux ioctls
  vt: use copy_to_user instead of __put_user in GIO_UNIMAP ioctl
  vt: use memdup_user in PIO_UNIMAP ioctl
  vt: drop access_ok() calls in unimap ioctls

 drivers/tty/vt/consolemap.c | 56 

 drivers/tty/vt/vt.c |  6 +++---
 drivers/tty/vt/vt_ioctl.c   |  8 
 3 files changed, 19 insertions(+), 51 deletions(-)

-- 
⢀⣴⠾⠻⢶⣦⠀ A tit a day keeps the vet away.
⣾⠁⢰⠒⠀⣿⡁
⢿⡄⠘⠷⠚⠋⠀ (Rejoice as my small-animal-murder-machine got unbroken after
⠈⠳⣄ nearly two years of no catch!)

[PATCH] vt: fix \e[2m using the wrong placeholder color on graphical consoles

[Adam Borowski]

Only vgacon and sisusbcon did it right, the rest (via generic code) tried
underline (usually cyan).

Signed-off-by: Adam Borowski <kilob...@angband.pl>
---
Compare this if clause with the two above it.  Nice copypaste. :)

 drivers/tty/vt/vt.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c
index 9c9945284bcf..9309c7da660a 100644
--- a/drivers/tty/vt/vt.c
+++ b/drivers/tty/vt/vt.c
@@ -425,7 +425,7 @@ static u8 build_attr(struct vc_data *vc, u8 _color, u8 
_intensity, u8 _blink,
else if (_underline)
a = (a & 0xf0) | vc->vc_ulcolor;
else if (_intensity == 0)
-   a = (a & 0xf0) | vc->vc_ulcolor;
+   a = (a & 0xf0) | vc->vc_halfcolor;
if (_reverse)
a = ((a) & 0x88) | a) >> 4) | ((a) << 4)) & 0x77);
if (_blink)
-- 
2.11.0

[PATCH] vt: fix \e[2m using the wrong placeholder color on graphical consoles

[Adam Borowski]

Only vgacon and sisusbcon did it right, the rest (via generic code) tried
underline (usually cyan).

Signed-off-by: Adam Borowski 
---
Compare this if clause with the two above it.  Nice copypaste. :)

 drivers/tty/vt/vt.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c
index 9c9945284bcf..9309c7da660a 100644
--- a/drivers/tty/vt/vt.c
+++ b/drivers/tty/vt/vt.c
@@ -425,7 +425,7 @@ static u8 build_attr(struct vc_data *vc, u8 _color, u8 
_intensity, u8 _blink,
else if (_underline)
a = (a & 0xf0) | vc->vc_ulcolor;
else if (_intensity == 0)
-   a = (a & 0xf0) | vc->vc_ulcolor;
+   a = (a & 0xf0) | vc->vc_halfcolor;
if (_reverse)
a = ((a) & 0x88) | a) >> 4) | ((a) << 4)) & 0x77);
if (_blink)
-- 
2.11.0

Re: [PATCH 0/2] blackfin: Remove dead DSA code

[Adam Borowski]

On Sun, May 28, 2017 at 04:59:47PM -0700, Florian Fainelli wrote:
> Hello? anyone still maintaining blackfin here?

Looks like people edit arch/blackfin/ a lot whenever it interferes with some
other work, but the only blackfin-specific fixes seem to be a couple of
drive-by ones by Al Viro, then nothing until first half of 2015.

Last maintainer pull request: 668b54a1 on 2015-04-24.

There was some mailing list traffic in Jan 2016:
https://sourceforge.net/p/adi-buildroot/mailman/adi-buildroot-devel/

So it looks pretty dead...

-- 
Don't be racist.  White, amber or black, all beers should be judged based
solely on their merits.  Heck, even if occasionally a cider applies for a
beer's job, why not?
On the other hand, corpo lager is not a race.

Re: [PATCH 0/2] blackfin: Remove dead DSA code

[Adam Borowski]

On Sun, May 28, 2017 at 04:59:47PM -0700, Florian Fainelli wrote:
> Hello? anyone still maintaining blackfin here?

Looks like people edit arch/blackfin/ a lot whenever it interferes with some
other work, but the only blackfin-specific fixes seem to be a couple of
drive-by ones by Al Viro, then nothing until first half of 2015.

Last maintainer pull request: 668b54a1 on 2015-04-24.

There was some mailing list traffic in Jan 2016:
https://sourceforge.net/p/adi-buildroot/mailman/adi-buildroot-devel/

So it looks pretty dead...

-- 
Don't be racist.  White, amber or black, all beers should be judged based
solely on their merits.  Heck, even if occasionally a cider applies for a
beer's job, why not?
On the other hand, corpo lager is not a race.

Re: [git pull] uaccess-related bits of vfs.git

[Adam Borowski]

On Sat, May 13, 2017 at 07:57:45AM +0100, Al Viro wrote:
> On Fri, May 12, 2017 at 06:00:44PM -0700, Linus Torvalds wrote:
> > But the *first* thing I'd like to do would be to just get rid of
> > __get_user/__put_user as a thing. It really does generate nasty code,
> > and we might as well just make it do that function call.
> 
> But IMO the first step is not to convert __get_user()/__put_user() to
> aliases of get_user()/put_user() - it's to get rid of their infestations
> outside of arch/*.  They are concentrated in relatively few places, so
> we can deal with those individually and then just fucking ban their use
> outside of arch/*.  That's easy enough to watch for - simple git grep will do,
> so anything creeping back will be immediately spotted.

As someone from the peanuts gallery, I took a look for __put_user() in my
usual haunt, drivers/tty/vt/

* use 1: con_[gs]et_trans_*():
  Copies a linear array of 256 bytes/shorts, one by one.
  The obvious patch has 9 insertions(+), 22 deletions(-).

* use 2: con_[gs]et_unimap():
  Ditto, up to 65535*2 shorts, also in a nice linear array.

* use 3: tioclinux():
  Does a __put into a place that was checked only for read.  This got me
  frightened as it initially looked like something that can allow an user to
  write where they shouldn't.  Fortunately, it turns out the first argument
  to access_ok() is ignored on every single architecture -- why does it even
  exist then?  I imagined it's there for some odd arch that allows writes
  when in privileged mode, but unlike what the docs say, VERIFY_WRITE is
  exactly same as VERIFY_READ.

Ie, every use in this sample is wrong.  I suspect the rest of the kernel
should be similar.


Meow!
-- 
Don't be racist.  White, amber or black, all beers should be judged based
solely on their merits.  Heck, even if occasionally a cider applies for a
beer's job, why not?
On the other hand, corpo lager is not a race.

Re: [git pull] uaccess-related bits of vfs.git

[Adam Borowski]

On Sat, May 13, 2017 at 07:57:45AM +0100, Al Viro wrote:
> On Fri, May 12, 2017 at 06:00:44PM -0700, Linus Torvalds wrote:
> > But the *first* thing I'd like to do would be to just get rid of
> > __get_user/__put_user as a thing. It really does generate nasty code,
> > and we might as well just make it do that function call.
> 
> But IMO the first step is not to convert __get_user()/__put_user() to
> aliases of get_user()/put_user() - it's to get rid of their infestations
> outside of arch/*.  They are concentrated in relatively few places, so
> we can deal with those individually and then just fucking ban their use
> outside of arch/*.  That's easy enough to watch for - simple git grep will do,
> so anything creeping back will be immediately spotted.

As someone from the peanuts gallery, I took a look for __put_user() in my
usual haunt, drivers/tty/vt/

* use 1: con_[gs]et_trans_*():
  Copies a linear array of 256 bytes/shorts, one by one.
  The obvious patch has 9 insertions(+), 22 deletions(-).

* use 2: con_[gs]et_unimap():
  Ditto, up to 65535*2 shorts, also in a nice linear array.

* use 3: tioclinux():
  Does a __put into a place that was checked only for read.  This got me
  frightened as it initially looked like something that can allow an user to
  write where they shouldn't.  Fortunately, it turns out the first argument
  to access_ok() is ignored on every single architecture -- why does it even
  exist then?  I imagined it's there for some odd arch that allows writes
  when in privileged mode, but unlike what the docs say, VERIFY_WRITE is
  exactly same as VERIFY_READ.

Ie, every use in this sample is wrong.  I suspect the rest of the kernel
should be similar.


Meow!
-- 
Don't be racist.  White, amber or black, all beers should be judged based
solely on their merits.  Heck, even if occasionally a cider applies for a
beer's job, why not?
On the other hand, corpo lager is not a race.

Re: sun50i-a64-pinctrl WARN_ON drivers/base/dd.c:349

[Adam Borowski]

On Fri, Apr 28, 2017 at 06:03:14PM -0400, Tejun Heo wrote:
> On Tue, Apr 18, 2017 at 10:12:16AM +0100, Andre Przywara wrote:
> > Yeah, so I stack-dumped on the zero allocations and indeed they are
> > called from cleanup functions:
> > drivers/pinctrl/pinmux.c:pinmux_generic_free_functions():
> > devm_kzalloc(sizeof(*indices) * pctldev->num_functions, ...)
> > (and another one I don't know from the top of the my head, logs at home)
> >
> > So my hunch was that once EPROBE_DEFER triggers the devres cleanup, it
> > uses some reverse list traversal to release all allocated resources
> > (backwards!), so missing those which get (appended) during the process.
> > But I don't think that would not work with the locking.
> > So I have to dig deeper tonight in my logs.
> 
> If this is a valid use case, we can change devm to repeat till empty
> but it's a weird thing to do to allocate from a release function.
> 
> So, something like this.  Only compile tested.

I've tested this, and the results are mixed.  It _does_ make the WARN go
away, but on the other hand, much later during the boot, another driver
(sun8i-emac) gets deferred and never undefers.

Without the patch, there's:
[ ok ] Synthesizing the initial hotplug events...done.
[6.666984] bus: 'platform': driver_probe_device: matched device 
1c3.ethernet with driver sun8i-emac
[6.676520] bus: 'platform': really_probe: probing driver sun8i-emac with 
device 1c3.ethernet
[6.686479] driver: 'sun8i-emac': driver_bound: bound to device 
'1c3.ethernet'
[] [6.694132] bus: 'platform': really_probe: bound device 
1c3.ethernet to driver sun8i-emac
[ ok ng for /dev to be fully populated...done.

[] Configuring network interfaces...[   11.414884] libphy: 
1c3.ethernet: probed
[   11.449742] bus: 'mdio_bus': driver_probe_device: matched device 
1c3.ethernet-0:00 with driver RTL8211E Gigabit Ethernet
[   11.461013] bus: 'mdio_bus': really_probe: probing driver RTL8211E Gigabit 
Ethernet with device 1c3.ethernet-0:00
[   11.471666] driver: 'RTL8211E Gigabit Ethernet': driver_bound: bound to 
device '1c3.ethernet-0:00'
[   11.481007] bus: 'mdio_bus': really_probe: bound device 
1c3.ethernet-0:00 to driver RTL8211E Gigabit Ethernet
[   11.491527] RTL8211E Gigabit Ethernet 1c3.ethernet-0:00: attached PHY 
driver [RTL8211E Gigabit Ethernet] (mii_bus:phy_addr=1c3.ethernet-0:00, 
irq=-1)
[   11.505608] sun8i-emac 1c3.ethernet: device MAC address slot 0 
02:ba:8f:9e:44:8f

With the patch:
[ ok ] Synthesizing the initial hotplug events...done.
[] Waiting for /dev to be fully populated...[6.325314] bus: 'platform': 
driver_probe_device: matched device 1c3.ethernet with driver sun8i-emac
[6.334842] bus: 'platform': really_probe: probing driver sun8i-emac with 
device 1c3.ethernet
[6.344253] platform 1c3.ethernet: Driver sun8i-emac requests probe 
deferral
[6.351662] platform 1c3.ethernet: Added to deferred list
done.

[] Configuring network interfaces...Cannot find device "eth0"
ifup: failed to bring up eth0
failed.


There's a concern, though, that this second driver is not in mainline (and
won't ever be -- it's being replaced by dwmac-sun8i, but the latter is not
yet up to scratch), thus I'm not sure how relevant this is.

The second concern is that I'm using the same tree
(icenowy/sunxi64-4.11-rc7) and config (plus filesystems) as Icenowy, on the
same hardware, yet I did see this warning in versions much earlier than her,
and it doesn't go away in -next for me either.  All that differs are
toolchain versions (I'm on Debian unstable) and perhaps some u-boot bits.

Because of the amount of churn in sunxi land and the driver's mainlining
status, I think it'd be good to wait after the merge window that starts
tomorrow, and re-test then.

Full console output with the patch applied attached.


Meow!


> diff --git a/drivers/base/devres.c b/drivers/base/devres.c
> index 71d5770..d2a9f34 100644
> --- a/drivers/base/devres.c
> +++ b/drivers/base/devres.c
> @@ -509,13 +509,21 @@ static int release_nodes(struct device *dev, struct 
> list_head *first,
>  int devres_release_all(struct device *dev)
>  {
>   unsigned long flags;
> + int cnt = 0, ret;
>  
>   /* Looks like an uninitialized device structure */
>   if (WARN_ON(dev->devres_head.next == NULL))
>   return -ENODEV;
> - spin_lock_irqsave(>devres_lock, flags);
> - return release_nodes(dev, dev->devres_head.next, >devres_head,
> -  flags);
> +
> + /* Release callbacks may create new nodes, repeat till empty */
> + do {
> + spin_lock_irqsave(>devres_lock, flags);
> + ret = release_nodes(dev, dev->devres_head.next,
> + >devres_head, flags);
> + cnt += ret;
> + } while (ret);
> +
> + return cnt;
>  }
>  
>  /**
> 

-- 
Don't be racist.  White, amber or black, all beers should be judged 

Re: sun50i-a64-pinctrl WARN_ON drivers/base/dd.c:349

[Adam Borowski]

On Fri, Apr 28, 2017 at 06:03:14PM -0400, Tejun Heo wrote:
> On Tue, Apr 18, 2017 at 10:12:16AM +0100, Andre Przywara wrote:
> > Yeah, so I stack-dumped on the zero allocations and indeed they are
> > called from cleanup functions:
> > drivers/pinctrl/pinmux.c:pinmux_generic_free_functions():
> > devm_kzalloc(sizeof(*indices) * pctldev->num_functions, ...)
> > (and another one I don't know from the top of the my head, logs at home)
> >
> > So my hunch was that once EPROBE_DEFER triggers the devres cleanup, it
> > uses some reverse list traversal to release all allocated resources
> > (backwards!), so missing those which get (appended) during the process.
> > But I don't think that would not work with the locking.
> > So I have to dig deeper tonight in my logs.
> 
> If this is a valid use case, we can change devm to repeat till empty
> but it's a weird thing to do to allocate from a release function.
> 
> So, something like this.  Only compile tested.

I've tested this, and the results are mixed.  It _does_ make the WARN go
away, but on the other hand, much later during the boot, another driver
(sun8i-emac) gets deferred and never undefers.

Without the patch, there's:
[ ok ] Synthesizing the initial hotplug events...done.
[6.666984] bus: 'platform': driver_probe_device: matched device 
1c3.ethernet with driver sun8i-emac
[6.676520] bus: 'platform': really_probe: probing driver sun8i-emac with 
device 1c3.ethernet
[6.686479] driver: 'sun8i-emac': driver_bound: bound to device 
'1c3.ethernet'
[] [6.694132] bus: 'platform': really_probe: bound device 
1c3.ethernet to driver sun8i-emac
[ ok ng for /dev to be fully populated...done.

[] Configuring network interfaces...[   11.414884] libphy: 
1c3.ethernet: probed
[   11.449742] bus: 'mdio_bus': driver_probe_device: matched device 
1c3.ethernet-0:00 with driver RTL8211E Gigabit Ethernet
[   11.461013] bus: 'mdio_bus': really_probe: probing driver RTL8211E Gigabit 
Ethernet with device 1c3.ethernet-0:00
[   11.471666] driver: 'RTL8211E Gigabit Ethernet': driver_bound: bound to 
device '1c3.ethernet-0:00'
[   11.481007] bus: 'mdio_bus': really_probe: bound device 
1c3.ethernet-0:00 to driver RTL8211E Gigabit Ethernet
[   11.491527] RTL8211E Gigabit Ethernet 1c3.ethernet-0:00: attached PHY 
driver [RTL8211E Gigabit Ethernet] (mii_bus:phy_addr=1c3.ethernet-0:00, 
irq=-1)
[   11.505608] sun8i-emac 1c3.ethernet: device MAC address slot 0 
02:ba:8f:9e:44:8f

With the patch:
[ ok ] Synthesizing the initial hotplug events...done.
[] Waiting for /dev to be fully populated...[6.325314] bus: 'platform': 
driver_probe_device: matched device 1c3.ethernet with driver sun8i-emac
[6.334842] bus: 'platform': really_probe: probing driver sun8i-emac with 
device 1c3.ethernet
[6.344253] platform 1c3.ethernet: Driver sun8i-emac requests probe 
deferral
[6.351662] platform 1c3.ethernet: Added to deferred list
done.

[] Configuring network interfaces...Cannot find device "eth0"
ifup: failed to bring up eth0
failed.


There's a concern, though, that this second driver is not in mainline (and
won't ever be -- it's being replaced by dwmac-sun8i, but the latter is not
yet up to scratch), thus I'm not sure how relevant this is.

The second concern is that I'm using the same tree
(icenowy/sunxi64-4.11-rc7) and config (plus filesystems) as Icenowy, on the
same hardware, yet I did see this warning in versions much earlier than her,
and it doesn't go away in -next for me either.  All that differs are
toolchain versions (I'm on Debian unstable) and perhaps some u-boot bits.

Because of the amount of churn in sunxi land and the driver's mainlining
status, I think it'd be good to wait after the merge window that starts
tomorrow, and re-test then.

Full console output with the patch applied attached.


Meow!


> diff --git a/drivers/base/devres.c b/drivers/base/devres.c
> index 71d5770..d2a9f34 100644
> --- a/drivers/base/devres.c
> +++ b/drivers/base/devres.c
> @@ -509,13 +509,21 @@ static int release_nodes(struct device *dev, struct 
> list_head *first,
>  int devres_release_all(struct device *dev)
>  {
>   unsigned long flags;
> + int cnt = 0, ret;
>  
>   /* Looks like an uninitialized device structure */
>   if (WARN_ON(dev->devres_head.next == NULL))
>   return -ENODEV;
> - spin_lock_irqsave(>devres_lock, flags);
> - return release_nodes(dev, dev->devres_head.next, >devres_head,
> -  flags);
> +
> + /* Release callbacks may create new nodes, repeat till empty */
> + do {
> + spin_lock_irqsave(>devres_lock, flags);
> + ret = release_nodes(dev, dev->devres_head.next,
> + >devres_head, flags);
> + cnt += ret;
> + } while (ret);
> +
> + return cnt;
>  }
>  
>  /**
> 

-- 
Don't be racist.  White, amber or black, all beers should be judged 

Re: [PATCH] btrfs: scrub: use do_div() for 64-by-32 division

[Adam Borowski]

On Sun, Apr 09, 2017 at 05:58:54AM +0200, Adam Borowski wrote:
> On Sat, Apr 08, 2017 at 11:07:37PM +0200, Adam Borowski wrote:
> > Unbreaks ARM and possibly other 32-bit architectures.
> 
> Turns out those "other 32-bit architectures" happen to include i386.
> 
> A modular build:
> ERROR: "__udivdi3" [fs/btrfs/btrfs.ko] undefined!
> With the patch, i386 builds fine.
> 
> > Tested on amd64 where all is fine, and on arm (Odroid-U2) where scrub
> > sometimes works, but, like most operations, randomly dies with some badness
> > that doesn't look related: io_schedule, kunmap_high.  That badness wasn't
> > there in 4.11-rc5, needs investigating, but since it's not connected to our
> > issue at hand, I consider this patch sort-of tested.
> 
> Looks like current -next is pretty broken: while amd64 is ok, on an i386 box
> (non-NX Pentium 4) it hangs very early during boot, way before filesystem
> modules would be loaded.  Qemu boots but has random hangs.

A non-modular i386_defconfig + btrfs of -next is ok; whatever the problem
is, it's not relevant to our division build failure in scrub.

But, it looks like parity scrub is ${EXPLETIVE}ed on 32-bit.  Not just on
-next, also on 4.11-rc5 and 4.9.  Test script I used is attached, although
it's enough to just scrub a kosher filesystem without even damaging it.
On 64-bit it mostly works, but still warns about bogus unrecoverable errors
when in fact it succeeded.

Thus, I'd recommend:
* applying this patch to at least make it compile
* taking steps to warn outside people about RAID5/6

Let's discuss the rest in another thread, it's no longer interesting to ARM
people, they just want no build failures.

-- 
⢀⣴⠾⠻⢶⣦⠀ Meow!
⣾⠁⢠⠒⠀⣿⡁
⢿⡄⠘⠷⠚⠋⠀ Collisions shmolisions, let's see them find a collision or second
⠈⠳⣄ preimage for double rot13!
#!/bin/sh
set -x
DATA=/usr/bin  # use whole /usr on non-bloated VMs


mkdir -p /mnt/vol1
umount /mnt/vol1; losetup -D   # clean up after repeats

dd if=/dev/zero bs=1048576 count=1 seek=4095 of=ra
dd if=/dev/zero bs=1048576 count=1 seek=4095 of=rb
dd if=/dev/zero bs=1048576 count=1 seek=4095 of=rc
dd if=/dev/zero bs=1048576 count=1 seek=4095 of=rd

mkfs.btrfs -draid10 -mraid1 ra rb rc rd

losetup -D
losetup -f ra
losetup -f rb
losetup -f rc
losetup -f rd
sleep 2  # race with fsid detection
mount -onoatime /dev/loop0 /mnt/vol1 || exit $?
cp -pr "$DATA" /mnt/vol1
btrfs fi sync /mnt/vol1
btrfs fi us /mnt/vol1

btrfs balance start -dconvert=raid5 -mconvert=raid6 /mnt/vol1

btrfs scrub start -B /mnt/vol1
btrfs scrub start -B /mnt/vol1

umount /mnt/vol1
dd if=/dev/urandom of=rd bs=1048576 seek=96 count=4000
mount -onoatime /dev/loop0 /mnt/vol1 || exit $?
btrfs scrub start -B /mnt/vol1
btrfs scrub start -B /mnt/vol1

btrfs balance start -dconvert=raid10 -mconvert=raid1 /mnt/vol1

btrfs scrub start -B /mnt/vol1
btrfs scrub start -B /mnt/vol1

umount /mnt/vol1
dd if=/dev/urandom of=rd bs=1048576 seek=96 count=4000
mount -onoatime /dev/loop0 /mnt/vol1 || exit $?
btrfs scrub start -B /mnt/vol1
btrfs scrub start -B /mnt/vol1

diff -urd --no-dereference "$DATA" /mnt/vol1/*

umount /mnt/vol1

Re: [PATCH] btrfs: scrub: use do_div() for 64-by-32 division

[Adam Borowski]

On Sun, Apr 09, 2017 at 05:58:54AM +0200, Adam Borowski wrote:
> On Sat, Apr 08, 2017 at 11:07:37PM +0200, Adam Borowski wrote:
> > Unbreaks ARM and possibly other 32-bit architectures.
> 
> Turns out those "other 32-bit architectures" happen to include i386.
> 
> A modular build:
> ERROR: "__udivdi3" [fs/btrfs/btrfs.ko] undefined!
> With the patch, i386 builds fine.
> 
> > Tested on amd64 where all is fine, and on arm (Odroid-U2) where scrub
> > sometimes works, but, like most operations, randomly dies with some badness
> > that doesn't look related: io_schedule, kunmap_high.  That badness wasn't
> > there in 4.11-rc5, needs investigating, but since it's not connected to our
> > issue at hand, I consider this patch sort-of tested.
> 
> Looks like current -next is pretty broken: while amd64 is ok, on an i386 box
> (non-NX Pentium 4) it hangs very early during boot, way before filesystem
> modules would be loaded.  Qemu boots but has random hangs.

A non-modular i386_defconfig + btrfs of -next is ok; whatever the problem
is, it's not relevant to our division build failure in scrub.

But, it looks like parity scrub is ${EXPLETIVE}ed on 32-bit.  Not just on
-next, also on 4.11-rc5 and 4.9.  Test script I used is attached, although
it's enough to just scrub a kosher filesystem without even damaging it.
On 64-bit it mostly works, but still warns about bogus unrecoverable errors
when in fact it succeeded.

Thus, I'd recommend:
* applying this patch to at least make it compile
* taking steps to warn outside people about RAID5/6

Let's discuss the rest in another thread, it's no longer interesting to ARM
people, they just want no build failures.

-- 
⢀⣴⠾⠻⢶⣦⠀ Meow!
⣾⠁⢠⠒⠀⣿⡁
⢿⡄⠘⠷⠚⠋⠀ Collisions shmolisions, let's see them find a collision or second
⠈⠳⣄ preimage for double rot13!
#!/bin/sh
set -x
DATA=/usr/bin  # use whole /usr on non-bloated VMs


mkdir -p /mnt/vol1
umount /mnt/vol1; losetup -D   # clean up after repeats

dd if=/dev/zero bs=1048576 count=1 seek=4095 of=ra
dd if=/dev/zero bs=1048576 count=1 seek=4095 of=rb
dd if=/dev/zero bs=1048576 count=1 seek=4095 of=rc
dd if=/dev/zero bs=1048576 count=1 seek=4095 of=rd

mkfs.btrfs -draid10 -mraid1 ra rb rc rd

losetup -D
losetup -f ra
losetup -f rb
losetup -f rc
losetup -f rd
sleep 2  # race with fsid detection
mount -onoatime /dev/loop0 /mnt/vol1 || exit $?
cp -pr "$DATA" /mnt/vol1
btrfs fi sync /mnt/vol1
btrfs fi us /mnt/vol1

btrfs balance start -dconvert=raid5 -mconvert=raid6 /mnt/vol1

btrfs scrub start -B /mnt/vol1
btrfs scrub start -B /mnt/vol1

umount /mnt/vol1
dd if=/dev/urandom of=rd bs=1048576 seek=96 count=4000
mount -onoatime /dev/loop0 /mnt/vol1 || exit $?
btrfs scrub start -B /mnt/vol1
btrfs scrub start -B /mnt/vol1

btrfs balance start -dconvert=raid10 -mconvert=raid1 /mnt/vol1

btrfs scrub start -B /mnt/vol1
btrfs scrub start -B /mnt/vol1

umount /mnt/vol1
dd if=/dev/urandom of=rd bs=1048576 seek=96 count=4000
mount -onoatime /dev/loop0 /mnt/vol1 || exit $?
btrfs scrub start -B /mnt/vol1
btrfs scrub start -B /mnt/vol1

diff -urd --no-dereference "$DATA" /mnt/vol1/*

umount /mnt/vol1

Re: [PATCH] btrfs: scrub: use do_div() for 64-by-32 division

[Adam Borowski]

On Sat, Apr 08, 2017 at 11:07:37PM +0200, Adam Borowski wrote:
> Unbreaks ARM and possibly other 32-bit architectures.

Turns out those "other 32-bit architectures" happen to include i386.

A modular build:

ERROR: "__udivdi3" [fs/btrfs/btrfs.ko] undefined!

With the patch, i386 builds fine.

> Tested on amd64 where all is fine, and on arm (Odroid-U2) where scrub
> sometimes works, but, like most operations, randomly dies with some badness
> that doesn't look related: io_schedule, kunmap_high.  That badness wasn't
> there in 4.11-rc5, needs investigating, but since it's not connected to our
> issue at hand, I consider this patch sort-of tested.

Looks like current -next is pretty broken: while amd64 is ok, on an i386 box
(non-NX Pentium 4) it hangs very early during boot, way before filesystem
modules would be loaded.  Qemu boots but has random hangs.

So it looks like it's compile only for now...

-- 
⢀⣴⠾⠻⢶⣦⠀ Meow!
⣾⠁⢠⠒⠀⣿⡁
⢿⡄⠘⠷⠚⠋⠀ Collisions shmolisions, let's see them find a collision or second
⠈⠳⣄ preimage for double rot13!

Re: [PATCH] btrfs: scrub: use do_div() for 64-by-32 division

[Adam Borowski]

On Sat, Apr 08, 2017 at 11:07:37PM +0200, Adam Borowski wrote:
> Unbreaks ARM and possibly other 32-bit architectures.

Turns out those "other 32-bit architectures" happen to include i386.

A modular build:

ERROR: "__udivdi3" [fs/btrfs/btrfs.ko] undefined!

With the patch, i386 builds fine.

> Tested on amd64 where all is fine, and on arm (Odroid-U2) where scrub
> sometimes works, but, like most operations, randomly dies with some badness
> that doesn't look related: io_schedule, kunmap_high.  That badness wasn't
> there in 4.11-rc5, needs investigating, but since it's not connected to our
> issue at hand, I consider this patch sort-of tested.

Looks like current -next is pretty broken: while amd64 is ok, on an i386 box
(non-NX Pentium 4) it hangs very early during boot, way before filesystem
modules would be loaded.  Qemu boots but has random hangs.

So it looks like it's compile only for now...

-- 
⢀⣴⠾⠻⢶⣦⠀ Meow!
⣾⠁⢠⠒⠀⣿⡁
⢿⡄⠘⠷⠚⠋⠀ Collisions shmolisions, let's see them find a collision or second
⠈⠳⣄ preimage for double rot13!

[PATCH] btrfs: scrub: use do_div() for 64-by-32 division

[Adam Borowski]

Unbreaks ARM and possibly other 32-bit architectures.

Fixes: 7d0ef8b4d: Btrfs: update scrub_parity to use u64 stripe_len
Reported-by: Icenowy Zheng <icen...@aosc.io>
Signed-off-by: Adam Borowski <kilob...@angband.pl>
---
You'd probably want to squash this with Liu's commit, to be nice to future
bisects.

Tested on amd64 where all is fine, and on arm (Odroid-U2) where scrub
sometimes works, but, like most operations, randomly dies with some badness
that doesn't look related: io_schedule, kunmap_high.  That badness wasn't
there in 4.11-rc5, needs investigating, but since it's not connected to our
issue at hand, I consider this patch sort-of tested.

 fs/btrfs/scrub.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/btrfs/scrub.c b/fs/btrfs/scrub.c
index b6fe1cd08048..95372e3679f3 100644
--- a/fs/btrfs/scrub.c
+++ b/fs/btrfs/scrub.c
@@ -2407,7 +2407,7 @@ static inline void __scrub_mark_bitmap(struct 
scrub_parity *sparity,
 
start -= sparity->logic_start;
start = div64_u64_rem(start, sparity->stripe_len, );
-   offset /= sectorsize;
+   do_div(offset, sectorsize);
nsectors = (int)len / sectorsize;
 
if (offset + nsectors <= sparity->nsectors) {
-- 
2.11.0

[PATCH] btrfs: scrub: use do_div() for 64-by-32 division

[Adam Borowski]

Unbreaks ARM and possibly other 32-bit architectures.

Fixes: 7d0ef8b4d: Btrfs: update scrub_parity to use u64 stripe_len
Reported-by: Icenowy Zheng 
Signed-off-by: Adam Borowski 
---
You'd probably want to squash this with Liu's commit, to be nice to future
bisects.

Tested on amd64 where all is fine, and on arm (Odroid-U2) where scrub
sometimes works, but, like most operations, randomly dies with some badness
that doesn't look related: io_schedule, kunmap_high.  That badness wasn't
there in 4.11-rc5, needs investigating, but since it's not connected to our
issue at hand, I consider this patch sort-of tested.

 fs/btrfs/scrub.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/btrfs/scrub.c b/fs/btrfs/scrub.c
index b6fe1cd08048..95372e3679f3 100644
--- a/fs/btrfs/scrub.c
+++ b/fs/btrfs/scrub.c
@@ -2407,7 +2407,7 @@ static inline void __scrub_mark_bitmap(struct 
scrub_parity *sparity,
 
start -= sparity->logic_start;
start = div64_u64_rem(start, sparity->stripe_len, );
-   offset /= sectorsize;
+   do_div(offset, sectorsize);
nsectors = (int)len / sectorsize;
 
if (offset + nsectors <= sparity->nsectors) {
-- 
2.11.0

Re: Linux next-20170407 failed to build on ARM due to usage of mod in btrfs code

[Adam Borowski]

On Sat, Apr 08, 2017 at 02:45:34PM -0300, Fabio Estevam wrote:
> On Sat, Apr 8, 2017 at 1:02 PM, Icenowy Zheng  wrote:
> > Hello everyone,
> > Today I tried to build a kernel with btrfs enabled on ARM, then when linking
> > I met such an error:
> >
> > ```
> > fs/built-in.o: In function `scrub_bio_end_io_worker':
> > acl.c:(.text+0x2f0450): undefined reference to `__aeabi_uldivmod'
> > fs/built-in.o: In function `scrub_extent_for_parity':
> > acl.c:(.text+0x2f0bcc): undefined reference to `__aeabi_uldivmod'
> > fs/built-in.o: In function `scrub_raid56_parity':
> > acl.c:(.text+0x2f12a8): undefined reference to `__aeabi_uldivmod'
> > acl.c:(.text+0x2f15c4): undefined reference to `__aeabi_uldivmod'
> > ```
> >
> > These functions are found at fs/btrfs/scrub.c .
> >
> > After disabling btrfs the kernel is successfully built.
> 
> I see the same error with ARM imx_v6_v7_defconfig + btrfs support.
> 
> Looks like it is caused by commit 7d0ef8b4dbbd220 ("Btrfs: update
> scrub_parity to use u64 stripe_len").

+1, my bisect just finished, same bad commit.

-- 
⢀⣴⠾⠻⢶⣦⠀ Meow!
⣾⠁⢠⠒⠀⣿⡁
⢿⡄⠘⠷⠚⠋⠀ Collisions shmolisions, let's see them find a collision or second
⠈⠳⣄ preimage for double rot13!

Re: Linux next-20170407 failed to build on ARM due to usage of mod in btrfs code

[Adam Borowski]

On Sat, Apr 08, 2017 at 02:45:34PM -0300, Fabio Estevam wrote:
> On Sat, Apr 8, 2017 at 1:02 PM, Icenowy Zheng  wrote:
> > Hello everyone,
> > Today I tried to build a kernel with btrfs enabled on ARM, then when linking
> > I met such an error:
> >
> > ```
> > fs/built-in.o: In function `scrub_bio_end_io_worker':
> > acl.c:(.text+0x2f0450): undefined reference to `__aeabi_uldivmod'
> > fs/built-in.o: In function `scrub_extent_for_parity':
> > acl.c:(.text+0x2f0bcc): undefined reference to `__aeabi_uldivmod'
> > fs/built-in.o: In function `scrub_raid56_parity':
> > acl.c:(.text+0x2f12a8): undefined reference to `__aeabi_uldivmod'
> > acl.c:(.text+0x2f15c4): undefined reference to `__aeabi_uldivmod'
> > ```
> >
> > These functions are found at fs/btrfs/scrub.c .
> >
> > After disabling btrfs the kernel is successfully built.
> 
> I see the same error with ARM imx_v6_v7_defconfig + btrfs support.
> 
> Looks like it is caused by commit 7d0ef8b4dbbd220 ("Btrfs: update
> scrub_parity to use u64 stripe_len").

+1, my bisect just finished, same bad commit.

-- 
⢀⣴⠾⠻⢶⣦⠀ Meow!
⣾⠁⢠⠒⠀⣿⡁
⢿⡄⠘⠷⠚⠋⠀ Collisions shmolisions, let's see them find a collision or second
⠈⠳⣄ preimage for double rot13!

Re: [PATCH v2 0/5] minitty: a minimal TTY layer alternative for embedded systems

[Adam Borowski]

On Mon, Apr 03, 2017 at 04:09:38PM -0400, Nicolas Pitre wrote:
> On Mon, 3 Apr 2017, Adam Borowski wrote:
> 
> > On Mon, Apr 03, 2017 at 08:31:03AM -0700, Andi Kleen wrote:
> > > Except for that (and possibly VT) it is unlikely that people really
> > > rely on the obsolete terminal features from the 70ies. So it's a kind
> > > of cleanup.
> > 
> > But... but... but what shall we do without OLCUC?!?
> > 
> > I guess sending these features to the pasture would be nice even in
> > mainstream TTY.  Probably even without a Kconfig option to restore them.
> 
> Thing is... those arcane features don't take much code at all:
> 
>   if (O_OLCUC(tty))
>   c = toupper(c);
> 
> That's it. I didn't make the minitty code 5x smaller just by omitting 
> those.  ;-)

Except, those two lines have two bugs:
* it mangles most non-ASCII (kernel's toupper() hard-codes ISO-8859-1
  which no one uses anymore)
* it mangles a number of ANSI codes, making them unusable on any vt100ish
  terminal (ie, any post-1980)

I just happened to send an April Fools pull request
(https://github.com/kilobyte/linux.git runes) in which the first commit
fixes these:
https://github.com/kilobyte/linux/commit/268cde7c6dde54fcbc81df68d66b2389d77d01f2

Even though it's a real fix (unlike the subsequent fun), guess why I'm not
sending it to Greg and Jiri...

-- 
⢀⣴⠾⠻⢶⣦⠀ Meow!
⣾⠁⢠⠒⠀⣿⡁
⢿⡄⠘⠷⠚⠋⠀ Collisions shmolisions, let's see them find a collision or second
⠈⠳⣄ preimage for double rot13!

Re: [PATCH v2 0/5] minitty: a minimal TTY layer alternative for embedded systems

[Adam Borowski]

On Mon, Apr 03, 2017 at 04:09:38PM -0400, Nicolas Pitre wrote:
> On Mon, 3 Apr 2017, Adam Borowski wrote:
> 
> > On Mon, Apr 03, 2017 at 08:31:03AM -0700, Andi Kleen wrote:
> > > Except for that (and possibly VT) it is unlikely that people really
> > > rely on the obsolete terminal features from the 70ies. So it's a kind
> > > of cleanup.
> > 
> > But... but... but what shall we do without OLCUC?!?
> > 
> > I guess sending these features to the pasture would be nice even in
> > mainstream TTY.  Probably even without a Kconfig option to restore them.
> 
> Thing is... those arcane features don't take much code at all:
> 
>   if (O_OLCUC(tty))
>   c = toupper(c);
> 
> That's it. I didn't make the minitty code 5x smaller just by omitting 
> those.  ;-)

Except, those two lines have two bugs:
* it mangles most non-ASCII (kernel's toupper() hard-codes ISO-8859-1
  which no one uses anymore)
* it mangles a number of ANSI codes, making them unusable on any vt100ish
  terminal (ie, any post-1980)

I just happened to send an April Fools pull request
(https://github.com/kilobyte/linux.git runes) in which the first commit
fixes these:
https://github.com/kilobyte/linux/commit/268cde7c6dde54fcbc81df68d66b2389d77d01f2

Even though it's a real fix (unlike the subsequent fun), guess why I'm not
sending it to Greg and Jiri...

-- 
⢀⣴⠾⠻⢶⣦⠀ Meow!
⣾⠁⢠⠒⠀⣿⡁
⢿⡄⠘⠷⠚⠋⠀ Collisions shmolisions, let's see them find a collision or second
⠈⠳⣄ preimage for double rot13!

Re: [PATCH v2 0/5] minitty: a minimal TTY layer alternative for embedded systems

[Adam Borowski]

On Mon, Apr 03, 2017 at 08:31:03AM -0700, Andi Kleen wrote:
> Except for that (and possibly VT) it is unlikely that people really
> rely on the obsolete terminal features from the 70ies. So it's a kind
> of cleanup.

But... but... but what shall we do without OLCUC?!?

I guess sending these features to the pasture would be nice even in
mainstream TTY.  Probably even without a Kconfig option to restore them.

-- 
⢀⣴⠾⠻⢶⣦⠀ Meow!
⣾⠁⢠⠒⠀⣿⡁
⢿⡄⠘⠷⠚⠋⠀ Collisions shmolisions, let's see them find a collision or second
⠈⠳⣄ preimage for double rot13!

Re: [PATCH v2 0/5] minitty: a minimal TTY layer alternative for embedded systems

[Adam Borowski]

On Mon, Apr 03, 2017 at 08:31:03AM -0700, Andi Kleen wrote:
> Except for that (and possibly VT) it is unlikely that people really
> rely on the obsolete terminal features from the 70ies. So it's a kind
> of cleanup.

But... but... but what shall we do without OLCUC?!?

I guess sending these features to the pasture would be nice even in
mainstream TTY.  Probably even without a Kconfig option to restore them.

-- 
⢀⣴⠾⠻⢶⣦⠀ Meow!
⣾⠁⢠⠒⠀⣿⡁
⢿⡄⠘⠷⠚⠋⠀ Collisions shmolisions, let's see them find a collision or second
⠈⠳⣄ preimage for double rot13!

[PATCH 3/4] n_tty: support th, ae and ng runes

[Adam Borowski]

Especially 'th' is a prominent letter in Elder Futhark, far more widespread
than 't'.  It has survived in English as 'þ' until a combination of disdain
from Latin-educated scribes and printing presses imported from Germany that
lacked this letter wiped it out.

Alas, we need to maintain a 1:1 relationship to keep alignment, thus you
need to write 'þ', 'æ' or 'ŋ' (or uppercase).  Unless you're Icelandic,
it's easiest to use the Compose key.

Signed-off-by: Adam Borowski <kilob...@angband.pl>
---
 drivers/tty/n_tty.c | 35 +--
 1 file changed, 33 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
index c36b9114f76b..3b8b745eb7cf 100644
--- a/drivers/tty/n_tty.c
+++ b/drivers/tty/n_tty.c
@@ -87,7 +87,7 @@
 # define n_tty_trace(f, args...)
 #endif
 
-enum { ESnormal, ESesc, EScsi, ESsetG };
+enum { ESnormal, ESesc, EScsi, ESsetG, ESxc3, ESxc5 };
 
 struct n_tty_data {
/* producer-published */
@@ -404,7 +404,7 @@ static inline int is_continuation(unsigned char c, struct 
tty_struct *tty)
 static int do_olcuc_char(unsigned char c, struct tty_struct *tty, int space)
 {
/* 3 bytes per character */
-   static const char *runes = "ᚨᛒᚳᛞᛖᚠᚷᚺᛁᛃᚲᛚᛗᚾᛟᛈᛩᚱᛊᛏᚢᚡᚹᛪᚣᛉ";
+   static const char *runes = "ᚨᛒᚳᛞᛖᚠᚷᚺᛁᛃᚲᛚᛗᚾᛟᛈᛩᚱᛊᛏᚢᚡᚹᛪᚣᛉᚦᛇᛜ";
struct n_tty_data *ldata = tty->disc_data;
 
switch (ldata->vt_state) {
@@ -419,6 +419,31 @@ static int do_olcuc_char(unsigned char c, struct 
tty_struct *tty, int space)
case ESsetG:
ldata->vt_state = ESnormal;
break;
+   case ESxc3:
+   if (c == 0xbe || c == 0x9e) { /* th */
+   c = 'A' + 26;
+   goto print_rune;
+   } else if (c == 0xa6 || c == 0x86) { /* ae */
+   c = 'A' + 27;
+   goto print_rune;
+   }
+   if (space < 2)
+   return -1;
+   tty_put_char(tty, 0xc3); /* no match, print the stolen prefix */
+   ldata->vt_state = ESnormal;
+   ldata->column++;
+   break;
+   case ESxc5:
+   if (c == 0x8b || c == 0x8a) { /* ng */
+   c = 'A' + 28;
+   goto print_rune;
+   }
+   if (space < 2)
+   return -1;
+   tty_put_char(tty, 0xc5); /* no match, print the stolen prefix */
+   ldata->vt_state = ESnormal;
+   ldata->column++;
+   break;
default:
if (c == '\e') {
ldata->vt_state = ESesc;
@@ -428,8 +453,10 @@ static int do_olcuc_char(unsigned char c, struct 
tty_struct *tty, int space)
c -= 32;
if (I_IUTF8(tty)) {
if (c >= 'A' && c <= 'Z') {
+print_rune:
if (space < 3)
return -1;
+   ldata->vt_state = ESnormal;
ldata->column++;
c -= 'A';
tty_put_char(tty, runes[3 * c + 0]);
@@ -437,6 +464,10 @@ static int do_olcuc_char(unsigned char c, struct 
tty_struct *tty, int space)
tty_put_char(tty, runes[3 * c + 2]);
return 1;
}
+   if (c == 0xc3)
+   return ldata->vt_state = ESxc3, 1;
+   if (c == 0xc5)
+   return ldata->vt_state = ESxc5, 1;
}
}
if (!iscntrl(c) && !is_continuation(c, tty))
-- 
2.11.0

[PATCH 3/4] n_tty: support th, ae and ng runes

[Adam Borowski]

Especially 'th' is a prominent letter in Elder Futhark, far more widespread
than 't'.  It has survived in English as 'þ' until a combination of disdain
from Latin-educated scribes and printing presses imported from Germany that
lacked this letter wiped it out.

Alas, we need to maintain a 1:1 relationship to keep alignment, thus you
need to write 'þ', 'æ' or 'ŋ' (or uppercase).  Unless you're Icelandic,
it's easiest to use the Compose key.

Signed-off-by: Adam Borowski 
---
 drivers/tty/n_tty.c | 35 +--
 1 file changed, 33 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
index c36b9114f76b..3b8b745eb7cf 100644
--- a/drivers/tty/n_tty.c
+++ b/drivers/tty/n_tty.c
@@ -87,7 +87,7 @@
 # define n_tty_trace(f, args...)
 #endif
 
-enum { ESnormal, ESesc, EScsi, ESsetG };
+enum { ESnormal, ESesc, EScsi, ESsetG, ESxc3, ESxc5 };
 
 struct n_tty_data {
/* producer-published */
@@ -404,7 +404,7 @@ static inline int is_continuation(unsigned char c, struct 
tty_struct *tty)
 static int do_olcuc_char(unsigned char c, struct tty_struct *tty, int space)
 {
/* 3 bytes per character */
-   static const char *runes = "ᚨᛒᚳᛞᛖᚠᚷᚺᛁᛃᚲᛚᛗᚾᛟᛈᛩᚱᛊᛏᚢᚡᚹᛪᚣᛉ";
+   static const char *runes = "ᚨᛒᚳᛞᛖᚠᚷᚺᛁᛃᚲᛚᛗᚾᛟᛈᛩᚱᛊᛏᚢᚡᚹᛪᚣᛉᚦᛇᛜ";
struct n_tty_data *ldata = tty->disc_data;
 
switch (ldata->vt_state) {
@@ -419,6 +419,31 @@ static int do_olcuc_char(unsigned char c, struct 
tty_struct *tty, int space)
case ESsetG:
ldata->vt_state = ESnormal;
break;
+   case ESxc3:
+   if (c == 0xbe || c == 0x9e) { /* th */
+   c = 'A' + 26;
+   goto print_rune;
+   } else if (c == 0xa6 || c == 0x86) { /* ae */
+   c = 'A' + 27;
+   goto print_rune;
+   }
+   if (space < 2)
+   return -1;
+   tty_put_char(tty, 0xc3); /* no match, print the stolen prefix */
+   ldata->vt_state = ESnormal;
+   ldata->column++;
+   break;
+   case ESxc5:
+   if (c == 0x8b || c == 0x8a) { /* ng */
+   c = 'A' + 28;
+   goto print_rune;
+   }
+   if (space < 2)
+   return -1;
+   tty_put_char(tty, 0xc5); /* no match, print the stolen prefix */
+   ldata->vt_state = ESnormal;
+   ldata->column++;
+   break;
default:
if (c == '\e') {
ldata->vt_state = ESesc;
@@ -428,8 +453,10 @@ static int do_olcuc_char(unsigned char c, struct 
tty_struct *tty, int space)
c -= 32;
if (I_IUTF8(tty)) {
if (c >= 'A' && c <= 'Z') {
+print_rune:
if (space < 3)
return -1;
+   ldata->vt_state = ESnormal;
ldata->column++;
c -= 'A';
tty_put_char(tty, runes[3 * c + 0]);
@@ -437,6 +464,10 @@ static int do_olcuc_char(unsigned char c, struct 
tty_struct *tty, int space)
tty_put_char(tty, runes[3 * c + 2]);
return 1;
}
+   if (c == 0xc3)
+   return ldata->vt_state = ESxc3, 1;
+   if (c == 0xc5)
+   return ldata->vt_state = ESxc5, 1;
}
}
if (!iscntrl(c) && !is_continuation(c, tty))
-- 
2.11.0

[PATCH 2/4] n_tty: use runes rather than uppercase in IUTF8 OLCUC mode

[Adam Borowski]

It is puzzling why OLCUC support has been added to Linux, despite it being
obsolete for a long long time before Linux was born.  The only explanation
I see, as on all-caps displays those were often called "Great Runes" (see
the Jargon File), is that the intention was to support runes but that
wasn't possible before Unicode.

As the kernel's tty discipline knows about UTF-8, that's now possible.

Standards compliance:
Elder Futhark (2nd to 8th centuries), other than:
'y' 'c' are from Anglo-Saxon runes (5th to 11th centuries), 'c' should use
kauna rather than cen but as the former is shared with 'k', I sacrificed
period accuracy for usability.
'q' 'v' 'x' are from Medieval runes (12th to 15th centuries).  'x' could
use Anglo-Saxon eolhx but that's same glyph (and Unicode codepoint) as
Elder Futhark algiz 'z'.

Signed-off-by: Adam Borowski <kilob...@angband.pl>
---
 drivers/tty/n_tty.c | 24 
 1 file changed, 20 insertions(+), 4 deletions(-)

diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
index bbc9f07c19fa..c36b9114f76b 100644
--- a/drivers/tty/n_tty.c
+++ b/drivers/tty/n_tty.c
@@ -401,8 +401,10 @@ static inline int is_continuation(unsigned char c, struct 
tty_struct *tty)
  * only some commands require lowercase.
  */
 
-static int do_olcuc_char(unsigned char c, struct tty_struct *tty)
+static int do_olcuc_char(unsigned char c, struct tty_struct *tty, int space)
 {
+   /* 3 bytes per character */
+   static const char *runes = "ᚨᛒᚳᛞᛖᚠᚷᚺᛁᛃᚲᛚᛗᚾᛟᛈᛩᚱᛊᛏᚢᚡᚹᛪᚣᛉ";
struct n_tty_data *ldata = tty->disc_data;
 
switch (ldata->vt_state) {
@@ -418,10 +420,24 @@ static int do_olcuc_char(unsigned char c, struct 
tty_struct *tty)
ldata->vt_state = ESnormal;
break;
default:
-   if (c == '\e')
+   if (c == '\e') {
ldata->vt_state = ESesc;
-   else if (c >= 'a' && c <= 'z')
+   break;
+   }
+   if (c >= 'a' && c <= 'z')
c -= 32;
+   if (I_IUTF8(tty)) {
+   if (c >= 'A' && c <= 'Z') {
+   if (space < 3)
+   return -1;
+   ldata->column++;
+   c -= 'A';
+   tty_put_char(tty, runes[3 * c + 0]);
+   tty_put_char(tty, runes[3 * c + 1]);
+   tty_put_char(tty, runes[3 * c + 2]);
+   return 1;
+   }
+   }
}
if (!iscntrl(c) && !is_continuation(c, tty))
ldata->column++;
@@ -500,7 +516,7 @@ static int do_output_char(unsigned char c, struct 
tty_struct *tty, int space)
break;
default:
if (O_OLCUC(tty))
-   return do_olcuc_char(c, tty);
+   return do_olcuc_char(c, tty, space);
if (!iscntrl(c) && !is_continuation(c, tty))
ldata->column++;
break;
-- 
2.11.0

[PATCH 2/4] n_tty: use runes rather than uppercase in IUTF8 OLCUC mode

[Adam Borowski]

It is puzzling why OLCUC support has been added to Linux, despite it being
obsolete for a long long time before Linux was born.  The only explanation
I see, as on all-caps displays those were often called "Great Runes" (see
the Jargon File), is that the intention was to support runes but that
wasn't possible before Unicode.

As the kernel's tty discipline knows about UTF-8, that's now possible.

Standards compliance:
Elder Futhark (2nd to 8th centuries), other than:
'y' 'c' are from Anglo-Saxon runes (5th to 11th centuries), 'c' should use
kauna rather than cen but as the former is shared with 'k', I sacrificed
period accuracy for usability.
'q' 'v' 'x' are from Medieval runes (12th to 15th centuries).  'x' could
use Anglo-Saxon eolhx but that's same glyph (and Unicode codepoint) as
Elder Futhark algiz 'z'.

Signed-off-by: Adam Borowski 
---
 drivers/tty/n_tty.c | 24 
 1 file changed, 20 insertions(+), 4 deletions(-)

diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
index bbc9f07c19fa..c36b9114f76b 100644
--- a/drivers/tty/n_tty.c
+++ b/drivers/tty/n_tty.c
@@ -401,8 +401,10 @@ static inline int is_continuation(unsigned char c, struct 
tty_struct *tty)
  * only some commands require lowercase.
  */
 
-static int do_olcuc_char(unsigned char c, struct tty_struct *tty)
+static int do_olcuc_char(unsigned char c, struct tty_struct *tty, int space)
 {
+   /* 3 bytes per character */
+   static const char *runes = "ᚨᛒᚳᛞᛖᚠᚷᚺᛁᛃᚲᛚᛗᚾᛟᛈᛩᚱᛊᛏᚢᚡᚹᛪᚣᛉ";
struct n_tty_data *ldata = tty->disc_data;
 
switch (ldata->vt_state) {
@@ -418,10 +420,24 @@ static int do_olcuc_char(unsigned char c, struct 
tty_struct *tty)
ldata->vt_state = ESnormal;
break;
default:
-   if (c == '\e')
+   if (c == '\e') {
ldata->vt_state = ESesc;
-   else if (c >= 'a' && c <= 'z')
+   break;
+   }
+   if (c >= 'a' && c <= 'z')
c -= 32;
+   if (I_IUTF8(tty)) {
+   if (c >= 'A' && c <= 'Z') {
+   if (space < 3)
+   return -1;
+   ldata->column++;
+   c -= 'A';
+   tty_put_char(tty, runes[3 * c + 0]);
+   tty_put_char(tty, runes[3 * c + 1]);
+   tty_put_char(tty, runes[3 * c + 2]);
+   return 1;
+   }
+   }
}
if (!iscntrl(c) && !is_continuation(c, tty))
ldata->column++;
@@ -500,7 +516,7 @@ static int do_output_char(unsigned char c, struct 
tty_struct *tty, int space)
break;
default:
if (O_OLCUC(tty))
-   return do_olcuc_char(c, tty);
+   return do_olcuc_char(c, tty, space);
if (!iscntrl(c) && !is_continuation(c, tty))
ldata->column++;
break;
-- 
2.11.0

[PATCH 4/4] n_tty: wrap all OLCUC code in a config option

[Adam Borowski]

Setting it to N, beside dropping runes, also disables old-style support
for all-caps OLCUC.  To get those 40 years old terminals to work, set
CONFIG_TTY_RUNES=y which will DTRT when stty iutf8 is off.

Signed-off-by: Adam Borowski <kilob...@angband.pl>
---
 drivers/tty/Kconfig | 17 +
 drivers/tty/n_tty.c | 10 ++
 2 files changed, 27 insertions(+)

diff --git a/drivers/tty/Kconfig b/drivers/tty/Kconfig
index 95103054c0e4..cfeed2b196e5 100644
--- a/drivers/tty/Kconfig
+++ b/drivers/tty/Kconfig
@@ -151,6 +151,23 @@ config LEGACY_PTY_COUNT
  When not in use, each legacy PTY occupies 12 bytes on 32-bit
  architectures and 24 bytes on 64-bit architectures.
 
+config TTY_RUNES
+   bool "Runes on OLCUC"
+   default y
+   ---help---
+ Certain terminals from the days of Unix' infancy supported only
+ all-caps, so-called "Great Runes".  Linux still has rudimentary
+ support for those, although how can you connect one is another
+ matter; you enable that via "stty olcuc".
+
+ On anything modern, though (as in "stty iutf8"), you do get actual
+ runes.  You need userspace fonts for that; modern distributions
+ tend to install both raster (xterm/rxvt) and TrueType (most
+ terminals) fonts on their default GUI setups.  Alas, this is not
+ the case on console due to sharply limited character sets.
+
+ C'mon, you know you want to say Y!
+
 config BFIN_JTAG_COMM
tristate "Blackfin JTAG Communication"
depends on BLACKFIN
diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
index 3b8b745eb7cf..14d090576f09 100644
--- a/drivers/tty/n_tty.c
+++ b/drivers/tty/n_tty.c
@@ -123,7 +123,9 @@ struct n_tty_data {
unsigned int column;
unsigned int canon_column;
size_t echo_tail;
+#ifdef CONFIG_TTY_RUNES
unsigned int vt_state;
+#endif
 
struct mutex atomic_read_lock;
struct mutex output_lock;
@@ -395,6 +397,7 @@ static inline int is_continuation(unsigned char c, struct 
tty_struct *tty)
return I_IUTF8(tty) && is_utf8_continuation(c);
 }
 
+#ifdef CONFIG_TTY_RUNES
 /* process one OLCUC char (possibly partial unicode)
  *
  * We need to partially parse ANSI sequences to avoid uppercasing them;
@@ -475,6 +478,7 @@ static int do_olcuc_char(unsigned char c, struct tty_struct 
*tty, int space)
tty_put_char(tty, c);
return 1;
 }
+#endif
 
 /**
  * do_output_char  -   output one character
@@ -546,8 +550,10 @@ static int do_output_char(unsigned char c, struct 
tty_struct *tty, int space)
ldata->column--;
break;
default:
+#ifdef CONFIG_TTY_RUNES
if (O_OLCUC(tty))
return do_olcuc_char(c, tty, space);
+#endif
if (!iscntrl(c) && !is_continuation(c, tty))
ldata->column++;
break;
@@ -650,8 +656,10 @@ static ssize_t process_output_block(struct tty_struct *tty,
ldata->column--;
break;
default:
+#ifdef CONFIG_TTY_RUNES
if (O_OLCUC(tty))
goto break_out;
+#endif
if (!iscntrl(c) && !is_continuation(c, tty))
ldata->column++;
break;
@@ -1975,7 +1983,9 @@ static int n_tty_open(struct tty_struct *tty)
ldata->num_overrun = 0;
ldata->no_room = 0;
ldata->lnext = 0;
+#ifdef CONFIG_TTY_RUNES
ldata->vt_state = ESnormal;
+#endif
tty->closing = 0;
/* indicate buffer work may resume */
clear_bit(TTY_LDISC_HALTED, >flags);
-- 
2.11.0

[PATCH 4/4] n_tty: wrap all OLCUC code in a config option

[Adam Borowski]

Setting it to N, beside dropping runes, also disables old-style support
for all-caps OLCUC.  To get those 40 years old terminals to work, set
CONFIG_TTY_RUNES=y which will DTRT when stty iutf8 is off.

Signed-off-by: Adam Borowski 
---
 drivers/tty/Kconfig | 17 +
 drivers/tty/n_tty.c | 10 ++
 2 files changed, 27 insertions(+)

diff --git a/drivers/tty/Kconfig b/drivers/tty/Kconfig
index 95103054c0e4..cfeed2b196e5 100644
--- a/drivers/tty/Kconfig
+++ b/drivers/tty/Kconfig
@@ -151,6 +151,23 @@ config LEGACY_PTY_COUNT
  When not in use, each legacy PTY occupies 12 bytes on 32-bit
  architectures and 24 bytes on 64-bit architectures.
 
+config TTY_RUNES
+   bool "Runes on OLCUC"
+   default y
+   ---help---
+ Certain terminals from the days of Unix' infancy supported only
+ all-caps, so-called "Great Runes".  Linux still has rudimentary
+ support for those, although how can you connect one is another
+ matter; you enable that via "stty olcuc".
+
+ On anything modern, though (as in "stty iutf8"), you do get actual
+ runes.  You need userspace fonts for that; modern distributions
+ tend to install both raster (xterm/rxvt) and TrueType (most
+ terminals) fonts on their default GUI setups.  Alas, this is not
+ the case on console due to sharply limited character sets.
+
+ C'mon, you know you want to say Y!
+
 config BFIN_JTAG_COMM
tristate "Blackfin JTAG Communication"
depends on BLACKFIN
diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
index 3b8b745eb7cf..14d090576f09 100644
--- a/drivers/tty/n_tty.c
+++ b/drivers/tty/n_tty.c
@@ -123,7 +123,9 @@ struct n_tty_data {
unsigned int column;
unsigned int canon_column;
size_t echo_tail;
+#ifdef CONFIG_TTY_RUNES
unsigned int vt_state;
+#endif
 
struct mutex atomic_read_lock;
struct mutex output_lock;
@@ -395,6 +397,7 @@ static inline int is_continuation(unsigned char c, struct 
tty_struct *tty)
return I_IUTF8(tty) && is_utf8_continuation(c);
 }
 
+#ifdef CONFIG_TTY_RUNES
 /* process one OLCUC char (possibly partial unicode)
  *
  * We need to partially parse ANSI sequences to avoid uppercasing them;
@@ -475,6 +478,7 @@ static int do_olcuc_char(unsigned char c, struct tty_struct 
*tty, int space)
tty_put_char(tty, c);
return 1;
 }
+#endif
 
 /**
  * do_output_char  -   output one character
@@ -546,8 +550,10 @@ static int do_output_char(unsigned char c, struct 
tty_struct *tty, int space)
ldata->column--;
break;
default:
+#ifdef CONFIG_TTY_RUNES
if (O_OLCUC(tty))
return do_olcuc_char(c, tty, space);
+#endif
if (!iscntrl(c) && !is_continuation(c, tty))
ldata->column++;
break;
@@ -650,8 +656,10 @@ static ssize_t process_output_block(struct tty_struct *tty,
ldata->column--;
break;
default:
+#ifdef CONFIG_TTY_RUNES
if (O_OLCUC(tty))
goto break_out;
+#endif
if (!iscntrl(c) && !is_continuation(c, tty))
ldata->column++;
break;
@@ -1975,7 +1983,9 @@ static int n_tty_open(struct tty_struct *tty)
ldata->num_overrun = 0;
ldata->no_room = 0;
ldata->lnext = 0;
+#ifdef CONFIG_TTY_RUNES
ldata->vt_state = ESnormal;
+#endif
tty->closing = 0;
/* indicate buffer work may resume */
clear_bit(TTY_LDISC_HALTED, >flags);
-- 
2.11.0

[PATCH 1/4] n_tty: don't mangle tty codes in OLCUC mode

[Adam Borowski]

Any terminal younger than ~40 years requires lowercase commands, thus they
need to be exempt from uppercasing.  And Linux doesn't really support
ancient all-uppercase terminals anyway (no XCASE, etc) even if someone made
the effort to somehow connect them electrically.

This bug is reproducible as of Linux 0.11 and I see it in 0.01 sources
(whose images fail to boot for me, I didn't try very hard).  It was less of
a failure then as the shell didn't produce tty codes for normal operation.

Signed-off-by: Adam Borowski <kilob...@angband.pl>
---
 drivers/tty/n_tty.c | 58 ++---
 1 file changed, 46 insertions(+), 12 deletions(-)

diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
index bdf0e6e89991..bbc9f07c19fa 100644
--- a/drivers/tty/n_tty.c
+++ b/drivers/tty/n_tty.c
@@ -87,6 +87,8 @@
 # define n_tty_trace(f, args...)
 #endif
 
+enum { ESnormal, ESesc, EScsi, ESsetG };
+
 struct n_tty_data {
/* producer-published */
size_t read_head;
@@ -121,6 +123,7 @@ struct n_tty_data {
unsigned int column;
unsigned int canon_column;
size_t echo_tail;
+   unsigned int vt_state;
 
struct mutex atomic_read_lock;
struct mutex output_lock;
@@ -392,6 +395,40 @@ static inline int is_continuation(unsigned char c, struct 
tty_struct *tty)
return I_IUTF8(tty) && is_utf8_continuation(c);
 }
 
+/* process one OLCUC char (possibly partial unicode)
+ *
+ * We need to partially parse ANSI sequences to avoid uppercasing them;
+ * only some commands require lowercase.
+ */
+
+static int do_olcuc_char(unsigned char c, struct tty_struct *tty)
+{
+   struct n_tty_data *ldata = tty->disc_data;
+
+   switch (ldata->vt_state) {
+   case ESesc:
+   ldata->vt_state = (c == '[') ? EScsi :
+ strchr("%()*+-./", c) ? ESsetG : ESnormal;
+   break;
+   case EScsi:
+   if (!strchr("?;0123456789>!c$\" \\", c))
+   ldata->vt_state = ESnormal;
+   break;
+   case ESsetG:
+   ldata->vt_state = ESnormal;
+   break;
+   default:
+   if (c == '\e')
+   ldata->vt_state = ESesc;
+   else if (c >= 'a' && c <= 'z')
+   c -= 32;
+   }
+   if (!iscntrl(c) && !is_continuation(c, tty))
+   ldata->column++;
+   tty_put_char(tty, c);
+   return 1;
+}
+
 /**
  * do_output_char  -   output one character
  * @c: character (or partial unicode symbol)
@@ -462,12 +499,10 @@ static int do_output_char(unsigned char c, struct 
tty_struct *tty, int space)
ldata->column--;
break;
default:
-   if (!iscntrl(c)) {
-   if (O_OLCUC(tty))
-   c = toupper(c);
-   if (!is_continuation(c, tty))
-   ldata->column++;
-   }
+   if (O_OLCUC(tty))
+   return do_olcuc_char(c, tty);
+   if (!iscntrl(c) && !is_continuation(c, tty))
+   ldata->column++;
break;
}
 
@@ -568,12 +603,10 @@ static ssize_t process_output_block(struct tty_struct 
*tty,
ldata->column--;
break;
default:
-   if (!iscntrl(c)) {
-   if (O_OLCUC(tty))
-   goto break_out;
-   if (!is_continuation(c, tty))
-   ldata->column++;
-   }
+   if (O_OLCUC(tty))
+   goto break_out;
+   if (!iscntrl(c) && !is_continuation(c, tty))
+   ldata->column++;
break;
}
}
@@ -1895,6 +1928,7 @@ static int n_tty_open(struct tty_struct *tty)
ldata->num_overrun = 0;
ldata->no_room = 0;
ldata->lnext = 0;
+   ldata->vt_state = ESnormal;
tty->closing = 0;
/* indicate buffer work may resume */
clear_bit(TTY_LDISC_HALTED, >flags);
-- 
2.11.0

[PATCH 1/4] n_tty: don't mangle tty codes in OLCUC mode

[Adam Borowski]

Any terminal younger than ~40 years requires lowercase commands, thus they
need to be exempt from uppercasing.  And Linux doesn't really support
ancient all-uppercase terminals anyway (no XCASE, etc) even if someone made
the effort to somehow connect them electrically.

This bug is reproducible as of Linux 0.11 and I see it in 0.01 sources
(whose images fail to boot for me, I didn't try very hard).  It was less of
a failure then as the shell didn't produce tty codes for normal operation.

Signed-off-by: Adam Borowski 
---
 drivers/tty/n_tty.c | 58 ++---
 1 file changed, 46 insertions(+), 12 deletions(-)

diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
index bdf0e6e89991..bbc9f07c19fa 100644
--- a/drivers/tty/n_tty.c
+++ b/drivers/tty/n_tty.c
@@ -87,6 +87,8 @@
 # define n_tty_trace(f, args...)
 #endif
 
+enum { ESnormal, ESesc, EScsi, ESsetG };
+
 struct n_tty_data {
/* producer-published */
size_t read_head;
@@ -121,6 +123,7 @@ struct n_tty_data {
unsigned int column;
unsigned int canon_column;
size_t echo_tail;
+   unsigned int vt_state;
 
struct mutex atomic_read_lock;
struct mutex output_lock;
@@ -392,6 +395,40 @@ static inline int is_continuation(unsigned char c, struct 
tty_struct *tty)
return I_IUTF8(tty) && is_utf8_continuation(c);
 }
 
+/* process one OLCUC char (possibly partial unicode)
+ *
+ * We need to partially parse ANSI sequences to avoid uppercasing them;
+ * only some commands require lowercase.
+ */
+
+static int do_olcuc_char(unsigned char c, struct tty_struct *tty)
+{
+   struct n_tty_data *ldata = tty->disc_data;
+
+   switch (ldata->vt_state) {
+   case ESesc:
+   ldata->vt_state = (c == '[') ? EScsi :
+ strchr("%()*+-./", c) ? ESsetG : ESnormal;
+   break;
+   case EScsi:
+   if (!strchr("?;0123456789>!c$\" \\", c))
+   ldata->vt_state = ESnormal;
+   break;
+   case ESsetG:
+   ldata->vt_state = ESnormal;
+   break;
+   default:
+   if (c == '\e')
+   ldata->vt_state = ESesc;
+   else if (c >= 'a' && c <= 'z')
+   c -= 32;
+   }
+   if (!iscntrl(c) && !is_continuation(c, tty))
+   ldata->column++;
+   tty_put_char(tty, c);
+   return 1;
+}
+
 /**
  * do_output_char  -   output one character
  * @c: character (or partial unicode symbol)
@@ -462,12 +499,10 @@ static int do_output_char(unsigned char c, struct 
tty_struct *tty, int space)
ldata->column--;
break;
default:
-   if (!iscntrl(c)) {
-   if (O_OLCUC(tty))
-   c = toupper(c);
-   if (!is_continuation(c, tty))
-   ldata->column++;
-   }
+   if (O_OLCUC(tty))
+   return do_olcuc_char(c, tty);
+   if (!iscntrl(c) && !is_continuation(c, tty))
+   ldata->column++;
break;
}
 
@@ -568,12 +603,10 @@ static ssize_t process_output_block(struct tty_struct 
*tty,
ldata->column--;
break;
default:
-   if (!iscntrl(c)) {
-   if (O_OLCUC(tty))
-   goto break_out;
-   if (!is_continuation(c, tty))
-   ldata->column++;
-   }
+   if (O_OLCUC(tty))
+   goto break_out;
+   if (!iscntrl(c) && !is_continuation(c, tty))
+   ldata->column++;
break;
}
}
@@ -1895,6 +1928,7 @@ static int n_tty_open(struct tty_struct *tty)
ldata->num_overrun = 0;
ldata->no_room = 0;
ldata->lnext = 0;
+   ldata->vt_state = ESnormal;
tty->closing = 0;
/* indicate buffer work may resume */
clear_bit(TTY_LDISC_HALTED, >flags);
-- 
2.11.0

[GIT PULL] runes

[Adam Borowski]

Hi Linus!
Please pull from

https://github.com/kilobyte/linux.git runes

It contains fixes for OLCUC handling, expanding its functionality to what I
presume was your intention in implementing OLCUC in 1991 but couldn't be
done then.

The highlight is a fix for a bug that bisects all the way to 0.01, although
it wasn't as visible then as shells didn't output ANSI codes during normal
operation.

The rest adds proper support for the Great Runes; enabled if iutf8 is set
(default on modern terminals).


Without these fixes, even basic shell output is mangled:

kilobyte@andunie:~$ stty olcuc
[01;32MKILOBYTE@ANDUNIE[00M:[01;34M~[00M$ LS
[01;34MGOATPORN[0M


With the fixes, to get the old-style OLCUC behaviour ASCII you can stty
-iutf8; neither ANSI codes nor Unicode will be mangled anymore.  But, that's
quite useless -- I wonder what was your reason to implement OLCUC back in
the day even though it was obsolete by decades even then, and was already
dropped from relevant standards.  Thus, I guess you want proper rune
support, this pull request adds this.  I've chosen Elder Futhark as the
variant to implement.

You might want to consider dropping patch 3, it is required only for full
coverage of the target set (Elder Futhark) but not for regular ASCII
mapping, and brings some code complexity.

Required userspace support is present in default GUI installs of all
distributions I checked, both on old-style terminals that use server-side X
fonts and with client-side {true,open}type -- but not on console unless
someone adds the required characters to the charset, which is tricky because
of 256/512 glyph limitations.  Thus, runes support currently works only on
graphical terminals.


Meow! -- sorry, ᛗᛖᛟᚹ!

Commits up to 14f34ba1d8748f252f941b5bb87efd7b1ed55868 on top of
c1ae3cfa0e89fa1a7ecc4c99031f5e9ae99d9201.


Adam Borowski (4):
  n_tty: don't mangle tty codes in OLCUC mode
  n_tty: use runes rather than uppercase in IUTF8 OLCUC mode
  n_tty: support th, ae and ng runes
  n_tty: wrap all OLCUC code in a config option

 drivers/tty/Kconfig |  17 
 drivers/tty/n_tty.c | 115 ++--
 2 files changed, 120 insertions(+), 12 deletions(-)

-- 
⢀⣴⠾⠻⢶⣦⠀ Meow!
⣾⠁⢠⠒⠀⣿⡁
⢿⡄⠘⠷⠚⠋⠀ Collisions shmolisions, let's see them find a collision or second
⠈⠳⣄ preimage for double rot13!


signature.asc
Description: PGP signature

[GIT PULL] runes

[Adam Borowski]

Hi Linus!
Please pull from

https://github.com/kilobyte/linux.git runes

It contains fixes for OLCUC handling, expanding its functionality to what I
presume was your intention in implementing OLCUC in 1991 but couldn't be
done then.

The highlight is a fix for a bug that bisects all the way to 0.01, although
it wasn't as visible then as shells didn't output ANSI codes during normal
operation.

The rest adds proper support for the Great Runes; enabled if iutf8 is set
(default on modern terminals).


Without these fixes, even basic shell output is mangled:

kilobyte@andunie:~$ stty olcuc
[01;32MKILOBYTE@ANDUNIE[00M:[01;34M~[00M$ LS
[01;34MGOATPORN[0M


With the fixes, to get the old-style OLCUC behaviour ASCII you can stty
-iutf8; neither ANSI codes nor Unicode will be mangled anymore.  But, that's
quite useless -- I wonder what was your reason to implement OLCUC back in
the day even though it was obsolete by decades even then, and was already
dropped from relevant standards.  Thus, I guess you want proper rune
support, this pull request adds this.  I've chosen Elder Futhark as the
variant to implement.

You might want to consider dropping patch 3, it is required only for full
coverage of the target set (Elder Futhark) but not for regular ASCII
mapping, and brings some code complexity.

Required userspace support is present in default GUI installs of all
distributions I checked, both on old-style terminals that use server-side X
fonts and with client-side {true,open}type -- but not on console unless
someone adds the required characters to the charset, which is tricky because
of 256/512 glyph limitations.  Thus, runes support currently works only on
graphical terminals.


Meow! -- sorry, ᛗᛖᛟᚹ!

Commits up to 14f34ba1d8748f252f941b5bb87efd7b1ed55868 on top of
c1ae3cfa0e89fa1a7ecc4c99031f5e9ae99d9201.


Adam Borowski (4):
  n_tty: don't mangle tty codes in OLCUC mode
  n_tty: use runes rather than uppercase in IUTF8 OLCUC mode
  n_tty: support th, ae and ng runes
  n_tty: wrap all OLCUC code in a config option

 drivers/tty/Kconfig |  17 
 drivers/tty/n_tty.c | 115 ++--
 2 files changed, 120 insertions(+), 12 deletions(-)

-- 
⢀⣴⠾⠻⢶⣦⠀ Meow!
⣾⠁⢠⠒⠀⣿⡁
⢿⡄⠘⠷⠚⠋⠀ Collisions shmolisions, let's see them find a collision or second
⠈⠳⣄ preimage for double rot13!


signature.asc
Description: PGP signature

[PATCH v2 2/2] vt: make mouse selection of non-ASCII consistent

[Adam Borowski]

For some reason a handful of ISO-8859-1 symbols are excluded from "word
chars" while the vast majority of Unicode is hard-coded as included, even
when inappropriate (we really would want to _not_ select line-drawing/etc).
Those symbols are: ¡¢£¤¥¦§¨©ª«¬®¯°±²³´µ¶·¸¹º»¼½¾¿×÷

Thus, let's not special-case any non-ASCII anymore.  Attempts to set these
via ioctl will be silently ignored.

As an extra bonus, we debloat the kernel by 128 bytes.

Signed-off-by: Adam Borowski <kilob...@angband.pl>
---
v2: Got rid of hard-coded array sizes.

 drivers/tty/vt/selection.c | 16 ++--
 1 file changed, 6 insertions(+), 10 deletions(-)

diff --git a/drivers/tty/vt/selection.c b/drivers/tty/vt/selection.c
index 2252e11d8347..accbd1257bc4 100644
--- a/drivers/tty/vt/selection.c
+++ b/drivers/tty/vt/selection.c
@@ -80,21 +80,17 @@ void clear_selection(void)
 
 /*
  * User settable table: what characters are to be considered alphabetic?
- * 256 bits. Locked by the console lock.
+ * 128 bits. Locked by the console lock.
  */
-static u32 inwordLut[8]={
+static u32 inwordLut[]={
   0x, /* control chars */
   0x03FFE000, /* digits and "-./"  */
   0x87FE, /* uppercase and '_' */
   0x07FE, /* lowercase */
-  0x,
-  0x,
-  0xFF7F, /* latin-1 accented letters, not multiplication sign */
-  0xFF7F  /* latin-1 accented letters, not division sign */
 };
 
 static inline int inword(const u16 c) {
-   return c > 0xff || (( inwordLut[c>>5] >> (c & 0x1F) ) & 1);
+   return c > 0x7f || (( inwordLut[c>>5] >> (c & 0x1F) ) & 1);
 }
 
 /**
@@ -106,10 +102,10 @@ static inline int inword(const u16 c) {
  */
 int sel_loadlut(char __user *p)
 {
-   u32 tmplut[8];
-   if (copy_from_user(tmplut, (u32 __user *)(p+4), 32))
+   u32 tmplut[ARRAY_SIZE(inwordLut)];
+   if (copy_from_user(tmplut, (u32 __user *)(p+4), sizeof(inwordLut)))
return -EFAULT;
-   memcpy(inwordLut, tmplut, 32);
+   memcpy(inwordLut, tmplut, sizeof(inwordLut));
return 0;
 }
 
-- 
2.11.0

[PATCH v2 2/2] vt: make mouse selection of non-ASCII consistent

[Adam Borowski]

For some reason a handful of ISO-8859-1 symbols are excluded from "word
chars" while the vast majority of Unicode is hard-coded as included, even
when inappropriate (we really would want to _not_ select line-drawing/etc).
Those symbols are: ¡¢£¤¥¦§¨©ª«¬®¯°±²³´µ¶·¸¹º»¼½¾¿×÷

Thus, let's not special-case any non-ASCII anymore.  Attempts to set these
via ioctl will be silently ignored.

As an extra bonus, we debloat the kernel by 128 bytes.

Signed-off-by: Adam Borowski 
---
v2: Got rid of hard-coded array sizes.

 drivers/tty/vt/selection.c | 16 ++--
 1 file changed, 6 insertions(+), 10 deletions(-)

diff --git a/drivers/tty/vt/selection.c b/drivers/tty/vt/selection.c
index 2252e11d8347..accbd1257bc4 100644
--- a/drivers/tty/vt/selection.c
+++ b/drivers/tty/vt/selection.c
@@ -80,21 +80,17 @@ void clear_selection(void)
 
 /*
  * User settable table: what characters are to be considered alphabetic?
- * 256 bits. Locked by the console lock.
+ * 128 bits. Locked by the console lock.
  */
-static u32 inwordLut[8]={
+static u32 inwordLut[]={
   0x, /* control chars */
   0x03FFE000, /* digits and "-./"  */
   0x87FE, /* uppercase and '_' */
   0x07FE, /* lowercase */
-  0x,
-  0x,
-  0xFF7F, /* latin-1 accented letters, not multiplication sign */
-  0xFF7F  /* latin-1 accented letters, not division sign */
 };
 
 static inline int inword(const u16 c) {
-   return c > 0xff || (( inwordLut[c>>5] >> (c & 0x1F) ) & 1);
+   return c > 0x7f || (( inwordLut[c>>5] >> (c & 0x1F) ) & 1);
 }
 
 /**
@@ -106,10 +102,10 @@ static inline int inword(const u16 c) {
  */
 int sel_loadlut(char __user *p)
 {
-   u32 tmplut[8];
-   if (copy_from_user(tmplut, (u32 __user *)(p+4), 32))
+   u32 tmplut[ARRAY_SIZE(inwordLut)];
+   if (copy_from_user(tmplut, (u32 __user *)(p+4), sizeof(inwordLut)))
return -EFAULT;
-   memcpy(inwordLut, tmplut, 32);
+   memcpy(inwordLut, tmplut, sizeof(inwordLut));
return 0;
 }
 
-- 
2.11.0

[PATCH v2 1/2] vt: set mouse selection word-chars to gpm's default

[Adam Borowski]

Since forever, gpm was this code's only user, and it overrides the table on
start so the default was never seen -- until Bill Allombert's "consolation"
came in.  The in-kernel set is "A-Za-z0-9_" which fails to catch typical
file names, etc.  Let's change this to gpm's conservative default, ie
"-A-Za-z0-9_./"; most terminals include more, for example xfce4-terminal has

[PATCH v2 1/2] vt: set mouse selection word-chars to gpm's default

[Adam Borowski]

Since forever, gpm was this code's only user, and it overrides the table on
start so the default was never seen -- until Bill Allombert's "consolation"
came in.  The in-kernel set is "A-Za-z0-9_" which fails to catch typical
file names, etc.  Let's change this to gpm's conservative default, ie
"-A-Za-z0-9_./"; most terminals include more, for example xfce4-terminal has

[PATCH 1/2] vt: set mouse selection word-chars to gpm's default

[Adam Borowski]

Since forever, gpm was this code's only user, and it overrides the table on
start so the default was never seen -- until Bill Allombert's "consolation"
came in.  The in-kernel set is "A-Za-z0-9_" which fails to catch typical
file names, etc.  Let's change this to gpm's conservative default, ie
"-A-Za-z0-9_./"; most terminals include more, for example xfce4-terminal has

[PATCH 1/2] vt: set mouse selection word-chars to gpm's default

[Adam Borowski]

Since forever, gpm was this code's only user, and it overrides the table on
start so the default was never seen -- until Bill Allombert's "consolation"
came in.  The in-kernel set is "A-Za-z0-9_" which fails to catch typical
file names, etc.  Let's change this to gpm's conservative default, ie
"-A-Za-z0-9_./"; most terminals include more, for example xfce4-terminal has

[PATCH 2/2] vt: make mouse selection of non-ASCII consistent

[Adam Borowski]

For some reason a handful of ISO-8859-1 symbols are excluded from "word
chars" while the vast majority of Unicode is hard-coded as included, even
when inappropriate (we really would want to _not_ select line-drawing/etc).
Those symbols are: ¡¢£¤¥¦§¨©ª«¬®¯°±²³´µ¶·¸¹º»¼½¾¿×÷

Thus, let's not special-case any non-ASCII anymore.  Attempts to set these
via ioctl will be silently ignored.

As an extra bonus, we debloat the kernel by 128 bytes.

Signed-off-by: Adam Borowski <kilob...@angband.pl>
---
 drivers/tty/vt/selection.c | 16 ++--
 1 file changed, 6 insertions(+), 10 deletions(-)

diff --git a/drivers/tty/vt/selection.c b/drivers/tty/vt/selection.c
index 2252e11d8347..c81c99165ea6 100644
--- a/drivers/tty/vt/selection.c
+++ b/drivers/tty/vt/selection.c
@@ -80,21 +80,17 @@ void clear_selection(void)
 
 /*
  * User settable table: what characters are to be considered alphabetic?
- * 256 bits. Locked by the console lock.
+ * 128 bits. Locked by the console lock.
  */
-static u32 inwordLut[8]={
+static u32 inwordLut[4]={
   0x, /* control chars */
   0x03FFE000, /* digits and "-./"  */
   0x87FE, /* uppercase and '_' */
   0x07FE, /* lowercase */
-  0x,
-  0x,
-  0xFF7F, /* latin-1 accented letters, not multiplication sign */
-  0xFF7F  /* latin-1 accented letters, not division sign */
 };
 
 static inline int inword(const u16 c) {
-   return c > 0xff || (( inwordLut[c>>5] >> (c & 0x1F) ) & 1);
+   return c > 0x7f || (( inwordLut[c>>5] >> (c & 0x1F) ) & 1);
 }
 
 /**
@@ -106,10 +102,10 @@ static inline int inword(const u16 c) {
  */
 int sel_loadlut(char __user *p)
 {
-   u32 tmplut[8];
-   if (copy_from_user(tmplut, (u32 __user *)(p+4), 32))
+   u32 tmplut[4];
+   if (copy_from_user(tmplut, (u32 __user *)(p+4), 16))
return -EFAULT;
-   memcpy(inwordLut, tmplut, 32);
+   memcpy(inwordLut, tmplut, 16);
return 0;
 }
 
-- 
2.11.0

[PATCH 2/2] vt: make mouse selection of non-ASCII consistent

[Adam Borowski]

For some reason a handful of ISO-8859-1 symbols are excluded from "word
chars" while the vast majority of Unicode is hard-coded as included, even
when inappropriate (we really would want to _not_ select line-drawing/etc).
Those symbols are: ¡¢£¤¥¦§¨©ª«¬®¯°±²³´µ¶·¸¹º»¼½¾¿×÷

Thus, let's not special-case any non-ASCII anymore.  Attempts to set these
via ioctl will be silently ignored.

As an extra bonus, we debloat the kernel by 128 bytes.

Signed-off-by: Adam Borowski 
---
 drivers/tty/vt/selection.c | 16 ++--
 1 file changed, 6 insertions(+), 10 deletions(-)

diff --git a/drivers/tty/vt/selection.c b/drivers/tty/vt/selection.c
index 2252e11d8347..c81c99165ea6 100644
--- a/drivers/tty/vt/selection.c
+++ b/drivers/tty/vt/selection.c
@@ -80,21 +80,17 @@ void clear_selection(void)
 
 /*
  * User settable table: what characters are to be considered alphabetic?
- * 256 bits. Locked by the console lock.
+ * 128 bits. Locked by the console lock.
  */
-static u32 inwordLut[8]={
+static u32 inwordLut[4]={
   0x, /* control chars */
   0x03FFE000, /* digits and "-./"  */
   0x87FE, /* uppercase and '_' */
   0x07FE, /* lowercase */
-  0x,
-  0x,
-  0xFF7F, /* latin-1 accented letters, not multiplication sign */
-  0xFF7F  /* latin-1 accented letters, not division sign */
 };
 
 static inline int inword(const u16 c) {
-   return c > 0xff || (( inwordLut[c>>5] >> (c & 0x1F) ) & 1);
+   return c > 0x7f || (( inwordLut[c>>5] >> (c & 0x1F) ) & 1);
 }
 
 /**
@@ -106,10 +102,10 @@ static inline int inword(const u16 c) {
  */
 int sel_loadlut(char __user *p)
 {
-   u32 tmplut[8];
-   if (copy_from_user(tmplut, (u32 __user *)(p+4), 32))
+   u32 tmplut[4];
+   if (copy_from_user(tmplut, (u32 __user *)(p+4), 16))
return -EFAULT;
-   memcpy(inwordLut, tmplut, 32);
+   memcpy(inwordLut, tmplut, 16);
return 0;
 }
 
-- 
2.11.0

[PATCH 0/2] vt: mouse selection word boundaries

[Adam Borowski]

Hi!
Here's a couple of really low priority fixes to how "word chars" for mouse
selection are determined.

Patch 1 (adds "-./") is an epitome of "apply if bored": for two decades,
only gpm used this, and it always ignored the defaults.  Bill Allombert made
a second implementation, "consolation", and we wasted a bit of time figuring
out why mouse selection is so limited.  As consolation now overrides the
defaults too, this patch will be helpful only for whoever makes a third user
of this code...

Patch 2, besides making a handful of non-ASCII symbols work the same as
everything else, also stops people from mysteriously losing that 'ü' after
naive gpm -l "-A-Za-z0-9_./".

-- 
⢀⣴⠾⠻⢶⣦⠀ Meow!
⣾⠁⢠⠒⠀⣿⡁
⢿⡄⠘⠷⠚⠋⠀ Collisions shmolisions, let's see them find a collision or second
⠈⠳⣄ preimage for double rot13!

[PATCH 0/2] vt: mouse selection word boundaries

[Adam Borowski]

Hi!
Here's a couple of really low priority fixes to how "word chars" for mouse
selection are determined.

Patch 1 (adds "-./") is an epitome of "apply if bored": for two decades,
only gpm used this, and it always ignored the defaults.  Bill Allombert made
a second implementation, "consolation", and we wasted a bit of time figuring
out why mouse selection is so limited.  As consolation now overrides the
defaults too, this patch will be helpful only for whoever makes a third user
of this code...

Patch 2, besides making a handful of non-ASCII symbols work the same as
everything else, also stops people from mysteriously losing that 'ü' after
naive gpm -l "-A-Za-z0-9_./".

-- 
⢀⣴⠾⠻⢶⣦⠀ Meow!
⣾⠁⢠⠒⠀⣿⡁
⢿⡄⠘⠷⠚⠋⠀ Collisions shmolisions, let's see them find a collision or second
⠈⠳⣄ preimage for double rot13!

Re: [PATCH linux-next] tty: Disable default console blanking interval

[Adam Borowski]

On Wed, Mar 22, 2017 at 07:50:32AM -0600, Tim Gardner wrote:
> BugLink: http://bugs.launchpad.net/bugs/869017
> 
> I'm not particularly knowledgable about console issues. Is a blaknking 
> interval
> relevant in a post CRT world ? The argument in the bug description seems
> compelling.

I have no direct knowledge about screen burn-in, but a quick search shows
that, while not as prevalent as in the CRT days, it's still an issue:
https://en.wikipedia.org/wiki/Screen_burn-in
https://encrypted.google.com/search?hl=en=lcd%20screen%20burn-in

> -static int blankinterval = 10*60;
> +static int blankinterval;

Thus, the current default might be safer: an even all-black image will keep
the display readable as there won't be any localized artefacts.  On the
other hand, the photos I see are nowhere as bad as it was the case on CRTs,
so this reason might be dismissed.


There is another concern, though: light pollution.  A white image makes the
room bright enough to read by.  Don't laugh but 20 years ago in the dorm I
used to print \e[37;47;1m then 2000 'X'es to the screen to do things[1] (the
overhead light would wake up the roommates).  That was a CRT, LCDs are
brighter.  Obviously, lightgrey text on a black background producess less
light than all-white, but a regular monitor probably still gives off a total
amount of light similar to that of an all-white smartphone.

I'm not sure how many non-X screens are placed in rooms where someone tries
to sleep, but it's not something to ignore entirely.


I'm not arguing a hard "no", but you should at least think of the above two
concerns.


[1]. Not reading obviously, I am not _that_ insane. :þ
-- 
⢀⣴⠾⠻⢶⣦⠀ Meow!
⣾⠁⢠⠒⠀⣿⡁
⢿⡄⠘⠷⠚⠋⠀ Collisions shmolisions, let's see them find a collision or second
⠈⠳⣄ preimage for double rot13!

Re: [PATCH linux-next] tty: Disable default console blanking interval

[Adam Borowski]

On Wed, Mar 22, 2017 at 07:50:32AM -0600, Tim Gardner wrote:
> BugLink: http://bugs.launchpad.net/bugs/869017
> 
> I'm not particularly knowledgable about console issues. Is a blaknking 
> interval
> relevant in a post CRT world ? The argument in the bug description seems
> compelling.

I have no direct knowledge about screen burn-in, but a quick search shows
that, while not as prevalent as in the CRT days, it's still an issue:
https://en.wikipedia.org/wiki/Screen_burn-in
https://encrypted.google.com/search?hl=en=lcd%20screen%20burn-in

> -static int blankinterval = 10*60;
> +static int blankinterval;

Thus, the current default might be safer: an even all-black image will keep
the display readable as there won't be any localized artefacts.  On the
other hand, the photos I see are nowhere as bad as it was the case on CRTs,
so this reason might be dismissed.


There is another concern, though: light pollution.  A white image makes the
room bright enough to read by.  Don't laugh but 20 years ago in the dorm I
used to print \e[37;47;1m then 2000 'X'es to the screen to do things[1] (the
overhead light would wake up the roommates).  That was a CRT, LCDs are
brighter.  Obviously, lightgrey text on a black background producess less
light than all-white, but a regular monitor probably still gives off a total
amount of light similar to that of an all-white smartphone.

I'm not sure how many non-X screens are placed in rooms where someone tries
to sleep, but it's not something to ignore entirely.


I'm not arguing a hard "no", but you should at least think of the above two
concerns.


[1]. Not reading obviously, I am not _that_ insane. :þ
-- 
⢀⣴⠾⠻⢶⣦⠀ Meow!
⣾⠁⢠⠒⠀⣿⡁
⢿⡄⠘⠷⠚⠋⠀ Collisions shmolisions, let's see them find a collision or second
⠈⠳⣄ preimage for double rot13!

Re: [PATCHv3] x86/mm: set x32 syscall bit in SET_PERSONALITY()

[Adam Borowski]

On Tue, Mar 21, 2017 at 08:47:11PM +0300, Dmitry Safonov wrote:
> After my changes to mmap(), its code now relies on the bitness of
> performing syscall. According to that, it chooses the base of allocation:
> mmap_base for 64-bit mmap() and mmap_compat_base for 32-bit syscall.
> It was done by:
>   commit 1b028f784e8c ("x86/mm: Introduce mmap_compat_base() for
> 32-bit mmap()").
> 
> The code afterwards relies on in_compat_syscall() returning true for
> 32-bit syscalls. It's usually so while we're in context of application
> that does 32-bit syscalls. But during exec() it is not valid for x32 ELF.
> The reason is that the application hasn't yet done any syscall, so x32
> bit has not being set.
> That results in -ENOMEM for x32 ELF files as there fired BAD_ADDR()
> in elf_map(), that is called from do_execve()->load_elf_binary().
> For i386 ELFs it works as SET_PERSONALITY() sets TS_COMPAT flag.
> 
> Set x32 bit before first return to userspace, during setting personality
> at exec(). This way we can rely on in_compat_syscall() during exec().
> Do also the reverse: drop x32 syscall bit at SET_PERSONALITY for 64-bits.
> 
> Fixes: commit 1b028f784e8c ("x86/mm: Introduce mmap_compat_base() for
> 32-bit mmap()")

Tested:
with bash:x32, mksh:amd64, posh:i386, zsh:armhf (binfmt:qemu), fork+exec
works for every parent-child combination.

Contrary to my naive initial reading of your fix, mixing syscalls from a
process of the wrong ABI also works as it did before.  While using a glibc
wrapper will call the right version, x32 processes calling amd64 syscalls is
surprisingly common -- this brings seccomp joy.

I've attached a freestanding test case for write() and mmap(); it's
freestanding asm as most of you don't have an x32 toolchain at hand, sorry
for unfriendly error messages.

So with these two patches:
x86/tls: Forcibly set the accessed bit in TLS segments
x86/mm: set x32 syscall bit in SET_PERSONALITY()
everything appears to be fine.

-- 
⢀⣴⠾⠻⢶⣦⠀ Meow!
⣾⠁⢠⠒⠀⣿⡁
⢿⡄⠘⠷⠚⠋⠀ Collisions shmolisions, let's see them find a collision or second
⠈⠳⣄ preimage for double rot13!
.globl _start
.data
msg:.ascii "Meow!\n"
badmsg: .ascii "syscall failed\n"
.text
_start:
# x32
mov $0x4001, %rax   # syscall: write
mov $1, %rdi
mov $msg, %rsi
mov $6, %rdx
syscall

# amd64
mov $1, %rax# syscall: write
mov $1, %rdi
mov $msg, %rsi
mov $6, %rdx
syscall

# i386
mov $4, %eax# syscall: write
mov $1, %ebx
mov $msg, %ecx
mov $6, %edx
int $0x80


# x32
mov $0x4009, %rax   # syscall: mmap
mov $0, %rdi
mov $0x1, %rsi
mov $3, %rdx# PROT_READ|PROT_WRITE
mov $0x62, %r10 # MAP_PRIVATE|MAP_ANON|MAP_32BIT
mov $-1, %r8
mov $0, %r9
syscall
or  %rax, %rax
js  badness

# amd64
mov $0x9, %rax  # syscall: mmap
mov $0, %rdi
mov $0x1, %rsi
mov $3, %rdx# PROT_READ|PROT_WRITE
mov $0x62, %r10 # MAP_PRIVATE|MAP_ANON|MAP_32BIT
mov $-1, %r8
mov $0, %r9
syscall
or  %rax, %rax
js  badness

jmp goodbye # m'kay, this one doesn't work, no regression
# i386
mov $0x90, %eax # syscall: mmap
mov $0, %ebx
mov $0x1, %ecx
mov $3, %edx# PROT_READ|PROT_WRITE
mov $0x62, %esi # MAP_PRIVATE|MAP_ANON|MAP_32BIT
mov $-1, %edi
mov $0, %ebp
int $0x80
movslq  %eax, %rax
or  %rax, %rax
js  badness

goodbye:
mov $0x403c, %rax   # syscall: _exit
xor %rdi, %rdi
syscall

badness:
# I'm too lazy to printf this as a number...
push%rax
mov $0x4001, %rax   # syscall: write
mov $1, %rdi
mov $badmsg, %rsi
mov $15, %rdx
syscall

mov $0x403c, %rax   # syscall: _exit
pop %rdi
syscall
# Any of amd64/x32/i386 will do.
X86=x86_64-linux-gnu

all: meow-x32 meow-amd64
clean:
rm -f meow-*

meow-x32: meow.s
$(X86)-as --x32 $^ -o $@.o
$(X86)-ld -melf32_x86_64 -s $@.o -o $@

meow-amd64: meow.s
$(X86)-as --64 $^ -o $@.o
$(X86)-ld -melf_x86_64 -s $@.o -o $@

Re: [PATCHv3] x86/mm: set x32 syscall bit in SET_PERSONALITY()

[Adam Borowski]

On Tue, Mar 21, 2017 at 08:47:11PM +0300, Dmitry Safonov wrote:
> After my changes to mmap(), its code now relies on the bitness of
> performing syscall. According to that, it chooses the base of allocation:
> mmap_base for 64-bit mmap() and mmap_compat_base for 32-bit syscall.
> It was done by:
>   commit 1b028f784e8c ("x86/mm: Introduce mmap_compat_base() for
> 32-bit mmap()").
> 
> The code afterwards relies on in_compat_syscall() returning true for
> 32-bit syscalls. It's usually so while we're in context of application
> that does 32-bit syscalls. But during exec() it is not valid for x32 ELF.
> The reason is that the application hasn't yet done any syscall, so x32
> bit has not being set.
> That results in -ENOMEM for x32 ELF files as there fired BAD_ADDR()
> in elf_map(), that is called from do_execve()->load_elf_binary().
> For i386 ELFs it works as SET_PERSONALITY() sets TS_COMPAT flag.
> 
> Set x32 bit before first return to userspace, during setting personality
> at exec(). This way we can rely on in_compat_syscall() during exec().
> Do also the reverse: drop x32 syscall bit at SET_PERSONALITY for 64-bits.
> 
> Fixes: commit 1b028f784e8c ("x86/mm: Introduce mmap_compat_base() for
> 32-bit mmap()")

Tested:
with bash:x32, mksh:amd64, posh:i386, zsh:armhf (binfmt:qemu), fork+exec
works for every parent-child combination.

Contrary to my naive initial reading of your fix, mixing syscalls from a
process of the wrong ABI also works as it did before.  While using a glibc
wrapper will call the right version, x32 processes calling amd64 syscalls is
surprisingly common -- this brings seccomp joy.

I've attached a freestanding test case for write() and mmap(); it's
freestanding asm as most of you don't have an x32 toolchain at hand, sorry
for unfriendly error messages.

So with these two patches:
x86/tls: Forcibly set the accessed bit in TLS segments
x86/mm: set x32 syscall bit in SET_PERSONALITY()
everything appears to be fine.

-- 
⢀⣴⠾⠻⢶⣦⠀ Meow!
⣾⠁⢠⠒⠀⣿⡁
⢿⡄⠘⠷⠚⠋⠀ Collisions shmolisions, let's see them find a collision or second
⠈⠳⣄ preimage for double rot13!
.globl _start
.data
msg:.ascii "Meow!\n"
badmsg: .ascii "syscall failed\n"
.text
_start:
# x32
mov $0x4001, %rax   # syscall: write
mov $1, %rdi
mov $msg, %rsi
mov $6, %rdx
syscall

# amd64
mov $1, %rax# syscall: write
mov $1, %rdi
mov $msg, %rsi
mov $6, %rdx
syscall

# i386
mov $4, %eax# syscall: write
mov $1, %ebx
mov $msg, %ecx
mov $6, %edx
int $0x80


# x32
mov $0x4009, %rax   # syscall: mmap
mov $0, %rdi
mov $0x1, %rsi
mov $3, %rdx# PROT_READ|PROT_WRITE
mov $0x62, %r10 # MAP_PRIVATE|MAP_ANON|MAP_32BIT
mov $-1, %r8
mov $0, %r9
syscall
or  %rax, %rax
js  badness

# amd64
mov $0x9, %rax  # syscall: mmap
mov $0, %rdi
mov $0x1, %rsi
mov $3, %rdx# PROT_READ|PROT_WRITE
mov $0x62, %r10 # MAP_PRIVATE|MAP_ANON|MAP_32BIT
mov $-1, %r8
mov $0, %r9
syscall
or  %rax, %rax
js  badness

jmp goodbye # m'kay, this one doesn't work, no regression
# i386
mov $0x90, %eax # syscall: mmap
mov $0, %ebx
mov $0x1, %ecx
mov $3, %edx# PROT_READ|PROT_WRITE
mov $0x62, %esi # MAP_PRIVATE|MAP_ANON|MAP_32BIT
mov $-1, %edi
mov $0, %ebp
int $0x80
movslq  %eax, %rax
or  %rax, %rax
js  badness

goodbye:
mov $0x403c, %rax   # syscall: _exit
xor %rdi, %rdi
syscall

badness:
# I'm too lazy to printf this as a number...
push%rax
mov $0x4001, %rax   # syscall: write
mov $1, %rdi
mov $badmsg, %rsi
mov $15, %rdx
syscall

mov $0x403c, %rax   # syscall: _exit
pop %rdi
syscall
# Any of amd64/x32/i386 will do.
X86=x86_64-linux-gnu

all: meow-x32 meow-amd64
clean:
rm -f meow-*

meow-x32: meow.s
$(X86)-as --x32 $^ -o $@.o
$(X86)-ld -melf32_x86_64 -s $@.o -o $@

meow-amd64: meow.s
$(X86)-as --64 $^ -o $@.o
$(X86)-ld -melf_x86_64 -s $@.o -o $@

Re: linux-next: x86: Unalbe to run x32 processes on the x86_64 kernel

[Adam Borowski]

On Tue, Mar 21, 2017 at 07:45:39AM +0100, Ingo Molnar wrote:
> * Andrei Vagin  wrote:
> 
> > # first bad commit: [45fc8757d1d2128e342b4e7ef39adedf7752faac] x86:
> > Make the GDT remapping read-only on 64-bit
> 
> Just wondering, does the following commit fix it:
> 
>   5b781c7e317f x86/tls: Forcibly set the accessed bit in TLS segments

It does fix i386 but not x32.

By "x32" I mean CONFIG_X86_X32, by "i386" CONFIG_IA32_EMULATION, contrary to
Andrei's first report.  The naming of the new ABI wasn't too fortunate...

-- 
⢀⣴⠾⠻⢶⣦⠀ Meow!
⣾⠁⢠⠒⠀⣿⡁
⢿⡄⠘⠷⠚⠋⠀ Collisions shmolisions, let's see them find a collision or second
⠈⠳⣄ preimage for double rot13!

Re: linux-next: x86: Unalbe to run x32 processes on the x86_64 kernel

[Adam Borowski]

On Tue, Mar 21, 2017 at 07:45:39AM +0100, Ingo Molnar wrote:
> * Andrei Vagin  wrote:
> 
> > # first bad commit: [45fc8757d1d2128e342b4e7ef39adedf7752faac] x86:
> > Make the GDT remapping read-only on 64-bit
> 
> Just wondering, does the following commit fix it:
> 
>   5b781c7e317f x86/tls: Forcibly set the accessed bit in TLS segments

It does fix i386 but not x32.

By "x32" I mean CONFIG_X86_X32, by "i386" CONFIG_IA32_EMULATION, contrary to
Andrei's first report.  The naming of the new ABI wasn't too fortunate...

-- 
⢀⣴⠾⠻⢶⣦⠀ Meow!
⣾⠁⢠⠒⠀⣿⡁
⢿⡄⠘⠷⠚⠋⠀ Collisions shmolisions, let's see them find a collision or second
⠈⠳⣄ preimage for double rot13!

Re: linux-next: x86: Unalbe to run x32 processes on the x86_64 kernel

[Adam Borowski]

On Mon, Mar 20, 2017 at 04:57:39PM -0700, Andrei Vagin wrote:
> We run CRIU tests on linux-next. And today we found that when we start
> x32 processes, a kernel bug is triggered:
> 
> [root@fc24 ~]# uname -a
> Linux fc24 4.11.0-rc2-next-20170320 #159 SMP Mon Mar 20 16:53:58 PDT
> 2017 x86_64 x86_64 x86_64 GNU/Linux
> [root@fc24 ~]# cat t.c
> int main()
> {
> return 0;
> }
> [root@fc24 ~]# gcc -m32 t.c

-m32 is i386, for x32 you need -mx32.

> [root@fc24 ~]# ./a.out
> Killed
> [root@fc24 ~]# dmesg
> [   90.033310] BUG: unable to handle kernel paging request at ff576060

Indeed, same for me for i386.
On x32 the process gets killed with SEGV with no core, no kernel output.

On the other hand, a bare glibc-less process (write(), _exit()) works fine
both on i386 and x32.

I haven't looked any closer yet.

-- 
⢀⣴⠾⠻⢶⣦⠀ Meow!
⣾⠁⢠⠒⠀⣿⡁
⢿⡄⠘⠷⠚⠋⠀ Collisions shmolisions, let's see them find a collision or second
⠈⠳⣄ preimage for double rot13!

Re: linux-next: x86: Unalbe to run x32 processes on the x86_64 kernel

[Adam Borowski]

On Mon, Mar 20, 2017 at 04:57:39PM -0700, Andrei Vagin wrote:
> We run CRIU tests on linux-next. And today we found that when we start
> x32 processes, a kernel bug is triggered:
> 
> [root@fc24 ~]# uname -a
> Linux fc24 4.11.0-rc2-next-20170320 #159 SMP Mon Mar 20 16:53:58 PDT
> 2017 x86_64 x86_64 x86_64 GNU/Linux
> [root@fc24 ~]# cat t.c
> int main()
> {
> return 0;
> }
> [root@fc24 ~]# gcc -m32 t.c

-m32 is i386, for x32 you need -mx32.

> [root@fc24 ~]# ./a.out
> Killed
> [root@fc24 ~]# dmesg
> [   90.033310] BUG: unable to handle kernel paging request at ff576060

Indeed, same for me for i386.
On x32 the process gets killed with SEGV with no core, no kernel output.

On the other hand, a bare glibc-less process (write(), _exit()) works fine
both on i386 and x32.

I haven't looked any closer yet.

-- 
⢀⣴⠾⠻⢶⣦⠀ Meow!
⣾⠁⢠⠒⠀⣿⡁
⢿⡄⠘⠷⠚⠋⠀ Collisions shmolisions, let's see them find a collision or second
⠈⠳⣄ preimage for double rot13!

Re: sun50i-a64-pinctrl WARN_ON drivers/base/dd.c:349

[Adam Borowski]

On Fri, Mar 17, 2017 at 10:44:22AM -0400, Tejun Heo wrote:
> On Fri, Mar 17, 2017 at 10:28:34PM +0800, Icenowy Zheng wrote:
> > > It's warning that the device has resources associated with it on
> > > probe. There gotta be something fishy going on with the probing
> > > sequence. How reproducible is the problem?
> > 
> > Do you mean in the first probing trial the driver didn't clean up well?
> 
> Possibly but devres should have released all resources after the
> previous probe failure or driver disassociation, so I have no idea how
> there can be resources left on that list.
> 
> > With the same driver I didn't see this problem in 4.11-rc{1,2}.
> 
> devres hasn't changed, so I have no idea what changed that.  Which
> kernels are affected?

It's a not-yet-mainlined part (that's why I sent the report to Icenowy and
Andre Przywara -- they work on the DT bindings and the driver itself).

The exact kernel is based on g...@github.com:Icenowy/linux.git
(icenowy/sunxi64-4.11-rc1); I've removed all additional commits that could
possibly be related.  I need at the very least a f2fs fix to boot this
machine (without reformatting).  I did not bother trimming things that are
obviously irrelevant (n_tty, vt, btrfs, sd); lemme retry with 4.11-rc2 that
has just the f2fs fix and "#define DEBUG" in drivers/base/dd.c to be sure.

I've first noticed the warning on pre-4.11 -next; because of very intense
work in these areas Icenowy, Andre and the rest are doing the pine64 parts
are hard to naively rebase -- and thus I haven't run intermediate versions.

> Can you bisect if the problem is easily reproducible?

While it reproduces 100%, the churn in required commits atop mainline would
make bisecting pretty tricky.  Do you have some other ideas over debugging
the hard way?  (I don't know these parts of the kernel myself thus I
reported first.)


In case my exact .config and tree are relevant:
https://angband.pl/tmp/config-kb-ice-4.11-rc1.xz
g...@github.com:kilobyte/linux.git kb-ice-4.11-rc1
-- 
⢀⣴⠾⠻⢶⣦⠀ Meow!
⣾⠁⢠⠒⠀⣿⡁
⢿⡄⠘⠷⠚⠋⠀ Collisions shmolisions, let's see them find a collision or second
⠈⠳⣄ preimage for double rot13!

Re: sun50i-a64-pinctrl WARN_ON drivers/base/dd.c:349

[Adam Borowski]

On Fri, Mar 17, 2017 at 10:44:22AM -0400, Tejun Heo wrote:
> On Fri, Mar 17, 2017 at 10:28:34PM +0800, Icenowy Zheng wrote:
> > > It's warning that the device has resources associated with it on
> > > probe. There gotta be something fishy going on with the probing
> > > sequence. How reproducible is the problem?
> > 
> > Do you mean in the first probing trial the driver didn't clean up well?
> 
> Possibly but devres should have released all resources after the
> previous probe failure or driver disassociation, so I have no idea how
> there can be resources left on that list.
> 
> > With the same driver I didn't see this problem in 4.11-rc{1,2}.
> 
> devres hasn't changed, so I have no idea what changed that.  Which
> kernels are affected?

It's a not-yet-mainlined part (that's why I sent the report to Icenowy and
Andre Przywara -- they work on the DT bindings and the driver itself).

The exact kernel is based on g...@github.com:Icenowy/linux.git
(icenowy/sunxi64-4.11-rc1); I've removed all additional commits that could
possibly be related.  I need at the very least a f2fs fix to boot this
machine (without reformatting).  I did not bother trimming things that are
obviously irrelevant (n_tty, vt, btrfs, sd); lemme retry with 4.11-rc2 that
has just the f2fs fix and "#define DEBUG" in drivers/base/dd.c to be sure.

I've first noticed the warning on pre-4.11 -next; because of very intense
work in these areas Icenowy, Andre and the rest are doing the pine64 parts
are hard to naively rebase -- and thus I haven't run intermediate versions.

> Can you bisect if the problem is easily reproducible?

While it reproduces 100%, the churn in required commits atop mainline would
make bisecting pretty tricky.  Do you have some other ideas over debugging
the hard way?  (I don't know these parts of the kernel myself thus I
reported first.)


In case my exact .config and tree are relevant:
https://angband.pl/tmp/config-kb-ice-4.11-rc1.xz
g...@github.com:kilobyte/linux.git kb-ice-4.11-rc1
-- 
⢀⣴⠾⠻⢶⣦⠀ Meow!
⣾⠁⢠⠒⠀⣿⡁
⢿⡄⠘⠷⠚⠋⠀ Collisions shmolisions, let's see them find a collision or second
⠈⠳⣄ preimage for double rot13!

Re: [PATCH v9 4/4] console: Make persistent scrollback a boot parameter

[Adam Borowski]

On Fri, Feb 03, 2017 at 05:04:15PM +0100, Manuel Schölling wrote:
> On Thu, 2017-02-02 at 15:07 -0500, Paul Gortmaker wrote:
> > On Tue, Jan 10, 2017 at 4:28 PM, Manuel Schölling
> >  wrote:
> > > The impact of the persistent scrollback feature on the code size is
> > > rather small, so the config option is removed. The feature stays
> > > disabled by default and can be enabled by using the boot command
> > > line
> > > parameter 'vgacon.scrollback_persistent=1' or by setting
> > > VGACON_SOFT_SCROLLBACK_PERSISTENT_ENABLE_BY_DEFAULT=y.
> > 
> > [...]
> > 
> > > +module_param_named(scrollback_persistent, scrollback_persistent,
> > > bool, );
> > > +MODULE_PARM_DESC(scrollback_persistent, "Enable persistent
> > > scrollback for all vga consoles");
> > 
> > Since this hasn't got widespread deployment yet and  only exists
> > in Greg's tree, can we please fix the above to use setup_param or
> > similar, since there is nothing modular about this code at all.
> Not sure what you mean here.
> If this is not the right may to declare it I'd be more than happy to
> change this. But I could not find any function/macro named setup_param
> [1].
> It would be great if you could give me a hint what function to use
> here!
> 
> [1] http://lxr.free-electrons.com/ident?i=setup_param

That shows only exact matches.

You want "git grep setup_param", which shows __setup_param() plus some
unrelated stuff.  I see only four uses in the kernel, but that's enough
to see how to use it.


Meow!
-- 
Autotools hint: to do a zx-spectrum build on a pdp11 host, type:
  ./configure --host=zx-spectrum --build=pdp11

Re: [PATCH v9 4/4] console: Make persistent scrollback a boot parameter

[Adam Borowski]

On Fri, Feb 03, 2017 at 05:04:15PM +0100, Manuel Schölling wrote:
> On Thu, 2017-02-02 at 15:07 -0500, Paul Gortmaker wrote:
> > On Tue, Jan 10, 2017 at 4:28 PM, Manuel Schölling
> >  wrote:
> > > The impact of the persistent scrollback feature on the code size is
> > > rather small, so the config option is removed. The feature stays
> > > disabled by default and can be enabled by using the boot command
> > > line
> > > parameter 'vgacon.scrollback_persistent=1' or by setting
> > > VGACON_SOFT_SCROLLBACK_PERSISTENT_ENABLE_BY_DEFAULT=y.
> > 
> > [...]
> > 
> > > +module_param_named(scrollback_persistent, scrollback_persistent,
> > > bool, );
> > > +MODULE_PARM_DESC(scrollback_persistent, "Enable persistent
> > > scrollback for all vga consoles");
> > 
> > Since this hasn't got widespread deployment yet and  only exists
> > in Greg's tree, can we please fix the above to use setup_param or
> > similar, since there is nothing modular about this code at all.
> Not sure what you mean here.
> If this is not the right may to declare it I'd be more than happy to
> change this. But I could not find any function/macro named setup_param
> [1].
> It would be great if you could give me a hint what function to use
> here!
> 
> [1] http://lxr.free-electrons.com/ident?i=setup_param

That shows only exact matches.

You want "git grep setup_param", which shows __setup_param() plus some
unrelated stuff.  I see only four uses in the kernel, but that's enough
to see how to use it.


Meow!
-- 
Autotools hint: to do a zx-spectrum build on a pdp11 host, type:
  ./configure --host=zx-spectrum --build=pdp11

Re: [PATCH v10 3/4] console: Add persistent scrollback buffers for all VGA consoles

[Adam Borowski]

On Fri, Jan 20, 2017 at 02:31:56PM +0100, Greg KH wrote:
> On Fri, Jan 20, 2017 at 02:16:11PM +0100, Adam Borowski wrote:
> > On Fri, Jan 20, 2017 at 12:04:12AM +0100, Adam Borowski wrote:
> > > On Thu, Jan 19, 2017 at 05:33:14PM +0100, Greg KH wrote:
> > > > I'd recommend that patch get to clear_console() first, having it use the
> > > > new escape sequence, if it isn't supported, shouldn't cause any
> > > > problems, right?
> > 
> > # Subject: Bug#845177 closed by Matthias Klose <d...@debian.org>
> > #
> > # This is an automatic notification regarding your Bug report
> > # which was filed against the bash package:
> > #
> > # #845177: clear_console: assumes VT switch clears scrollback
> > #
> > # It has been closed by Matthias Klose <d...@debian.org>.
> > [...]
> > # Changes:
> > #* clear_console: Securely erase the current console. Closes: #845177.
> 
> This means it was accepted?  Or rejected?

Accepted.

It's in unstable (or will be in the next mirror pulse), we're at the fastest
possible moment to get stuff to the next stable release -- it'll be in
Stretch (about to freeze).  Ubuntu migration is currently open so it'll get
there soon, in time for Zesty (17.04).  Other Debian derivatives likewise
pull at their own pace.

Most distributions unrelated to Debian don't seem to ship clear_console but
I have no real idea: just checked Fedora 25, it doesn't have it in the
default install at least.


Meow!
-- 
Autotools hint: to do a zx-spectrum build on a pdp11 host, type:
  ./configure --host=zx-spectrum --build=pdp11

Re: [PATCH v10 3/4] console: Add persistent scrollback buffers for all VGA consoles

[Adam Borowski]

On Fri, Jan 20, 2017 at 02:31:56PM +0100, Greg KH wrote:
> On Fri, Jan 20, 2017 at 02:16:11PM +0100, Adam Borowski wrote:
> > On Fri, Jan 20, 2017 at 12:04:12AM +0100, Adam Borowski wrote:
> > > On Thu, Jan 19, 2017 at 05:33:14PM +0100, Greg KH wrote:
> > > > I'd recommend that patch get to clear_console() first, having it use the
> > > > new escape sequence, if it isn't supported, shouldn't cause any
> > > > problems, right?
> > 
> > # Subject: Bug#845177 closed by Matthias Klose 
> > #
> > # This is an automatic notification regarding your Bug report
> > # which was filed against the bash package:
> > #
> > # #845177: clear_console: assumes VT switch clears scrollback
> > #
> > # It has been closed by Matthias Klose .
> > [...]
> > # Changes:
> > #* clear_console: Securely erase the current console. Closes: #845177.
> 
> This means it was accepted?  Or rejected?

Accepted.

It's in unstable (or will be in the next mirror pulse), we're at the fastest
possible moment to get stuff to the next stable release -- it'll be in
Stretch (about to freeze).  Ubuntu migration is currently open so it'll get
there soon, in time for Zesty (17.04).  Other Debian derivatives likewise
pull at their own pace.

Most distributions unrelated to Debian don't seem to ship clear_console but
I have no real idea: just checked Fedora 25, it doesn't have it in the
default install at least.


Meow!
-- 
Autotools hint: to do a zx-spectrum build on a pdp11 host, type:
  ./configure --host=zx-spectrum --build=pdp11

Re: [PATCH v10 3/4] console: Add persistent scrollback buffers for all VGA consoles

[Adam Borowski]

On Fri, Jan 20, 2017 at 12:04:12AM +0100, Adam Borowski wrote:
> On Thu, Jan 19, 2017 at 05:33:14PM +0100, Greg KH wrote:
> > On Thu, Jan 19, 2017 at 05:12:15PM +0100, Manuel Schölling wrote:
> > > On Thu, 2017-01-19 at 14:23 +0100, Greg KH wrote:
> > > > On Fri, Jan 13, 2017 at 09:07:57PM +0100, Manuel Schölling wrote:
> > > > > +   This feature might break your tool of choice to flush
> > > > > the scrollback
> > > > > +   buffer, e.g. clear(1) will work fine but Debian's
> > > > > clear_console(1)
> > > > > +   will be broken, which might cause security issues.
> > > > > +   You can use the escape sequence \e[3J instead if this
> > > > > feature is
> > > > > +   activated.
> > 
> > I'd recommend that patch get to clear_console() first, having it use the
> > new escape sequence, if it isn't supported, shouldn't cause any
> > problems, right?
> 
> doko: would you consider, pretty please with a cherry on top, applying the
> patch I've sent to this bug?  The privacy/security issue is pretty minor and
> applies only to a tiny fraction of users, but I understand why Greg is
> reluctant.

# Subject: Bug#845177 closed by Matthias Klose <d...@debian.org>
#
# This is an automatic notification regarding your Bug report
# which was filed against the bash package:
#
# #845177: clear_console: assumes VT switch clears scrollback
#
# It has been closed by Matthias Klose <d...@debian.org>.
[...]
# Changes:
#* clear_console: Securely erase the current console. Closes: #845177.

-- 
Autotools hint: to do a zx-spectrum build on a pdp11 host, type:
  ./configure --host=zx-spectrum --build=pdp11

Re: [PATCH v10 3/4] console: Add persistent scrollback buffers for all VGA consoles

[Adam Borowski]

On Fri, Jan 20, 2017 at 12:04:12AM +0100, Adam Borowski wrote:
> On Thu, Jan 19, 2017 at 05:33:14PM +0100, Greg KH wrote:
> > On Thu, Jan 19, 2017 at 05:12:15PM +0100, Manuel Schölling wrote:
> > > On Thu, 2017-01-19 at 14:23 +0100, Greg KH wrote:
> > > > On Fri, Jan 13, 2017 at 09:07:57PM +0100, Manuel Schölling wrote:
> > > > > +   This feature might break your tool of choice to flush
> > > > > the scrollback
> > > > > +   buffer, e.g. clear(1) will work fine but Debian's
> > > > > clear_console(1)
> > > > > +   will be broken, which might cause security issues.
> > > > > +   You can use the escape sequence \e[3J instead if this
> > > > > feature is
> > > > > +   activated.
> > 
> > I'd recommend that patch get to clear_console() first, having it use the
> > new escape sequence, if it isn't supported, shouldn't cause any
> > problems, right?
> 
> doko: would you consider, pretty please with a cherry on top, applying the
> patch I've sent to this bug?  The privacy/security issue is pretty minor and
> applies only to a tiny fraction of users, but I understand why Greg is
> reluctant.

# Subject: Bug#845177 closed by Matthias Klose 
#
# This is an automatic notification regarding your Bug report
# which was filed against the bash package:
#
# #845177: clear_console: assumes VT switch clears scrollback
#
# It has been closed by Matthias Klose .
[...]
# Changes:
#* clear_console: Securely erase the current console. Closes: #845177.

-- 
Autotools hint: to do a zx-spectrum build on a pdp11 host, type:
  ./configure --host=zx-spectrum --build=pdp11

Re: [PATCH v10 3/4] console: Add persistent scrollback buffers for all VGA consoles

[Adam Borowski]

On Thu, Jan 19, 2017 at 05:33:14PM +0100, Greg KH wrote:
> On Thu, Jan 19, 2017 at 05:12:15PM +0100, Manuel Schölling wrote:
> > On Thu, 2017-01-19 at 14:23 +0100, Greg KH wrote:
> > > On Fri, Jan 13, 2017 at 09:07:57PM +0100, Manuel Schölling wrote:
> > > > +     This feature might break your tool of choice to flush
> > > > the scrollback
> > > > +     buffer, e.g. clear(1) will work fine but Debian's
> > > > clear_console(1)
> > > > +     will be broken, which might cause security issues.
> > > > +     You can use the escape sequence \e[3J instead if this
> > > > feature is
> > > > +     activated.
> > > 
> > > This issue is the one that makes me the most worried.  Why doesn't
> > > clear_console() work anymore?  Why doesn't it use \e[3J ?
> > 
> > Well, clear_console() just switches from one console to another and
> > back again. It just assumes that the scrollback buffer is flushed when
> > switching.
> > My plan is to make a patch for clear_console() as soon as these patches
> > are in the kernel - it's chicken-and-egg problem.
> 
> I'd recommend that patch get to clear_console() first, having it use the
> new escape sequence, if it isn't supported, shouldn't cause any
> problems, right?

In that case, we need to hurry -- the last day for any non-serious fixes in
Debian is Jan 26, after that it'll be frozen for months, and any subsequent
changes won't get to stable users for around two years.

doko: would you consider, pretty please with a cherry on top, applying the
patch I've sent to this bug?  The privacy/security issue is pretty minor and
applies only to a tiny fraction of users, but I understand why Greg is
reluctant.

Manuel's scrollback changes won't go to 4.9, and won't be enabled by default
for the time being, but using a newer kernel on old userspace is something
really widespread, be it via bpo, containers on an updated host, etc.


Meow!
-- 
Autotools hint: to do a zx-spectrum build on a pdp11 host, type:
  ./configure --host=zx-spectrum --build=pdp11

Re: [PATCH v10 3/4] console: Add persistent scrollback buffers for all VGA consoles

[Adam Borowski]

On Thu, Jan 19, 2017 at 05:33:14PM +0100, Greg KH wrote:
> On Thu, Jan 19, 2017 at 05:12:15PM +0100, Manuel Schölling wrote:
> > On Thu, 2017-01-19 at 14:23 +0100, Greg KH wrote:
> > > On Fri, Jan 13, 2017 at 09:07:57PM +0100, Manuel Schölling wrote:
> > > > +     This feature might break your tool of choice to flush
> > > > the scrollback
> > > > +     buffer, e.g. clear(1) will work fine but Debian's
> > > > clear_console(1)
> > > > +     will be broken, which might cause security issues.
> > > > +     You can use the escape sequence \e[3J instead if this
> > > > feature is
> > > > +     activated.
> > > 
> > > This issue is the one that makes me the most worried.  Why doesn't
> > > clear_console() work anymore?  Why doesn't it use \e[3J ?
> > 
> > Well, clear_console() just switches from one console to another and
> > back again. It just assumes that the scrollback buffer is flushed when
> > switching.
> > My plan is to make a patch for clear_console() as soon as these patches
> > are in the kernel - it's chicken-and-egg problem.
> 
> I'd recommend that patch get to clear_console() first, having it use the
> new escape sequence, if it isn't supported, shouldn't cause any
> problems, right?

In that case, we need to hurry -- the last day for any non-serious fixes in
Debian is Jan 26, after that it'll be frozen for months, and any subsequent
changes won't get to stable users for around two years.

doko: would you consider, pretty please with a cherry on top, applying the
patch I've sent to this bug?  The privacy/security issue is pretty minor and
applies only to a tiny fraction of users, but I understand why Greg is
reluctant.

Manuel's scrollback changes won't go to 4.9, and won't be enabled by default
for the time being, but using a newer kernel on old userspace is something
really widespread, be it via bpo, containers on an updated host, etc.


Meow!
-- 
Autotools hint: to do a zx-spectrum build on a pdp11 host, type:
  ./configure --host=zx-spectrum --build=pdp11

Re: [PATCH v10 3/4] console: Add persistent scrollback buffers for all VGA consoles

[Adam Borowski]

On Thu, Jan 19, 2017 at 05:12:15PM +0100, Manuel Schölling wrote:
> On Thu, 2017-01-19 at 14:23 +0100, Greg KH wrote:
> > On Fri, Jan 13, 2017 at 09:07:57PM +0100, Manuel Schölling wrote:
> > > Add a scrollback buffers for each VGA console.  The benefit is that
> > > the scrollback history is not flushed when switching between consoles
> > > but is persistent.  The buffers are allocated on demand when a new
> > > console is opened.
> > > 
> > > This breaks tools like clear_console that rely on flushing the
> > > scrollback history by switching back and forth between consoles
> > > which is why this feature is disabled by default.
> > > Use the escape sequence \e[3J instead for flushing the buffer.
> > 
> > This issue is the one that makes me the most worried.  Why doesn't
> > clear_console() work anymore?  Why doesn't it use \e[3J ?
> 
> Well, clear_console() just switches from one console to another and
> back again. It just assumes that the scrollback buffer is flushed when
> switching.
> My plan is to make a patch for clear_console() as soon as these patches
> are in the kernel - it's chicken-and-egg problem.

No need to wait, \e[3J is supported since Linux 2.6.39; the problem I
spotted was that a previous version of your patch would break that.

It is also safe to output that sequence to a terminal unconditionally: I've
tested a number of terminals, they all either support it (most X terminals,
our console) or silently ignore it.

We can't, though, rely on terminfo to do so: it knows about this capability
(which it calls "E3") for TERM=linux only since very recently.  The TERM
variable is also unreliable: it fails to carry over a serial link while
blindly printing \e[3J works.

As for patching clear_console, https://bugs.debian.org/845177 has a minimal
fix; although for all setups supported by Debian that program could be be
better replaced with just "printf '\e[3J\e[2J'" which would make it work on
strictly more terminals than current code does.  Same for distributions
which copied clear_console (it originates from Ubuntu, maintained in Debian
since then).


Meow!
-- 
Autotools hint: to do a zx-spectrum build on a pdp11 host, type:
  ./configure --host=zx-spectrum --build=pdp11