[LU-9431] class_process_proc_param can't handle sysfs

• Status: Resolved
• Project: Lustre
• Affects Version/s: Lustre 2.10.0
• Fix Version/s: Lustre 2.11.0
• Type: Bug
• Priority: Major
• Reporter: James A Simmons
• Assignee: James A Simmons
• Resolution: Fixed
• Labels: patch
• Epic/Theme: upstream
• Severity: 3

Description

Lustre has the ability to tune proc values using lctl conf_param on the MGS server. Those changes are stored and distributed to all the nodes avoiding to the complex process of setting ever node. The function class_process_proc_param() is used to change the setting using the lprocfs_vars that map to the proc file. With the move to sysfs/debugfs this no longer works. A new approach has to be written to handle sysfs settings. Also we should consider dropping setting debugfs entries since they tend to contain large quantities which the user will typical not set that kind of data

Issue Links

• is related to: LU-8066 Move lustre procfs handling to sysfs ...
• is related to: LU-7004 fix "lctl set_param -P" to allow depr...
• is related to: LU-10869 conf-sanity test 76a fails with 'erro...
• is related to: LU-10756 Send Uevents for interesting Lustre c...

Comments

[Andreas Dilger]

There is already "lctl set_param -P" available for some time that replaces the "lctl conf_param" mechanism. The benefit is that "set_param -P" is using an upcall to lctl in userspace with parameter names the same as regular set_param is doing, unlike conf_param which parses these in a different manner internally and sets the parameters directly.

The main drawback is that set_param -P has some issues that haven't been resolved since the patch was originally landed from Xyratex (LU-7183 and LU-7004 at least). As a result, we haven't been able to totally deprecate conf_param.

As for debugfs parameters that have complex values for output, I can assure you that any of them which accept writes do not allow such complex values for input. In general, any writable parameters would allow clearing the counters that generate the output, or setting some limited tunable value. I don't think it makes sense to explicitly prevent writing to debugfs parameters beyond the existing case of making the parameter read-only if there is no write handler.

[James A Simmons]

Funny Oleg had the discussion with Greg about using lctl to set parameter which he didn't like. Yet looking at the upstream client this infrastructure is there. So you are suggesting we should instead fix set_param -P and truly deprecate conf_param. Question I have is do we have a list of where set_param -P is broken?

[Andreas Dilger]

There are several issues linked under LU-7004 that show problems with "set_param -P", but of course there could be others. My suggestion is to change the scripts under lustre/tests to preferentially use lctl set_param -P, and only leave a few places that use lctl conf_param for compatibility testing. That way we can be sure that set_param -P is at least working well enough for all the existing regression tests.

I see conf_param used in the following test scripts:

lustre/tests/conf-sanity.sh:67
lustre/tests/test-framework.sh:12
lustre/tests/sanity.sh:9
lustre/tests/sanity-quota.sh:3
lustre/tests/ost-pools.sh:2
lustre/tests/sanityn.sh:2

[Peter Jones]

James

Is this something that you are intending to work on?

Peter

[Andreas Dilger]

Based on the previous comments, it looks like this is a prerequisite for the sysfs changes to land.

[James A Simmons]

Yep. The current method for lctl conf_param only works for debugfs or proc but not sysfs. lctl set_parm -P will work with both. I just updated https://review.whamcloud.com/#/c/24031 to test lctl set_param -P for the llite layer.

[Peter Jones]

ok - thanks James

[Gerrit Updater]

James Simmons ([e-mail]) uploaded a new patch: https://review.whamcloud.com/30143
Subject: LU-9431 obd: resolve config log sysfs issues
Project: fs/lustre-release
Branch: master
Current Patch Set: 1
Commit: 4145792fc0f91ec83debd3f613767ddecf490bc5

[James A Simmons]

I managed to lctl conf_param working with sysfs and debugfs. For the debugfs case for conf_param and in general for lctl set_param -P uevents are now being sent. In turn udev sees the event and acts on it based on the udev rules. By default we have:

SUBSYSTEM=="lustre", ACTION=="change", ENV{PARAM}=="?*", RUN+="/usr/sbin/lctl set_param $env{PARAM}=$env{SETTING}"

As you can see this approach allows a lot more flexibility. You can for example filter based on DEVPATH to only change variables on specific obd devices. Secondly this approach allows admins to run a different application or script instead of lctl. For example a admin might like to know if a tunable has been changed. Next an interesting feature that comes out of this is the ability to configure lustre with udev rules. For example we could do a

SUBSYSTEM=="lustre", ACTION=="add", DEVPATH=="fs/lustre/osc/lustre-OST*", ATTR{max_dirty_mb}=64

This opens up new possibilities here.

[Andreas Dilger]

Very interesting. I think this is a good way to implement the "set_param -P" functionality without having to use an upcall. We still need to fork/exec for each parameter, but that is no worse than the old upcall method.

I assume we can just put a "lustre.conf" (or whatever) file in /etc/udev.d/ directory and this will work?

[James A Simmons]

I fixed the last remaining bug in this patch. Pleased review. Yes have a lustre.conf gile would work in /etc/udev.d to configure lustre.

[Gerrit Updater]

Oleg Drokin ([e-mail]) merged in patch https://review.whamcloud.com/30143/
Subject: LU-9431 obd: resolve config log sysfs issues
Project: fs/lustre-release
Branch: master
Current Patch Set:
Commit: a74b2b5bce71367506e59f1bdba163eb21235a29

[Peter Jones]

Landed for 2.11