Messages in this thread | | | Subject | Re: [PATCH 04/16] GFS2: Daemons and address space operations | From | Steven Whitehouse <> | Date | Tue, 05 Sep 2006 11:04:47 +0100 |
| |
Hi,
On Mon, 2006-09-04 at 16:36 +0200, Jan Engelhardt wrote: > >> >+ offset += (2*sizeof(__be64) - 1); > >> > >> >+#ifndef __LOPS_DOT_H__ > >> >+#define __LOPS_DOT_H__ > >> > >> +struct gfs2_log_operations; > >> > >> Making sure every .h file would "compile" on its own, this also means #include > >> <linux/list.h> for the below, f.ex.. > >> > >Is this really a requirement? I suspect there are a fair few exception > >to this over the kernel code. > > A requirement - not yet. I could not find my own post about it, but this > one is a similar one two years earlier http://lkml.org/lkml/2004/6/15/90 > Ok. I've had a go at that: http://www.kernel.org/git/?p=linux/kernel/git/steve/gfs2-2.6.git;a=commitdiff;h=f2f7ba5237e2fe10ba3e328a4f728b9e1ff141da
> >> Maybe there should be at least one humna person listen in AUTHOR. > >> > >Ok, I'll get back to you on that one :-) > > Should have been "human" of course. > Yes, I'd realised that, its a question of which one to put in. Even though I've been working on this for almost a year now, its still true to say that Ken Preslan's code is more numerous than mine. So I'm not sure that I should claim authorship for myself, on the other hand, if this is a statement of where bug fixes should be sent, then I'm probably as good a choice as any. There are of course a lot of other contributors both from within Red Hat, and particularly since the review process started, from outside Red Hat too.
> >Are you saying that they should all end in a , or that they should not, > >or even just that it should be consistent? > > There seems to be no explicit CodingStyle rule at this point, so you are > free to choose either. Just be consistent (like with the goto labels). > Ok, now fixed in: http://www.kernel.org/git/?p=linux/kernel/git/steve/gfs2-2.6.git;a=commitdiff;h=ea67eedb211d3418fa62fe3477e0d19b2888225e
> >> >+++ b/fs/gfs2/ops_address.c > >> >+ if (likely(file != &gfs2_internal_file_sentinal)) { > >> > >> The thing is usually called "sentinel". Alan might prove me wrong that both > >> spelling variants are possible :-) > >> > >I think you are right, so I've changed it. > > http://dictionary.reference.com/search?q=sentinal&x=0&y=0 > W.W.W.W.W. > Yes, and my (rather small) treeware dictionary says likewise - its fixed now anyway.
> >> >+static void stuck_releasepage(struct buffer_head *bh) > >> >+{ > >> >+static unsigned limit = 0; > >> > >> Is this really ok to have? > >> > >I think so. I don't really care about the odd race here. All I want to > >do is ensure that in the (very unlikely, I hope) situation of this > >function being called, we don't land up generating huge amounts of > >debugging information. Usually only the first message will have the > > There is printk_ratelimit() and SUBSYSTEM_ratelimit(). > > >useful information in it, so this was just to ensure that we are not > >flooded. I have made a slight change to it though. Let me know if you'd > >like some further changes in this area. > > > Jan Engelhardt
Hmm. I'm not sure that this would really be the right thing... what I want is to limit the maximum number of times that this is triggered rather than limiting the rate at which its triggered. I think thats a subtle difference from the ratelimit functions,
Steve.
- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |