Re: Preempt & Xfs Question

2005-01-27 Thread Nathan Scott
On Thu, Jan 27, 2005 at 08:51:45AM -0800, Chris Wedgwood wrote:
> On Thu, Jan 27, 2005 at 06:24:13PM +, Matthias-Christian Ott wrote:
> > I'll submit it to the mailinglist as a seperate patch, so Linus can
> > apply it to the current Kernel.

Chris' fix for this is in Linus' mail, queued to be merged hopefully RSN.
(*prod prod*)

> XFS patches/suggestions should go to linux-xfs@oss.sgi.com and the
> maintainers there can comment as needed.

cheers.

-- 
Nathan
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: Preempt & Xfs Question

2005-01-27 Thread Matthias-Christian Ott
Chris Wedgwood wrote:
On Thu, Jan 27, 2005 at 06:24:13PM +, Matthias-Christian Ott wrote:
 

Well calling such a internal function (__function) is not a cleaning
coding style but works best :-) .
   

__foo does NOT mean it's an internal function necessarily or that it's
unclean to use it (sadly Linux has pretty vague (nothing consistent)
rules on how to interpret __foo versus foo really.
 

Combined with the current_cpu() fixes I mentioned, it looks like
this:
   

(1) your patch is mangled/wrapped
(2) this is fixed in CVS already (for maybe a week or so now?)
 

I'll submit it to the mailinglist as a seperate patch, so Linus can
apply it to the current Kernel.
   

XFS patches/suggestions should go to linux-xfs@oss.sgi.com and the
maintainers there can comment as needed.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
 

Hi!
I didn't know about the xfs development status. But it's good to hear 
that it's already fixed. When will it be included to the Kernel?

Matthias-Christian Ott
--
http://unixforge.org/~matthias-christian-ott/
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: Preempt & Xfs Question

2005-01-27 Thread Chris Wedgwood
On Thu, Jan 27, 2005 at 06:24:13PM +, Matthias-Christian Ott wrote:

> Well calling such a internal function (__function) is not a cleaning
> coding style but works best :-) .

__foo does NOT mean it's an internal function necessarily or that it's
unclean to use it (sadly Linux has pretty vague (nothing consistent)
rules on how to interpret __foo versus foo really.

> Combined with the current_cpu() fixes I mentioned, it looks like
> this:

(1) your patch is mangled/wrapped

(2) this is fixed in CVS already (for maybe a week or so now?)

> I'll submit it to the mailinglist as a seperate patch, so Linus can
> apply it to the current Kernel.

XFS patches/suggestions should go to linux-xfs@oss.sgi.com and the
maintainers there can comment as needed.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: Preempt & Xfs Question

2005-01-27 Thread Matthias-Christian Ott
Chris Wedgwood wrote:
On Thu, Jan 27, 2005 at 05:46:54PM +, Matthias-Christian Ott wrote:
 

How did you fix it?
   

I suggested:
= fs/xfs/linux-2.6/xfs_stats.h 1.9 vs edited =
Index: cw-current/fs/xfs/linux-2.6/xfs_stats.h
===
--- cw-current.orig/fs/xfs/linux-2.6/xfs_stats.h2005-01-17 
16:03:59.656946818 -0800
+++ cw-current/fs/xfs/linux-2.6/xfs_stats.h 2005-01-17 16:06:50.692361597 
-0800
@@ -142,9 +142,9 @@
/* We don't disable preempt, not too worried about poking the
 * wrong cpu's stat for now */
-#define XFS_STATS_INC(count)   (__get_cpu_var(xfsstats).count++)
-#define XFS_STATS_DEC(count)   (__get_cpu_var(xfsstats).count--)
-#define XFS_STATS_ADD(count, inc)  (__get_cpu_var(xfsstats).count += (inc))
+#define XFS_STATS_INC(count)   (per_cpu(xfsstats, 
__smp_processor_id()).count++)
+#define XFS_STATS_DEC(count)   (per_cpu(xfsstats, 
__smp_processor_id()).count--)
+#define XFS_STATS_ADD(count, inc)  (per_cpu(xfsstats, 
__smp_processor_id()).count += (inc))
extern void xfs_init_procfs(void);
extern void xfs_cleanup_procfs(void);
but what was checked in was a bit cleaner.
 

Well calling such a internal function (__function) is not a cleaning 
coding style but works best :-) .
Combined with the current_cpu() fixes I mentioned, it looks like this:

diff -Nru linux-2.6.11-rc2/fs/xfs/linux-2.6/xfs_linux.h 
linux-2.6.11-rc2-ott/fs/xfs/linux-2.6/xfs_linux.h
--- linux-2.6.11-rc2/fs/xfs/linux-2.6/xfs_linux.h2004-12-24 
21:35:50.0 +
+++ linux-2.6.11-rc2-ott/fs/xfs/linux-2.6/xfs_linux.h2005-01-27 
18:13:09.0 +
@@ -144,7 +144,7 @@
#define xfs_inherit_nosymlinksxfs_params.inherit_nosym.val
#define xfs_rotorstepxfs_params.rotorstep.val

-#define current_cpu()smp_processor_id()
+#define current_cpu()__smp_processor_id()
#define current_pid()(current->pid)
#define current_fsuid(cred)(current->fsuid)
#define current_fsgid(cred)(current->fsgid)
diff -Nru linux-2.6.11-rc2/fs/xfs/linux-2.6/xfs_stats.h 
linux-2.6.11-rc2-ott/fs/xfs/linux-2.6/xfs_stats.h
--- linux-2.6.11-rc2/fs/xfs/linux-2.6/xfs_stats.h2004-12-24 
21:34:29.0 +
+++ linux-2.6.11-rc2-ott/fs/xfs/linux-2.6/xfs_stats.h2005-01-27 
18:13:44.0 +
@@ -142,9 +142,9 @@

/* We don't disable preempt, not too worried about poking the
 * wrong cpu's stat for now */
-#define XFS_STATS_INC(count)(__get_cpu_var(xfsstats).count++)
-#define XFS_STATS_DEC(count)(__get_cpu_var(xfsstats).count--)
-#define XFS_STATS_ADD(count, inc)(__get_cpu_var(xfsstats).count += 
(inc))
+#define XFS_STATS_INC(count)(per_cpu(xfsstats, 
__smp_processor_id()).count++)
+#define XFS_STATS_DEC(count)(per_cpu(xfsstats, 
__smp_processor_id()).count--)
+#define XFS_STATS_ADD(count, inc)(per_cpu(xfsstats, 
__smp_processor_id()).count += (inc))

extern void xfs_init_procfs(void);
extern void xfs_cleanup_procfs(void);
I'll submit it to the mailinglist as a seperate patch, so Linus can 
apply it to the current Kernel.

Matthias-Christian Ott
--
http://unixforge.org/~matthias-christian-ott/
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: Preempt & Xfs Question

2005-01-27 Thread Chris Wedgwood
On Thu, Jan 27, 2005 at 05:46:54PM +, Matthias-Christian Ott wrote:

> How did you fix it?

I suggested:

= fs/xfs/linux-2.6/xfs_stats.h 1.9 vs edited =
Index: cw-current/fs/xfs/linux-2.6/xfs_stats.h
===
--- cw-current.orig/fs/xfs/linux-2.6/xfs_stats.h2005-01-17 
16:03:59.656946818 -0800
+++ cw-current/fs/xfs/linux-2.6/xfs_stats.h 2005-01-17 16:06:50.692361597 
-0800
@@ -142,9 +142,9 @@
 
 /* We don't disable preempt, not too worried about poking the
  * wrong cpu's stat for now */
-#define XFS_STATS_INC(count)   (__get_cpu_var(xfsstats).count++)
-#define XFS_STATS_DEC(count)   (__get_cpu_var(xfsstats).count--)
-#define XFS_STATS_ADD(count, inc)  (__get_cpu_var(xfsstats).count += (inc))
+#define XFS_STATS_INC(count)   (per_cpu(xfsstats, 
__smp_processor_id()).count++)
+#define XFS_STATS_DEC(count)   (per_cpu(xfsstats, 
__smp_processor_id()).count--)
+#define XFS_STATS_ADD(count, inc)  (per_cpu(xfsstats, 
__smp_processor_id()).count += (inc))
 
 extern void xfs_init_procfs(void);
 extern void xfs_cleanup_procfs(void);

but what was checked in was a bit cleaner.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: Preempt & Xfs Question

2005-01-27 Thread Matthias-Christian Ott
Chris Wedgwood wrote:
BUG: using smp_processor_id() in preemptible [0001] code:
khelper/892
   

fixed in CVS, I guess it will hit mainline soon
 

How did you fix it?
Matthias-Christian Ott
--
http://unixforge.org/~matthias-christian-ott/
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: Preempt & Xfs Question

2005-01-27 Thread Matthias-Christian Ott
Steve Lord wrote:
Matthias-Christian Ott wrote:
Hi!
I have a question: Why do I get such debug messages:
BUG: using smp_processor_id() in preemptible [0001] code: 
khelper/892
caller is _pagebuf_lookup_pages+0x11b/0x362
[] smp_processor_id+0xa3/0xb4
[] _pagebuf_lookup_pages+0x11b/0x362
[] _pagebuf_lookup_pages+0x11b/0x362

.
Does the XFS Module avoid preemption rules? If so, why?

It is probably coming from these macros which keep various statistics
inside xfs as per cpu variables.
in fs//xfs/linux-2.6/xfs_stats.h:
DECLARE_PER_CPU(struct xfsstats, xfsstats);
/* We don't disable preempt, not too worried about poking the
 * wrong cpu's stat for now */
#define XFS_STATS_INC(count)(__get_cpu_var(xfsstats).count++)
#define XFS_STATS_DEC(count)(__get_cpu_var(xfsstats).count--)
#define XFS_STATS_ADD(count, inc)   (__get_cpu_var(xfsstats).count 
+= (inc))

So it knows about the fact that preemption can mess up the result of 
this,
but it does not really matter for the purpose it is used for here. The
stats are just informational but very handy for working out what is going
on inside xfs. Using a global instead of a per cpu variable would
lead to cache line contention.

If you want to make it go away on a preemptable kernel, then use the
alternate definition of the stat macros which is just below the
above code.
Steve
p.s. try running xfs_stats.pl -f which comes with the xfs-cmds source to
watch the stats.

Hi!
It happens to all functions -- not only to the stat functions -- because 
of this macro:
#define current_cpu()   smp_processor_id()

But this it not my problem (setting it 0 fixes it or get_cpu()). I just 
wanted to know why it breaks the preemption rules.

Matthias-Christian
--
http://unixforge.org/~matthias-christian-ott/
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: Preempt & Xfs Question

2005-01-27 Thread Chris Wedgwood
> BUG: using smp_processor_id() in preemptible [0001] code:
> khelper/892

fixed in CVS, I guess it will hit mainline soon
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: Preempt & Xfs Question

2005-01-27 Thread Steve Lord
Matthias-Christian Ott wrote:
Hi!
I have a question: Why do I get such debug messages:
BUG: using smp_processor_id() in preemptible [0001] code: khelper/892
caller is _pagebuf_lookup_pages+0x11b/0x362
[] smp_processor_id+0xa3/0xb4
[] _pagebuf_lookup_pages+0x11b/0x362
[] _pagebuf_lookup_pages+0x11b/0x362
.
Does the XFS Module avoid preemption rules? If so, why?
It is probably coming from these macros which keep various statistics
inside xfs as per cpu variables.
in fs//xfs/linux-2.6/xfs_stats.h:
DECLARE_PER_CPU(struct xfsstats, xfsstats);
/* We don't disable preempt, not too worried about poking the
 * wrong cpu's stat for now */
#define XFS_STATS_INC(count)(__get_cpu_var(xfsstats).count++)
#define XFS_STATS_DEC(count)(__get_cpu_var(xfsstats).count--)
#define XFS_STATS_ADD(count, inc)   (__get_cpu_var(xfsstats).count += (inc))
So it knows about the fact that preemption can mess up the result of this,
but it does not really matter for the purpose it is used for here. The
stats are just informational but very handy for working out what is going
on inside xfs. Using a global instead of a per cpu variable would
lead to cache line contention.
If you want to make it go away on a preemptable kernel, then use the
alternate definition of the stat macros which is just below the
above code.
Steve
p.s. try running xfs_stats.pl -f which comes with the xfs-cmds source to
watch the stats.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Preempt & Xfs Question

2005-01-27 Thread Matthias-Christian Ott
Hi!
I have a question: Why do I get such debug messages:
BUG: using smp_processor_id() in preemptible [0001] code: khelper/892
caller is _pagebuf_lookup_pages+0x11b/0x362
[] smp_processor_id+0xa3/0xb4
[] _pagebuf_lookup_pages+0x11b/0x362
[] _pagebuf_lookup_pages+0x11b/0x362
[] xfs_buf_get_flags+0x106/0x141
[] xfs_buf_read_flags+0x3e/0xae
[] xfs_trans_read_buf+0x18b/0x389
[] xfs_da_do_buf+0x735/0x88c
[] run_timer_softirq+0x12d/0x1c7
[] xfs_da_read_buf+0x47/0x4b
[] xfs_dir2_block_lookup_int+0x52/0x1b4
[] xfs_dir2_block_lookup_int+0x52/0x1b4
[] vprintk+0x133/0x16a
[] kernel_thread_helper+0x5/0xb
[] __kernel_text_address+0x28/0x36
[] xfs_dir2_block_lookup+0x2f/0xef
[] xfs_dir2_isblock+0x2c/0x74
[] xfs_dir2_lookup+0x176/0x186
[] __print_symbol+0x86/0xef
[] xfs_dir_lookup_int+0x4c/0x144
[] _spin_lock+0x16/0x7c
[] xfs_lookup+0x55/0x8a
[] linvfs_lookup+0x52/0x99
[] real_lookup+0xc2/0xe3
[] do_lookup+0x96/0xa1
[] link_path_walk+0x199/0xe3a
[] do_notify_parent+0x108/0x214
[] path_lookup+0x97/0x15f
[] open_exec+0x2f/0x102
[] task_rq_lock+0x36/0x66
[] do_execve+0x42/0x201
[] sys_execve+0x46/0x9c
[] syscall_call+0x7/0xb
[] call_usermodehelper+0xa1/0xc0
[] call_usermodehelper+0x0/0xc0
[] kernel_thread_helper+0x5/0xb
Does the XFS Module avoid preemption rules? If so, why?
Thanks
Matthias-Christian Ott
--
http://unixforge.org/~matthias-christian-ott/
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Preempt Xfs Question

2005-01-27 Thread Matthias-Christian Ott
Hi!
I have a question: Why do I get such debug messages:
BUG: using smp_processor_id() in preemptible [0001] code: khelper/892
caller is _pagebuf_lookup_pages+0x11b/0x362
[c03119c7] smp_processor_id+0xa3/0xb4
[c02ef802] _pagebuf_lookup_pages+0x11b/0x362
[c02ef802] _pagebuf_lookup_pages+0x11b/0x362
[c02efe31] xfs_buf_get_flags+0x106/0x141
[c02efeaa] xfs_buf_read_flags+0x3e/0xae
[c02e0fd5] xfs_trans_read_buf+0x18b/0x389
[c02ada32] xfs_da_do_buf+0x735/0x88c
[c0127f46] run_timer_softirq+0x12d/0x1c7
[c02adc1b] xfs_da_read_buf+0x47/0x4b
[c02b1b22] xfs_dir2_block_lookup_int+0x52/0x1b4
[c02b1b22] xfs_dir2_block_lookup_int+0x52/0x1b4
[c011f189] vprintk+0x133/0x16a
[c0101319] kernel_thread_helper+0x5/0xb
[c0130253] __kernel_text_address+0x28/0x36
[c02b1a10] xfs_dir2_block_lookup+0x2f/0xef
[c02b0967] xfs_dir2_isblock+0x2c/0x74
[c02aff4e] xfs_dir2_lookup+0x176/0x186
[c01389d9] __print_symbol+0x86/0xef
[c02e214e] xfs_dir_lookup_int+0x4c/0x144
[c04cf6aa] _spin_lock+0x16/0x7c
[c02e7cc8] xfs_lookup+0x55/0x8a
[c02f4736] linvfs_lookup+0x52/0x99
[c01698a2] real_lookup+0xc2/0xe3
[c0169b2c] do_lookup+0x96/0xa1
[c0169cd0] link_path_walk+0x199/0xe3a
[c012a2f1] do_notify_parent+0x108/0x214
[c016ac39] path_lookup+0x97/0x15f
[c0166612] open_exec+0x2f/0x102
[c011834b] task_rq_lock+0x36/0x66
[c01676ed] do_execve+0x42/0x201
[c0101cc1] sys_execve+0x46/0x9c
[c010318b] syscall_call+0x7/0xb
[c012e60e] call_usermodehelper+0xa1/0xc0
[c012e56d] call_usermodehelper+0x0/0xc0
[c0101319] kernel_thread_helper+0x5/0xb
Does the XFS Module avoid preemption rules? If so, why?
Thanks
Matthias-Christian Ott
--
http://unixforge.org/~matthias-christian-ott/
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: Preempt Xfs Question

2005-01-27 Thread Steve Lord
Matthias-Christian Ott wrote:
Hi!
I have a question: Why do I get such debug messages:
BUG: using smp_processor_id() in preemptible [0001] code: khelper/892
caller is _pagebuf_lookup_pages+0x11b/0x362
[c03119c7] smp_processor_id+0xa3/0xb4
[c02ef802] _pagebuf_lookup_pages+0x11b/0x362
[c02ef802] _pagebuf_lookup_pages+0x11b/0x362
.
Does the XFS Module avoid preemption rules? If so, why?
It is probably coming from these macros which keep various statistics
inside xfs as per cpu variables.
in fs//xfs/linux-2.6/xfs_stats.h:
DECLARE_PER_CPU(struct xfsstats, xfsstats);
/* We don't disable preempt, not too worried about poking the
 * wrong cpu's stat for now */
#define XFS_STATS_INC(count)(__get_cpu_var(xfsstats).count++)
#define XFS_STATS_DEC(count)(__get_cpu_var(xfsstats).count--)
#define XFS_STATS_ADD(count, inc)   (__get_cpu_var(xfsstats).count += (inc))
So it knows about the fact that preemption can mess up the result of this,
but it does not really matter for the purpose it is used for here. The
stats are just informational but very handy for working out what is going
on inside xfs. Using a global instead of a per cpu variable would
lead to cache line contention.
If you want to make it go away on a preemptable kernel, then use the
alternate definition of the stat macros which is just below the
above code.
Steve
p.s. try running xfs_stats.pl -f which comes with the xfs-cmds source to
watch the stats.
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: Preempt Xfs Question

2005-01-27 Thread Chris Wedgwood
 BUG: using smp_processor_id() in preemptible [0001] code:
 khelper/892

fixed in CVS, I guess it will hit mainline soon
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: Preempt Xfs Question

2005-01-27 Thread Matthias-Christian Ott
Steve Lord wrote:
Matthias-Christian Ott wrote:
Hi!
I have a question: Why do I get such debug messages:
BUG: using smp_processor_id() in preemptible [0001] code: 
khelper/892
caller is _pagebuf_lookup_pages+0x11b/0x362
[c03119c7] smp_processor_id+0xa3/0xb4
[c02ef802] _pagebuf_lookup_pages+0x11b/0x362
[c02ef802] _pagebuf_lookup_pages+0x11b/0x362

.
Does the XFS Module avoid preemption rules? If so, why?

It is probably coming from these macros which keep various statistics
inside xfs as per cpu variables.
in fs//xfs/linux-2.6/xfs_stats.h:
DECLARE_PER_CPU(struct xfsstats, xfsstats);
/* We don't disable preempt, not too worried about poking the
 * wrong cpu's stat for now */
#define XFS_STATS_INC(count)(__get_cpu_var(xfsstats).count++)
#define XFS_STATS_DEC(count)(__get_cpu_var(xfsstats).count--)
#define XFS_STATS_ADD(count, inc)   (__get_cpu_var(xfsstats).count 
+= (inc))

So it knows about the fact that preemption can mess up the result of 
this,
but it does not really matter for the purpose it is used for here. The
stats are just informational but very handy for working out what is going
on inside xfs. Using a global instead of a per cpu variable would
lead to cache line contention.

If you want to make it go away on a preemptable kernel, then use the
alternate definition of the stat macros which is just below the
above code.
Steve
p.s. try running xfs_stats.pl -f which comes with the xfs-cmds source to
watch the stats.

Hi!
It happens to all functions -- not only to the stat functions -- because 
of this macro:
#define current_cpu()   smp_processor_id()

But this it not my problem (setting it 0 fixes it or get_cpu()). I just 
wanted to know why it breaks the preemption rules.

Matthias-Christian
--
http://unixforge.org/~matthias-christian-ott/
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: Preempt Xfs Question

2005-01-27 Thread Matthias-Christian Ott
Chris Wedgwood wrote:
BUG: using smp_processor_id() in preemptible [0001] code:
khelper/892
   

fixed in CVS, I guess it will hit mainline soon
 

How did you fix it?
Matthias-Christian Ott
--
http://unixforge.org/~matthias-christian-ott/
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: Preempt Xfs Question

2005-01-27 Thread Chris Wedgwood
On Thu, Jan 27, 2005 at 05:46:54PM +, Matthias-Christian Ott wrote:

 How did you fix it?

I suggested:

= fs/xfs/linux-2.6/xfs_stats.h 1.9 vs edited =
Index: cw-current/fs/xfs/linux-2.6/xfs_stats.h
===
--- cw-current.orig/fs/xfs/linux-2.6/xfs_stats.h2005-01-17 
16:03:59.656946818 -0800
+++ cw-current/fs/xfs/linux-2.6/xfs_stats.h 2005-01-17 16:06:50.692361597 
-0800
@@ -142,9 +142,9 @@
 
 /* We don't disable preempt, not too worried about poking the
  * wrong cpu's stat for now */
-#define XFS_STATS_INC(count)   (__get_cpu_var(xfsstats).count++)
-#define XFS_STATS_DEC(count)   (__get_cpu_var(xfsstats).count--)
-#define XFS_STATS_ADD(count, inc)  (__get_cpu_var(xfsstats).count += (inc))
+#define XFS_STATS_INC(count)   (per_cpu(xfsstats, 
__smp_processor_id()).count++)
+#define XFS_STATS_DEC(count)   (per_cpu(xfsstats, 
__smp_processor_id()).count--)
+#define XFS_STATS_ADD(count, inc)  (per_cpu(xfsstats, 
__smp_processor_id()).count += (inc))
 
 extern void xfs_init_procfs(void);
 extern void xfs_cleanup_procfs(void);

but what was checked in was a bit cleaner.
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: Preempt Xfs Question

2005-01-27 Thread Matthias-Christian Ott
Chris Wedgwood wrote:
On Thu, Jan 27, 2005 at 05:46:54PM +, Matthias-Christian Ott wrote:
 

How did you fix it?
   

I suggested:
= fs/xfs/linux-2.6/xfs_stats.h 1.9 vs edited =
Index: cw-current/fs/xfs/linux-2.6/xfs_stats.h
===
--- cw-current.orig/fs/xfs/linux-2.6/xfs_stats.h2005-01-17 
16:03:59.656946818 -0800
+++ cw-current/fs/xfs/linux-2.6/xfs_stats.h 2005-01-17 16:06:50.692361597 
-0800
@@ -142,9 +142,9 @@
/* We don't disable preempt, not too worried about poking the
 * wrong cpu's stat for now */
-#define XFS_STATS_INC(count)   (__get_cpu_var(xfsstats).count++)
-#define XFS_STATS_DEC(count)   (__get_cpu_var(xfsstats).count--)
-#define XFS_STATS_ADD(count, inc)  (__get_cpu_var(xfsstats).count += (inc))
+#define XFS_STATS_INC(count)   (per_cpu(xfsstats, 
__smp_processor_id()).count++)
+#define XFS_STATS_DEC(count)   (per_cpu(xfsstats, 
__smp_processor_id()).count--)
+#define XFS_STATS_ADD(count, inc)  (per_cpu(xfsstats, 
__smp_processor_id()).count += (inc))
extern void xfs_init_procfs(void);
extern void xfs_cleanup_procfs(void);
but what was checked in was a bit cleaner.
 

Well calling such a internal function (__function) is not a cleaning 
coding style but works best :-) .
Combined with the current_cpu() fixes I mentioned, it looks like this:

diff -Nru linux-2.6.11-rc2/fs/xfs/linux-2.6/xfs_linux.h 
linux-2.6.11-rc2-ott/fs/xfs/linux-2.6/xfs_linux.h
--- linux-2.6.11-rc2/fs/xfs/linux-2.6/xfs_linux.h2004-12-24 
21:35:50.0 +
+++ linux-2.6.11-rc2-ott/fs/xfs/linux-2.6/xfs_linux.h2005-01-27 
18:13:09.0 +
@@ -144,7 +144,7 @@
#define xfs_inherit_nosymlinksxfs_params.inherit_nosym.val
#define xfs_rotorstepxfs_params.rotorstep.val

-#define current_cpu()smp_processor_id()
+#define current_cpu()__smp_processor_id()
#define current_pid()(current-pid)
#define current_fsuid(cred)(current-fsuid)
#define current_fsgid(cred)(current-fsgid)
diff -Nru linux-2.6.11-rc2/fs/xfs/linux-2.6/xfs_stats.h 
linux-2.6.11-rc2-ott/fs/xfs/linux-2.6/xfs_stats.h
--- linux-2.6.11-rc2/fs/xfs/linux-2.6/xfs_stats.h2004-12-24 
21:34:29.0 +
+++ linux-2.6.11-rc2-ott/fs/xfs/linux-2.6/xfs_stats.h2005-01-27 
18:13:44.0 +
@@ -142,9 +142,9 @@

/* We don't disable preempt, not too worried about poking the
 * wrong cpu's stat for now */
-#define XFS_STATS_INC(count)(__get_cpu_var(xfsstats).count++)
-#define XFS_STATS_DEC(count)(__get_cpu_var(xfsstats).count--)
-#define XFS_STATS_ADD(count, inc)(__get_cpu_var(xfsstats).count += 
(inc))
+#define XFS_STATS_INC(count)(per_cpu(xfsstats, 
__smp_processor_id()).count++)
+#define XFS_STATS_DEC(count)(per_cpu(xfsstats, 
__smp_processor_id()).count--)
+#define XFS_STATS_ADD(count, inc)(per_cpu(xfsstats, 
__smp_processor_id()).count += (inc))

extern void xfs_init_procfs(void);
extern void xfs_cleanup_procfs(void);
I'll submit it to the mailinglist as a seperate patch, so Linus can 
apply it to the current Kernel.

Matthias-Christian Ott
--
http://unixforge.org/~matthias-christian-ott/
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: Preempt Xfs Question

2005-01-27 Thread Chris Wedgwood
On Thu, Jan 27, 2005 at 06:24:13PM +, Matthias-Christian Ott wrote:

 Well calling such a internal function (__function) is not a cleaning
 coding style but works best :-) .

__foo does NOT mean it's an internal function necessarily or that it's
unclean to use it (sadly Linux has pretty vague (nothing consistent)
rules on how to interpret __foo versus foo really.

 Combined with the current_cpu() fixes I mentioned, it looks like
 this:

(1) your patch is mangled/wrapped

(2) this is fixed in CVS already (for maybe a week or so now?)

 I'll submit it to the mailinglist as a seperate patch, so Linus can
 apply it to the current Kernel.

XFS patches/suggestions should go to linux-xfs@oss.sgi.com and the
maintainers there can comment as needed.
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: Preempt Xfs Question

2005-01-27 Thread Matthias-Christian Ott
Chris Wedgwood wrote:
On Thu, Jan 27, 2005 at 06:24:13PM +, Matthias-Christian Ott wrote:
 

Well calling such a internal function (__function) is not a cleaning
coding style but works best :-) .
   

__foo does NOT mean it's an internal function necessarily or that it's
unclean to use it (sadly Linux has pretty vague (nothing consistent)
rules on how to interpret __foo versus foo really.
 

Combined with the current_cpu() fixes I mentioned, it looks like
this:
   

(1) your patch is mangled/wrapped
(2) this is fixed in CVS already (for maybe a week or so now?)
 

I'll submit it to the mailinglist as a seperate patch, so Linus can
apply it to the current Kernel.
   

XFS patches/suggestions should go to linux-xfs@oss.sgi.com and the
maintainers there can comment as needed.
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
 

Hi!
I didn't know about the xfs development status. But it's good to hear 
that it's already fixed. When will it be included to the Kernel?

Matthias-Christian Ott
--
http://unixforge.org/~matthias-christian-ott/
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: Preempt Xfs Question

2005-01-27 Thread Nathan Scott
On Thu, Jan 27, 2005 at 08:51:45AM -0800, Chris Wedgwood wrote:
 On Thu, Jan 27, 2005 at 06:24:13PM +, Matthias-Christian Ott wrote:
  I'll submit it to the mailinglist as a seperate patch, so Linus can
  apply it to the current Kernel.

Chris' fix for this is in Linus' mail, queued to be merged hopefully RSN.
(*prod prod*)

 XFS patches/suggestions should go to linux-xfs@oss.sgi.com and the
 maintainers there can comment as needed.

cheers.

-- 
Nathan
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/