|Summary:||Add functionality to set default alignment behavior on ARM in rc.sysinit|
|Product:||[Fedora] Fedora||Reporter:||Gordan Bobic <gordan>|
|Component:||initscripts||Assignee:||Bill Nottingham <notting>|
|Status:||CLOSED WONTFIX||QA Contact:||Fedora Extras Quality Assurance <extras-qa>|
|Version:||rawhide||CC:||iarlyy, jonathan, ndevos, notting, plautrba, rvokal|
|Fixed In Version:||Doc Type:||Bug Fix|
|Doc Text:||Story Points:||---|
|Last Closed:||2011-06-27 12:23:06 UTC||Type:||---|
|oVirt Team:||---||RHEL 7.3 requirements from Atomic Host:|
|Cloudforms Team:||---||Target Upstream Version:|
|Bug Depends On:|
Description Gordan Bobic 2011-01-29 20:04:55 UTC
Created attachment 475963 [details] rc.sysinit patch to apply alignment fixup settings Description of problem: Kernel's default behaviour on ARM platform is to ignore alignment errors. Unfortunately a lot of software is buggy and relies on the hardware silently fixing this (e.g. x86, ARMv7/Cortex A series and later), which comes with a performance penalty. ARMv6 and earlier platforms do not have transparent hardware fixing for this so these bugs cause erroneous data to be received by the software rather merely causing a performance penalty. Kernel's default behaviour is to ignore alignment errors. This patch adds a feature for this to be controllable via a setting in /etc/sysconfig/. Attached are the patch and a sample config (/etc/sysconfig/alignment). Version-Release number of selected component (if applicable): initscripts-9.12.1-1.fc13.armv5tel
Comment 1 Gordan Bobic 2011-01-29 20:05:43 UTC
Created attachment 475964 [details] /etc/sysconfig/alignment sample/default config
Comment 2 Niels de Vos 2011-01-31 20:52:51 UTC
(In reply to comment #0) > Created attachment 475963 [details] > rc.sysinit patch to apply alignment fixup settings I've marked this as patch now (it was an octet-stream) and did a little review. As this patch should conditionally be included in the .armv5tel release of initscripts, I do not think there is the need for the else-if-not-arm part. You have added an extra grep to check if the variable is a number. rc.sysinit does not seem to care about bad formatted options/variables anywhere. I think the extra check is good, though also not complete (i.e. value 9 is undefined?). To keep the code simple, I would suggest to remove the check. > Kernel's default behaviour is to ignore alignment errors. Which setting is that in the config? I guess fix+warn, but it would be nice to have that confirmation from you. Cheers, Niels
Comment 3 Gordan Bobic 2011-01-31 21:34:50 UTC
The values are documented in the other attached file, in the comments. If you have a look there, the value I put in the attached file is 3 - fix+warn. I'm not sure if multiple rc.sysinit scripts for arch dependance is a good idea. I think it'd be better if they were unified. There are unlikely to be THAT many arch specific things like this. Regarding the check, I like the extra error checking just to make sure. However, there is another point to be made. I'm not sure if this is the case on all ARMv7 CPUs, but it is certainly the case on all Cortex A8 and A9 derived CPUs I have seen - and they automatically and transparently, silently fix alignment errors (AFAIK, the speed penalty is still there, but we get no say in the matter - changing the settings does nothing). Unless there is an exception among ARMv7 series, the chances are that this is irrelevant on ARMv7 and will also be on later CPUs. So I could be persuaded that the check should only cover ARMvX where X <= 6. I'm not convinced the miniscule extra complexity and precedents for not error checking input elsewhere is a good reason to avoid using good practices in the future. By that reasoning we shouldn't fix bugs because there are already other bugs in the distro. :)
Comment 4 Bill Nottingham 2011-02-01 17:52:29 UTC
Why on earth isn't this a sysctl in the kernel, and therefore something that can be easily set in a distribution-neutral sysctl.conf file?
Comment 5 Gordan Bobic 2011-02-01 23:02:31 UTC
I completely agree that a sysctl would be far more sensible. Having said that, with so many things moving to /sys, when are we going to get sysctl.conf support for applying configuration to things under /sys ? Meanwhile, something like the attached patch is likely to remain the only way to auto-apply this setting for the foreseeable future.
Comment 6 Bill Nottingham 2011-02-02 19:08:42 UTC
(In reply to comment #5) > I completely agree that a sysctl would be far more sensible. Having said that, > with so many things moving to /sys, when are we going to get sysctl.conf > support for applying configuration to things under /sys ? Huh? We already have that (and architecture-specific files in the package, to boot.)
Comment 7 Bill Nottingham 2011-02-02 19:39:54 UTC
Also, would the use of something like http://kojipkgs.fedoraproject.org/packages/prctl/1.5/2/src/ be appropriate?
Comment 8 Gordan Bobic 2011-02-02 22:18:10 UTC
I'm not sure prctl is quite what is needed here. Either way, this is an issue that needs to be addressed one way or the other. I provided a patch that works today, with today's kernel. If it is to be made into a sysctl, then that would need to be discussed upstream. Sure, I'd prefer it to be a simple setting in sysctl.conf (or the equivalent for /sys as opposed to /proc/sys, which to the best of my knowledge doesn't exist, but please correct me if I'm wrong). But that requires kernel changes, and this probably needs to be done before F16.
Comment 9 Bill Nottingham 2011-02-25 21:07:23 UTC
prctl is an already-existing interface for setting alignment behavior, that's why I asked.
Comment 10 Bug Zapper 2011-05-30 11:37:09 UTC
This message is a reminder that Fedora 13 is nearing its end of life. Approximately 30 (thirty) days from now Fedora will stop maintaining and issuing updates for Fedora 13. It is Fedora's policy to close all bug reports from releases that are no longer maintained. At that time this bug will be closed as WONTFIX if it remains open with a Fedora 'version' of '13'. Package Maintainer: If you wish for this bug to remain open because you plan to fix it in a currently maintained version, simply change the 'version' to a later Fedora version prior to Fedora 13's end of life. Bug Reporter: Thank you for reporting this issue and we are sorry that we may not be able to fix it before Fedora 13 is end of life. If you would still like to see this bug fixed and are able to reproduce it against a later version of Fedora please change the 'version' of this bug to the applicable version. If you are unable to change the version, please add a comment here and someone will do it for you. Although we aim to fix as many bugs as possible during every release's lifetime, sometimes those efforts are overtaken by events. Often a more recent Fedora release includes newer upstream software that fixes bugs or makes them obsolete. The process we are following is described here: http://fedoraproject.org/wiki/BugZappers/HouseKeeping
Comment 11 Bug Zapper 2011-06-27 12:23:06 UTC
Fedora 13 changed to end-of-life (EOL) status on 2011-06-25. Fedora 13 is no longer maintained, which means that it will not receive any further security or bug fix updates. As a result we are closing this bug. If you can reproduce this bug against a currently maintained version of Fedora please feel free to reopen this bug against that version. Thank you for reporting this bug and we are sorry it could not be fixed.
Comment 12 Gordan Bobic 2011-08-05 20:20:09 UTC
Would it be OK to have this re-opened against rawhide?
Comment 13 Bill Nottingham 2011-08-05 20:33:27 UTC
I'm still not sure why this can't just use the existing prctl alignment frobbing mechanisms.
Comment 14 Gordan Bobic 2011-08-05 20:45:25 UTC
Maybe it can, but I can'd find the prctl package anywhere. This lists it as orphaned: https://admin.fedoraproject.org/pkgdb/acls/name/prctl The git link lists it as dead. Can you provide an up to date link to the package/source?
Comment 15 Bill Nottingham 2011-08-05 21:18:24 UTC
http://kojipkgs.fedoraproject.org/packages/prctl/1.5/2/src/prctl-1.5-2.src.rpm was the latest build. It's something that was only necessary on ia64, so it was dropped from Fedora a while back.