Re: [Suggestion] kernel: cgroup: mount failure in LTP cgroup_regression_test.sh

2013-11-20 Thread Chen Gang
On 11/21/2013 01:35 AM, Tejun Heo wrote:
> Hello,
> 
> Sorry about the delay.
> 

Oh, No problem, every members time resources are expensive, thank you
for your reply.


> On Fri, Nov 08, 2013 at 04:15:23PM +0800, Chen Gang wrote:
>> After simplify, the related operation, environments and output are:
>>
>>   [root@gchenlinux tmp]# df -Th | grep cgroup
>>   tmpfs tmpfs1001M 0 1001M   0% 
>> /sys/fs/cgroup
>>   [root@gchenlinux tmp]# lsof | grep cgroup | grep -v grep
>>   systemd   1  root6r  DIR   0,18 0  
>>  5998 /sys/fs/cgroup/systemd/system
>>   [root@gchenlinux tmp]# cat /proc/cgroups 
>>   #subsys_name   hierarchy   num_cgroups enabled
>>   cpuset 3   4   1
>>   cpu4   35  1
>>   cpuacct4   35  1
>>   freezer5   4   1
>>   [root@gchenlinux tmp]# mkdir cgroup
>>   [root@gchenlinux tmp]# mount -t cgroup -o freezer,cpuacct xxx cgroup/
>>   mount: xxx already mounted or cgroup/ busy
>>
>> Is it real issue of cgroup? If it is, I will/should continue analyzing.
> 
> Hmmm... I'm a bit confused.  What is it testing?  "cat /proc/cgroup"
> is showing that freezer is already mounted and the kernel seems to
> have correctly refused to mount it in a different hierarchy.  What am
> I missing here?
> 

Hmm... I am not quit familiar about it, either, but at least I can
contact related LTP members (I guess they can make sure about it).


Excuse me, I am changing/finding new job, so I should not use Asianux
Corporation mail again.  And it seems I can not be focus on upstream
kernel as much as before.  :-(

But at least, I will/should continue providing some contributions (more
or less) to Public Open Source as volunteers. And for upstream kernel,
I plan to make 1-3 patches per month (mainly for cross compiling).

 - if can not get companies support, I can not provide enough time
   resource on upstream kernel (I have to get pay check for family).

 - New company may mainly focus on open source tool chains (not only
   kernel), so I will/should also provide some contributions to them
   (e.g. KVM, GCC ...).


At last, sorry again for I have to give up 10 patches per month for
upstream kernel.


Thanks.
-- 
Chen Gang
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [Suggestion] kernel: cgroup: mount failure in LTP cgroup_regression_test.sh

2013-11-20 Thread Tejun Heo
Hello,

Sorry about the delay.

On Fri, Nov 08, 2013 at 04:15:23PM +0800, Chen Gang wrote:
> After simplify, the related operation, environments and output are:
> 
>   [root@gchenlinux tmp]# df -Th | grep cgroup
>   tmpfs tmpfs1001M 0 1001M   0% 
> /sys/fs/cgroup
>   [root@gchenlinux tmp]# lsof | grep cgroup | grep -v grep
>   systemd   1  root6r  DIR   0,18 0   
> 5998 /sys/fs/cgroup/systemd/system
>   [root@gchenlinux tmp]# cat /proc/cgroups 
>   #subsys_namehierarchy   num_cgroups enabled
>   cpuset  3   4   1
>   cpu 4   35  1
>   cpuacct 4   35  1
>   freezer 5   4   1
>   [root@gchenlinux tmp]# mkdir cgroup
>   [root@gchenlinux tmp]# mount -t cgroup -o freezer,cpuacct xxx cgroup/
>   mount: xxx already mounted or cgroup/ busy
> 
> Is it real issue of cgroup? If it is, I will/should continue analyzing.

Hmmm... I'm a bit confused.  What is it testing?  "cat /proc/cgroup"
is showing that freezer is already mounted and the kernel seems to
have correctly refused to mount it in a different hierarchy.  What am
I missing here?

Thanks.

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


Re: [Suggestion] kernel: cgroup: mount failure in LTP cgroup_regression_test.sh

2013-11-20 Thread Tejun Heo
Hello,

Sorry about the delay.

On Fri, Nov 08, 2013 at 04:15:23PM +0800, Chen Gang wrote:
 After simplify, the related operation, environments and output are:
 
   [root@gchenlinux tmp]# df -Th | grep cgroup
   tmpfs tmpfs1001M 0 1001M   0% 
 /sys/fs/cgroup
   [root@gchenlinux tmp]# lsof | grep cgroup | grep -v grep
   systemd   1  root6r  DIR   0,18 0   
 5998 /sys/fs/cgroup/systemd/system
   [root@gchenlinux tmp]# cat /proc/cgroups 
   #subsys_namehierarchy   num_cgroups enabled
   cpuset  3   4   1
   cpu 4   35  1
   cpuacct 4   35  1
   freezer 5   4   1
   [root@gchenlinux tmp]# mkdir cgroup
   [root@gchenlinux tmp]# mount -t cgroup -o freezer,cpuacct xxx cgroup/
   mount: xxx already mounted or cgroup/ busy
 
 Is it real issue of cgroup? If it is, I will/should continue analyzing.

Hmmm... I'm a bit confused.  What is it testing?  cat /proc/cgroup
is showing that freezer is already mounted and the kernel seems to
have correctly refused to mount it in a different hierarchy.  What am
I missing here?

Thanks.

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


Re: [Suggestion] kernel: cgroup: mount failure in LTP cgroup_regression_test.sh

2013-11-20 Thread Chen Gang
On 11/21/2013 01:35 AM, Tejun Heo wrote:
 Hello,
 
 Sorry about the delay.
 

Oh, No problem, every members time resources are expensive, thank you
for your reply.


 On Fri, Nov 08, 2013 at 04:15:23PM +0800, Chen Gang wrote:
 After simplify, the related operation, environments and output are:

   [root@gchenlinux tmp]# df -Th | grep cgroup
   tmpfs tmpfs1001M 0 1001M   0% 
 /sys/fs/cgroup
   [root@gchenlinux tmp]# lsof | grep cgroup | grep -v grep
   systemd   1  root6r  DIR   0,18 0  
  5998 /sys/fs/cgroup/systemd/system
   [root@gchenlinux tmp]# cat /proc/cgroups 
   #subsys_name   hierarchy   num_cgroups enabled
   cpuset 3   4   1
   cpu4   35  1
   cpuacct4   35  1
   freezer5   4   1
   [root@gchenlinux tmp]# mkdir cgroup
   [root@gchenlinux tmp]# mount -t cgroup -o freezer,cpuacct xxx cgroup/
   mount: xxx already mounted or cgroup/ busy

 Is it real issue of cgroup? If it is, I will/should continue analyzing.
 
 Hmmm... I'm a bit confused.  What is it testing?  cat /proc/cgroup
 is showing that freezer is already mounted and the kernel seems to
 have correctly refused to mount it in a different hierarchy.  What am
 I missing here?
 

Hmm... I am not quit familiar about it, either, but at least I can
contact related LTP members (I guess they can make sure about it).


Excuse me, I am changing/finding new job, so I should not use Asianux
Corporation mail again.  And it seems I can not be focus on upstream
kernel as much as before.  :-(

But at least, I will/should continue providing some contributions (more
or less) to Public Open Source as volunteers. And for upstream kernel,
I plan to make 1-3 patches per month (mainly for cross compiling).

 - if can not get companies support, I can not provide enough time
   resource on upstream kernel (I have to get pay check for family).

 - New company may mainly focus on open source tool chains (not only
   kernel), so I will/should also provide some contributions to them
   (e.g. KVM, GCC ...).


At last, sorry again for I have to give up 10 patches per month for
upstream kernel.


Thanks.
-- 
Chen Gang
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[Suggestion] kernel: cgroup: mount failure in LTP cgroup_regression_test.sh

2013-11-08 Thread Chen Gang
Hello Maintainers:

On Fedora16 with defconfig for next-20131107 kernel, use latest LTP
(Linux Test Project) version, test_5() in cgroup_regression_test.sh
will be fail. The related LTP output:

  <<>>
  tag=cgroup stime=1383562810
  cmdline="   cgroup_regression_test.sh"
  contacts=""
  analysis=exit
  <<>>
  cgroup_regression_test1  TPASS  :  no kernel bug was found
  /opt/ltp/testcases/bin/cgroup_regression_test.sh: line 118: 13283 Terminated  
./fork_processes
  cgroup_regression_test2  TPASS  :  notify_on_release is inherited
  cgroup_regression_test3  TCONF  :  CONFIG_SCHED_DEBUG is not enabled
  cgroup_regression_test4  TCONF  :  CONFIG_LOCKDEP is not enabled
  mount: xxx already mounted or cgroup/ busy
  cgroup_regression_test5  TFAIL  :  mount freezer and cpuacct failed
  cgroup_regression_test6  TCONF  :  CONFIG_CGROUP_NS
  /opt/ltp/testcases/bin/cgroup_regression_test.sh: line 360: 24428 Terminated  
sleep 100 < cgroup/0
  /opt/ltp/testcases/bin/cgroup_regression_test.sh: line 381: 24447 Terminated  
sleep 100 < cgroup/0
  cgroup_regression_test7  TPASS  :  no kernel bug was found
  cgroup_regression_test8  TPASS  :  no kernel bug was found
  cgroup_regression_test9  TPASS  :  no kernel warning was found
  rmdir: failed to remove `cgroup/0': No such file or directory
  umount: /sys/fs/cgroup/systemd: device is busy.
  (In some cases useful info about processes that use
   the device is found by lsof(8) or fuser(1))
  cgroup_regression_test   10  TPASS  :  no kernel warning was found
  <<>>
  initiation_status="ok"
  duration=62 termination_type=exited termination_id=1 corefile=no
  cutime=3341 cstime=6353
  <<>>

After simplify, the related operation, environments and output are:

  [root@gchenlinux tmp]# df -Th | grep cgroup
  tmpfs tmpfs1001M 0 1001M   0% 
/sys/fs/cgroup
  [root@gchenlinux tmp]# lsof | grep cgroup | grep -v grep
  systemd   1  root6r  DIR   0,18 0 
  5998 /sys/fs/cgroup/systemd/system
  [root@gchenlinux tmp]# cat /proc/cgroups 
  #subsys_name  hierarchy   num_cgroups enabled
  cpuset3   4   1
  cpu   4   35  1
  cpuacct   4   35  1
  freezer   5   4   1
  [root@gchenlinux tmp]# mkdir cgroup
  [root@gchenlinux tmp]# mount -t cgroup -o freezer,cpuacct xxx cgroup/
  mount: xxx already mounted or cgroup/ busy

Is it real issue of cgroup? If it is, I will/should continue analyzing.


Welcome any additional suggestions and completions.

Thanks.
--
Chen Gang
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[Suggestion] kernel: cgroup: mount failure in LTP cgroup_regression_test.sh

2013-11-08 Thread Chen Gang
Hello Maintainers:

On Fedora16 with defconfig for next-20131107 kernel, use latest LTP
(Linux Test Project) version, test_5() in cgroup_regression_test.sh
will be fail. The related LTP output:

  test_start
  tag=cgroup stime=1383562810
  cmdline=   cgroup_regression_test.sh
  contacts=
  analysis=exit
  test_output
  cgroup_regression_test1  TPASS  :  no kernel bug was found
  /opt/ltp/testcases/bin/cgroup_regression_test.sh: line 118: 13283 Terminated  
./fork_processes
  cgroup_regression_test2  TPASS  :  notify_on_release is inherited
  cgroup_regression_test3  TCONF  :  CONFIG_SCHED_DEBUG is not enabled
  cgroup_regression_test4  TCONF  :  CONFIG_LOCKDEP is not enabled
  mount: xxx already mounted or cgroup/ busy
  cgroup_regression_test5  TFAIL  :  mount freezer and cpuacct failed
  cgroup_regression_test6  TCONF  :  CONFIG_CGROUP_NS
  /opt/ltp/testcases/bin/cgroup_regression_test.sh: line 360: 24428 Terminated  
sleep 100  cgroup/0
  /opt/ltp/testcases/bin/cgroup_regression_test.sh: line 381: 24447 Terminated  
sleep 100  cgroup/0
  cgroup_regression_test7  TPASS  :  no kernel bug was found
  cgroup_regression_test8  TPASS  :  no kernel bug was found
  cgroup_regression_test9  TPASS  :  no kernel warning was found
  rmdir: failed to remove `cgroup/0': No such file or directory
  umount: /sys/fs/cgroup/systemd: device is busy.
  (In some cases useful info about processes that use
   the device is found by lsof(8) or fuser(1))
  cgroup_regression_test   10  TPASS  :  no kernel warning was found
  execution_status
  initiation_status=ok
  duration=62 termination_type=exited termination_id=1 corefile=no
  cutime=3341 cstime=6353
  test_end

After simplify, the related operation, environments and output are:

  [root@gchenlinux tmp]# df -Th | grep cgroup
  tmpfs tmpfs1001M 0 1001M   0% 
/sys/fs/cgroup
  [root@gchenlinux tmp]# lsof | grep cgroup | grep -v grep
  systemd   1  root6r  DIR   0,18 0 
  5998 /sys/fs/cgroup/systemd/system
  [root@gchenlinux tmp]# cat /proc/cgroups 
  #subsys_name  hierarchy   num_cgroups enabled
  cpuset3   4   1
  cpu   4   35  1
  cpuacct   4   35  1
  freezer   5   4   1
  [root@gchenlinux tmp]# mkdir cgroup
  [root@gchenlinux tmp]# mount -t cgroup -o freezer,cpuacct xxx cgroup/
  mount: xxx already mounted or cgroup/ busy

Is it real issue of cgroup? If it is, I will/should continue analyzing.


Welcome any additional suggestions and completions.

Thanks.
--
Chen Gang
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [Suggestion] kernel/rcutorture.c: about using scnprintf() instead of sprintf().

2013-10-15 Thread Chen Gang
On 10/15/2013 04:31 PM, Paul E. McKenney wrote:
> On Tue, Oct 15, 2013 at 09:40:40AM +0800, Chen Gang wrote:
>> > Hmm... Can it really work on 1024 CPUs? sorry I don't know. But in fact,
>> > that is not about this patch (it is just one of case which may cause
>> > issues).
>> > 
>> > This patch is only about "use truncation instead of memory overflow, and
>> > make sure the modification without negative effect". So in my opinion,
>> > current test case is enough for this requirement.
> Yes, the patch is about "use truncation instead of memory overflow",
> but the truncation would also be a problem on large systems.  Is it
> possible to prevent memory from overflow in the first place?
> 

Yes of cause it can, but in my opinion, for a test module, truncating
information is acceptable, but memory overflow is not acceptable.

In fact, every information truncation case, can be "prevented memory
from overflow in the first place".  It depends on whether we have enough
interest to do that.


>> > We have to face the efficiency: it is only a test module, if the tester
>> > (assume he/she is a programmer, too) notices about the truncation, they
>> > can simply extend the maximize length to avoid truncation.
> True.  But you can make a change that is just as simple that allows you
> to test what would happen on a 1024-CPU system even though your own
> system has only 2 CPUs.  Can you see what that change is?

In fact, from modifying code, we can virtual most of cases, if current
test case is enough, we need not do that (leave it for guys who have
real 1024-CPUs).


Thanks.
-- 
Chen Gang
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [Suggestion] kernel/rcutorture.c: about using scnprintf() instead of sprintf().

2013-10-15 Thread Paul E. McKenney
On Tue, Oct 15, 2013 at 09:40:40AM +0800, Chen Gang wrote:
> On 10/14/2013 07:24 PM, Paul E. McKenney wrote:
> > On Mon, Oct 14, 2013 at 10:22:20AM +0800, Chen Gang wrote:
> >>>  - intend to shrink maximized buffer (PAGE_SIZE -> 64, 256 ..) for test.
> > 
> > This is a good start!  However, you should also test the original kernel
> > to be sure that it really fails.  You should of course start by asking
> > yourself how you would expect it to fail.
> 
> When shrink size, for scnprintf() we already got truncation, that means
> if we still use sprintf(), it will cause memory overflow.
> 
> For memory overflow, it does not means OS will stop quickly, one of the
> worst condition is "it may still seems going 'well' but sometimes
> may/will show some amazing things which is not easy to find root cause".
> 
> So for this kind of memory overflow, "shrinking size and letting
> scnprintf() instead of sprintf() to see whether truncation" is well enough.
> 
> > What other change should you make to test this in order to be sure that
> > it will really work when someone tries it on 1024 CPUs?  (I am asking
> > rather than telling because you really need to have this stuff worked
> > out on your initial submission.)
> 
> Hmm... Can it really work on 1024 CPUs? sorry I don't know. But in fact,
> that is not about this patch (it is just one of case which may cause
> issues).
> 
> This patch is only about "use truncation instead of memory overflow, and
> make sure the modification without negative effect". So in my opinion,
> current test case is enough for this requirement.

Yes, the patch is about "use truncation instead of memory overflow",
but the truncation would also be a problem on large systems.  Is it
possible to prevent memory from overflow in the first place?

> Maybe you want to let this module get additional test to find additional
> issues? (If so, I will try when my time resource is possible).
> 
> > Another way of thinking about this is to ask yourself the question "If
> > someone ran rcutorture with torture_type=srcu on a system with 1024 CPUs
> > (to say nothing of 4096 CPUs), what would they want to happen?"  Then:
> > "How would I test for that on a smaller system?"
> 
> We have to face the efficiency: it is only a test module, if the tester
> (assume he/she is a programmer, too) notices about the truncation, they
> can simply extend the maximize length to avoid truncation.

True.  But you can make a change that is just as simple that allows you
to test what would happen on a 1024-CPU system even though your own
system has only 2 CPUs.  Can you see what that change is?

Thanx, Paul

> At least for me, "rcutorture.c" is really easy enough for a programmer
> to test rcu (and also, it is really a useful tool for test rcu).
> 
> And for "1024 CPUs",  I think one of efficient way is: "when some
> members use it under 1024 CPUs, if they find something can be
> improvement, they can report/send related patch".
> 
> 
> Hmm... maybe the comment "it is ... additional test or consideration"
> need be removed: 'additional' and 'consideration' are not suitable words
> to be appeared in comments (they are not precise word).
> 
> 
> Thanks.
> -- 
> Chen Gang
> 

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


Re: [Suggestion] kernel/rcutorture.c: about using scnprintf() instead of sprintf().

2013-10-15 Thread Paul E. McKenney
On Tue, Oct 15, 2013 at 09:40:40AM +0800, Chen Gang wrote:
 On 10/14/2013 07:24 PM, Paul E. McKenney wrote:
  On Mon, Oct 14, 2013 at 10:22:20AM +0800, Chen Gang wrote:
   - intend to shrink maximized buffer (PAGE_SIZE - 64, 256 ..) for test.
  
  This is a good start!  However, you should also test the original kernel
  to be sure that it really fails.  You should of course start by asking
  yourself how you would expect it to fail.
 
 When shrink size, for scnprintf() we already got truncation, that means
 if we still use sprintf(), it will cause memory overflow.
 
 For memory overflow, it does not means OS will stop quickly, one of the
 worst condition is it may still seems going 'well' but sometimes
 may/will show some amazing things which is not easy to find root cause.
 
 So for this kind of memory overflow, shrinking size and letting
 scnprintf() instead of sprintf() to see whether truncation is well enough.
 
  What other change should you make to test this in order to be sure that
  it will really work when someone tries it on 1024 CPUs?  (I am asking
  rather than telling because you really need to have this stuff worked
  out on your initial submission.)
 
 Hmm... Can it really work on 1024 CPUs? sorry I don't know. But in fact,
 that is not about this patch (it is just one of case which may cause
 issues).
 
 This patch is only about use truncation instead of memory overflow, and
 make sure the modification without negative effect. So in my opinion,
 current test case is enough for this requirement.

Yes, the patch is about use truncation instead of memory overflow,
but the truncation would also be a problem on large systems.  Is it
possible to prevent memory from overflow in the first place?

 Maybe you want to let this module get additional test to find additional
 issues? (If so, I will try when my time resource is possible).
 
  Another way of thinking about this is to ask yourself the question If
  someone ran rcutorture with torture_type=srcu on a system with 1024 CPUs
  (to say nothing of 4096 CPUs), what would they want to happen?  Then:
  How would I test for that on a smaller system?
 
 We have to face the efficiency: it is only a test module, if the tester
 (assume he/she is a programmer, too) notices about the truncation, they
 can simply extend the maximize length to avoid truncation.

True.  But you can make a change that is just as simple that allows you
to test what would happen on a 1024-CPU system even though your own
system has only 2 CPUs.  Can you see what that change is?

Thanx, Paul

 At least for me, rcutorture.c is really easy enough for a programmer
 to test rcu (and also, it is really a useful tool for test rcu).
 
 And for 1024 CPUs,  I think one of efficient way is: when some
 members use it under 1024 CPUs, if they find something can be
 improvement, they can report/send related patch.
 
 
 Hmm... maybe the comment it is ... additional test or consideration
 need be removed: 'additional' and 'consideration' are not suitable words
 to be appeared in comments (they are not precise word).
 
 
 Thanks.
 -- 
 Chen Gang
 

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


Re: [Suggestion] kernel/rcutorture.c: about using scnprintf() instead of sprintf().

2013-10-15 Thread Chen Gang
On 10/15/2013 04:31 PM, Paul E. McKenney wrote:
 On Tue, Oct 15, 2013 at 09:40:40AM +0800, Chen Gang wrote:
  Hmm... Can it really work on 1024 CPUs? sorry I don't know. But in fact,
  that is not about this patch (it is just one of case which may cause
  issues).
  
  This patch is only about use truncation instead of memory overflow, and
  make sure the modification without negative effect. So in my opinion,
  current test case is enough for this requirement.
 Yes, the patch is about use truncation instead of memory overflow,
 but the truncation would also be a problem on large systems.  Is it
 possible to prevent memory from overflow in the first place?
 

Yes of cause it can, but in my opinion, for a test module, truncating
information is acceptable, but memory overflow is not acceptable.

In fact, every information truncation case, can be prevented memory
from overflow in the first place.  It depends on whether we have enough
interest to do that.


  We have to face the efficiency: it is only a test module, if the tester
  (assume he/she is a programmer, too) notices about the truncation, they
  can simply extend the maximize length to avoid truncation.
 True.  But you can make a change that is just as simple that allows you
 to test what would happen on a 1024-CPU system even though your own
 system has only 2 CPUs.  Can you see what that change is?

In fact, from modifying code, we can virtual most of cases, if current
test case is enough, we need not do that (leave it for guys who have
real 1024-CPUs).


Thanks.
-- 
Chen Gang
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [Suggestion] kernel/rcutorture.c: about using scnprintf() instead of sprintf().

2013-10-14 Thread Chen Gang
On 10/14/2013 07:24 PM, Paul E. McKenney wrote:
> On Mon, Oct 14, 2013 at 10:22:20AM +0800, Chen Gang wrote:
>>>  - intend to shrink maximized buffer (PAGE_SIZE -> 64, 256 ..) for test.
> 
> This is a good start!  However, you should also test the original kernel
> to be sure that it really fails.  You should of course start by asking
> yourself how you would expect it to fail.
> 

When shrink size, for scnprintf() we already got truncation, that means
if we still use sprintf(), it will cause memory overflow.

For memory overflow, it does not means OS will stop quickly, one of the
worst condition is "it may still seems going 'well' but sometimes
may/will show some amazing things which is not easy to find root cause".

So for this kind of memory overflow, "shrinking size and letting
scnprintf() instead of sprintf() to see whether truncation" is well enough.


> What other change should you make to test this in order to be sure that
> it will really work when someone tries it on 1024 CPUs?  (I am asking
> rather than telling because you really need to have this stuff worked
> out on your initial submission.)
> 

Hmm... Can it really work on 1024 CPUs? sorry I don't know. But in fact,
that is not about this patch (it is just one of case which may cause
issues).

This patch is only about "use truncation instead of memory overflow, and
make sure the modification without negative effect". So in my opinion,
current test case is enough for this requirement.

Maybe you want to let this module get additional test to find additional
issues? (If so, I will try when my time resource is possible).


> Another way of thinking about this is to ask yourself the question "If
> someone ran rcutorture with torture_type=srcu on a system with 1024 CPUs
> (to say nothing of 4096 CPUs), what would they want to happen?"  Then:
> "How would I test for that on a smaller system?"
> 

We have to face the efficiency: it is only a test module, if the tester
(assume he/she is a programmer, too) notices about the truncation, they
can simply extend the maximize length to avoid truncation.

At least for me, "rcutorture.c" is really easy enough for a programmer
to test rcu (and also, it is really a useful tool for test rcu).

And for "1024 CPUs",  I think one of efficient way is: "when some
members use it under 1024 CPUs, if they find something can be
improvement, they can report/send related patch".


Hmm... maybe the comment "it is ... additional test or consideration"
need be removed: 'additional' and 'consideration' are not suitable words
to be appeared in comments (they are not precise word).


Thanks.
-- 
Chen Gang
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [Suggestion] kernel/rcutorture.c: about using scnprintf() instead of sprintf().

2013-10-14 Thread Paul E. McKenney
On Mon, Oct 14, 2013 at 10:22:20AM +0800, Chen Gang wrote:
> On 10/14/2013 09:41 AM, Chen Gang wrote:
> > On 10/13/2013 07:05 PM, Paul E. McKenney wrote:
> >> On Tue, Oct 08, 2013 at 04:32:53PM +0800, Chen Gang wrote:
> >>> Hello Maintainers:
> >>>
> >>> In srcu_torture_stats(), if cpus are more than 1K, the PAGE_SIZE will
> >>> not be enough.
> >>>
> >>> In rcu_torture_printk(), the 'page' maximized size is 4096, it has a
> >>> function pointer for printing, which not tell its maximized length.
> >>>
> >>> Welcome any additional suggestions or completions.
> >>
> >> I never have run rcutorture on a system with that many CPUs.  ;-)
> > 
> > I guess most of members (include me), never have run under that case.

Indeed, there are not yet very many people with systems that large.

> >> Given that rcutorture is not used in production, my approach would be to
> >> fix this when I encountered it.  But what change would you suggest, and,
> >> more importantly, how would you go about testing it before submitting
> >> a patch?
> >>
> > 
> > OK, thanks, I will/should give a fix and test.
> > 
> > Hmm, In my opinion, we need:
> > 
> >  - let it pass LTP common simple test (so I can know how to test it).
> > 
> 
> Oh, after read "Documentation/RCU/torture.txt", it is a test module, so
> except related special test (e.g. shrink maximized buffer), it seems not
> need additional common test for it (e.g. LTP).

Yep!

> >  - intend to shrink maximized buffer (PAGE_SIZE -> 64, 256 ..) for test.

This is a good start!  However, you should also test the original kernel
to be sure that it really fails.  You should of course start by asking
yourself how you would expect it to fail.

What other change should you make to test this in order to be sure that
it will really work when someone tries it on 1024 CPUs?  (I am asking
rather than telling because you really need to have this stuff worked
out on your initial submission.)

Another way of thinking about this is to ask yourself the question "If
someone ran rcutorture with torture_type=srcu on a system with 1024 CPUs
(to say nothing of 4096 CPUs), what would they want to happen?"  Then:
"How would I test for that on a smaller system?"

> >  - read your original mail again (about testing contents) as reference.
> > 
> > Excuse me, I have to do some other things of company, so I will/should
> > try to finish it within this week (2013-10-20), if this time point is
> > not quite suitable, please let me know, thanks.
> > 
> >> Or if you are simply reporting this as a bug, please let me know that.
> > 
> > I will/should do: in q4 of 2013, I will/should spend part of my time
> > resources on testing.

OK, sounds good.

> > Welcome any additional suggestions or completions.

Please see above.

Thanx, Paul

> > Thanks.
> > 
> 
> 
> -- 
> Chen Gang
> 

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


Re: [Suggestion] kernel/rcutorture.c: about using scnprintf() instead of sprintf().

2013-10-14 Thread Paul E. McKenney
On Mon, Oct 14, 2013 at 10:22:20AM +0800, Chen Gang wrote:
 On 10/14/2013 09:41 AM, Chen Gang wrote:
  On 10/13/2013 07:05 PM, Paul E. McKenney wrote:
  On Tue, Oct 08, 2013 at 04:32:53PM +0800, Chen Gang wrote:
  Hello Maintainers:
 
  In srcu_torture_stats(), if cpus are more than 1K, the PAGE_SIZE will
  not be enough.
 
  In rcu_torture_printk(), the 'page' maximized size is 4096, it has a
  function pointer for printing, which not tell its maximized length.
 
  Welcome any additional suggestions or completions.
 
  I never have run rcutorture on a system with that many CPUs.  ;-)
  
  I guess most of members (include me), never have run under that case.

Indeed, there are not yet very many people with systems that large.

  Given that rcutorture is not used in production, my approach would be to
  fix this when I encountered it.  But what change would you suggest, and,
  more importantly, how would you go about testing it before submitting
  a patch?
 
  
  OK, thanks, I will/should give a fix and test.
  
  Hmm, In my opinion, we need:
  
   - let it pass LTP common simple test (so I can know how to test it).
  
 
 Oh, after read Documentation/RCU/torture.txt, it is a test module, so
 except related special test (e.g. shrink maximized buffer), it seems not
 need additional common test for it (e.g. LTP).

Yep!

   - intend to shrink maximized buffer (PAGE_SIZE - 64, 256 ..) for test.

This is a good start!  However, you should also test the original kernel
to be sure that it really fails.  You should of course start by asking
yourself how you would expect it to fail.

What other change should you make to test this in order to be sure that
it will really work when someone tries it on 1024 CPUs?  (I am asking
rather than telling because you really need to have this stuff worked
out on your initial submission.)

Another way of thinking about this is to ask yourself the question If
someone ran rcutorture with torture_type=srcu on a system with 1024 CPUs
(to say nothing of 4096 CPUs), what would they want to happen?  Then:
How would I test for that on a smaller system?

   - read your original mail again (about testing contents) as reference.
  
  Excuse me, I have to do some other things of company, so I will/should
  try to finish it within this week (2013-10-20), if this time point is
  not quite suitable, please let me know, thanks.
  
  Or if you are simply reporting this as a bug, please let me know that.
  
  I will/should do: in q4 of 2013, I will/should spend part of my time
  resources on testing.

OK, sounds good.

  Welcome any additional suggestions or completions.

Please see above.

Thanx, Paul

  Thanks.
  
 
 
 -- 
 Chen Gang
 

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


Re: [Suggestion] kernel/rcutorture.c: about using scnprintf() instead of sprintf().

2013-10-14 Thread Chen Gang
On 10/14/2013 07:24 PM, Paul E. McKenney wrote:
 On Mon, Oct 14, 2013 at 10:22:20AM +0800, Chen Gang wrote:
  - intend to shrink maximized buffer (PAGE_SIZE - 64, 256 ..) for test.
 
 This is a good start!  However, you should also test the original kernel
 to be sure that it really fails.  You should of course start by asking
 yourself how you would expect it to fail.
 

When shrink size, for scnprintf() we already got truncation, that means
if we still use sprintf(), it will cause memory overflow.

For memory overflow, it does not means OS will stop quickly, one of the
worst condition is it may still seems going 'well' but sometimes
may/will show some amazing things which is not easy to find root cause.

So for this kind of memory overflow, shrinking size and letting
scnprintf() instead of sprintf() to see whether truncation is well enough.


 What other change should you make to test this in order to be sure that
 it will really work when someone tries it on 1024 CPUs?  (I am asking
 rather than telling because you really need to have this stuff worked
 out on your initial submission.)
 

Hmm... Can it really work on 1024 CPUs? sorry I don't know. But in fact,
that is not about this patch (it is just one of case which may cause
issues).

This patch is only about use truncation instead of memory overflow, and
make sure the modification without negative effect. So in my opinion,
current test case is enough for this requirement.

Maybe you want to let this module get additional test to find additional
issues? (If so, I will try when my time resource is possible).


 Another way of thinking about this is to ask yourself the question If
 someone ran rcutorture with torture_type=srcu on a system with 1024 CPUs
 (to say nothing of 4096 CPUs), what would they want to happen?  Then:
 How would I test for that on a smaller system?
 

We have to face the efficiency: it is only a test module, if the tester
(assume he/she is a programmer, too) notices about the truncation, they
can simply extend the maximize length to avoid truncation.

At least for me, rcutorture.c is really easy enough for a programmer
to test rcu (and also, it is really a useful tool for test rcu).

And for 1024 CPUs,  I think one of efficient way is: when some
members use it under 1024 CPUs, if they find something can be
improvement, they can report/send related patch.


Hmm... maybe the comment it is ... additional test or consideration
need be removed: 'additional' and 'consideration' are not suitable words
to be appeared in comments (they are not precise word).


Thanks.
-- 
Chen Gang
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [Suggestion] kernel/rcutorture.c: about using scnprintf() instead of sprintf().

2013-10-13 Thread Chen Gang
On 10/14/2013 09:41 AM, Chen Gang wrote:
> On 10/13/2013 07:05 PM, Paul E. McKenney wrote:
>> On Tue, Oct 08, 2013 at 04:32:53PM +0800, Chen Gang wrote:
>>> Hello Maintainers:
>>>
>>> In srcu_torture_stats(), if cpus are more than 1K, the PAGE_SIZE will
>>> not be enough.
>>>
>>> In rcu_torture_printk(), the 'page' maximized size is 4096, it has a
>>> function pointer for printing, which not tell its maximized length.
>>>
>>> Welcome any additional suggestions or completions.
>>
>> I never have run rcutorture on a system with that many CPUs.  ;-)
>>
> 
> I guess most of members (include me), never have run under that case.
> 
> 
>> Given that rcutorture is not used in production, my approach would be to
>> fix this when I encountered it.  But what change would you suggest, and,
>> more importantly, how would you go about testing it before submitting
>> a patch?
>>
> 
> OK, thanks, I will/should give a fix and test.
> 
> Hmm, In my opinion, we need:
> 
>  - let it pass LTP common simple test (so I can know how to test it).
> 

Oh, after read "Documentation/RCU/torture.txt", it is a test module, so
except related special test (e.g. shrink maximized buffer), it seems not
need additional common test for it (e.g. LTP).

>  - intend to shrink maximized buffer (PAGE_SIZE -> 64, 256 ..) for test.
> 
>  - read your original mail again (about testing contents) as reference.
> 
> Excuse me, I have to do some other things of company, so I will/should
> try to finish it within this week (2013-10-20), if this time point is
> not quite suitable, please let me know, thanks.
> 
> 
>> Or if you are simply reporting this as a bug, please let me know that.
>>
> 
> I will/should do: in q4 of 2013, I will/should spend part of my time
> resources on testing.
> 
> 
> 
> Welcome any additional suggestions or completions.
> 
> Thanks.
> 


-- 
Chen Gang
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [Suggestion] kernel/rcutorture.c: about using scnprintf() instead of sprintf().

2013-10-13 Thread Chen Gang
On 10/13/2013 07:05 PM, Paul E. McKenney wrote:
> On Tue, Oct 08, 2013 at 04:32:53PM +0800, Chen Gang wrote:
>> Hello Maintainers:
>>
>> In srcu_torture_stats(), if cpus are more than 1K, the PAGE_SIZE will
>> not be enough.
>>
>> In rcu_torture_printk(), the 'page' maximized size is 4096, it has a
>> function pointer for printing, which not tell its maximized length.
>>
>> Welcome any additional suggestions or completions.
> 
> I never have run rcutorture on a system with that many CPUs.  ;-)
> 

I guess most of members (include me), never have run under that case.


> Given that rcutorture is not used in production, my approach would be to
> fix this when I encountered it.  But what change would you suggest, and,
> more importantly, how would you go about testing it before submitting
> a patch?
> 

OK, thanks, I will/should give a fix and test.

Hmm, In my opinion, we need:

 - let it pass LTP common simple test (so I can know how to test it).

 - intend to shrink maximized buffer (PAGE_SIZE -> 64, 256 ..) for test.

 - read your original mail again (about testing contents) as reference.

Excuse me, I have to do some other things of company, so I will/should
try to finish it within this week (2013-10-20), if this time point is
not quite suitable, please let me know, thanks.


> Or if you are simply reporting this as a bug, please let me know that.
> 

I will/should do: in q4 of 2013, I will/should spend part of my time
resources on testing.



Welcome any additional suggestions or completions.

Thanks.
-- 
Chen Gang
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [Suggestion] kernel/rcutorture.c: about using scnprintf() instead of sprintf().

2013-10-13 Thread Paul E. McKenney
On Tue, Oct 08, 2013 at 04:32:53PM +0800, Chen Gang wrote:
> Hello Maintainers:
> 
> In srcu_torture_stats(), if cpus are more than 1K, the PAGE_SIZE will
> not be enough.
> 
> In rcu_torture_printk(), the 'page' maximized size is 4096, it has a
> function pointer for printing, which not tell its maximized length.
> 
> Welcome any additional suggestions or completions.

I never have run rcutorture on a system with that many CPUs.  ;-)

Given that rcutorture is not used in production, my approach would be to
fix this when I encountered it.  But what change would you suggest, and,
more importantly, how would you go about testing it before submitting
a patch?

Or if you are simply reporting this as a bug, please let me know that.

Thanx, Paul

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


Re: [Suggestion] kernel/rcutorture.c: about using scnprintf() instead of sprintf().

2013-10-13 Thread Paul E. McKenney
On Tue, Oct 08, 2013 at 04:32:53PM +0800, Chen Gang wrote:
 Hello Maintainers:
 
 In srcu_torture_stats(), if cpus are more than 1K, the PAGE_SIZE will
 not be enough.
 
 In rcu_torture_printk(), the 'page' maximized size is 4096, it has a
 function pointer for printing, which not tell its maximized length.
 
 Welcome any additional suggestions or completions.

I never have run rcutorture on a system with that many CPUs.  ;-)

Given that rcutorture is not used in production, my approach would be to
fix this when I encountered it.  But what change would you suggest, and,
more importantly, how would you go about testing it before submitting
a patch?

Or if you are simply reporting this as a bug, please let me know that.

Thanx, Paul

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


Re: [Suggestion] kernel/rcutorture.c: about using scnprintf() instead of sprintf().

2013-10-13 Thread Chen Gang
On 10/13/2013 07:05 PM, Paul E. McKenney wrote:
 On Tue, Oct 08, 2013 at 04:32:53PM +0800, Chen Gang wrote:
 Hello Maintainers:

 In srcu_torture_stats(), if cpus are more than 1K, the PAGE_SIZE will
 not be enough.

 In rcu_torture_printk(), the 'page' maximized size is 4096, it has a
 function pointer for printing, which not tell its maximized length.

 Welcome any additional suggestions or completions.
 
 I never have run rcutorture on a system with that many CPUs.  ;-)
 

I guess most of members (include me), never have run under that case.


 Given that rcutorture is not used in production, my approach would be to
 fix this when I encountered it.  But what change would you suggest, and,
 more importantly, how would you go about testing it before submitting
 a patch?
 

OK, thanks, I will/should give a fix and test.

Hmm, In my opinion, we need:

 - let it pass LTP common simple test (so I can know how to test it).

 - intend to shrink maximized buffer (PAGE_SIZE - 64, 256 ..) for test.

 - read your original mail again (about testing contents) as reference.

Excuse me, I have to do some other things of company, so I will/should
try to finish it within this week (2013-10-20), if this time point is
not quite suitable, please let me know, thanks.


 Or if you are simply reporting this as a bug, please let me know that.
 

I will/should do: in q4 of 2013, I will/should spend part of my time
resources on testing.



Welcome any additional suggestions or completions.

Thanks.
-- 
Chen Gang
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [Suggestion] kernel/rcutorture.c: about using scnprintf() instead of sprintf().

2013-10-13 Thread Chen Gang
On 10/14/2013 09:41 AM, Chen Gang wrote:
 On 10/13/2013 07:05 PM, Paul E. McKenney wrote:
 On Tue, Oct 08, 2013 at 04:32:53PM +0800, Chen Gang wrote:
 Hello Maintainers:

 In srcu_torture_stats(), if cpus are more than 1K, the PAGE_SIZE will
 not be enough.

 In rcu_torture_printk(), the 'page' maximized size is 4096, it has a
 function pointer for printing, which not tell its maximized length.

 Welcome any additional suggestions or completions.

 I never have run rcutorture on a system with that many CPUs.  ;-)

 
 I guess most of members (include me), never have run under that case.
 
 
 Given that rcutorture is not used in production, my approach would be to
 fix this when I encountered it.  But what change would you suggest, and,
 more importantly, how would you go about testing it before submitting
 a patch?

 
 OK, thanks, I will/should give a fix and test.
 
 Hmm, In my opinion, we need:
 
  - let it pass LTP common simple test (so I can know how to test it).
 

Oh, after read Documentation/RCU/torture.txt, it is a test module, so
except related special test (e.g. shrink maximized buffer), it seems not
need additional common test for it (e.g. LTP).

  - intend to shrink maximized buffer (PAGE_SIZE - 64, 256 ..) for test.
 
  - read your original mail again (about testing contents) as reference.
 
 Excuse me, I have to do some other things of company, so I will/should
 try to finish it within this week (2013-10-20), if this time point is
 not quite suitable, please let me know, thanks.
 
 
 Or if you are simply reporting this as a bug, please let me know that.

 
 I will/should do: in q4 of 2013, I will/should spend part of my time
 resources on testing.
 
 
 
 Welcome any additional suggestions or completions.
 
 Thanks.
 


-- 
Chen Gang
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[Suggestion] kernel/rcutorture.c: about using scnprintf() instead of sprintf().

2013-10-08 Thread Chen Gang
Hello Maintainers:

In srcu_torture_stats(), if cpus are more than 1K, the PAGE_SIZE will
not be enough.

In rcu_torture_printk(), the 'page' maximized size is 4096, it has a
function pointer for printing, which not tell its maximized length.


Welcome any additional suggestions or completions.


Thanks.
-- 
Chen Gang
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[Suggestion] kernel/rcutorture.c: about using scnprintf() instead of sprintf().

2013-10-08 Thread Chen Gang
Hello Maintainers:

In srcu_torture_stats(), if cpus are more than 1K, the PAGE_SIZE will
not be enough.

In rcu_torture_printk(), the 'page' maximized size is 4096, it has a
function pointer for printing, which not tell its maximized length.


Welcome any additional suggestions or completions.


Thanks.
-- 
Chen Gang
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[Suggestion] kernel/irqdesc.c: not checking whether failure occurs for alloc_desc() in early_irq_init().

2013-05-14 Thread Chen Gang
Hello Maintainers:

For early_irq_init(), it may be failure (can return an error code), and
for alloc_desc(), also may return 'NULL' when '-ENOMEM'.

But it seems the authors are sure that early_irq_init() are always
success, is it correct ? (or early_irq_init() will never be used ?)

I think, it is better to also check alloc_desc() return value in
early_irq_init().

Please help check when you have time.


Thanks.
-- 
Chen Gang

Asianux Corporation
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[Suggestion] kernel/irqdesc.c: not checking whether failure occurs for alloc_desc() in early_irq_init().

2013-05-14 Thread Chen Gang
Hello Maintainers:

For early_irq_init(), it may be failure (can return an error code), and
for alloc_desc(), also may return 'NULL' when '-ENOMEM'.

But it seems the authors are sure that early_irq_init() are always
success, is it correct ? (or early_irq_init() will never be used ?)

I think, it is better to also check alloc_desc() return value in
early_irq_init().

Please help check when you have time.


Thanks.
-- 
Chen Gang

Asianux Corporation
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [Suggestion] kernel/cgroup.c: about kfree after 'get_new_cssid'

2013-05-07 Thread Chen Gang
On 2013年05月08日 08:50, Li Zefan wrote:
>>> There's a bug in cgroup_unload_subsys() that idr_destroy() should be called 
>>> after
>>> >> ss->css_free(). That said, given there's no modular cgroup subsystem 
>>> >> using css_id,
>>> >> and the whole css_id thing will be eliminated in 3.11, why bother fixing 
>>> >> it.
>>> >>
>> > 
>> > I just find it by reading code (I also want to learn about kernel).
>> > 
>> > I guess, for some stable versions, may focus on it, they are the
>> > different branches from the latest version.
>> > 
>> > So, is it suitable to send related patch for the bug ?
>> > 
> As I said, there's no modular cgroup subsystem using css_id, so the bug
> doesn't exist in real world.
> 
> 
> 

Excuse me, my English is not quite well, I guess what you said is:

  a. idr_destroy() should be called after ss->css_free()
  b. need check the return value of cgroup_init_idr() in cgroup_init().

  but all features which related with the 2 issues above, never be used
in any main linux main branches.
  so, the 2 bugs doesn't exist in real world.

Is what I guess correct ?

Thanks.

-- 
Chen Gang

Asianux Corporation
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [Suggestion] kernel/cgroup.c: about kfree after 'get_new_cssid'

2013-05-07 Thread Li Zefan
>> There's a bug in cgroup_unload_subsys() that idr_destroy() should be called 
>> after
>> ss->css_free(). That said, given there's no modular cgroup subsystem using 
>> css_id,
>> and the whole css_id thing will be eliminated in 3.11, why bother fixing it.
>>
> 
> I just find it by reading code (I also want to learn about kernel).
> 
> I guess, for some stable versions, may focus on it, they are the
> different branches from the latest version.
> 
> So, is it suitable to send related patch for the bug ?
> 

As I said, there's no modular cgroup subsystem using css_id, so the bug
doesn't exist in real world.

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


Re: [Suggestion] kernel/cgroup.c: about kfree after 'get_new_cssid'

2013-05-07 Thread Chen Gang
On 2013年05月07日 19:01, Li Zefan wrote:
> On 2013/5/7 18:46, Chen Gang wrote:
>> Hello Maintainers:
>>
>> After call get_new_cssid(), I can not find the related free function
>> (it seems free_css_id() is for that, but not used).
>>
>> The memory location is:
>>   get_new_cssid() --> kzalloc() for 'struct css_id'
>>   get_new_cssid() --> idr_alloc() for 'ss->idr'
>>
>> One work flow:
>>   cgroup_load_subsys() --> cgroup_init_idr() --> get_new_cssid()
>>   when get_new_cssid() fails, it will:
>>   cgroup_load_subsys() --> cgroup_unload_subsys() --> idr_destroy(),
>>   and also:
>>   cgroup_load_subsys() --> cgroup_unload_subsys() --> ss->css_free();
>> ('css_free' may 'debug_css_free', or 'freezer_css_free' ...)
>>
>> It seems the work flow above is not 'kfree' 'struct css_id', is it true?
>>
>> BTW: I also guess, for cgroup_init_idr() in cgroup_init(), need check
>> the return value.
>>
>> Please help check.
>>
> 
> It's the specific cgroup subsystem that calls free_css_id() in it's 
> subsys->css_free()
> callback. See __mem_cgroup_free() for example.
> 

OK, thank you for your confirmation.

> There's a bug in cgroup_unload_subsys() that idr_destroy() should be called 
> after
> ss->css_free(). That said, given there's no modular cgroup subsystem using 
> css_id,
> and the whole css_id thing will be eliminated in 3.11, why bother fixing it.
> 

I just find it by reading code (I also want to learn about kernel).

I guess, for some stable versions, may focus on it, they are the
different branches from the latest version.

So, is it suitable to send related patch for the bug ?

Thanks.

-- 
Chen Gang

Asianux Corporation
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [Suggestion] kernel/cgroup.c: about kfree after 'get_new_cssid'

2013-05-07 Thread Li Zefan
On 2013/5/7 18:46, Chen Gang wrote:
> Hello Maintainers:
> 
> After call get_new_cssid(), I can not find the related free function
> (it seems free_css_id() is for that, but not used).
> 
> The memory location is:
>   get_new_cssid() --> kzalloc() for 'struct css_id'
>   get_new_cssid() --> idr_alloc() for 'ss->idr'
> 
> One work flow:
>   cgroup_load_subsys() --> cgroup_init_idr() --> get_new_cssid()
>   when get_new_cssid() fails, it will:
>   cgroup_load_subsys() --> cgroup_unload_subsys() --> idr_destroy(),
>   and also:
>   cgroup_load_subsys() --> cgroup_unload_subsys() --> ss->css_free();
> ('css_free' may 'debug_css_free', or 'freezer_css_free' ...)
> 
> It seems the work flow above is not 'kfree' 'struct css_id', is it true?
> 
> BTW: I also guess, for cgroup_init_idr() in cgroup_init(), need check
> the return value.
> 
> Please help check.
> 

It's the specific cgroup subsystem that calls free_css_id() in it's 
subsys->css_free()
callback. See __mem_cgroup_free() for example.

There's a bug in cgroup_unload_subsys() that idr_destroy() should be called 
after
ss->css_free(). That said, given there's no modular cgroup subsystem using 
css_id,
and the whole css_id thing will be eliminated in 3.11, why bother fixing it.

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


[Suggestion] kernel/cgroup.c: about kfree after 'get_new_cssid'

2013-05-07 Thread Chen Gang
Hello Maintainers:

After call get_new_cssid(), I can not find the related free function
(it seems free_css_id() is for that, but not used).

The memory location is:
  get_new_cssid() --> kzalloc() for 'struct css_id'
  get_new_cssid() --> idr_alloc() for 'ss->idr'

One work flow:
  cgroup_load_subsys() --> cgroup_init_idr() --> get_new_cssid()
  when get_new_cssid() fails, it will:
  cgroup_load_subsys() --> cgroup_unload_subsys() --> idr_destroy(),
  and also:
  cgroup_load_subsys() --> cgroup_unload_subsys() --> ss->css_free();
('css_free' may 'debug_css_free', or 'freezer_css_free' ...)

It seems the work flow above is not 'kfree' 'struct css_id', is it true?

BTW: I also guess, for cgroup_init_idr() in cgroup_init(), need check
the return value.

Please help check.


Thanks.

--
 Chen Gang

 Asianux Corporation
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[Suggestion] kernel/cgroup.c: about kfree after 'get_new_cssid'

2013-05-07 Thread Chen Gang
Hello Maintainers:

After call get_new_cssid(), I can not find the related free function
(it seems free_css_id() is for that, but not used).

The memory location is:
  get_new_cssid() -- kzalloc() for 'struct css_id'
  get_new_cssid() -- idr_alloc() for 'ss-idr'

One work flow:
  cgroup_load_subsys() -- cgroup_init_idr() -- get_new_cssid()
  when get_new_cssid() fails, it will:
  cgroup_load_subsys() -- cgroup_unload_subsys() -- idr_destroy(),
  and also:
  cgroup_load_subsys() -- cgroup_unload_subsys() -- ss-css_free();
('css_free' may 'debug_css_free', or 'freezer_css_free' ...)

It seems the work flow above is not 'kfree' 'struct css_id', is it true?

BTW: I also guess, for cgroup_init_idr() in cgroup_init(), need check
the return value.

Please help check.


Thanks.

--
 Chen Gang

 Asianux Corporation
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [Suggestion] kernel/cgroup.c: about kfree after 'get_new_cssid'

2013-05-07 Thread Li Zefan
On 2013/5/7 18:46, Chen Gang wrote:
 Hello Maintainers:
 
 After call get_new_cssid(), I can not find the related free function
 (it seems free_css_id() is for that, but not used).
 
 The memory location is:
   get_new_cssid() -- kzalloc() for 'struct css_id'
   get_new_cssid() -- idr_alloc() for 'ss-idr'
 
 One work flow:
   cgroup_load_subsys() -- cgroup_init_idr() -- get_new_cssid()
   when get_new_cssid() fails, it will:
   cgroup_load_subsys() -- cgroup_unload_subsys() -- idr_destroy(),
   and also:
   cgroup_load_subsys() -- cgroup_unload_subsys() -- ss-css_free();
 ('css_free' may 'debug_css_free', or 'freezer_css_free' ...)
 
 It seems the work flow above is not 'kfree' 'struct css_id', is it true?
 
 BTW: I also guess, for cgroup_init_idr() in cgroup_init(), need check
 the return value.
 
 Please help check.
 

It's the specific cgroup subsystem that calls free_css_id() in it's 
subsys-css_free()
callback. See __mem_cgroup_free() for example.

There's a bug in cgroup_unload_subsys() that idr_destroy() should be called 
after
ss-css_free(). That said, given there's no modular cgroup subsystem using 
css_id,
and the whole css_id thing will be eliminated in 3.11, why bother fixing it.

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


Re: [Suggestion] kernel/cgroup.c: about kfree after 'get_new_cssid'

2013-05-07 Thread Chen Gang
On 2013年05月07日 19:01, Li Zefan wrote:
 On 2013/5/7 18:46, Chen Gang wrote:
 Hello Maintainers:

 After call get_new_cssid(), I can not find the related free function
 (it seems free_css_id() is for that, but not used).

 The memory location is:
   get_new_cssid() -- kzalloc() for 'struct css_id'
   get_new_cssid() -- idr_alloc() for 'ss-idr'

 One work flow:
   cgroup_load_subsys() -- cgroup_init_idr() -- get_new_cssid()
   when get_new_cssid() fails, it will:
   cgroup_load_subsys() -- cgroup_unload_subsys() -- idr_destroy(),
   and also:
   cgroup_load_subsys() -- cgroup_unload_subsys() -- ss-css_free();
 ('css_free' may 'debug_css_free', or 'freezer_css_free' ...)

 It seems the work flow above is not 'kfree' 'struct css_id', is it true?

 BTW: I also guess, for cgroup_init_idr() in cgroup_init(), need check
 the return value.

 Please help check.

 
 It's the specific cgroup subsystem that calls free_css_id() in it's 
 subsys-css_free()
 callback. See __mem_cgroup_free() for example.
 

OK, thank you for your confirmation.

 There's a bug in cgroup_unload_subsys() that idr_destroy() should be called 
 after
 ss-css_free(). That said, given there's no modular cgroup subsystem using 
 css_id,
 and the whole css_id thing will be eliminated in 3.11, why bother fixing it.
 

I just find it by reading code (I also want to learn about kernel).

I guess, for some stable versions, may focus on it, they are the
different branches from the latest version.

So, is it suitable to send related patch for the bug ?

Thanks.

-- 
Chen Gang

Asianux Corporation
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [Suggestion] kernel/cgroup.c: about kfree after 'get_new_cssid'

2013-05-07 Thread Li Zefan
 There's a bug in cgroup_unload_subsys() that idr_destroy() should be called 
 after
 ss-css_free(). That said, given there's no modular cgroup subsystem using 
 css_id,
 and the whole css_id thing will be eliminated in 3.11, why bother fixing it.

 
 I just find it by reading code (I also want to learn about kernel).
 
 I guess, for some stable versions, may focus on it, they are the
 different branches from the latest version.
 
 So, is it suitable to send related patch for the bug ?
 

As I said, there's no modular cgroup subsystem using css_id, so the bug
doesn't exist in real world.

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


Re: [Suggestion] kernel/cgroup.c: about kfree after 'get_new_cssid'

2013-05-07 Thread Chen Gang
On 2013年05月08日 08:50, Li Zefan wrote:
 There's a bug in cgroup_unload_subsys() that idr_destroy() should be called 
 after
  ss-css_free(). That said, given there's no modular cgroup subsystem 
  using css_id,
  and the whole css_id thing will be eliminated in 3.11, why bother fixing 
  it.
 
  
  I just find it by reading code (I also want to learn about kernel).
  
  I guess, for some stable versions, may focus on it, they are the
  different branches from the latest version.
  
  So, is it suitable to send related patch for the bug ?
  
 As I said, there's no modular cgroup subsystem using css_id, so the bug
 doesn't exist in real world.
 
 
 

Excuse me, my English is not quite well, I guess what you said is:

  a. idr_destroy() should be called after ss-css_free()
  b. need check the return value of cgroup_init_idr() in cgroup_init().

  but all features which related with the 2 issues above, never be used
in any main linux main branches.
  so, the 2 bugs doesn't exist in real world.

Is what I guess correct ?

Thanks.

-- 
Chen Gang

Asianux Corporation
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [Suggestion] kernel: 'now' may be used uninitialized in posix_cpu_timer_schedule function

2013-03-26 Thread Chen Gang
On 2013年03月26日 20:27, Frederic Weisbecker wrote:
> 2013/3/26 Chen Gang :
>> > Hello Maintainers:
>> >
>> >   compiling with EXTRA_CFLAGS=-W:
>> > make V=1 EXTRA_CFLAGS=-W ARCH=arm s3c2410_defconfig
>> > make V=1 EXTRA_CFLAGS=-W ARCH=arm menuconfig
>> >   set 'arm-linux-gnu-' for cross chain prefix
>> > make V=1 EXTRA_CFLAGS=-W ARCH=arm
>> >
>> >   it will report:
>> > kernel/posix-cpu-timers.c:1065:19: warning: �now� may be used 
>> > uninitialized in this function [-Wuninitialized]
>> >
>> >   it seems it is really a bug.
>> > can any member help to fix it ?
>> > or provide additional suggestion ?
>> >   (it seems only "unsigned long long now = 0" is not enough).
> Yeah it's missing a call to cpu_timer_sample_group() before
> clear_dead_task(). Andrew Morton reported the warning and I have a
> pending patch to fix that. I'm just checking a few other things before
> sending it. These clear_dead_task() calls seem to also conflict with
> cleanup_timers(). I'm fixing that too.
> 
> Thanks for your report!
> 
> 

  thank you, too.

  :-)

-- 
Chen Gang

Asianux Corporation
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [Suggestion] kernel: 'now' may be used uninitialized in posix_cpu_timer_schedule function

2013-03-26 Thread Frederic Weisbecker
2013/3/26 Chen Gang :
> Hello Maintainers:
>
>   compiling with EXTRA_CFLAGS=-W:
> make V=1 EXTRA_CFLAGS=-W ARCH=arm s3c2410_defconfig
> make V=1 EXTRA_CFLAGS=-W ARCH=arm menuconfig
>   set 'arm-linux-gnu-' for cross chain prefix
> make V=1 EXTRA_CFLAGS=-W ARCH=arm
>
>   it will report:
> kernel/posix-cpu-timers.c:1065:19: warning: ‘now’ may be used 
> uninitialized in this function [-Wuninitialized]
>
>   it seems it is really a bug.
> can any member help to fix it ?
> or provide additional suggestion ?
>   (it seems only "unsigned long long now = 0" is not enough).

Yeah it's missing a call to cpu_timer_sample_group() before
clear_dead_task(). Andrew Morton reported the warning and I have a
pending patch to fix that. I'm just checking a few other things before
sending it. These clear_dead_task() calls seem to also conflict with
cleanup_timers(). I'm fixing that too.

Thanks for your report!
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [Suggestion] kernel: 'now' may be used uninitialized in posix_cpu_timer_schedule function

2013-03-26 Thread Chen Gang

 oh, sorry, it seems better to let ARM folks know about it.

 ;-)


On 2013年03月26日 14:36, Chen Gang wrote:
> Hello Maintainers:
> 
>   compiling with EXTRA_CFLAGS=-W:
> make V=1 EXTRA_CFLAGS=-W ARCH=arm s3c2410_defconfig
> make V=1 EXTRA_CFLAGS=-W ARCH=arm menuconfig
>   set 'arm-linux-gnu-' for cross chain prefix
> make V=1 EXTRA_CFLAGS=-W ARCH=arm
> 
>   it will report:
> kernel/posix-cpu-timers.c:1065:19: warning: ‘now’ may be used 
> uninitialized in this function [-Wuninitialized]
> 
>   it seems it is really a bug.
> can any member help to fix it ?
> or provide additional suggestion ?
>   (it seems only "unsigned long long now = 0" is not enough).
> 
>   :-)
> 
> 
> 
> in kernel/posix-cpu-timers.c:
>   for variable 'now' is defined without initialization (line 1029)
>   it may be used without initialization (line 1066)
> 
> 
> 1026 void posix_cpu_timer_schedule(struct k_itimer *timer)
> 1027 {
> 1028 struct task_struct *p = timer->it.cpu.task;
> 1029 unsigned long long now;
> 1030 
> 1031 if (unlikely(p == NULL))
> 1032 /*
> 1033  * The task was cleaned up already, no future firings.
> 1034  */
> 1035 goto out;
> 1036 
> 1037 /*
> 1038  * Fetch the current sample and update the timer's expiry time.
> 1039  */
> 1040 if (CPUCLOCK_PERTHREAD(timer->it_clock)) {
> 1041 cpu_clock_sample(timer->it_clock, p, );
> 1042 bump_cpu_timer(timer, now);
> 1043 if (unlikely(p->exit_state)) {
> 1044 clear_dead_task(timer, now);
> 1045 goto out;
> 1046 }
> 1047 read_lock(_lock); /* arm_timer needs it.  */
> 1048 spin_lock(>sighand->siglock);
> 1049 } else {
> 1050 read_lock(_lock);
> 1051 if (unlikely(p->sighand == NULL)) {
> 1052 /*
> 1053  * The process has been reaped.
> 1054  * We can't even collect a sample any more.
> 1055  */
> 1056 put_task_struct(p);
> 1057 timer->it.cpu.task = p = NULL;
> 1058 timer->it.cpu.expires = 0;
> 1059 goto out_unlock;
> 1060 } else if (unlikely(p->exit_state) && 
> thread_group_empty(p)) {
> 1061 /*
> 1062  * We've noticed that the thread is dead, but
> 1063  * not yet reaped.  Take this opportunity to
> 1064  * drop our task ref.
> 1065  */
> 1066 clear_dead_task(timer, now);
> 1067 goto out_unlock;
> 1068 }
> 1069 spin_lock(>sighand->siglock);
> 1070 cpu_timer_sample_group(timer->it_clock, p, );
> 1071 bump_cpu_timer(timer, now);
> 1072 /* Leave the tasklist_lock locked for the call below.  */
> 1073 }
> 1074 
> 1075 /*
> 1076  * Now re-arm for the new expiry time.
> 1077  */
> 1078 BUG_ON(!irqs_disabled());
> 1079 arm_timer(timer);
> 1080 spin_unlock(>sighand->siglock);
> 1081 
> 1082 out_unlock:
> 1083 read_unlock(_lock);
> 1084 
> 1085 out:
> 1086 timer->it_overrun_last = timer->it_overrun;
> 1087 timer->it_overrun = -1;
> 1088 ++timer->it_requeue_pending;
> 1089 }
> 


-- 
Chen Gang

Asianux Corporation
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[Suggestion] kernel: 'now' may be used uninitialized in posix_cpu_timer_schedule function

2013-03-26 Thread Chen Gang
Hello Maintainers:

  compiling with EXTRA_CFLAGS=-W:
make V=1 EXTRA_CFLAGS=-W ARCH=arm s3c2410_defconfig
make V=1 EXTRA_CFLAGS=-W ARCH=arm menuconfig
  set 'arm-linux-gnu-' for cross chain prefix
make V=1 EXTRA_CFLAGS=-W ARCH=arm

  it will report:
kernel/posix-cpu-timers.c:1065:19: warning: ‘now’ may be used uninitialized 
in this function [-Wuninitialized]

  it seems it is really a bug.
can any member help to fix it ?
or provide additional suggestion ?
  (it seems only "unsigned long long now = 0" is not enough).

  :-)



in kernel/posix-cpu-timers.c:
  for variable 'now' is defined without initialization (line 1029)
  it may be used without initialization (line 1066)


1026 void posix_cpu_timer_schedule(struct k_itimer *timer)
1027 {
1028 struct task_struct *p = timer->it.cpu.task;
1029 unsigned long long now;
1030 
1031 if (unlikely(p == NULL))
1032 /*
1033  * The task was cleaned up already, no future firings.
1034  */
1035 goto out;
1036 
1037 /*
1038  * Fetch the current sample and update the timer's expiry time.
1039  */
1040 if (CPUCLOCK_PERTHREAD(timer->it_clock)) {
1041 cpu_clock_sample(timer->it_clock, p, );
1042 bump_cpu_timer(timer, now);
1043 if (unlikely(p->exit_state)) {
1044 clear_dead_task(timer, now);
1045 goto out;
1046 }
1047 read_lock(_lock); /* arm_timer needs it.  */
1048 spin_lock(>sighand->siglock);
1049 } else {
1050 read_lock(_lock);
1051 if (unlikely(p->sighand == NULL)) {
1052 /*
1053  * The process has been reaped.
1054  * We can't even collect a sample any more.
1055  */
1056 put_task_struct(p);
1057 timer->it.cpu.task = p = NULL;
1058 timer->it.cpu.expires = 0;
1059 goto out_unlock;
1060 } else if (unlikely(p->exit_state) && 
thread_group_empty(p)) {
1061 /*
1062  * We've noticed that the thread is dead, but
1063  * not yet reaped.  Take this opportunity to
1064  * drop our task ref.
1065  */
1066 clear_dead_task(timer, now);
1067 goto out_unlock;
1068 }
1069 spin_lock(>sighand->siglock);
1070 cpu_timer_sample_group(timer->it_clock, p, );
1071 bump_cpu_timer(timer, now);
1072 /* Leave the tasklist_lock locked for the call below.  */
1073 }
1074 
1075 /*
1076  * Now re-arm for the new expiry time.
1077  */
1078 BUG_ON(!irqs_disabled());
1079 arm_timer(timer);
1080 spin_unlock(>sighand->siglock);
1081 
1082 out_unlock:
1083 read_unlock(_lock);
1084 
1085 out:
1086 timer->it_overrun_last = timer->it_overrun;
1087 timer->it_overrun = -1;
1088 ++timer->it_requeue_pending;
1089 }

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


[Suggestion] kernel: 'now' may be used uninitialized in posix_cpu_timer_schedule function

2013-03-26 Thread Chen Gang
Hello Maintainers:

  compiling with EXTRA_CFLAGS=-W:
make V=1 EXTRA_CFLAGS=-W ARCH=arm s3c2410_defconfig
make V=1 EXTRA_CFLAGS=-W ARCH=arm menuconfig
  set 'arm-linux-gnu-' for cross chain prefix
make V=1 EXTRA_CFLAGS=-W ARCH=arm

  it will report:
kernel/posix-cpu-timers.c:1065:19: warning: ‘now’ may be used uninitialized 
in this function [-Wuninitialized]

  it seems it is really a bug.
can any member help to fix it ?
or provide additional suggestion ?
  (it seems only unsigned long long now = 0 is not enough).

  :-)



in kernel/posix-cpu-timers.c:
  for variable 'now' is defined without initialization (line 1029)
  it may be used without initialization (line 1066)


1026 void posix_cpu_timer_schedule(struct k_itimer *timer)
1027 {
1028 struct task_struct *p = timer-it.cpu.task;
1029 unsigned long long now;
1030 
1031 if (unlikely(p == NULL))
1032 /*
1033  * The task was cleaned up already, no future firings.
1034  */
1035 goto out;
1036 
1037 /*
1038  * Fetch the current sample and update the timer's expiry time.
1039  */
1040 if (CPUCLOCK_PERTHREAD(timer-it_clock)) {
1041 cpu_clock_sample(timer-it_clock, p, now);
1042 bump_cpu_timer(timer, now);
1043 if (unlikely(p-exit_state)) {
1044 clear_dead_task(timer, now);
1045 goto out;
1046 }
1047 read_lock(tasklist_lock); /* arm_timer needs it.  */
1048 spin_lock(p-sighand-siglock);
1049 } else {
1050 read_lock(tasklist_lock);
1051 if (unlikely(p-sighand == NULL)) {
1052 /*
1053  * The process has been reaped.
1054  * We can't even collect a sample any more.
1055  */
1056 put_task_struct(p);
1057 timer-it.cpu.task = p = NULL;
1058 timer-it.cpu.expires = 0;
1059 goto out_unlock;
1060 } else if (unlikely(p-exit_state)  
thread_group_empty(p)) {
1061 /*
1062  * We've noticed that the thread is dead, but
1063  * not yet reaped.  Take this opportunity to
1064  * drop our task ref.
1065  */
1066 clear_dead_task(timer, now);
1067 goto out_unlock;
1068 }
1069 spin_lock(p-sighand-siglock);
1070 cpu_timer_sample_group(timer-it_clock, p, now);
1071 bump_cpu_timer(timer, now);
1072 /* Leave the tasklist_lock locked for the call below.  */
1073 }
1074 
1075 /*
1076  * Now re-arm for the new expiry time.
1077  */
1078 BUG_ON(!irqs_disabled());
1079 arm_timer(timer);
1080 spin_unlock(p-sighand-siglock);
1081 
1082 out_unlock:
1083 read_unlock(tasklist_lock);
1084 
1085 out:
1086 timer-it_overrun_last = timer-it_overrun;
1087 timer-it_overrun = -1;
1088 ++timer-it_requeue_pending;
1089 }

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


Re: [Suggestion] kernel: 'now' may be used uninitialized in posix_cpu_timer_schedule function

2013-03-26 Thread Chen Gang

 oh, sorry, it seems better to let ARM folks know about it.

 ;-)


On 2013年03月26日 14:36, Chen Gang wrote:
 Hello Maintainers:
 
   compiling with EXTRA_CFLAGS=-W:
 make V=1 EXTRA_CFLAGS=-W ARCH=arm s3c2410_defconfig
 make V=1 EXTRA_CFLAGS=-W ARCH=arm menuconfig
   set 'arm-linux-gnu-' for cross chain prefix
 make V=1 EXTRA_CFLAGS=-W ARCH=arm
 
   it will report:
 kernel/posix-cpu-timers.c:1065:19: warning: ‘now’ may be used 
 uninitialized in this function [-Wuninitialized]
 
   it seems it is really a bug.
 can any member help to fix it ?
 or provide additional suggestion ?
   (it seems only unsigned long long now = 0 is not enough).
 
   :-)
 
 
 
 in kernel/posix-cpu-timers.c:
   for variable 'now' is defined without initialization (line 1029)
   it may be used without initialization (line 1066)
 
 
 1026 void posix_cpu_timer_schedule(struct k_itimer *timer)
 1027 {
 1028 struct task_struct *p = timer-it.cpu.task;
 1029 unsigned long long now;
 1030 
 1031 if (unlikely(p == NULL))
 1032 /*
 1033  * The task was cleaned up already, no future firings.
 1034  */
 1035 goto out;
 1036 
 1037 /*
 1038  * Fetch the current sample and update the timer's expiry time.
 1039  */
 1040 if (CPUCLOCK_PERTHREAD(timer-it_clock)) {
 1041 cpu_clock_sample(timer-it_clock, p, now);
 1042 bump_cpu_timer(timer, now);
 1043 if (unlikely(p-exit_state)) {
 1044 clear_dead_task(timer, now);
 1045 goto out;
 1046 }
 1047 read_lock(tasklist_lock); /* arm_timer needs it.  */
 1048 spin_lock(p-sighand-siglock);
 1049 } else {
 1050 read_lock(tasklist_lock);
 1051 if (unlikely(p-sighand == NULL)) {
 1052 /*
 1053  * The process has been reaped.
 1054  * We can't even collect a sample any more.
 1055  */
 1056 put_task_struct(p);
 1057 timer-it.cpu.task = p = NULL;
 1058 timer-it.cpu.expires = 0;
 1059 goto out_unlock;
 1060 } else if (unlikely(p-exit_state)  
 thread_group_empty(p)) {
 1061 /*
 1062  * We've noticed that the thread is dead, but
 1063  * not yet reaped.  Take this opportunity to
 1064  * drop our task ref.
 1065  */
 1066 clear_dead_task(timer, now);
 1067 goto out_unlock;
 1068 }
 1069 spin_lock(p-sighand-siglock);
 1070 cpu_timer_sample_group(timer-it_clock, p, now);
 1071 bump_cpu_timer(timer, now);
 1072 /* Leave the tasklist_lock locked for the call below.  */
 1073 }
 1074 
 1075 /*
 1076  * Now re-arm for the new expiry time.
 1077  */
 1078 BUG_ON(!irqs_disabled());
 1079 arm_timer(timer);
 1080 spin_unlock(p-sighand-siglock);
 1081 
 1082 out_unlock:
 1083 read_unlock(tasklist_lock);
 1084 
 1085 out:
 1086 timer-it_overrun_last = timer-it_overrun;
 1087 timer-it_overrun = -1;
 1088 ++timer-it_requeue_pending;
 1089 }
 


-- 
Chen Gang

Asianux Corporation
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [Suggestion] kernel: 'now' may be used uninitialized in posix_cpu_timer_schedule function

2013-03-26 Thread Frederic Weisbecker
2013/3/26 Chen Gang gang.c...@asianux.com:
 Hello Maintainers:

   compiling with EXTRA_CFLAGS=-W:
 make V=1 EXTRA_CFLAGS=-W ARCH=arm s3c2410_defconfig
 make V=1 EXTRA_CFLAGS=-W ARCH=arm menuconfig
   set 'arm-linux-gnu-' for cross chain prefix
 make V=1 EXTRA_CFLAGS=-W ARCH=arm

   it will report:
 kernel/posix-cpu-timers.c:1065:19: warning: ‘now’ may be used 
 uninitialized in this function [-Wuninitialized]

   it seems it is really a bug.
 can any member help to fix it ?
 or provide additional suggestion ?
   (it seems only unsigned long long now = 0 is not enough).

Yeah it's missing a call to cpu_timer_sample_group() before
clear_dead_task(). Andrew Morton reported the warning and I have a
pending patch to fix that. I'm just checking a few other things before
sending it. These clear_dead_task() calls seem to also conflict with
cleanup_timers(). I'm fixing that too.

Thanks for your report!
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [Suggestion] kernel: 'now' may be used uninitialized in posix_cpu_timer_schedule function

2013-03-26 Thread Chen Gang
On 2013年03月26日 20:27, Frederic Weisbecker wrote:
 2013/3/26 Chen Gang gang.c...@asianux.com:
  Hello Maintainers:
 
compiling with EXTRA_CFLAGS=-W:
  make V=1 EXTRA_CFLAGS=-W ARCH=arm s3c2410_defconfig
  make V=1 EXTRA_CFLAGS=-W ARCH=arm menuconfig
set 'arm-linux-gnu-' for cross chain prefix
  make V=1 EXTRA_CFLAGS=-W ARCH=arm
 
it will report:
  kernel/posix-cpu-timers.c:1065:19: warning: �now� may be used 
  uninitialized in this function [-Wuninitialized]
 
it seems it is really a bug.
  can any member help to fix it ?
  or provide additional suggestion ?
(it seems only unsigned long long now = 0 is not enough).
 Yeah it's missing a call to cpu_timer_sample_group() before
 clear_dead_task(). Andrew Morton reported the warning and I have a
 pending patch to fix that. I'm just checking a few other things before
 sending it. These clear_dead_task() calls seem to also conflict with
 cleanup_timers(). I'm fixing that too.
 
 Thanks for your report!
 
 

  thank you, too.

  :-)

-- 
Chen Gang

Asianux Corporation
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: SUGGESTION: Kernel

2005-04-21 Thread James Purser
There is always make gconfig, this is presents a GUI for you. However
keep in mind that compiling your own kernel is never going to be an
idiot proof activity, instead it is something that is going to require a
little bit of knowledge about how the kernel works and how the compile
process works.
-- 
James Purser
http://ksit.dynalias.com

-
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/


SUGGESTION: Kernel

2005-04-21 Thread John Mac Donald
i would like to suggest a graphical way to setup and install the
kernel and kernel components, this would make it easier for idiots
like myself to install the kernel with more ease and could solve this
issue of 'bloating'. Making a graphical kernel installer could make it
easier for people to select exactly what they want in their kernel, i 
have no idea if this is even remotely possible because im quite the
noob when it comes to compiling stuff on linux but if it is that could
be a good idea

>_JMN
-
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/


SUGGESTION: Kernel

2005-04-21 Thread John Mac Donald
i would like to suggest a graphical way to setup and install the
kernel and kernel components, this would make it easier for idiots
like myself to install the kernel with more ease and could solve this
issue of 'bloating'. Making a graphical kernel installer could make it
easier for people to select exactly what they want in their kernel, i 
have no idea if this is even remotely possible because im quite the
noob when it comes to compiling stuff on linux but if it is that could
be a good idea

_JMN
-
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: SUGGESTION: Kernel

2005-04-21 Thread James Purser
There is always make gconfig, this is presents a GUI for you. However
keep in mind that compiling your own kernel is never going to be an
idiot proof activity, instead it is something that is going to require a
little bit of knowledge about how the kernel works and how the compile
process works.
-- 
James Purser
http://ksit.dynalias.com

-
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/