On Sun, 16 Mar 2014 16:27:28 +0000 Anton Altaparmakov <[email protected]> wrote:
> Hi, > > On 16 Mar 2014, at 16:12, Joe Perches <[email protected]> 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 <[email protected]> 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! Hi Anton, AFAICS It works with CONFIG_NTFS_DEBUG=y and /proc/sys/fs/ntfs-debug=1. Current debug behaviour stays the same as __debug_ntfs is already under #ifdef DEBUG but I'm applying some advices by Joe before sending a new version. Best regards, Fabian > > 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 [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
