2006-03-03 01:48:36

by Dave Peterson

[permalink] [raw]
Subject: [PATCH 10/15] EDAC: edac_mc_add_mc() fix [1/2]

This is part 1 of a 2-part patch set. The code changes are split into
two parts to make the patches more readable.

Move complete_mc_list_del() and del_mc_from_global_list() so we can
call del_mc_from_global_list() from edac_mc_add_mc() without forward
declarations. Perhaps using forward declarations would be better?
I'm doing things this way because the rest of the code is missing
them.

Signed-Off-By: David S. Peterson <[email protected]> <[email protected]>
---

Index: linux-2.6.16-rc5-edac/drivers/edac/edac_mc.c
===================================================================
--- linux-2.6.16-rc5-edac.orig/drivers/edac/edac_mc.c 2006-02-27 17:05:48.000000000 -0800
+++ linux-2.6.16-rc5-edac/drivers/edac/edac_mc.c 2006-02-27 17:06:17.000000000 -0800
@@ -1405,6 +1405,24 @@ static int add_mc_to_global_list (struct
}


+static void complete_mc_list_del (struct rcu_head *head)
+{
+ struct mem_ctl_info *mci;
+
+ mci = container_of(head, struct mem_ctl_info, rcu);
+ INIT_LIST_HEAD(&mci->link);
+ complete(&mci->complete);
+}
+
+
+static void del_mc_from_global_list (struct mem_ctl_info *mci)
+{
+ list_del_rcu(&mci->link);
+ init_completion(&mci->complete);
+ call_rcu(&mci->rcu, complete_mc_list_del);
+ wait_for_completion(&mci->complete);
+}
+

EXPORT_SYMBOL(edac_mc_add_mc);

@@ -1466,24 +1484,6 @@ finish:
}


-
-static void complete_mc_list_del (struct rcu_head *head)
-{
- struct mem_ctl_info *mci;
-
- mci = container_of(head, struct mem_ctl_info, rcu);
- INIT_LIST_HEAD(&mci->link);
- complete(&mci->complete);
-}
-
-static void del_mc_from_global_list (struct mem_ctl_info *mci)
-{
- list_del_rcu(&mci->link);
- init_completion(&mci->complete);
- call_rcu(&mci->rcu, complete_mc_list_del);
- wait_for_completion(&mci->complete);
-}
-
EXPORT_SYMBOL(edac_mc_del_mc);

/**


2006-03-03 02:33:10

by Andrew Morton

[permalink] [raw]
Subject: Re: [PATCH 10/15] EDAC: edac_mc_add_mc() fix [1/2]

Dave Peterson <[email protected]> wrote:
>
> This is part 1 of a 2-part patch set. The code changes are split into
> two parts to make the patches more readable.

Will the code compile and run with just #1-of-2 applied?

If not, we should combine the patches (which I can do in a jiffy). Because
hitting a won't-compile in the middle of a git-bisect session is quite
painful.

Similarly we should aim for compiles-and-works at each step of the whole
series, if possible/sane.

> Move complete_mc_list_del() and del_mc_from_global_list() so we can
> call del_mc_from_global_list() from edac_mc_add_mc() without forward
> declarations. Perhaps using forward declarations would be better?
> I'm doing things this way because the rest of the code is missing
> them.

Well I prefer it the way you've done it in this patch. But my first
language was Pascal ;) (yes, they had computers then)

2006-03-03 19:03:24

by Dave Peterson

[permalink] [raw]
Subject: Re: [PATCH 10/15] EDAC: edac_mc_add_mc() fix [1/2]

On Thursday 02 March 2006 18:31, Andrew Morton wrote:
> Dave Peterson <[email protected]> wrote:
> > This is part 1 of a 2-part patch set. The code changes are split into
> > two parts to make the patches more readable.
>
> Will the code compile and run with just #1-of-2 applied?

It should compile. Assuming that it does, would it still have been
preferable to just combine the two into a single patch?

2006-03-03 20:49:33

by Andrew Morton

[permalink] [raw]
Subject: Re: [PATCH 10/15] EDAC: edac_mc_add_mc() fix [1/2]

Dave Peterson <[email protected]> wrote:
>
> On Thursday 02 March 2006 18:31, Andrew Morton wrote:
> > Dave Peterson <[email protected]> wrote:
> > > This is part 1 of a 2-part patch set. The code changes are split into
> > > two parts to make the patches more readable.
> >
> > Will the code compile and run with just #1-of-2 applied?
>
> It should compile. Assuming that it does, would it still have been
> preferable to just combine the two into a single patch?

It's better as you had it. First patch moves the functions without
changing them, the second patch changes them. The mantra is "one concept
per patch".