[vioscsi] Extend VioScsiReadRegistryParameter() #1216
Conversation
|
If anyone knows how to get As we're using a SAS BusType, we might need to set the |
|
I was minded to think how this PR might be of benefit to the Jira issue you previously mentioned. Firstly, the issue mentions enabling SG_IO. You can probably speak best as to whether or not this would still work, as Anyway, as for this PR it would mean such disks could co-exist with disks of a variety of backing characteristics because each HBA could have it's own NOPB and MaximumTransferLength. The solution that I imagined, would require a PR for new NOPB calculations, like the one I mentioned here. The final part of the solution would be a PR for QEMU to set There's probably other ways to do it too - perhaps even solely on the driver side via VPD - but I believe this would require a HBA restart with new parameters rather than single init during Best regards, |
benyamin-codez
force-pushed
the
vioscsi-read-reg-per-hba
branch
2 times, most recently
from
411fe52 to
ff22c8e
Compare
benyamin-codez
force-pushed
the
vioscsi-read-reg-per-hba
branch
from
ff22c8e to
09b0439
Compare
benyamin-codez
requested review from
vrozenfe and
YanVugenfirer
as code owners
Adds capability to do per HBA registry reads. This permits, e.g. PhysicalBreaks_001, PhysicalBreaks_002, etc. as an alternative to \Parameters\Device(d) by instead using the format \Parameters\Device\Valuename_123. Includes FIXME NOTE regarding StorPortGetSystemPortNumber(). StorPortGetSystemPortNumber() should return before HBA-specific registry reads can successfully use \Parameters\Device(d). The HBA ID that determines the "123" in Valuename_123 of the registry path \Parameters\Device\Valuename_123 is the HBA slot number minus 1. Credit to @MartinDrab for eagle-eyed code review. 8^D Signed-off-by: benyamin-codez <[email protected]>
benyamin-codez
force-pushed
the
vioscsi-read-reg-per-hba
branch
from
09b0439 to
07693aa
Compare
|
I have reverted I also split coincidental traces in We now just set the identifier to This one should be right to go... 8^d |
|
Green ✔️ for the Anymore checks for this PR @kostyanf14 ..? |
|
Just a The mentions in PR #1297 apply also to this PR...
|
|
Green ✔️ for the |
|
@benyamin-codez Best, |
I'm unsure if this will provide sufficient insight or justification, but these are the registry values defined in my WIP: #define REGISTRY_BUSTYPE "BusType"
#define REGISTRY_MAX_PH_BREAKS "PhysicalBreaks"
#define REGISTRY_MAX_PH_SEGMENTS "MaxPhysicalSegments"
#define REGISTRY_ACTION_ON_RESET "VioscsiActionOnReset"
#define REGISTRY_RESP_TIME_LIMIT "TraceResponseTime"
#define REGISTRY_ALLOC_FOR_CPU_HOTPLUG "AllocForCpuHotplug"
#define REGISTRY_ONLY_DMA32BITADDRESSES "OnlyDma32BitAddresses"
#define REGISTRY_INITIATOR_BUS_ID "InitiatorBusId"I'm unsure how useful the first and last are to the project. They would be per-HBA. Maybe the same for the reset values?
I agree that the best way to initially set this is via Leaving Workloads involved in the processing and transfer of large files will obviously benefit from larger values of MaxXferLen, which is directly proportional to the number of segments, and this mechanism is really the only way to set this. The reality is some workload and backing combinations perform better with a higher number of segments and others perform better when the queue characteristics are matched to those of the backing. Per RHEL-56722, sometimes they MUST be the same value. Some work better with a multiple of the backing's max_segments queue value; some don't care if it's a multiple. Some max_segments values are not Base2-aligned and sometimes this matters, and other times not. One argument for per-HBA tuning would be for the scenarios where we are mixing workloads, e.g. where a transactional RDBMS might be in use, such as in Exchange and SQL Server, where we might have DATA disks with 64KiB clusters and LOG disks with 4KiB clusters, and where a specific number of segments has the best performance for each disk. The fix really needs to come via QEMU. We need it to report the backing's max_segments queue value for breaking cases such as the one in RHEL-56722. So this then would be safe if not ideal. It would then be up to sys engineers or admins during implementation / commissioning to tune this if necessary or desired. There are no doubt many things that QEMU devs would need to consider when coming up with a fix. One consideration needs to be: when in non-pass-through mode with multiple backings with different max_segments queue values. I would suggest the HBA would need to be set to the lowest value. Regardless of any fix to QEMU, having the capability to tune each and every Does that make sense? Best regards, |
| * Storport provided slot_number minus one, padded to hundreds, | ||
| * e.g. \Parameters\Device\Valuename_123. | ||
| * | ||
| * This permits up to 999 HBAs. That ought to be enough... c( O.O )ɔ |
There was a problem hiding this comment.
@vrozenfe @YanVugenfirer
c( O.O )ɔ <-- This is me looking at you guys in astonishment...
... after saying catering for 999 (incorrectly - it should be 1,000 btw) HBAs in a system should be enough...!
I decided to use a generous value to demonstrate the ASCII conversion below.
If you want to provide a more realistic number it could simplify that part.
You will notice the division is on Base10 boundaries (units, tens, hundreds).
Therefore, the maximums would be 10, 100 and 1,000 HBAs.
I suspect we only need units and tens, catering for up to 100 HBAs, but let me know what you think.
| * | ||
| * Difficult to determine what is wrong here... | ||
| * ¯\_(ツ)_/¯ | ||
| * |
There was a problem hiding this comment.
My present thoughts on this issue with Parameters\Device(d) not initialising:
Being a BusTypeSas adapter, I suspect we may not be setting adaptExt->port_idx early enough.
I'll try to test this in the next few days...
...but if anyone has any insights, please do let me know ASAP before I burn more time on this.
Adds capability to do per-HBA registry reads.
This permits, e.g.
PhysicalBreaks_001,PhysicalBreaks_002, etc.This is an alternative to
\Parameters\Device(d)by instead using the format\Parameters\Device\Valuename_123.Includes
FIXME NOTEregardingStorPortGetSystemPortNumber().StorPortGetSystemPortNumber()should return ......before HBA-specific registry reads can successfully use
\Parameters\Device(d).Coincidentally updated the following in
VioScsiFindAdapter():adaptExt->dump_mode(new tracing)adaptExt->hba_id(new tracing and new definition)The HBA ID that determines the "123" in
Valuename_123of ......the registry path
\Parameters\Device\Valuename_123is the HBA slot number minus 1.