On 2015/8/13 21:58, Cyril Hrubis wrote:
Hi,
Thank you for your reply and advice, first.
> Have you used checkpatch.pl to check the patch before sending?
> (checkpatch.pl is shipped with Linux kernel sources)
I forget it, sorry.
>
>> +#include <errno.h>
>> +
>> +#include "test.h"
>> +
>> +char *TCID = "edf_test01";
>> +int TST_TOTAL = 1;
>> +
>> +#define gettid() syscall(__NR_gettid)
>
> Even since this is not particulary harmful, can we use static inline
> function instead?
OK.
>
>> +#define SCHED_DEADLINE 6
>
> This is defined in /usr/include/linux/sched.h, Why don't just include
> the header.
>
"EDF(earliest deadline first algorithm) was added to the Linux scheduler in
version 3.14".
And the glibc doesn't support it now.
So the 'SCHED_DEADLINE'(and 'sched_attr') can't be found in the system which
has kernel 3.13 or before.
Should I create include/lapi/sched.h file in LTP and add to it?
>> +/* XXX use the proper syscall numbers */
>> +#ifdef __x86_64__
>> +#define __NR_sched_setattr 314
>> +#define __NR_sched_getattr 315
>> +#endif
>> +
>> +#ifdef __i386__
>> +#define __NR_sched_setattr 351
>> +#define __NR_sched_getattr 352
>> +#endif
>> +
>> +#ifdef __arm__
>> +#define __NR_sched_setattr 380
>> +#define __NR_sched_getattr 381
>> +#endif
>
> You should add these to the tables in testcases/kernel/include/,
> regenerate them and include linux_syscall_numbers.h here.
OK.
>
>> +struct sched_attr {
>> + __u32 size;
>> +
>> + __u32 sched_policy;
>> + __u64 sched_flags;
>> +
>> + /* SCHED_NORMAL, SCHED_BATCH */
>> + __s32 sched_nice;
>> +
>> + /* SCHED_FIFO, SCHED_RR */
>> + __u32 sched_priority;
>> +
>> + /* SCHED_DEADLINE (nsec) */
>> + __u64 sched_runtime;
>> + __u64 sched_deadline;
>> + __u64 sched_period;
>> +};
>
> Where is this structure from?
We can find it in include/linux/sched.h of kernel source (linux 3.14 or later).
Glibc doesn't support it now, so I define it in testcase.
Should I add it to include/lapi/sched.h?
>
> I can find it in manual page, so I guess that it's correct, but even
> then it should be in include/lapi/sched.h and we should use userspace
> types, i.e. uint32_t instead of __u32 (wich are glibc internal types if
> I remeber correctly).
I have a problem there. Use u32 (get by include/linux/sched.h), __u32 (get by
Documentation/scheduler/sched-deadline.txt
in linux 4.1) or uint32_t ?
>
>> +int sched_setattr(pid_t pid,
>> + const struct sched_attr *attr,
>> + unsigned int flags)
> ^
> Spaces before tabs.
>> +{
>> + return syscall(__NR_sched_setattr, pid, attr, flags);
>> +}
>> +
>> +int sched_getattr(pid_t pid,
>> + struct sched_attr *attr,
>> + unsigned int size,
>> + unsigned int flags)
> ^
> Here as well.
>> +{
>> + return syscall(__NR_sched_getattr, pid, attr, size, flags);
>> +}
>
> These should go to the lapi/sched.h as well.
OK.
>
>> +
>> + if ( (10000000 != attr_copy.sched_runtime) ||
>> + (30000000 != attr_copy.sched_period) ||
>> + (30000000 != attr_copy.sched_deadline) ) {
>> + tst_brkm(TFAIL, NULL, "edf_test01 failed\n");
>
> Do not include name of the test in the
> error message. It will be include there
> in the test library.
>
>
> Also split this if to three ifs and print
> what exactly was different that we
> expected. I.e.
>
> #define RUNTIME_VAL 10000000
>
>
> ...
>
> int fail = 0;
>
> if (attr_copy.sched_runtime != RUNTIME_VAL) {
> tst_resm(TINFO, "sched_runtime is incorrect (%" PRIu64
> ") expected %u", attr.sched_runtime,
> RUNTIME_VAL);
> fail++;
> }
>
>
> ...
>
>
> if (fail)
> tst_resm(TFAIL, "attributes were read back incorrectly");
> else
> tst_resm(TPASS, "attributes were read back correctly");
>
>
>> + }
It looks better than mine. Thank you.
>> + pthread_create(&thread, NULL, run_deadline, NULL);
>> +
>> + sleep(1);
>
> Do not use sleep this way in testcases ever. The main thread will sleep
> in the pthread_join() until the thread that does the test exits. This
> will only slow the execution because it will wait 1 second even after
> the test thread has finished allready.
>
OK.
>
> Looking at the test it may be more suitable to be in
> syscalls/sched_getattr/ directory, since it's not testing nothing more
> than the syscalls can set and read back it's parameters.
It's good.
I want to add some testcases for testing edf scheduler which be supported by
linux 3.14.
And there may be a lot of mistakes. Thank you for your advice.
Thanks,
Cui Bixuan
------------------------------------------------------------------------------
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list
