Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add BIOS settings for new equipment #65

Merged
merged 6 commits into from
Dec 6, 2021
Merged

Add BIOS settings for new equipment #65

merged 6 commits into from
Dec 6, 2021

Conversation

kmdkuk
Copy link
Contributor

@kmdkuk kmdkuk commented Nov 26, 2021

  • add BIOS.ProcSettings.NumaNodesPerSocket settings for R6525 and R7525
  • update DSU_VERSION to 21.11.26
  • remove document for rkt

Signed-off-by: kouki kouworld0123@gmail.com

@kmdkuk kmdkuk self-assigned this Nov 26, 2021
@kmdkuk kmdkuk force-pushed the add-bios-settings branch 2 times, most recently from 858bcb5 to 15bfac1 Compare November 30, 2021 08:56
@kmdkuk kmdkuk changed the title WIP Add BIOS settings for new equipment Add BIOS settings for new equipment Dec 2, 2021
Signed-off-by: kouki <kouworld0123@gmail.com>
Signed-off-by: kouki <kouworld0123@gmail.com>
Signed-off-by: kouki <kouworld0123@gmail.com>
@kmdkuk kmdkuk marked this pull request as ready for review December 2, 2021 08:03
}
case lib.R7525:
// https://www.dell.com/support/manuals/ja-jp/poweredge-r7525/r7525_ism_pub/%E3%83%A1%E3%83%A2%E3%83%AA%E3%83%BC-%E3%83%A2%E3%82%B8%E3%83%A5%E3%83%BC%E3%83%AB%E5%8F%96%E3%82%8A%E4%BB%98%E3%81%91%E3%82%AC%E3%82%A4%E3%83%89%E3%83%A9%E3%82%A4%E3%83%B3?guid=guid-80b1c1ad-14b7-4dd6-b122-abb0c82bd3e8&lang=ja-jp
return dc.enqueueConfig(ctx, numaSettingsKey, "4")
Copy link
Member

@ymmt2005 ymmt2005 Dec 2, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We might change the configurations within a single Dell generation.
Why don't we check # of CPUs & memory units and set the appropriate NUMA setting?

return err
}

numaSettingsKey := "BIOS.ProcSettings.NumaNodesPerSocket"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is used only once. I don't like that the definition and usage are placed so distantly.
Why don't you embed the literal directly at the usage?

Comment on lines 284 to 285
case lib.R6525:
case lib.R7525:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
case lib.R6525:
case lib.R7525:
case lib.R6525, lib.R7525:

@kmdkuk kmdkuk force-pushed the add-bios-settings branch 2 times, most recently from 269fd59 to e27e0e4 Compare December 2, 2021 09:19
ymmt2005
ymmt2005 previously approved these changes Dec 2, 2021
Copy link
Member

@ymmt2005 ymmt2005 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

lib/hardware.go Outdated Show resolved Hide resolved
lib/hardware.go Outdated Show resolved Hide resolved
lib/hardware.go Outdated Show resolved Hide resolved
lib/hardware.go Outdated Show resolved Hide resolved
Signed-off-by: kouki <kouworld0123@gmail.com>
Signed-off-by: kouki <kouworld0123@gmail.com>
Signed-off-by: kouki <kouworld0123@gmail.com>
Copy link
Contributor

@morimoto-cybozu morimoto-cybozu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@kmdkuk kmdkuk merged commit dffedde into main Dec 6, 2021
@kmdkuk kmdkuk deleted the add-bios-settings branch December 6, 2021 02:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants