Message ID | cover.1721066206.git.dsterba@suse.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | [GIT,PULL] Btrfs updates for 6.11 | expand |
The pull request you sent on Mon, 15 Jul 2024 20:12:20 +0200:
> git://git.kernel.org/pub/scm/linux/kernel/git/kdave/linux.git tags/for-6.11-tag
has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/a1b547f0f217cfb06af7eb4ce8488b02d83a0370
Thank you!
On Mon, 15 Jul 2024 at 11:12, David Sterba <dsterba@suse.com> wrote: > > There's a merge conflict caused by the latency fixes from last week in > extent_map.c:btrfs_scan_inode(), related commits 4e660ca3a98d931809734 > and b3ebb9b7e92a928344a. Resolved in branch for-6.11-merged and that's > been in linux-next for a few days. Oh, and I notice that my resolution is slightly different, but looks like the actual code is equivalent. I kept the "q" logic that had been introduced by commit 4e660ca3a98d ("btrfs: use a regular rb_root instead of cached rb_root for extent_map_tree"). I don't know how you prefer the code, but it kept the two "while (node)" loops in that file looking similar. Of course, they were very different in other respects (ie drop_all_extent_maps_fast() does all extents unconditionally with that retry logic, while btrfs_scan_inode() has that whole "bail out on any contention" model, so whatever. Anyway, it all *looks* ok to me, but please go and double-check that I didn't mess something up. Linus
On Wed, Jul 17, 2024 at 01:21:17PM -0700, Linus Torvalds wrote: > On Mon, 15 Jul 2024 at 11:12, David Sterba <dsterba@suse.com> wrote: > > > > There's a merge conflict caused by the latency fixes from last week in > > extent_map.c:btrfs_scan_inode(), related commits 4e660ca3a98d931809734 > > and b3ebb9b7e92a928344a. Resolved in branch for-6.11-merged and that's > > been in linux-next for a few days. > > Oh, and I notice that my resolution is slightly different, but looks > like the actual code is equivalent. > > I kept the "q" logic that had been introduced by commit 4e660ca3a98d > ("btrfs: use a regular rb_root instead of cached rb_root for > extent_map_tree"). > > I don't know how you prefer the code, but it kept the two "while > (node)" loops in that file looking similar. That is fine, keeping the same style makes sense. > Of course, they were very different in other respects (ie > drop_all_extent_maps_fast() does all extents unconditionally with that > retry logic, while btrfs_scan_inode() has that whole "bail out on any > contention" model, so whatever. > > Anyway, it all *looks* ok to me, but please go and double-check that I > didn't mess something up. Looks correct to me as well.
On Mon, Jul 15, 2024 at 8:12 PM David Sterba <dsterba@suse.com> wrote: > please pull the changes described below. The hilights are new logic > behind background block group reclaim, automatic removal of qgroup > after removing a subvolume and new 'rescue=' mount options. The rest is > optimizations, cleanups and refactoring. > > There's a merge conflict caused by the latency fixes from last week in > extent_map.c:btrfs_scan_inode(), related commits 4e660ca3a98d931809734 > and b3ebb9b7e92a928344a. Resolved in branch for-6.11-merged and that's > been in linux-next for a few days. FTR, this is broken on 32-bit (doesn't build, good ;-) and on big-endian (compiler warnings, no idea how it behaves :-(, so you better don't trust your data to it in the latter case... Gr{oetje,eeting}s, Geert
Hi David, On Fri, Jul 19, 2024 at 1:25 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote: > On Mon, Jul 15, 2024 at 8:12 PM David Sterba <dsterba@suse.com> wrote: > > please pull the changes described below. The hilights are new logic > > behind background block group reclaim, automatic removal of qgroup > > after removing a subvolume and new 'rescue=' mount options. The rest is > > optimizations, cleanups and refactoring. > > > > There's a merge conflict caused by the latency fixes from last week in > > extent_map.c:btrfs_scan_inode(), related commits 4e660ca3a98d931809734 > > and b3ebb9b7e92a928344a. Resolved in branch for-6.11-merged and that's > > been in linux-next for a few days. > > FTR, this is broken on 32-bit (doesn't build, good ;-) and on big-endian > (compiler warnings, no idea how it behaves :-(, so you better don't > trust your data to it in the latter case... I cannot find any other report of this, and don't know yet where it was introduced, but the bots started reporting this last May: 3 fs/btrfs/inode.c:5711:5: warning: ‘location.type’ may be used uninitialized in this function [-Wmaybe-uninitialized] 3 fs/btrfs/inode.c:5640:9: warning: ‘location.objectid’ may be used uninitialized in this function [-Wmaybe-uninitialized] https://lore.kernel.org/all/6655b55f.170a0220.406f9.2e0e@mx.google.com/ and I'm seeing failures in e.g. my m68k allmodconfig builds with gcc 9.5 due to CONFIG_WERROR=y. I suspect the big-endian accessors in fs/btrfs/accessors.h lack some initializations? Gr{oetje,eeting}s, Geert
On Fri, Jul 19, 2024 at 01:35:53PM +0200, Geert Uytterhoeven wrote: > Hi David, > > On Fri, Jul 19, 2024 at 1:25 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote: > > On Mon, Jul 15, 2024 at 8:12 PM David Sterba <dsterba@suse.com> wrote: > > > please pull the changes described below. The hilights are new logic > > > behind background block group reclaim, automatic removal of qgroup > > > after removing a subvolume and new 'rescue=' mount options. The rest is > > > optimizations, cleanups and refactoring. > > > > > > There's a merge conflict caused by the latency fixes from last week in > > > extent_map.c:btrfs_scan_inode(), related commits 4e660ca3a98d931809734 > > > and b3ebb9b7e92a928344a. Resolved in branch for-6.11-merged and that's > > > been in linux-next for a few days. > > > > FTR, this is broken on 32-bit (doesn't build, good ;-) and on big-endian > > (compiler warnings, no idea how it behaves :-(, so you better don't > > trust your data to it in the latter case... Internet forums are full of such quick and wrong conclusions, you don't need to write more. > I cannot find any other report of this, and don't know yet where it > was introduced, but the bots started reporting this last May: > > 3 fs/btrfs/inode.c:5711:5: warning: ‘location.type’ may be used > uninitialized in this function [-Wmaybe-uninitialized] > 3 fs/btrfs/inode.c:5640:9: warning: ‘location.objectid’ may be > used uninitialized in this function [-Wmaybe-uninitialized] > > https://lore.kernel.org/all/6655b55f.170a0220.406f9.2e0e@mx.google.com/ > > and I'm seeing failures in e.g. my m68k allmodconfig builds with > gcc 9.5 due to CONFIG_WERROR=y. Older compilers like 9.5 could not be able to reason about variable validity in case it's passed by address, as is in this case: 5707 ret = btrfs_inode_by_name(BTRFS_I(dir), dentry, &location, &di_type); 5708 if (ret < 0) 5709 return ERR_PTR(ret); and btrfs_inode_by_name() returns either a valid 'location' or an error that the caller handles and does not use the variable. > I suspect the big-endian accessors in fs/btrfs/accessors.h lack some > initializations? There are no special accessors on big endian hosts, same code, same bytes in memory only a different order. We fix warnings caused -Wmaybe-uninitialized even if it's because of old compilers, but it's hard to notice reports if they're burried in some mailinglist. I do read your build reports after each -rcN but there are only some modpost warnings in 6.10-rc7 https://lore.kernel.org/all/20240710083744.2885335-1-geert@linux-m68k.org/