[PATCH 03/25] Unionfs: display informational messages only if debug is on
This is to avoid filling the console/logs with messages that are primarily of debugging use. Signed-off-by: Erez Zadok <[EMAIL PROTECTED]> Acked-by: Josef 'Jeff' Sipek <[EMAIL PROTECTED]> --- fs/unionfs/commonfops.c |4 ++-- fs/unionfs/dentry.c |6 +++--- fs/unionfs/union.h |4 3 files changed, 9 insertions(+), 5 deletions(-) diff --git a/fs/unionfs/commonfops.c b/fs/unionfs/commonfops.c index 87cbb09..e69ccf6 100644 --- a/fs/unionfs/commonfops.c +++ b/fs/unionfs/commonfops.c @@ -394,8 +394,8 @@ int unionfs_file_revalidate(struct file *file, bool willwrite) if (willwrite && IS_WRITE_FLAG(file->f_flags) && !IS_WRITE_FLAG(unionfs_lower_file(file)->f_flags) && is_robranch(dentry)) { - printk(KERN_DEBUG "unionfs: do delay copyup of \"%s\"\n", - dentry->d_name.name); + dprintk(KERN_DEBUG "unionfs: do delay copyup of \"%s\"\n", + dentry->d_name.name); err = do_delayed_copyup(file); } diff --git a/fs/unionfs/dentry.c b/fs/unionfs/dentry.c index 9e0742d..08b5722 100644 --- a/fs/unionfs/dentry.c +++ b/fs/unionfs/dentry.c @@ -46,9 +46,9 @@ static bool __unionfs_d_revalidate_one(struct dentry *dentry, /* if the dentry is unhashed, do NOT revalidate */ if (d_deleted(dentry)) { - printk(KERN_DEBUG "unionfs: unhashed dentry being " - "revalidated: %*s\n", - dentry->d_name.len, dentry->d_name.name); + dprintk(KERN_DEBUG "unionfs: unhashed dentry being " + "revalidated: %*s\n", + dentry->d_name.len, dentry->d_name.name); goto out; } diff --git a/fs/unionfs/union.h b/fs/unionfs/union.h index 140b8ae..5e9843b 100644 --- a/fs/unionfs/union.h +++ b/fs/unionfs/union.h @@ -507,6 +507,8 @@ static inline void unionfs_mntput(struct dentry *dentry, int bindex) #ifdef CONFIG_UNION_FS_DEBUG +#define dprintk(args...) printk(args) + /* useful for tracking code reachability */ #define UDBG printk("DBG:%s:%s:%d\n",__FILE__,__FUNCTION__,__LINE__) @@ -543,6 +545,8 @@ extern void __show_inode_counts(const struct inode *inode, #else /* not CONFIG_UNION_FS_DEBUG */ +#define dprintk(args...) do { } while (0) + /* we leave useful hooks for these check functions throughout the code */ #define unionfs_check_inode(i) do { } while(0) #define unionfs_check_dentry(d)do { } while(0) -- 1.5.2.2 - To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 03/25] Unionfs: display informational messages only if debug is on
On Sep 25 2007 23:09, Erez Zadok wrote: >--- a/fs/unionfs/commonfops.c >+++ b/fs/unionfs/commonfops.c >@@ -394,8 +394,8 @@ int unionfs_file_revalidate(struct file *file, bool >willwrite) > if (willwrite && IS_WRITE_FLAG(file->f_flags) && > !IS_WRITE_FLAG(unionfs_lower_file(file)->f_flags) && > is_robranch(dentry)) { >- printk(KERN_DEBUG "unionfs: do delay copyup of \"%s\"\n", >- dentry->d_name.name); >+ dprintk(KERN_DEBUG "unionfs: do delay copyup of \"%s\"\n", >+ dentry->d_name.name); Don't we have pr_debug() for that? - To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 03/25] Unionfs: display informational messages only if debug is on
In message <[EMAIL PROTECTED]>, Jan Engelhardt writes: > > On Sep 25 2007 23:09, Erez Zadok wrote: > >--- a/fs/unionfs/commonfops.c > >+++ b/fs/unionfs/commonfops.c > >@@ -394,8 +394,8 @@ int unionfs_file_revalidate(struct file *file, bool > >willwrite) > > if (willwrite && IS_WRITE_FLAG(file->f_flags) && > > !IS_WRITE_FLAG(unionfs_lower_file(file)->f_flags) && > > is_robranch(dentry)) { > >-printk(KERN_DEBUG "unionfs: do delay copyup of \"%s\"\n", > >- dentry->d_name.name); > >+dprintk(KERN_DEBUG "unionfs: do delay copyup of \"%s\"\n", > >+dentry->d_name.name); > > Don't we have pr_debug() for that? Jan, what's the policy on people writing their own debugging printk systems. I've looked at what other file systems do, and found out that it varies a lot: some use a simple mapping wrapper from printk->dprintk, while others define complex ways to have debugging levels/bitmaps where users can selectively decide (say, by a mount option), what debugging output they'd like to see or not. Here's a small sampling of what some file systems use: 9p/fid.c: P9_DPRINTK(P9_DEBUG_VFS, "fid %d dentry %s\n", autofs/inode.c: DPRINTK(("autofs: shutting down\n")); lockd/svcproc.c:dprintk("lockd: GRANTED_RES called\n"); ncpfs/dir.c:DDPRINTK("ncp_lookup_validate: result=%d\n", val); nfs/client.c: dprintk("<-- nfs_free_client()\n"); nfsd/export.c: dprintk("found domain %s\n", buf); partitions/efi.c: Dprintk("GPT my_lba incorrect: %lld != %lld\n", Not much standardization there :-) but "dprintk" does appear to be the more common use. I wanted something simple to allow me to not printk something that's just for informational/debugging purposes, but w/o having to #ifdef the printk in question, or define a complex debugging-level printk system. Now, looking at pr_debug (in linux/kernel.h), and indeed some filesystems (e.g., affs and configfs) use it. But pr_debug is only active when #define DEBUG is on (not CONFIG_DEBUG). I didn't see a config option that enable DEBUG: is there one? >From other file systems, it appears that the more common convention is for filesystem XYZ to offer a config option CONFIG_DEBUG_XYZ, which makes its debugging more self-contained and keeps the config option closer to where the f/s itself is enabled. Thanks, Erez. - To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 03/25] Unionfs: display informational messages only if debug is on
On Sep 26 2007 10:01, Erez Zadok wrote: >> >> On Sep 25 2007 23:09, Erez Zadok wrote: >> >--- a/fs/unionfs/commonfops.c >> >+++ b/fs/unionfs/commonfops.c >> >@@ -394,8 +394,8 @@ int unionfs_file_revalidate(struct file *file, bool >> >willwrite) >> >if (willwrite && IS_WRITE_FLAG(file->f_flags) && >> >!IS_WRITE_FLAG(unionfs_lower_file(file)->f_flags) && >> >is_robranch(dentry)) { >> >- printk(KERN_DEBUG "unionfs: do delay copyup of \"%s\"\n", >> >- dentry->d_name.name); >> >+ dprintk(KERN_DEBUG "unionfs: do delay copyup of \"%s\"\n", >> >+ dentry->d_name.name); >> >> Don't we have pr_debug() for that? > >Jan, what's the policy on people writing their own debugging printk systems. Generally, the rule is "don't (re)invent another debug system" >I've looked at what other file systems do, and found out that it varies a lot: Oh that's either old code or code that has not been reviewed properly. Fact is that I have been made aware of pr_debug() "often enough" on netfilter-devel, and it looks like a good idea. >I wanted something simple to allow me to not printk something that's just >for informational/debugging purposes, but w/o having to #ifdef the printk in >question, or define a complex debugging-level printk system. Surprise, pr_debug() is just that all nicely wrapped up. Want debug? Do it like this. #ifdef CONFIG_UNIONFS_DEBUG # define DEBUG 1 #endif and pr_debug() works magic. >Now, looking at pr_debug (in linux/kernel.h), and indeed some filesystems >(e.g., affs and configfs) use it. But pr_debug is only active when #define >DEBUG is on (not CONFIG_DEBUG). I didn't see a config option that enable >DEBUG: is there one? No I do not think so. But when you grep for dprintk, you should also grep for DEBUG, to be fair :p - To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 03/25] Unionfs: display informational messages only if debug is on
In message <[EMAIL PROTECTED]>, Jan Engelhardt writes: [...] > Surprise, pr_debug() is just that all nicely wrapped up. > Want debug? Do it like this. > > #ifdef CONFIG_UNIONFS_DEBUG > # define DEBUG 1 > #endif > > and pr_debug() works magic. [...] Sounds good. I'll do that. Thanks, Erez. - To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html