On Thu, Nov 18, 2010 at 10:29 AM, Cyril Hrubis <[email protected]> wrote:
> Hi!
>> aio_suspend may be interrupted by IO request's completion signal, so we
>> should use aio_error instead signal hander to check whether this request
>> has complete.
>>
>> Signed-off-by: Bian Naimeng <[email protected]>
>>
>> ---
>> .../conformance/interfaces/aio_suspend/1-1.c | 51
>> +++++++-------------
>> 1 files changed, 17 insertions(+), 34 deletions(-)
>>
>> diff --git
>> a/testcases/open_posix_testsuite/conformance/interfaces/aio_suspend/1-1.c
>> b/testcases/open_posix_testsuite/conformance/interfaces/aio_suspend/1-1.c
>> index 7d33e62..8c85ca5 100644
>> --- a/testcases/open_posix_testsuite/conformance/interfaces/aio_suspend/1-1.c
>> +++ b/testcases/open_posix_testsuite/conformance/interfaces/aio_suspend/1-1.c
>> @@ -44,19 +44,11 @@
>> #define BUF_SIZE 1024*1024
>> #define WAIT_FOR_AIOCB 6
>>
>> -int received_selected = 0;
>> int received_all = 0;
>>
>> void
>> sigrt1_handler(int signum, siginfo_t *info, void *context)
>> {
>> - if (info->si_value.sival_int == WAIT_FOR_AIOCB)
>> - received_selected = 1;
>> -}
>> -
>> -void
>> -sigrt2_handler(int signum, siginfo_t *info, void *context)
>> -{
>> received_all = 1;
>> }
>>
>> @@ -123,30 +115,19 @@ main ()
>> aiocbs[i]->aio_buf = &bufs[i*BUF_SIZE];
>> aiocbs[i]->aio_nbytes = BUF_SIZE;
>> aiocbs[i]->aio_lio_opcode = LIO_READ;
>> -
>> - /* Use SIRTMIN+1 for individual completions */
>> - aiocbs[i]->aio_sigevent.sigev_notify = SIGEV_SIGNAL;
>> - aiocbs[i]->aio_sigevent.sigev_signo = SIGRTMIN+1;
>> - aiocbs[i]->aio_sigevent.sigev_value.sival_int = i;
>> }
>>
>> - /* Use SIGRTMIN+2 for list completion */
>> + /* Use SIGRTMIN+1 for list completion */
>> event.sigev_notify = SIGEV_SIGNAL;
>> - event.sigev_signo = SIGRTMIN+2;
>> + event.sigev_signo = SIGRTMIN+1;
>> event.sigev_value.sival_ptr = NULL;
>>
>> - /* Setup handler for individual operation completion */
>> + /* Setup handler for list completion */
>> action.sa_sigaction = sigrt1_handler;
>> sigemptyset(&action.sa_mask);
>> action.sa_flags = SA_SIGINFO|SA_RESTART;
>> sigaction(SIGRTMIN+1, &action, NULL);
>>
>> - /* Setup handler for list completion */
>> - action.sa_sigaction = sigrt2_handler;
>> - sigemptyset(&action.sa_mask);
>> - action.sa_flags = SA_SIGINFO|SA_RESTART;
>> - sigaction(SIGRTMIN+2, &action, NULL);
>> -
>> /* Setup suspend list */
>> plist[0] = NULL;
>> plist[1] = aiocbs[WAIT_FOR_AIOCB];
>> @@ -155,7 +136,8 @@ main ()
>> ret = lio_listio(LIO_NOWAIT, aiocbs, NUM_AIOCBS, &event);
>>
>> if (ret) {
>> - printf(TNAME " Error at lio_listio() %d: %s\n", errno,
>> strerror(errno));
>> + printf (TNAME " Error at lio_listio() %d: %s\n",
>> + errno, strerror(errno));
>
> According to lkml coding style (that we are trying to use here), space
> should be only between C keywords and (). Not betweeen library calls
> like printf and so.
This is true for LTP code, but this is third party code. The
documentation claims to conform to the LKML coding style though (even
though I know it isn't true).
>> for (i=0; i<NUM_AIOCBS; i++)
>> free (aiocbs[i]);
>> free (bufs);
>> @@ -165,9 +147,10 @@ main ()
>> }
>>
>> /* Check selected request has not completed yet */
>> - if (received_selected) {
>> - printf (TNAME " Error : AIOCB %d already completed before
>> suspend\n",
>> - WAIT_FOR_AIOCB);
>> + err = aio_error(aiocbs[WAIT_FOR_AIOCB]);
>> + if (err != EINPROGRESS) {
>> + printf (TNAME " Error : AIOCB %d should not have completed "
>> + "before suspend\n", WAIT_FOR_AIOCB);
>> for (i=0; i<NUM_AIOCBS; i++)
>> free (aiocbs[i]);
>> free (bufs);
> ....
>> exit(PTS_FAIL);
>>}
>
> Another problem lies here. The test relies that aio operation in not
> finished when we got here. And as some linux kernels fallbacks aio as
> synchronous IO, when we got there, it's allready finished. So I think we
> should change this PTS_FAIL to PTS_UNRESOLVED.
>
> Garrett what you think?
If an operating system doesn't fully implement a set of requirements,
it shouldn't claim to do so. If it does then that's a bug in the
implementation.
>> @@ -178,11 +161,9 @@ main ()
>>
>> /* Suspend on selected request */
>> ret = aio_suspend((const struct aiocb **)plist, 2, NULL);
>
> Please remove useless cast to (const struct aiocb**)
Depends on whether or not this is correct. According to GCC it matters:
../../../conformance/interfaces/aio_suspend/1-1.c:180: warning:
passing argument 1 of 'aio_suspend' from incompatible pointer type
This should probably have the restrict c99 keyword attached to it though.
>> -
>> - /* Check selected request has completed */
>> - if (!received_selected) {
>> - printf (TNAME " Error : AIOCB %d should have completed after
>> suspend\n",
>> - WAIT_FOR_AIOCB);
>> + if (ret) {
>> + printf (TNAME " Error at aio_suspend() %d: %s\n",
>> + errno, strerror (errno));
>> for (i=0; i<NUM_AIOCBS; i++)
>> free (aiocbs[i]);
>> free (bufs);
>> @@ -191,9 +172,11 @@ main ()
>> exit (PTS_FAIL);
>> }
>
> Here again with the printf.
>
>> -
>> - if (ret) {
>> - printf (TNAME " Error at aio_suspend() %d: %s\n", errno,
>> strerror (errno));
>> + /* Check selected request has completed */
>> + err = aio_error(aiocbs[WAIT_FOR_AIOCB]);
>> + if (err) {
>> + printf (TNAME " Error : AIOCB %d should have completed after "
>> + "suspend\n", WAIT_FOR_AIOCB);
>> for (i=0; i<NUM_AIOCBS; i++)
>> free (aiocbs[i]);
>> free (bufs);
>
> And here.
I've cleaned up the style in this test to be more logical.
Unfortunately the bull.net contributed tests weren't cleaned up on
import to posix_testsuite, just like some of the syscall testcases in
LTP proper.
Thanks,
-Garrett
------------------------------------------------------------------------------
Beautiful is writing same markup. Internet Explorer 9 supports
standards for HTML5, CSS3, SVG 1.1, ECMAScript5, and DOM L2 & L3.
Spend less time writing and rewriting code and more time creating great
experiences on the web. Be a part of the beta today
http://p.sf.net/sfu/msIE9-sfdev2dev
_______________________________________________
Ltp-list mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/ltp-list
