| Message ID | 20201021104611.2744565-1-qais.yousef@arm.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Add support for Asymmetric AArch32 systems | expand |
On Wed, Oct 21, 2020 at 11:46:07AM +0100, Qais Yousef wrote: > This series adds basic support for Asymmetric AArch32 systems. Full rationale > is in v1's cover letter. > > https://lore.kernel.org/linux-arch/20201008181641.32767-1-qais.yousef@arm.com/ That is not good, provide full rational in each place, not everyone has web access at all time. Also, you forgot to document this in Documentation/ABI/ like is required for sysfs files, and I thought I asked for last time. > Changes in v2: > > * We now reset vcpu->arch.target to force re-initialized for KVM patch. > (Marc) > > * Fix a bug where this_cpu_has_cap() must be called with preemption > disabled in check_aarch32_cpumask(). > > * Add new sysctl.enable_asym_32bit. (Catalin) > > * Export id_aar64fpr0 register in sysfs which allows user space to > discover which cpus support 32bit@EL0. The sysctl must be enabled for > the user space to discover the asymmetry. (Will/Catalin) > > * Fixing up affinity in the kernel approach was dropped. The support > assumes the user space that wants to enable this support knows how to > guarantee correct affinities for 32bit apps by using cpusets. I asked you to work with Intel to come up with an agreement of how this is going to be represented in userspace. Why did you not do that? Without even looking at the patch set, this is not ok... greg k-h
Hi Greg On 10/21/20 13:26, Greg Kroah-Hartman wrote: > On Wed, Oct 21, 2020 at 11:46:07AM +0100, Qais Yousef wrote: > > This series adds basic support for Asymmetric AArch32 systems. Full rationale > > is in v1's cover letter. > > > > https://lore.kernel.org/linux-arch/20201008181641.32767-1-qais.yousef@arm.com/ > > That is not good, provide full rational in each place, not everyone has > web access at all time. Sorry. I usually copy the whole thing but for the first time I do this as I thought it'd be better to be less wordy. I'll copy the whole thing again next time. > Also, you forgot to document this in Documentation/ABI/ like is required > for sysfs files, and I thought I asked for last time. Last time there was no sysfs info. It's introduced for the first time here. There's still no consensus on which direction to go, that is fix it in the scheduler or let user space handle it all. > > Changes in v2: > > > > * We now reset vcpu->arch.target to force re-initialized for KVM patch. > > (Marc) > > > > * Fix a bug where this_cpu_has_cap() must be called with preemption > > disabled in check_aarch32_cpumask(). > > > > * Add new sysctl.enable_asym_32bit. (Catalin) > > > > * Export id_aar64fpr0 register in sysfs which allows user space to > > discover which cpus support 32bit@EL0. The sysctl must be enabled for > > the user space to discover the asymmetry. (Will/Catalin) > > > > * Fixing up affinity in the kernel approach was dropped. The support > > assumes the user space that wants to enable this support knows how to > > guarantee correct affinities for 32bit apps by using cpusets. > > I asked you to work with Intel to come up with an agreement of how this > is going to be represented in userspace. Why did you not do that? I did chip in to that thread. AFAIU they're doing something completely different and unrelated. Their goal is unclear too. They care about big.LITTLE type of support for Intel and collating already existing information in a different/new place. I don't see the point. I saw they had several similar comments from others. They need to send a new version to see if anything changes. > Without even looking at the patch set, this is not ok... Sorry about that. Please keep in mind we're still debating if we want to support this upstream. And if we do, what shape this should take. My first version highlighted how things could look like if scheduler took care of the problem. Now this RFC tries to highlight how things could look like if we go with pure user space based solution. It's to help maintainers get a better appreciation of what implementation details incurred in either direction. At least that was my intention. I'll improve on the cover letter next time. Thanks -- Qais Yousef
On Wed, Oct 21, 2020 at 02:15:04PM +0100, Qais Yousef wrote: > > Without even looking at the patch set, this is not ok... > > Sorry about that. Please keep in mind we're still debating if we want to > support this upstream. What do you mean by this? Do you mean you will keep an out-of-tree patchset for all time for all future kernel versions for everyone to pull from and you will support for all chips that end up with this type of functionality? That's a huge task to do, you all must have a lot of money to burn! It is a "trivial cost" to get changes merged upstream compared to the amount of time and money it costs to keep stuff out of the tree. Why you would not want to do this is beyond me. But hey, I'm not in charge of your company's budget, for good reasons :) good luck! greg k-h
On 10/21/20 15:31, Greg Kroah-Hartman wrote: > On Wed, Oct 21, 2020 at 02:15:04PM +0100, Qais Yousef wrote: > > > Without even looking at the patch set, this is not ok... > > > > Sorry about that. Please keep in mind we're still debating if we want to > > support this upstream. > > What do you mean by this? Err. I meant that we don't know how and if upstream would like to support this. The patches are on the list to discuss what it takes for inclusion. Like all other patches, it could be accepted or rejected. I am working on it to be accepted *of course* but it's not my decision. I just can't assume or take it for granted this will go in. I didn't see a straight no yet, so hopefully this is moving in the right direction. My point is there's still more discussion to be had and what presented in this RFC could change completely. Thanks -- Qais Yousef > Do you mean you will keep an out-of-tree patchset for all time for all > future kernel versions for everyone to pull from and you will support > for all chips that end up with this type of functionality? > > That's a huge task to do, you all must have a lot of money to burn! > > It is a "trivial cost" to get changes merged upstream compared to the > amount of time and money it costs to keep stuff out of the tree. Why > you would not want to do this is beyond me. > > But hey, I'm not in charge of your company's budget, for good reasons :) > > good luck! > > greg k-h
On Wed, Oct 21, 2020 at 03:31:05PM +0200, Greg Kroah-Hartman wrote: > On Wed, Oct 21, 2020 at 02:15:04PM +0100, Qais Yousef wrote: > > > Without even looking at the patch set, this is not ok... > > > > Sorry about that. Please keep in mind we're still debating if we want to > > support this upstream. > > What do you mean by this? > > Do you mean you will keep an out-of-tree patchset for all time for all > future kernel versions for everyone to pull from and you will support > for all chips that end up with this type of functionality? > > That's a huge task to do, you all must have a lot of money to burn! From a maintainer perspective (and not as an Arm employee), hopefully the high cost would be a deterrent to other such hardware combinations in the future ;). Even if it will (likely) end up in mainline, we shouldn't make it straightforward.