[igt-dev] [PATCH i-g-t] Replaced instances of mmap64() with mmap()
Jake Freeland
jake at technologyfriends.net
Wed Oct 5 20:38:50 UTC 2022
Petri and Kamil,
I just sent in a new patch under the name "Replaced instances of mmap64()
with drm_mmap()".
This patch follows Rob Clark's advice and uses drm_mmap() instead of mmap()
so 32-bit
machines are (partially) accounted for.
Thank you,
Jake Freeland
On Mon, Oct 3, 2022 at 8:17 AM Petri Latvala <petri.latvala at intel.com>
wrote:
> On Wed, Sep 28, 2022 at 05:36:36PM +0200, Kamil Konieczny wrote:
> > On 2022-09-28 at 15:44:54 +0300, Petri Latvala wrote:
> > > On Wed, Sep 28, 2022 at 02:33:14PM +0200, Kamil Konieczny wrote:
> > > > Hi Jake,
> > > >
> > > > this patch is missing commit message. Please put here short
> > > > description of change and why you do it. Change looks resonable
> > > > as on 64-bit systems both functions are the same.
> > > >
> > > > On 2022-09-18 at 17:15:35 -0500, Jake Freeland wrote:
> > > > > Signed-off-by: Jake Freeland <jfree at freebsd.org>
> > > >
> > > > Adding Petri on cc as I do not know if anyone compiles this on
> > > > 32-bit platforms.
> > >
> > > Aren't you supposed to use mmap() with _LARGEFILE_something even on
> > > 32-bit builds, instead of mmap64?
> > >
> > >
> > > --
> > > Petri Latvala
> > >
> >
> > imho _LARGEFILE_... is indeed needed on 32-bit builds, no need
> > for this on our current builds as Linux runs now on 64-bit
> > machines.
> >
> > I was asking if you know of anyone compiling and using igt i915
> > code for 32-bit platform.
> >
> > I see that this patch touches also igt_vgem lib so maybe we
> > should also ask arm igt users ?
>
> Rob Clark suggested just using drm_mmap() and letting it figure out
> whether to call mmap or mmap64. Although it just seems to call mmap()
> anyway and assumes largefile always, except on android, except when
> __LP64__ is set.
>
> For IGT we could either use drm_mmap or just make the same documented
> assumption.
>
> --
> Petri Latvala
>
>
> >
> > --
> > Kamil
> >
> > >
> > >
> > > >
> > > > Regards,
> > > > Kamil
> > > >
> > > > > ---
> > > > > lib/i915/gem_mman.c | 4 ++--
> > > > > lib/igt_vgem.c | 2 +-
> > > > > tests/i915/gem_mmap_gtt.c | 22 +++++++++++-----------
> > > > > tests/i915/gem_mmap_offset.c | 14 +++++++-------
> > > > > 4 files changed, 21 insertions(+), 21 deletions(-)
> > > > >
> > > > > diff --git a/lib/i915/gem_mman.c b/lib/i915/gem_mman.c
> > > > > index aa9ac6f3..0b41ba70 100644
> > > > > --- a/lib/i915/gem_mman.c
> > > > > +++ b/lib/i915/gem_mman.c
> > > > > @@ -118,7 +118,7 @@ void *__gem_mmap__gtt(int fd, uint32_t handle,
> uint64_t size, unsigned prot)
> > > > > if (igt_ioctl(fd, DRM_IOCTL_I915_GEM_MMAP_GTT, &mmap_arg))
> > > > > return NULL;
> > > > >
> > > > > - ptr = mmap64(0, size, prot, MAP_SHARED, fd,
> mmap_arg.offset);
> > > > > + ptr = mmap(0, size, prot, MAP_SHARED, fd, mmap_arg.offset);
> > > > > if (ptr == MAP_FAILED)
> > > > > ptr = NULL;
> > > > > else
> > > > > @@ -331,7 +331,7 @@ void *__gem_mmap_offset(int fd, uint32_t
> handle, uint64_t offset, uint64_t size,
> > > > > if (igt_ioctl(fd, DRM_IOCTL_I915_GEM_MMAP_OFFSET, &arg))
> > > > > return NULL;
> > > > >
> > > > > - ptr = mmap64(0, size, prot, MAP_SHARED, fd, arg.offset +
> offset);
> > > > > + ptr = mmap(0, size, prot, MAP_SHARED, fd, arg.offset +
> offset);
> > > > >
> > > > > if (ptr == MAP_FAILED)
> > > > > ptr = NULL;
> > > > > diff --git a/lib/igt_vgem.c b/lib/igt_vgem.c
> > > > > index 7f933b23..468383c7 100644
> > > > > --- a/lib/igt_vgem.c
> > > > > +++ b/lib/igt_vgem.c
> > > > > @@ -76,7 +76,7 @@ void *__vgem_mmap(int fd, struct vgem_bo *bo,
> unsigned prot)
> > > > > if (drmIoctl(fd, DRM_IOCTL_MODE_MAP_DUMB, &arg))
> > > > > return NULL;
> > > > >
> > > > > - ptr = mmap64(0, bo->size, prot, MAP_SHARED, fd,
> arg.offset);
> > > > > + ptr = mmap(0, bo->size, prot, MAP_SHARED, fd, arg.offset);
> > > > > if (ptr == MAP_FAILED)
> > > > > return NULL;
> > > > >
> > > > > diff --git a/tests/i915/gem_mmap_gtt.c b/tests/i915/gem_mmap_gtt.c
> > > > > index 4f05eb18..683eaf16 100644
> > > > > --- a/tests/i915/gem_mmap_gtt.c
> > > > > +++ b/tests/i915/gem_mmap_gtt.c
> > > > > @@ -113,11 +113,11 @@ test_access(int fd)
> > > > > mmap_arg.handle = handle;
> > > > > do_ioctl(fd, DRM_IOCTL_I915_GEM_MMAP_GTT, &mmap_arg);
> > > > >
> > > > > - igt_assert(mmap64(0, OBJECT_SIZE, PROT_READ | PROT_WRITE,
> > > > > + igt_assert(mmap(0, OBJECT_SIZE, PROT_READ | PROT_WRITE,
> > > > > MAP_SHARED, fd, mmap_arg.offset));
> > > > >
> > > > > /* Check that the same offset on the other fd doesn't
> work. */
> > > > > - igt_assert(mmap64(0, OBJECT_SIZE, PROT_READ | PROT_WRITE,
> > > > > + igt_assert(mmap(0, OBJECT_SIZE, PROT_READ | PROT_WRITE,
> > > > > MAP_SHARED, fd2, mmap_arg.offset) ==
> MAP_FAILED);
> > > > > igt_assert(errno == EACCES);
> > > > >
> > > > > @@ -128,7 +128,7 @@ test_access(int fd)
> > > > >
> > > > > /* Recheck that it works after flink. */
> > > > > /* Check that the same offset on the other fd doesn't
> work. */
> > > > > - igt_assert(mmap64(0, OBJECT_SIZE, PROT_READ | PROT_WRITE,
> > > > > + igt_assert(mmap(0, OBJECT_SIZE, PROT_READ | PROT_WRITE,
> > > > > MAP_SHARED, fd2, mmap_arg.offset));
> > > > > }
> > > > >
> > > > > @@ -159,11 +159,11 @@ test_short(int fd)
> > > > > for (pages = 1; pages <= OBJECT_SIZE / PAGE_SIZE; pages
> <<= 1) {
> > > > > uint8_t *r, *w;
> > > > >
> > > > > - w = mmap64(0, pages * PAGE_SIZE, PROT_READ |
> PROT_WRITE,
> > > > > + w = mmap(0, pages * PAGE_SIZE, PROT_READ |
> PROT_WRITE,
> > > > > MAP_SHARED, fd, mmap_arg.offset);
> > > > > igt_assert(w != MAP_FAILED);
> > > > >
> > > > > - r = mmap64(0, pages * PAGE_SIZE, PROT_READ,
> > > > > + r = mmap(0, pages * PAGE_SIZE, PROT_READ,
> > > > > MAP_SHARED, fd, mmap_arg.offset);
> > > > > igt_assert(r != MAP_FAILED);
> > > > >
> > > > > @@ -384,13 +384,13 @@ test_isolation(int i915)
> > > > >
> > > > > close(B);
> > > > >
> > > > > - ptr = mmap64(0, 4096, PROT_READ, MAP_SHARED, A, offset_a);
> > > > > + ptr = mmap(0, 4096, PROT_READ, MAP_SHARED, A, offset_a);
> > > > > igt_assert(ptr != MAP_FAILED);
> > > > > munmap(ptr, 4096);
> > > > >
> > > > > close(A);
> > > > >
> > > > > - ptr = mmap64(0, 4096, PROT_READ, MAP_SHARED, A, offset_a);
> > > > > + ptr = mmap(0, 4096, PROT_READ, MAP_SHARED, A, offset_a);
> > > > > igt_assert(ptr == MAP_FAILED);
> > > > > }
> > > > >
> > > > > @@ -400,7 +400,7 @@ test_close_race(int i915)
> > > > > const int ncpus = sysconf(_SC_NPROCESSORS_ONLN);
> > > > > _Atomic uint32_t *handles;
> > > > >
> > > > > - handles = mmap64(0, 4096, PROT_WRITE, MAP_SHARED |
> MAP_ANON, -1, 0);
> > > > > + handles = mmap(0, 4096, PROT_WRITE, MAP_SHARED | MAP_ANON,
> -1, 0);
> > > > > igt_assert(handles != MAP_FAILED);
> > > > >
> > > > > igt_fork(child, ncpus + 1) {
> > > > > @@ -418,7 +418,7 @@ test_close_race(int i915)
> > > > > &mmap_arg) != -1) {
> > > > > void *ptr;
> > > > >
> > > > > - ptr = mmap64(0, 4096,
> > > > > + ptr = mmap(0, 4096,
> > > > > PROT_WRITE,
> MAP_SHARED, i915,
> > > > > mmap_arg.offset);
> > > > > if (ptr != MAP_FAILED) {
> > > > > @@ -444,7 +444,7 @@ test_flink_race(int i915)
> > > > > const int ncpus = sysconf(_SC_NPROCESSORS_ONLN);
> > > > > _Atomic uint32_t *handles;
> > > > >
> > > > > - handles = mmap64(0, 4096, PROT_WRITE, MAP_SHARED |
> MAP_ANON, -1, 0);
> > > > > + handles = mmap(0, 4096, PROT_WRITE, MAP_SHARED | MAP_ANON,
> -1, 0);
> > > > > igt_assert(handles != MAP_FAILED);
> > > > >
> > > > > igt_fork(child, ncpus + 1) {
> > > > > @@ -469,7 +469,7 @@ test_flink_race(int i915)
> > > > > &mmap_arg) != -1) {
> > > > > void *ptr;
> > > > >
> > > > > - ptr = mmap64(0, 4096,
> > > > > + ptr = mmap(0, 4096,
> > > > > PROT_WRITE,
> MAP_SHARED, fd,
> > > > > mmap_arg.offset);
> > > > > if (ptr != MAP_FAILED) {
> > > > > diff --git a/tests/i915/gem_mmap_offset.c
> b/tests/i915/gem_mmap_offset.c
> > > > > index 5781b2fb..0933ed94 100644
> > > > > --- a/tests/i915/gem_mmap_offset.c
> > > > > +++ b/tests/i915/gem_mmap_offset.c
> > > > > @@ -66,7 +66,7 @@ __mmap_offset(int i915, uint32_t handle,
> uint64_t offset, uint64_t size,
> > > > > if (mmap_offset_ioctl(i915, &arg))
> > > > > return NULL;
> > > > >
> > > > > - ptr = mmap64(0, size, prot, MAP_SHARED, i915, arg.offset +
> offset);
> > > > > + ptr = mmap(0, size, prot, MAP_SHARED, i915, arg.offset +
> offset);
> > > > > if (ptr == MAP_FAILED)
> > > > > ptr = NULL;
> > > > > else
> > > > > @@ -214,12 +214,12 @@ static void isolation(int i915)
> > > > > t->name, B, b, offset_b);
> > > > >
> > > > > errno = 0;
> > > > > - ptr = mmap64(0, 4096, PROT_READ, MAP_SHARED, i915,
> offset_a);
> > > > > + ptr = mmap(0, 4096, PROT_READ, MAP_SHARED, i915,
> offset_a);
> > > > > igt_assert(ptr == MAP_FAILED);
> > > > > igt_assert_eq(errno, EACCES);
> > > > >
> > > > > errno = 0;
> > > > > - ptr = mmap64(0, 4096, PROT_READ, MAP_SHARED, i915,
> offset_b);
> > > > > + ptr = mmap(0, 4096, PROT_READ, MAP_SHARED, i915,
> offset_b);
> > > > > igt_assert(ptr == MAP_FAILED);
> > > > > igt_assert_eq(errno, EACCES);
> > > > >
> > > > > @@ -237,13 +237,13 @@ static void isolation(int i915)
> > > > >
> > > > > close(B);
> > > > >
> > > > > - ptr = mmap64(0, 4096, PROT_READ, MAP_SHARED, A,
> offset_a);
> > > > > + ptr = mmap(0, 4096, PROT_READ, MAP_SHARED, A,
> offset_a);
> > > > > igt_assert(ptr != MAP_FAILED);
> > > > > munmap(ptr, 4096);
> > > > >
> > > > > close(A);
> > > > >
> > > > > - ptr = mmap64(0, 4096, PROT_READ, MAP_SHARED, A,
> offset_a);
> > > > > + ptr = mmap(0, 4096, PROT_READ, MAP_SHARED, A,
> offset_a);
> > > > > igt_assert(ptr == MAP_FAILED);
> > > > > }
> > > > > }
> > > > > @@ -401,7 +401,7 @@ static void close_race(int i915, int timeout)
> > > > > _Atomic uint32_t *handles;
> > > > > size_t len = ALIGN((ncpus + 1) * sizeof(uint32_t), 4096);
> > > > >
> > > > > - handles = mmap64(0, len, PROT_WRITE, MAP_SHARED |
> MAP_ANON, -1, 0);
> > > > > + handles = mmap(0, len, PROT_WRITE, MAP_SHARED | MAP_ANON,
> -1, 0);
> > > > > igt_assert(handles != MAP_FAILED);
> > > > >
> > > > > igt_fork(child, ncpus + 1) {
> > > > > @@ -420,7 +420,7 @@ static void close_race(int i915, int timeout)
> > > > > &mmap_arg) != -1) {
> > > > > void *ptr;
> > > > >
> > > > > - ptr = mmap64(0, 4096,
> > > > > + ptr = mmap(0, 4096,
> > > > > PROT_WRITE,
> MAP_SHARED, i915,
> > > > > mmap_arg.offset);
> > > > > if (ptr != MAP_FAILED) {
> > > > > --
> > > > > 2.37.3
> > > > >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/igt-dev/attachments/20221005/fce6eaa8/attachment-0001.htm>
More information about the igt-dev
mailing list