Hi,
On 16 Mar 2014, at 16:12, Joe Perches <j...@perches.com> wrote:
> On Sun, 2014-03-16 at 15:53 +0000, Anton Altaparmakov wrote:
>> Hi,
>>
>> Looks good, thanks. You can add my Acked-by if you like. Can I assume you
>> have test it builds?
>>
>> Andrew, can you please merge this via your patch series? Thanks!
>
> I think a few things need fixing first
Yes, quite right - I had assume that had been tested!
> On 16 Mar 2014, at 12:27, Fabian Frederick <f...@skynet.be> wrote:
>>
>>> -All printk(KERN_foo converted to pr_foo().
>>> -Add pr_fmt and remove redundant prefixes.
>
> I'm not sure there are "redundant prefixes"
>
> This changes prefixes from "NTFS-fs" to "ntfs"
> and should be at a minimum mentioned in the
> changelog.
As long as it says ntfs in the string so dmesg output can be "grep -i ntfs"-ed
I am not fussed.
> The va_end location needs moving.
Yes, well spotted, thanks.
> Converting printk(KERN_DEBUG -> pr_debug will
> not always emit that message now. Now, only if
> DEBUG is #defined or dynamic_debugging is enabled
> for the build _and_ the message is specifically
> enabled will the message be output.
>
> So, the debugging output has been silenced by default.
>
> That's not necessarily good.
No that is not good at all. I didn't know about those pr_debug semantics so
thank you for pointing them out. That needs fixing otherwise we might as well
not have the messages at all... Being able to enable all ntfs debug messages
using a insmod option or via /proc as is at the moment is a very valuable
debugging too and I have scripts that use this so am not keen on this being
changed at all.
Fabian, can you please fix the issues pointed out by Joe and also please make
sure you actually test it!
Thanks a lot in advance!
Best regards,
Anton
> diff --git a/fs/ntfs/debug.c b/fs/ntfs/debug.c
> []
>>> @@ -59,17 +53,15 @@ void __ntfs_warning(const char *function, const struct
>>> super_block *sb,
>>> #endif
>>> if (function)
>>> flen = strlen(function);
>>> - spin_lock(&err_buf_lock);
>>> va_start(args, fmt);
>>> - vsnprintf(err_buf, sizeof(err_buf), fmt, args);
>>> + vaf.fmt = fmt;
>>> + vaf.va = &args;
>
>>> va_end(args);
>
> This va_end should be moved after the pr_warns.
>
>>> if (sb)
>>> - printk(KERN_ERR "NTFS-fs warning (device %s): %s(): %s\n",
>>> - sb->s_id, flen ? function : "", err_buf);
>>> + pr_warn("(device %s): %s(): %pV\n",
>>> + sb->s_id, flen ? function : "", &vaf);
>>> else
>>> - printk(KERN_ERR "NTFS-fs warning: %s(): %s\n",
>>> - flen ? function : "", err_buf);
>>> - spin_unlock(&err_buf_lock);
>>> + pr_warn("%s(): %pV\n", flen ? function : "", &vaf);
>>> }
>>>
>>> /**
>>> @@ -94,6 +86,7 @@ void __ntfs_warning(const char *function, const struct
>>> super_block *sb,
>>> void __ntfs_error(const char *function, const struct super_block *sb,
>>> const char *fmt, ...)
>>> {
>>> + struct va_format vaf;
>>> va_list args;
>>> int flen = 0;
>>>
>>> @@ -103,17 +96,15 @@ void __ntfs_error(const char *function, const struct
>>> super_block *sb,
>>> #endif
>>> if (function)
>>> flen = strlen(function);
>>> - spin_lock(&err_buf_lock);
>>> va_start(args, fmt);
>>> - vsnprintf(err_buf, sizeof(err_buf), fmt, args);
>>> + vaf.fmt = fmt;
>>> + vaf.va = &args;
>>> va_end(args);
>
> Here too
>
>>> if (sb)
>>> - printk(KERN_ERR "NTFS-fs error (device %s): %s(): %s\n",
>>> - sb->s_id, flen ? function : "", err_buf);
>>> + pr_err("(device %s): %s(): %pV\n",
>>> + sb->s_id, flen ? function : "", &vaf);
>>> else
>>> - printk(KERN_ERR "NTFS-fs error: %s(): %s\n",
>>> - flen ? function : "", err_buf);
>>> - spin_unlock(&err_buf_lock);
>>> + pr_err("%s(): %pV\n", flen ? function : "", &vaf);
--
Anton Altaparmakov <aia21 at cam.ac.uk> (replace at with @)
Unix Support, Computing Service, University of Cambridge
J.J. Thomson Avenue, Cambridge, CB3 0RB, UK
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
