add new notifier function ,take2

Hi,

A big thanks to everybody who read and replied to first version. I have tried to incorporate reviewer's comments and suggestions.

changelog take1 -> take2

- Rebased 2.6.23
- comment updated
- renamed the notifiner name "tunable_notifier" to "tunable_atomic_notifier"
- fixed typo

These patches add new notifier function and implement it to panic_notifier_list. We used the hardcoded notifier chain so far, but it was not flexible. New notifier is very flexible, because user can change a list of order by debugfs.

Thanks,

Example)

# cd /sys/kernel/debug/
# ls
kprobes  pktcdvd
# insmod ipmi_msghandler.ko
# ls
kprobes  panic_notifier_list  pktcdvd
# cd panic_notifier_list/
# ls
ipmi_msghandler
# insmod ipmi_watchdog.ko
# ls
ipmi_msghandler  ipmi_wdog
# cat ipmi_msghandler/priority
200
# cat ipmi_wdog/priority
150
#
Kernel panic - not syncing: panic
ipmi_msghandler : notifier calls panic_event().
ipmi_watchdog : notifier calls wdog_panic_handler().

.....(reboot)

# cat ipmi_msghandler/priority
200
# cat ipmi_wdog/priority
150
# echo 300 > ipmi_wdog/priority
#
Kernel panic - not syncing: panic
ipmi_watchdog : notifier calls wdog_panic_handler().
ipmi_msghandler : notifier calls panic_event().

Quoted 44 lines · On Thu, 18 Oct 2007 15:45:08 +0900 Takenori Nagano <[email protected]>…

Hi,

A big thanks to everybody who read and replied to first version. I have tried to incorporate reviewer's comments and suggestions.

changelog take1 -> take2

- Rebased 2.6.23
- comment updated
- renamed the notifiner name "tunable_notifier" to "tunable_atomic_notifier"
- fixed typo

These patches add new notifier function and implement it to panic_notifier_list. We used the hardcoded notifier chain so far, but it was not flexible. New notifier is very flexible, because user can change a list of order by debugfs.

Thanks,

Example)

# cd /sys/kernel/debug/
# ls
kprobes  pktcdvd
# insmod ipmi_msghandler.ko
# ls
kprobes  panic_notifier_list  pktcdvd
# cd panic_notifier_list/
# ls
ipmi_msghandler
# insmod ipmi_watchdog.ko
# ls
ipmi_msghandler  ipmi_wdog
# cat ipmi_msghandler/priority
200
# cat ipmi_wdog/priority
150
#
Kernel panic - not syncing: panic
ipmi_msghandler : notifier calls panic_event().
ipmi_watchdog : notifier calls wdog_panic_handler().

.....(reboot)

Quoted 9 lines

# cat ipmi_msghandler/priority
200
# cat ipmi_wdog/priority
150
# echo 300 > ipmi_wdog/priority
#
Kernel panic - not syncing: panic
ipmi_watchdog : notifier calls wdog_panic_handler().
ipmi_msghandler : notifier calls panic_event().

I can sort-of see what this is doing. Runtime-definable management of which notifier functions will be called on a panic? Or maybe I misunderstood.

But even if I did understand, I don't understand why Linux needs this feature - what are the use cases, what is the value to our users?

Can you please flesh that information out a bit more?

The patches are somewhat wordwrapped - please check your email client configuration, thanks.

Quoted 10 lines · Andrew Morton

On Thu, 18 Oct 2007 15:45:08 +0900 Takenori Nagano <[email protected]> wrote:
I can sort-of see what this is doing. Runtime-definable management of which notifier functions will be called on a panic? Or maybe I misunderstood.

But even if I did understand, I don't understand why Linux needs this feature - what are the use cases, what is the value to our users?

Can you please flesh that information out a bit more?

Hi Andrew,

Takenori wants to make kdb and kdump co-exist. Currently after panic() panic_notifier_list is not executed if kdump is configured. Before list is executed, system will boot into second kernel to capture the dump. Hence if even if kdb was registered on panic_notifier_list, it will never get a chance to run.

Quoted 7 lines

My stance is that _all_ the RAS tools (kdb, kgdb, nlkd, netdump, lkcd, crash, kdump etc.) should be using a common interface that safely puts the entire system in a stopped state and saves the state of each cpu. Then each tool can do what it likes, instead of every RAS tool doing its own thing and they all conflict with each other, which is why this thread started.

Quoted 5 lines

It is not the kernel's job to decide which RAS tool runs first, second etc., it is the user's decision to set that policy. Different sites will want different orders, some will say "go straight to kdump", other sites will want to invoke a debugger first. Sites must be able to define that policy, but we hard code the policy into the kernel.

Quoted 2 lines

The patches are somewhat wordwrapped - please check your email client configuration, thanks.

Quoted 7 lines · Vivek Goyal

Quoted 7 lines

My stance is that _all_ the RAS tools (kdb, kgdb, nlkd, netdump, lkcd, crash, kdump etc.) should be using a common interface that safely puts the entire system in a stopped state and saves the state of each cpu. Then each tool can do what it likes, instead of every RAS tool doing its own thing and they all conflict with each other, which is why this thread started.

Quoted 5 lines

Quoted 5 lines

It is not the kernel's job to decide which RAS tool runs first, second etc., it is the user's decision to set that policy. Different sites will want different orders, some will say "go straight to kdump", other sites will want to invoke a debugger first. Sites must be able to define that policy, but we hard code the policy into the kernel.

I agreed with him and I made new notifier function that users can change the order. Priority value in notifier blocks are hardcoded. If users want to change list order, they have to rebuild kernel. I think it is very unhappy.

Quoted 2 lines · Nick Piggin

One thing I'd suggest is not to use debugfs, if it is going to be a useful end-user feature.

Is /sys/kernel/notifier_name/ an appropriate place?

Quoted 6 lines · Takenori Nagano

Quoted 2 lines · Nick Piggin

One thing I'd suggest is not to use debugfs, if it is going to be a useful end-user feature.

Is /sys/kernel/notifier_name/ an appropriate place?

I'm curious about the /sys/kernel/ namespace. I had presumed that it is intended to replace /proc/sys/ basically with the same functionality.

I _assume_ these are system software stats and tunables that are not exactly linked to device drivers (OTOH, where do you draw the line? eg. Would filesystems go here?

Core network algorithm tunables might, but per interface ones probably not...).

I don't know. Is there guidelines for sysfs (and procfs for that matter)? Is anyone maintaining it (not the infrastructure, but the actual content)?

It's kind of ironic that /proc/sys/ looks like one of the best organised directories in proc, while /sys/kernel seems to be in danger of becoming a mess: it has kexec and uevent files in the base directory, rather than in subdirectories...

Quoted 5 lines · Nick Piggin

Quoted 4 lines · Takenori Nagano

Quoted 2 lines · Nick Piggin

One thing I'd suggest is not to use debugfs, if it is going to be a useful end-user feature.

Quoted 5 lines

Quoted 1 line

Is /sys/kernel/notifier_name/ an appropriate place?

I'm curious about the /sys/kernel/ namespace. I had presumed that it is intended to replace /proc/sys/ basically with the same functionality.

It was intended to be something like /proc/sys/kernel/ only.

Quoted 3 lines

I _assume_ these are system software stats and tunables that are not exactly linked to device drivers (OTOH, where do you draw the line? eg. Would filesystems go here?

We already have /sys/fs/ ?

Quoted 2 lines

Core network algorithm tunables might, but per interface ones probably not...).

We will merge the nonsense of "block/", "class/" and "bus/" to one "subsystem". The block, class, bus directories will only be kept as symlinks for compatibility. Then every subsystem has a directory like:
/sys/subsystem/block/, /sys/subsystem/net/ and the devices of the subsystem are in a devices/ directory below that. Just like the /sys/bus/< name>/devices/ layout looks today. All subsystem-global tunables can go below the /sys/subsystem/<name>/ directory, without clashing with the list of devices or anything else.

Quoted 3 lines

I don't know. Is there guidelines for sysfs (and procfs for that matter)? Is anyone maintaining it (not the infrastructure, but the actual content)?

Unfortunately, there was never really a guideline.

Quoted 4 lines

It's kind of ironic that /proc/sys/ looks like one of the best organised directories in proc, while /sys/kernel seems to be in danger of becoming a mess: it has kexec and uevent files in the base directory, rather than in subdirectories...

True, just looking at it now, people do crazy things like:
/sys/kernel/notes, which is a file with binary content, and a name nobody will ever be able to guess what it is good for. That should definitely go into a section/ directory. Also the VM stuff there should probably move to a /sys/vm/ directory along with the weird placed top-level /sys/slab/.

Quoted 12 lines · Kay Sievers

Quoted 10 lines · Nick Piggin

Quoted 5 lines · Takenori Nagano

Quoted 2 lines · Nick Piggin

One thing I'd suggest is not to use debugfs, if it is going to be a useful end-user feature.

Is /sys/kernel/notifier_name/ an appropriate place?

I'm curious about the /sys/kernel/ namespace. I had presumed that it is intended to replace /proc/sys/ basically with the same functionality.

Quoted 1 line

It was intended to be something like /proc/sys/kernel/ only.

Really? So you'd be happy to have a /sys/dev /sys/fs /sys/kernel /sys/net /sys/vm etc? "kernel" to me shouldn't really imply the stuff under the kernel/ source directory or other random stuff that doesn't fit into another directory, but attributes that are directly related to the kernel software (rather than directly associated with any device).

Quoted 9 lines

Quoted 3 lines

I _assume_ these are system software stats and tunables that are not exactly linked to device drivers (OTOH, where do you draw the line? eg. Would filesystems go here?

We already have /sys/fs/ ?

Quoted 2 lines

Core network algorithm tunables might, but per interface ones probably not...).

Quoted 8 lines

We will merge the nonsense of "block/", "class/" and "bus/" to one "subsystem". The block, class, bus directories will only be kept as symlinks for compatibility. Then every subsystem has a directory like:
/sys/subsystem/block/, /sys/subsystem/net/ and the devices of the subsystem are in a devices/ directory below that. Just like the /sys/bus/< name>/devices/ layout looks today. All subsystem-global tunables can go below the /sys/subsystem/<name>/ directory, without clashing with the list of devices or anything else.

Makes sense.

Quoted 11 lines

Quoted 3 lines

I don't know. Is there guidelines for sysfs (and procfs for that matter)? Is anyone maintaining it (not the infrastructure, but the actual content)?

Unfortunately, there was never really a guideline.

Quoted 4 lines

It's kind of ironic that /proc/sys/ looks like one of the best organised directories in proc, while /sys/kernel seems to be in danger of becoming a mess: it has kexec and uevent files in the base directory, rather than in subdirectories...

Quoted 6 lines

True, just looking at it now, people do crazy things like:
/sys/kernel/notes, which is a file with binary content, and a name nobody will ever be able to guess what it is good for. That should definitely go into a section/ directory. Also the VM stuff there should probably move to a /sys/vm/ directory along with the weird placed top-level /sys/slab/.

Top level directory IMO should be kept as sparse as possible. If you agree to /sys/mm for example, that's fine, but then slab should go under that. (I'd prefer all to go underneath /sys/kernel, but...).

It would be nice to get a sysfs content maintainer or two. Just having new additions occasionally reviewed along with the rest of a patch, by random people, doesn't really aid consistency. Would it be much trouble to ask that _all_ additions to sysfs be accompanied by notification to this maintainer, along with a few line description? (then merge would require SOB from said maintainer).

Quoted 11 lines · Nick Piggin

Quoted 3 lines · Kay Sievers

On 10/24/07, Nick Piggin <[email protected]> wrote:

It was intended to be something like /proc/sys/kernel/ only.

Really? So you'd be happy to have a /sys/dev /sys/fs /sys/kernel /sys/net /sys/vm etc? "kernel" to me shouldn't really imply the stuff under the kernel/ source directory or other random stuff that doesn't fit into another directory, but attributes that are directly related to the kernel software (rather than directly associated with any device).

What would you want in /sys/net and /sys/dev and /sys/vm? I don't mind putting subdirs in /sys/kernel/ if you want it.

Quoted 6 lines

It would be nice to get a sysfs content maintainer or two. Just having new additions occasionally reviewed along with the rest of a patch, by random people, doesn't really aid consistency. Would it be much trouble to ask that _all_ additions to sysfs be accompanied by notification to this maintainer, along with a few line description? (then merge would require SOB from said maintainer).

No, I would _love_ that. We should make the requirement that all new sysfs files be documented in Documentation/API/ like that details.

I'll be glad to review it, but as it's pretty trivial to add sysfs files, everyone ends up doing it :)

Quoted 5 lines

Quoted 5 lines

Is it possible to use a single bit of common code and a single notifier for these things? Or is it too difficult?

I'm sorry, I can't understand your image well. I'd like to know details of your image.

Rather than have each of "RAS tools" have their own notifier, and have the user specify the priority of the notifiers, introduce some layer which _knows_ that, for example, only one of these subsystems will be called (it could arbitrate, perhaps distinguish between destructive and non-destructive ones). It would need only a single notifier, but would then have a specific way of calling into the ras modules.

Does this make sense? I guess it is a lot more work to do, so maybe your solution is the best one for now.