Re: [PATCH 0/3] add new notifier function ,take4

[Eric W. Biederman]

Takenori Nagano <t-nagano@xxxxxxxxxxxxx> writes:

> Hi,
>
> changelog take3 -> take4
>
> - Rebased 2.6.25-mm1
> - Add a document
> - Add kdump on panic_notifier
>
> 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 control
> files.
>
> And, third patch moves crash_kexec() to panic_notifier. It helps us to do
> something before taking a crash dump. It's useful for some RAS tools developer.
> If you want to use it, you have to set config option DUMP_ON_PANIC_NOTIFIER to
> Y. Default value of DUMP_ON_PANIC_NOTIFIER is N.
> If you set DUMP_ON_PANIC_NOTIFIER to N, kdump has no difference before.

NAK.

Reasonable alternatives have been suggested.  No rebuttal has been given.
Just buggy patches with insufficient description of what you are doing
and why.

I am tired of seeing the same patch come up again and again without even
all of the easy problems that are pointed out addressed, much less the
design issues considered or addressed.

I see no evidence that we need this mechanism to achieve any of
the goals proposed.

This mechanism drastically reduces the maintainability of the
kexec on panic code as it makes the code path indiscoverable
and thus unreviewable.

This mechanism gives an unnecessary and confusing policy control
to users when we should be able to auto-tune based on the situation.

CONFIGURABILITY IS BAD in this context.

I think this entire approach is a BAD IDEA.  Please go back to
the drawing board.  Please describe the specific problems you
are trying to solve and why the existing mechanisms can not be
made to work and we can work with you.

Eric
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/