Re: [PATCH v2 12/29] ktap: add generic object handling code(kernel/trace/ktap/kp_obj.[c|h])

[Jovi Zhangwei]

On Sun, Mar 30, 2014 at 11:56 AM, Andi Kleen <andi@xxxxxxxxxxxxxx> wrote:
>> + * You should have received a copy of the GNU General Public License along with
>> + * this program; if not, write to the Free Software Foundation, Inc.,
>> + * 51 Franklin St - Fifth Floor, Boston, MA 02110-1301 USA.
>
> We're not supposed to use the address anymore.
>
I will update it.

>> +/* memory allocation flag */
>> +#define KTAP_ALLOC_FLAGS ((GFP_KERNEL | __GFP_NORETRY | __GFP_NOWARN) \
>> +                      & ~__GFP_WAIT)
>> +
>> +void *kp_malloc(ktap_state_t *ks, int size)
>> +{
>> +     void *addr;
>> +
>> +     addr = kmalloc(size, KTAP_ALLOC_FLAGS);
>> +     if (unlikely(!addr)) {
>> +             kp_error(ks, "kmalloc failed\n");
>> +     }
>> +     return addr;
>
> Please remove this pointless wrapper. Similar for the functions below.
> Just use kmalloc etc. directly.
>
Reasonable, save a extra function call.

>> +     case KTAP_TNUM:
>> +             kp_printf(ks, "NUM %ld", nvalue(v));
>
> Similar here. That's all printk
>
Hmm, kp_printf is not printk, there is not printk in ktap,
all content is dump to ring buffer, we cannot use printk in prove context.

And we allow multiple ktap instances running at same time,
so different ktap instance have different ring buffer, that's why we have to
pass the context variable "ks" into kp_printf.

Thanks.

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