Bug 1212691

Did you install the latest kernel-firmware-amdgpu package?

No firmware updates were offered since I upgraded to Leap 15.5.

# env LANGUAGE=en-us zypper info kernel-firmware-amdgpu
Loading repository data...
Reading installed packages...


Information for package kernel-firmware-amdgpu:
-----------------------------------------------
Repository     : Main Repository
Name           : kernel-firmware-amdgpu
Version        : 20230320-150500.1.1
Arch           : noarch
Vendor         : SUSE LLC <https://www.suse.com/>
Installed Size : 16,0 MiB
Installed      : Yes (automatically)
Status         : up-to-date
Source package : kernel-firmware-20230320-150500.1.1.src
Upstream URL   : https://git.kernel.org/cgit/linux/kernel/git/firmware/linux-firmware.git/
Summary        : Kernel firmware files for AMDGPU graphics driver
Description    : 
    This package contains compressed kernel firmware files for
    AMDGPU graphics driver.

OK, then check the contents of initrd by "lsinitrd" command.  Check whether the initrd contains /lib/firmware/amdgpu/* files, and whether /lib/firmware/amdgpu/polaris11_ce.bin (or polaris11_ce.bin.xz) is present.

Created attachment 868170 [details]
lsinitrd

# env LANGUAGE=en-us lsinitrd /boot/initrd-5.14.21-150500.53-default

/lib/firmware/amdgpu/* files (present)

/lib/firmware/amdgpu/polaris11_ce.bin (absent)

/lib/firmware/amdgpu/polaris11_ce.bin.xz (absent)

/lib/firmware/amdgpu/polaris11* files:
polaris11_k2_smc.bin.xz
polaris11_k_mc.bin.xz
polaris11_k_smc.bin.xz
polaris11_mc.bin.xz
polaris11_me_2.bin.xz
polaris11_me.bin.xz
polaris11_mec2_2.bin.xz
polaris11_mec_2.bin.xz
polaris11_mec.bin.xz
polaris11_pfp_2.bin.xz
polaris11_pfp.bin.xz
polaris11_rlc.bin.xz
polaris11_sdma1.bin.xz -> polaris10_sdma1.bin.xz
polaris11_sdma.bin.xz -> polaris10_sdma.bin.xz
polaris11_smc.bin.xz
polaris11_smc_sk.bin.xz
polaris11_uvd.bin.xz -> polaris10_uvd.bin.xz
polaris11_vce.bin.xz -> polaris10_vce.bin.xz

Now please check three things:
- the output of "modinfo amdgpu"; whether it contains the missing entries for polaris11
- whether you have the actual file in /lib/firmware/*
- whether you have enough disk space for /boot

If all look OK, then it's likely something wrong with dracut.
You can try to add a file /etc/dracut.conf.d/90-polaris11.conf containing the following line:

install_items+=" /lib/firmware/amdgpu/polaris11_ce.bin.xz"

then rebuild initrd and retest.

Created attachment 868181 [details]
detailed info + 90-polaris11.conf result

- the output of "modinfo amdgpu";
whether it contains the missing entries for polaris11 (NO)

- whether you have the actual file in /lib/firmware/* (YES)
# dir /lib/firmware/amdgpu/polaris11_ce*
polaris11_ce_2.bin.xz -> polaris10_ce_2.bin.xz
polaris11_ce.bin.xz -> polaris10_ce.bin.xz

- whether you have enough disk space for /boot (YES)
# btrfs fi usage /
Overall:
    Free (estimated):             31.52GiB      (min: 31.52GiB)
    Free (statfs, df):            31.52GiB

- add a file /etc/dracut.conf.d/90-polaris11.conf (OK)

- rebuild initrd (OK)
# dracut --rebuild /boot/initrd-5.14.21-150500.53-default

- retest lsinit (OK)
# lsinitrd /boot/initrd-5.14.21-150500.53-default | grep -i polaris11_ce
lib/firmware/amdgpu/polaris11_ce.bin.xz -> polaris10_ce.bin.xz

- retest reboot (OK)

The Leap 15.5 with kernel-default-5.14.21-150500.53 booted just fine after adding the /etc/dracut.conf.d/90-polaris11.conf file.

(In reply to Bruno Damasceno Freire from comment #6)
> Created attachment 868181 [details]
> detailed info + 90-polaris11.conf result
> 
> - the output of "modinfo amdgpu";
> whether it contains the missing entries for polaris11 (NO)

Wait... That's an unexpected answer.

Could you give the output of "modinfo amdgpu"?

Scratch my previous comment, I saws you already gave the info.

But, as far as I see, "modinfo amdgpu" gives the line
  firmware:       amdgpu/polaris11_ce.bin
and yet this firmware wasn't included in initrd?

If so, please run dracut with --debug option, and give the whole output.

Created attachment 868183 [details]
dracut debug log

(In reply to Takashi Iwai from comment #8)
> Scratch my previous comment, I saws you already gave the info.
> 
> But, as far as I see, "modinfo amdgpu" gives the line
>   firmware:       amdgpu/polaris11_ce.bin
> and yet this firmware wasn't included in initrd?
> 
> If so, please run dracut with --debug option, and give the whole output.

You got it right. I should have marked this question with an "YES". Sorry.

And YES, even listed on "modinfo gpu", it wasn´t included in initrd AFAICS.

Dracut log attached.

Oh, could you try to remove /etc/dracut.d/* stuff you've added, and give the dracut --debug output?  That is, I'd like to see what dracut does (or doesn't) for the missing firmware.

Also, add --force option to dracut to forcibly rebuilding an initrd.

Created attachment 868209 [details]
dracut debug + console capture + some filtering

Thanks.  So the problematic part looks like:

dracut-install: dracut_install('/lib/firmware/amdgpu/polaris10_ce.bin.xz', '/lib/firmware/amdgpu/polaris10_ce.bin.xz', 0, 0, 1)
dracut-install: dracut_install ret = 0
dracut-install: cp '/lib/firmware/amdgpu/polaris10_ce.bin.xz' '/var/tmp/dracut.H9Cp1T/initramfs/lib/firmware/amdgpu/polaris10_ce.bin.xz'
cp: setting attribute 'btrfs.compression' for 'btrfs.compression': Invalid argument
dracut-install: cp ret = 256
dracut-install: dracut_install ret = 256
dracut-install: '/lib/firmware/amdgpu/polaris10_ce.bin.xz' install error
dracut-install: Possible missing firmware amdgpu/polaris11_ce.bin for kernel module amdgpu
dracut-install: stat(/lib/firmware/5.14.21-150500.53-default/amdgpu/polaris11_ce.bin) != 0
dracut-install: Possible missing firmware amdgpu/polaris11_ce.bin for kernel module amdgpu


... and this implies that the problem is somehow related with btrfs.

Adding filesystem guys to Cc.

btrfs.compression can take value "lzo", "zlib", "zstd", "no", "none" or "". "btrfs.compression" is not a valid argument.

Which script is trying to set btrfs.compression?

(In reply to Goldwyn Rodrigues from comment #14)
> btrfs.compression can take value "lzo", "zlib", "zstd", "no", "none" or "".
> "btrfs.compression" is not a valid argument.
> 
> Which script is trying to set btrfs.compression?

Is your question related to the following log entry?

> cp: setting attribute 'btrfs.compression' for 'btrfs.compression': Invalid argument

No dracut script is setting btrfs attributes, it's failing the `cp` with `--preserve=mode,xattr,timestamps,ownership`.

My guess, this is because of a bug fix that was backported to the kernel release being used:

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=0e852ab8974cd2b5946766b2d9baf82c78ace03d

The target file, or the target directory, has nodatacow set - that should fail with -EINVAL when setting the compression property/xattr, as described in the commit's changelog - it's what happens via chattr for example, as compression needs COW to work.

Just check if the target directory has nodatacow set (the "C" in lsattr's output), see the changelog.

(In reply to Filipe Manana from comment #16)
> ...
> Just check if the target directory has nodatacow set (the "C" in lsattr's
> output), see the changelog.

# btrfs property get /lib
compression=zstd:1

# btrfs property get /lib/firmware/amdgpu/polaris10_ce.bin.xz
compression=zstd

# lsattr /lib/firmware/amdgpu/polaris10_ce.bin.xz
--------c------------- /lib/firmware/amdgpu/polaris10_ce.bin.xz

# lsattr /var/tmp
---------------C------ /var/tmp/zypp.iy8OXE
---------------C------ /var/tmp/zypp.0yjeP4
---------------C------ /var/tmp/zypp.BQbptY
---------------C------ /var/tmp/zypp.56R7ao
---------------C------ /var/tmp/systemd-private-e82735bfd4554146a13b8fbd8427a622-systemd-logind.service-1kub9g
---------------C------ /var/tmp/systemd-private-e82735bfd4554146a13b8fbd8427a622-chronyd.service-iJn1dh
---------------C------ /var/tmp/systemd-private-e82735bfd4554146a13b8fbd8427a622-upower.service-cAkj6i
---------------C------ /var/tmp/systemd-private-e82735bfd4554146a13b8fbd8427a622-rtkit-daemon.service-morsdg
---------------C------ /var/tmp/systemd-private-e82735bfd4554146a13b8fbd8427a622-power-profiles-daemon.service-ZEQGKg

# env LANGUAGE=en-us lsattr /var/tmp/polaris10_ce.bin
lsattr: No such file or directory while trying to stat /var/tmp/polaris10_ce.bin

# env LANGUAGE=en-us cp --preserve=mode,xattr,timestamps,ownership /lib/firmware/amdgpu/polaris10_ce.bin.xz /var/tmp
cp: setting attribute 'btrfs.compression' for 'btrfs.compression': Invalid argument

# lsattr /var/tmp/polaris10_ce.bin
---------------C------ /var/tmp/polaris10_ce.bin.xz

(In reply to Bruno Damasceno Freire from comment #17)
> (In reply to Filipe Manana from comment #16)
> > ...
> > Just check if the target directory has nodatacow set (the "C" in lsattr's
> > output), see the changelog.
> 
> # btrfs property get /lib
> compression=zstd:1
> 
> # btrfs property get /lib/firmware/amdgpu/polaris10_ce.bin.xz
> compression=zstd
> 
> # lsattr /lib/firmware/amdgpu/polaris10_ce.bin.xz
> --------c------------- /lib/firmware/amdgpu/polaris10_ce.bin.xz
> 
> # lsattr /var/tmp
> ---------------C------ /var/tmp/zypp.iy8OXE
> ---------------C------ /var/tmp/zypp.0yjeP4
> ---------------C------ /var/tmp/zypp.BQbptY
> ---------------C------ /var/tmp/zypp.56R7ao
> ---------------C------

Ok, this confirms my hunch.

/lib/firmware/amdgpu/polaris10_ce.bin.xz has compression set (xattr "btrfs.compression") and /var/tmp has the nodatacow bit set (C).

So when polaris10_ce.bin.xz is created at /var/tmp, it inherits the nodatacow bit and then cp finds the compression xattr and attempts to set it in the file, which fails with -EINVAL since the nodatacow attribute is set.

The course of action I see is to teach cp to not set the btrfs.compression xattr if the destination file has the nodatacow bit set.


> /var/tmp/systemd-private-e82735bfd4554146a13b8fbd8427a622-systemd-logind.
> service-1kub9g
> ---------------C------
> /var/tmp/systemd-private-e82735bfd4554146a13b8fbd8427a622-chronyd.service-
> iJn1dh
> ---------------C------
> /var/tmp/systemd-private-e82735bfd4554146a13b8fbd8427a622-upower.service-
> cAkj6i
> ---------------C------
> /var/tmp/systemd-private-e82735bfd4554146a13b8fbd8427a622-rtkit-daemon.
> service-morsdg
> ---------------C------
> /var/tmp/systemd-private-e82735bfd4554146a13b8fbd8427a622-power-profiles-
> daemon.service-ZEQGKg
> 
> # env LANGUAGE=en-us lsattr /var/tmp/polaris10_ce.bin
> lsattr: No such file or directory while trying to stat
> /var/tmp/polaris10_ce.bin
> 
> # env LANGUAGE=en-us cp --preserve=mode,xattr,timestamps,ownership
> /lib/firmware/amdgpu/polaris10_ce.bin.xz /var/tmp
> cp: setting attribute 'btrfs.compression' for 'btrfs.compression': Invalid
> argument
> 
> # lsattr /var/tmp/polaris10_ce.bin
> ---------------C------ /var/tmp/polaris10_ce.bin.xz

Not sure if we can put filesystem specifc code in coreutils/cp.

In the meantime, the workaround is to set environment variable DRACUT_NO_XATTR=true while running dracut.

Created attachment 868386 [details]
dracut console capture + some filtering (7)

(In reply to Goldwyn Rodrigues from comment #19)
> Not sure if we can put filesystem specifc code in coreutils/cp.
> 
> In the meantime, the workaround is to set environment variable
> DRACUT_NO_XATTR=true while running dracut.

Please correct me if I am getting anything wrong:

My corner case is about getting an existing Leap, with btrfs compression property on /lib and /usr, properly upgraded.

The expanded case is initrd being created with missing files because btrfs compression property due incompatible file attrib between system folder and tmp folder.

Then these two possibilities came to my mind:
1) Changing the dracut tmp subfolder attrib to COW on btrfs partitions.
2) Scan for btrfs compression property beforehand and set the DRACUT_NO_XATTR=true env variable if appropriate.

I don´t know the complexities associated with these suggestions so they are purely speculative brainstorm.

About my machine I did some experiments I got these results (more on the attachment):

                                |debug3|debug5|debug6|debug7|
console string count
--> 'cp ret = 256'              | 1111 | 1110 |  697 |    0 |
--> 'dracut_install ret = 256'	| 1111 | 1110 |  697 |    0 |
--> 'install error'             |  116 |  115 |  115 |    0 |
--> 'dracut-install: ERROR:'	|  132 |  132 |  129 |    0 |
--> 'missing'               	| 1632 | 1632 | 1650 | 1650 |
--> 'hash hit'              	| 5901 | 5902 | 5902 | 6009 |
--> ' OK'                   	|  243 |  244 |  624 | 1104 |
btrfs property compression
--> polaris10_ce.bin.xz     	|   x  |   -  |   -  |   -  |
--> /lib                    	|   x  |   x  |   -  |   -  |
--> /usr                    	|   x  |   x  |   x  |   x  |
other info
--> working video               |   -  |   x  |   x  |   x  |
--> DRACUT_NO_XATTR=true       	|   -  |   -  |   -  |   x  |
--> lsinitrd file count         |   -  | 1834 |   -  | 1911 |

So, for now, I will be taking Goldwyn Rodrigues advice and will apply DRACUT_NO_XATTR=true on /etc/environment on my 5 installations (2 Leap, 3 Tumbleweed) since they all will remain with btrfs compression property set on /usr. The btrfs compression property shouldn´t be set on /lib and must have been a leftover from some test. For my "luck" it was this very leftover that triggered this issue.

(In reply to Goldwyn Rodrigues from comment #19)
> Not sure if we can put filesystem specifc code in coreutils/cp.

Agreed, I doubt coreutils would find it acceptable to add FS xattr specific behaviour to work around this issue. 

> In the meantime, the workaround is to set environment variable
> DRACUT_NO_XATTR=true while running dracut.

Any xattrs will be stripped when the files eventually go into the cpio archive, so I think we should be able to consider making DRACUT_NO_XATTR=true the default - @Antonio: do you know why Dracut defaults to copying xattrs when staging for cpio? With https://github.com/dracutdevs/dracut/issues/1662 I'd hoped to drop the staging area completely in future.

(In reply to David Disseldorp from comment #21)
> (In reply to Goldwyn Rodrigues from comment #19)
> > Not sure if we can put filesystem specifc code in coreutils/cp.
> 
> Agreed, I doubt coreutils would find it acceptable to add FS xattr specific
> behaviour to work around this issue. 
> 
> > In the meantime, the workaround is to set environment variable
> > DRACUT_NO_XATTR=true while running dracut.
> 
> Any xattrs will be stripped when the files eventually go into the cpio
> archive, so I think we should be able to consider making
> DRACUT_NO_XATTR=true the default - @Antonio: do you know why Dracut defaults
> to copying xattrs when staging for cpio? With

For IMA. See https://github.com/dracutdevs/dracut/pull/161

> https://github.com/dracutdevs/dracut/issues/1662 I'd hoped to drop the
> staging area completely in future.

But your proposal was only for dracut-cpio, or for all the cases?

(In reply to Antonio Feijoo from comment #22)
...
> > Any xattrs will be stripped when the files eventually go into the cpio
> > archive, so I think we should be able to consider making
> > DRACUT_NO_XATTR=true the default - @Antonio: do you know why Dracut defaults
> > to copying xattrs when staging for cpio? With
> 
> For IMA. See https://github.com/dracutdevs/dracut/pull/161

Thanks for the link.

> > https://github.com/dracutdevs/dracut/issues/1662 I'd hoped to drop the
> > staging area completely in future.
> 
> But your proposal was only for dracut-cpio, or for all the cases?

Just dracut-cpio, but it already works as a generic replacement for dracut's GNU cpio usage.

I think the best way forward to solve the issue in comment#22 would be to modify cp to copy security xattrs. Currently --preserve=context does this, but it is limited to SELinux attributes only.

Alternatively, getfattr -m "security.*" could be used to find if it needs to copy the security xattrs, but that may be a long winded road.

I think there are a few viable options here:
1. change cp to preserve a subset of xattrs, as proposed by Filipe and Goldwyn
   + "--preserve=context" implies further filtering may be acceptable upstream
   + it'd cater to other filesystems which may also carry conflicting xattrs
   - it'd introduce an ugly coreutils version dependency for Dracut
2. revert the Btrfs restrictions (0e852ab8974cd and possibly f37c563bab429)
   + removes regression in xattr / fileattr user API
   + should fix some other applications where xattr copy is expected to just work
   - confusing for users; would need documentation for 'C' precedence over 'c'
3. change Dracut's default behaviour to ignore xattrs unless IMA is enabled
   + would workaround the issue for most use-cases
   - still leaves IMA+Btrfs broken, depending on IMA policy enforcement
4. modify Dracut to avoid staging area and write cpio directly ( https://github.com/dracutdevs/dracut/issues/1662 )
   + xattr propagation can be avoided completely
   + performance benefits from reduced I/O
   - relatively invasive Dracut code changes required

I'm leaning towards a combination of 3 (temporary workaround) & 4 but I'm open to other suggestions.

(In reply to Takashi Iwai from comment #13)
> Thanks.  So the problematic part looks like:
> 
> dracut-install: dracut_install('/lib/firmware/amdgpu/polaris10_ce.bin.xz',
> '/lib/firmware/amdgpu/polaris10_ce.bin.xz', 0, 0, 1)
> dracut-install: dracut_install ret = 0
> dracut-install: cp '/lib/firmware/amdgpu/polaris10_ce.bin.xz'
> '/var/tmp/dracut.H9Cp1T/initramfs/lib/firmware/amdgpu/polaris10_ce.bin.xz'
> cp: setting attribute 'btrfs.compression' for 'btrfs.compression': Invalid
> argument
> dracut-install: cp ret = 256
> dracut-install: dracut_install ret = 256
> dracut-install: '/lib/firmware/amdgpu/polaris10_ce.bin.xz' install error
> dracut-install: Possible missing firmware amdgpu/polaris11_ce.bin for kernel
> module amdgpu
> dracut-install:
> stat(/lib/firmware/5.14.21-150500.53-default/amdgpu/polaris11_ce.bin) != 0
> dracut-install: Possible missing firmware amdgpu/polaris11_ce.bin for kernel
> module amdgpu

I think the dracut-install.c::install_firmware() behaviour here should also be changed to propagate errors upwards rather than only logging them.

(In reply to David Disseldorp from comment #25)
> I think there are a few viable options here:
> 1. change cp to preserve a subset of xattrs, as proposed by Filipe and
> Goldwyn

My suggestion was not to preserve only a subset of the xattrs.

It's to make cp aware of the rules for the btrfs.compression xattr and skip it, and maybe print a warning too, in case the destination file has the nodatacaw bit set.

I think this is really necessary, as cp is one of the most basic and widely used tools to copy files, and the failure will surprise users and maybe be a blocker for some - or at least make them waste time trying to figure out what went wrong, possibly asking questions on the mailing list, opening bug reports, etc.

We also have (or had, if someone fixed it in the meanwhile) other problems and surprising behaviors regarding compression. For example if the source file has compression enabled, through the the file attributes ioctl inherited from ext*, cp works by creating the destination file, copying all data and then setting the compression flag on the destination file.

This often causes a surprising behavior because if the data is still in delalloc, page cache only, before setting the compression bit, then the destination file will end up being compressed, just like the source - what users expect.  However if delalloc is fully or partially flushed, due to memory pressure (for e.g. it's a huge file), then the file may not end compressed at all, or just partially compressed, because the compression bit was set after writeback happened.

I think eventually cp will have to be taught about these cases and have a non-surprising behavior for users.


>    + "--preserve=context" implies further filtering may be acceptable
> upstream
>    + it'd cater to other filesystems which may also carry conflicting xattrs
>    - it'd introduce an ugly coreutils version dependency for Dracut
> 2. revert the Btrfs restrictions (0e852ab8974cd and possibly f37c563bab429)
>    + removes regression in xattr / fileattr user API
>    + should fix some other applications where xattr copy is expected to just
> work
>    - confusing for users; would need documentation for 'C' precedence over
> 'c'
> 3. change Dracut's default behaviour to ignore xattrs unless IMA is enabled
>    + would workaround the issue for most use-cases
>    - still leaves IMA+Btrfs broken, depending on IMA policy enforcement
> 4. modify Dracut to avoid staging area and write cpio directly (
> https://github.com/dracutdevs/dracut/issues/1662 )
>    + xattr propagation can be avoided completely
>    + performance benefits from reduced I/O
>    - relatively invasive Dracut code changes required
> 
> I'm leaning towards a combination of 3 (temporary workaround) & 4 but I'm
> open to other suggestions.

Thanks for the feedback Filipe...

(In reply to Filipe Manana from comment #27)
> (In reply to David Disseldorp from comment #25)
> > I think there are a few viable options here:
> > 1. change cp to preserve a subset of xattrs, as proposed by Filipe and
> > Goldwyn
> 
> My suggestion was not to preserve only a subset of the xattrs.
> 
> It's to make cp aware of the rules for the btrfs.compression xattr and skip
> it, and maybe print a warning too, in case the destination file has the
> nodatacaw bit set.

Understood, although I see next to no chance that something like this would be accepted by upstream coreutils - many filesystems introduce their own special interfaces via xattr. Teaching 'cp' the intricacies of these FS specific interfaces should IMO be out of scope of a generic file copy utility.

> I think this is really necessary, as cp is one of the most basic and widely
> used tools to copy files, and the failure will surprise users and maybe be a
> blocker for some - or at least make them waste time trying to figure out
> what went wrong, possibly asking questions on the mailing list, opening bug
> reports, etc.

If this is a concern, then I really think reverting the kernel changes (option 2) should be considered. Disregarding `compress-force`, I think many users are already aware that compression is a best-effort intent flag. The same could be said for nodatacow alongside snapshots.

> We also have (or had, if someone fixed it in the meanwhile) other problems
> and surprising behaviors regarding compression. For example if the source
> file has compression enabled, through the the file attributes ioctl
> inherited from ext*, cp works by creating the destination file, copying all
> data and then setting the compression flag on the destination file.
>
> This often causes a surprising behavior because if the data is still in
> delalloc, page cache only, before setting the compression bit, then the
> destination file will end up being compressed, just like the source - what
> users expect.  However if delalloc is fully or partially flushed, due to
> memory pressure (for e.g. it's a huge file), then the file may not end
> compressed at all, or just partially compressed, because the compression bit
> was set after writeback happened.
> 
> I think eventually cp will have to be taught about these cases and have a
> non-surprising behavior for users.

I don't think FS specific xattrs are comparable to something generic like reflinks or fileattrs. Besides that, Btrfs could also in future add support for compressed extents alongside nodatanow, in which case this kind of logic would have to be revisited to accept the functionality, further confusing users.

One further note regarding FS xattr interfaces: CephFS hides many of its API-like xattrs (see https://docs.ceph.com/en/latest/cephfs/file-layouts/ ) from listxattr(), which tends to avoid this sort of problem.
Doing so now for "btrfs.compression" wouldn't really make sense, as it'd break user behaviour, but it might be worth considering for any future xattr interfaces.