On 2013/8/14 11:32, Dhaval Giani wrote:
> On Tue, Aug 13, 2013 at 10:07 PM, Weng Meiling
> <wengmeiling.w...@huawei.com> wrote:
>> On 2013/8/14 4:38, Dhaval Giani wrote:
>>> Hi,
>>>
>>> On Tue, Aug 13, 2013 at 6:24 AM, Weng Meiling
>>> <wengmeiling.w...@huawei.com> wrote:
>>>>
>>>> From: Weng Meiling <wengmeiling.w...@huawei.com>
>>>>
>>>> When setting the value of cgroup's control file by cgset -r, it's more
>>>> appositely to yield an error when user try to set a read-only file or
>>>> set a file not existed. So fix this by copying the dirty value in function
>>>> cgroup_copy_controller_values().
>>>>
>>>
>>> I am not sure I understand the issue. Do you have a failing case?
>>>
>> In fact, it's just a problem about user experience.
>>
>> Example:
>>
>> # lscgroup
>> cpu:/
>> cpu:/test
>> cpuset:/
>> cpuset:/test
>>
>> if user set the control value which is not existed:
>>
>> # cgset -r cpu.share=1000 test
>> #
>>
>> or set a read-only control value:
>>
>> # cgset -r cpuset.memory_pressure=2 test
>> #
>>
>> as we can see, there is nothing when user do these operations,
>> is it better to yield an error to let user known what happen?:)
>
> So in its current form, I refuse to apply the patch.
>
> We dont know whether applying the value failed because it was
> read-only, or because the name was wrong. This is just a hack, which
> might hide other issues later on. I would much rather have a check
> made (I am not sure where yet), which will confirm the file exists,
> and if so, check whether it is read only.
>
> Thanks!
> Dhaval
>
>
Hi Dhaval,
In fact, when setting the writable file for a invalidate value, there is
also no any messages like this:
# cgset -r cpu.shares=-1 test
#
Does the 'check' you said mean that checking whether the file exists or
it is read only when setting the value, and then giving the apposite messages?
If so, actually the libcg code has done all the check in function
cg_set_control_value() , but the error is ignored in function
cgroup_modify_cgroup（） like this:
error = cg_set_control_value(path,
cgroup->controller[i]->values[j]->value);
free(path);
path = NULL;
/* don't consider error in files directly written by
* the user as fatal */
if (error && !cgroup->controller[i]->values[j]->dirty) {
error = 0;
continue;
}
if (error)
goto err;
cgroup->controller[i]->values[j]->dirty = false;
because in function cgroup_copy_controller_values(), the dirty value is
not copied, the dirty value always is zero, so the error is ignored. The
patch adds the dirty value copy, actually the patch will change the result
of 'cgset -r', before the patch when specifying multiple -r, just the validate
value will be set, the wrong value is ignored. After the patch, if one of
the values is wrong, the left values will not be set even if they are validate.
I just think it's more friendly to let user know what happen.
Thanks!
Weng Meiling
------------------------------------------------------------------------------
Get 100% visibility into Java/.NET code with AppDynamics Lite!
It's a free troubleshooting tool designed for production.
Get down to code-level detail for bottlenecks, with <2% overhead.
Download for free and get started troubleshooting in minutes.
http://pubads.g.doubleclick.net/gampad/clk?id=48897031&iu=/4140/ostg.clktrk
_______________________________________________
Libcg-devel mailing list
Libcg-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/libcg-devel
