On 2010-10-27 23:32, Trond Myklebust wrote: > On Wed, 2010-10-27 at 23:29 +0200, Benny Halevy wrote: >> On 2010-10-27 23:23, Trond Myklebust wrote: >>> On Wed, 2010-10-27 at 23:00 +0200, Benny Halevy wrote: >>>> On 2010-10-27 22:17, Fred Isaman wrote: >>>>> On Wed, Oct 27, 2010 at 4:06 PM, Benny Halevy <bhalevy@xxxxxxxxxxx> wrote: >>>>>> On 2010-10-27 21:49, Fred Isaman wrote: >>>>>>> The change to printk was in response to Trond's complaint about >>>>>>> successive dprintks. >>>>>>> >>>>>>> Instead, the following would work: >>>>>>> >>>>>>> >>>>>>> diff --git a/fs/nfs/nfs4filelayout.c b/fs/nfs/nfs4filelayout.c >>>>>>> index 5f52e6f..2ce393c 100644 >>>>>>> --- a/fs/nfs/nfs4filelayout.c >>>>>>> +++ b/fs/nfs/nfs4filelayout.c >>>>>>> @@ -585,7 +585,8 @@ filelayout_commit(struct nfs_write_data *data, int sync) >>>>>>> } >>>>>> >>>>>> If we're going this way, the ifdebug could cover the following >>>>>> printout as well... >>>>>> >>>>> >>>>> Did you mean preceding printout? By the way - the complaint about >>>> >>>> Yeah, preceding the call to print_ds (following my comment :) >>>> >>>>> successive dprintks was regarding >>>>> print_ds_list repeatedly calling print_ds, which at the time used dprintk. >>>> >>>> Why do we care to optimize the debug case so much? >>>> print_ds_list is already calling print_ds inside ifdebug(FACILITY) >>>> so the common, non-debug case is optimized correctly. I.e. we don't >>>> repeatedly check the debug flag normally. >>> >>> It's not about optimizing the debug case. It's about avoiding having to >>> check ifdebug(FACILITY) all the time when we're _not_ debugging. >> >> Right, and so we do, as the whole loop in print_ds_list is enclosed >> in ifdebug(FACILITY). > > In that case, I agree: the whole thing can be converted to use ordinary > printks... The point is it has a singular caller outside of this loop for which the caller needs to check ifdebug(FACILITY) before calling print_ds. Not a big deal, but I think it'd be cleaner if print_ds will do a dprintk to simplify its singular usage, while the cost of that will be extra tests when called, possibly many times, in debug mode from print_ds_list. Benny > > Trond > -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
