Paths

Table of Contentst

cam/ata: Postpone removal of two compat sysctl until 15
ClosedPublic
Actions

Authored by zlei on Oct 30 2023, 8:43 AM.

Details

Reviewers

imp
ken

Group Reviewers

cam

Commits

rGd9e6e99a8525: cam/ata: Postpone removal of two compat sysctls until 15
rG6c7ddf2c1437: cam/ata: Postpone removal of two compat sysctls until 15
rGfb288d493989: cam/ata: Postpone removal of two compat sysctls until 15
rGd24729b2fd66: cam/ata: Postpone removal of two compat sysctls until 15

Summary

Prefer UNMAPPEDIO and ROTATING from flags sysctl. See

aeab0812e68c (Add flags sysctl to ada)
cf3ff63e55e4 (Convert unmappedio over to a flag)
96eb32bf0f5a (Convert rotating to a flag bit)

MFC after: 1 day

Diff Detail

Repository

rG FreeBSD src repository

Lint

Lint Skipped

Unit

Tests Skipped

Event Timeline

zlei created this revision.Oct 30 2023, 8:43 AM

Herald added a reviewer: cam. · View Herald TranscriptOct 30 2023, 8:43 AM

zlei requested review of this revision.Oct 30 2023, 8:43 AM

imp accepted this revision.Oct 30 2023, 3:11 PM

This revision is now accepted and ready to land.Oct 30 2023, 3:11 PM

This makes it sound like unmapped I/O and rotating media support will be removed in FreeBSD 15.

From the context, I think you just mean that the separate sysctl variables to control those settings will be removed, not the underlying support. Is that correct?

If so, maybe we could change the description to something like "Unmapped I/O sysctl var is *DEPRECATED*, gone in FreeBSD 15"

If not, and you really mean that unmapped I/O and rotating media are getting removed, then I probably missed the discussion and I'd like to know why.

This revision now requires changes to proceed.Oct 30 2023, 3:25 PM

In D42402#967566, @ken wrote:

This makes it sound like unmapped I/O and rotating media support will be removed in FreeBSD 15.

From the context, I think you just mean that the separate sysctl variables to control those settings will be removed, not the underlying support. Is that correct?

Yes you are right.

The unmapped I/O and rotating media are now flag bits. The sysctl knobs are kept only for compatibility (with old programs).

If so, maybe we could change the description to something like "Unmapped I/O sysctl var is *DEPRECATED*, gone in FreeBSD 15"

Sounds good.

If not, and you really mean that unmapped I/O and rotating media are getting removed, then I probably missed the discussion and I'd like to know why.

It's just the sysctl. The info has moved. But I added it and people have been slow to move... so maybe we don't rush the removal...

Plus these all are read only: there's no control here.

zlei updated this revision to Diff 129546.Oct 31 2023, 1:18 PM

zlei retitled this revision from cam/ata: Postpone removal of unmapped I/O and rotating media until 15 to cam/ata: Postpone removal of two sysctl knobs until 15.

zlei edited the summary of this revision. (Show Details)

Address @ken 's comments.

Within the context, Unmapped I/O sysctl should be clear enough.

Looks good, thank you!

This revision is now accepted and ready to land.Oct 31 2023, 1:24 PM

imp added inline comments.Oct 31 2023, 2:25 PM

sys/cam/ata/ata_da.c
1546	"The device is using unmapped I/O. sysctl..." Might be better with a similar rewording below.

zlei added inline comments.Oct 31 2023, 2:46 PM

sys/cam/ata/ata_da.c
1546	What about `Use unmapped I/O. This sysctl is DEPRECATED, gone in FreeBSD 15` ? That is shorter. 1 means use, 0 otherwise.
1550	`Rotating media. This sysctl is DEPRECATED, gone in FreeBSD 15` sounds much clear. @imp How about this one ?

zlei retitled this revision from cam/ata: Postpone removal of two sysctl knobs until 15 to cam/ata: Postpone removal of two compat sysctl until 15.Oct 31 2023, 2:47 PM

A friendly ping before committing this.

imp added inline comments.Nov 2 2023, 4:23 AM

sys/cam/ata/ata_da.c
1546	"Uses unmapped I/O." sounds better, but otherwise I like the above.
1550	I like it.

Also... with the tweak. This is approved

Closed by commit rGd24729b2fd66: cam/ata: Postpone removal of two compat sysctls until 15 (authored by zlei). · Explain WhyNov 2 2023, 5:18 AM

This revision was automatically updated to reflect the committed changes.

zlei added a commit: rGd24729b2fd66: cam/ata: Postpone removal of two compat sysctls until 15.

zlei added a commit: rGfb288d493989: cam/ata: Postpone removal of two compat sysctls until 15.Nov 2 2023, 5:22 AM

zlei added a commit: rG6c7ddf2c1437: cam/ata: Postpone removal of two compat sysctls until 15.Nov 2 2023, 5:24 AM

zlei added a commit: rGd9e6e99a8525: cam/ata: Postpone removal of two compat sysctls until 15.

Is there enough space in the message to help users by giving them a reference to the new interface to express the same configuration and what form of reference would work well for this?

In D42402#969272, @crest_freebsd_rlwinm.de wrote:

Is there enough space in the message to help users by giving them a reference to the new interface to express the same configuration and what form of reference would work well for this?

Prefer UNMAPPEDIO and ROTATING from flags sysctl

I think the commit message is clear. Maybe I should point to the flags sysctl directly.
This is one from my machine.

# sysctl kern.cam.ada.0.flags
kern.cam.ada.0.flags: 0x1be3bde<CAN_48BIT,CAN_FLUSHCACHE,CAN_NCQ,CAN_DMA,WAS_OTAG,CAN_TRIM,OPEN,SCTX_INIT,CAN_POWERMGT,CAN_DMA48,CAN_LOG,CAN_WCACHE,CAN_RAHEAD,PROBED,ANNOUNCED,DIRTY,PIM_ATA_EXT,UNMAPPEDIO>

Revision Contents
Changeset List

Path

Size

sys/

cam/

ata/

ata_da.c

4 lines

Diff 129546

View Options

cam/ata: Postpone removal of two compat sysctl until 15ClosedPublicActions