[LTP] Bug Report: diotest4 fails on mips64

[LTP] Bug Report: diotest4 fails on mips64

[Hongzhi, Song]

*Ltp:*

latest master


*Kernel:*

After v5.1 ARCH=mips64


*Error info:*

diotest4 10 TBROK : diotest4.c:368: can't mmap file: Invalid argument
diotest4 11 TBROK : diotest4.c:368: Remaining cases broken


I think the first argument of mmap is invalid.

 ?? >shm_base = (char *)(((long)sbrk(0) + (shmsz - 1)) & ~(shmsz - 1));

 ?? >shm_base = mmap(shm_base, 0x100000, PROT_READ | PROT_WRITE,


Regards,

--Hongzhi

[LTP] Bug Report: diotest4 fails on mips64

[Jan Stancek]

----- Original Message -----
> *Ltp:*
> 
> latest master
> 
> 
> *Kernel:*
> 
> After v5.1 ARCH=mips64
> 
> 
> *Error info:*
> 
> diotest4 10 TBROK : diotest4.c:368: can't mmap file: Invalid argument
> diotest4 11 TBROK : diotest4.c:368: Remaining cases broken
> 
> 
> I think the first argument of mmap is invalid.
> 
>  ?? >shm_base = (char *)(((long)sbrk(0) + (shmsz - 1)) & ~(shmsz - 1));
> 
>  ?? >shm_base = mmap(shm_base, 0x100000, PROT_READ | PROT_WRITE,

I don't see any note why that mmap needs to be MAP_FIXED. I'd drop it,
let kernel pick an address:

diff --git a/testcases/kernel/io/direct_io/diotest4.c b/testcases/kernel/io/direct_io/diotest4.c
index e4616e400abd..bf200cd41a27 100644
--- a/testcases/kernel/io/direct_io/diotest4.c
+++ b/testcases/kernel/io/direct_io/diotest4.c
@@ -352,18 +352,14 @@ int main(int argc, char *argv[])
        total++;

        /* Test-10: read, write to a mmaped file */
-       shm_base = (char *)(((long)sbrk(0) + (shmsz - 1)) & ~(shmsz - 1));
-       if (shm_base == NULL) {
-               tst_brkm(TBROK, cleanup, "sbrk failed: %s", strerror(errno));
-       }
        offset = 4096;
        count = bufsize;
        if ((fd = open(filename, O_DIRECT | O_RDWR)) < 0) {
                tst_brkm(TBROK, cleanup, "can't open %s: %s",
                         filename, strerror(errno));
        }
-       shm_base = mmap(shm_base, 0x100000, PROT_READ | PROT_WRITE,
-                       MAP_SHARED | MAP_FIXED, fd, 0);
+       shm_base = mmap(0, 0x100000, PROT_READ | PROT_WRITE,
+                       MAP_SHARED, fd, 0);
        if (shm_base == (caddr_t) - 1) {
                tst_brkm(TBROK, cleanup, "can't mmap file: %s",
                         strerror(errno));

[LTP] Bug Report: diotest4 fails on mips64

[Hongzhi, Song]

Thanks for your work.

I will backport the patch to my project after merged.


--Hongzhi



On 6/28/19 7:09 PM, Jan Stancek wrote:
>
> ----- Original Message -----
>> *Ltp:*
>>
>> latest master
>>
>>
>> *Kernel:*
>>
>> After v5.1 ARCH=mips64
>>
>>
>> *Error info:*
>>
>> diotest4 10 TBROK : diotest4.c:368: can't mmap file: Invalid argument
>> diotest4 11 TBROK : diotest4.c:368: Remaining cases broken
>>
>>
>> I think the first argument of mmap is invalid.
>>
>>   ?? >shm_base = (char *)(((long)sbrk(0) + (shmsz - 1)) & ~(shmsz - 1));
>>
>>   ?? >shm_base = mmap(shm_base, 0x100000, PROT_READ | PROT_WRITE,
> I don't see any note why that mmap needs to be MAP_FIXED. I'd drop it,
> let kernel pick an address:
>
> diff --git a/testcases/kernel/io/direct_io/diotest4.c b/testcases/kernel/io/direct_io/diotest4.c
> index e4616e400abd..bf200cd41a27 100644
> --- a/testcases/kernel/io/direct_io/diotest4.c
> +++ b/testcases/kernel/io/direct_io/diotest4.c
> @@ -352,18 +352,14 @@ int main(int argc, char *argv[])
>          total++;
>
>          /* Test-10: read, write to a mmaped file */
> -       shm_base = (char *)(((long)sbrk(0) + (shmsz - 1)) & ~(shmsz - 1));
> -       if (shm_base == NULL) {
> -               tst_brkm(TBROK, cleanup, "sbrk failed: %s", strerror(errno));
> -       }
>          offset = 4096;
>          count = bufsize;
>          if ((fd = open(filename, O_DIRECT | O_RDWR)) < 0) {
>                  tst_brkm(TBROK, cleanup, "can't open %s: %s",
>                           filename, strerror(errno));
>          }
> -       shm_base = mmap(shm_base, 0x100000, PROT_READ | PROT_WRITE,
> -                       MAP_SHARED | MAP_FIXED, fd, 0);
> +       shm_base = mmap(0, 0x100000, PROT_READ | PROT_WRITE,
> +                       MAP_SHARED, fd, 0);
>          if (shm_base == (caddr_t) - 1) {
>                  tst_brkm(TBROK, cleanup, "can't mmap file: %s",
>                           strerror(errno));
>

[LTP] Bug Report: diotest4 fails on mips64

[Jan Stancek]

----- Original Message -----
> Thanks for your work.
> 
> I will backport the patch to my project after merged.

Does it mean, that patch I posted fixes problem for you?

Do you have good/bad kernel version? I was looking at recent
upstream changes, but didn't spot anything that could explain
that EINVAL.

> 
> 
> --Hongzhi
> 
> 
> 
> On 6/28/19 7:09 PM, Jan Stancek wrote:
> >
> > ----- Original Message -----
> >> *Ltp:*
> >>
> >> latest master
> >>
> >>
> >> *Kernel:*
> >>
> >> After v5.1 ARCH=mips64
> >>
> >>
> >> *Error info:*
> >>
> >> diotest4 10 TBROK : diotest4.c:368: can't mmap file: Invalid argument
> >> diotest4 11 TBROK : diotest4.c:368: Remaining cases broken
> >>
> >>
> >> I think the first argument of mmap is invalid.
> >>
> >>   ?? >shm_base = (char *)(((long)sbrk(0) + (shmsz - 1)) & ~(shmsz - 1));
> >>
> >>   ?? >shm_base = mmap(shm_base, 0x100000, PROT_READ | PROT_WRITE,
> > I don't see any note why that mmap needs to be MAP_FIXED. I'd drop it,
> > let kernel pick an address:
> >
> > diff --git a/testcases/kernel/io/direct_io/diotest4.c
> > b/testcases/kernel/io/direct_io/diotest4.c
> > index e4616e400abd..bf200cd41a27 100644
> > --- a/testcases/kernel/io/direct_io/diotest4.c
> > +++ b/testcases/kernel/io/direct_io/diotest4.c
> > @@ -352,18 +352,14 @@ int main(int argc, char *argv[])
> >          total++;
> >
> >          /* Test-10: read, write to a mmaped file */
> > -       shm_base = (char *)(((long)sbrk(0) + (shmsz - 1)) & ~(shmsz - 1));
> > -       if (shm_base == NULL) {
> > -               tst_brkm(TBROK, cleanup, "sbrk failed: %s",
> > strerror(errno));
> > -       }
> >          offset = 4096;
> >          count = bufsize;
> >          if ((fd = open(filename, O_DIRECT | O_RDWR)) < 0) {
> >                  tst_brkm(TBROK, cleanup, "can't open %s: %s",
> >                           filename, strerror(errno));
> >          }
> > -       shm_base = mmap(shm_base, 0x100000, PROT_READ | PROT_WRITE,
> > -                       MAP_SHARED | MAP_FIXED, fd, 0);
> > +       shm_base = mmap(0, 0x100000, PROT_READ | PROT_WRITE,
> > +                       MAP_SHARED, fd, 0);
> >          if (shm_base == (caddr_t) - 1) {
> >                  tst_brkm(TBROK, cleanup, "can't mmap file: %s",
> >                           strerror(errno));
> >
> 

[LTP] Bug Report: diotest4 fails on mips64

[Jan Stancek]

----- Original Message -----
> 
> ----- Original Message -----
> > Thanks for your work.
> > 
> > I will backport the patch to my project after merged.
> 
> Does it mean, that patch I posted fixes problem for you?
> 
> Do you have good/bad kernel version? I was looking at recent
> upstream changes, but didn't spot anything that could explain
> that EINVAL.

Problem appears to be that "mmap.h" doesn't include "config.h",
so MMAP_GRANULARITY ends up being just single page.

Following fixes it for me on qemu emulated 5kc-malta, running 5.2.0-rc7:

diff --git a/include/lapi/mmap.h b/include/lapi/mmap.h
index 18547c0d4..98b6ade1e 100644
--- a/include/lapi/mmap.h
+++ b/include/lapi/mmap.h
@@ -19,6 +19,8 @@
 #ifndef LAPI_MMAP_H__
 #define LAPI_MMAP_H__

+#include "config.h"
+
 #ifndef MAP_HUGETLB
 # define MAP_HUGETLB 0x40000
 #endif

but still, perhaps we should also drop that MAP_FIXED.

> 
> > 
> > 
> > --Hongzhi
> > 
> > 
> > 
> > On 6/28/19 7:09 PM, Jan Stancek wrote:
> > >
> > > ----- Original Message -----
> > >> *Ltp:*
> > >>
> > >> latest master
> > >>
> > >>
> > >> *Kernel:*
> > >>
> > >> After v5.1 ARCH=mips64
> > >>
> > >>
> > >> *Error info:*
> > >>
> > >> diotest4 10 TBROK : diotest4.c:368: can't mmap file: Invalid argument
> > >> diotest4 11 TBROK : diotest4.c:368: Remaining cases broken
> > >>
> > >>
> > >> I think the first argument of mmap is invalid.
> > >>
> > >>   ?? >shm_base = (char *)(((long)sbrk(0) + (shmsz - 1)) & ~(shmsz - 1));
> > >>
> > >>   ?? >shm_base = mmap(shm_base, 0x100000, PROT_READ | PROT_WRITE,
> > > I don't see any note why that mmap needs to be MAP_FIXED. I'd drop it,
> > > let kernel pick an address:
> > >
> > > diff --git a/testcases/kernel/io/direct_io/diotest4.c
> > > b/testcases/kernel/io/direct_io/diotest4.c
> > > index e4616e400abd..bf200cd41a27 100644
> > > --- a/testcases/kernel/io/direct_io/diotest4.c
> > > +++ b/testcases/kernel/io/direct_io/diotest4.c
> > > @@ -352,18 +352,14 @@ int main(int argc, char *argv[])
> > >          total++;
> > >
> > >          /* Test-10: read, write to a mmaped file */
> > > -       shm_base = (char *)(((long)sbrk(0) + (shmsz - 1)) & ~(shmsz -
> > > 1));
> > > -       if (shm_base == NULL) {
> > > -               tst_brkm(TBROK, cleanup, "sbrk failed: %s",
> > > strerror(errno));
> > > -       }
> > >          offset = 4096;
> > >          count = bufsize;
> > >          if ((fd = open(filename, O_DIRECT | O_RDWR)) < 0) {
> > >                  tst_brkm(TBROK, cleanup, "can't open %s: %s",
> > >                           filename, strerror(errno));
> > >          }
> > > -       shm_base = mmap(shm_base, 0x100000, PROT_READ | PROT_WRITE,
> > > -                       MAP_SHARED | MAP_FIXED, fd, 0);
> > > +       shm_base = mmap(0, 0x100000, PROT_READ | PROT_WRITE,
> > > +                       MAP_SHARED, fd, 0);
> > >          if (shm_base == (caddr_t) - 1) {
> > >                  tst_brkm(TBROK, cleanup, "can't mmap file: %s",
> > >                           strerror(errno));
> > >
> > 
> 
> --
> Mailing list info: https://lists.linux.it/listinfo/ltp
> 

[LTP] Bug Report: diotest4 fails on mips64

[Hongzhi, Song]

On 7/3/19 8:53 PM, Jan Stancek wrote:
> ----- Original Message -----
>> Thanks for your work.
>>
>> I will backport the patch to my project after merged.
> Does it mean, that patch I posted fixes problem for you?


Not for me, for LTP.

I just found the bug. Even if there is no me, the bug still exists.


Do you think the manually selecting addr for mmap is reasonable?

And the addr points to the top of heap.


|  >shm_base = (char *)(((long)sbrk(0) + (shmsz - 1)) & ~(shmsz - 1));

|? >shm_base = mmap(shm_base, 0x100000, PROT_READ | PROT_WRITE,


>
> Do you have good/bad kernel version? I was looking at recent
> upstream changes, but didn't spot anything that could explain
> that EINVAL.


I didn't test all versions of kernel. But after v5.1-mips can raise the 
problem.

Maybe the bug exists on mips since the testcase is created.

I guess that few people use mips now, so the bug is not found in time.


--Hongzhi


>>
>> --Hongzhi
>>
>>
>>
>> On 6/28/19 7:09 PM, Jan Stancek wrote:
>>> ----- Original Message -----
>>>> *Ltp:*
>>>>
>>>> latest master
>>>>
>>>>
>>>> *Kernel:*
>>>>
>>>> After v5.1 ARCH=mips64
>>>>
>>>>
>>>> *Error info:*
>>>>
>>>> diotest4 10 TBROK : diotest4.c:368: can't mmap file: Invalid argument
>>>> diotest4 11 TBROK : diotest4.c:368: Remaining cases broken
>>>>
>>>>
>>>> I think the first argument of mmap is invalid.
>>>>
>>>>    ?? >shm_base = (char *)(((long)sbrk(0) + (shmsz - 1)) & ~(shmsz - 1));
>>>>
>>>>    ?? >shm_base = mmap(shm_base, 0x100000, PROT_READ | PROT_WRITE,
>>> I don't see any note why that mmap needs to be MAP_FIXED. I'd drop it,
>>> let kernel pick an address:
>>>
>>> diff --git a/testcases/kernel/io/direct_io/diotest4.c
>>> b/testcases/kernel/io/direct_io/diotest4.c
>>> index e4616e400abd..bf200cd41a27 100644
>>> --- a/testcases/kernel/io/direct_io/diotest4.c
>>> +++ b/testcases/kernel/io/direct_io/diotest4.c
>>> @@ -352,18 +352,14 @@ int main(int argc, char *argv[])
>>>           total++;
>>>
>>>           /* Test-10: read, write to a mmaped file */
>>> -       shm_base = (char *)(((long)sbrk(0) + (shmsz - 1)) & ~(shmsz - 1));
>>> -       if (shm_base == NULL) {
>>> -               tst_brkm(TBROK, cleanup, "sbrk failed: %s",
>>> strerror(errno));
>>> -       }
>>>           offset = 4096;
>>>           count = bufsize;
>>>           if ((fd = open(filename, O_DIRECT | O_RDWR)) < 0) {
>>>                   tst_brkm(TBROK, cleanup, "can't open %s: %s",
>>>                            filename, strerror(errno));
>>>           }
>>> -       shm_base = mmap(shm_base, 0x100000, PROT_READ | PROT_WRITE,
>>> -                       MAP_SHARED | MAP_FIXED, fd, 0);
>>> +       shm_base = mmap(0, 0x100000, PROT_READ | PROT_WRITE,
>>> +                       MAP_SHARED, fd, 0);
>>>           if (shm_base == (caddr_t) - 1) {
>>>                   tst_brkm(TBROK, cleanup, "can't mmap file: %s",
>>>                            strerror(errno));
>>>

[LTP] Bug Report: diotest4 fails on mips64

[Hongzhi, Song]

On 7/4/19 8:02 AM, Jan Stancek wrote:
>
> ----- Original Message -----
>> ----- Original Message -----
>>> Thanks for your work.
>>>
>>> I will backport the patch to my project after merged.
>> Does it mean, that patch I posted fixes problem for you?
>>
>> Do you have good/bad kernel version? I was looking at recent
>> upstream changes, but didn't spot anything that could explain
>> that EINVAL.
> Problem appears to be that "mmap.h" doesn't include "config.h",
> so MMAP_GRANULARITY ends up being just single page.

Sorry, can you tell me the relationship between config.h and 
MMAP_GRANULARITY ?

I tested several times that including "config.h" indeed fixed the bug.


>
> Following fixes it for me on qemu emulated 5kc-malta, running 5.2.0-rc7:
>
> diff --git a/include/lapi/mmap.h b/include/lapi/mmap.h
> index 18547c0d4..98b6ade1e 100644
> --- a/include/lapi/mmap.h
> +++ b/include/lapi/mmap.h
> @@ -19,6 +19,8 @@
>   #ifndef LAPI_MMAP_H__
>   #define LAPI_MMAP_H__
>
> +#include "config.h"
> +
>   #ifndef MAP_HUGETLB
>   # define MAP_HUGETLB 0x40000
>   #endif
>
> but still, perhaps we should also drop that MAP_FIXED.


Agree with it. Could I send the patch with your signed-off?

--Hongzhi


>
>>>
>>> --Hongzhi
>>>
>>>
>>>
>>> On 6/28/19 7:09 PM, Jan Stancek wrote:
>>>> ----- Original Message -----
>>>>> *Ltp:*
>>>>>
>>>>> latest master
>>>>>
>>>>>
>>>>> *Kernel:*
>>>>>
>>>>> After v5.1 ARCH=mips64
>>>>>
>>>>>
>>>>> *Error info:*
>>>>>
>>>>> diotest4 10 TBROK : diotest4.c:368: can't mmap file: Invalid argument
>>>>> diotest4 11 TBROK : diotest4.c:368: Remaining cases broken
>>>>>
>>>>>
>>>>> I think the first argument of mmap is invalid.
>>>>>
>>>>>    ?? >shm_base = (char *)(((long)sbrk(0) + (shmsz - 1)) & ~(shmsz - 1));
>>>>>
>>>>>    ?? >shm_base = mmap(shm_base, 0x100000, PROT_READ | PROT_WRITE,
>>>> I don't see any note why that mmap needs to be MAP_FIXED. I'd drop it,
>>>> let kernel pick an address:
>>>>
>>>> diff --git a/testcases/kernel/io/direct_io/diotest4.c
>>>> b/testcases/kernel/io/direct_io/diotest4.c
>>>> index e4616e400abd..bf200cd41a27 100644
>>>> --- a/testcases/kernel/io/direct_io/diotest4.c
>>>> +++ b/testcases/kernel/io/direct_io/diotest4.c
>>>> @@ -352,18 +352,14 @@ int main(int argc, char *argv[])
>>>>           total++;
>>>>
>>>>           /* Test-10: read, write to a mmaped file */
>>>> -       shm_base = (char *)(((long)sbrk(0) + (shmsz - 1)) & ~(shmsz -
>>>> 1));
>>>> -       if (shm_base == NULL) {
>>>> -               tst_brkm(TBROK, cleanup, "sbrk failed: %s",
>>>> strerror(errno));
>>>> -       }
>>>>           offset = 4096;
>>>>           count = bufsize;
>>>>           if ((fd = open(filename, O_DIRECT | O_RDWR)) < 0) {
>>>>                   tst_brkm(TBROK, cleanup, "can't open %s: %s",
>>>>                            filename, strerror(errno));
>>>>           }
>>>> -       shm_base = mmap(shm_base, 0x100000, PROT_READ | PROT_WRITE,
>>>> -                       MAP_SHARED | MAP_FIXED, fd, 0);
>>>> +       shm_base = mmap(0, 0x100000, PROT_READ | PROT_WRITE,
>>>> +                       MAP_SHARED, fd, 0);
>>>>           if (shm_base == (caddr_t) - 1) {
>>>>                   tst_brkm(TBROK, cleanup, "can't mmap file: %s",
>>>>                            strerror(errno));
>>>>
>> --
>> Mailing list info: https://lists.linux.it/listinfo/ltp
>>

[LTP] Bug Report: diotest4 fails on mips64

[Jan Stancek]

----- Original Message -----
> 
> On 7/4/19 8:02 AM, Jan Stancek wrote:
> >
> > ----- Original Message -----
> >> ----- Original Message -----
> >>> Thanks for your work.
> >>>
> >>> I will backport the patch to my project after merged.
> >> Does it mean, that patch I posted fixes problem for you?
> >>
> >> Do you have good/bad kernel version? I was looking at recent
> >> upstream changes, but didn't spot anything that could explain
> >> that EINVAL.
> > Problem appears to be that "mmap.h" doesn't include "config.h",
> > so MMAP_GRANULARITY ends up being just single page.
> 
> Sorry, can you tell me the relationship between config.h and
> MMAP_GRANULARITY ?

configure detects that you have sys/smh.h, which is recorded in config.h:
  /* Define to 1 if you have the <sys/shm.h> header file. */
  #define HAVE_SYS_SHM_H 1

If your platform has shm.h, then MMAP_GRANULARITY is define as SHMLBA,
otherwise as single page.

If lapi/mmap.h doesn't include config.h, then it's always the latter,
because HAVE_SYS_SHM_H is undefined.

> 
> I tested several times that including "config.h" indeed fixed the bug.
> 
> 
> >
> > Following fixes it for me on qemu emulated 5kc-malta, running 5.2.0-rc7:
> >
> > diff --git a/include/lapi/mmap.h b/include/lapi/mmap.h
> > index 18547c0d4..98b6ade1e 100644
> > --- a/include/lapi/mmap.h
> > +++ b/include/lapi/mmap.h
> > @@ -19,6 +19,8 @@
> >   #ifndef LAPI_MMAP_H__
> >   #define LAPI_MMAP_H__
> >
> > +#include "config.h"
> > +
> >   #ifndef MAP_HUGETLB
> >   # define MAP_HUGETLB 0x40000
> >   #endif
> >
> > but still, perhaps we should also drop that MAP_FIXED.
> 
> 
> Agree with it. Could I send the patch with your signed-off?

I sent one ~20 minutes ago, you should be on CC.

> 
> --Hongzhi
> 
> 
> >
> >>>
> >>> --Hongzhi
> >>>
> >>>
> >>>
> >>> On 6/28/19 7:09 PM, Jan Stancek wrote:
> >>>> ----- Original Message -----
> >>>>> *Ltp:*
> >>>>>
> >>>>> latest master
> >>>>>
> >>>>>
> >>>>> *Kernel:*
> >>>>>
> >>>>> After v5.1 ARCH=mips64
> >>>>>
> >>>>>
> >>>>> *Error info:*
> >>>>>
> >>>>> diotest4 10 TBROK : diotest4.c:368: can't mmap file: Invalid argument
> >>>>> diotest4 11 TBROK : diotest4.c:368: Remaining cases broken
> >>>>>
> >>>>>
> >>>>> I think the first argument of mmap is invalid.
> >>>>>
> >>>>>    ?? >shm_base = (char *)(((long)sbrk(0) + (shmsz - 1)) & ~(shmsz -
> >>>>>    ?? >1));
> >>>>>
> >>>>>    ?? >shm_base = mmap(shm_base, 0x100000, PROT_READ | PROT_WRITE,
> >>>> I don't see any note why that mmap needs to be MAP_FIXED. I'd drop it,
> >>>> let kernel pick an address:
> >>>>
> >>>> diff --git a/testcases/kernel/io/direct_io/diotest4.c
> >>>> b/testcases/kernel/io/direct_io/diotest4.c
> >>>> index e4616e400abd..bf200cd41a27 100644
> >>>> --- a/testcases/kernel/io/direct_io/diotest4.c
> >>>> +++ b/testcases/kernel/io/direct_io/diotest4.c
> >>>> @@ -352,18 +352,14 @@ int main(int argc, char *argv[])
> >>>>           total++;
> >>>>
> >>>>           /* Test-10: read, write to a mmaped file */
> >>>> -       shm_base = (char *)(((long)sbrk(0) + (shmsz - 1)) & ~(shmsz -
> >>>> 1));
> >>>> -       if (shm_base == NULL) {
> >>>> -               tst_brkm(TBROK, cleanup, "sbrk failed: %s",
> >>>> strerror(errno));
> >>>> -       }
> >>>>           offset = 4096;
> >>>>           count = bufsize;
> >>>>           if ((fd = open(filename, O_DIRECT | O_RDWR)) < 0) {
> >>>>                   tst_brkm(TBROK, cleanup, "can't open %s: %s",
> >>>>                            filename, strerror(errno));
> >>>>           }
> >>>> -       shm_base = mmap(shm_base, 0x100000, PROT_READ | PROT_WRITE,
> >>>> -                       MAP_SHARED | MAP_FIXED, fd, 0);
> >>>> +       shm_base = mmap(0, 0x100000, PROT_READ | PROT_WRITE,
> >>>> +                       MAP_SHARED, fd, 0);
> >>>>           if (shm_base == (caddr_t) - 1) {
> >>>>                   tst_brkm(TBROK, cleanup, "can't mmap file: %s",
> >>>>                            strerror(errno));
> >>>>
> >> --
> >> Mailing list info: https://lists.linux.it/listinfo/ltp
> >>
> 

[LTP] Bug Report: diotest4 fails on mips64

[Hongzhi, Song]

On 7/4/19 4:17 PM, Jan Stancek wrote:
>
> ----- Original Message -----
>> On 7/4/19 8:02 AM, Jan Stancek wrote:
>>> ----- Original Message -----
>>>> ----- Original Message -----
>>>>> Thanks for your work.
>>>>>
>>>>> I will backport the patch to my project after merged.
>>>> Does it mean, that patch I posted fixes problem for you?
>>>>
>>>> Do you have good/bad kernel version? I was looking at recent
>>>> upstream changes, but didn't spot anything that could explain
>>>> that EINVAL.
>>> Problem appears to be that "mmap.h" doesn't include "config.h",
>>> so MMAP_GRANULARITY ends up being just single page.
>> Sorry, can you tell me the relationship between config.h and
>> MMAP_GRANULARITY ?
> configure detects that you have sys/smh.h, which is recorded in config.h:
>    /* Define to 1 if you have the <sys/shm.h> header file. */
>    #define HAVE_SYS_SHM_H 1
>
> If your platform has shm.h, then MMAP_GRANULARITY is define as SHMLBA,
> otherwise as single page.
>
> If lapi/mmap.h doesn't include config.h, then it's always the latter,
> because HAVE_SYS_SHM_H is undefined.
>
>> I tested several times that including "config.h" indeed fixed the bug.
>>
>>
>>> Following fixes it for me on qemu emulated 5kc-malta, running 5.2.0-rc7:
>>>
>>> diff --git a/include/lapi/mmap.h b/include/lapi/mmap.h
>>> index 18547c0d4..98b6ade1e 100644
>>> --- a/include/lapi/mmap.h
>>> +++ b/include/lapi/mmap.h
>>> @@ -19,6 +19,8 @@
>>>    #ifndef LAPI_MMAP_H__
>>>    #define LAPI_MMAP_H__
>>>
>>> +#include "config.h"
>>> +
>>>    #ifndef MAP_HUGETLB
>>>    # define MAP_HUGETLB 0x40000
>>>    #endif
>>>
>>> but still, perhaps we should also drop that MAP_FIXED.
>>
>> Agree with it. Could I send the patch with your signed-off?
> I sent one ~20 minutes ago, you should be on CC.


Yeah, I see it. Thank you very much.

--Hongzhi


>
>> --Hongzhi
>>
>>
>>>>> --Hongzhi
>>>>>
>>>>>
>>>>>
>>>>> On 6/28/19 7:09 PM, Jan Stancek wrote:
>>>>>> ----- Original Message -----
>>>>>>> *Ltp:*
>>>>>>>
>>>>>>> latest master
>>>>>>>
>>>>>>>
>>>>>>> *Kernel:*
>>>>>>>
>>>>>>> After v5.1 ARCH=mips64
>>>>>>>
>>>>>>>
>>>>>>> *Error info:*
>>>>>>>
>>>>>>> diotest4 10 TBROK : diotest4.c:368: can't mmap file: Invalid argument
>>>>>>> diotest4 11 TBROK : diotest4.c:368: Remaining cases broken
>>>>>>>
>>>>>>>
>>>>>>> I think the first argument of mmap is invalid.
>>>>>>>
>>>>>>>     ?? >shm_base = (char *)(((long)sbrk(0) + (shmsz - 1)) & ~(shmsz -
>>>>>>>     ?? >1));
>>>>>>>
>>>>>>>     ?? >shm_base = mmap(shm_base, 0x100000, PROT_READ | PROT_WRITE,
>>>>>> I don't see any note why that mmap needs to be MAP_FIXED. I'd drop it,
>>>>>> let kernel pick an address:
>>>>>>
>>>>>> diff --git a/testcases/kernel/io/direct_io/diotest4.c
>>>>>> b/testcases/kernel/io/direct_io/diotest4.c
>>>>>> index e4616e400abd..bf200cd41a27 100644
>>>>>> --- a/testcases/kernel/io/direct_io/diotest4.c
>>>>>> +++ b/testcases/kernel/io/direct_io/diotest4.c
>>>>>> @@ -352,18 +352,14 @@ int main(int argc, char *argv[])
>>>>>>            total++;
>>>>>>
>>>>>>            /* Test-10: read, write to a mmaped file */
>>>>>> -       shm_base = (char *)(((long)sbrk(0) + (shmsz - 1)) & ~(shmsz -
>>>>>> 1));
>>>>>> -       if (shm_base == NULL) {
>>>>>> -               tst_brkm(TBROK, cleanup, "sbrk failed: %s",
>>>>>> strerror(errno));
>>>>>> -       }
>>>>>>            offset = 4096;
>>>>>>            count = bufsize;
>>>>>>            if ((fd = open(filename, O_DIRECT | O_RDWR)) < 0) {
>>>>>>                    tst_brkm(TBROK, cleanup, "can't open %s: %s",
>>>>>>                             filename, strerror(errno));
>>>>>>            }
>>>>>> -       shm_base = mmap(shm_base, 0x100000, PROT_READ | PROT_WRITE,
>>>>>> -                       MAP_SHARED | MAP_FIXED, fd, 0);
>>>>>> +       shm_base = mmap(0, 0x100000, PROT_READ | PROT_WRITE,
>>>>>> +                       MAP_SHARED, fd, 0);
>>>>>>            if (shm_base == (caddr_t) - 1) {
>>>>>>                    tst_brkm(TBROK, cleanup, "can't mmap file: %s",
>>>>>>                             strerror(errno));
>>>>>>
>>>> --
>>>> Mailing list info: https://lists.linux.it/listinfo/ltp
>>>>