From mboxrd@z Thu Jan 1 00:00:00 1970 From: Suresh Jayaraman Subject: Re: [PATCH] cifs: trivial: cleanup fscache cFYI and cERROR messages Date: Mon, 20 Jun 2011 12:03:55 +0530 Message-ID: <4DFEE9D3.8090305@suse.de> References: <4DF7804B.8000502@suse.de> <20110614120048.7c04567d@corrin.poochiereds.net> <4DF87E1D.70802@suse.de> <20110615085841.633bb3c3@corrin.poochiereds.net> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Cc: Steve French , linux-cifs To: Jeff Layton Return-path: In-Reply-To: <20110615085841.633bb3c3-4QP7MXygkU+dMjc06nkz3ljfA9RmPOcC@public.gmane.org> Sender: linux-cifs-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org List-ID: On 06/15/2011 06:28 PM, Jeff Layton wrote: > On Wed, 15 Jun 2011 15:10:45 +0530 > Suresh Jayaraman wrote: > >> On 06/14/2011 09:30 PM, Jeff Layton wrote: >>> On Tue, 14 Jun 2011 21:07:47 +0530 >>> Suresh Jayaraman wrote: >>> >>>> ... for uniformity and cleaner debug logs. >>>> >>>> Signed-off-by: Suresh Jayaraman >>>> --- >>>> fs/cifs/cache.c | 6 +++--- >>>> fs/cifs/fscache.c | 53 +++++++++++++++++++++++++---------------------------- >>>> 2 files changed, 28 insertions(+), 31 deletions(-) >>>> >>>> diff --git a/fs/cifs/cache.c b/fs/cifs/cache.c >>>> index dd8584d..545509c 100644 >>>> --- a/fs/cifs/cache.c >>>> +++ b/fs/cifs/cache.c >>>> @@ -92,7 +92,7 @@ static uint16_t cifs_server_get_key(const void *cookie_netfs_data, >>>> break; >>>> >>>> default: >>>> - cERROR(1, "CIFS: Unknown network family '%d'", sa->sa_family); >>>> + cERROR(1, "Unknown network family '%d'", sa->sa_family); >>> ^^^^^^^^^ >>> Maybe this would be a good time to add in a new >>> cFYI/cERROR "flag" for fscache and convert all of these >>> to use it? >> >> Sounds like a good idea to flag fsc debug messages separately but >> flagging errors separately would be useful? >> > > Good point. Now that you mention it, I'm not sure what purpose the > first argument to cERROR serves. Might be a good thing to do a global > search and replace -- "s/cERROR(1, /cERROR(/" and fix up the macro. > >> Also, I don't understand the idea behing the "set" currently. >> >> we have >> >> #define cFYI(set, fmt, arg...) \ >> do { \ >> if (set) \ >> cifsfyi(fmt, ##arg); \ >> } while (0) >> >> and we call cFYI with always pass like this: >> cFYI(1, ".."); >> >> > > I think for cFYI, the idea was to have a bitmask that would allow you > to selectively turn on certain classes of debug messages (similar to > how NFS' dfprintk macro works). In practice though, it's rarely set to > anything but "1". Maybe we should consider eliminating that argument as > well? > Currently the separate debug flags CIFS_RC and CIFS_TIMER doesn't look justified. It would be a good idea to have different classes of debug messages based on the functionality. The ones that comes to my mind are DFS, Kerberos/Spnego, FS-Cache and Xattr/ACL etc. Of course we need to see how GetXid()/FreeXid() that spans across can be handled. Would it be worth the exercise or we would be better of with a single debug flag that is used always? Ideas/Thoughts/Comments? -- Suresh Jayaraman