2014-07-15 01:05:38

by Chen Gang

[permalink] [raw]
Subject: [PATCH] fs/ext4/super.c: Let 'counters' switched by SMP

When SMP is disabled, 'struct percpu_counter' has no 'counters' member,
so need skip it when in !SMP.

The related git commit:

"feab139 ext4: rearrange initialization to fix EXT4FS_DEBUG".

The related error (with allmodconfig under score):

CC [M] fs/ext4/super.o
fs/ext4/super.c: In function 'ext4_commit_super':
fs/ext4/super.c:4546:41: error: 'struct percpu_counter' has no member named 'counters'
if (EXT4_SB(sb)->s_freeclusters_counter.counters)
^
fs/ext4/super.c:4550:39: error: 'struct percpu_counter' has no member named 'counters'
if (EXT4_SB(sb)->s_freeinodes_counter.counters)
^

Signed-off-by: Chen Gang <[email protected]>
---
fs/ext4/super.c | 4 ++++
1 file changed, 4 insertions(+)

diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index 30767cd..3cd4fe1 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -4543,11 +4543,15 @@ static int ext4_commit_super(struct super_block *sb, int sync)
else
es->s_kbytes_written =
cpu_to_le64(EXT4_SB(sb)->s_kbytes_written);
+#ifdef CONFIG_SMP
if (EXT4_SB(sb)->s_freeclusters_counter.counters)
+#endif
ext4_free_blocks_count_set(es,
EXT4_C2B(EXT4_SB(sb), percpu_counter_sum_positive(
&EXT4_SB(sb)->s_freeclusters_counter)));
+#ifdef CONFIG_SMP
if (EXT4_SB(sb)->s_freeinodes_counter.counters)
+#endif
es->s_free_inodes_count =
cpu_to_le32(percpu_counter_sum_positive(
&EXT4_SB(sb)->s_freeinodes_counter));
--
1.9.2.459.g68773ac


2014-07-15 09:32:24

by Theodore Ts'o

[permalink] [raw]
Subject: Re: [PATCH] fs/ext4/super.c: Let 'counters' switched by SMP

On Tue, Jul 15, 2014 at 09:05:29AM +0800, Chen Gang wrote:
> When SMP is disabled, 'struct percpu_counter' has no 'counters' member,
> so need skip it when in !SMP.
>
> The related git commit:
>
> "feab139 ext4: rearrange initialization to fix EXT4FS_DEBUG".
>
> The related error (with allmodconfig under score):
>
> CC [M] fs/ext4/super.o
> fs/ext4/super.c: In function 'ext4_commit_super':
> fs/ext4/super.c:4546:41: error: 'struct percpu_counter' has no member named 'counters'
> if (EXT4_SB(sb)->s_freeclusters_counter.counters)
> ^
> fs/ext4/super.c:4550:39: error: 'struct percpu_counter' has no member named 'counters'
> if (EXT4_SB(sb)->s_freeinodes_counter.counters)

Thanks for the pointing out the problem. I though think the right way
of fixing this problem, though, instead of using the #ifdef
CONFIG_SMP, is to do this:

if (percpu_counter_initialized(&EXT4_SB(sb)->s_freeclusters_counter))
...

I'll respin the "ext4: rearrange initialization to fix EXT4FS_DEBUG"
commit to include this fix and give you credit in the commit logs for
pointing out this issue.

Cheers,

- Ted

2014-07-15 10:06:59

by Chen Gang

[permalink] [raw]
Subject: Re: [PATCH] fs/ext4/super.c: Let 'counters' switched by SMP

On 07/15/2014 05:31 PM, Theodore Ts'o wrote:
> On Tue, Jul 15, 2014 at 09:05:29AM +0800, Chen Gang wrote:
>> When SMP is disabled, 'struct percpu_counter' has no 'counters' member,
>> so need skip it when in !SMP.
>>
>> The related git commit:
>>
>> "feab139 ext4: rearrange initialization to fix EXT4FS_DEBUG".
>>
>> The related error (with allmodconfig under score):
>>
>> CC [M] fs/ext4/super.o
>> fs/ext4/super.c: In function 'ext4_commit_super':
>> fs/ext4/super.c:4546:41: error: 'struct percpu_counter' has no member named 'counters'
>> if (EXT4_SB(sb)->s_freeclusters_counter.counters)
>> ^
>> fs/ext4/super.c:4550:39: error: 'struct percpu_counter' has no member named 'counters'
>> if (EXT4_SB(sb)->s_freeinodes_counter.counters)
>
> Thanks for the pointing out the problem. I though think the right way
> of fixing this problem, though, instead of using the #ifdef
> CONFIG_SMP, is to do this:
>
> if (percpu_counter_initialized(&EXT4_SB(sb)->s_freeclusters_counter))
> ...
>

That sounds fine to me (unlike my ugly fix, your fix is common and
standard), thanks.

> I'll respin the "ext4: rearrange initialization to fix EXT4FS_DEBUG"
> commit to include this fix and give you credit in the commit logs for
> pointing out this issue.
>

Thank you for your work.


Thanks.
--
Chen Gang

Open, share, and attitude like air, water, and life which God blessed

2014-07-21 17:40:09

by Olof Johansson

[permalink] [raw]
Subject: Re: [PATCH] fs/ext4/super.c: Let 'counters' switched by SMP

Hi Ted,

On Tue, Jul 15, 2014 at 2:31 AM, Theodore Ts'o <[email protected]> wrote:
> On Tue, Jul 15, 2014 at 09:05:29AM +0800, Chen Gang wrote:
>> When SMP is disabled, 'struct percpu_counter' has no 'counters' member,
>> so need skip it when in !SMP.
>>
>> The related git commit:
>>
>> "feab139 ext4: rearrange initialization to fix EXT4FS_DEBUG".
>>
>> The related error (with allmodconfig under score):
>>
>> CC [M] fs/ext4/super.o
>> fs/ext4/super.c: In function 'ext4_commit_super':
>> fs/ext4/super.c:4546:41: error: 'struct percpu_counter' has no member named 'counters'
>> if (EXT4_SB(sb)->s_freeclusters_counter.counters)
>> ^
>> fs/ext4/super.c:4550:39: error: 'struct percpu_counter' has no member named 'counters'
>> if (EXT4_SB(sb)->s_freeinodes_counter.counters)
>
> Thanks for the pointing out the problem. I though think the right way
> of fixing this problem, though, instead of using the #ifdef
> CONFIG_SMP, is to do this:
>
> if (percpu_counter_initialized(&EXT4_SB(sb)->s_freeclusters_counter))
> ...
>
> I'll respin the "ext4: rearrange initialization to fix EXT4FS_DEBUG"
> commit to include this fix and give you credit in the commit logs for
> pointing out this issue.

Friendly ping, since all !SMP builds in -next seem to be broken by this still.


-Olof

2014-07-21 19:12:34

by Theodore Ts'o

[permalink] [raw]
Subject: Re: [PATCH] fs/ext4/super.c: Let 'counters' switched by SMP

On Mon, Jul 21, 2014 at 10:40:02AM -0700, Olof Johansson wrote:
> > I'll respin the "ext4: rearrange initialization to fix EXT4FS_DEBUG"
> > commit to include this fix and give you credit in the commit logs for
> > pointing out this issue.
>
> Friendly ping, since all !SMP builds in -next seem to be broken by this still.

Right, sorry, I'm currently in vacation in Rome, and the network
connection was really lousy here now that I'm no longer at the ACACES
summer school in Fiuggi. (Also, now that I'm no longer actually
teaching, I've beeen trying to actually do the disconnecting and
sightseeing thing. :-)

Pushed now.

- Ted

2014-07-23 22:55:06

by David Rientjes

[permalink] [raw]
Subject: Re: [PATCH] fs/ext4/super.c: Let 'counters' switched by SMP

On Mon, 21 Jul 2014, Theodore Ts'o wrote:

> Right, sorry, I'm currently in vacation in Rome, and the network
> connection was really lousy here now that I'm no longer at the ACACES
> summer school in Fiuggi. (Also, now that I'm no longer actually
> teaching, I've beeen trying to actually do the disconnecting and
> sightseeing thing. :-)
>

Confirmed this is now fixed in linux-next, thanks Ted!