2022-12-25 22:18:29

by Linus Torvalds

[permalink] [raw]
Subject: Linux 6.2-rc1

So it's Christmas Day here, but it's also Sunday afternoon two weeks
after the 6.2 merge window opened. So holidays or not, the kernel
development show must go on.

Thanks to a lot of people sending their pull requests early, I got
much of the merge window work done before the holidays started in
earnest, and mostly before my pre-xmas travel. So despite flight
delays, missed connections, and the resulting airport hotel
excursions, the merge window mostly went smoothly, and there was no
reason to delay rc1.

That said, realistically I expect most people to be on vacation for at
least another week, so I wouldn't be surprised if we end up with a
delayed final release due to the season. But it's too early to worry
about that yet, we'll just have to see how it goes.

Also, 6.2 looks like it's a bigger release (certainly bigger than 6.1
was). The summary below is, as usual, just my merge log: we've got
about 13.5k commits from ~1800 people in total in this merge window,
which is actually not that far off the total size of the whole 6.1
release. But let's hope that despite the size, and despite the likely
slow start of the post-merge-window calming down period, we'll have a
smooth release.

And in the meantime, Merry Christmas and a Happy New Year to all
(replace as appropriate with whatever holiday, if any, you are
celebrating).

Linus

---

Al Viro (5):
elf coredumping updates
alpha updates
iov_iter updates
namespace fix
misc vfs updates

Alex Williamson (1):
VFIO updates

Alexander Gordeev (1):
s390 updates

Alexandre Belloni (2):
i3c updates
RTC updates

Andreas Gruenbacher (1):
gfs2 updtaes

Andrew Morton (5):
non-MM updates
MM updates
more mm updates
fault-injection updates
hotfixes

Ard Biesheuvel (1):
EFI updates

Arnaldo Carvalho de Melo (2):
perf tools updates
more perf tools updates

Arnd Bergmann (6):
ARM SoC defconfig updates
ARM SoC code updates
ARM SoC driver updates
ARM SoC DT updates
ARM SoC fixes
asm-generic updates

Bartosz Golaszewski (1):
gpio updates

Benjamin Tissoires (1):
HID updates

Bjorn Andersson (1):
remoteproc updates

Bjorn Helgaas (1):
PCI updates

Borislav Petkov (10):
EDAC updates
x86 RAS updates
x86 alternative update
x86 asm updates
x86 boot updates
x86 cpu updates
x86 microcode and IFS updates
x86 paravirt update
x86 sev updates
x86 core updates

Christian Brauner (7):
VFS acl updates
setgid inheritance updates
vfsuid updates
idmapping updates
simple-xattr updates
vfsuid cleanup
mount propagation fix

Christoph Hellwig (3):
configfs updates
dma-mapping updates
dma-mapping fixes

Chuck Lever (2):
nfsd updates
more nfsd updates

Corey Minyard (1):
IPMI updates

Damien Le Moal (1):
ata updates

Dan Williams (1):
cxl updates

Darrick Wong (3):
vfs remap_range update
iomap update
XFS updates

Dave Airlie (2):
drm updates
drm fixes

Dave Hansen (6):
x86 sgx updates
x86 tdx updates
x86 cache resource control updates
x86 splitlock updates
x86 fpu updates
x86 mm updates

David Howells (1):
afs update

David Kleikamp (1):
jfs updates

David Sterba (1):
btrfs updates

David Teigland (1):
dlm updates

Dennis Zhou (1):
percpu updates

Dmitry Torokhov (1):
input updates

Dominique Martinet (1):
9p updates

Eric Biggers (2):
fscrypt updates
fsverity updates

Gao Xiang (1):
erofs updates

Geert Uytterhoeven (1):
m68k updates

Greg KH (6):
USB and Thunderbolt driver updates
staging driver updates
tty/serial driver updates
char/misc driver updates
driver core updates
SPDX/License additions

Greg Ungerer (1):
m68knommu update

Guenter Roeck (1):
hwmon updates

Guo Ren (1):
arch/csky updates

Hans de Goede (1):
x86 platform driver updates

Helge Deller (2):
fbdev updates
parisc updates

Herbert Xu (1):
crypto updates

Huacai Chen (1):
LoongArch updates

Ilya Dryomov (1):
cph update

Ingo Molnar (3):
locking updates
perf events updates
scheduler updates

Jaegeuk Kim (1):
f2fs updates

Jakub Kicinski (1):
networking fixes

James Bottomley (2):
SCSI updates
more SCSI updates

Jan Kara (1):
udf and ext2 fixes

Jarkko Sakkinen (1):
tpm updates

Jason Donenfeld (3):
unsigned-char conversion
random number generator updates
more random number generator updates

Jason Gunthorpe (3):
iommufd implementation
rdma updates
rdma fixes

Jassi Brar (1):
mailbox updates

Jeff Layton (1):
file locking updates

Jens Axboe (6):
io_uring updates
io_uring updates part two
block updates
writeback updates
io_uring fixes
block fixes

Jiri Kosina (1):
HID updates

Joerg Roedel (1):
iommu updates

John Johansen (1):
apparmor updates

Jonathan Corbet (1):
documentation updates

Juergen Gross (1):
xen updates

Julia Lawall (1):
coccicheck update

Kees Cook (6):
pstore updates
seccomp updates
execve updates
kernel hardening updates
pstore fixes
kernel hardening fixes

Konstantin Komarov (1):
ntfs3 updates

Lee Jones (2):
MFD updates
backlight update

Linus Walleij (1):
pin control updates

Luis Chamberlain (2):
modules updates
sysctl updates

Marc Zyngier (1):
MSI fixes

Mark Brown (5):
regmap updates
regulator updates
spi updates
regulator fixes
spi fix

Masahiro Yamada (1):
Kbuild updates

Mauro Carvalho Chehab (2):
media updates
media fixes

Max Filippov (1):
Xtensa updates

Michael Ellerman (1):
powerpc updates

Michal Simek (1):
microblaze updates

Mickaël Salaün (1):
landlock updates

Miguel Ojeda (1):
rust updates

Mike Marshall (1):
orangefs updates

Mike Rapoport (1):
memblock updates

Mike Snitzer (1):
device mapper updates

Miklos Szeredi (2):
overlayfs update
fuse update

Mimi Zohar (1):
integrity updates

Miquel Raynal (1):
mtd updates

Namjae Jeon (1):
exfat update

Nick Terrell (1):
zstd updates

Palmer Dabbelt (1):
RISC-V updates

Paolo Abeni (1):
networking updates

Paolo Bonzini (2):
kvm updates
RISC-V kvm updates

Paul McKenney (5):
RCU updates
kernel memory model documentation updates
KCSAN updates
nolibc updates
RCU fix

Paul Moore (3):
audit updates
selinux updates
lsm updates

Pavel Machek (1):
LED updates

Petr Mladek (2):
printk updates
livepatching update

Rafael Wysocki (5):
power management updates
ACPI and PNP updates
thermal control updates
more thermal control updates
more ACPI updates

Rob Herring (2):
devicetree updates
more devicetree updates

Russell King (1):
ARM updates

Sebastian Reichel (2):
power supply and reset updates
HSI updates

Seth Forshee (2):
squashfs update
xattr audit fix

Shuah Khan (2):
Kselftest updates
KUnit updates

Stephen Boyd (1):
clk driver updates

Steve French (3):
ksmbd updates
cifs client updates
cifs fixes

Steven Rostedt (5):
ktest updates
tracing tools updates
tracing updates
trace probes updates
tracing fix

Takashi Iwai (2):
sound updates
more sound updates

Ted Ts'o (1):
ext4 updates

Tejun Heo (1):
cgroup updates

Thierry Reding (1):
pwm updates

Thomas Bogendoerfer (2):
MIPS updates
MIPS fixes

Thomas Gleixner (8):
x86 fixes
debugobjects update
irq updates
CPU hotplug updates
x86 apic update
x86 cleanups
timer updates
misc x86 updates

Trond Myklebust (1):
NFS client updates

Tzung-Bi Shih (1):
chrome platform updates

Ulf Hansson (1):
MMC and MEMSTICK updates

Vinod Koul (3):
phy updates
soundwire updates
dmaengine updates

Vlastimil Babka (1):
slab updates

Wei Liu (1):
hyperv updates

Will Deacon (2):
arm64 updates
arm64 fixes

Wim Van Sebroeck (1):
watchdog updates

Wolfram Sang (1):
i2c updates


2022-12-26 20:35:38

by Guenter Roeck

[permalink] [raw]
Subject: Re: Linux 6.2-rc1

On Sun, Dec 25, 2022 at 02:07:05PM -0800, Linus Torvalds wrote:
> So it's Christmas Day here, but it's also Sunday afternoon two weeks
> after the 6.2 merge window opened. So holidays or not, the kernel
> development show must go on.
>
> Thanks to a lot of people sending their pull requests early, I got
> much of the merge window work done before the holidays started in
> earnest, and mostly before my pre-xmas travel. So despite flight
> delays, missed connections, and the resulting airport hotel
> excursions, the merge window mostly went smoothly, and there was no
> reason to delay rc1.
>
> That said, realistically I expect most people to be on vacation for at
> least another week, so I wouldn't be surprised if we end up with a
> delayed final release due to the season. But it's too early to worry
> about that yet, we'll just have to see how it goes.
>
> Also, 6.2 looks like it's a bigger release (certainly bigger than 6.1
> was). The summary below is, as usual, just my merge log: we've got
> about 13.5k commits from ~1800 people in total in this merge window,
> which is actually not that far off the total size of the whole 6.1
> release. But let's hope that despite the size, and despite the likely
> slow start of the post-merge-window calming down period, we'll have a
> smooth release.
>

Test results:

Build results:
total: 155 pass: 151 fail: 4
Failed builds:
powerpc:allmodconfig
sh:defconfig
sh:shx3_defconfig
xtensa:allmodconfig
Qemu test results:
total: 500 pass: 498 fail: 2
Failed tests:
arm:xilinx-zynq-a9:multi_v7_defconfig:usb0:mem128:net,default:zynq-zc702:rootfs
arm:xilinx-zynq-a9:multi_v7_defconfig:usb0:mem128:zynq-zed:rootfs

Details below.

Guenter

---
Build errors
============

Building powerpc:allmodconfig ... failed
--------------
Error log:
In file included from include/linux/string.h:253,
from arch/powerpc/include/asm/paca.h:16,
from arch/powerpc/include/asm/current.h:13,
from include/linux/thread_info.h:23,
from include/asm-generic/preempt.h:5,
from ./arch/powerpc/include/generated/asm/preempt.h:1,
from include/linux/preempt.h:78,
from include/linux/spinlock.h:56,
from include/linux/wait.h:9,
from include/linux/wait_bit.h:8,
from include/linux/fs.h:6,
from fs/f2fs/inline.c:9:
fs/f2fs/inline.c: In function 'f2fs_move_inline_dirents':
include/linux/fortify-string.h:59:33: error: '__builtin_memset' pointer overflow between offset [28, 898293814] and size [-898293787, -1] [-Werror=array-bounds]
59 | #define __underlying_memset __builtin_memset
| ^
include/linux/fortify-string.h:337:9: note: in expansion of macro '__underlying_memset'
337 | __underlying_memset(p, c, __fortify_size); \
| ^~~~~~~~~~~~~~~~~~~
include/linux/fortify-string.h:345:25: note: in expansion of macro '__fortify_memset_chk'
345 | #define memset(p, c, s) __fortify_memset_chk(p, c, s, \
| ^~~~~~~~~~~~~~~~~~~~
fs/f2fs/inline.c:430:9: note: in expansion of macro 'memset'
430 | memset(dst.bitmap + src.nr_bitmap, 0, dst.nr_bitmap - src.nr_bitmap);
| ^~~~~~
cc1: all warnings being treated as errors

xtensa:allmodconfig

Building xtensa:allmodconfig ... failed
--------------
Error log:
kernel/kcsan/kcsan_test.c: In function '__report_matches':
kernel/kcsan/kcsan_test.c:257:1: error: the frame size of 1680 bytes is larger than 1536 bytes

Bisect for both points to commit e240e53ae0abb08 ("mm, slub: add
CONFIG_SLUB_TINY"). Reverting it on its own is not possible, but
reverting the following two patches fixes the problem.

149b6fa228ed mm, slob: rename CONFIG_SLOB to CONFIG_SLOB_DEPRECATED
e240e53ae0ab mm, slub: add CONFIG_SLUB_TINY

Context: CONFIG_SLUB_TINY is enabled with allmodconfig builds.
This enables some previously disabled configurations and disables
some previously enabled configurations. Not sure if that is a good
thing or a bad thing, but it does result in the above errors.

---
sh:defconfig
sh:shx3_defconfig

Building sh:defconfig ... failed
--------------
Error log:
In file included from <command-line>:
In function 'follow_pmd_mask',
inlined from 'follow_pud_mask' at mm/gup.c:735:9,
inlined from 'follow_p4d_mask' at mm/gup.c:752:9,
inlined from 'follow_page_mask' at mm/gup.c:809:9:
include/linux/compiler_types.h:358:45: error: call to '__compiletime_assert_263' declared with attribute error: Unsupported access size for {READ,WRITE}_ONCE().
358 | _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)

Bisect points to commit 0862ff059c9e ("sh/mm: Make pmd_t similar to pte_t").
This commit introduces

-typedef struct { unsigned long long pmd; } pmd_t;
+typedef struct {
+ struct {
+ unsigned long pmd_low;
+ unsigned long pmd_high;
+ };
+ unsigned long long pmd;
+} pmd_t;

That should probably be "typedef union", not "typedef struct".

=============

Runtime:

Boot tests of arm:xilinx-zynq-a9 fail after

[ 5.849451] ci_hdrc ci_hdrc.0: failed to register ULPI interface
[ 5.849577] ci_hdrc: probe of ci_hdrc.0 failed with error -110

Caused by commit 8a7b31d545d3 ("usb: ulpi: defer ulpi_register on
ulpi_read_id timeout"). Revert is pending.

---

Not exactly a regression, but worth mentioning:

CONFIG_MEMCPY_KUNIT_TEST now sometimes takes several minutes to
execute in qemu. On top of that, it may result in hung task timeouts
if the hung task timeout is set to low values (45 seconds and below).
Example, seen with s390:

...
[ 18.494320] ok 2 memcpy_test
[ 52.969037] ok 3 memcpy_large_test
...
[ 52.974505] ok 4 memmove_test
[ 87.325400] ok 5 memmove_large_test
[ 143.562760] INFO: task swapper/0:1 blocked for more than 46 seconds.
...
[ 143.564441] Call Trace:
[ 143.564689] [<0000000000f1ec80>] __schedule+0x370/0x720
[ 143.565175] [<0000000000f1f098>] schedule+0x68/0x110
[ 143.565374] [<0000000000f278d4>] schedule_timeout+0xc4/0x160
[ 143.565603] [<0000000000f1fde2>] __wait_for_common+0xda/0x250
[ 143.565816] [<0000000000903c90>] kunit_try_catch_run+0x98/0x178
[ 143.566029] [<0000000000f05c9c>] kunit_run_case_catch_errors+0x7c/0xb8
[ 143.566956] [<00000000009023c0>] kunit_run_tests+0x220/0x638
...

That is too much for my test bed. I dropped this test as result. This means
that extending the tests has, at least in the context of my testing, the
opposite effect.

2022-12-26 21:06:40

by Linus Torvalds

[permalink] [raw]
Subject: Re: Linux 6.2-rc1

On Mon, Dec 26, 2022 at 11:52 AM Guenter Roeck <[email protected]> wrote:
>
> fs/f2fs/inline.c: In function 'f2fs_move_inline_dirents':
> include/linux/fortify-string.h:59:33: error: '__builtin_memset' pointer overflow between offset [28, 898293814] and size [-898293787, -1] [-Werror=array-bounds]
> fs/f2fs/inline.c:430:9: note: in expansion of macro 'memset'
> 430 | memset(dst.bitmap + src.nr_bitmap, 0, dst.nr_bitmap - src.nr_bitmap);
> | ^~~~~~

Well, that's unfortunate.

> kernel/kcsan/kcsan_test.c: In function '__report_matches':
> kernel/kcsan/kcsan_test.c:257:1: error: the frame size of 1680 bytes is larger than 1536 bytes
>
> Bisect for both points to commit e240e53ae0abb08 ("mm, slub: add
> CONFIG_SLUB_TINY"). Reverting it on its own is not possible, but
> reverting the following two patches fixes the problem.
>
> 149b6fa228ed mm, slob: rename CONFIG_SLOB to CONFIG_SLOB_DEPRECATED
> e240e53ae0ab mm, slub: add CONFIG_SLUB_TINY

No, I think CONFIG_SLUB_TINY should probably have a

depends on !COMPILE_TEST

or something like that instead.

It already has a

depends on SLUB && EXPERT

which is basically supposed to disable it for any normal builds, but
obviously allmodconfig will enable EXPERT etc anyway.

That said, that f2fs case also sounds like this code triggers the
compiler being unhappy, so it might be worth having some clarification
from the f2fs people.

I'm not sure what triggers that problem just on powerpc, and only with
that CONFIG_SLUB_TINY option. Maybe those make_dentry_ptr_inline() and
make_dentry_ptr_block() functions don't get inlined in that case, and
that then makes gcc not see the values for those bitmap sizes?

Does changing the "inline" to "always_inline" perhaps fix the compiler
unpahhiness too?

> sh:defconfig
> sh:shx3_defconfig
>
> Building sh:defconfig ... failed
> --------------
> Error log:
> In file included from <command-line>:
> In function 'follow_pmd_mask',
> inlined from 'follow_pud_mask' at mm/gup.c:735:9,
> inlined from 'follow_p4d_mask' at mm/gup.c:752:9,
> inlined from 'follow_page_mask' at mm/gup.c:809:9:
> include/linux/compiler_types.h:358:45: error: call to '__compiletime_assert_263' declared with attribute error: Unsupported access size for {READ,WRITE}_ONCE().
> 358 | _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
>
> Bisect points to commit 0862ff059c9e ("sh/mm: Make pmd_t similar to pte_t").
> This commit introduces
>
> -typedef struct { unsigned long long pmd; } pmd_t;
> +typedef struct {
> + struct {
> + unsigned long pmd_low;
> + unsigned long pmd_high;
> + };
> + unsigned long long pmd;
> +} pmd_t;
>
> That should probably be "typedef union", not "typedef struct".

Yeah. PeterZ?

> Boot tests of arm:xilinx-zynq-a9 fail after
>
> [ 5.849451] ci_hdrc ci_hdrc.0: failed to register ULPI interface
> [ 5.849577] ci_hdrc: probe of ci_hdrc.0 failed with error -110
>
> Caused by commit 8a7b31d545d3 ("usb: ulpi: defer ulpi_register on
> ulpi_read_id timeout"). Revert is pending.

Good.

> Not exactly a regression, but worth mentioning:
>
> CONFIG_MEMCPY_KUNIT_TEST now sometimes takes several minutes to
> execute in qemu. On top of that, it may result in hung task timeouts
> if the hung task timeout is set to low values (45 seconds and below).
> Example, seen with s390:
>
> ...
> [ 18.494320] ok 2 memcpy_test
> [ 52.969037] ok 3 memcpy_large_test
> ...
> [ 52.974505] ok 4 memmove_test
> [ 87.325400] ok 5 memmove_large_test
> [ 143.562760] INFO: task swapper/0:1 blocked for more than 46 seconds.
> ...
> [ 143.564441] Call Trace:
> [ 143.564689] [<0000000000f1ec80>] __schedule+0x370/0x720
> [ 143.565175] [<0000000000f1f098>] schedule+0x68/0x110
> [ 143.565374] [<0000000000f278d4>] schedule_timeout+0xc4/0x160
> [ 143.565603] [<0000000000f1fde2>] __wait_for_common+0xda/0x250
> [ 143.565816] [<0000000000903c90>] kunit_try_catch_run+0x98/0x178
> [ 143.566029] [<0000000000f05c9c>] kunit_run_case_catch_errors+0x7c/0xb8
> [ 143.566956] [<00000000009023c0>] kunit_run_tests+0x220/0x638
> ...
>
> That is too much for my test bed. I dropped this test as result. This means
> that extending the tests has, at least in the context of my testing, the
> opposite effect.

Kees? This indeed seems counter-productive..

Linus

2022-12-26 22:03:23

by Max Filippov

[permalink] [raw]
Subject: Re: Linux 6.2-rc1

On Mon, Dec 26, 2022 at 12:44 PM Guenter Roeck <[email protected]> wrote:
> xtensa:allmodconfig
>
> Building xtensa:allmodconfig ... failed
> --------------
> Error log:
> kernel/kcsan/kcsan_test.c: In function '__report_matches':
> kernel/kcsan/kcsan_test.c:257:1: error: the frame size of 1680 bytes is larger than 1536 bytes
>
> Bisect for both points to commit e240e53ae0abb08 ("mm, slub: add
> CONFIG_SLUB_TINY"). Reverting it on its own is not possible, but
> reverting the following two patches fixes the problem.
>
> 149b6fa228ed mm, slob: rename CONFIG_SLOB to CONFIG_SLOB_DEPRECATED
> e240e53ae0ab mm, slub: add CONFIG_SLUB_TINY
>
> Context: CONFIG_SLUB_TINY is enabled with allmodconfig builds.
> This enables some previously disabled configurations and disables
> some previously enabled configurations. Not sure if that is a good
> thing or a bad thing, but it does result in the above errors.

For xtensa I've posted the following fix:
https://lore.kernel.org/lkml/[email protected]/
I suspect that previously KCSAN was disabled in allmodconfig builds for xtensa.

--
Thanks.
-- Max

2022-12-26 22:03:27

by Kees Cook

[permalink] [raw]
Subject: Re: Linux 6.2-rc1

On December 26, 2022 12:56:29 PM PST, Linus Torvalds <[email protected]> wrote:
>On Mon, Dec 26, 2022 at 11:52 AM Guenter Roeck <[email protected]> wrote:
>>
>> fs/f2fs/inline.c: In function 'f2fs_move_inline_dirents':
>> include/linux/fortify-string.h:59:33: error: '__builtin_memset' pointer overflow between offset [28, 898293814] and size [-898293787, -1] [-Werror=array-bounds]
>> fs/f2fs/inline.c:430:9: note: in expansion of macro 'memset'
>> 430 | memset(dst.bitmap + src.nr_bitmap, 0, dst.nr_bitmap - src.nr_bitmap);
>> | ^~~~~~
>
>Well, that's unfortunate.

I'll look into this.

> [...]
>> Not exactly a regression, but worth mentioning:
>>
>> CONFIG_MEMCPY_KUNIT_TEST now sometimes takes several minutes to
>> execute in qemu. On top of that, it may result in hung task timeouts
>> if the hung task timeout is set to low values (45 seconds and below).
>> Example, seen with s390:
>>
>> ...
>> [ 18.494320] ok 2 memcpy_test
>> [ 52.969037] ok 3 memcpy_large_test
>> ...
>> [ 52.974505] ok 4 memmove_test
>> [ 87.325400] ok 5 memmove_large_test
>> [ 143.562760] INFO: task swapper/0:1 blocked for more than 46 seconds.
>> ...
>> [ 143.564441] Call Trace:
>> [ 143.564689] [<0000000000f1ec80>] __schedule+0x370/0x720
>> [ 143.565175] [<0000000000f1f098>] schedule+0x68/0x110
>> [ 143.565374] [<0000000000f278d4>] schedule_timeout+0xc4/0x160
>> [ 143.565603] [<0000000000f1fde2>] __wait_for_common+0xda/0x250
>> [ 143.565816] [<0000000000903c90>] kunit_try_catch_run+0x98/0x178
>> [ 143.566029] [<0000000000f05c9c>] kunit_run_case_catch_errors+0x7c/0xb8
>> [ 143.566956] [<00000000009023c0>] kunit_run_tests+0x220/0x638
>> ...
>>
>> That is too much for my test bed. I dropped this test as result. This means
>> that extending the tests has, at least in the context of my testing, the
>> opposite effect.
>
>Kees? This indeed seems counter-productive..

Hrm, that is not supposed to take THAT long... But yes a cross-arxh qemu run would be slow. :(

The changes there were to help find any future memcpy problem (like seen while porting the i386 memcpy asm...) I'll try to get this reduced. Dropping the test isn't so great. :)



--
Kees Cook

2022-12-26 22:26:43

by Guenter Roeck

[permalink] [raw]
Subject: Re: Linux 6.2-rc1

On Mon, Dec 26, 2022 at 01:10:40PM -0800, Max Filippov wrote:
> On Mon, Dec 26, 2022 at 12:44 PM Guenter Roeck <[email protected]> wrote:
> > xtensa:allmodconfig
> >
> > Building xtensa:allmodconfig ... failed
> > --------------
> > Error log:
> > kernel/kcsan/kcsan_test.c: In function '__report_matches':
> > kernel/kcsan/kcsan_test.c:257:1: error: the frame size of 1680 bytes is larger than 1536 bytes
> >
> > Bisect for both points to commit e240e53ae0abb08 ("mm, slub: add
> > CONFIG_SLUB_TINY"). Reverting it on its own is not possible, but
> > reverting the following two patches fixes the problem.
> >
> > 149b6fa228ed mm, slob: rename CONFIG_SLOB to CONFIG_SLOB_DEPRECATED
> > e240e53ae0ab mm, slub: add CONFIG_SLUB_TINY
> >
> > Context: CONFIG_SLUB_TINY is enabled with allmodconfig builds.
> > This enables some previously disabled configurations and disables
> > some previously enabled configurations. Not sure if that is a good
> > thing or a bad thing, but it does result in the above errors.
>
> For xtensa I've posted the following fix:
> https://lore.kernel.org/lkml/[email protected]/
> I suspect that previously KCSAN was disabled in allmodconfig builds for xtensa.
>

Correect.

Thanks,
Guenter

2022-12-26 22:51:48

by Guenter Roeck

[permalink] [raw]
Subject: Re: Linux 6.2-rc1

On Mon, Dec 26, 2022 at 01:03:59PM -0800, Kees Cook wrote:
> >>
> >> ...
> >> [ 18.494320] ok 2 memcpy_test
> >> [ 52.969037] ok 3 memcpy_large_test
> >> ...
> >> [ 52.974505] ok 4 memmove_test
> >> [ 87.325400] ok 5 memmove_large_test
> >> [ 143.562760] INFO: task swapper/0:1 blocked for more than 46 seconds.
[ ... ]
> >>
> >> That is too much for my test bed. I dropped this test as result. This means
> >> that extending the tests has, at least in the context of my testing, the
> >> opposite effect.
> >
> >Kees? This indeed seems counter-productive..
>
> Hrm, that is not supposed to take THAT long... But yes a cross-arxh qemu run would be slow. :(
>
> The changes there were to help find any future memcpy problem (like seen while porting the i386 memcpy asm...) I'll try to get this reduced. Dropping the test isn't so great. :)
>

Would it be possible to hide the expensive tests behind an extra Kconfig
option ?

Thanks,
Guenter

2022-12-26 22:52:34

by Vlastimil Babka

[permalink] [raw]
Subject: Re: Linux 6.2-rc1

On 12/26/22 21:56, Linus Torvalds wrote:
> On Mon, Dec 26, 2022 at 11:52 AM Guenter Roeck <[email protected]> wrote:
>>
>> fs/f2fs/inline.c: In function 'f2fs_move_inline_dirents':
>> include/linux/fortify-string.h:59:33: error: '__builtin_memset' pointer overflow between offset [28, 898293814] and size [-898293787, -1] [-Werror=array-bounds]
>> fs/f2fs/inline.c:430:9: note: in expansion of macro 'memset'
>> 430 | memset(dst.bitmap + src.nr_bitmap, 0, dst.nr_bitmap - src.nr_bitmap);
>> | ^~~~~~
>
> Well, that's unfortunate.
>
>> kernel/kcsan/kcsan_test.c: In function '__report_matches':
>> kernel/kcsan/kcsan_test.c:257:1: error: the frame size of 1680 bytes is larger than 1536 bytes
>>
>> Bisect for both points to commit e240e53ae0abb08 ("mm, slub: add
>> CONFIG_SLUB_TINY"). Reverting it on its own is not possible, but
>> reverting the following two patches fixes the problem.
>>
>> 149b6fa228ed mm, slob: rename CONFIG_SLOB to CONFIG_SLOB_DEPRECATED
>> e240e53ae0ab mm, slub: add CONFIG_SLUB_TINY
>
> No, I think CONFIG_SLUB_TINY should probably have a
>
> depends on !COMPILE_TEST
>
> or something like that instead.

We can do that, although if things are on track to be fixed, maybe it's
unnecessary?

> It already has a
>
> depends on SLUB && EXPERT
>
> which is basically supposed to disable it for any normal builds, but
> obviously allmodconfig will enable EXPERT etc anyway.
>
> That said, that f2fs case also sounds like this code triggers the
> compiler being unhappy, so it might be worth having some clarification
> from the f2fs people.
>
> I'm not sure what triggers that problem just on powerpc, and only with
> that CONFIG_SLUB_TINY option. Maybe those make_dentry_ptr_inline() and

I think it's because e240e53ae0ab makes KASAN depend on !SLUB_TINY, because
KASAN does "select SLUB_DEBUG" which depends on !SLUB_TINY; but kconfig will
still honor the select even with dependencies unmet and only warn about it
(and the build would fail) so I prevented it this way. (maybe instead
SLUB_TINY depend on !KASAN would have worked better in retrospect?) So now
allmodconfig will have SLUB_TINY enabled and KASAN thus disabled.

On the other hand there are configs like KCSAN and KMSAN that depend on
!KASAN, so with KASAN disabled, now those become enabled. KCSAN becoming
enabled would be relevant for the xtensa problem. For the powerpc issue I'm
not sure as the macro expansion lines for include/linux/fortify-string.h in
Guenter's report make no sense in my 6.2-rc1 checkout for some reason. But
the header does test for KASAN and KMSAN at several points, to perhaps it's
also related to that?

> make_dentry_ptr_block() functions don't get inlined in that case, and
> that then makes gcc not see the values for those bitmap sizes?
>
> Does changing the "inline" to "always_inline" perhaps fix the compiler
> unpahhiness too?
>

2022-12-27 01:05:45

by Guenter Roeck

[permalink] [raw]
Subject: Re: Linux 6.2-rc1

On Mon, Dec 26, 2022 at 01:03:59PM -0800, Kees Cook wrote:
> On December 26, 2022 12:56:29 PM PST, Linus Torvalds <[email protected]> wrote:
> >On Mon, Dec 26, 2022 at 11:52 AM Guenter Roeck <[email protected]> wrote:
> >>
> >> fs/f2fs/inline.c: In function 'f2fs_move_inline_dirents':
> >> include/linux/fortify-string.h:59:33: error: '__builtin_memset' pointer overflow between offset [28, 898293814] and size [-898293787, -1] [-Werror=array-bounds]
> >> fs/f2fs/inline.c:430:9: note: in expansion of macro 'memset'
> >> 430 | memset(dst.bitmap + src.nr_bitmap, 0, dst.nr_bitmap - src.nr_bitmap);
> >> | ^~~~~~
> >
> >Well, that's unfortunate.
>
> I'll look into this.
>

I did some more testing. The problem is seen with gcc 11.3.0, but not with
gcc 12.2.0 nor with gcc 10.3.0. gcc bug ? Should I switch to gcc 12.2.0 for
powerpc when build testing the latest kernel ?

Guenter

2022-12-27 01:36:02

by Kees Cook

[permalink] [raw]
Subject: Re: Linux 6.2-rc1

On December 26, 2022 4:29:41 PM PST, Guenter Roeck <[email protected]> wrote:
>On Mon, Dec 26, 2022 at 01:03:59PM -0800, Kees Cook wrote:
>> On December 26, 2022 12:56:29 PM PST, Linus Torvalds <[email protected]> wrote:
>> >On Mon, Dec 26, 2022 at 11:52 AM Guenter Roeck <[email protected]> wrote:
>> >>
>> >> fs/f2fs/inline.c: In function 'f2fs_move_inline_dirents':
>> >> include/linux/fortify-string.h:59:33: error: '__builtin_memset' pointer overflow between offset [28, 898293814] and size [-898293787, -1] [-Werror=array-bounds]
>> >> fs/f2fs/inline.c:430:9: note: in expansion of macro 'memset'
>> >> 430 | memset(dst.bitmap + src.nr_bitmap, 0, dst.nr_bitmap - src.nr_bitmap);
>> >> | ^~~~~~
>> >
>> >Well, that's unfortunate.
>>
>> I'll look into this.
>>
>
>I did some more testing. The problem is seen with gcc 11.3.0, but not with
>gcc 12.2.0 nor with gcc 10.3.0.

That's what I'd expect: 10 didn't have variable range tracking wired up to -Warray-bounds, 11 does, and we disable -Warray-bounds on 12 because of 3 separate 12-only GCC bugs.

> gcc bug ? Should I switch to gcc 12.2.0 for
>powerpc when build testing the latest kernel ?

Sure? But that'll just hide it. I suspect GCC has found a way for dst.nr_bitmap to be compile-time 27, so the size is always negative.


--
Kees Cook

2022-12-27 06:34:54

by Guenter Roeck

[permalink] [raw]
Subject: Re: Linux 6.2-rc1

On Mon, Dec 26, 2022 at 05:32:28PM -0800, Kees Cook wrote:
> On December 26, 2022 4:29:41 PM PST, Guenter Roeck <[email protected]> wrote:
> >On Mon, Dec 26, 2022 at 01:03:59PM -0800, Kees Cook wrote:
> >> On December 26, 2022 12:56:29 PM PST, Linus Torvalds <[email protected]> wrote:
> >> >On Mon, Dec 26, 2022 at 11:52 AM Guenter Roeck <[email protected]> wrote:
> >> >>
> >> >> fs/f2fs/inline.c: In function 'f2fs_move_inline_dirents':
> >> >> include/linux/fortify-string.h:59:33: error: '__builtin_memset' pointer overflow between offset [28, 898293814] and size [-898293787, -1] [-Werror=array-bounds]
> >> >> fs/f2fs/inline.c:430:9: note: in expansion of macro 'memset'
> >> >> 430 | memset(dst.bitmap + src.nr_bitmap, 0, dst.nr_bitmap - src.nr_bitmap);
> >> >> | ^~~~~~
> >> >
> >> >Well, that's unfortunate.
> >>
> >> I'll look into this.
> >>
> >
> >I did some more testing. The problem is seen with gcc 11.3.0, but not with
> >gcc 12.2.0 nor with gcc 10.3.0.
>
> That's what I'd expect: 10 didn't have variable range tracking wired up to -Warray-bounds, 11 does, and we disable -Warray-bounds on 12 because of 3 separate 12-only GCC bugs.
>
> > gcc bug ? Should I switch to gcc 12.2.0 for
> >powerpc when build testing the latest kernel ?
>
> Sure? But that'll just hide it. I suspect GCC has found a way for dst.nr_bitmap to be compile-time 27, so the size is always negative.
>
dst.nr_bitmap is initialized with SIZE_OF_DENTRY_BITMAP,
which is defined as:

#define NR_DENTRY_IN_BLOCK 214 /* the number of dentry in a block */
#define SIZE_OF_DIR_ENTRY 11 /* by byte */
#define SIZE_OF_DENTRY_BITMAP ((NR_DENTRY_IN_BLOCK + BITS_PER_BYTE - 1) / \
BITS_PER_BYTE)

((214 + 8 - 1) / 8 = 27, so dst.nr_bitmap is indeed compile-time 27.

Not sure how would know that src.nr_bitmap can be > 27, though.
Am I missing something ?

Thanks,
Guenter

2022-12-27 09:05:45

by Geert Uytterhoeven

[permalink] [raw]
Subject: Build regressions/improvements in v6.2-rc1

Below is the list of build error/warning regressions/improvements in
v6.2-rc1[1] compared to v6.1[2].

Summarized:
- build errors: +11/-13
- build warnings: +13/-10

Happy fixing! ;-)

Thanks to the linux-next team for providing the build service.

[1] http://kisskb.ellerman.id.au/kisskb/branch/linus/head/1b929c02afd37871d5afb9d498426f83432e71c2/ (all 152 configs)
[2] http://kisskb.ellerman.id.au/kisskb/branch/linus/head/830b3c68c1fb1e9176028d02ef86f3cf76aa2476/ (all 152 configs)


*** ERRORS ***

11 error regressions:
+ /kisskb/src/drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn31/display_mode_vba_31.c: error: the frame size of 2224 bytes is larger than 2048 bytes [-Werror=frame-larger-than=]: => 7082:1
+ /kisskb/src/drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn314/display_mode_vba_314.c: error: the frame size of 2208 bytes is larger than 2048 bytes [-Werror=frame-larger-than=]: => 7127:1
+ /kisskb/src/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c: error: array subscript 2 is above array bounds of 'u32[2]' {aka 'unsigned int[2]'} [-Werror=array-bounds]: => 641:28
+ /kisskb/src/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c: error: array subscript 3 is above array bounds of 'u32[2]' {aka 'unsigned int[2]'} [-Werror=array-bounds]: => 641:28
+ /kisskb/src/include/linux/bitfield.h: error: call to '__field_overflow' declared with attribute error: value doesn't fit into mask: => 151:3
+ /kisskb/src/include/linux/compiler_types.h: error: call to '__compiletime_assert_262' declared with attribute error: Unsupported access size for {READ,WRITE}_ONCE().: => 358:45
+ /kisskb/src/include/linux/compiler_types.h: error: call to '__compiletime_assert_263' declared with attribute error: Unsupported access size for {READ,WRITE}_ONCE().: => 358:45
+ /kisskb/src/include/linux/fortify-string.h: error: '__builtin_memcpy' offset [0, 127] is out of the bounds [0, 0] [-Werror=array-bounds]: => 57:33
+ /kisskb/src/include/linux/fortify-string.h: error: '__builtin_memset' pointer overflow between offset [28, 898293814] and size [-898293787, -1] [-Werror=array-bounds]: => 59:33
+ /kisskb/src/kernel/kcsan/kcsan_test.c: error: the frame size of 1680 bytes is larger than 1536 bytes [-Werror=frame-larger-than=]: => 257:1
+ {standard input}: Error: unknown pseudo-op: `.cfi_def_c': => 1718

13 error improvements:
- /kisskb/src/arch/sh/include/asm/io.h: error: cast to pointer from integer of different size [-Werror=int-to-pointer-cast]: 239:34 =>
- /kisskb/src/drivers/infiniband/hw/qib/qib_wc_x86_64.c: error: 'X86_VENDOR_AMD' undeclared (first use in this function): 149:37 =>
- /kisskb/src/drivers/infiniband/hw/qib/qib_wc_x86_64.c: error: 'struct cpuinfo_um' has no member named 'x86_vendor': 149:22 =>
- /kisskb/src/drivers/infiniband/hw/qib/qib_wc_x86_64.c: error: control reaches end of non-void function [-Werror=return-type]: 150:1 =>
- /kisskb/src/drivers/infiniband/sw/rdmavt/qp.c: error: 'struct cpuinfo_um' has no member named 'x86_cache_size': 88:22 =>
- /kisskb/src/drivers/infiniband/sw/rdmavt/qp.c: error: control reaches end of non-void function [-Werror=return-type]: 89:1 =>
- /kisskb/src/drivers/infiniband/sw/rdmavt/qp.c: error: implicit declaration of function '__copy_user_nocache' [-Werror=implicit-function-declaration]: 100:2 =>
- /kisskb/src/drivers/net/ethernet/marvell/prestera/prestera_flower.c: error: 'rule' is used uninitialized [-Werror=uninitialized]: 480:34 =>
- {standard input}: Error: displacement to undefined symbol .L377 overflows 12-bit field: 2286 =>
- {standard input}: Error: displacement to undefined symbol .L378 overflows 8-bit field : 2302 =>
- {standard input}: Error: displacement to undefined symbol .L382 overflows 8-bit field : 2213 =>
- {standard input}: Error: pcrel too far: 2231, 2204, 2209, 2293, 2262, 2249, 2261, 2229, 2206, 2247, 2232, 2221, 2274, 2217, 2215, 2259, 2248, 2216 =>
- {standard input}: Error: unknown pseudo-op: `.l': 2305 =>


*** WARNINGS ***

13 warning regressions:
+ modpost: WARNING: modpost: "__ndelay" [drivers/gpio/gpio-latch.ko] has no CRC!: => N/A
+ modpost: WARNING: modpost: "__udelay" [drivers/iio/adc/max11410.ko] has no CRC!: => N/A
+ modpost: WARNING: modpost: "__udelay" [drivers/input/keyboard/tegra-kbc.ko] has no CRC!: => N/A
+ modpost: WARNING: modpost: "__udelay" [drivers/mfd/axp20x.ko] has no CRC!: => N/A
+ modpost: WARNING: modpost: "__udelay" [drivers/mmc/host/sunplus-mmc.ko] has no CRC!: => N/A
+ modpost: WARNING: modpost: "__udelay" [drivers/net/ethernet/renesas/rswitch_drv.ko] has no CRC!: => N/A
+ modpost: WARNING: modpost: "__udelay" [drivers/net/wireless/mediatek/mt76/mt7996/mt7996e.ko] has no CRC!: => N/A
+ modpost: WARNING: modpost: "__udelay" [drivers/net/wireless/realtek/rtw89/rtw89_8852b.ko] has no CRC!: => N/A
+ modpost: WARNING: modpost: "__udelay" [drivers/phy/renesas/r8a779f0-ether-serdes.ko] has no CRC!: => N/A
+ modpost: WARNING: modpost: "__udelay" [drivers/ptp/ptp_idt82p33.ko] has no CRC!: => N/A
+ modpost: WARNING: modpost: "__udelay" [drivers/usb/fotg210/fotg210.ko] has no CRC!: => N/A
+ modpost: WARNING: modpost: "__udelay" [fs/xfs/xfs.ko] has no CRC!: => N/A
+ modpost: WARNING: modpost: "empty_zero_page" [net/rxrpc/rxperf.ko] has no CRC!: => N/A

10 warning improvements:
- modpost: WARNING: modpost: "__ashldi3" [lib/zstd/zstd_compress.ko] has no CRC!: N/A =>
- modpost: WARNING: modpost: "__udelay" [drivers/net/can/pch_can.ko] has no CRC!: N/A =>
- modpost: WARNING: modpost: "__udelay" [drivers/net/ethernet/fealnx.ko] has no CRC!: N/A =>
- modpost: WARNING: modpost: "__udelay" [drivers/net/ethernet/smsc/smc911x.ko] has no CRC!: N/A =>
- modpost: WARNING: modpost: "__udelay" [drivers/net/pcs/pcs-altera-tse.ko] has no CRC!: N/A =>
- modpost: WARNING: modpost: "__udelay" [drivers/usb/host/fotg210-hcd.ko] has no CRC!: N/A =>
- modpost: WARNING: modpost: drivers/net/ethernet/qlogic/qed/qed.o: section mismatch in reference: qed_mfw_ext_maps (section: .data) -> qed_mfw_legacy_bb_100g (section: .init.rodata): N/A =>
- modpost: WARNING: modpost: drivers/net/ethernet/qlogic/qed/qed.o: section mismatch in reference: qed_mfw_legacy_maps (section: .data) -> qed_mfw_legacy_bb_100g (section: .init.rodata): N/A =>
- modpost: WARNING: modpost: drivers/net/ethernet/qlogic/qede/qede.o: section mismatch in reference: qede_forced_speed_maps (section: .data) -> qede_forced_speed_100000 (section: .init.rodata): N/A =>
- modpost: WARNING: modpost: vmlinux.o: section mismatch in reference: __trace_event_discard_commit (section: .text.unlikely) -> initcall_level_names (section: .init.data): N/A =>

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

2022-12-28 03:50:30

by Kees Cook

[permalink] [raw]
Subject: Re: Linux 6.2-rc1

On December 26, 2022 9:52:12 PM PST, Guenter Roeck <[email protected]> wrote:
>On Mon, Dec 26, 2022 at 05:32:28PM -0800, Kees Cook wrote:
>> On December 26, 2022 4:29:41 PM PST, Guenter Roeck <[email protected]> wrote:
>> >On Mon, Dec 26, 2022 at 01:03:59PM -0800, Kees Cook wrote:
>> >> On December 26, 2022 12:56:29 PM PST, Linus Torvalds <[email protected]> wrote:
>> >> >On Mon, Dec 26, 2022 at 11:52 AM Guenter Roeck <[email protected]> wrote:
>> >> >>
>> >> >> fs/f2fs/inline.c: In function 'f2fs_move_inline_dirents':
>> >> >> include/linux/fortify-string.h:59:33: error: '__builtin_memset' pointer overflow between offset [28, 898293814] and size [-898293787, -1] [-Werror=array-bounds]
>> >> >> fs/f2fs/inline.c:430:9: note: in expansion of macro 'memset'
>> >> >> 430 | memset(dst.bitmap + src.nr_bitmap, 0, dst.nr_bitmap - src.nr_bitmap);
>> >> >> | ^~~~~~
>> >> >
>> >> >Well, that's unfortunate.
>> >>
>> >> I'll look into this.
>> >>
>> >
>> >I did some more testing. The problem is seen with gcc 11.3.0, but not with
>> >gcc 12.2.0 nor with gcc 10.3.0.
>>
>> That's what I'd expect: 10 didn't have variable range tracking wired up to -Warray-bounds, 11 does, and we disable -Warray-bounds on 12 because of 3 separate 12-only GCC bugs.
>>
>> > gcc bug ? Should I switch to gcc 12.2.0 for
>> >powerpc when build testing the latest kernel ?
>>
>> Sure? But that'll just hide it. I suspect GCC has found a way for dst.nr_bitmap to be compile-time 27, so the size is always negative.
>>
>dst.nr_bitmap is initialized with SIZE_OF_DENTRY_BITMAP,
>which is defined as:
>
>#define NR_DENTRY_IN_BLOCK 214 /* the number of dentry in a block */
>#define SIZE_OF_DIR_ENTRY 11 /* by byte */
>#define SIZE_OF_DENTRY_BITMAP ((NR_DENTRY_IN_BLOCK + BITS_PER_BYTE - 1) / \
> BITS_PER_BYTE)
>
>((214 + 8 - 1) / 8 = 27, so dst.nr_bitmap is indeed compile-time 27.
>
>Not sure how would know that src.nr_bitmap can be > 27, though.
>Am I missing something ?

I think it's saying it can't rule out it being larger? I.e. there is no obvious bounds checking for it. Perhaps:

if (src.nr_bitmap > dst.nr_bitmap) {
err = -EFSCORRUPTED;
goto out;
}


-Kees


--
Kees Cook

2022-12-28 15:12:23

by Guenter Roeck

[permalink] [raw]
Subject: Re: Linux 6.2-rc1

On Tue, Dec 27, 2022 at 07:40:30PM -0800, Kees Cook wrote:
> On December 26, 2022 9:52:12 PM PST, Guenter Roeck <[email protected]> wrote:
> >On Mon, Dec 26, 2022 at 05:32:28PM -0800, Kees Cook wrote:
> >> On December 26, 2022 4:29:41 PM PST, Guenter Roeck <[email protected]> wrote:
> >> >On Mon, Dec 26, 2022 at 01:03:59PM -0800, Kees Cook wrote:
> >> >> On December 26, 2022 12:56:29 PM PST, Linus Torvalds <[email protected]> wrote:
> >> >> >On Mon, Dec 26, 2022 at 11:52 AM Guenter Roeck <[email protected]> wrote:
> >> >> >>
> >> >> >> fs/f2fs/inline.c: In function 'f2fs_move_inline_dirents':
> >> >> >> include/linux/fortify-string.h:59:33: error: '__builtin_memset' pointer overflow between offset [28, 898293814] and size [-898293787, -1] [-Werror=array-bounds]
> >> >> >> fs/f2fs/inline.c:430:9: note: in expansion of macro 'memset'
> >> >> >> 430 | memset(dst.bitmap + src.nr_bitmap, 0, dst.nr_bitmap - src.nr_bitmap);
> >> >> >> | ^~~~~~
> >> >> >
> >> >> >Well, that's unfortunate.
> >> >>
> >> >> I'll look into this.
> >> >>
> >> >
> >> >I did some more testing. The problem is seen with gcc 11.3.0, but not with
> >> >gcc 12.2.0 nor with gcc 10.3.0.
> >>
> >> That's what I'd expect: 10 didn't have variable range tracking wired up to -Warray-bounds, 11 does, and we disable -Warray-bounds on 12 because of 3 separate 12-only GCC bugs.
> >>
> >> > gcc bug ? Should I switch to gcc 12.2.0 for
> >> >powerpc when build testing the latest kernel ?
> >>
> >> Sure? But that'll just hide it. I suspect GCC has found a way for dst.nr_bitmap to be compile-time 27, so the size is always negative.
> >>
> >dst.nr_bitmap is initialized with SIZE_OF_DENTRY_BITMAP,
> >which is defined as:
> >
> >#define NR_DENTRY_IN_BLOCK 214 /* the number of dentry in a block */
> >#define SIZE_OF_DIR_ENTRY 11 /* by byte */
> >#define SIZE_OF_DENTRY_BITMAP ((NR_DENTRY_IN_BLOCK + BITS_PER_BYTE - 1) / \
> > BITS_PER_BYTE)
> >
> >((214 + 8 - 1) / 8 = 27, so dst.nr_bitmap is indeed compile-time 27.
> >
> >Not sure how would know that src.nr_bitmap can be > 27, though.
> >Am I missing something ?
>
> I think it's saying it can't rule out it being larger? I.e. there is no obvious bounds checking for it. Perhaps:
>
> if (src.nr_bitmap > dst.nr_bitmap) {
> err = -EFSCORRUPTED;
> goto out;
> }
>

After going through all calculations, using maximum values (or minimum
values where appropriate) everywhere, I calculated that src.nr_bitmap
is always <= 24. The actual inode is sanity checked in
fs/f2fs/inode.c:sanity_check_inode().

Also, why is this only seen when I try to build powerpc test images ?

Thanks,
Guenter

2023-01-01 01:40:24

by Rob Landley

[permalink] [raw]
Subject: Re: Build regressions/improvements in v6.2-rc1

On 12/27/22 02:35, Geert Uytterhoeven wrote:
> sh4-gcc11/sh-allmodconfig (ICE = internal compiler error)

What's your actual test config here? Because when I try make ARCH=sh
allmodconfig; make ARCH=sh it dies in arch/sh/kernel/cpu/sh2/setup-sh7619.c with:

./include/linux/sh_intc.h:100:63: error: division 'sizeof (void *) / sizeof
(void)' does not compute the number of array elements [-Werror=sizeof-pointer-div]
100 | #define _INTC_ARRAY(a) a, __same_type(a, NULL) ? 0 : sizeof(a)/sizeof(*a)

(Which isn't new, lots of configs won't compile off x86 and arm. I'm not sure
allmodconfig is picking a sane/actual cpu/board combo?)

What actual configuration are you trying to build?

Rob

P.S. Also my ssh cross gcc is 9.4 so I may need to build gcc-11 to see the
error, but I thought I'd try to reproduce the easy way first...

2023-01-01 12:37:44

by Geert Uytterhoeven

[permalink] [raw]
Subject: Re: Build regressions/improvements in v6.2-rc1

Hi Rob,

On Sun, Jan 1, 2023 at 2:22 AM Rob Landley <[email protected]> wrote:
> On 12/27/22 02:35, Geert Uytterhoeven wrote:
> > sh4-gcc11/sh-allmodconfig (ICE = internal compiler error)
>
> What's your actual test config here? Because when I try make ARCH=sh
> allmodconfig; make ARCH=sh it dies in arch/sh/kernel/cpu/sh2/setup-sh7619.c with:

[re-adding the URL you deleted]

> > [2] http://kisskb.ellerman.id.au/kisskb/branch/linus/head/830b3c68c1fb1e9176028d02ef86f3cf76aa2476/ (all 152 configs)

Following to
http://kisskb.ellerman.id.au/kisskb/target/212841/ and
http://kisskb.ellerman.id.au/kisskb/buildresult/14854440/
gives you a page with a link to the config.

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

2023-01-04 07:07:20

by Michael Ellerman

[permalink] [raw]
Subject: Re: Build regressions/improvements in v6.2-rc1

Geert Uytterhoeven <[email protected]> writes:
> Hi Rob,
>
> On Sun, Jan 1, 2023 at 2:22 AM Rob Landley <[email protected]> wrote:
>> On 12/27/22 02:35, Geert Uytterhoeven wrote:
>> > sh4-gcc11/sh-allmodconfig (ICE = internal compiler error)
>>
>> What's your actual test config here? Because when I try make ARCH=sh
>> allmodconfig; make ARCH=sh it dies in arch/sh/kernel/cpu/sh2/setup-sh7619.c with:
>
> [re-adding the URL you deleted]
>
>> > [2] http://kisskb.ellerman.id.au/kisskb/branch/linus/head/830b3c68c1fb1e9176028d02ef86f3cf76aa2476/ (all 152 configs)
>
> Following to
> http://kisskb.ellerman.id.au/kisskb/target/212841/ and
> http://kisskb.ellerman.id.au/kisskb/buildresult/14854440/
> gives you a page with a link to the config.

It's possible there's something wrong with the toolchain setup, I don't
know much about sh.

But it's just the kernel.org crosstool sh4 compiler, nothing else fancy.

cheers

2023-01-04 19:40:42

by Pali Rohár

[permalink] [raw]
Subject: Re: Linux 6.2-rc1

Hello! This 6.2-rc1 release is breaking support for CD-RW, DVD-RW,
DVD+RW and partially also DVD-RAM medias because of pktcdvd driver
removal from the kernel in this release.

Driver is still used and userspace tools for it are part of the udftools
project, which is still under active maintenance. More people already
informed me about this "surprise".

Any comments on this? Because until now nobody answered why this
actively used driver was removed from kernel without informing anybody:
https://lore.kernel.org/lkml/20221224095353.w32xhmyzlft6qi4v@pali/

2023-01-04 20:00:33

by Linus Torvalds

[permalink] [raw]
Subject: Re: Linux 6.2-rc1

On Wed, Jan 4, 2023 at 11:01 AM Pali Rohár <[email protected]> wrote:
>
> Driver is still used and userspace tools for it are part of the udftools
> project, which is still under active maintenance. More people already
> informed me about this "surprise".

Why is that driver used?

It's *literally* pointless. It's just a shell that forwards ioctl's to
the real drivers.

> Any comments on this? Because until now nobody answered why this
> actively used driver was removed from kernel without informing anybody:

Well, it's been marked as deprecated for five years, so any kernel
config should have gotten this notice for the help entry

Note: This driver is deprecated and will be removed from the
kernel in the near future!

but I guess people didn't notice.

It could be re-instated, but it really is a completely useless driver.
Just use the *regular* device nodes, not the pointless pktcd ones.

Is there any real reason why udftools can't just use the normal device node?

The historical reason for this driver being pointless goes back *much*
longer than five years - it used to be that the pktcd driver was
special, and was the only thing that did raw commands.

But the regular block layer was taught to do that back around 2004, so
the "pktcd" driver has literally just been a pointless shell for
almost two decades.

And I know it was in 2004, because I actually did most of that "make
SCSI commands generic" work myself (but had to go back to the old BK
archives to find the exact date - it's been two decades, after all).

I did it because I was fed up with the crazy pktcd driver requiring
extra work, when I just wanted to write CD's on my regular IDE CD-ROM
the obvious way.

So if there is some reason to actually use the pktcd driver, please
tell us what that is.

Linus

2023-01-04 21:09:09

by Pali Rohár

[permalink] [raw]
Subject: Re: Linux 6.2-rc1

On Wednesday 04 January 2023 11:25:41 Linus Torvalds wrote:
> On Wed, Jan 4, 2023 at 11:01 AM Pali Rohár <[email protected]> wrote:
> >
> > Driver is still used and userspace tools for it are part of the udftools
> > project, which is still under active maintenance. More people already
> > informed me about this "surprise".
>
> Why is that driver used?
>
> It's *literally* pointless. It's just a shell that forwards ioctl's to
> the real drivers.
>
> > Any comments on this? Because until now nobody answered why this
> > actively used driver was removed from kernel without informing anybody:
>
> Well, it's been marked as deprecated for five years, so any kernel
> config should have gotten this notice for the help entry
>
> Note: This driver is deprecated and will be removed from the
> kernel in the near future!
>
> but I guess people didn't notice.
>
> It could be re-instated, but it really is a completely useless driver.
> Just use the *regular* device nodes, not the pointless pktcd ones.
>
> Is there any real reason why udftools can't just use the normal device node?
>
> The historical reason for this driver being pointless goes back *much*
> longer than five years - it used to be that the pktcd driver was
> special, and was the only thing that did raw commands.
>
> But the regular block layer was taught to do that back around 2004, so
> the "pktcd" driver has literally just been a pointless shell for
> almost two decades.
>
> And I know it was in 2004, because I actually did most of that "make
> SCSI commands generic" work myself (but had to go back to the old BK
> archives to find the exact date - it's been two decades, after all).
>
> I did it because I was fed up with the crazy pktcd driver requiring
> extra work, when I just wanted to write CD's on my regular IDE CD-ROM
> the obvious way.
>
> So if there is some reason to actually use the pktcd driver, please
> tell us what that is.
>
> Linus

Last time I did big retest of optical media was two years ago. At that
time kernel was not able to mount CD-RW disc in full read-write mode
from the normal node /dev/cdrom. Via pktcdvd driver mapping it was
possible without any issue. Was there any change in last 5 (or more)
years in this CD-RW area? Mounting CD-RW media in read-only mode via
normal /dev/cdrom node always worked fine. Also "burning" CD-R media
with userspace burning tools on normal /dev/cdrom node also worked.
But here it is CD-RW media in read-write mode with kernel udf filesystem
driver without any userspace involved (after proper formatting).

2023-01-04 21:41:08

by Pali Rohár

[permalink] [raw]
Subject: Re: Linux 6.2-rc1

On Wednesday 04 January 2023 21:56:40 Pali Rohár wrote:
> On Wednesday 04 January 2023 11:25:41 Linus Torvalds wrote:
> > On Wed, Jan 4, 2023 at 11:01 AM Pali Rohár <[email protected]> wrote:
> > >
> > > Driver is still used and userspace tools for it are part of the udftools
> > > project, which is still under active maintenance. More people already
> > > informed me about this "surprise".
> >
> > Why is that driver used?
> >
> > It's *literally* pointless. It's just a shell that forwards ioctl's to
> > the real drivers.
> >
> > > Any comments on this? Because until now nobody answered why this
> > > actively used driver was removed from kernel without informing anybody:
> >
> > Well, it's been marked as deprecated for five years, so any kernel
> > config should have gotten this notice for the help entry
> >
> > Note: This driver is deprecated and will be removed from the
> > kernel in the near future!
> >
> > but I guess people didn't notice.
> >
> > It could be re-instated, but it really is a completely useless driver.
> > Just use the *regular* device nodes, not the pointless pktcd ones.
> >
> > Is there any real reason why udftools can't just use the normal device node?
> >
> > The historical reason for this driver being pointless goes back *much*
> > longer than five years - it used to be that the pktcd driver was
> > special, and was the only thing that did raw commands.
> >
> > But the regular block layer was taught to do that back around 2004, so
> > the "pktcd" driver has literally just been a pointless shell for
> > almost two decades.
> >
> > And I know it was in 2004, because I actually did most of that "make
> > SCSI commands generic" work myself (but had to go back to the old BK
> > archives to find the exact date - it's been two decades, after all).
> >
> > I did it because I was fed up with the crazy pktcd driver requiring
> > extra work, when I just wanted to write CD's on my regular IDE CD-ROM
> > the obvious way.
> >
> > So if there is some reason to actually use the pktcd driver, please
> > tell us what that is.
> >
> > Linus
>
> Last time I did big retest of optical media was two years ago. At that
> time kernel was not able to mount CD-RW disc in full read-write mode
> from the normal node /dev/cdrom. Via pktcdvd driver mapping it was
> possible without any issue. Was there any change in last 5 (or more)
> years in this CD-RW area? Mounting CD-RW media in read-only mode via
> normal /dev/cdrom node always worked fine. Also "burning" CD-R media
> with userspace burning tools on normal /dev/cdrom node also worked.
> But here it is CD-RW media in read-write mode with kernel udf filesystem
> driver without any userspace involved (after proper formatting).

In commit where was pktcdvd dropped is written:
https://git.kernel.org/torvalds/c/f40eb99897af665f11858dd7b56edcb62c3f3c67

* At the lowest level, there is the standard driver for the CD/DVD device,
* such as drivers/scsi/sr.c. This driver can handle read and write requests,
* but it doesn't know anything about the special restrictions that apply to
* packet writing. One restriction is that write requests must be aligned to
* packet boundaries on the physical media, and the size of a write request
* must be equal to the packet size. Another restriction is that a
* GPCMD_FLUSH_CACHE command has to be issued to the drive before a read
* command, if the previous command was a write.
*
* The purpose of the packet writing driver is to hide these restrictions from
* higher layers, such as file systems, and present a block device that can be
* randomly read and written using 2kB-sized blocks.

Were all these write restrictions implemented in sr.c driver? Do you
remember other details?

Because CD-RW support into kernel was really introduced in 2004 in
this historical commit, but it was not for SCSI sr.c driver:
https://git.kernel.org/history/history/c/2f8e2dc86c9876edca632e8ef2ab1f68d1b753f0

2023-01-04 21:41:23

by Linus Torvalds

[permalink] [raw]
Subject: Re: Linux 6.2-rc1

On Wed, Jan 4, 2023 at 12:56 PM Pali Rohár <[email protected]> wrote:
>
> Last time I did big retest of optical media was two years ago. At that
> time kernel was not able to mount CD-RW disc in full read-write mode
> from the normal node /dev/cdrom. Via pktcdvd driver mapping it was
> possible without any issue.

So that is part of the problem here: pretty much nobody uses optical
media any more, and the whole driver has been orphaned and has no
maintainer (for the last five years - it's why it was removed, after
all).

And people _have_ been using the normal /dev/cdrom for this, so it
almost certainly isn't entirely broken. But the test coverage is
getting increasingly sparse...

> Was there any change in last 5 (or more) years in this CD-RW area?

There's been a fair amount of cleanup wrt all the SCSI ioctl handling
in the last 5 years (and before).

But:

> Mounting CD-RW media in read-only mode via normal /dev/cdrom node
> always worked fine. Also "burning" CD-R media with userspace burning
> tools on normal /dev/cdrom node also worked.

Yeah, that's all I ever personally have done, and that most definitely
has been there for decades...

> But here it is CD-RW media in read-write mode with kernel udf
> filesystem driver without any userspace involved (after proper
> formatting).

... but I'm not sure about direct writeable mount support.

That may indeed be an area that only pktcdvd ended up doing. I've
never used it myself, even historically.

Let's bring in more people. Because they may not have thought about
some RW UDF case.

The removal seems to revert cleanly, although it does require
reverting a few subsequent commits too (that removed code that only
pktcdvd used):

git revert db1c7d779767 85d6ce58e493 f40eb99897af

where we have

db1c7d779767 block: bio_copy_data_iter
85d6ce58e493 block: remove devnode callback from struct
block_device_operations
f40eb99897af pktcdvd: remove driver.

Christoph, Jens? See

https://lore.kernel.org/lkml/20230104190115.ceglfefco475ev6c@pali/

for the beginning of this thread.

Linus

2023-01-04 22:29:52

by Jens Axboe

[permalink] [raw]
Subject: Re: Linux 6.2-rc1

On 1/4/23 2:32?PM, Linus Torvalds wrote:
>> But here it is CD-RW media in read-write mode with kernel udf
>> filesystem driver without any userspace involved (after proper
>> formatting).
>
> ... but I'm not sure about direct writeable mount support.
>
> That may indeed be an area that only pktcdvd ended up doing. I've
> never used it myself, even historically.
>
> Let's bring in more people. Because they may not have thought about
> some RW UDF case.

We did think about it, since that's the only reason for pktcdvd to
exist. Basically what the driver does is ensure that any write is 32K in
size, which is the size which can be written to media. It'll gather data
as needed to make that happen. Thats it. Outside of that, it's just some
setup and closing code.

This obviously would be better to handle in userspace, all of it. Back
when I wrote this driver, we didn't have a lot of the fancier stuff we
have today. It could be done via ublk, for example, or something like
that.

The surprising bit here is:

1) Someone is still using this driver, and
2) It actually works!

While I'd love to nudge folks in other directions for this use case, and
I strongly think that we should, it also doesn't seem fair to just yank
it while folks are using it... But I'd like to VERY strongly encourage
folks to come up with a new solution for this use case. It really isn't
a solution that belongs in the kernel today.

> The removal seems to revert cleanly, although it does require
> reverting a few subsequent commits too (that removed code that only
> pktcdvd used):
>
> git revert db1c7d779767 85d6ce58e493 f40eb99897af
>
> where we have
>
> db1c7d779767 block: bio_copy_data_iter
> 85d6ce58e493 block: remove devnode callback from struct
> block_device_operations
> f40eb99897af pktcdvd: remove driver.

I'll queue this up - and unless I hear valid complaints to why we should
not just reinstate the driver for now, it'll go out with the next pull
request.

--
Jens Axboe

2023-01-05 11:44:51

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: Linux 6.2-rc1

On Wed, Jan 04, 2023 at 02:43:16PM -0700, Jens Axboe wrote:
> > The removal seems to revert cleanly, although it does require
> > reverting a few subsequent commits too (that removed code that only
> > pktcdvd used):
> >
> > git revert db1c7d779767 85d6ce58e493 f40eb99897af
> >
> > where we have
> >
> > db1c7d779767 block: bio_copy_data_iter
> > 85d6ce58e493 block: remove devnode callback from struct
> > block_device_operations
> > f40eb99897af pktcdvd: remove driver.
>
> I'll queue this up - and unless I hear valid complaints to why we should
> not just reinstate the driver for now, it'll go out with the next pull
> request.

If you do revert these, watch out for a build warning from the driver
core when you revert this, I think there's a 'const' missing from
somewhere that you might have to add back in due to other driver core
changes.

thanks,

greg k-h

2023-01-05 16:06:03

by Jens Axboe

[permalink] [raw]
Subject: Re: Linux 6.2-rc1

On 1/5/23 4:25 AM, Greg Kroah-Hartman wrote:
> On Wed, Jan 04, 2023 at 02:43:16PM -0700, Jens Axboe wrote:
>>> The removal seems to revert cleanly, although it does require
>>> reverting a few subsequent commits too (that removed code that only
>>> pktcdvd used):
>>>
>>> git revert db1c7d779767 85d6ce58e493 f40eb99897af
>>>
>>> where we have
>>>
>>> db1c7d779767 block: bio_copy_data_iter
>>> 85d6ce58e493 block: remove devnode callback from struct
>>> block_device_operations
>>> f40eb99897af pktcdvd: remove driver.
>>
>> I'll queue this up - and unless I hear valid complaints to why we should
>> not just reinstate the driver for now, it'll go out with the next pull
>> request.
>
> If you do revert these, watch out for a build warning from the driver
> core when you revert this, I think there's a 'const' missing from
> somewhere that you might have to add back in due to other driver core
> changes.

The revert compiles cleanly, even merged with current master.

--
Jens Axboe


2023-01-05 18:05:14

by Jens Axboe

[permalink] [raw]
Subject: Re: Linux 6.2-rc1

On 1/5/23 10:42?AM, Pali Roh?r wrote:
> On Wednesday 04 January 2023 14:43:16 Jens Axboe wrote:
>> On 1/4/23 2:32?PM, Linus Torvalds wrote:
>>>> But here it is CD-RW media in read-write mode with kernel udf
>>>> filesystem driver without any userspace involved (after proper
>>>> formatting).
>>>
>>> ... but I'm not sure about direct writeable mount support.
>>>
>>> That may indeed be an area that only pktcdvd ended up doing. I've
>>> never used it myself, even historically.
>>>
>>> Let's bring in more people. Because they may not have thought about
>>> some RW UDF case.
>>
>> We did think about it, since that's the only reason for pktcdvd to
>> exist. Basically what the driver does is ensure that any write is 32K in
>> size, which is the size which can be written to media. It'll gather data
>> as needed to make that happen. Thats it. Outside of that, it's just some
>> setup and closing code.
>>
>> This obviously would be better to handle in userspace, all of it. Back
>> when I wrote this driver, we didn't have a lot of the fancier stuff we
>> have today. It could be done via ublk, for example, or something like
>> that.
>>
>> The surprising bit here is:
>>
>> 1) Someone is still using this driver, and
>> 2) It actually works!
>
> Yes, there are still users and userspace tools (cdrwtool / pktsetup) are
> still receiving either small patches or issue reports. I think that it
> was two years ago when cdrwtool received big fixups to support
> formatting CD-RW discs on new CD/DVD drives.
>
>> While I'd love to nudge folks in other directions for this use case, and
>> I strongly think that we should, it also doesn't seem fair to just yank
>> it while folks are using it... But I'd like to VERY strongly encourage
>> folks to come up with a new solution for this use case. It really isn't
>> a solution that belongs in the kernel today.
>
> Linus in previous email wrote that he did "make SCSI commands generic"
> work in past so direct usage of /dev/cdrom device works for CD-R burning
> and read-only mounting.

Not quite sure what that refers to, as I'm pretty sure I did all of that
work. But maybe Linus can refresh my memory here :-)

> So could not be (for example sr.c) driver extended to directly do
> pktcdvd's work? So when somebody opens /dev/cdrom device in O_RDWR mode
> and CD-RW medium is present then it would behave like pktcdvd device...
> To have /dev/cdrom generic also for CD-RW write access.

As mentioned, I don't think this kind of code belongs in the kernel. sr
or cdrom could easily be modified to support the necessary bits to
handle a writeable open, but the grunt of the pktcdvd code deals with
retrieving and writing out bigger chunks of data. And that part really
does belong in userspace imho.

--
Jens Axboe

2023-01-05 18:06:04

by Pali Rohár

[permalink] [raw]
Subject: Re: Linux 6.2-rc1

On Wednesday 04 January 2023 14:43:16 Jens Axboe wrote:
> On 1/4/23 2:32?PM, Linus Torvalds wrote:
> >> But here it is CD-RW media in read-write mode with kernel udf
> >> filesystem driver without any userspace involved (after proper
> >> formatting).
> >
> > ... but I'm not sure about direct writeable mount support.
> >
> > That may indeed be an area that only pktcdvd ended up doing. I've
> > never used it myself, even historically.
> >
> > Let's bring in more people. Because they may not have thought about
> > some RW UDF case.
>
> We did think about it, since that's the only reason for pktcdvd to
> exist. Basically what the driver does is ensure that any write is 32K in
> size, which is the size which can be written to media. It'll gather data
> as needed to make that happen. Thats it. Outside of that, it's just some
> setup and closing code.
>
> This obviously would be better to handle in userspace, all of it. Back
> when I wrote this driver, we didn't have a lot of the fancier stuff we
> have today. It could be done via ublk, for example, or something like
> that.
>
> The surprising bit here is:
>
> 1) Someone is still using this driver, and
> 2) It actually works!

Yes, there are still users and userspace tools (cdrwtool / pktsetup) are
still receiving either small patches or issue reports. I think that it
was two years ago when cdrwtool received big fixups to support
formatting CD-RW discs on new CD/DVD drives.

> While I'd love to nudge folks in other directions for this use case, and
> I strongly think that we should, it also doesn't seem fair to just yank
> it while folks are using it... But I'd like to VERY strongly encourage
> folks to come up with a new solution for this use case. It really isn't
> a solution that belongs in the kernel today.

Linus in previous email wrote that he did "make SCSI commands generic"
work in past so direct usage of /dev/cdrom device works for CD-R burning
and read-only mounting.

So could not be (for example sr.c) driver extended to directly do
pktcdvd's work? So when somebody opens /dev/cdrom device in O_RDWR mode
and CD-RW medium is present then it would behave like pktcdvd device...
To have /dev/cdrom generic also for CD-RW write access.

> > The removal seems to revert cleanly, although it does require
> > reverting a few subsequent commits too (that removed code that only
> > pktcdvd used):
> >
> > git revert db1c7d779767 85d6ce58e493 f40eb99897af
> >
> > where we have
> >
> > db1c7d779767 block: bio_copy_data_iter
> > 85d6ce58e493 block: remove devnode callback from struct
> > block_device_operations
> > f40eb99897af pktcdvd: remove driver.
>
> I'll queue this up - and unless I hear valid complaints to why we should
> not just reinstate the driver for now, it'll go out with the next pull
> request.
>
> --
> Jens Axboe
>

2023-01-05 19:18:03

by Linus Torvalds

[permalink] [raw]
Subject: Re: Linux 6.2-rc1

On Thu, Jan 5, 2023 at 9:45 AM Jens Axboe <[email protected]> wrote:
>
> Not quite sure what that refers to, as I'm pretty sure I did all of that
> work. But maybe Linus can refresh my memory here :-)

I was definitely there, part of making it actually work for *every*
block device.

Long long ago, it used to be limited to the sg_io() interface, and
only worked for SCSI devices.

So you couldn't actually burn CD's with the regular IDE/ATA CD ROM
drivers directly, but had to use a shim driver, kind of like pktcdvd.
Except I think it was just /dev/cdrom.

See

https://git.kernel.org/pub/scm/linux/kernel/git/tglx/history.git/commit/?id=90df68e70b

for some of it (exposing SG_IO to all the block ioctls), and the "make
it more usable" parts that made it do sane permission checking in

https://git.kernel.org/pub/scm/linux/kernel/git/tglx/history.git/commit/?id=a75aaa84276

and the commits preceding it for that part of the work.

But yes, you were very much involved too.

> As mentioned, I don't think this kind of code belongs in the kernel. sr
> or cdrom could easily be modified to support the necessary bits to
> handle a writeable open, but the grunt of the pktcdvd code deals with
> retrieving and writing out bigger chunks of data. And that part really
> does belong in userspace imho.

Well, it's the UDF write support that is the issue.. I didn't even
realize people did that.

You'd presumably have to re-do it as a FUSE thing.

Linus

2023-01-05 20:09:57

by Jens Axboe

[permalink] [raw]
Subject: Re: Linux 6.2-rc1

On 1/5/23 12:06 PM, Linus Torvalds wrote:
> On Thu, Jan 5, 2023 at 9:45 AM Jens Axboe <[email protected]> wrote:
>>
>> Not quite sure what that refers to, as I'm pretty sure I did all of that
>> work. But maybe Linus can refresh my memory here :-)
>
> I was definitely there, part of making it actually work for *every*
> block device.
>
> Long long ago, it used to be limited to the sg_io() interface, and
> only worked for SCSI devices.
>
> So you couldn't actually burn CD's with the regular IDE/ATA CD ROM
> drivers directly, but had to use a shim driver, kind of like pktcdvd.
> Except I think it was just /dev/cdrom.
>
> See
>
> https://git.kernel.org/pub/scm/linux/kernel/git/tglx/history.git/commit/?id=90df68e70b
>
> for some of it (exposing SG_IO to all the block ioctls), and the "make
> it more usable" parts that made it do sane permission checking in
>
> https://git.kernel.org/pub/scm/linux/kernel/git/tglx/history.git/commit/?id=a75aaa84276
>
> and the commits preceding it for that part of the work.
>
> But yes, you were very much involved too.

I knew that'd get you digging into the archives ;-)

Fair point, I was mostly thinking of the block infrastructure for doing
non-fs commands.

>> As mentioned, I don't think this kind of code belongs in the kernel. sr
>> or cdrom could easily be modified to support the necessary bits to
>> handle a writeable open, but the grunt of the pktcdvd code deals with
>> retrieving and writing out bigger chunks of data. And that part really
>> does belong in userspace imho.
>
> Well, it's the UDF write support that is the issue.. I didn't even
> realize people did that.
>
> You'd presumably have to re-do it as a FUSE thing.

Or even implement it in UDF itself somehow. But yes, ideally we'd punt all
of this data gathering to userspace and just leave the trivial init/stop
atapi/scsi commands to cdrom/sr.

--
Jens Axboe


2023-01-05 20:15:23

by Pali Rohár

[permalink] [raw]
Subject: Re: Linux 6.2-rc1

On Thursday 05 January 2023 11:06:21 Linus Torvalds wrote:
> > As mentioned, I don't think this kind of code belongs in the kernel. sr
> > or cdrom could easily be modified to support the necessary bits to
> > handle a writeable open, but the grunt of the pktcdvd code deals with
> > retrieving and writing out bigger chunks of data. And that part really
> > does belong in userspace imho.
>
> Well, it's the UDF write support that is the issue.. I didn't even
> realize people did that.
>
> You'd presumably have to re-do it as a FUSE thing.

It is not UDF / filesystem specific stuff. You can use any other
filesystem which you like, for example ext4. And in past some people
really used ext2 on CD-RWs. And on Windows systems before Vista, only
FAT32 was supported in read-write mode on CD-RW. UDF even on CD-RW was
read-only.

So FUSE here really does not help. As we are talking about block
storage, not filesystem storage.

2023-01-05 20:35:08

by Linus Torvalds

[permalink] [raw]
Subject: Re: Linux 6.2-rc1

On Thu, Jan 5, 2023 at 11:40 AM Jens Axboe <[email protected]> wrote:
>
> Or even implement it in UDF itself somehow. But yes, ideally we'd punt all
> of this data gathering to userspace and just leave the trivial init/stop
> atapi/scsi commands to cdrom/sr.

I wonder how much of that could be done by just having a different
elevator algorithm for cdrw devices..

Anyway, realistically I suspect the real answer is that "nobody cares
enough any more". I suspect most people haven't used RW optical media
in over a decade, and we're talking about an increasingly dwindling
niche use.

Optical media may still make sense for backup, but probably not the
"filesystem" kind.

So nobody is going to be motivated to do any development in this area,
and the best we can do is probably to just keep it limping along.

Now, it's a bit sad how pktcdvd is the only user of that 'struct
block_device_operations' devnode thing, and I liked how that went away
after the removal of this driver.

And I'm not sure why pktcdvd needs it, everybody else seems to be
happy with gendisk->disk_name.

There's probably other cruft in pktcdvd that could be removed without
removing the whole driver, but I do get the feeling that it's just
less pain to keep the status quo, and that there isn't really much
motivation for anybody to do anything else.

Linus

2023-01-05 20:46:49

by Jens Axboe

[permalink] [raw]
Subject: Re: Linux 6.2-rc1

On 1/5/23 1:03?PM, Linus Torvalds wrote:
> On Thu, Jan 5, 2023 at 11:40 AM Jens Axboe <[email protected]> wrote:
>>
>> Or even implement it in UDF itself somehow. But yes, ideally we'd punt all
>> of this data gathering to userspace and just leave the trivial init/stop
>> atapi/scsi commands to cdrom/sr.
>
> I wonder how much of that could be done by just having a different
> elevator algorithm for cdrw devices..
>
> Anyway, realistically I suspect the real answer is that "nobody cares
> enough any more". I suspect most people haven't used RW optical media
> in over a decade, and we're talking about an increasingly dwindling
> niche use.

While there's some overlap with IO scheduling, I don't think that'd be a
good layer to solve it at. And since this isn't exactly up-and-coming
technology that we expect to proliferate, that makes me especially
hesitant to invest any time in that particular direction.

I still think that doing something with ublk would be the best approach,
and push the data gathering and fixed sized write bits in userspace.
That would still allow arbitrary filesystem usage for these kinds of
devices.

> Optical media may still make sense for backup, but probably not the
> "filesystem" kind.

I don't think it ever made sense, except from a convenience point of
view. And that's most likely what drove the adoption there. It is way
easier to mount a cdrw read/write and copy files there, even if it's
slower than burning an iso image...

> So nobody is going to be motivated to do any development in this area,
> and the best we can do is probably to just keep it limping along.

Indeed...

> Now, it's a bit sad how pktcdvd is the only user of that 'struct
> block_device_operations' devnode thing, and I liked how that went away
> after the removal of this driver.
>
> And I'm not sure why pktcdvd needs it, everybody else seems to be
> happy with gendisk->disk_name.

Let me look into that, I actually don't know. Would be nice if we could
fix that up and re-instate that particular patch.

> There's probably other cruft in pktcdvd that could be removed without
> removing the whole driver, but I do get the feeling that it's just
> less pain to keep the status quo, and that there isn't really much
> motivation for anybody to do anything else.

I'm reluctant to touch it outside of changes that are driven by core
changes, and of course the motivation to remove it was driven by not
wanting to do that either. Any kind of re-architecting of how it works I
would not advocate for. It supposedly works well enough that none of the
(few) users are reporting issues with it, best to just let it remain
like that imho.

--
Jens Axboe

2023-01-06 17:17:37

by Pali Rohár

[permalink] [raw]
Subject: Re: Linux 6.2-rc1

On Thursday 05 January 2023 13:33:11 Jens Axboe wrote:
> On 1/5/23 1:03?PM, Linus Torvalds wrote:
> > So nobody is going to be motivated to do any development in this area,
> > and the best we can do is probably to just keep it limping along.
>
> Indeed...
...
> > There's probably other cruft in pktcdvd that could be removed without
> > removing the whole driver, but I do get the feeling that it's just
> > less pain to keep the status quo, and that there isn't really much
> > motivation for anybody to do anything else.
>
> I'm reluctant to touch it outside of changes that are driven by core
> changes, and of course the motivation to remove it was driven by not
> wanting to do that either. Any kind of re-architecting of how it works I
> would not advocate for. It supposedly works well enough that none of the
> (few) users are reporting issues with it, best to just let it remain
> like that imho.

Yea, I agree. This code is in state when it is _used_ and not developed
anymore. Nobody is really motivated to re-architecture or rewrite this
code. Such work has big probability to break something which currently
works fine. And because lot of users are on stable/LTS kernel versions,
it is possible that we would not notice breakage earlier than (lets say)
in 5 years.

2023-01-06 17:25:48

by Jens Axboe

[permalink] [raw]
Subject: Re: Linux 6.2-rc1

On 1/6/23 9:58?AM, Pali Roh?r wrote:
> On Thursday 05 January 2023 13:33:11 Jens Axboe wrote:
>> On 1/5/23 1:03?PM, Linus Torvalds wrote:
>>> So nobody is going to be motivated to do any development in this area,
>>> and the best we can do is probably to just keep it limping along.
>>
>> Indeed...
> ...
>>> There's probably other cruft in pktcdvd that could be removed without
>>> removing the whole driver, but I do get the feeling that it's just
>>> less pain to keep the status quo, and that there isn't really much
>>> motivation for anybody to do anything else.
>>
>> I'm reluctant to touch it outside of changes that are driven by core
>> changes, and of course the motivation to remove it was driven by not
>> wanting to do that either. Any kind of re-architecting of how it works I
>> would not advocate for. It supposedly works well enough that none of the
>> (few) users are reporting issues with it, best to just let it remain
>> like that imho.
>
> Yea, I agree. This code is in state when it is _used_ and not developed
> anymore. Nobody is really motivated to re-architecture or rewrite this
> code. Such work has big probability to break something which currently
> works fine. And because lot of users are on stable/LTS kernel versions,
> it is possible that we would not notice breakage earlier than (lets say)
> in 5 years.

I did sent out the revert this morning, would be great if you can test
6.2-rc3 when it is out. I'm a bit skeptical on the whole devnode front,
and suspect we might need to convert that to disk_name manipulation.
Which would be fine, as we can then drop the devnode reinstate revert as
well going forward. But I need to find a bit of time to look closer at
this part.

--
Jens Axboe

2023-01-07 00:35:06

by Jaegeuk Kim

[permalink] [raw]
Subject: Re: Linux 6.2-rc1

On 12/28, Guenter Roeck wrote:
> On Tue, Dec 27, 2022 at 07:40:30PM -0800, Kees Cook wrote:
> > On December 26, 2022 9:52:12 PM PST, Guenter Roeck <[email protected]> wrote:
> > >On Mon, Dec 26, 2022 at 05:32:28PM -0800, Kees Cook wrote:
> > >> On December 26, 2022 4:29:41 PM PST, Guenter Roeck <[email protected]> wrote:
> > >> >On Mon, Dec 26, 2022 at 01:03:59PM -0800, Kees Cook wrote:
> > >> >> On December 26, 2022 12:56:29 PM PST, Linus Torvalds <[email protected]> wrote:
> > >> >> >On Mon, Dec 26, 2022 at 11:52 AM Guenter Roeck <[email protected]> wrote:
> > >> >> >>
> > >> >> >> fs/f2fs/inline.c: In function 'f2fs_move_inline_dirents':
> > >> >> >> include/linux/fortify-string.h:59:33: error: '__builtin_memset' pointer overflow between offset [28, 898293814] and size [-898293787, -1] [-Werror=array-bounds]
> > >> >> >> fs/f2fs/inline.c:430:9: note: in expansion of macro 'memset'
> > >> >> >> 430 | memset(dst.bitmap + src.nr_bitmap, 0, dst.nr_bitmap - src.nr_bitmap);
> > >> >> >> | ^~~~~~
> > >> >> >
> > >> >> >Well, that's unfortunate.
> > >> >>
> > >> >> I'll look into this.
> > >> >>
> > >> >
> > >> >I did some more testing. The problem is seen with gcc 11.3.0, but not with
> > >> >gcc 12.2.0 nor with gcc 10.3.0.
> > >>
> > >> That's what I'd expect: 10 didn't have variable range tracking wired up to -Warray-bounds, 11 does, and we disable -Warray-bounds on 12 because of 3 separate 12-only GCC bugs.
> > >>
> > >> > gcc bug ? Should I switch to gcc 12.2.0 for
> > >> >powerpc when build testing the latest kernel ?
> > >>
> > >> Sure? But that'll just hide it. I suspect GCC has found a way for dst.nr_bitmap to be compile-time 27, so the size is always negative.
> > >>
> > >dst.nr_bitmap is initialized with SIZE_OF_DENTRY_BITMAP,
> > >which is defined as:
> > >
> > >#define NR_DENTRY_IN_BLOCK 214 /* the number of dentry in a block */
> > >#define SIZE_OF_DIR_ENTRY 11 /* by byte */
> > >#define SIZE_OF_DENTRY_BITMAP ((NR_DENTRY_IN_BLOCK + BITS_PER_BYTE - 1) / \
> > > BITS_PER_BYTE)
> > >
> > >((214 + 8 - 1) / 8 = 27, so dst.nr_bitmap is indeed compile-time 27.
> > >
> > >Not sure how would know that src.nr_bitmap can be > 27, though.
> > >Am I missing something ?
> >
> > I think it's saying it can't rule out it being larger? I.e. there is no obvious bounds checking for it. Perhaps:
> >
> > if (src.nr_bitmap > dst.nr_bitmap) {
> > err = -EFSCORRUPTED;
> > goto out;
> > }
> >
>
> After going through all calculations, using maximum values (or minimum
> values where appropriate) everywhere, I calculated that src.nr_bitmap
> is always <= 24. The actual inode is sanity checked in
> fs/f2fs/inode.c:sanity_check_inode().

I also cannot find any case where src.nr_bitmap > 24. May this be a GCC issue?

>
> Also, why is this only seen when I try to build powerpc test images ?
>
> Thanks,
> Guenter

2023-01-28 19:35:10

by Pali Rohár

[permalink] [raw]
Subject: pktcdvd

On Friday 06 January 2023 10:04:47 Jens Axboe wrote:
> On 1/6/23 9:58?AM, Pali Roh?r wrote:
> > On Thursday 05 January 2023 13:33:11 Jens Axboe wrote:
> >> On 1/5/23 1:03?PM, Linus Torvalds wrote:
> >>> So nobody is going to be motivated to do any development in this area,
> >>> and the best we can do is probably to just keep it limping along.
> >>
> >> Indeed...
> > ...
> >>> There's probably other cruft in pktcdvd that could be removed without
> >>> removing the whole driver, but I do get the feeling that it's just
> >>> less pain to keep the status quo, and that there isn't really much
> >>> motivation for anybody to do anything else.
> >>
> >> I'm reluctant to touch it outside of changes that are driven by core
> >> changes, and of course the motivation to remove it was driven by not
> >> wanting to do that either. Any kind of re-architecting of how it works I
> >> would not advocate for. It supposedly works well enough that none of the
> >> (few) users are reporting issues with it, best to just let it remain
> >> like that imho.
> >
> > Yea, I agree. This code is in state when it is _used_ and not developed
> > anymore. Nobody is really motivated to re-architecture or rewrite this
> > code. Such work has big probability to break something which currently
> > works fine. And because lot of users are on stable/LTS kernel versions,
> > it is possible that we would not notice breakage earlier than (lets say)
> > in 5 years.
>
> I did sent out the revert this morning, would be great if you can test
> 6.2-rc3 when it is out. I'm a bit skeptical on the whole devnode front,
> and suspect we might need to convert that to disk_name manipulation.
> Which would be fine, as we can then drop the devnode reinstate revert as
> well going forward. But I need to find a bit of time to look closer at
> this part.

Hello! Sorry for a longer delay. Now I have started testing it with
Linux 6.2.0-rc5. Adding mapping works fine. Reading also works. Mounting
filesystem also works, reading mounted fs also. But after writing some
data to fs and calling sync cause kernel oops. Below is the dmesg log.
"sync" freezes and never finish.

[ 1284.701497] pktcdvd: pktcdvd0: writer mapped to sr0
[ 1321.432589] pktcdvd: pktcdvd0: Fixed packets, 32 blocks, Mode-2 disc
[ 1321.437543] pktcdvd: pktcdvd0: maximum media speed: 10
[ 1321.437546] pktcdvd: pktcdvd0: write speed 10x
[ 1327.098955] pktcdvd: pktcdvd0: 590528kB available on disc
[ 1329.737263] UDF-fs: INFO Mounting volume 'LinuxUDF', timestamp 2023/01/28 19:16 (103c)
[ 1435.627449] ------------[ cut here ]------------
[ 1435.627466] kernel BUG at drivers/block/pktcdvd.c:2434!
[ 1435.627472] invalid opcode: 0000 [#1] PREEMPT SMP PTI
[ 1435.627476] CPU: 3 PID: 9 Comm: kworker/u8:0 Tainted: G S U 6.2.0-rc5 #4
[ 1435.627478] Hardware name: Dell Inc. Latitude E6440/02P3T1, BIOS A05 02/18/2014
[ 1435.627480] Workqueue: writeback wb_workfn (flush-253:0)
[ 1435.627487] RIP: 0010:pkt_submit_bio+0x398/0x430 [pktcdvd]
[ 1435.627494] Code: 55 28 41 89 55 28 41 3b 55 40 7c 07 41 83 7d 44 01 74 7c 4c 89 f7 e8 97 32 e1 e9 48 8b 7c 24 10 e8 8d 32 e1 e9 e9 e6 fe ff ff <0f> 0b 0f 0b 49 8b 3f 48 c7 c1 a0 97 d0 c0 ba 00 0c 00 00 48 89 c6
[ 1435.627496] RSP: 0018:ffffb6a10007b828 EFLAGS: 00010283
[ 1435.627498] RAX: 0000000000000080 RBX: ffff8a4757170800 RCX: 0000000000001800
[ 1435.627500] RDX: 0000000000001604 RSI: 0000000000001680 RDI: 0000000000001603
[ 1435.627501] RBP: ffff8a4750a37480 R08: 0000000000000200 R09: ffffffffffffff80
[ 1435.627502] R10: 0000000000000400 R11: 0000000000000040 R12: ffff8a4721bd0dc0
[ 1435.627504] R13: 0000000000001000 R14: 0000000000000000 R15: ffff8a4739e56000
[ 1435.627505] FS: 0000000000000000(0000) GS:ffff8a4826b80000(0000) knlGS:0000000000000000
[ 1435.627507] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 1435.627508] CR2: 0000560a548f8490 CR3: 0000000067210005 CR4: 00000000001706e0
[ 1435.627510] Call Trace:
[ 1435.627512] <TASK>
[ 1435.627514] ? __mod_memcg_lruvec_state+0x72/0xd0
[ 1435.627519] ? __mod_lruvec_page_state+0x93/0x130
[ 1435.627523] __submit_bio+0x89/0x130
[ 1435.627528] submit_bio_noacct_nocheck+0xe5/0x2b0
[ 1435.627532] __mpage_writepage+0x6f9/0x860
[ 1435.627536] ? __mod_memcg_lruvec_state+0x72/0xd0
[ 1435.627539] ? folio_clear_dirty_for_io+0x13c/0x190
[ 1435.627542] write_cache_pages+0x18a/0x4d0
[ 1435.627555] ? __pfx___mpage_writepage+0x10/0x10
[ 1435.627558] mpage_writepages+0x56/0xb0
[ 1435.627561] ? __pfx_udf_get_block+0x10/0x10 [udf]
[ 1435.627571] do_writepages+0xd5/0x1b0
[ 1435.627573] ? __wb_calc_thresh+0x3a/0x120
[ 1435.627576] __writeback_single_inode+0x41/0x360
[ 1435.627579] writeback_sb_inodes+0x1f0/0x460
[ 1435.627583] __writeback_inodes_wb+0x5f/0xd0
[ 1435.627586] wb_writeback+0x235/0x2d0
[ 1435.627589] wb_workfn+0x311/0x480
[ 1435.627592] ? _raw_spin_unlock+0x15/0x30
[ 1435.627595] ? finish_task_switch+0x91/0x2f0
[ 1435.627600] ? __switch_to+0x106/0x430
[ 1435.627606] process_one_work+0x1b3/0x380
[ 1435.627611] worker_thread+0x30/0x360
[ 1435.627614] ? __pfx_worker_thread+0x10/0x10
[ 1435.627617] kthread+0xe8/0x110
[ 1435.627620] ? __pfx_kthread+0x10/0x10
[ 1435.627623] ret_from_fork+0x2c/0x50
[ 1435.627627] </TASK>
[ 1435.627628] Modules linked in: udf crc_itu_t pktcdvd rfcomm ctr ccm cmac algif_hash bnep binfmt_misc ip6_tables ip6t_rt xt_set xt_multiport xt_recent xt_tcpudp ip_tables xt_conntrack nft_compat x_tables nft_chain_nat nf_nat nf_conntrack nf_defrag_ipv6 nf_defrag_ipv4 nf_tables ip_set_hash_ipport ip_set nfnetlink nls_ascii nls_cp437 vfat fat lp btusb btrtl btbcm btintel btmtk bluetooth uvcvideo jitterentropy_rng videobuf2_vmalloc videobuf2_memops videobuf2_v4l2 drbg videodev ansi_cprng videobuf2_common ecdh_generic ecc mc snd_hda_codec_hdmi amdgpu intel_rapl_msr intel_rapl_common qmi_wwan qcserial gpu_sched cdc_wdm usb_wwan usbnet usbserial mii x86_pkg_temp_thermal intel_powerclamp i915 radeon snd_ctl_led coretemp snd_hda_codec_realtek snd_hda_codec_generic drm_buddy kvm_intel drm_display_helper snd_hda_intel iwldvm cec snd_intel_dspcfg snd_soc_rt5640 snd_intel_sdw_acpi drm_ttm_helper rc_core kvm mac80211 snd_hda_codec snd_soc_rl6231 dell_laptop ttm dell_wmi msr ledtrig_audio irqbypass
[ 1435.627676] libarc4 snd_hda_core ppdev sparse_keymap rapl drm_kms_helper snd_soc_core dell_smbios intel_cstate dell_smm_hwmon snd_hwdep snd_compress joydev evdev iwlwifi iTCO_wdt dcdbas intel_uncore intel_pmc_bxt drm pcspkr efi_pstore serio_raw wmi_bmof iTCO_vendor_support dell_wmi_descriptor lis3lv02d_i2c watchdog parport_pc i2c_algo_bit cfg80211 snd_pcm lis3lv02d at24 snd_timer sg parport snd dell_rbtn soundcore dell_smo8800 rfkill ac button ext4 crc16 mbcache jbd2 btrfs blake2b_generic xor raid6_pq libcrc32c crc32c_generic algif_skcipher af_alg dm_crypt dm_mod hid_cherry hid_generic usbhid hid sd_mod t10_pi crc64_rocksoft_generic sr_mod crc64_rocksoft crc_t10dif cdrom crct10dif_generic crc64 ahci libahci crct10dif_pclmul crct10dif_common crc32_pclmul crc32c_intel libata ghash_clmulni_intel sha512_ssse3 sha512_generic e1000e aesni_intel sdhci_pci ehci_pci xhci_pci ehci_hcd cqhci sdhci xhci_hcd crypto_simd scsi_mod ptp i2c_i801 psmouse cryptd i2c_smbus lpc_ich scsi_common mmc_core
[ 1435.627729] usbcore pps_core usb_common battery video wmi [last unloaded: pktcdvd]
[ 1435.627735] ---[ end trace 0000000000000000 ]---
[ 1435.788193] RIP: 0010:pkt_submit_bio+0x398/0x430 [pktcdvd]
[ 1435.788204] Code: 55 28 41 89 55 28 41 3b 55 40 7c 07 41 83 7d 44 01 74 7c 4c 89 f7 e8 97 32 e1 e9 48 8b 7c 24 10 e8 8d 32 e1 e9 e9 e6 fe ff ff <0f> 0b 0f 0b 49 8b 3f 48 c7 c1 a0 97 d0 c0 ba 00 0c 00 00 48 89 c6
[ 1435.788207] RSP: 0018:ffffb6a10007b828 EFLAGS: 00010283
[ 1435.788210] RAX: 0000000000000080 RBX: ffff8a4757170800 RCX: 0000000000001800
[ 1435.788212] RDX: 0000000000001604 RSI: 0000000000001680 RDI: 0000000000001603
[ 1435.788214] RBP: ffff8a4750a37480 R08: 0000000000000200 R09: ffffffffffffff80
[ 1435.788216] R10: 0000000000000400 R11: 0000000000000040 R12: ffff8a4721bd0dc0
[ 1435.788218] R13: 0000000000001000 R14: 0000000000000000 R15: ffff8a4739e56000
[ 1435.788221] FS: 0000000000000000(0000) GS:ffff8a4826b80000(0000) knlGS:0000000000000000
[ 1435.788223] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 1435.788226] CR2: 0000560a548f8490 CR3: 00000001949f0003 CR4: 00000000001706e0
[ 1435.788230] ------------[ cut here ]------------
[ 1435.788231] WARNING: CPU: 3 PID: 9 at kernel/exit.c:812 do_exit+0x91b/0xbe0
[ 1435.788237] Modules linked in: udf crc_itu_t pktcdvd rfcomm ctr ccm cmac algif_hash bnep binfmt_misc ip6_tables ip6t_rt xt_set xt_multiport xt_recent xt_tcpudp ip_tables xt_conntrack nft_compat x_tables nft_chain_nat nf_nat nf_conntrack nf_defrag_ipv6 nf_defrag_ipv4 nf_tables ip_set_hash_ipport ip_set nfnetlink nls_ascii nls_cp437 vfat fat lp btusb btrtl btbcm btintel btmtk bluetooth uvcvideo jitterentropy_rng videobuf2_vmalloc videobuf2_memops videobuf2_v4l2 drbg videodev ansi_cprng videobuf2_common ecdh_generic ecc mc snd_hda_codec_hdmi amdgpu intel_rapl_msr intel_rapl_common qmi_wwan qcserial gpu_sched cdc_wdm usb_wwan usbnet usbserial mii x86_pkg_temp_thermal intel_powerclamp i915 radeon snd_ctl_led coretemp snd_hda_codec_realtek snd_hda_codec_generic drm_buddy kvm_intel drm_display_helper snd_hda_intel iwldvm cec snd_intel_dspcfg snd_soc_rt5640 snd_intel_sdw_acpi drm_ttm_helper rc_core kvm mac80211 snd_hda_codec snd_soc_rl6231 dell_laptop ttm dell_wmi msr ledtrig_audio irqbypass
[ 1435.788319] libarc4 snd_hda_core ppdev sparse_keymap rapl drm_kms_helper snd_soc_core dell_smbios intel_cstate dell_smm_hwmon snd_hwdep snd_compress joydev evdev iwlwifi iTCO_wdt dcdbas intel_uncore intel_pmc_bxt drm pcspkr efi_pstore serio_raw wmi_bmof iTCO_vendor_support dell_wmi_descriptor lis3lv02d_i2c watchdog parport_pc i2c_algo_bit cfg80211 snd_pcm lis3lv02d at24 snd_timer sg parport snd dell_rbtn soundcore dell_smo8800 rfkill ac button ext4 crc16 mbcache jbd2 btrfs blake2b_generic xor raid6_pq libcrc32c crc32c_generic algif_skcipher af_alg dm_crypt dm_mod hid_cherry hid_generic usbhid hid sd_mod t10_pi crc64_rocksoft_generic sr_mod crc64_rocksoft crc_t10dif cdrom crct10dif_generic crc64 ahci libahci crct10dif_pclmul crct10dif_common crc32_pclmul crc32c_intel libata ghash_clmulni_intel sha512_ssse3 sha512_generic e1000e aesni_intel sdhci_pci ehci_pci xhci_pci ehci_hcd cqhci sdhci xhci_hcd crypto_simd scsi_mod ptp i2c_i801 psmouse cryptd i2c_smbus lpc_ich scsi_common mmc_core
[ 1435.788398] usbcore pps_core usb_common battery video wmi [last unloaded: pktcdvd]
[ 1435.788407] CPU: 3 PID: 9 Comm: kworker/u8:0 Tainted: G S UD 6.2.0-rc5 #4
[ 1435.788410] Hardware name: Dell Inc. Latitude E6440/02P3T1, BIOS A05 02/18/2014
[ 1435.788413] Workqueue: writeback wb_workfn (flush-253:0)
[ 1435.788419] RIP: 0010:do_exit+0x91b/0xbe0
[ 1435.788423] Code: e8 8a 36 a5 00 8b 83 60 13 00 00 65 01 05 51 e9 f7 55 e9 a8 fe ff ff 48 8b bb 98 09 00 00 31 f6 e8 ea d9 ff ff e9 24 fd ff ff <0f> 0b e9 49 f7 ff ff 4c 89 ee bf 05 06 00 00 e8 61 f0 00 00 e9 ea
[ 1435.788426] RSP: 0018:ffffb6a10007bee0 EFLAGS: 00010286
[ 1435.788429] RAX: 0000000000000000 RBX: ffff8a4700aacbc0 RCX: 0000000000000001
[ 1435.788431] RDX: 0000000000000001 RSI: 0000000000002710 RDI: 00000000ffffffff
[ 1435.788433] RBP: ffff8a4700aa4800 R08: 0000000000000000 R09: c0000000ffffefff
[ 1435.788436] R10: 0000000000000001 R11: ffffb6a10007b4e8 R12: ffff8a4700a90840
[ 1435.788438] R13: 000000000000000b R14: 0000000000000000 R15: ffffb6a10007b778
[ 1435.788440] FS: 0000000000000000(0000) GS:ffff8a4826b80000(0000) knlGS:0000000000000000
[ 1435.788443] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 1435.788445] CR2: 0000560a548f8490 CR3: 00000001949f0003 CR4: 00000000001706e0
[ 1435.788447] Call Trace:
[ 1435.788449] <TASK>
[ 1435.788451] ? worker_thread+0x30/0x360
[ 1435.788458] make_task_dead+0x6f/0x80
[ 1435.788462] rewind_stack_and_make_dead+0x17/0x20
[ 1435.788466] RIP: 0000:0x0
[ 1435.788470] Code: Unable to access opcode bytes at 0xffffffffffffffd6.
[ 1435.788471] RSP: 0000:0000000000000000 EFLAGS: 00000000 ORIG_RAX: 0000000000000000
[ 1435.788473] RAX: 0000000000000000 RBX: 0000000000000000 RCX: 0000000000000000
[ 1435.788474] RDX: 0000000000000000 RSI: 0000000000000000 RDI: 0000000000000000
[ 1435.788475] RBP: 0000000000000000 R08: 0000000000000000 R09: 0000000000000000
[ 1435.788476] R10: 0000000000000000 R11: 0000000000000000 R12: 0000000000000000
[ 1435.788477] R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000000000
[ 1435.788479] </TASK>
[ 1435.788480] ---[ end trace 0000000000000000 ]---


2023-01-28 19:44:10

by Linus Torvalds

[permalink] [raw]
Subject: Re: pktcdvd

On Sat, Jan 28, 2023 at 11:35 AM Pali Rohár <[email protected]> wrote:
>
> Hello! Sorry for a longer delay. Now I have started testing it with
> Linux 6.2.0-rc5. Adding mapping works fine. Reading also works. Mounting
> filesystem also works, reading mounted fs also. But after writing some
> data to fs and calling sync cause kernel oops. Below is the dmesg log.
> "sync" freezes and never finish.
>
> [ 1284.701497] pktcdvd: pktcdvd0: writer mapped to sr0
> [ 1321.432589] pktcdvd: pktcdvd0: Fixed packets, 32 blocks, Mode-2 disc
> [ 1321.437543] pktcdvd: pktcdvd0: maximum media speed: 10
> [ 1321.437546] pktcdvd: pktcdvd0: write speed 10x
> [ 1327.098955] pktcdvd: pktcdvd0: 590528kB available on disc
> [ 1329.737263] UDF-fs: INFO Mounting volume 'LinuxUDF', timestamp 2023/01/28 19:16 (103c)
> [ 1435.627449] ------------[ cut here ]------------
> [ 1435.627466] kernel BUG at drivers/block/pktcdvd.c:2434!

Well, this is very much an example of one of the BUG_ON() cases I
absolutely hate - not only did it cause the traceback (which can be
interesting), it also effectively killed the machine in the process.

So that BUG_ON() most definitely shouldn't be a BUG_ON().

Turning it into a WARN_ON() (possibly even of the "ONCE" variety)
together with then finishing the IO with a bio_io_error() would have
been a better option for debugging.

Of course, the real fix is to fix whatever causes it, and I don't know
what that is.

So I'm just piping up to once more highlight my hatred of using
BUG_ON() for "this shouldn't happen" debug code. It's basically never
the right thing to do unless you are in core code that would kill the
machine anyway.

Linus

2023-01-29 21:54:17

by Jens Axboe

[permalink] [raw]
Subject: Re: pktcdvd

On 1/28/23 12:43 PM, Linus Torvalds wrote:
> On Sat, Jan 28, 2023 at 11:35 AM Pali Rohár <[email protected]> wrote:
>>
>> Hello! Sorry for a longer delay. Now I have started testing it with
>> Linux 6.2.0-rc5. Adding mapping works fine. Reading also works. Mounting
>> filesystem also works, reading mounted fs also. But after writing some
>> data to fs and calling sync cause kernel oops. Below is the dmesg log.
>> "sync" freezes and never finish.
>>
>> [ 1284.701497] pktcdvd: pktcdvd0: writer mapped to sr0
>> [ 1321.432589] pktcdvd: pktcdvd0: Fixed packets, 32 blocks, Mode-2 disc
>> [ 1321.437543] pktcdvd: pktcdvd0: maximum media speed: 10
>> [ 1321.437546] pktcdvd: pktcdvd0: write speed 10x
>> [ 1327.098955] pktcdvd: pktcdvd0: 590528kB available on disc
>> [ 1329.737263] UDF-fs: INFO Mounting volume 'LinuxUDF', timestamp 2023/01/28 19:16 (103c)
>> [ 1435.627449] ------------[ cut here ]------------
>> [ 1435.627466] kernel BUG at drivers/block/pktcdvd.c:2434!
>
> Well, this is very much an example of one of the BUG_ON() cases I
> absolutely hate - not only did it cause the traceback (which can be
> interesting), it also effectively killed the machine in the process.
>
> So that BUG_ON() most definitely shouldn't be a BUG_ON().
>
> Turning it into a WARN_ON() (possibly even of the "ONCE" variety)
> together with then finishing the IO with a bio_io_error() would have
> been a better option for debugging.
>
> Of course, the real fix is to fix whatever causes it, and I don't know
> what that is.
>
> So I'm just piping up to once more highlight my hatred of using
> BUG_ON() for "this shouldn't happen" debug code. It's basically never
> the right thing to do unless you are in core code that would kill the
> machine anyway.

To be fair, this code is 20 years old... It should not be using
BUG_ON(), totally agree, but that it was quite common back in those
days.

--
Jens Axboe



2023-01-29 21:55:40

by Jens Axboe

[permalink] [raw]
Subject: Re: pktcdvd

On 1/28/23 12:34?PM, Pali Roh?r wrote:
> On Friday 06 January 2023 10:04:47 Jens Axboe wrote:
>> On 1/6/23 9:58?AM, Pali Roh?r wrote:
>>> On Thursday 05 January 2023 13:33:11 Jens Axboe wrote:
>>>> On 1/5/23 1:03?PM, Linus Torvalds wrote:
>>>>> So nobody is going to be motivated to do any development in this area,
>>>>> and the best we can do is probably to just keep it limping along.
>>>>
>>>> Indeed...
>>> ...
>>>>> There's probably other cruft in pktcdvd that could be removed without
>>>>> removing the whole driver, but I do get the feeling that it's just
>>>>> less pain to keep the status quo, and that there isn't really much
>>>>> motivation for anybody to do anything else.
>>>>
>>>> I'm reluctant to touch it outside of changes that are driven by core
>>>> changes, and of course the motivation to remove it was driven by not
>>>> wanting to do that either. Any kind of re-architecting of how it works I
>>>> would not advocate for. It supposedly works well enough that none of the
>>>> (few) users are reporting issues with it, best to just let it remain
>>>> like that imho.
>>>
>>> Yea, I agree. This code is in state when it is _used_ and not developed
>>> anymore. Nobody is really motivated to re-architecture or rewrite this
>>> code. Such work has big probability to break something which currently
>>> works fine. And because lot of users are on stable/LTS kernel versions,
>>> it is possible that we would not notice breakage earlier than (lets say)
>>> in 5 years.
>>
>> I did sent out the revert this morning, would be great if you can test
>> 6.2-rc3 when it is out. I'm a bit skeptical on the whole devnode front,
>> and suspect we might need to convert that to disk_name manipulation.
>> Which would be fine, as we can then drop the devnode reinstate revert as
>> well going forward. But I need to find a bit of time to look closer at
>> this part.
>
> Hello! Sorry for a longer delay. Now I have started testing it with
> Linux 6.2.0-rc5. Adding mapping works fine. Reading also works. Mounting
> filesystem also works, reading mounted fs also. But after writing some
> data to fs and calling sync cause kernel oops. Below is the dmesg log.
> "sync" freezes and never finish.
>
> [ 1284.701497] pktcdvd: pktcdvd0: writer mapped to sr0
> [ 1321.432589] pktcdvd: pktcdvd0: Fixed packets, 32 blocks, Mode-2 disc
> [ 1321.437543] pktcdvd: pktcdvd0: maximum media speed: 10
> [ 1321.437546] pktcdvd: pktcdvd0: write speed 10x
> [ 1327.098955] pktcdvd: pktcdvd0: 590528kB available on disc
> [ 1329.737263] UDF-fs: INFO Mounting volume 'LinuxUDF', timestamp 2023/01/28 19:16 (103c)
> [ 1435.627449] ------------[ cut here ]------------
> [ 1435.627466] kernel BUG at drivers/block/pktcdvd.c:2434!
> [ 1435.627472] invalid opcode: 0000 [#1] PREEMPT SMP PTI
> [ 1435.627476] CPU: 3 PID: 9 Comm: kworker/u8:0 Tainted: G S U 6.2.0-rc5 #4
> [ 1435.627478] Hardware name: Dell Inc. Latitude E6440/02P3T1, BIOS A05 02/18/2014
> [ 1435.627480] Workqueue: writeback wb_workfn (flush-253:0)
> [ 1435.627487] RIP: 0010:pkt_submit_bio+0x398/0x430 [pktcdvd]
> [ 1435.627494] Code: 55 28 41 89 55 28 41 3b 55 40 7c 07 41 83 7d 44 01 74 7c 4c 89 f7 e8 97 32 e1 e9 48 8b 7c 24 10 e8 8d 32 e1 e9 e9 e6 fe ff ff <0f> 0b 0f 0b 49 8b 3f 48 c7 c1 a0 97 d0 c0 ba 00 0c 00 00 48 89 c6
> [ 1435.627496] RSP: 0018:ffffb6a10007b828 EFLAGS: 00010283
> [ 1435.627498] RAX: 0000000000000080 RBX: ffff8a4757170800 RCX: 0000000000001800
> [ 1435.627500] RDX: 0000000000001604 RSI: 0000000000001680 RDI: 0000000000001603
> [ 1435.627501] RBP: ffff8a4750a37480 R08: 0000000000000200 R09: ffffffffffffff80
> [ 1435.627502] R10: 0000000000000400 R11: 0000000000000040 R12: ffff8a4721bd0dc0
> [ 1435.627504] R13: 0000000000001000 R14: 0000000000000000 R15: ffff8a4739e56000
> [ 1435.627505] FS: 0000000000000000(0000) GS:ffff8a4826b80000(0000) knlGS:0000000000000000
> [ 1435.627507] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [ 1435.627508] CR2: 0000560a548f8490 CR3: 0000000067210005 CR4: 00000000001706e0
> [ 1435.627510] Call Trace:
> [ 1435.627512] <TASK>
> [ 1435.627514] ? __mod_memcg_lruvec_state+0x72/0xd0
> [ 1435.627519] ? __mod_lruvec_page_state+0x93/0x130
> [ 1435.627523] __submit_bio+0x89/0x130
> [ 1435.627528] submit_bio_noacct_nocheck+0xe5/0x2b0
> [ 1435.627532] __mpage_writepage+0x6f9/0x860
> [ 1435.627536] ? __mod_memcg_lruvec_state+0x72/0xd0
> [ 1435.627539] ? folio_clear_dirty_for_io+0x13c/0x190
> [ 1435.627542] write_cache_pages+0x18a/0x4d0
> [ 1435.627555] ? __pfx___mpage_writepage+0x10/0x10
> [ 1435.627558] mpage_writepages+0x56/0xb0
> [ 1435.627561] ? __pfx_udf_get_block+0x10/0x10 [udf]
> [ 1435.627571] do_writepages+0xd5/0x1b0
> [ 1435.627573] ? __wb_calc_thresh+0x3a/0x120
> [ 1435.627576] __writeback_single_inode+0x41/0x360
> [ 1435.627579] writeback_sb_inodes+0x1f0/0x460
> [ 1435.627583] __writeback_inodes_wb+0x5f/0xd0
> [ 1435.627586] wb_writeback+0x235/0x2d0
> [ 1435.627589] wb_workfn+0x311/0x480
> [ 1435.627592] ? _raw_spin_unlock+0x15/0x30
> [ 1435.627595] ? finish_task_switch+0x91/0x2f0
> [ 1435.627600] ? __switch_to+0x106/0x430
> [ 1435.627606] process_one_work+0x1b3/0x380
> [ 1435.627611] worker_thread+0x30/0x360
> [ 1435.627614] ? __pfx_worker_thread+0x10/0x10
> [ 1435.627617] kthread+0xe8/0x110
> [ 1435.627620] ? __pfx_kthread+0x10/0x10
> [ 1435.627623] ret_from_fork+0x2c/0x50
> [ 1435.627627] </TASK>
> [ 1435.627628] Modules linked in: udf crc_itu_t pktcdvd rfcomm ctr ccm cmac algif_hash bnep binfmt_misc ip6_tables ip6t_rt xt_set xt_multiport xt_recent xt_tcpudp ip_tables xt_conntrack nft_compat x_tables nft_chain_nat nf_nat nf_conntrack nf_defrag_ipv6 nf_defrag_ipv4 nf_tables ip_set_hash_ipport ip_set nfnetlink nls_ascii nls_cp437 vfat fat lp btusb btrtl btbcm btintel btmtk bluetooth uvcvideo jitterentropy_rng videobuf2_vmalloc videobuf2_memops videobuf2_v4l2 drbg videodev ansi_cprng videobuf2_common ecdh_generic ecc mc snd_hda_codec_hdmi amdgpu intel_rapl_msr intel_rapl_common qmi_wwan qcserial gpu_sched cdc_wdm usb_wwan usbnet usbserial mii x86_pkg_temp_thermal intel_powerclamp i915 radeon snd_ctl_led coretemp snd_hda_codec_realtek snd_hda_codec_generic drm_buddy kvm_intel drm_display_helper snd_hda_intel iwldvm cec snd_intel_dspcfg snd_soc_rt5640 snd_intel_sdw_acpi drm_ttm_helper rc_core kvm mac80211 snd_hda_codec snd_soc_rl6231 dell_laptop ttm dell_wmi msr ledtrig_audio irqbypass
> [ 1435.627676] libarc4 snd_hda_core ppdev sparse_keymap rapl drm_kms_helper snd_soc_core dell_smbios intel_cstate dell_smm_hwmon snd_hwdep snd_compress joydev evdev iwlwifi iTCO_wdt dcdbas intel_uncore intel_pmc_bxt drm pcspkr efi_pstore serio_raw wmi_bmof iTCO_vendor_support dell_wmi_descriptor lis3lv02d_i2c watchdog parport_pc i2c_algo_bit cfg80211 snd_pcm lis3lv02d at24 snd_timer sg parport snd dell_rbtn soundcore dell_smo8800 rfkill ac button ext4 crc16 mbcache jbd2 btrfs blake2b_generic xor raid6_pq libcrc32c crc32c_generic algif_skcipher af_alg dm_crypt dm_mod hid_cherry hid_generic usbhid hid sd_mod t10_pi crc64_rocksoft_generic sr_mod crc64_rocksoft crc_t10dif cdrom crct10dif_generic crc64 ahci libahci crct10dif_pclmul crct10dif_common crc32_pclmul crc32c_intel libata ghash_clmulni_intel sha512_ssse3 sha512_generic e1000e aesni_intel sdhci_pci ehci_pci xhci_pci ehci_hcd cqhci sdhci xhci_hcd crypto_simd scsi_mod ptp i2c_i801 psmouse cryptd i2c_smbus lpc_ich scsi_common mmc_core
> [ 1435.627729] usbcore pps_core usb_common battery video wmi [last unloaded: pktcdvd]
> [ 1435.627735] ---[ end trace 0000000000000000 ]---
> [ 1435.788193] RIP: 0010:pkt_submit_bio+0x398/0x430 [pktcdvd]
> [ 1435.788204] Code: 55 28 41 89 55 28 41 3b 55 40 7c 07 41 83 7d 44 01 74 7c 4c 89 f7 e8 97 32 e1 e9 48 8b 7c 24 10 e8 8d 32 e1 e9 e9 e6 fe ff ff <0f> 0b 0f 0b 49 8b 3f 48 c7 c1 a0 97 d0 c0 ba 00 0c 00 00 48 89 c6
> [ 1435.788207] RSP: 0018:ffffb6a10007b828 EFLAGS: 00010283
> [ 1435.788210] RAX: 0000000000000080 RBX: ffff8a4757170800 RCX: 0000000000001800
> [ 1435.788212] RDX: 0000000000001604 RSI: 0000000000001680 RDI: 0000000000001603
> [ 1435.788214] RBP: ffff8a4750a37480 R08: 0000000000000200 R09: ffffffffffffff80
> [ 1435.788216] R10: 0000000000000400 R11: 0000000000000040 R12: ffff8a4721bd0dc0
> [ 1435.788218] R13: 0000000000001000 R14: 0000000000000000 R15: ffff8a4739e56000
> [ 1435.788221] FS: 0000000000000000(0000) GS:ffff8a4826b80000(0000) knlGS:0000000000000000
> [ 1435.788223] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [ 1435.788226] CR2: 0000560a548f8490 CR3: 00000001949f0003 CR4: 00000000001706e0
> [ 1435.788230] ------------[ cut here ]------------
> [ 1435.788231] WARNING: CPU: 3 PID: 9 at kernel/exit.c:812 do_exit+0x91b/0xbe0
> [ 1435.788237] Modules linked in: udf crc_itu_t pktcdvd rfcomm ctr ccm cmac algif_hash bnep binfmt_misc ip6_tables ip6t_rt xt_set xt_multiport xt_recent xt_tcpudp ip_tables xt_conntrack nft_compat x_tables nft_chain_nat nf_nat nf_conntrack nf_defrag_ipv6 nf_defrag_ipv4 nf_tables ip_set_hash_ipport ip_set nfnetlink nls_ascii nls_cp437 vfat fat lp btusb btrtl btbcm btintel btmtk bluetooth uvcvideo jitterentropy_rng videobuf2_vmalloc videobuf2_memops videobuf2_v4l2 drbg videodev ansi_cprng videobuf2_common ecdh_generic ecc mc snd_hda_codec_hdmi amdgpu intel_rapl_msr intel_rapl_common qmi_wwan qcserial gpu_sched cdc_wdm usb_wwan usbnet usbserial mii x86_pkg_temp_thermal intel_powerclamp i915 radeon snd_ctl_led coretemp snd_hda_codec_realtek snd_hda_codec_generic drm_buddy kvm_intel drm_display_helper snd_hda_intel iwldvm cec snd_intel_dspcfg snd_soc_rt5640 snd_intel_sdw_acpi drm_ttm_helper rc_core kvm mac80211 snd_hda_codec snd_soc_rl6231 dell_laptop ttm dell_wmi msr ledtrig_audio irqbypass
> [ 1435.788319] libarc4 snd_hda_core ppdev sparse_keymap rapl drm_kms_helper snd_soc_core dell_smbios intel_cstate dell_smm_hwmon snd_hwdep snd_compress joydev evdev iwlwifi iTCO_wdt dcdbas intel_uncore intel_pmc_bxt drm pcspkr efi_pstore serio_raw wmi_bmof iTCO_vendor_support dell_wmi_descriptor lis3lv02d_i2c watchdog parport_pc i2c_algo_bit cfg80211 snd_pcm lis3lv02d at24 snd_timer sg parport snd dell_rbtn soundcore dell_smo8800 rfkill ac button ext4 crc16 mbcache jbd2 btrfs blake2b_generic xor raid6_pq libcrc32c crc32c_generic algif_skcipher af_alg dm_crypt dm_mod hid_cherry hid_generic usbhid hid sd_mod t10_pi crc64_rocksoft_generic sr_mod crc64_rocksoft crc_t10dif cdrom crct10dif_generic crc64 ahci libahci crct10dif_pclmul crct10dif_common crc32_pclmul crc32c_intel libata ghash_clmulni_intel sha512_ssse3 sha512_generic e1000e aesni_intel sdhci_pci ehci_pci xhci_pci ehci_hcd cqhci sdhci xhci_hcd crypto_simd scsi_mod ptp i2c_i801 psmouse cryptd i2c_smbus lpc_ich scsi_common mmc_core
> [ 1435.788398] usbcore pps_core usb_common battery video wmi [last unloaded: pktcdvd]
> [ 1435.788407] CPU: 3 PID: 9 Comm: kworker/u8:0 Tainted: G S UD 6.2.0-rc5 #4
> [ 1435.788410] Hardware name: Dell Inc. Latitude E6440/02P3T1, BIOS A05 02/18/2014
> [ 1435.788413] Workqueue: writeback wb_workfn (flush-253:0)
> [ 1435.788419] RIP: 0010:do_exit+0x91b/0xbe0
> [ 1435.788423] Code: e8 8a 36 a5 00 8b 83 60 13 00 00 65 01 05 51 e9 f7 55 e9 a8 fe ff ff 48 8b bb 98 09 00 00 31 f6 e8 ea d9 ff ff e9 24 fd ff ff <0f> 0b e9 49 f7 ff ff 4c 89 ee bf 05 06 00 00 e8 61 f0 00 00 e9 ea
> [ 1435.788426] RSP: 0018:ffffb6a10007bee0 EFLAGS: 00010286
> [ 1435.788429] RAX: 0000000000000000 RBX: ffff8a4700aacbc0 RCX: 0000000000000001
> [ 1435.788431] RDX: 0000000000000001 RSI: 0000000000002710 RDI: 00000000ffffffff
> [ 1435.788433] RBP: ffff8a4700aa4800 R08: 0000000000000000 R09: c0000000ffffefff
> [ 1435.788436] R10: 0000000000000001 R11: ffffb6a10007b4e8 R12: ffff8a4700a90840
> [ 1435.788438] R13: 000000000000000b R14: 0000000000000000 R15: ffffb6a10007b778
> [ 1435.788440] FS: 0000000000000000(0000) GS:ffff8a4826b80000(0000) knlGS:0000000000000000
> [ 1435.788443] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [ 1435.788445] CR2: 0000560a548f8490 CR3: 00000001949f0003 CR4: 00000000001706e0
> [ 1435.788447] Call Trace:
> [ 1435.788449] <TASK>
> [ 1435.788451] ? worker_thread+0x30/0x360
> [ 1435.788458] make_task_dead+0x6f/0x80
> [ 1435.788462] rewind_stack_and_make_dead+0x17/0x20
> [ 1435.788466] RIP: 0000:0x0
> [ 1435.788470] Code: Unable to access opcode bytes at 0xffffffffffffffd6.
> [ 1435.788471] RSP: 0000:0000000000000000 EFLAGS: 00000000 ORIG_RAX: 0000000000000000
> [ 1435.788473] RAX: 0000000000000000 RBX: 0000000000000000 RCX: 0000000000000000
> [ 1435.788474] RDX: 0000000000000000 RSI: 0000000000000000 RDI: 0000000000000000
> [ 1435.788475] RBP: 0000000000000000 R08: 0000000000000000 R09: 0000000000000000
> [ 1435.788476] R10: 0000000000000000 R11: 0000000000000000 R12: 0000000000000000
> [ 1435.788477] R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000000000
> [ 1435.788479] </TASK>
> [ 1435.788480] ---[ end trace 0000000000000000 ]---

This does not look like a new issue (eg related to this series), and
this is exactly one of the reasons we wanted to get rid of this code -
basically nobody tests it, as nobody has the ability to. That means it's
very time consuming to debug issues with it.

What is the newest kernel you've run the pktcdvd driver on?

--
Jens Axboe


2023-01-29 22:21:40

by Pali Rohár

[permalink] [raw]
Subject: Re: pktcdvd

On Sunday 29 January 2023 14:55:27 Jens Axboe wrote:
> This does not look like a new issue (eg related to this series), and
> this is exactly one of the reasons we wanted to get rid of this code -
> basically nobody tests it, as nobody has the ability to. That means it's
> very time consuming to debug issues with it.

I understand. I could try to look at it. The main issue for me is that I
have not looked at this low level block device kernel area and I do not
understand what is the code around doing...

> What is the newest kernel you've run the pktcdvd driver on?

4.19 from Debian 10. I'm not sure if I used newer version as most stuff
is still on Debian 10 as I have not find a time to do upgrade of
everything.

2023-01-29 22:34:12

by Jens Axboe

[permalink] [raw]
Subject: Re: pktcdvd

On 1/29/23 3:21 PM, Pali Rohár wrote:
> On Sunday 29 January 2023 14:55:27 Jens Axboe wrote:
>> This does not look like a new issue (eg related to this series), and
>> this is exactly one of the reasons we wanted to get rid of this code -
>> basically nobody tests it, as nobody has the ability to. That means it's
>> very time consuming to debug issues with it.
>
> I understand. I could try to look at it. The main issue for me is that I
> have not looked at this low level block device kernel area and I do not
> understand what is the code around doing...

I'll be happy to answer questions if you want to dive in... Looking at
the oops, it's clear that a bio arrived (or was split) that didn't end
up on a packet size alignment. We didn't error on the full size
alignment, so the total size of the write is a multiple of the packet
size. The bio_split() is pretty straightforward, so perhaps the starting
sector wasn't a multiple of the packet size to begin with?

>> What is the newest kernel you've run the pktcdvd driver on?
>
> 4.19 from Debian 10. I'm not sure if I used newer version as most stuff
> is still on Debian 10 as I have not find a time to do upgrade of
> everything.

Ok, yeah that's pretty old...

--
Jens Axboe