Skip to content
Snippets Groups Projects

Fix up PREEMPT configs

Merged Justin M. Forbes requested to merge jmflinuxtx/kernel-ark:preempt_tak2 into os-build

While preempt_dynamic was greatly improved at the beginning of the merge window, the config to make things work was messy and the new BEHAVIOUR configs were confusing to some. With commit a8b76910 the BEHAVIOUR config options are gone and we now default to whichever CONFIG_PREEMPT is selected. This MR moves us to the new config options, but should leave the kernel behaving as before with PREEMPT_VOLUNTARY as the default with PREEMPT_DYNAMIC enabled for all arches but s390 which does not enable DYNAMIC and builds with PREEMPT_NONE.

Signed-off-by: Justin M. Forbes jforbes@fedoraproject.org

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • Phil Auld pauld@redhat.com commented via email:

    On Mon, Nov 15, 2021 at 06:28:11PM -0000 Justin M. Forbes (via Email Bridge) wrote:
    > From: Justin M. Forbes <jforbes@fedoraproject.org>
    > 
    > Fix up PREEMPT configs
    > 
    > While preempt_dynamic was greatly improved at the beginning of the merge
    > window, the config to make things work was messy and the new BEHAVIOUR
    > configs were confusing to some. With commit
    > a8b76910e465d718effce0cad306a21fa4f3526b the BEHAVIOUR config options
    > are gone and we now default to whichever CONFIG_PREEMPT is selected.
    > This MR moves us to the new config options, but should leave the kernel
    > behaving as before with PREEMPT_VOLUNTARY as the default with
    > PREEMPT_DYNAMIC enabled for all arches but s390 which does not enable
    > DYNAMIC and builds with PREEMPT_NONE.
    > 
    > Signed-off-by: Justin M. Forbes <jforbes@fedoraproject.org>
    >
    
    Thanks Justin. The rest in that series didn't seem to go in so I think
    we can just go with this.
    
    Acked-by: Phil Auld <pauld@redhat.com>
    
    > diff --git a/redhat/configs/common/generic/CONFIG_PREEMPT b/redhat/configs/common/generic/CONFIG_PREEMPT
    > index blahblah..blahblah 100644
    > --- a/redhat/configs/common/generic/CONFIG_PREEMPT
    > +++ b/redhat/configs/common/generic/CONFIG_PREEMPT
    > @@ -1 +1 @@
    > -CONFIG_PREEMPT=y
    > +# CONFIG_PREEMPT is not set
    > diff --git a/redhat/configs/common/generic/CONFIG_PREEMPT_BEHAVIOUR b/redhat/configs/common/generic/CONFIG_PREEMPT_BEHAVIOUR
    > deleted file mode 100644
    > index blahblah..blahblah 0
    > --- a/redhat/configs/common/generic/CONFIG_PREEMPT_BEHAVIOUR
    > +++ /dev/null
    > @@ -1 +0,0 @@
    > -# CONFIG_PREEMPT_BEHAVIOUR is not set
    > diff --git a/redhat/configs/common/generic/CONFIG_PREEMPT_NONE b/redhat/configs/common/generic/CONFIG_PREEMPT_NONE
    > index blahblah..blahblah 100644
    > --- a/redhat/configs/common/generic/CONFIG_PREEMPT_NONE
    > +++ b/redhat/configs/common/generic/CONFIG_PREEMPT_NONE
    > @@ -1 +1 @@
    > -CONFIG_PREEMPT_NONE=y
    > +# CONFIG_PREEMPT_NONE is not set
    > diff --git a/redhat/configs/common/generic/CONFIG_PREEMPT_NONE_BEHAVIOUR b/redhat/configs/common/generic/CONFIG_PREEMPT_NONE_BEHAVIOUR
    > deleted file mode 100644
    > index blahblah..blahblah 0
    > --- a/redhat/configs/common/generic/CONFIG_PREEMPT_NONE_BEHAVIOUR
    > +++ /dev/null
    > @@ -1 +0,0 @@
    > -# CONFIG_PREEMPT_NONE_BEHAVIOUR is not set
    > diff --git a/redhat/configs/common/generic/CONFIG_PREEMPT_VOLUNTARY_BEHAVIOUR b/redhat/configs/common/generic/CONFIG_PREEMPT_VOLUNTARY_BEHAVIOUR
    > deleted file mode 100644
    > index blahblah..blahblah 0
    > --- a/redhat/configs/common/generic/CONFIG_PREEMPT_VOLUNTARY_BEHAVIOUR
    > +++ /dev/null
    > @@ -1 +0,0 @@
    > -CONFIG_PREEMPT_VOLUNTARY_BEHAVIOUR=y
    > diff --git a/redhat/configs/common/generic/s390x/CONFIG_PREEMPT_NONE_BEHAVIOUR b/redhat/configs/common/generic/s390x/CONFIG_PREEMPT_NONE_BEHAVIOUR
    > deleted file mode 100644
    > index blahblah..blahblah 0
    > --- a/redhat/configs/common/generic/s390x/CONFIG_PREEMPT_NONE_BEHAVIOUR
    > +++ /dev/null
    > @@ -1 +0,0 @@
    > -CONFIG_PREEMPT_NONE_BEHAVIOUR=y
    > diff --git a/redhat/configs/common/generic/s390x/CONFIG_PREEMPT_VOLUNTARY_BEHAVIOUR b/redhat/configs/common/generic/s390x/CONFIG_PREEMPT_VOLUNTARY_BEHAVIOUR
    > deleted file mode 100644
    > index blahblah..blahblah 0
    > --- a/redhat/configs/common/generic/s390x/CONFIG_PREEMPT_VOLUNTARY_BEHAVIOUR
    > +++ /dev/null
    > @@ -1 +0,0 @@
    > -# CONFIG_PREEMPT_VOLUNTARY_BEHAVIOUR is not set
    > diff --git a/redhat/configs/pending-common/generic/CONFIG_PREEMPT b/redhat/configs/pending-common/generic/CONFIG_PREEMPT
    > deleted file mode 100644
    > index blahblah..blahblah 0
    > --- a/redhat/configs/pending-common/generic/CONFIG_PREEMPT
    > +++ /dev/null
    > @@ -1 +0,0 @@
    > -# CONFIG_PREEMPT is not set
    > diff --git a/redhat/configs/pending-common/generic/CONFIG_PREEMPT_NONE b/redhat/configs/pending-common/generic/CONFIG_PREEMPT_NONE
    > deleted file mode 100644
    > index blahblah..blahblah 0
    > --- a/redhat/configs/pending-common/generic/CONFIG_PREEMPT_NONE
    > +++ /dev/null
    > @@ -1 +0,0 @@
    > -# CONFIG_PREEMPT_NONE is not set
    > diff --git a/redhat/configs/pending-common/generic/s390x/CONFIG_PREEMPT_NONE b/redhat/configs/pending-common/generic/s390x/CONFIG_PREEMPT_NONE
    > deleted file mode 100644
    > index blahblah..blahblah 0
    > --- a/redhat/configs/pending-common/generic/s390x/CONFIG_PREEMPT_NONE
    > +++ /dev/null
    > @@ -1 +0,0 @@
    > -CONFIG_PREEMPT_NONE=y
    > diff --git a/redhat/configs/pending-common/generic/s390x/CONFIG_PREEMPT_VOLUNTARY b/redhat/configs/pending-common/generic/s390x/CONFIG_PREEMPT_VOLUNTARY
    > deleted file mode 100644
    > index blahblah..blahblah 0
    > --- a/redhat/configs/pending-common/generic/s390x/CONFIG_PREEMPT_VOLUNTARY
    > +++ /dev/null
    > @@ -1 +0,0 @@
    > -# CONFIG_PREEMPT_VOLUNTARY is not set
    > 
    > --
    > https://gitlab.com/cki-project/kernel-ark/-/merge_requests/1500
    > _______________________________________________
    > kernel mailing list -- kernel@lists.fedoraproject.org
    > To unsubscribe send an email to kernel-leave@lists.fedoraproject.org
    > Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/
    > List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
    > List Archives: https://lists.fedoraproject.org/archives/list/kernel@lists.fedoraproject.org
    > Do not reply to spam on the list, report it: https://pagure.io/fedora-infrastructure
  • CKI KWF Bot added AcksOK label and removed AcksNeedsReview label

    added AcksOK label and removed AcksNeedsReview label

  • Phil Auld pauld@redhat.com commented via email:

    Hi Justin,
    
    On Mon, Nov 15, 2021 at 06:28:11PM -0000 Justin M. Forbes (via Email Bridge) wrote:
    > From: Justin M. Forbes <jforbes@fedoraproject.org>
    > 
    > Fix up PREEMPT configs
    > 
    > While preempt_dynamic was greatly improved at the beginning of the merge
    > window, the config to make things work was messy and the new BEHAVIOUR
    > configs were confusing to some. With commit
    > a8b76910e465d718effce0cad306a21fa4f3526b the BEHAVIOUR config options
    > are gone and we now default to whichever CONFIG_PREEMPT is selected.
    > This MR moves us to the new config options, but should leave the kernel
    > behaving as before with PREEMPT_VOLUNTARY as the default with
    > PREEMPT_DYNAMIC enabled for all arches but s390 which does not enable
    > DYNAMIC and builds with PREEMPT_NONE.
    >
    
    
    This commit, or something like it, seems to have gone in but without
    much of a commit message.  Was that on purpsoe?
    
    Also, I noticed that the dynamic preemption is only enabled for x86_64 due
    to none of the other archs enabling HAVE_PREEMPT_DYNAMIC. There are arm
    patches posted but not yet merged to enable this there.
    
    
    Cheers,
    Phil
    
    
    > Signed-off-by: Justin M. Forbes <jforbes@fedoraproject.org>
    > 
    > diff --git a/redhat/configs/common/generic/CONFIG_PREEMPT b/redhat/configs/common/generic/CONFIG_PREEMPT
    > index blahblah..blahblah 100644
    > --- a/redhat/configs/common/generic/CONFIG_PREEMPT
    > +++ b/redhat/configs/common/generic/CONFIG_PREEMPT
    > @@ -1 +1 @@
    > -CONFIG_PREEMPT=y
    > +# CONFIG_PREEMPT is not set
    > diff --git a/redhat/configs/common/generic/CONFIG_PREEMPT_BEHAVIOUR b/redhat/configs/common/generic/CONFIG_PREEMPT_BEHAVIOUR
    > deleted file mode 100644
    > index blahblah..blahblah 0
    > --- a/redhat/configs/common/generic/CONFIG_PREEMPT_BEHAVIOUR
    > +++ /dev/null
    > @@ -1 +0,0 @@
    > -# CONFIG_PREEMPT_BEHAVIOUR is not set
    > diff --git a/redhat/configs/common/generic/CONFIG_PREEMPT_NONE b/redhat/configs/common/generic/CONFIG_PREEMPT_NONE
    > index blahblah..blahblah 100644
    > --- a/redhat/configs/common/generic/CONFIG_PREEMPT_NONE
    > +++ b/redhat/configs/common/generic/CONFIG_PREEMPT_NONE
    > @@ -1 +1 @@
    > -CONFIG_PREEMPT_NONE=y
    > +# CONFIG_PREEMPT_NONE is not set
    > diff --git a/redhat/configs/common/generic/CONFIG_PREEMPT_NONE_BEHAVIOUR b/redhat/configs/common/generic/CONFIG_PREEMPT_NONE_BEHAVIOUR
    > deleted file mode 100644
    > index blahblah..blahblah 0
    > --- a/redhat/configs/common/generic/CONFIG_PREEMPT_NONE_BEHAVIOUR
    > +++ /dev/null
    > @@ -1 +0,0 @@
    > -# CONFIG_PREEMPT_NONE_BEHAVIOUR is not set
    > diff --git a/redhat/configs/common/generic/CONFIG_PREEMPT_VOLUNTARY_BEHAVIOUR b/redhat/configs/common/generic/CONFIG_PREEMPT_VOLUNTARY_BEHAVIOUR
    > deleted file mode 100644
    > index blahblah..blahblah 0
    > --- a/redhat/configs/common/generic/CONFIG_PREEMPT_VOLUNTARY_BEHAVIOUR
    > +++ /dev/null
    > @@ -1 +0,0 @@
    > -CONFIG_PREEMPT_VOLUNTARY_BEHAVIOUR=y
    > diff --git a/redhat/configs/common/generic/s390x/CONFIG_PREEMPT_NONE_BEHAVIOUR b/redhat/configs/common/generic/s390x/CONFIG_PREEMPT_NONE_BEHAVIOUR
    > deleted file mode 100644
    > index blahblah..blahblah 0
    > --- a/redhat/configs/common/generic/s390x/CONFIG_PREEMPT_NONE_BEHAVIOUR
    > +++ /dev/null
    > @@ -1 +0,0 @@
    > -CONFIG_PREEMPT_NONE_BEHAVIOUR=y
    > diff --git a/redhat/configs/common/generic/s390x/CONFIG_PREEMPT_VOLUNTARY_BEHAVIOUR b/redhat/configs/common/generic/s390x/CONFIG_PREEMPT_VOLUNTARY_BEHAVIOUR
    > deleted file mode 100644
    > index blahblah..blahblah 0
    > --- a/redhat/configs/common/generic/s390x/CONFIG_PREEMPT_VOLUNTARY_BEHAVIOUR
    > +++ /dev/null
    > @@ -1 +0,0 @@
    > -# CONFIG_PREEMPT_VOLUNTARY_BEHAVIOUR is not set
    > diff --git a/redhat/configs/pending-common/generic/CONFIG_PREEMPT b/redhat/configs/pending-common/generic/CONFIG_PREEMPT
    > deleted file mode 100644
    > index blahblah..blahblah 0
    > --- a/redhat/configs/pending-common/generic/CONFIG_PREEMPT
    > +++ /dev/null
    > @@ -1 +0,0 @@
    > -# CONFIG_PREEMPT is not set
    > diff --git a/redhat/configs/pending-common/generic/CONFIG_PREEMPT_NONE b/redhat/configs/pending-common/generic/CONFIG_PREEMPT_NONE
    > deleted file mode 100644
    > index blahblah..blahblah 0
    > --- a/redhat/configs/pending-common/generic/CONFIG_PREEMPT_NONE
    > +++ /dev/null
    > @@ -1 +0,0 @@
    > -# CONFIG_PREEMPT_NONE is not set
    > diff --git a/redhat/configs/pending-common/generic/s390x/CONFIG_PREEMPT_NONE b/redhat/configs/pending-common/generic/s390x/CONFIG_PREEMPT_NONE
    > deleted file mode 100644
    > index blahblah..blahblah 0
    > --- a/redhat/configs/pending-common/generic/s390x/CONFIG_PREEMPT_NONE
    > +++ /dev/null
    > @@ -1 +0,0 @@
    > -CONFIG_PREEMPT_NONE=y
    > diff --git a/redhat/configs/pending-common/generic/s390x/CONFIG_PREEMPT_VOLUNTARY b/redhat/configs/pending-common/generic/s390x/CONFIG_PREEMPT_VOLUNTARY
    > deleted file mode 100644
    > index blahblah..blahblah 0
    > --- a/redhat/configs/pending-common/generic/s390x/CONFIG_PREEMPT_VOLUNTARY
    > +++ /dev/null
    > @@ -1 +0,0 @@
    > -# CONFIG_PREEMPT_VOLUNTARY is not set
    > 
    > --
    > https://gitlab.com/cki-project/kernel-ark/-/merge_requests/1500
    > _______________________________________________
    > kernel mailing list -- kernel@lists.fedoraproject.org
    > To unsubscribe send an email to kernel-leave@lists.fedoraproject.org
    > Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/
    > List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
    > List Archives: https://lists.fedoraproject.org/archives/list/kernel@lists.fedoraproject.org
    > Do not reply to spam on the list, report it: https://pagure.io/fedora-infrastructure
  • This commit, or something like it, seems to have gone in but without much of a commit message. Was that on purpsoe?

    Yes, there are files in pending-common that go in without a merge request. Typically those are set to default values because they are new config options, but in this case it had to be set by hand because these are not new config items, but are currently set in a way that is unbuildable. Everything in pending-common is set without review, and should have a corresponding MR to move the items from pending-common and into the correct place. If we did not do this with pending common, this tree would never build until every single config MR was reviewed and merged. It is a method to keep the tree usable, but make sure that nothing makes it into final RHEL configs until reviewed.

    Also, I noticed that the dynamic preemption is only enabled for x86_64 due to none of the other archs enabling HAVE_PREEMPT_DYNAMIC. There are arm patches posted but not yet merged to enable this there.

    This is true, but as other arches add HAVE_PREEMPT_DYNAMIC, this commit message will still be what is seen when people check the values for the configs. In the case of aarch64, I expect that will be soon. In the case of power, it may or may not happen, but the commit message still explains the situation, as it is PREEMPT_VOLUNTARY, it just can't be overridden unless patches are merged to enable dynamic. S390 is still not dynamic and ships with PREEMPT_NONE. In all of these cases, it matches what is currently in RHEL 9/RHEL 8, and stable Fedora with the addition of dynamic override allowed where the arch supports it.

  • Phil Auld pauld@redhat.com commented via email:

    On Tue, Nov 16, 2021 at 09:19:09PM -0000 Justin M. Forbes (via Email Bridge) wrote:
    > From: Justin M. Forbes on gitlab.com
    > https://gitlab.com/cki-project/kernel-ark/-/merge_requests/1500#note_735053347
    > 
    > > This commit, or something like it, seems to have gone in but without
    > > much of a commit message.  Was that on purpsoe?
    > 
    > 
    > Yes, there are files in pending-common that go in without a merge request.
    > Typically those are set to default values because they are new config options,
    > but in this case it had to be set by hand because these are not new config
    > items, but are currently set in a way that is unbuildable. Everything in
    > pending-common is set without review, and should have a corresponding MR to
    > move the items from pending-common and into the correct place.   If we did not
    > do this with pending common, this tree would never build until every single
    > config MR was reviewed and merged.  It is a method to keep the tree usable,
    > but make sure that nothing makes it into final RHEL configs until reviewed.
    > 
    > > Also, I noticed that the dynamic preemption is only enabled for x86_64 due
    > > to none of the other archs enabling HAVE_PREEMPT_DYNAMIC. There are arm
    > > patches posted but not yet merged to enable this there.
    > 
    > This is true, but as other arches add HAVE_PREEMPT_DYNAMIC, this commit
    > message will still be what is seen when people check the values for the
    > configs.  In the case of aarch64, I expect that will be soon. In the case of
    > power, it may or may not happen, but the commit message still explains the
    > situation, as it is PREEMPT_VOLUNTARY, it just can't be overridden unless
    > patches are merged to enable dynamic.  S390 is still not dynamic and ships
    > with PREEMPT_NONE. In all of these cases, it matches what is currently in RHEL
    > 9/RHEL 8, and stable Fedora with the addition of dynamic override allowed
    > where the arch supports it.
    
    
    Yes, I get that part. I was talking about os-build:50d16ce3099, which has
    a one line commit message and says nothing about keeping VOLUNTARY etc. 
    
    The commit message on what you posted here looked more like what I was
    expecting. But maybe I'm just confused...
    
    
    Cheers,
    Phil
    
    > _______________________________________________
    > kernel mailing list -- kernel@lists.fedoraproject.org
    > To unsubscribe send an email to kernel-leave@lists.fedoraproject.org
    > Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/
    > List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
    > List Archives: https://lists.fedoraproject.org/archives/list/kernel@lists.fedoraproject.org
    > Do not reply to spam on the list, report it: https://pagure.io/fedora-infrastructure
  • Yes, I get that part. I was talking about os-build:50d16ce3, which has a one line commit message and says nothing about keeping VOLUNTARY etc.

    The commit message on what you posted here looked more like what I was expecting. But maybe I'm just confused...

    Those are Fedora commits that have no impact on RHEL and do not go through MRs so as to not waste RHEL engineers time with review requests which are not relevant to them. As I had to fix up both pending-common and fedora anyway, it was done in the same commit, but no RHEL configs outside of pending were changed.

  • Phil Auld approved this merge request

    approved this merge request

  • Patrick Talbert mentioned in commit 3e4408d3

    mentioned in commit 3e4408d3

Please register or sign in to reply
Loading