Linux 6.2-rc1

Linux 6.2-rc1

[Linus Torvalds]

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

Re: Linux 6.2-rc1

[Guenter Roeck]

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.

Re: Linux 6.2-rc1

[Linus Torvalds]

On Mon, Dec 26, 2022 at 11:52 AM Guenter Roeck <linux@roeck-us.net> 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

Re: Linux 6.2-rc1

[Kees Cook]

On December 26, 2022 12:56:29 PM PST, Linus Torvalds <torvalds@linux-foundation.org> wrote:
>On Mon, Dec 26, 2022 at 11:52 AM Guenter Roeck <linux@roeck-us.net> 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

Re: Linux 6.2-rc1

[Guenter Roeck]

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

Re: Linux 6.2-rc1

[Guenter Roeck]

On Mon, Dec 26, 2022 at 01:03:59PM -0800, Kees Cook wrote:
> On December 26, 2022 12:56:29 PM PST, Linus Torvalds <torvalds@linux-foundation.org> wrote:
> >On Mon, Dec 26, 2022 at 11:52 AM Guenter Roeck <linux@roeck-us.net> 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

Re: Linux 6.2-rc1

[Kees Cook]

On December 26, 2022 4:29:41 PM PST, Guenter Roeck <linux@roeck-us.net> 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 <torvalds@linux-foundation.org> wrote:
>> >On Mon, Dec 26, 2022 at 11:52 AM Guenter Roeck <linux@roeck-us.net> 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

Re: Linux 6.2-rc1

[Guenter Roeck]

On Mon, Dec 26, 2022 at 05:32:28PM -0800, Kees Cook wrote:
> On December 26, 2022 4:29:41 PM PST, Guenter Roeck <linux@roeck-us.net> 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 <torvalds@linux-foundation.org> wrote:
> >> >On Mon, Dec 26, 2022 at 11:52 AM Guenter Roeck <linux@roeck-us.net> 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

Re: Linux 6.2-rc1

[Kees Cook]

On December 26, 2022 9:52:12 PM PST, Guenter Roeck <linux@roeck-us.net> 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 <linux@roeck-us.net> 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 <torvalds@linux-foundation.org> wrote:
>> >> >On Mon, Dec 26, 2022 at 11:52 AM Guenter Roeck <linux@roeck-us.net> 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

Re: Linux 6.2-rc1

[Guenter Roeck]

On Tue, Dec 27, 2022 at 07:40:30PM -0800, Kees Cook wrote:
> On December 26, 2022 9:52:12 PM PST, Guenter Roeck <linux@roeck-us.net> 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 <linux@roeck-us.net> 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 <torvalds@linux-foundation.org> wrote:
> >> >> >On Mon, Dec 26, 2022 at 11:52 AM Guenter Roeck <linux@roeck-us.net> 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

Re: Linux 6.2-rc1

[Jaegeuk Kim]

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 <linux@roeck-us.net> 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 <linux@roeck-us.net> 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 <torvalds@linux-foundation.org> wrote:
> > >> >> >On Mon, Dec 26, 2022 at 11:52 AM Guenter Roeck <linux@roeck-us.net> 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

Re: Linux 6.2-rc1

[Vlastimil Babka]

On 12/26/22 21:56, Linus Torvalds wrote:
> On Mon, Dec 26, 2022 at 11:52 AM Guenter Roeck <linux@roeck-us.net> 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?
> 

Re: Linux 6.2-rc1

[Max Filippov]

On Mon, Dec 26, 2022 at 12:44 PM Guenter Roeck <linux@roeck-us.net> 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/20221223074238.4092772-1-jcmvbkbc@gmail.com/
I suspect that previously KCSAN was disabled in allmodconfig builds for xtensa.

-- 
Thanks.
-- Max

Re: Linux 6.2-rc1

[Guenter Roeck]

On Mon, Dec 26, 2022 at 01:10:40PM -0800, Max Filippov wrote:
> On Mon, Dec 26, 2022 at 12:44 PM Guenter Roeck <linux@roeck-us.net> 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/20221223074238.4092772-1-jcmvbkbc@gmail.com/
> I suspect that previously KCSAN was disabled in allmodconfig builds for xtensa.
> 

Correect.

Thanks,
Guenter

Build regressions/improvements in v6.2-rc1

[Geert Uytterhoeven]

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 -- geert@linux-m68k.org

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

Re: Build regressions/improvements in v6.2-rc1

[Geert Uytterhoeven]

[-- Attachment #1: Type: text/plain, Size: 3959 bytes --]

On Tue, 27 Dec 2022, Geert Uytterhoeven wrote:
> 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

amd-gfx@lists.freedesktop.org
linux-arm-kernel@lists.infradead.org
linux-media@vger.kernel.org
linux-wireless@vger.kernel.org
linux-mips@vger.kernel.org
linux-sh@vger.kernel.org
linux-f2fs-devel@lists.sourceforge.net
linuxppc-dev@lists.ozlabs.org
kasan-dev@googlegroups.com
linux-xtensa@linux-xtensa.org

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

arm64-gcc5/arm64-allmodconfig

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

m68k-gcc8/m68k-allmodconfig
See also https://lore.kernel.org/all/CAMuHMdWpPX2mpqFEWjjbjsQvDBQOXyjjdpKnQu9qURAuVZXmMw@mail.gmail.com

   + /kisskb/src/include/linux/bitfield.h: error: call to '__field_overflow' declared with attribute error: value doesn't fit into mask:  => 151:3

In function 'u32_encode_bits',
     inlined from 'ieee80211_mlo_multicast_tx' at /kisskb/src/net/mac80211/tx.c:4435:17,
     inlined from 'ieee80211_subif_start_xmit' at /kisskb/src/net/mac80211/tx.c:4483:3:

mipsel-gcc5/mips-allmodconfig

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

In function 'follow_pmd_mask',
     inlined from 'follow_pud_mask' at /kisskb/src/mm/gup.c:735:9,
     inlined from 'follow_p4d_mask' at /kisskb/src/mm/gup.c:752:9,
     inlined from 'follow_page_mask' at /kisskb/src/mm/gup.c:809:9:

sh4-gcc11/sh-defconfig (Günter wondered if pmd_t should use union)

   + /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/arch/s390/kernel/setup.c: In function 'setup_lowcore_dat_on':
s390x-gcc11/s390-all{mod,yes}config

   + /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/fs/f2fs/inline.c: In function 'f2fs_move_inline_dirents':

powerpc-gcc11/ppc64_book3e_allmodconfig
powerpc-gcc11/powerpc-all{mod,yes}config

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

xtensa-gcc11/xtensa-allmodconfig (patch available)

   + {standard input}: Error: unknown pseudo-op: `.cfi_def_c':  => 1718

sh4-gcc11/sh-allmodconfig (ICE = internal compiler error)

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

Gr{oetje,eeting}s,

 						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

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

Re: Build regressions/improvements in v6.2-rc1

[Rob Landley]

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

Re: Build regressions/improvements in v6.2-rc1

[Geert Uytterhoeven]

Hi Rob,

On Sun, Jan 1, 2023 at 2:22 AM Rob Landley <rob@landley.net> 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 -- geert@linux-m68k.org

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

Re: Build regressions/improvements in v6.2-rc1

[Michael Ellerman]

Geert Uytterhoeven <geert@linux-m68k.org> writes:
> Hi Rob,
>
> On Sun, Jan 1, 2023 at 2:22 AM Rob Landley <rob@landley.net> 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

Re: Build regressions/improvements in v6.2-rc1

[John Paul Adrian Glaubitz]

Hi Geert!

On 12/27/22 09:35, Geert Uytterhoeven wrote:
>    + /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
> 
> In function 'follow_pmd_mask',
>      inlined from 'follow_pud_mask' at /kisskb/src/mm/gup.c:735:9,
>      inlined from 'follow_p4d_mask' at /kisskb/src/mm/gup.c:752:9,
>      inlined from 'follow_page_mask' at /kisskb/src/mm/gup.c:809:9:
> 
> sh4-gcc11/sh-defconfig (Günter wondered if pmd_t should use union)

I'm seeing this, too. Also for sh7785lcr_defconfig.

> sh4-gcc11/sh-allmodconfig (ICE = internal compiler error)

I'm not seeing this one, but I am getting this one instead:

In file included from ./arch/sh/include/asm/hw_irq.h:6,
                  from ./include/linux/irq.h:596,
                  from ./include/asm-generic/hardirq.h:17,
                  from ./arch/sh/include/asm/hardirq.h:9,
                  from ./include/linux/hardirq.h:11,
                  from ./include/linux/interrupt.h:11,
                  from ./include/linux/serial_core.h:13,
                  from ./include/linux/serial_sci.h:6,
                  from arch/sh/kernel/cpu/sh2/setup-sh7619.c:11:
./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)
       |                                                               ^
./include/linux/sh_intc.h:105:31: note: in expansion of macro '_INTC_ARRAY'
   105 |         _INTC_ARRAY(vectors), _INTC_ARRAY(groups),      \
       |                               ^~~~~~~~~~~

Adrian

-- 
  .''`.  John Paul Adrian Glaubitz
: :' :  Debian Developer
`. `'   Physicist
   `-    GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913

Re: Build regressions/improvements in v6.2-rc1

[Geert Uytterhoeven]

Hi John,

On Fri, Jan 6, 2023 at 4:10 PM John Paul Adrian Glaubitz
<glaubitz@physik.fu-berlin.de> wrote:
> On 12/27/22 09:35, Geert Uytterhoeven wrote:
> >    + /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
> >
> > In function 'follow_pmd_mask',
> >      inlined from 'follow_pud_mask' at /kisskb/src/mm/gup.c:735:9,
> >      inlined from 'follow_p4d_mask' at /kisskb/src/mm/gup.c:752:9,
> >      inlined from 'follow_page_mask' at /kisskb/src/mm/gup.c:809:9:
> >
> > sh4-gcc11/sh-defconfig (Günter wondered if pmd_t should use union)
>
> I'm seeing this, too. Also for sh7785lcr_defconfig.
>
> > sh4-gcc11/sh-allmodconfig (ICE = internal compiler error)
>
> I'm not seeing this one, but I am getting this one instead:
>
> In file included from ./arch/sh/include/asm/hw_irq.h:6,
>                   from ./include/linux/irq.h:596,
>                   from ./include/asm-generic/hardirq.h:17,
>                   from ./arch/sh/include/asm/hardirq.h:9,
>                   from ./include/linux/hardirq.h:11,
>                   from ./include/linux/interrupt.h:11,
>                   from ./include/linux/serial_core.h:13,
>                   from ./include/linux/serial_sci.h:6,
>                   from arch/sh/kernel/cpu/sh2/setup-sh7619.c:11:
> ./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)
>        |                                                               ^
> ./include/linux/sh_intc.h:105:31: note: in expansion of macro '_INTC_ARRAY'
>    105 |         _INTC_ARRAY(vectors), _INTC_ARRAY(groups),      \
>        |                               ^~~~~~~~~~~

The easiest fix for the latter is to disable CONFIG_WERROR.
Unfortunately I don't know a simple solution to get rid of the warning.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

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

Re: Build regressions/improvements in v6.2-rc1

[Geert Uytterhoeven]

On Fri, Jan 6, 2023 at 4:17 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>
> Hi John,

Bummer, "Hi Adrian", ofc ;-)

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

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

Calculating array sizes in C - was: Re: Build regressions/improvements in v6.2-rc1

[John Paul Adrian Glaubitz]

Hi Geert!

On 1/6/23 16:17, Geert Uytterhoeven wrote:
>> I'm not seeing this one, but I am getting this one instead:
>>
>> In file included from ./arch/sh/include/asm/hw_irq.h:6,
>>                    from ./include/linux/irq.h:596,
>>                    from ./include/asm-generic/hardirq.h:17,
>>                    from ./arch/sh/include/asm/hardirq.h:9,
>>                    from ./include/linux/hardirq.h:11,
>>                    from ./include/linux/interrupt.h:11,
>>                    from ./include/linux/serial_core.h:13,
>>                    from ./include/linux/serial_sci.h:6,
>>                    from arch/sh/kernel/cpu/sh2/setup-sh7619.c:11:
>> ./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)
>>         |                                                               ^
>> ./include/linux/sh_intc.h:105:31: note: in expansion of macro '_INTC_ARRAY'
>>     105 |         _INTC_ARRAY(vectors), _INTC_ARRAY(groups),      \
>>         |                               ^~~~~~~~~~~
> 
> The easiest fix for the latter is to disable CONFIG_WERROR.
> Unfortunately I don't know a simple solution to get rid of the warning.

I did some research and it seems that what the macro _INT_ARRAY() does with "sizeof(a)/sizeof(*a)"
is a commonly used way to calculate array sizes and the kernel has even its own macro for that
called ARRAY_SIZE() which Linus asks people to use here [1].

So, I replaced _INTC_ARRAY() with ARRAY_SIZE() (see below), however the kernel's own ARRAY_SIZE()
macro triggers the same compiler warning. I'm CC'ing Michael Karcher who has more knowledge on
writing proper C code than me and maybe an idea how to fix this warning.

Thanks,
Adrian

> [1] https://lkml.org/lkml/2015/9/3/428

diff --git a/include/linux/sh_intc.h b/include/linux/sh_intc.h
index c255273b0281..07a187686a84 100644
--- a/include/linux/sh_intc.h
+++ b/include/linux/sh_intc.h
@@ -97,14 +97,12 @@ struct intc_hw_desc {
         unsigned int nr_subgroups;
  };
  
-#define _INTC_ARRAY(a) a, __same_type(a, NULL) ? 0 : sizeof(a)/sizeof(*a)
-
  #define INTC_HW_DESC(vectors, groups, mask_regs,       \
                      prio_regs, sense_regs, ack_regs)   \
  {                                                      \
-       _INTC_ARRAY(vectors), _INTC_ARRAY(groups),      \
-       _INTC_ARRAY(mask_regs), _INTC_ARRAY(prio_regs), \
-       _INTC_ARRAY(sense_regs), _INTC_ARRAY(ack_regs), \
+       ARRAY_SIZE(vectors), ARRAY_SIZE(groups),        \
+       ARRAY_SIZE(mask_regs), ARRAY_SIZE(prio_regs),   \
+       ARRAY_SIZE(sense_regs), ARRAY_SIZE(ack_regs),   \
  }
  
  struct intc_desc {

-- 
  .''`.  John Paul Adrian Glaubitz
: :' :  Debian Developer
`. `'   Physicist
   `-    GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913

Re: Calculating array sizes in C - was: Re: Build regressions/improvements in v6.2-rc1

[Geert Uytterhoeven]

Hi Adrian,

On Tue, Jan 17, 2023 at 5:42 PM John Paul Adrian Glaubitz
<glaubitz@physik.fu-berlin.de> wrote:
> On 1/6/23 16:17, Geert Uytterhoeven wrote:
> >> I'm not seeing this one, but I am getting this one instead:
> >>
> >> In file included from ./arch/sh/include/asm/hw_irq.h:6,
> >>                    from ./include/linux/irq.h:596,
> >>                    from ./include/asm-generic/hardirq.h:17,
> >>                    from ./arch/sh/include/asm/hardirq.h:9,
> >>                    from ./include/linux/hardirq.h:11,
> >>                    from ./include/linux/interrupt.h:11,
> >>                    from ./include/linux/serial_core.h:13,
> >>                    from ./include/linux/serial_sci.h:6,
> >>                    from arch/sh/kernel/cpu/sh2/setup-sh7619.c:11:
> >> ./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)
> >>         |                                                               ^
> >> ./include/linux/sh_intc.h:105:31: note: in expansion of macro '_INTC_ARRAY'
> >>     105 |         _INTC_ARRAY(vectors), _INTC_ARRAY(groups),      \
> >>         |                               ^~~~~~~~~~~
> >
> > The easiest fix for the latter is to disable CONFIG_WERROR.
> > Unfortunately I don't know a simple solution to get rid of the warning.
>
> I did some research and it seems that what the macro _INT_ARRAY() does with "sizeof(a)/sizeof(*a)"
> is a commonly used way to calculate array sizes and the kernel has even its own macro for that
> called ARRAY_SIZE() which Linus asks people to use here [1].
>
> So, I replaced _INTC_ARRAY() with ARRAY_SIZE() (see below), however the kernel's own ARRAY_SIZE()
> macro triggers the same compiler warning. I'm CC'ing Michael Karcher who has more knowledge on
> writing proper C code than me and maybe an idea how to fix this warning.
>
> Thanks,
> Adrian
>
> > [1] https://lkml.org/lkml/2015/9/3/428
>
> diff --git a/include/linux/sh_intc.h b/include/linux/sh_intc.h
> index c255273b0281..07a187686a84 100644
> --- a/include/linux/sh_intc.h
> +++ b/include/linux/sh_intc.h
> @@ -97,14 +97,12 @@ struct intc_hw_desc {
>          unsigned int nr_subgroups;
>   };
>
> -#define _INTC_ARRAY(a) a, __same_type(a, NULL) ? 0 : sizeof(a)/sizeof(*a)
> -
>   #define INTC_HW_DESC(vectors, groups, mask_regs,       \
>                       prio_regs, sense_regs, ack_regs)   \
>   {                                                      \
> -       _INTC_ARRAY(vectors), _INTC_ARRAY(groups),      \
> -       _INTC_ARRAY(mask_regs), _INTC_ARRAY(prio_regs), \
> -       _INTC_ARRAY(sense_regs), _INTC_ARRAY(ack_regs), \
> +       ARRAY_SIZE(vectors), ARRAY_SIZE(groups),        \
> +       ARRAY_SIZE(mask_regs), ARRAY_SIZE(prio_regs),   \
> +       ARRAY_SIZE(sense_regs), ARRAY_SIZE(ack_regs),   \
>   }

The issue is that some of the parameters are not arrays, but
NULL. E.g.:

arch/sh/kernel/cpu/sh2/setup-sh7619.c:static
DECLARE_INTC_DESC(intc_desc, "sh7619", vectors, NULL,
arch/sh/kernel/cpu/sh2/setup-sh7619.c-                   NULL,
prio_registers, NULL);
--

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

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

Re: Calculating array sizes in C - was: Re: Build regressions/improvements in v6.2-rc1

[John Paul Adrian Glaubitz]

Hi!

On 1/17/23 18:01, Geert Uytterhoeven wrote:
> The issue is that some of the parameters are not arrays, but
> NULL. E.g.:
> 
> arch/sh/kernel/cpu/sh2/setup-sh7619.c:static
> DECLARE_INTC_DESC(intc_desc, "sh7619", vectors, NULL,
> arch/sh/kernel/cpu/sh2/setup-sh7619.c-                   NULL,
> prio_registers, NULL);

Isn't this supposed to be caught by this check:

	a, __same_type(a, NULL)

?

Adrian

-- 
  .''`.  John Paul Adrian Glaubitz
: :' :  Debian Developer
`. `'   Physicist
   `-    GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913

Re: Calculating array sizes in C - was: Re: Build regressions/improvements in v6.2-rc1

[Geert Uytterhoeven]

Hi Adrian,

On Tue, Jan 17, 2023 at 6:06 PM John Paul Adrian Glaubitz
<glaubitz@physik.fu-berlin.de> wrote:
> On 1/17/23 18:01, Geert Uytterhoeven wrote:
> > The issue is that some of the parameters are not arrays, but
> > NULL. E.g.:
> >
> > arch/sh/kernel/cpu/sh2/setup-sh7619.c:static
> > DECLARE_INTC_DESC(intc_desc, "sh7619", vectors, NULL,
> > arch/sh/kernel/cpu/sh2/setup-sh7619.c-                   NULL,
> > prio_registers, NULL);
>
> Isn't this supposed to be caught by this check:
>
>         a, __same_type(a, NULL)
>
> ?

Yeah, but gcc thinks it is smarter than us...
Probably it drops the test, assuming UB cannot happen.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

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

Re: Calculating array sizes in C - was: Re: Build regressions/improvements in v6.2-rc1

[John Paul Adrian Glaubitz]

Hi!

On 1/17/23 21:05, Geert Uytterhoeven wrote:
>> Isn't this supposed to be caught by this check:
>>
>>          a, __same_type(a, NULL)
>>
>> ?
> 
> Yeah, but gcc thinks it is smarter than us...
> Probably it drops the test, assuming UB cannot happen.

Hmm, sounds like a GGC bug to me then. Not sure how to fix this then.

Adrian

-- 
  .''`.  John Paul Adrian Glaubitz
: :' :  Debian Developer
`. `'   Physicist
   `-    GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913

Re: Calculating array sizes in C - was: Re: Build regressions/improvements in v6.2-rc1

[Michael.Karcher]

Isn't this supposed to be caught by this check:
>>>
>>>          a, __same_type(a, NULL)
>>>
>>> ?
>>
>> Yeah, but gcc thinks it is smarter than us...
>> Probably it drops the test, assuming UB cannot happen.
> Hmm, sounds like a GGC bug to me then. Not sure how to fix this then.


I don't see a clear bug at this point. We are talking about the C expression

   __same_type((void*)0, (void*)0)? 0 : sizeof((void*)0)/sizeof(*((void*0))

This expression is valid (assuming __same_type works, which is a GCC 
extension), and should return 0. As of now, I have no indication that 
this expression does not return 0. Also, it is true that this expression 
contains the suspicious pattern "sizeof(void*)/sizeof(void)", which is 
does not calculate the size of any array. GCC is free to emit as much 
warnings is it wants for any kind of expressions. From a C standard 
point of view, it's just a "quality of implementation" issue, and an 
implementation that emits useless warnings is of low quality, but not 
non-conforming.

In this case, we requested that gcc refuses to compile if it emits any 
kind of warning, which instructs gcc to reject programs that would be 
valid according to the C standard, but are deemed to be "likely incorrect".

I suggest to file a bug against gcc complaining about a "spurious 
warning", and using "-Werror -Wno-error-sizeof-pointer-div" until gcc is 
adapted to not emit the warning about the pointer division if the result 
is not used.


Regards,
   Michael Karcher

Re: Calculating array sizes in C - was: Re: Build regressions/improvements in v6.2-rc1

[Rob Landley]

On 1/19/23 16:11, Michael.Karcher wrote:
> Isn't this supposed to be caught by this check:
>>>>
>>>>          a, __same_type(a, NULL)
>>>>
>>>> ?
>>>
>>> Yeah, but gcc thinks it is smarter than us...
>>> Probably it drops the test, assuming UB cannot happen.
>> Hmm, sounds like a GGC bug to me then. Not sure how to fix this then.
> 
> 
> I don't see a clear bug at this point. We are talking about the C expression
> 
>    __same_type((void*)0, (void*)0)? 0 : sizeof((void*)0)/sizeof(*((void*0))

*(void*) is type "void" which does not have a size.

The problem is gcc "optimizing out" an earlier type check, the same way it
"optimizes out" checks for signed integer math overflowing, or "optimizes out" a
comparison to pointers from two different local variables from different
function calls trying to calculate the amount of stack used, or "optimizes out"
using char *x = (char *)1; as a flag value and then doing "if (!(x-1)) because
it can "never happen"...
> I suggest to file a bug against gcc complaining about a "spurious 
> warning", and using "-Werror -Wno-error-sizeof-pointer-div" until gcc is 
> adapted to not emit the warning about the pointer division if the result 
> is not used.

Remember when gcc got rewritten in c++ starting in 2007?

Historically the main marketing push of C++ was that it contains the whole of C
and therefore MUST be just as good a language, the same way a mud pie contains
an entire glass of water and therefore MUST be just as good a beverage. Anything
C can do that C++ _can't_ do is seen as an existential threat by C++ developers.
They've worked dilligently to "fix" C not being a giant pile of "undefined
behavior" the way C++ is for 15 years now.

I have... opinions on this.

> Regards,
>    Michael Karcher

Rob

Re: Calculating array sizes in C - was: Re: Build regressions/improvements in v6.2-rc1

[Segher Boessenkool]

On Thu, Jan 19, 2023 at 09:31:21PM -0600, Rob Landley wrote:
> On 1/19/23 16:11, Michael.Karcher wrote:
> > I don't see a clear bug at this point. We are talking about the C expression
> > 
> >    __same_type((void*)0, (void*)0)? 0 : sizeof((void*)0)/sizeof(*((void*0))

(__same_type is a kernel macro, it expands to something with
__builtin_compatible_type()).

> *(void*) is type "void" which does not have a size.

It has size 1, in GCC, so that you can do arithmetic on pointers to
void.  This is a long-standing and very widely used GCC extension.

"""
6.24 Arithmetic on 'void'- and Function-Pointers
================================================

In GNU C, addition and subtraction operations are supported on pointers
to 'void' and on pointers to functions.  This is done by treating the
size of a 'void' or of a function as 1.

 A consequence of this is that 'sizeof' is also allowed on 'void' and on
function types, and returns 1.

 The option '-Wpointer-arith' requests a warning if these extensions are
used.
"""

> The problem is gcc "optimizing out" an earlier type check, the same way it
> "optimizes out" checks for signed integer math overflowing, or "optimizes out" a
> comparison to pointers from two different local variables from different
> function calls trying to calculate the amount of stack used, or "optimizes out"

Are you saying something in the kernel code here is invalid code?
Because your other examples are.

> using char *x = (char *)1; as a flag value and then doing "if (!(x-1)) because
> it can "never happen"...

Like here.  And no, this is not allowed by -fno-strict-aliasing.

> > I suggest to file a bug against gcc complaining about a "spurious 
> > warning", and using "-Werror -Wno-error-sizeof-pointer-div" until gcc is 
> > adapted to not emit the warning about the pointer division if the result 
> > is not used.

Yeah.  If the first operand of a conditional operator is non-zero, the
second operand is not evaluated, and if the first is zero, the third
operand is not evaluated.  It is better if we do not warn about
something we do not evaluate.  In cases like here where it is clear at
compile time which branch is taken, that shouldn't be too hard.

Can someone please file a GCC PR?  With reduced testcase preferably.


Segher

Re: Calculating array sizes in C - was: Re: Build regressions/improvements in v6.2-rc1

[Michael.Karcher]

Hello!
> Can someone please file a GCC PR?  With reduced testcase preferably.

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108483

There you are.

Kind regars,
   Michael Karcher

Re: Calculating array sizes in C - was: Re: Build regressions/improvements in v6.2-rc1

[John Paul Adrian Glaubitz]

Hi Michael!

On 1/19/23 23:11, Michael.Karcher wrote:
> I suggest to file a bug against gcc complaining about a "spurious warning",
> and using "-Werror -Wno-error-sizeof-pointer-div" until gcc is adapted to
> not emit the warning about the pointer division if the result is not used.

Could you post a kernel patch for that? I would be happy to test it on my
SH-7785CLR board. Also, I'm going to file a bug report against GCC.

Adrian

-- 
  .''`.  John Paul Adrian Glaubitz
: :' :  Debian Developer
`. `'   Physicist
   `-    GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913

Re: Calculating array sizes in C - was: Re: Build regressions/improvements in v6.2-rc1

[Michael Karcher]

[-- Attachment #1: Type: text/plain, Size: 561 bytes --]

Hello Adrian,
> Could you post a kernel patch for that? I would be happy to test it on my
> SH-7785CLR board. Also, I'm going to file a bug report against GCC.

I filed the bug already. It's 
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108483.

The diff is attached. It's published as CC0 in case anyone considers 
this trivial change copyrightable. This patch prevents this one specific 
warning from being upgraded to "error" even if you configure the kernel 
to use "-Werror". It still keeps it active as warning, though.

Kind regards,
   Michael Karcher

[-- Attachment #2: werror.diff --]
[-- Type: text/plain, Size: 469 bytes --]

diff --git a/Makefile b/Makefile
index e09fe100efb2..b4cd075c6a19 100644
--- a/Makefile
+++ b/Makefile
@@ -870,7 +870,7 @@ stackp-flags-$(CONFIG_STACKPROTECTOR_STRONG)      := -fstack-protector-strong
 
 KBUILD_CFLAGS += $(stackp-flags-y)
 
-KBUILD_CPPFLAGS-$(CONFIG_WERROR) += -Werror
+KBUILD_CPPFLAGS-$(CONFIG_WERROR) += -Werror -Wno-error=sizeof-pointer-div
 KBUILD_CPPFLAGS += $(KBUILD_CPPFLAGS-y)
 KBUILD_CFLAGS-$(CONFIG_CC_NO_ARRAY_BOUNDS) += -Wno-array-bounds
 

Re: Calculating array sizes in C - was: Re: Build regressions/improvements in v6.2-rc1

[John Paul Adrian Glaubitz]

Hi!

On 1/20/23 20:29, Michael Karcher wrote:
> Hello Adrian,
>> Could you post a kernel patch for that? I would be happy to test it on my
>> SH-7785CLR board. Also, I'm going to file a bug report against GCC.
> 
> I filed the bug already. It's https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108483.
> 
> The diff is attached. It's published as CC0 in case anyone considers this trivial change copyrightable. This patch prevents this one specific warning from being upgraded to "error" even if you configure the kernel to use "-Werror". It still keeps it active as warning, though.

I used the following variant and it fixes the issue for me:

diff --git a/arch/sh/Makefile b/arch/sh/Makefile
index 5c8776482530..11b22f7167d2 100644
--- a/arch/sh/Makefile
+++ b/arch/sh/Makefile
@@ -167,7 +167,7 @@ drivers-y                   += arch/sh/drivers/
  cflags-y       += $(foreach d, $(cpuincdir-y), -I $(srctree)/arch/sh/include/$(d)) \
                    $(foreach d, $(machdir-y), -I $(srctree)/arch/sh/include/$(d))
  
-KBUILD_CFLAGS          += -pipe $(cflags-y)
+KBUILD_CFLAGS          += -pipe -Wno-error=sizeof-pointer-div $(cflags-y)
  KBUILD_CPPFLAGS                += $(cflags-y)
  KBUILD_AFLAGS          += $(cflags-y)

If you agree, can you post a patch to LKML so we can unbreak the SH build for CONFIG_WERROR?

Thanks,
Adrian

-- 
  .''`.  John Paul Adrian Glaubitz
: :' :  Debian Developer
`. `'   Physicist
   `-    GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913

Re: Build regressions/improvements in v6.2-rc1

[Alex Deucher]

On Tue, Dec 27, 2022 at 10:34 AM Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
>
> On Tue, 27 Dec 2022, Geert Uytterhoeven wrote:
> > 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
>
> amd-gfx@lists.freedesktop.org
> linux-arm-kernel@lists.infradead.org
> linux-media@vger.kernel.org
> linux-wireless@vger.kernel.org
> linux-mips@vger.kernel.org
> linux-sh@vger.kernel.org
> linux-f2fs-devel@lists.sourceforge.net
> linuxppc-dev@lists.ozlabs.org
> kasan-dev@googlegroups.com
> linux-xtensa@linux-xtensa.org
>
>    + /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
>

@Siqueira, Rodrigo @Mahfooz, Hamza

Can you take a look at fixing the DML stack size here up?

Alex


> arm64-gcc5/arm64-allmodconfig
>
>    + /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
>
> m68k-gcc8/m68k-allmodconfig
> See also https://lore.kernel.org/all/CAMuHMdWpPX2mpqFEWjjbjsQvDBQOXyjjdpKnQu9qURAuVZXmMw@mail.gmail.com
>
>    + /kisskb/src/include/linux/bitfield.h: error: call to '__field_overflow' declared with attribute error: value doesn't fit into mask:  => 151:3
>
> In function 'u32_encode_bits',
>      inlined from 'ieee80211_mlo_multicast_tx' at /kisskb/src/net/mac80211/tx.c:4435:17,
>      inlined from 'ieee80211_subif_start_xmit' at /kisskb/src/net/mac80211/tx.c:4483:3:
>
> mipsel-gcc5/mips-allmodconfig
>
>    + /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
>
> In function 'follow_pmd_mask',
>      inlined from 'follow_pud_mask' at /kisskb/src/mm/gup.c:735:9,
>      inlined from 'follow_p4d_mask' at /kisskb/src/mm/gup.c:752:9,
>      inlined from 'follow_page_mask' at /kisskb/src/mm/gup.c:809:9:
>
> sh4-gcc11/sh-defconfig (Günter wondered if pmd_t should use union)
>
>    + /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/arch/s390/kernel/setup.c: In function 'setup_lowcore_dat_on':
> s390x-gcc11/s390-all{mod,yes}config
>
>    + /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/fs/f2fs/inline.c: In function 'f2fs_move_inline_dirents':
>
> powerpc-gcc11/ppc64_book3e_allmodconfig
> powerpc-gcc11/powerpc-all{mod,yes}config
>
>    + /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
>
> xtensa-gcc11/xtensa-allmodconfig (patch available)
>
>    + {standard input}: Error: unknown pseudo-op: `.cfi_def_c':  => 1718
>
> sh4-gcc11/sh-allmodconfig (ICE = internal compiler error)
>
> > [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)
>
> Gr{oetje,eeting}s,
>
>                                                 Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
>
> 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

Re: Linux 6.2-rc1

[Pali Rohár]

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/

Re: Linux 6.2-rc1

[Linus Torvalds]

On Wed, Jan 4, 2023 at 11:01 AM Pali Rohár <pali@kernel.org> 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

Re: Linux 6.2-rc1

[Pali Rohár]

On Wednesday 04 January 2023 11:25:41 Linus Torvalds wrote:
> On Wed, Jan 4, 2023 at 11:01 AM Pali Rohár <pali@kernel.org> 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).

Re: Linux 6.2-rc1

[Pali Rohár]

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 <pali@kernel.org> 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

Re: Linux 6.2-rc1

[Linus Torvalds]

On Wed, Jan 4, 2023 at 12:56 PM Pali Rohár <pali@kernel.org> 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

Re: Linux 6.2-rc1

[Jens Axboe]

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

Re: Linux 6.2-rc1

[Greg Kroah-Hartman]

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

Re: Linux 6.2-rc1

[Jens Axboe]

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

Re: Linux 6.2-rc1

[Pali Rohár]

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
> 

Re: Linux 6.2-rc1

[Jens Axboe]

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

Re: Linux 6.2-rc1

[Linus Torvalds]

On Thu, Jan 5, 2023 at 9:45 AM Jens Axboe <axboe@kernel.dk> 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

Re: Linux 6.2-rc1

[Pali Rohár]

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.

Re: Linux 6.2-rc1

[Jens Axboe]

On 1/5/23 12:06 PM, Linus Torvalds wrote:
> On Thu, Jan 5, 2023 at 9:45 AM Jens Axboe <axboe@kernel.dk> 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

Re: Linux 6.2-rc1

[Linus Torvalds]

On Thu, Jan 5, 2023 at 11:40 AM Jens Axboe <axboe@kernel.dk> 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

Re: Linux 6.2-rc1

[Jens Axboe]

On 1/5/23 1:03?PM, Linus Torvalds wrote:
> On Thu, Jan 5, 2023 at 11:40 AM Jens Axboe <axboe@kernel.dk> 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

Re: Linux 6.2-rc1

[Pali Rohár]

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.

Re: Linux 6.2-rc1

[Jens Axboe]

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

pktcdvd

[Pali Rohár]

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

Re: pktcdvd

[Linus Torvalds]

On Sat, Jan 28, 2023 at 11:35 AM Pali Rohár <pali@kernel.org> 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

Re: pktcdvd

[Jens Axboe]

On 1/28/23 12:43 PM, Linus Torvalds wrote:
> On Sat, Jan 28, 2023 at 11:35 AM Pali Rohár <pali@kernel.org> 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

Re: pktcdvd

[Jens Axboe]

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

Re: pktcdvd

[Pali Rohár]

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.

Re: pktcdvd

[Jens Axboe]

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