Message-ID: <43E89181.3070604@sw.ru>
Date: Tue, 07 Feb 2006 15:24:33 +0300
From: Kirill Korotaev <dev@sw.ru>
User-Agent: Mozilla/5.0 (X11; U; Linux i686; ru-RU; rv:1.2.1) Gecko/20030426
MIME-Version: 1.0
To: Dave Hansen <haveblue@us.ibm.com>
CC: Kirill Korotaev <dev@openvz.org>, Linus Torvalds <torvalds@osdl.org>,
       akpm@osdl.org, linux-kernel@vger.kernel.org, frankeh@watson.ibm.com,
       clg@fr.ibm.com, greg@kroah.com, alan@lxorguk.ukuu.org.uk,
       serue@us.ibm.com, arjan@infradead.org, riel@redhat.com,
       kuznet@ms2.inr.ac.ru, saw@sawoct.com, devel@openvz.org,
       Dmitry Mishin <dim@sw.ru>, Andi Kleen <ak@suse.de>
Subject: Re: [PATCH 1/4] Virtualization/containers: introduction
References: <43E7C65F.3050609@openvz.org> <1139266858.6189.125.camel@localhost.localdomain>
In-Reply-To: <1139266858.6189.125.camel@localhost.localdomain>
Content-Type: text/plain; charset=us-ascii; format=flowed
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 1367
Lines: 33

>>@@ -1132,6 +1133,7 @@ static task_t *copy_process(unsigned lon
>>        p->ioprio = current->ioprio;
>> 
>>        SET_LINKS(p);
>>+       (void)get_container(p->container);
>>        if (unlikely(p->ptrace & PT_PTRACED))
>>                __ptrace_link(p, current->parent); 
> 
> 
> This entire patch looks nice and very straightforward, except for this
> bit. :)  The "(void)" bit isn't usual kernel coding style.  You can
> probably kill it.
it is to avoid warning message the value has no effect.

> BTW, why does get_container() return the container argument?
> get_task_struct(), for instance is just a do{}while(0) loop, so it
> doesn't have a return value.  Is there some magic later on in your patch
> set that utilizes this?
ok, I will remake it without a return value. not a real problem at all.

> One other really minor thing: I usually try to do is keep the !
> CONFIG_FOO functions static inlines, just like the full versions.  The
> advantage is that you get some compile-time type checking, even when
> your CONFIG option is off.
it is not always appropriate :( I try to follow this as well :)

Kirill

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