On Fri, Aug 3, 2018 at 6:17 PM, Tony Battersby <tonyb@xxxxxxxxxxxxxxx> wrote:
> On 08/03/2018 09:41 AM, Tony Battersby wrote:
>> On 08/03/2018 04:56 AM, Andy Shevchenko wrote:
>>> On Thu, Aug 2, 2018 at 10:57 PM, Tony Battersby <tonyb@xxxxxxxxxxxxxxx> wrote:
>>>> Remove code duplication in error messages. It is now safe to pas a NULL
>>>> dev to dev_err(), so the checks to avoid doing so are no longer
>>>> necessary.
>>>>
>>>> Example:
>>>>
>>>> Error message with dev != NULL:
>>>> mpt3sas 0000:02:00.0: dma_pool_destroy chain pool, (____ptrval____) busy
>>>>
>>>> Same error message with dev == NULL before patch:
>>>> dma_pool_destroy chain pool, (____ptrval____) busy
>>>>
>>>> Same error message with dev == NULL after patch:
>>>> (NULL device *): dma_pool_destroy chain pool, (____ptrval____) busy
>>> Have you checked a history of this?
>>>
>>> I'm pretty sure this was created in an order to avoid bad looking (and
>>> in some cases frightening) "NULL device *" part.
>>>
>>> If it it's the case, I would rather leave it as is, and even not the
>>> case, I'm slightly more bent to the current state.
>>>
>> I did. "drivers/base/dmapool.c", later moved to "mm/dmapool.c", was
>> added in linux-2.6.3, for which dev_err() did not work will a NULL dev,
>> so the check was necessary back then. I agree that the (NULL device *):
>> bit is ugly, but these messages should be printed only after a kernel
>> bug, so it is not like they will be making a regular appearance in
>> dmesg. Considering that, I think that it is better to keep it simple.
>>
>
> My original unsubmitted patch used the following:
>
> +#define pool_err(pool, fmt, args...) \
> + do { \
> + if ((pool)->dev) \
> + dev_err((pool)->dev, fmt, args); \
> + else \
> + pr_err(fmt, args); \
> + } while (0)
>
> But then I decided to simplify it to just use dev_err(). I still have
> the old version. When I submit v3 of the patchset, which would you prefer?
JFYI: git log --no-merges --grep 'NULL device \*'
P.S. I already shared my opinion on this anyway.
--
With Best Regards,
Andy Shevchenko
