On Tue, Jan 31, 2023 at 07:56:29PM +0900, Hyunmin Lee wrote: > On Mon, Jan 30, 2023 at 12:14:04PM +0200, Mike Rapoport wrote: > > Hi, > > > > On Fri, Jan 27, 2023 at 08:58:44PM +0900, Hyunmin Lee wrote: > > > Replace unnacessary BUG_ON to WARN_ON. These BUG_ONs verify aruguments of a function. Thus, the WARN_ONs return an EINVAL error when their condition is true. > > > > Some users enable panic_on_warn, so for them WARN_ON will still crash a > > machine. > > > > I think a simple if() will be sufficient. > > > Hi Mike, > > Thank you for your advice. > Would you please give feedback about the below opinion? > - Printing warning messages is helpful to inform what happened in the system to the users. > - When a simple if() is used instead of WARN_ON, the if() should print a warning message. > - The condition of the simple if() should also have unlikely() for optimization of system performance. > - WARN_ON is a macro doing like thoes easily. It has a notifying function and unlikely optimization. > - Eventhough WARN_ON will still crash like BUG_ON by some users who enable panic_on_warn, it is their intention. They should accept the crash by WARN_ON. > - Therefore, using WARN_ON looks like natural and efficient. As this is a validation of the function parameters, there is no need in warning messages and if(unlikely()) will do. There is really no point in WARN_ON() for something that's totally recoverable and very unlikely to happen. > Best, > Hyunmin -- Sincerely yours, Mike.
