On Wed, May 01, 08:36, Darrick J. Wong wrote
You could send this patch to the stable list, but my guess is that they'd prefer a straight backport of all three commits...
Hm, cherry-picking the first commit onto 4.9,171 already gives four conflicting files. The conflicts are trivial to resolve (git cherry-pick -xX theirs 21ec54168b36 does it), but that doesn't compile because xfs_btree_query_all() is missing. So e9a2599a249ed (xfs: create a function to query all records in a btree) is needed as well. But then, applying 86210fbebae (xfs: move various type verifiers to common file) on top of that gives non-trivial conflicts.
Ah, I suspected that might happen. Backports are hard. :(
I suppose one saving grace of the patch you sent is that it'll likely break the build if anyone ever /does/ attempt a backport of those first two commits. Perhaps that is the most practical way forward.
So, for automatic backporting we would need to cherry-pick even more, and each backported commit should be tested of course. Given this, do you still think Greg prefers a rather large set of straight backports over the simple commit that just pulls in the missing function?
I think you'd have to ask him that, if you decide not to send yesterday's patch.
Let's try. I've added a sentence to the commit message which explains why a straight backport is not practical, and how to proceed if anyone wants to backport the earlier commits.
Greg: Under the given circumstances, would you be willing to accept the patch below for 4.9?
Thanks Andre --- commit 6a12a8bda15d5223103df76b8f92cc277c2f5e69 Author: Dave Chinner dchinner@redhat.com Date: Mon Nov 19 13:31:08 2018 -0800
xfs: finobt AG reserves don't consider last AG can be a runt
This is a backport of upstream commit c08768977b9 and the part of 21ec54168b36 which is needed by c08768977b9 to fix a ENOSPC issue due to a bug in the finobt per-ag space reservation code. The bug was observed on an almost empty 100T large filesystem whose last AG happened to be very small.
A direct backport of the above mentioned two commits is not possible because the second commit relies on further infrastructure that was introduced earlier. If anyone ever attempts to backport those earlier commits, this commit has to be reverted first so that the earlier commits apply, and c08768977b9 has to be applied on top of that.
Commit log of c08768977b9 follows.
commit c08768977b9a65cab9bcfd1ba30ffb686b2b7c69 Author: Dave Chinner dchinner@redhat.com Date: Mon Nov 19 13:31:08 2018 -0800
xfs: finobt AG reserves don't consider last AG can be a runt
The last AG may be very small comapred to all other AGs, and hence AG reservations based on the superblock AG size may actually consume more space than the AG actually has. This results on assert failures like:
XFS: Assertion failed: xfs_perag_resv(pag, XFS_AG_RESV_METADATA)->ar_reserved + xfs_perag_resv(pag, XFS_AG_RESV_RMAPBT)->ar_reserved <= pag->pagf_freeblks + pag->pagf_flcount, file: fs/xfs/libxfs/xfs_ag_resv.c, line: 319 [ 48.932891] xfs_ag_resv_init+0x1bd/0x1d0 [ 48.933853] xfs_fs_reserve_ag_blocks+0x37/0xb0 [ 48.934939] xfs_mountfs+0x5b3/0x920 [ 48.935804] xfs_fs_fill_super+0x462/0x640 [ 48.936784] ? xfs_test_remount_options+0x60/0x60 [ 48.937908] mount_bdev+0x178/0x1b0 [ 48.938751] mount_fs+0x36/0x170 [ 48.939533] vfs_kern_mount.part.43+0x54/0x130 [ 48.940596] do_mount+0x20e/0xcb0 [ 48.941396] ? memdup_user+0x3e/0x70 [ 48.942249] ksys_mount+0xba/0xd0 [ 48.943046] __x64_sys_mount+0x21/0x30 [ 48.943953] do_syscall_64+0x54/0x170 [ 48.944835] entry_SYSCALL_64_after_hwframe+0x49/0xbe
Hence we need to ensure the finobt per-ag space reservations take into account the size of the last AG rather than treat it like all the other full size AGs.
Note that both refcountbt and rmapbt already take the size of the AG into account via reading the AGF length directly.
Signed-off-by: Dave Chinner dchinner@redhat.com Reviewed-by: Christoph Hellwig hch@lst.de Reviewed-by: Darrick J. Wong darrick.wong@oracle.com Signed-off-by: Darrick J. Wong darrick.wong@oracle.com
Suggested-by: Darrick J. Wong darrick.wong@oracle.com Tested-by: Andre Noll maan@tuebingen.mpg.de
diff --git a/fs/xfs/libxfs/xfs_ialloc_btree.c b/fs/xfs/libxfs/xfs_ialloc_btree.c index b9c351ff0422..33905989929e 100644 --- a/fs/xfs/libxfs/xfs_ialloc_btree.c +++ b/fs/xfs/libxfs/xfs_ialloc_btree.c @@ -502,17 +502,33 @@ xfs_inobt_rec_check_count( } #endif /* DEBUG */
+/* Find the size of the AG, in blocks. */ +static xfs_agblock_t +xfs_ag_block_count( + struct xfs_mount *mp, + xfs_agnumber_t agno) +{ + ASSERT(agno < mp->m_sb.sb_agcount); + + if (agno < mp->m_sb.sb_agcount - 1) + return mp->m_sb.sb_agblocks; + return mp->m_sb.sb_dblocks - (agno * mp->m_sb.sb_agblocks); +} + static xfs_extlen_t xfs_inobt_max_size( - struct xfs_mount *mp) + struct xfs_mount *mp, + xfs_agnumber_t agno) { + xfs_agblock_t agblocks = xfs_ag_block_count(mp, agno); + /* Bail out if we're uninitialized, which can happen in mkfs. */ if (mp->m_inobt_mxr[0] == 0) return 0;
return xfs_btree_calc_size(mp, mp->m_inobt_mnr, - (uint64_t)mp->m_sb.sb_agblocks * mp->m_sb.sb_inopblock / - XFS_INODES_PER_CHUNK); + (uint64_t)agblocks * mp->m_sb.sb_inopblock / + XFS_INODES_PER_CHUNK); }
static int @@ -558,7 +574,7 @@ xfs_finobt_calc_reserves( if (error) return error;
- *ask += xfs_inobt_max_size(mp); + *ask += xfs_inobt_max_size(mp, agno); *used += tree_len; return 0; }
On Wed, May 01, 2019 at 06:59:33PM +0200, Andre Noll wrote:
On Wed, May 01, 08:36, Darrick J. Wong wrote
You could send this patch to the stable list, but my guess is that they'd prefer a straight backport of all three commits...
Hm, cherry-picking the first commit onto 4.9,171 already gives four conflicting files. The conflicts are trivial to resolve (git cherry-pick -xX theirs 21ec54168b36 does it), but that doesn't compile because xfs_btree_query_all() is missing. So e9a2599a249ed (xfs: create a function to query all records in a btree) is needed as well. But then, applying 86210fbebae (xfs: move various type verifiers to common file) on top of that gives non-trivial conflicts.
Ah, I suspected that might happen. Backports are hard. :(
I suppose one saving grace of the patch you sent is that it'll likely break the build if anyone ever /does/ attempt a backport of those first two commits. Perhaps that is the most practical way forward.
So, for automatic backporting we would need to cherry-pick even more, and each backported commit should be tested of course. Given this, do you still think Greg prefers a rather large set of straight backports over the simple commit that just pulls in the missing function?
I think you'd have to ask him that, if you decide not to send yesterday's patch.
Let's try. I've added a sentence to the commit message which explains why a straight backport is not practical, and how to proceed if anyone wants to backport the earlier commits.
Greg: Under the given circumstances, would you be willing to accept the patch below for 4.9?
If the xfs maintainers say this is ok, it is fine with me.
thanks,
greg k-h
On Wed, May 01, 19:15, Greg Kroah-Hartman wrote
On Wed, May 01, 2019 at 06:59:33PM +0200, Andre Noll wrote:
On Wed, May 01, 08:36, Darrick J. Wong wrote
You could send this patch to the stable list, but my guess is that they'd prefer a straight backport of all three commits...
Hm, cherry-picking the first commit onto 4.9,171 already gives four conflicting files. The conflicts are trivial to resolve (git cherry-pick -xX theirs 21ec54168b36 does it), but that doesn't compile because xfs_btree_query_all() is missing. So e9a2599a249ed (xfs: create a function to query all records in a btree) is needed as well. But then, applying 86210fbebae (xfs: move various type verifiers to common file) on top of that gives non-trivial conflicts.
Ah, I suspected that might happen. Backports are hard. :(
I suppose one saving grace of the patch you sent is that it'll likely break the build if anyone ever /does/ attempt a backport of those first two commits. Perhaps that is the most practical way forward.
So, for automatic backporting we would need to cherry-pick even more, and each backported commit should be tested of course. Given this, do you still think Greg prefers a rather large set of straight backports over the simple commit that just pulls in the missing function?
I think you'd have to ask him that, if you decide not to send yesterday's patch.
Let's try. I've added a sentence to the commit message which explains why a straight backport is not practical, and how to proceed if anyone wants to backport the earlier commits.
Greg: Under the given circumstances, would you be willing to accept the patch below for 4.9?
If the xfs maintainers say this is ok, it is fine with me.
Darrick said, he's in favor of the patch, so I guess I can add his Acked-by. Would you also like to see the ack from Dave (the author of the original commit)?
Anything else that needs to be changed?
Thanks Andre
On Wed, May 01, 2019 at 07:51:29PM +0200, Andre Noll wrote:
On Wed, May 01, 19:15, Greg Kroah-Hartman wrote
On Wed, May 01, 2019 at 06:59:33PM +0200, Andre Noll wrote:
On Wed, May 01, 08:36, Darrick J. Wong wrote
You could send this patch to the stable list, but my guess is that they'd prefer a straight backport of all three commits...
Hm, cherry-picking the first commit onto 4.9,171 already gives four conflicting files. The conflicts are trivial to resolve (git cherry-pick -xX theirs 21ec54168b36 does it), but that doesn't compile because xfs_btree_query_all() is missing. So e9a2599a249ed (xfs: create a function to query all records in a btree) is needed as well. But then, applying 86210fbebae (xfs: move various type verifiers to common file) on top of that gives non-trivial conflicts.
Ah, I suspected that might happen. Backports are hard. :(
I suppose one saving grace of the patch you sent is that it'll likely break the build if anyone ever /does/ attempt a backport of those first two commits. Perhaps that is the most practical way forward.
So, for automatic backporting we would need to cherry-pick even more, and each backported commit should be tested of course. Given this, do you still think Greg prefers a rather large set of straight backports over the simple commit that just pulls in the missing function?
I think you'd have to ask him that, if you decide not to send yesterday's patch.
Let's try. I've added a sentence to the commit message which explains why a straight backport is not practical, and how to proceed if anyone wants to backport the earlier commits.
Greg: Under the given circumstances, would you be willing to accept the patch below for 4.9?
If the xfs maintainers say this is ok, it is fine with me.
Darrick said, he's in favor of the patch, so I guess I can add his Acked-by. Would you also like to see the ack from Dave (the author of the original commit)?
FWIW it seems fine to me, though Dave [cc'd] might have stronger opinions...
Acked-by: Darrick J. Wong darrick.wong@oracle.com
--D
Anything else that needs to be changed?
Thanks Andre -- Max Planck Institute for Developmental Biology Max-Planck-Ring 5, 72076 Tübingen, Germany. Phone: (+49) 7071 601 829 http://people.tuebingen.mpg.de/maan/
On Wed, May 01, 2019 at 12:28:22PM -0700, Darrick J. Wong wrote:
On Wed, May 01, 2019 at 07:51:29PM +0200, Andre Noll wrote:
On Wed, May 01, 19:15, Greg Kroah-Hartman wrote
On Wed, May 01, 2019 at 06:59:33PM +0200, Andre Noll wrote:
On Wed, May 01, 08:36, Darrick J. Wong wrote
> You could send this patch to the stable list, but my guess is that > they'd prefer a straight backport of all three commits...
Hm, cherry-picking the first commit onto 4.9,171 already gives four conflicting files. The conflicts are trivial to resolve (git cherry-pick -xX theirs 21ec54168b36 does it), but that doesn't compile because xfs_btree_query_all() is missing. So e9a2599a249ed (xfs: create a function to query all records in a btree) is needed as well. But then, applying 86210fbebae (xfs: move various type verifiers to common file) on top of that gives non-trivial conflicts.
Ah, I suspected that might happen. Backports are hard. :(
I suppose one saving grace of the patch you sent is that it'll likely break the build if anyone ever /does/ attempt a backport of those first two commits. Perhaps that is the most practical way forward.
So, for automatic backporting we would need to cherry-pick even more, and each backported commit should be tested of course. Given this, do you still think Greg prefers a rather large set of straight backports over the simple commit that just pulls in the missing function?
I think you'd have to ask him that, if you decide not to send yesterday's patch.
Let's try. I've added a sentence to the commit message which explains why a straight backport is not practical, and how to proceed if anyone wants to backport the earlier commits.
Greg: Under the given circumstances, would you be willing to accept the patch below for 4.9?
If the xfs maintainers say this is ok, it is fine with me.
Darrick said, he's in favor of the patch, so I guess I can add his Acked-by. Would you also like to see the ack from Dave (the author of the original commit)?
FWIW it seems fine to me, though Dave [cc'd] might have stronger opinions...
Only thing I care about is whether it is QA'd properly. Greg, Sasha, is the 4.9 stable kernel having fstests run on it as part of the release gating?
Cheers,
Dave.
On Thu, May 02, 2019 at 08:11:07AM +1000, Dave Chinner wrote:
On Wed, May 01, 2019 at 12:28:22PM -0700, Darrick J. Wong wrote:
On Wed, May 01, 2019 at 07:51:29PM +0200, Andre Noll wrote:
On Wed, May 01, 19:15, Greg Kroah-Hartman wrote
On Wed, May 01, 2019 at 06:59:33PM +0200, Andre Noll wrote:
On Wed, May 01, 08:36, Darrick J. Wong wrote
> > You could send this patch to the stable list, but my guess is that > > they'd prefer a straight backport of all three commits... > > Hm, cherry-picking the first commit onto 4.9,171 already gives > four conflicting files. The conflicts are trivial to resolve (git > cherry-pick -xX theirs 21ec54168b36 does it), but that doesn't > compile because xfs_btree_query_all() is missing. So e9a2599a249ed > (xfs: create a function to query all records in a btree) is needed as > well. But then, applying 86210fbebae (xfs: move various type verifiers > to common file) on top of that gives non-trivial conflicts.
Ah, I suspected that might happen. Backports are hard. :(
I suppose one saving grace of the patch you sent is that it'll likely break the build if anyone ever /does/ attempt a backport of those first two commits. Perhaps that is the most practical way forward.
> So, for automatic backporting we would need to cherry-pick even more, > and each backported commit should be tested of course. Given this, do > you still think Greg prefers a rather large set of straight backports > over the simple commit that just pulls in the missing function?
I think you'd have to ask him that, if you decide not to send yesterday's patch.
Let's try. I've added a sentence to the commit message which explains why a straight backport is not practical, and how to proceed if anyone wants to backport the earlier commits.
Greg: Under the given circumstances, would you be willing to accept the patch below for 4.9?
If the xfs maintainers say this is ok, it is fine with me.
Darrick said, he's in favor of the patch, so I guess I can add his Acked-by. Would you also like to see the ack from Dave (the author of the original commit)?
FWIW it seems fine to me, though Dave [cc'd] might have stronger opinions...
Only thing I care about is whether it is QA'd properly. Greg, Sasha, is the 4.9 stable kernel having fstests run on it as part of the release gating?
I do not know about fstests, I know Linaro was looking into doing it as part of the test suites that they verify before I do a release. But I doubt it's run on an XFS filesystem.
Sasha was doing some work in this area though, Sasha?
greg k-h
On Thu, May 02, 2019 at 01:44:40PM +0200, Greg Kroah-Hartman wrote:
On Thu, May 02, 2019 at 08:11:07AM +1000, Dave Chinner wrote:
On Wed, May 01, 2019 at 12:28:22PM -0700, Darrick J. Wong wrote:
On Wed, May 01, 2019 at 07:51:29PM +0200, Andre Noll wrote:
On Wed, May 01, 19:15, Greg Kroah-Hartman wrote
On Wed, May 01, 2019 at 06:59:33PM +0200, Andre Noll wrote:
On Wed, May 01, 08:36, Darrick J. Wong wrote > > > You could send this patch to the stable list, but my guess is that > > > they'd prefer a straight backport of all three commits... > > > > Hm, cherry-picking the first commit onto 4.9,171 already gives > > four conflicting files. The conflicts are trivial to resolve (git > > cherry-pick -xX theirs 21ec54168b36 does it), but that doesn't > > compile because xfs_btree_query_all() is missing. So e9a2599a249ed > > (xfs: create a function to query all records in a btree) is needed as > > well. But then, applying 86210fbebae (xfs: move various type verifiers > > to common file) on top of that gives non-trivial conflicts. > > Ah, I suspected that might happen. Backports are hard. :( > > I suppose one saving grace of the patch you sent is that it'll likely > break the build if anyone ever /does/ attempt a backport of those first > two commits. Perhaps that is the most practical way forward. > > > So, for automatic backporting we would need to cherry-pick even more, > > and each backported commit should be tested of course. Given this, do > > you still think Greg prefers a rather large set of straight backports > > over the simple commit that just pulls in the missing function? > > I think you'd have to ask him that, if you decide not to send > yesterday's patch.
Let's try. I've added a sentence to the commit message which explains why a straight backport is not practical, and how to proceed if anyone wants to backport the earlier commits.
Greg: Under the given circumstances, would you be willing to accept the patch below for 4.9?
If the xfs maintainers say this is ok, it is fine with me.
Darrick said, he's in favor of the patch, so I guess I can add his Acked-by. Would you also like to see the ack from Dave (the author of the original commit)?
FWIW it seems fine to me, though Dave [cc'd] might have stronger opinions...
Only thing I care about is whether it is QA'd properly. Greg, Sasha, is the 4.9 stable kernel having fstests run on it as part of the release gating?
I do not know about fstests, I know Linaro was looking into doing it as part of the test suites that they verify before I do a release. But I doubt it's run on an XFS filesystem.
Sasha was doing some work in this area though, Sasha?
[actually adding Sasha to the email thread this time...]
On Thu, May 02, 2019 at 01:44:40PM +0200, Greg Kroah-Hartman wrote:
On Thu, May 02, 2019 at 08:11:07AM +1000, Dave Chinner wrote:
On Wed, May 01, 2019 at 12:28:22PM -0700, Darrick J. Wong wrote:
On Wed, May 01, 2019 at 07:51:29PM +0200, Andre Noll wrote:
On Wed, May 01, 19:15, Greg Kroah-Hartman wrote
On Wed, May 01, 2019 at 06:59:33PM +0200, Andre Noll wrote:
On Wed, May 01, 08:36, Darrick J. Wong wrote > > > You could send this patch to the stable list, but my guess is that > > > they'd prefer a straight backport of all three commits... > > > > Hm, cherry-picking the first commit onto 4.9,171 already gives > > four conflicting files. The conflicts are trivial to resolve (git > > cherry-pick -xX theirs 21ec54168b36 does it), but that doesn't > > compile because xfs_btree_query_all() is missing. So e9a2599a249ed > > (xfs: create a function to query all records in a btree) is needed as > > well. But then, applying 86210fbebae (xfs: move various type verifiers > > to common file) on top of that gives non-trivial conflicts. > > Ah, I suspected that might happen. Backports are hard. :( > > I suppose one saving grace of the patch you sent is that it'll likely > break the build if anyone ever /does/ attempt a backport of those first > two commits. Perhaps that is the most practical way forward. > > > So, for automatic backporting we would need to cherry-pick even more, > > and each backported commit should be tested of course. Given this, do > > you still think Greg prefers a rather large set of straight backports > > over the simple commit that just pulls in the missing function? > > I think you'd have to ask him that, if you decide not to send > yesterday's patch.
Let's try. I've added a sentence to the commit message which explains why a straight backport is not practical, and how to proceed if anyone wants to backport the earlier commits.
Greg: Under the given circumstances, would you be willing to accept the patch below for 4.9?
If the xfs maintainers say this is ok, it is fine with me.
Darrick said, he's in favor of the patch, so I guess I can add his Acked-by. Would you also like to see the ack from Dave (the author of the original commit)?
FWIW it seems fine to me, though Dave [cc'd] might have stronger opinions...
Only thing I care about is whether it is QA'd properly. Greg, Sasha, is the 4.9 stable kernel having fstests run on it as part of the release gating?
I do not know about fstests, I know Linaro was looking into doing it as part of the test suites that they verify before I do a release. But I doubt it's run on an XFS filesystem.
Sasha was doing some work in this area though, Sasha?
My biggest blocker at this point that it is extremely difficult to get a baseline for a kernel version.
Even after adding all the "known" failures to the expunge list, I ket hitting issues that reproduced once every 100 runs, and once those are expunged I started hitting even rarer stuff.
While there's no actual real difficulty in building these expunge lists, it ended up being somewhat of a loop where I couldn't establish a solid baseline since random things kept breaking.
-- Thanks, Sasha
On Thu, May 02, 2019 at 09:20:27AM -0400, Sasha Levin wrote:
On Thu, May 02, 2019 at 01:44:40PM +0200, Greg Kroah-Hartman wrote:
On Thu, May 02, 2019 at 08:11:07AM +1000, Dave Chinner wrote:
On Wed, May 01, 2019 at 12:28:22PM -0700, Darrick J. Wong wrote:
On Wed, May 01, 2019 at 07:51:29PM +0200, Andre Noll wrote:
On Wed, May 01, 19:15, Greg Kroah-Hartman wrote
On Wed, May 01, 2019 at 06:59:33PM +0200, Andre Noll wrote: > On Wed, May 01, 08:36, Darrick J. Wong wrote > > > > You could send this patch to the stable list, but my guess is that > > > > they'd prefer a straight backport of all three commits... > > > > > > Hm, cherry-picking the first commit onto 4.9,171 already gives > > > four conflicting files. The conflicts are trivial to resolve (git > > > cherry-pick -xX theirs 21ec54168b36 does it), but that doesn't > > > compile because xfs_btree_query_all() is missing. So e9a2599a249ed > > > (xfs: create a function to query all records in a btree) is needed as > > > well. But then, applying 86210fbebae (xfs: move various type verifiers > > > to common file) on top of that gives non-trivial conflicts. > > > > Ah, I suspected that might happen. Backports are hard. :( > > > > I suppose one saving grace of the patch you sent is that it'll likely > > break the build if anyone ever /does/ attempt a backport of those first > > two commits. Perhaps that is the most practical way forward. > > > > > So, for automatic backporting we would need to cherry-pick even more, > > > and each backported commit should be tested of course. Given this, do > > > you still think Greg prefers a rather large set of straight backports > > > over the simple commit that just pulls in the missing function? > > > > I think you'd have to ask him that, if you decide not to send > > yesterday's patch. > > Let's try. I've added a sentence to the commit message which explains > why a straight backport is not practical, and how to proceed if anyone > wants to backport the earlier commits. > > Greg: Under the given circumstances, would you be willing to accept > the patch below for 4.9?
If the xfs maintainers say this is ok, it is fine with me.
Darrick said, he's in favor of the patch, so I guess I can add his Acked-by. Would you also like to see the ack from Dave (the author of the original commit)?
FWIW it seems fine to me, though Dave [cc'd] might have stronger opinions...
Only thing I care about is whether it is QA'd properly. Greg, Sasha, is the 4.9 stable kernel having fstests run on it as part of the release gating?
I do not know about fstests, I know Linaro was looking into doing it as part of the test suites that they verify before I do a release. But I doubt it's run on an XFS filesystem.
Sasha was doing some work in this area though, Sasha?
My biggest blocker at this point that it is extremely difficult to get a baseline for a kernel version.
Even after adding all the "known" failures to the expunge list, I ket hitting issues that reproduced once every 100 runs, and once those are expunged I started hitting even rarer stuff.
While there's no actual real difficulty in building these expunge lists, it ended up being somewhat of a loop where I couldn't establish a solid baseline since random things kept breaking.
Ok, then how about we hold off on this patch for 4.9.y then. "no one" should be using 4.9.y in a "server system" anymore, unless you happen to have an enterprise kernel based on it. So we should be fine as the users of the older kernels don't run xfs.
thanks,
greg k-h
On Thu, May 02, 16:10, Greg Kroah-Hartman wrote
Ok, then how about we hold off on this patch for 4.9.y then. "no one" should be using 4.9.y in a "server system" anymore, unless you happen to have an enterprise kernel based on it. So we should be fine as the users of the older kernels don't run xfs.
Well, we do run xfs on top of bcache on vanilla 4.9 kernels on a few dozen production servers here. Mainly because we ran into all sorts of issues with newer kernels (not necessary related to xfs). 4.9, OTOH, appears to be rock solid for our workload.
But I'm OK with holding off on the patch. According to Darrick, the issue that is fixed by the patch should not cause serious problems anyway if xfs debugging is turned off.
Best Andre
On Thu, May 02, 2019 at 05:27:36PM +0200, Andre Noll wrote:
On Thu, May 02, 16:10, Greg Kroah-Hartman wrote
Ok, then how about we hold off on this patch for 4.9.y then. "no one" should be using 4.9.y in a "server system" anymore, unless you happen to have an enterprise kernel based on it. So we should be fine as the users of the older kernels don't run xfs.
Well, we do run xfs on top of bcache on vanilla 4.9 kernels on a few dozen production servers here. Mainly because we ran into all sorts of issues with newer kernels (not necessary related to xfs). 4.9, OTOH, appears to be rock solid for our workload.
Great, but what is wrong with 4.14.y or better yet, 4.19.y? Do those also work for your workload? If not, we should fix that, and soon :)
I would _STRONGLY_ recommend moving of of 4.9 on any non-SoC-based system at this point in time, there should not be any reason to stick with it, unless you are paying a company to provide support for it.
thanks,
greg k-h
On Thu, May 02, 18:52, Greg Kroah-Hartman wrote
On Thu, May 02, 2019 at 05:27:36PM +0200, Andre Noll wrote:
On Thu, May 02, 16:10, Greg Kroah-Hartman wrote
Ok, then how about we hold off on this patch for 4.9.y then. "no one" should be using 4.9.y in a "server system" anymore, unless you happen to have an enterprise kernel based on it. So we should be fine as the users of the older kernels don't run xfs.
Well, we do run xfs on top of bcache on vanilla 4.9 kernels on a few dozen production servers here. Mainly because we ran into all sorts of issues with newer kernels (not necessary related to xfs). 4.9, OTOH, appears to be rock solid for our workload.
Great, but what is wrong with 4.14.y or better yet, 4.19.y? Do those also work for your workload? If not, we should fix that, and soon :)
Some months ago we tried 4.14 and it was a real disaster: random crashes with nothing in the logs on the file servers and unkillable hung processes on the compute machines. The thing is, I can't afford an extended downtime of these production systems, or test patches, or enable debugging options which slow down the systems too much. Also, 10 of the compute nodes load the nvidia module, so all bets are off anyway. But we've seen the hung processes also on the non-gpu nodes where the nvidia module is not loaded.
As for 4.19, xfs on bcache was broken until a couple of weeks ago. Meanwhile the fix (e578f90d8a9c) went in, so I benchmarked 4.19.x on one system briefly. To my surprise the results were *worse* than with 4.9. This seems to be another cache bypass issue, but I need to have a closer look, and more reliable numbers.
I would _STRONGLY_ recommend moving of of 4.9 on any non-SoC-based system at this point in time, there should not be any reason to stick with it, unless you are paying a company to provide support for it.
That's really bad news :(
Thanks for sharing your thoughts about the future of 4.9, though. I'll try to spend some time on the bcache issue on 4.19.
Best Andre
On Thu, May 02, 2019 at 07:45:16PM +0200, Andre Noll wrote:
On Thu, May 02, 18:52, Greg Kroah-Hartman wrote
On Thu, May 02, 2019 at 05:27:36PM +0200, Andre Noll wrote:
On Thu, May 02, 16:10, Greg Kroah-Hartman wrote
Ok, then how about we hold off on this patch for 4.9.y then. "no one" should be using 4.9.y in a "server system" anymore, unless you happen to have an enterprise kernel based on it. So we should be fine as the users of the older kernels don't run xfs.
Well, we do run xfs on top of bcache on vanilla 4.9 kernels on a few dozen production servers here. Mainly because we ran into all sorts of issues with newer kernels (not necessary related to xfs). 4.9, OTOH, appears to be rock solid for our workload.
Great, but what is wrong with 4.14.y or better yet, 4.19.y? Do those also work for your workload? If not, we should fix that, and soon :)
Some months ago we tried 4.14 and it was a real disaster: random crashes with nothing in the logs on the file servers and unkillable hung processes on the compute machines. The thing is, I can't afford an extended downtime of these production systems, or test patches, or enable debugging options which slow down the systems too much. Also, 10 of the compute nodes load the nvidia module, so all bets are off anyway. But we've seen the hung processes also on the non-gpu nodes where the nvidia module is not loaded.
As for 4.19, xfs on bcache was broken until a couple of weeks ago. Meanwhile the fix (e578f90d8a9c) went in, so I benchmarked 4.19.x on one system briefly. To my surprise the results were *worse* than with 4.9. This seems to be another cache bypass issue, but I need to have a closer look, and more reliable numbers.
Is this something you can reproduce outside of those 10 magical machines?
-- Thanks, Sasha
On Thu, May 02, 2019 at 08:11:07AM +1000, Dave Chinner wrote:
On Wed, May 01, 2019 at 12:28:22PM -0700, Darrick J. Wong wrote:
On Wed, May 01, 2019 at 07:51:29PM +0200, Andre Noll wrote:
On Wed, May 01, 19:15, Greg Kroah-Hartman wrote
On Wed, May 01, 2019 at 06:59:33PM +0200, Andre Noll wrote:
On Wed, May 01, 08:36, Darrick J. Wong wrote
> > You could send this patch to the stable list, but my guess is that > > they'd prefer a straight backport of all three commits... > > Hm, cherry-picking the first commit onto 4.9,171 already gives > four conflicting files. The conflicts are trivial to resolve (git > cherry-pick -xX theirs 21ec54168b36 does it), but that doesn't > compile because xfs_btree_query_all() is missing. So e9a2599a249ed > (xfs: create a function to query all records in a btree) is needed as > well. But then, applying 86210fbebae (xfs: move various type verifiers > to common file) on top of that gives non-trivial conflicts.
Ah, I suspected that might happen. Backports are hard. :(
I suppose one saving grace of the patch you sent is that it'll likely break the build if anyone ever /does/ attempt a backport of those first two commits. Perhaps that is the most practical way forward.
> So, for automatic backporting we would need to cherry-pick even more, > and each backported commit should be tested of course. Given this, do > you still think Greg prefers a rather large set of straight backports > over the simple commit that just pulls in the missing function?
I think you'd have to ask him that, if you decide not to send yesterday's patch.
Let's try. I've added a sentence to the commit message which explains why a straight backport is not practical, and how to proceed if anyone wants to backport the earlier commits.
Greg: Under the given circumstances, would you be willing to accept the patch below for 4.9?
If the xfs maintainers say this is ok, it is fine with me.
Darrick said, he's in favor of the patch, so I guess I can add his Acked-by. Would you also like to see the ack from Dave (the author of the original commit)?
FWIW it seems fine to me, though Dave [cc'd] might have stronger opinions...
Only thing I care about is whether it is QA'd properly. Greg, Sasha, is the 4.9 stable kernel having fstests run on it as part of the release gating?
I test only for 5.1 and 4.19 and this point. I don't have a solid baseline for anything older.
-- Thanks, Sasha
linux-stable-mirror@lists.linaro.org
-
Andre Noll -
Darrick J. Wong -
Dave Chinner -
Greg Kroah-Hartman -
Sasha Levin