On Tue, Jan 21, 2025 at 10:48:08AM +0900, Hyeonggon Yoo wrote: > > > On 1/21/2025 9:54 AM, Hyesoo Yu wrote: > > On Tue, Jan 21, 2025 at 12:41:01AM +0900, Hyeonggon Yoo wrote: > > > On Mon, Jan 20, 2025 at 5:30 PM Hyesoo Yu <hyesoo.yu@xxxxxxxxxxx> wrote: > > > > > > > > If a slab object is corrupted or an error occurs in its internal > > > > value, continuing after restoration may cause other side effects. > > > > At this point, it is difficult to debug because the problem occurred > > > > in the past. A flag has been added that can cause a panic when there > > > > is a problem with the object. > > > > > > Hi Hyesoo, > > > > > > I'm concerned about this because it goes against the effort to avoid > > > introducing new BUG() calls [1]. > > > > > > And I think it would be more appropriate to use existing panic_on_warn > > > functionality [2] which causes > > > a panic on WARN(), rather than introducing a SLUB-specific knob to do > > > the same thing. > > > > > > However SLUB does not call WARN() and uses pr_err() instead when > > > reporting an error. > > > Vlastimil and I talked about changing it to use WARN() a while ago > > > [3], but neither of us > > > have done that yet. > > > > > > Probably you may want to look at it, as it also aligns with your purpose? > > > FYI, if you would like to work on it, please make sure that it WARN() > > > is suppressed during kunit test. > > > > > > [1] https://docs.kernel.org/process/deprecated.html#bug-and-bug-on > > > [2] https://www.kernel.org/doc/html/v6.9/admin-guide/sysctl/kernel.html#panic-on-warn > > > [3] https://lore.kernel.org/linux-mm/d4219cd9-32d3-4697-93b9-6a44bf77d50c@xxxxxxx > > > > > > Best, > > > Hyeonggon > > > > Thanks for response. > > > > Using warn() instead of panic, is a great idea. > > Thanks for pointing out what I missed. > > Just for clarification, I think changing the common error reporting > logic (like, slab_bug()) to use WARN() will be preferable to inserting > new WARN()s at random points, which is what this patch does now. > > Best, > Hyeonggon > Thanks you for clarification. Actually, I considered adding BUG_ON() to slab_bug. However if we add BUG_ON() to slab_bug, it will prevent many meaningful error log from being printed subsequently. As you know, slab_bug is the log that usually is printed at the biginning of a bug in the slab. As a result, it would be difficult to figure out the problems based on the logs during our large-scale test-bed. Similary, even if I use WARN() in slab_bug, we won't be able to obtain the logs when panic_on_warn is enabled. I don't think it is useful to include WARN in slab_bug. Instead, I will implement a solution where WARN is only used in slab_fix before slab object is restored. If I add it to slab_fix, I think warning is suppressed on kunit test by slab_add_kunit_errors handling. Thanks, Regards.
