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

[collectd 6] src/mmc.c port to collectd 6 #4077

Merged
merged 7 commits into from
Feb 17, 2023

Conversation

hnez
Copy link
Contributor

@hnez hnez commented Jan 11, 2023

ChangeLog: mmc plugin: Port to collectd 6

The mmc plugin was added to the main branch after the collectd-6 branch was split off.
This PR:

This brings the plugin to feature parity with the main branch.

feckert and others added 4 commits December 14, 2022 09:08
Signed-off-by: Florian Eckert <fe@dev.tdt.de>
* mmc plugin: integrate into configure.ac

The mmc plugin is not fully integrated in the configure.ac.
Change that.

Signed-off-by: Leonard Göhrs <l.goehrs@pengutronix.de>

* mmc plugin: Skip mmc paths in /sys that start with a '.' (like "." and "..")

The plugin tries to (and obiously fails to) use "." and "..", that come out of
listdir, as mmc devices.
Filter these two out by skipping hidden files/directories.

Signed-off-by: Leonard Göhrs <l.goehrs@pengutronix.de>

* mmc plugin: read standard eMMC 5.0 health metrics

Signed-off-by: Leonard Göhrs <l.goehrs@pengutronix.de>

* mmc plugin: remove type-name defines

These defines can become confusing, especially when combined with the defines
for attribute names in the sysfs. This will only get worse when more
vendor-specific metrics are supported.
Remove the defines and use the type names directly.

Signed-off-by: Leonard Göhrs <l.goehrs@pengutronix.de>

* mmc plugin: remove sysfs-attribute defines

These defines are used only once or twice and do not help with readability.
Replace them with just the raw strings.

Signed-off-by: Leonard Göhrs <l.goehrs@pengutronix.de>

* mmc plugin: port to libudev

While using the sysfs directly works fine for the swissbit and generic eMMC
driver it does not scale well to other vendor-specific interfaces where one has
to open the block device in /dev to perform ioctls.

Signed-off-by: Leonard Göhrs <l.goehrs@pengutronix.de>

* mmc plugin: add micron eMMC support

While this patch was only tested with a single product (MTFC16GAPALBH) I am
fairly confident that it will generalize to others as well, as micron
themselves ship a single tool[1], which this patch uses as a reference, to read
similar info from all of their eMMCs.

This patch also increases the maximum value of mmc_bad_blocks to infinity,
as it can be any 16 bit integer for micron eMMC but could be even larger for
other vendors.

[1]: https://github.com/arcus-smart-home/arcushubos/blob/master/meta-iris/recipes-core/iris-utils/files/emmcparm_1.0.c

Signed-off-by: Leonard Göhrs <l.goehrs@pengutronix.de>

* mmc plugin: add sandisk eMMC support

While this patch was only tested with a single product (SDINBDG4-8G), I am
fairly confident that it should generalize to other devices as well,
as the current product portfolio on their website looks very similar to the one
I tested and new devies will likely use a Western Digital manufacturer ID.

Signed-off-by: Leonard Göhrs <l.goehrs@pengutronix.de>
Udev rules can contain a "watch" option, which is described in the man page as:

  Watch the device node with inotify; when the node is closed after being
  opened for writing, a change uevent is synthesized.

This watch option is enabled by default for all block devices[1].
The intention behind this is to be notified about changes to the partition
table. The mmc plugin does however also need to open the block device for
writing, even though it never modifies its content, in order to be able to
issue ioctls with vendor defined MMC-commands.

Reduce the amount of generated change events from one per read to one per
collectd runtime by caching the open file descriptor.

[1]: https://github.com/systemd/systemd/blob/ef2ad30aee9fa99b0fdb8fe7efda397513cec6af/rules.d/60-block.rules

Fixes: 2f15c70 (mmc: add more vendor specific and generic data sources (collectd#4006))
Signed-off-by: Leonard Göhrs <l.goehrs@pengutronix.de>
Signed-off-by: Leonard Göhrs <l.goehrs@pengutronix.de>
@hnez hnez requested a review from a team as a code owner January 11, 2023 11:06
@collectd-bot collectd-bot added this to the 6.0 milestone Jan 11, 2023
@hnez hnez changed the title [collectd 6] src/memory.c port to collectd 6 [collectd 6] src/mmc.c port to collectd 6 Jan 11, 2023
@eero-t
Copy link
Contributor

eero-t commented Jan 11, 2023

@elfiesmelfie Details link does not show what fails with the v6 RH el6_x86_64 build. Could you help?

@elfiesmelfie
Copy link
Contributor

elfiesmelfie commented Jan 12, 2023

@elfiesmelfie Details link does not show what fails with the v6 RH el6_x86_64 build. Could you help?

There appear to be some errors in building the mmc plugin:

src/mmc.c:34:29: error: linux/mmc/ioctl.h: No such file or directory 
src/mmc.c: In function 'mmc_micron_cmd56': 
src/mmc.c:268: error: variable 'cmd' has initializer but incomplete type 
src/mmc.c:269: error: unknown field 'opcode' specified in initializer cc1: warnings being treated as errors 
src/mmc.c:269: 
error: excess elements in struct initializer 
src/mmc.c:269: 
error: (near initialization for 'cmd') 
src/mmc.c:270: 
error: unknown field 'arg' specified in initializer 
src/mmc.c:270: 
error: excess elements in struct initializer src/mmc.c:270: error: (near initialization for 'cmd') 
src/mmc.c:271: error: unknown field 'flags' specified in initializer 
src/mmc.c:271: error: excess elements in struct initializer 
src/mmc.c:271: error: (near initialization for 'cmd') 
src/mmc.c:272: error: unknown field 'blksz' specified in initializer 
src/mmc.c:272: error: excess elements in struct initializer 
src/mmc.c:272: error: (near initialization for 'cmd') 
src/mmc.c:273: error: unknown field 'blocks' specified in initializer 
src/mmc.c:273: error: excess elements in struct initializer 
src/mmc.c:273: error: (near initialization for 'cmd') 
src/mmc.c:268: error: storage size of 'cmd' isn't known 
src/mmc.c:276: error: implicit declaration of function 'mmc_ioc_cmd_set_data' 
src/mmc.c:278: error: 'MMC_IOC_CMD' undeclared (first use in this function) 
src/mmc.c:278: error: (Each undeclared identifier is reported only once src/mmc.c:278: error: for each function it appears in.) 
src/mmc.c:268: error: unused variable 'cmd' src/mmc.c: In function 'mmc_read_sandisk': 
src/mmc.c:388: error: variable 'cmd_en_report_mode' has initializer but incomplete type 
src/mmc.c:389: error: unknown field 'opcode' specified in initializer 
src/mmc.c:389: error: excess elements in struct initializer 
src/mmc.c:389: error: (near initialization for 'cmd_en_report_mode') 
src/mmc.c:390: error: unknown field 'arg' specified in initializer 
src/mmc.c:390: error: excess elements in struct initializer 
src/mmc.c:390: error: (near initialization for 'cmd_en_report_mode') 
src/mmc.c:391: error: unknown field 'flags' specified in initializer 
src/mmc.c:391: error: excess elements in struct initializer 
src/mmc.c:391: error: (near initialization for 'cmd_en_report_mode') 
src/mmc.c:388: error: storage size of 'cmd_en_report_mode' isn't known 
src/mmc.c:393: error: variable 'cmd_read_report' has initializer but incomplete type 
src/mmc.c:394: error: unknown field 'opcode' specified in initializer src/mmc.c:394: error: excess elements in struct initializer 
src/mmc.c:394: error: (near initialization for 'cmd_read_report') 
src/mmc.c:395: error: unknown field 'arg' specified in initializer src/mmc.c:395: error: excess elements in struct initializer 
src/mmc.c:395: error: (near initialization for 'cmd_read_report') src/mmc.c:396: error: unknown field 'flags' specified in initializer 
src/mmc.c:396: error: excess elements in struct initializer 
src/mmc.c:396: error: (near initialization for 'cmd_read_report') src/mmc.c:397: error: unknown field 'blksz' specified in initializer 
src/mmc.c:397: error: excess elements in struct initializer 
src/mmc.c:397: error: (near initialization for 'cmd_read_report') 
src/mmc.c:398: error: unknown field 'blocks' specified in initializer 
src/mmc.c:398: error: excess elements in struct initializer 
src/mmc.c:398: error: (near initialization for 'cmd_read_report') 
src/mmc.c:393: error: storage size of 'cmd_read_report' isn't known 
src/mmc.c:412: error: 'MMC_IOC_CMD' undeclared (first use in this function) 
src/mmc.c:393: error: unused variable 'cmd_read_report' 
src/mmc.c:388: error: unused variable 'cmd_en_report_mode' make[1]: *** [src/mmc_la-mmc.lo] Error 1

Source: https://api.cirrus-ci.com/v1/task/4589352408317952/logs/build.log

@eero-t
Copy link
Contributor

eero-t commented Jan 12, 2023

@elfiesmelfie Details link does not show what fails with the v6 RH el6_x86_64 build. Could you help?

There appear to be some errors in building the mmc plugin:

src/mmc.c:34:29: error: linux/mmc/ioctl.h: No such file or directory 
...

Source: https://api.cirrus-ci.com/v1/task/4589352408317952/logs/build.log

Thanks! I assume the missing kernel header to be causing also the other problems, as other builds went fine. What kernel is used by the RH EL6 build?

@elfiesmelfie
Copy link
Contributor

Thanks! I assume the missing kernel header to be causing also the other problems, as other builds went fine. What kernel is used by the RH EL6 build?

EL6 is CentOS 6. Active support ended on 2017, and security support ended in 2020. There won't be updates, so perhaps we should remove it from CI.

https://endoflife.date/centos

@eero-t
Copy link
Contributor

eero-t commented Jan 12, 2023

EL6 is CentOS 6. Active support ended on 2017, and security support ended in 2020. There won't be updates, so perhaps we should remove it from CI.

@mrunge Yes, definitely. Especially for v6 branch that does not have even a release yet.

@hnez
Copy link
Contributor Author

hnez commented Jan 30, 2023

Hi,

I was a bit surprised that this PR failed the CI run, as it is just a port of a plugin already in main and I did not remember it failing there. As it turns out this is just a bit of fallout from collectd-6s divergence from main, as CentOS6 was already disabled in main by #3784.
Should I open a PR that cherry-picks that change?

@eero-t
Copy link
Contributor

eero-t commented Jan 30, 2023

As it turns out this is just a bit of fallout from collectd-6s divergence from main, as CentOS6 was already disabled in main by #3784. Should I open a PR that cherry-picks that change?

Yes, please!

@hnez
Copy link
Contributor Author

hnez commented Jan 30, 2023

As it turns out this is just a bit of fallout from collectd-6s divergence from main, as CentOS6 was already disabled in main by #3784. Should I open a PR that cherry-picks that change?

Yes, please!

Alright then. While I was at it I've also picked some other changes and created #4084. Reviews welcome.

@eero-t
Copy link
Contributor

eero-t commented Feb 17, 2023

CI failures are for virt, smart, nut, mount and cpufreq plugins, and not related to these changes. They are known issues from v5 main branch.

@mrunge mrunge merged commit 4bd5638 into collectd:collectd-6.0 Feb 17, 2023
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

6 participants