On Mon, 12 Jul 2021, Greg Kroah-Hartman wrote:
This is the start of the stable review cycle for the 5.13.2 release. There are 800 patches in this series, all will be posted as a response to this one. If anyone has any issues with these being applied, please let me know.
Responses should be made by Wed, 14 Jul 2021 06:02:46 +0000. Anything received after that time might be too late.
The whole patch series can be found in one patch at: https://www.kernel.org/pub/linux/kernel/v5.x/stable-review/patch-5.13.2-rc1.... or in the git tree and branch at: git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git linux-5.13.y and the diffstat can be found below.
thanks,
greg k-h
Pseudo-Shortlog of commits:
Greg Kroah-Hartman gregkh@linuxfoundation.org Linux 5.13.2-rc1
Hi Greg,
Sorry to be making waves, but please, what's up with the 5.13.2-rc, 5.12.17-rc, 5.10.50-rc, 5.4.132-rc stable release candidates?
Amongst the 2000+ patches posted today, there are a significant number of them Signed-off-by Andrew, Signed-off-by Linus, Signed-off-by Sasha: yet never Cc'ed to stable (nor even posted as AUTOSELs, I think).
Am I out of date? I thought that had been agreed not to happen: https://lore.kernel.org/linux-mm/20190808000533.7701-1-mike.kravetz@oracle.c... is the thread I found when I looked for confirmation, but I believe the same has been agreed before and since too.
Andrew goes to a lot of trouble to establish which Fixes from his tree ought to go to stable. Of course there will be exceptions which we later decide should go in after all; but it's worrying when there's a wholesale breach like this, and I think most of them should be dropped.
To pick on just one of many examples (sorry Miaohe!), a patch that surprises me, but I've not had time to look into so far, and would not want accelerated into X stable releases, 385/800
Miaohe Lin linmiaohe@huawei.com mm/shmem: fix shmem_swapin() race with swapoff
Hugh
On Mon, Jul 12, 2021 at 10:55:01PM -0700, Hugh Dickins wrote:
On Mon, 12 Jul 2021, Greg Kroah-Hartman wrote:
This is the start of the stable review cycle for the 5.13.2 release. There are 800 patches in this series, all will be posted as a response to this one. If anyone has any issues with these being applied, please let me know.
Responses should be made by Wed, 14 Jul 2021 06:02:46 +0000. Anything received after that time might be too late.
The whole patch series can be found in one patch at: https://www.kernel.org/pub/linux/kernel/v5.x/stable-review/patch-5.13.2-rc1.... or in the git tree and branch at: git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git linux-5.13.y and the diffstat can be found below.
thanks,
greg k-h
Pseudo-Shortlog of commits:
Greg Kroah-Hartman gregkh@linuxfoundation.org Linux 5.13.2-rc1
Hi Greg,
Sorry to be making waves, but please, what's up with the 5.13.2-rc, 5.12.17-rc, 5.10.50-rc, 5.4.132-rc stable release candidates?
They show the problem that we currently have where maintainers wait at the end of the -rc cycle and keep valid fixes from being sent to Linus. They "bunch up" and come out only in -rc1 and so the first few stable releases after -rc1 comes out is huge. It's been happening for the past few years and only getting worse. These stable releases are proof of that, the 5.13.2-rc release was the largest we have ever done and it broke one of my scripts because of it :(
I know personally I do this for my subsystems, having fixes that are trivial things batch up for -rc1 just because they are generally not worth getting into -final. But that is not the case with many other subsystems as you can see by these huge patch sequences.
Amongst the 2000+ patches posted today, there are a significant number of them Signed-off-by Andrew, Signed-off-by Linus, Signed-off-by Sasha: yet never Cc'ed to stable (nor even posted as AUTOSELs, I think).
Am I out of date? I thought that had been agreed not to happen: https://lore.kernel.org/linux-mm/20190808000533.7701-1-mike.kravetz@oracle.c... is the thread I found when I looked for confirmation, but I believe the same has been agreed before and since too.
Andrew goes to a lot of trouble to establish which Fixes from his tree ought to go to stable. Of course there will be exceptions which we later decide should go in after all; but it's worrying when there's a wholesale breach like this, and I think most of them should be dropped.
To pick on just one of many examples (sorry Miaohe!), a patch that surprises me, but I've not had time to look into so far, and would not want accelerated into X stable releases, 385/800
Miaohe Lin linmiaohe@huawei.com mm/shmem: fix shmem_swapin() race with swapoff
Sasha, and I, take patches from Linus's tree like the above one that have "Fixes:" tags in them as many many maintainers do not remember to put "cc: stable" on their patches.
The above patch says it fixes a problem in the 5.1 kernel release, so Sasha queued it up for 5.10, 5.12, and 5.13. Odds are he should have also sent a "FAILED" notice for 5.4, but we don't always do that for patches only with a Fixes tag all the time as we only have so much we can do...
So is that tag incorrect? If not, why was it not cc: stable? Why is it not valid for a stable release? So far, all automated testing seems to show that there are no regressions in these releases with these commits in them. If there was a problem, how would it show up?
And as far as I know, mm/ stuff is still not triggered by the AUTOSEL bot, but that is not what caused this commit to be added to a stable release.
Trying to keep a "do not apply" list for Fixes: tags only is much harder for both of us as we do these semi-manually and review them individually. Trying to remember what subsystem only does Fixes tags yet really doesn't mean it is an impossible task.
We are glad to drop any patch added to a -rc release, just let us know. We are also glad to revert them later on if a developer/maintainer does not catch them in time before a release happens.
thanks,
greg k-h
On Tue, Jul 13, 2021 at 08:31:57AM +0200, Greg Kroah-Hartman wrote:
On Mon, Jul 12, 2021 at 10:55:01PM -0700, Hugh Dickins wrote:
On Mon, 12 Jul 2021, Greg Kroah-Hartman wrote:
This is the start of the stable review cycle for the 5.13.2 release. There are 800 patches in this series, all will be posted as a response to this one. If anyone has any issues with these being applied, please let me know.
Responses should be made by Wed, 14 Jul 2021 06:02:46 +0000. Anything received after that time might be too late.
The whole patch series can be found in one patch at: https://www.kernel.org/pub/linux/kernel/v5.x/stable-review/patch-5.13.2-rc1.... or in the git tree and branch at: git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git linux-5.13.y and the diffstat can be found below.
thanks,
greg k-h
Pseudo-Shortlog of commits:
Greg Kroah-Hartman gregkh@linuxfoundation.org Linux 5.13.2-rc1
Hi Greg,
Sorry to be making waves, but please, what's up with the 5.13.2-rc, 5.12.17-rc, 5.10.50-rc, 5.4.132-rc stable release candidates?
They show the problem that we currently have where maintainers wait at the end of the -rc cycle and keep valid fixes from being sent to Linus. They "bunch up" and come out only in -rc1 and so the first few stable releases after -rc1 comes out is huge. It's been happening for the past few years and only getting worse. These stable releases are proof of that, the 5.13.2-rc release was the largest we have ever done and it broke one of my scripts because of it :(
I know personally I do this for my subsystems, having fixes that are trivial things batch up for -rc1 just because they are generally not worth getting into -final. But that is not the case with many other subsystems as you can see by these huge patch sequences.
Hm, maybe it really isn't a "problem" here, as the % overall is still quite low of patches with fixes: and cc: stable on them compared to the overall number of commits going in for -rc1 vs. later -rcX releases.
So it's just what it is, large numbers of changes happening, small % of them are needed to be backported. If someone wanted to, odds are they could get a master's thesis out of analyzing all of this stuff :)
thanks,
greg k-h
On Tue, 13 Jul 2021 08:31:57 +0200 Greg Kroah-Hartman gregkh@linuxfoundation.org wrote:
Amongst the 2000+ patches posted today, there are a significant number of them Signed-off-by Andrew, Signed-off-by Linus, Signed-off-by Sasha: yet never Cc'ed to stable (nor even posted as AUTOSELs, I think).
Am I out of date? I thought that had been agreed not to happen: https://lore.kernel.org/linux-mm/20190808000533.7701-1-mike.kravetz@oracle.c... is the thread I found when I looked for confirmation, but I believe the same has been agreed before and since too.
Andrew goes to a lot of trouble to establish which Fixes from his tree ought to go to stable. Of course there will be exceptions which we later decide should go in after all; but it's worrying when there's a wholesale breach like this, and I think most of them should be dropped.
To pick on just one of many examples (sorry Miaohe!), a patch that surprises me, but I've not had time to look into so far, and would not want accelerated into X stable releases, 385/800
Miaohe Lin linmiaohe@huawei.com mm/shmem: fix shmem_swapin() race with swapoff
Sasha, and I, take patches from Linus's tree like the above one that have "Fixes:" tags in them as many many maintainers do not remember to put "cc: stable" on their patches.
As do many many developers. I always check.
The above patch says it fixes a problem in the 5.1 kernel release, so Sasha queued it up for 5.10, 5.12, and 5.13. Odds are he should have also sent a "FAILED" notice for 5.4, but we don't always do that for patches only with a Fixes tag all the time as we only have so much we can do...
So is that tag incorrect? If not, why was it not cc: stable? Why is it not valid for a stable release?
Usually because we judged that the seriousness of the problem did not justify the risk & churn of backporting its fix.
So far, all automated testing seems to show that there are no regressions in these releases with these commits in them. If there was a problem, how would it show up?
And as far as I know, mm/ stuff is still not triggered by the AUTOSEL bot, but that is not what caused this commit to be added to a stable release.
Trying to keep a "do not apply" list for Fixes: tags only is much harder for both of us as we do these semi-manually and review them individually. Trying to remember what subsystem only does Fixes tags yet really doesn't mean it is an impossible task.
Well, it shouldn't be super hard to skip all patches which have Fixes:, Signed-off-by:akpm and no cc:stable?
I'd really really prefer this, please. At present this -stable promiscuity is overriding the (sometime carefully) considered decisions of the MM developers, and that's a bit scary. I've actually been spending the past couple of years believing that if I left off cc:stable, the fix wasn't going to go into -stable!
Alternatively I could just invent a new tag to replace the "Fixes:" ("Fixes-no-backport?") to be used on patches which fix a known previous commit but which we don't want backported.
On 14. 07. 21, 3:28, Andrew Morton wrote:
Alternatively I could just invent a new tag to replace the "Fixes:" ("Fixes-no-backport?") to be used on patches which fix a known previous commit but which we don't want backported.
Or what about: No-stable: <reason> like: No-stable: too intrusive No-stable: too dangerous No-stable: <5.10, isn't vulnerable
So that the reason is known at least.
thanks,
On Tue 13-07-21 18:28:13, Andrew Morton wrote: [...]
Trying to keep a "do not apply" list for Fixes: tags only is much harder for both of us as we do these semi-manually and review them individually. Trying to remember what subsystem only does Fixes tags yet really doesn't mean it is an impossible task.
Well, it shouldn't be super hard to skip all patches which have Fixes:, Signed-off-by:akpm and no cc:stable?
I'd really really prefer this, please.
Yes please!
At present this -stable promiscuity is overriding the (sometime carefully) considered decisions of the MM developers, and that's a bit scary.
Not only scary, it is also a waste of precious time of those who carefuly evaluate stable tree backports.
I've actually been spending the past couple of years believing that if I left off cc:stable, the fix wasn't going to go into -stable!
Alternatively I could just invent a new tag to replace the "Fixes:" ("Fixes-no-backport?") to be used on patches which fix a known previous commit but which we don't want backported.
Please no. We already do have a way to mark for stable trees. The fact that stable kernel maintainers tend oto ignore that shouldn't put the burden to developers/maintainers. But hey, if stable maintainers really want to push to quantity over quality then be it....
On Wed, Jul 14, 2021 at 09:52:58AM +0200, Michal Hocko wrote:
On Tue 13-07-21 18:28:13, Andrew Morton wrote:
At present this -stable promiscuity is overriding the (sometime carefully) considered decisions of the MM developers, and that's a bit scary.
Not only scary, it is also a waste of precious time of those who carefuly evaluate stable tree backports.
I'm just as concerned with the other direction: we end up missing quite a lot of patches that are needed in practice, and no one is circling back to make sure that we have everything we need.
I took a peek at SUSE's tree to see how things work there, and looking at the very latest mm/ commit:
commit c8c7b321edcf7a7e8c22dc66e0366f72aa2390f0 Author: Michal Koutný mkoutny@suse.com Date: Tue May 4 11:12:10 2021 +0200
mm: memcontrol: fix cpuhotplug statistics flushing (bsc#1185606).
suse-commit: 3bba386a33fac144abf2507554cb21552acb16af
This seems to be commit a3d4c05a4474 ("mm: memcontrol: fix cpuhotplug statistics flushing") upstream, and I assume that it was picked because it fixed a real bug someone cares about.
I can maybe understand that at the time that the patch was written/committed it didn't seem like stable@ material and thus there was no cc to stable.
But once someone realized it needs to be backported, why weren't we told to take it into stable too?
On Wed 14-07-21 11:30:46, Sasha Levin wrote:
On Wed, Jul 14, 2021 at 09:52:58AM +0200, Michal Hocko wrote:
On Tue 13-07-21 18:28:13, Andrew Morton wrote:
At present this -stable promiscuity is overriding the (sometime carefully) considered decisions of the MM developers, and that's a bit scary.
Not only scary, it is also a waste of precious time of those who carefuly evaluate stable tree backports.
I'm just as concerned with the other direction: we end up missing quite a lot of patches that are needed in practice, and no one is circling back to make sure that we have everything we need.
I took a peek at SUSE's tree to see how things work there, and looking at the very latest mm/ commit:
commit c8c7b321edcf7a7e8c22dc66e0366f72aa2390f0 Author: Michal Koutný mkoutny@suse.com Date: Tue May 4 11:12:10 2021 +0200
mm: memcontrol: fix cpuhotplug statistics flushing (bsc#1185606). suse-commit: 3bba386a33fac144abf2507554cb21552acb16afThis seems to be commit a3d4c05a4474 ("mm: memcontrol: fix cpuhotplug statistics flushing") upstream, and I assume that it was picked because it fixed a real bug someone cares about.
Nope. It has been identified as potentially useful/nice to have. There was no actual bug report requiring it. We do that a lot. In fact we do have a full infrastructure around git fixes and backport fixes proactively. Mostly because stable tree, which we used to track in the past, has turned out to be overwhelming with questionable/risky backports. The thing, though, is that those fixes are carefully reviewed by a domain expert before backporting.
I can maybe understand that at the time that the patch was written/committed it didn't seem like stable@ material and thus there was no cc to stable.
But once someone realized it needs to be backported, why weren't we told to take it into stable too?
We tend to do that for many real bug reports.
On Wed, Jul 14, 2021 at 10:30 AM Sasha Levin sashal@kernel.org wrote:
On Wed, Jul 14, 2021 at 09:52:58AM +0200, Michal Hocko wrote:
On Tue 13-07-21 18:28:13, Andrew Morton wrote:
At present this -stable promiscuity is overriding the (sometime carefully) considered decisions of the MM developers, and that's a bit scary.
Not only scary, it is also a waste of precious time of those who carefuly evaluate stable tree backports.
I'm just as concerned with the other direction: we end up missing quite a lot of patches that are needed in practice, and no one is circling back to make sure that we have everything we need.
It does work both ways. For those of us maintaining a kernel for a community distro, without an army of engineers actually paying attention, the current stable process has fixed more bugs than it has introduced. But it does occasionally introduce them as well. When it does, it is typically pretty easy to see where, as stable releases tend to be smaller than this set was, so only a few patches in any given subsystem or driver. If we go back to a case where only Cc: stable patches are selected, I suppose the logical step would be for maintainers like me to make sure that we send a message to stable whenever we pull a patch from upstream that fixes an actual issue that users are seeing. I don't have a strong objection to this, but it is more work.
Justin
I took a peek at SUSE's tree to see how things work there, and looking at the very latest mm/ commit:
commit c8c7b321edcf7a7e8c22dc66e0366f72aa2390f0 Author: Michal Koutný mkoutny@suse.com Date: Tue May 4 11:12:10 2021 +0200
mm: memcontrol: fix cpuhotplug statistics flushing (bsc#1185606). suse-commit: 3bba386a33fac144abf2507554cb21552acb16afThis seems to be commit a3d4c05a4474 ("mm: memcontrol: fix cpuhotplug statistics flushing") upstream, and I assume that it was picked because it fixed a real bug someone cares about.
I can maybe understand that at the time that the patch was written/committed it didn't seem like stable@ material and thus there was no cc to stable.
But once someone realized it needs to be backported, why weren't we told to take it into stable too?
-- Thanks, Sasha
On Tue, Jul 13, 2021 at 06:28:13PM -0700, Andrew Morton wrote:
On Tue, 13 Jul 2021 08:31:57 +0200 Greg Kroah-Hartman gregkh@linuxfoundation.org wrote:
Amongst the 2000+ patches posted today, there are a significant number of them Signed-off-by Andrew, Signed-off-by Linus, Signed-off-by Sasha: yet never Cc'ed to stable (nor even posted as AUTOSELs, I think).
Am I out of date? I thought that had been agreed not to happen: https://lore.kernel.org/linux-mm/20190808000533.7701-1-mike.kravetz@oracle.c... is the thread I found when I looked for confirmation, but I believe the same has been agreed before and since too.
Andrew goes to a lot of trouble to establish which Fixes from his tree ought to go to stable. Of course there will be exceptions which we later decide should go in after all; but it's worrying when there's a wholesale breach like this, and I think most of them should be dropped.
To pick on just one of many examples (sorry Miaohe!), a patch that surprises me, but I've not had time to look into so far, and would not want accelerated into X stable releases, 385/800
Miaohe Lin linmiaohe@huawei.com mm/shmem: fix shmem_swapin() race with swapoff
Sasha, and I, take patches from Linus's tree like the above one that have "Fixes:" tags in them as many many maintainers do not remember to put "cc: stable" on their patches.
As do many many developers. I always check.
The above patch says it fixes a problem in the 5.1 kernel release, so Sasha queued it up for 5.10, 5.12, and 5.13. Odds are he should have also sent a "FAILED" notice for 5.4, but we don't always do that for patches only with a Fixes tag all the time as we only have so much we can do...
So is that tag incorrect? If not, why was it not cc: stable? Why is it not valid for a stable release?
Usually because we judged that the seriousness of the problem did not justify the risk & churn of backporting its fix.
So far, all automated testing seems to show that there are no regressions in these releases with these commits in them. If there was a problem, how would it show up?
And as far as I know, mm/ stuff is still not triggered by the AUTOSEL bot, but that is not what caused this commit to be added to a stable release.
Trying to keep a "do not apply" list for Fixes: tags only is much harder for both of us as we do these semi-manually and review them individually. Trying to remember what subsystem only does Fixes tags yet really doesn't mean it is an impossible task.
Well, it shouldn't be super hard to skip all patches which have Fixes:, Signed-off-by:akpm and no cc:stable?
Ok, I will do this now (goes and writes this down...)
But it really feels odd that you all take the time to add a "Hey, this fixes this specific commit!" tag in the changelog, yet you do not actually want to go and fix the kernels that have that commit in it. This is an odd signal to others that watch the changelogs for context clues. Perhaps you might not want to do that anymore.
I'd really really prefer this, please. At present this -stable promiscuity is overriding the (sometime carefully) considered decisions of the MM developers, and that's a bit scary. I've actually been spending the past couple of years believing that if I left off cc:stable, the fix wasn't going to go into -stable!
That used to be the case, but we have had to deal with all of the subsystems where people were NOT putting cc: stable on them, and only Fixes: tags. It's slowly getting better, but some subsystems refuse to do this for some reason (it's hard to wrangle 4000 people to all do the same thing...)
Alternatively I could just invent a new tag to replace the "Fixes:" ("Fixes-no-backport?") to be used on patches which fix a known previous commit but which we don't want backported.
No please, that's not needed, I'll just ignore these types of patches now, and will go drop these from the queues.
Sasha, can you also add these to your "do not apply" script as well?
thanks,
greg k-h
On Wed, Jul 14, 2021 at 11:18:14AM +0200, Greg Kroah-Hartman wrote:
On Tue, Jul 13, 2021 at 06:28:13PM -0700, Andrew Morton wrote:
On Tue, 13 Jul 2021 08:31:57 +0200 Greg Kroah-Hartman gregkh@linuxfoundation.org wrote:
Amongst the 2000+ patches posted today, there are a significant number of them Signed-off-by Andrew, Signed-off-by Linus, Signed-off-by Sasha: yet never Cc'ed to stable (nor even posted as AUTOSELs, I think).
Am I out of date? I thought that had been agreed not to happen: https://lore.kernel.org/linux-mm/20190808000533.7701-1-mike.kravetz@oracle.c... is the thread I found when I looked for confirmation, but I believe the same has been agreed before and since too.
Andrew goes to a lot of trouble to establish which Fixes from his tree ought to go to stable. Of course there will be exceptions which we later decide should go in after all; but it's worrying when there's a wholesale breach like this, and I think most of them should be dropped.
To pick on just one of many examples (sorry Miaohe!), a patch that surprises me, but I've not had time to look into so far, and would not want accelerated into X stable releases, 385/800
Miaohe Lin linmiaohe@huawei.com mm/shmem: fix shmem_swapin() race with swapoff
Sasha, and I, take patches from Linus's tree like the above one that have "Fixes:" tags in them as many many maintainers do not remember to put "cc: stable" on their patches.
As do many many developers. I always check.
The above patch says it fixes a problem in the 5.1 kernel release, so Sasha queued it up for 5.10, 5.12, and 5.13. Odds are he should have also sent a "FAILED" notice for 5.4, but we don't always do that for patches only with a Fixes tag all the time as we only have so much we can do...
So is that tag incorrect? If not, why was it not cc: stable? Why is it not valid for a stable release?
Usually because we judged that the seriousness of the problem did not justify the risk & churn of backporting its fix.
So far, all automated testing seems to show that there are no regressions in these releases with these commits in them. If there was a problem, how would it show up?
And as far as I know, mm/ stuff is still not triggered by the AUTOSEL bot, but that is not what caused this commit to be added to a stable release.
Trying to keep a "do not apply" list for Fixes: tags only is much harder for both of us as we do these semi-manually and review them individually. Trying to remember what subsystem only does Fixes tags yet really doesn't mean it is an impossible task.
Well, it shouldn't be super hard to skip all patches which have Fixes:, Signed-off-by:akpm and no cc:stable?
Ok, I will do this now (goes and writes this down...)
But it really feels odd that you all take the time to add a "Hey, this fixes this specific commit!" tag in the changelog, yet you do not actually want to go and fix the kernels that have that commit in it. This is an odd signal to others that watch the changelogs for context clues. Perhaps you might not want to do that anymore.
I looked at some of these patches and it seems really odd to me that you all are marking them with Fixes: tags, but do not want them backported.
First example is babbbdd08af9 ("mm/huge_memory.c: don't discard hugepage if other processes are mapping it")
Why is this not ok to backport?
Also what about e6be37b2e7bd ("mm/huge_memory.c: add missing read-only THP checking in transparent_hugepage_enabled()")?
And 41eb5df1cbc9 ("mm: memcg/slab: properly set up gfp flags for objcg pointer array")?
And 6acfb5ba150c ("mm: migrate: fix missing update page_private to hugetlb_page_subpool")?
And 832b50725373 ("mm: mmap_lock: use local locks instead of disabling preemption")? (the RT people want that...)
And f7ec104458e0 ("mm/page_alloc: fix counting of managed_pages")?
Do you want to rely on systems where these fixes are not applied?
I can understand if you all want to send them to us later after they have been "tested out" in Linus's tree, that's fine, but to just not want them applied at all feels odd to me.
thanks,
greg k-h
On Wed, 14 Jul 2021 15:23:50 +0200 Greg Kroah-Hartman gregkh@linuxfoundation.org wrote:
But it really feels odd that you all take the time to add a "Hey, this fixes this specific commit!" tag in the changelog, yet you do not actually want to go and fix the kernels that have that commit in it. This is an odd signal to others that watch the changelogs for context clues. Perhaps you might not want to do that anymore.
I looked at some of these patches and it seems really odd to me that you all are marking them with Fixes: tags, but do not want them backported.
First example is babbbdd08af9 ("mm/huge_memory.c: don't discard hugepage if other processes are mapping it")
Why is this not ok to backport?
Also what about e6be37b2e7bd ("mm/huge_memory.c: add missing read-only THP checking in transparent_hugepage_enabled()")?
And 41eb5df1cbc9 ("mm: memcg/slab: properly set up gfp flags for objcg pointer array")?
And 6acfb5ba150c ("mm: migrate: fix missing update page_private to hugetlb_page_subpool")?
And 832b50725373 ("mm: mmap_lock: use local locks instead of disabling preemption")? (the RT people want that...)
And f7ec104458e0 ("mm/page_alloc: fix counting of managed_pages")?
Do you want to rely on systems where these fixes are not applied?
I can understand if you all want to send them to us later after they have been "tested out" in Linus's tree, that's fine, but to just not want them applied at all feels odd to me.
Broadly speaking: end-user impact. If we don't have reports of the issue causing a user-visible problem and we don't expect such things to occur, don't backport. Why risk causing some regression when we cannot identify any benefit? (and boy do my fingers get tired asking people to describe the user-visible effects of the bug they claim to have fixed!)
Of course, screwups can happen and user-useful patches may be passed over.
On Wed, Jul 14, 2021 at 03:23:50PM +0200, Greg Kroah-Hartman wrote:
On Wed, Jul 14, 2021 at 11:18:14AM +0200, Greg Kroah-Hartman wrote:
On Tue, Jul 13, 2021 at 06:28:13PM -0700, Andrew Morton wrote:
On Tue, 13 Jul 2021 08:31:57 +0200 Greg Kroah-Hartman gregkh@linuxfoundation.org wrote:
So far, all automated testing seems to show that there are no regressions in these releases with these commits in them. If there was a problem, how would it show up?
And as far as I know, mm/ stuff is still not triggered by the AUTOSEL bot, but that is not what caused this commit to be added to a stable release.
Trying to keep a "do not apply" list for Fixes: tags only is much harder for both of us as we do these semi-manually and review them individually. Trying to remember what subsystem only does Fixes tags yet really doesn't mean it is an impossible task.
Well, it shouldn't be super hard to skip all patches which have Fixes:, Signed-off-by:akpm and no cc:stable?
Ok, I will do this now (goes and writes this down...)
But it really feels odd that you all take the time to add a "Hey, this fixes this specific commit!" tag in the changelog, yet you do not actually want to go and fix the kernels that have that commit in it. This is an odd signal to others that watch the changelogs for context clues. Perhaps you might not want to do that anymore.
I looked at some of these patches and it seems really odd to me that you all are marking them with Fixes: tags, but do not want them backported.
First example is babbbdd08af9 ("mm/huge_memory.c: don't discard hugepage if other processes are mapping it")
Why is this not ok to backport?
Also what about e6be37b2e7bd ("mm/huge_memory.c: add missing read-only THP checking in transparent_hugepage_enabled()")?
And 41eb5df1cbc9 ("mm: memcg/slab: properly set up gfp flags for objcg pointer array")?
And 6acfb5ba150c ("mm: migrate: fix missing update page_private to hugetlb_page_subpool")?
And 832b50725373 ("mm: mmap_lock: use local locks instead of disabling preemption")? (the RT people want that...)
This one at least is theoritical in nature for a backport because PREEMPT_RT cannot be selected as no arch defines ARCH_SUPPORTS_RT yet. If is was heading to any stable branch, it would be under https://git.kernel.org/pub/scm/linux/kernel/git/rt/linux-stable-rt.git/. The latest kernel there is v5.10-rt and the Fixes tag is for 5.11 so that fix would be ignored.
On Wed, Jul 14, 2021 at 11:18:14AM +0200, Greg Kroah-Hartman wrote:
On Tue, Jul 13, 2021 at 06:28:13PM -0700, Andrew Morton wrote:
Alternatively I could just invent a new tag to replace the "Fixes:" ("Fixes-no-backport?") to be used on patches which fix a known previous commit but which we don't want backported.
No please, that's not needed, I'll just ignore these types of patches now, and will go drop these from the queues.
Sasha, can you also add these to your "do not apply" script as well?
Sure, but I don't see how this is viable in the long term. Look at distros that don't follow LTS trees and cherry pick only important fixes, and see how many of those don't have a stable@ tag.
On Wed, Jul 14, 2021 at 09:52:53AM -0400, Sasha Levin wrote:
On Wed, Jul 14, 2021 at 11:18:14AM +0200, Greg Kroah-Hartman wrote:
On Tue, Jul 13, 2021 at 06:28:13PM -0700, Andrew Morton wrote:
Alternatively I could just invent a new tag to replace the "Fixes:" ("Fixes-no-backport?") to be used on patches which fix a known previous commit but which we don't want backported.
No please, that's not needed, I'll just ignore these types of patches now, and will go drop these from the queues.
Sasha, can you also add these to your "do not apply" script as well?
Sure, but I don't see how this is viable in the long term. Look at distros that don't follow LTS trees and cherry pick only important fixes, and see how many of those don't have a stable@ tag.
I've been talking to an enterprise distro who chooses not to use the LTS releases, and it's mainly because they tried it, and there was too many regressions leading to their customers filing problem reports which get escalated to their engineers, leading to unhappy customers and extra work for their engineers. (And they have numbers to back up this assertion; this isn't just a gut feel sort of thing.)
There are a couple of ways of solving it. Once is that perhaps we need to have more people testing the stable trees --- and not just for functional regressions but also for performance regressions. Ideally we would be doing lots of performance regression testing all the time, for all releases, and not just for the stable kernels, but the reality is that performance testing takes a lot of time, effort, and in some cases large amounts of expensive equipment.
We have syzbot and the zero-day bot; perhaps we can see if some company might be interested in setting up a "perfbot"?
Another solution (and these don't have to be mutually exclusive) might be for maintainers can explicitly state that certain patches shouldn't be backported into stable kernels. I think having an explicit "No-Backport: <Reason>" might be useful, since it documents why a maintainer requested that the patch not be backported, and being an explicit tag, it makes it clear that it wasn't just a case of the developer forgetting the "Cc: stable" tag. This makes it much better than implicit rules such as "If from: akpm then don't backport" hidden in various stable maintainers' scripts.
- Ted
On Wed, Jul 14, 2021 at 11:35:29AM -0400, Theodore Y. Ts'o wrote:
On Wed, Jul 14, 2021 at 09:52:53AM -0400, Sasha Levin wrote:
On Wed, Jul 14, 2021 at 11:18:14AM +0200, Greg Kroah-Hartman wrote:
On Tue, Jul 13, 2021 at 06:28:13PM -0700, Andrew Morton wrote:
Alternatively I could just invent a new tag to replace the "Fixes:" ("Fixes-no-backport?") to be used on patches which fix a known previous commit but which we don't want backported.
No please, that's not needed, I'll just ignore these types of patches now, and will go drop these from the queues.
Sasha, can you also add these to your "do not apply" script as well?
Sure, but I don't see how this is viable in the long term. Look at distros that don't follow LTS trees and cherry pick only important fixes, and see how many of those don't have a stable@ tag.
I've been talking to an enterprise distro who chooses not to use the LTS releases, and it's mainly because they tried it, and there was too many regressions leading to their customers filing problem reports which get escalated to their engineers, leading to unhappy customers and extra work for their engineers. (And they have numbers to back up this assertion; this isn't just a gut feel sort of thing.)
When did they last actually do this? Before or after we started testing stable releases better?
I have numbers to back up the other side, along with the security research showing that to ignore these stable releases puts systems at documented risk.
But enterprise distros really are a small market these days, a rounding error compared to Android phones, so maybe we just ignore what they do as it's a very tiny niche market these days? :)
thanks,
greg k-h
On Wed, Jul 14, 2021 at 11:35:29AM -0400, Theodore Y. Ts'o wrote:
Another solution (and these don't have to be mutually exclusive) might be for maintainers can explicitly state that certain patches shouldn't be backported into stable kernels. I think having an explicit "No-Backport: <Reason>" might be useful, since it documents why a maintainer requested that the patch not be backported, and being an explicit tag, it makes it clear that it wasn't just a case of the developer forgetting the "Cc: stable" tag. This makes it much better than implicit rules such as "If from: akpm then don't backport" hidden in various stable maintainers' scripts.
The number of valid cases where someone puts a "Fixes:" tag, and that patch should NOT be backported is really really slim. Why would you put that tag and not want to have known-broken kernels fixed?
If it really is not an issue, just do not put the "Fixes:" tag?
thanks,
greg k-h
On Wed, Jul 14, 2021 at 05:46:22PM +0200, Greg Kroah-Hartman wrote:
The number of valid cases where someone puts a "Fixes:" tag, and that patch should NOT be backported is really really slim. Why would you put that tag and not want to have known-broken kernels fixed?
If it really is not an issue, just do not put the "Fixes:" tag?
I think it really boils down to what the tags are supposed to mean and what do they imply.
The argument from the other side is if the Stable maintainers are interpreting the Fixes: tag as an implicit "CC: stable", why should we have the "Cc: stable" tag at all in that case?
We could also have the policy that in the case where you have a fix for a bug, but it's super subtle, and shouldn't be automatically backported, that the this should be explained in the commit, e.g.,
This commit fixes a bug in "1adeadbeef33: lorem ipsum dolor sit amet" but it is touching code which subtle and quick to anger, the bug isn't all that serious. So please don't backport it automatically; someone should manually do the backport and run the fooblat test before sumitting it to the stable maintainers.
Andrew seems to be of the opinion that these sorts of cases are very common. I don't have enough data to have a strong opinion either way. But if you are right that it is a rare case, then sure, simply omitting the Fixes: tag and using text in the commit description would work. We just need to agree that this is the convention that we all shoulf be using.
I still wonder though what's the point of having the "Cc: stable" tag if it's implicitly assumed to be there if there is a Fixes: tagle.
Cheers,
- Ted
On Wed, Jul 14, 2021 at 01:21:59PM -0400, Theodore Y. Ts'o wrote:
On Wed, Jul 14, 2021 at 05:46:22PM +0200, Greg Kroah-Hartman wrote:
The number of valid cases where someone puts a "Fixes:" tag, and that patch should NOT be backported is really really slim. Why would you put that tag and not want to have known-broken kernels fixed?
If it really is not an issue, just do not put the "Fixes:" tag?
I think it really boils down to what the tags are supposed to mean and what do they imply.
The argument from the other side is if the Stable maintainers are interpreting the Fixes: tag as an implicit "CC: stable", why should we have the "Cc: stable" tag at all in that case?
I would love to not have to look at the Fixes: tag, but today we have to because not all subsystems DO use cc: stable.
We miss loads of real fixes if we only go by cc: stable right now. If you can go and fix those subsystems to actually remember to do this "properly", wonderful, we will not have to mess with only Fixes: tags again.
But until that happens, we have to live with what we have. And all we have are "hints" like Fixes: to go off of.
We could also have the policy that in the case where you have a fix for a bug, but it's super subtle, and shouldn't be automatically backported, that the this should be explained in the commit, e.g.,
This commit fixes a bug in "1adeadbeef33: lorem ipsum dolor sit amet" but it is touching code which subtle and quick to anger, the bug isn't all that serious. So please don't backport it automatically; someone should manually do the backport and run the fooblat test before sumitting it to the stable maintainers.
That's wonderful, I would love to see that more, and we do see that on some commits. And we mostly catch them (I miss them at times, but that's my fault, not the developer/maintainers fault.)
Andrew seems to be of the opinion that these sorts of cases are very common. I don't have enough data to have a strong opinion either way. But if you are right that it is a rare case, then sure, simply omitting the Fixes: tag and using text in the commit description would work. We just need to agree that this is the convention that we all shoulf be using.
I still wonder though what's the point of having the "Cc: stable" tag if it's implicitly assumed to be there if there is a Fixes: tagle.
Because cc: stable came first, and for some reason people think that it is all that is necessary to get patches committed to the stable tree, despite it never being documented or that way. I have to correct someone about this about 2x a month on the stable@vger list.
thanks,
greg k-h
Hi Greg et al,
On Wed, Jul 14, 2021 at 7:36 PM Greg Kroah-Hartman gregkh@linuxfoundation.org wrote:
On Wed, Jul 14, 2021 at 01:21:59PM -0400, Theodore Y. Ts'o wrote:
On Wed, Jul 14, 2021 at 05:46:22PM +0200, Greg Kroah-Hartman wrote:
The number of valid cases where someone puts a "Fixes:" tag, and that patch should NOT be backported is really really slim. Why would you put that tag and not want to have known-broken kernels fixed?
If it really is not an issue, just do not put the "Fixes:" tag?
I think it really boils down to what the tags are supposed to mean and what do they imply.
The argument from the other side is if the Stable maintainers are interpreting the Fixes: tag as an implicit "CC: stable", why should we have the "Cc: stable" tag at all in that case?
I would love to not have to look at the Fixes: tag, but today we have to because not all subsystems DO use cc: stable.
We miss loads of real fixes if we only go by cc: stable right now. If you can go and fix those subsystems to actually remember to do this "properly", wonderful, we will not have to mess with only Fixes: tags again.
But until that happens, we have to live with what we have. And all we have are "hints" like Fixes: to go off of.
IMHO the biggest issues with "Cc: stable" are that (a) in general it's hard to know if a patch is (not) worthwhile to be backported, and (b) without a Fixes: tag it doesn't tell you what version(s) it should be applied to.
Just having a Fixes: tag fixes the latter, and allows you to defer the decision to backport.
We could also have the policy that in the case where you have a fix for a bug, but it's super subtle, and shouldn't be automatically backported, that the this should be explained in the commit, e.g.,
This commit fixes a bug in "1adeadbeef33: lorem ipsum dolor sit amet" but it is touching code which subtle and quick to anger, the bug isn't all that serious. So please don't backport it automatically; someone should manually do the backport and run the fooblat test before sumitting it to the stable maintainers.
That's wonderful, I would love to see that more, and we do see that on some commits. And we mostly catch them (I miss them at times, but that's my fault, not the developer/maintainers fault.)
In my experience, the hardest cases are the ones where a patch fixes a real bug, but the fix has an obscure implicit dependency on another commit in a different subsystem (e.g. driver and DTS interaction). When backporting, a regression is introduced if the dependency is not present yet in the stable tree.
Andrew seems to be of the opinion that these sorts of cases are very common. I don't have enough data to have a strong opinion either way. But if you are right that it is a rare case, then sure, simply omitting the Fixes: tag and using text in the commit description would work. We just need to agree that this is the convention that we all shoulf be using.
I still wonder though what's the point of having the "Cc: stable" tag if it's implicitly assumed to be there if there is a Fixes: tagle.
Because cc: stable came first, and for some reason people think that it is all that is necessary to get patches committed to the stable tree, despite it never being documented or that way. I have to correct someone about this about 2x a month on the stable@vger list.
For a developer, it's much easier to not care about "Cc: stable" at all, because as soon as you add a "Cc: stable" to a patch, or CC stable, someone will compain ;-) Much easier to just add a Fixes: tag, and know it will be backported to trees that have the "buggy" commit.
Gr{oetje,eeting}s,
Geert
On Thu, Jul 15, 2021 at 11:01:04AM +0200, Geert Uytterhoeven wrote:
Because cc: stable came first, and for some reason people think that it is all that is necessary to get patches committed to the stable tree, despite it never being documented or that way. I have to correct someone about this about 2x a month on the stable@vger list.
For a developer, it's much easier to not care about "Cc: stable" at all, because as soon as you add a "Cc: stable" to a patch, or CC stable, someone will compain ;-) Much easier to just add a Fixes: tag, and know it will be backported to trees that have the "buggy" commit.
What sort of complaints have you gotten? I add "cc: stable" for the ext4 tree, and I can't say I've gotten any complaints.
- Ted
Hi Ted,
On Thu, Jul 15, 2021 at 4:47 PM Theodore Y. Ts'o tytso@mit.edu wrote:
On Thu, Jul 15, 2021 at 11:01:04AM +0200, Geert Uytterhoeven wrote:
Because cc: stable came first, and for some reason people think that it is all that is necessary to get patches committed to the stable tree, despite it never being documented or that way. I have to correct someone about this about 2x a month on the stable@vger list.
For a developer, it's much easier to not care about "Cc: stable" at all, because as soon as you add a "Cc: stable" to a patch, or CC stable, someone will compain ;-) Much easier to just add a Fixes: tag, and know it will be backported to trees that have the "buggy" commit.
What sort of complaints have you gotten? I add "cc: stable" for the ext4 tree, and I can't say I've gotten any complaints.
Usually a complaint about using the wrong process for subsystem X.
Gr{oetje,eeting}s,
Geert
linux-stable-mirror@lists.linaro.org
-
Andrew Morton -
Geert Uytterhoeven -
Greg Kroah-Hartman -
Hugh Dickins -
Jiri Slaby -
Justin Forbes -
Mel Gorman -
Michal Hocko -
Sasha Levin -
Theodore Y. Ts'o