ClientNativePixmapFactoryDmabuf uses ioctl, instead of drmIoctl.

ui/gfx/linux/client_native_pixmap_dmabuf.cc

Index: ui/gfx/linux/client_native_pixmap_dmabuf.cc
diff --git a/ui/gfx/linux/client_native_pixmap_dmabuf.cc b/ui/gfx/linux/client_native_pixmap_dmabuf.cc
index d656c338f0a6d9eb86e9a3e0a1dd304559d4e28e..d59623580b1010ec9676a66c0ef7f3dad07025c2 100644
--- a/ui/gfx/linux/client_native_pixmap_dmabuf.cc
+++ b/ui/gfx/linux/client_native_pixmap_dmabuf.cc
@@ -7,36 +7,35 @@
 #include <fcntl.h>
 #include <linux/version.h>
 #include <stddef.h>
+#include <sys/ioctl.h>
 #include <sys/mman.h>
 #include <xf86drm.h>
 
 #include "base/debug/crash_logging.h"
 #include "base/memory/ptr_util.h"
+#include "base/posix/eintr_wrapper.h"
 #include "base/process/memory.h"
 #include "base/process/process_metrics.h"
 #include "base/strings/stringprintf.h"
 #include "base/trace_event/trace_event.h"
 
-#if LINUX_VERSION_CODE < KERNEL_VERSION(4, 6, 0)
+#if LINUX_VERSION_CODE >= KERNEL_VERSION(4, 6, 0)

[gurchetansingh, 2017/04/06 01:39:09]

Can we just check #ifdef _DMA_BUF_UAPI_H_ (exported by linux/dma-buf.h) and not
duplicate code?  If the header is not there (it will be for ChromeOS since
Dongseong added it), we shouldn't do local definitions.

[spang, 2017/04/06 02:17:46]

If the header is not there, you have two options:

(1) Fail the build
(2) Make a local definition since these APIs cannot change anyway

There's an option, but it's the worst one:

(3) Stub it out

I don't think we should ever do (3).

You'll probably fail on some buildbot if you do (1); what's why these
definitions are here.

[dshwang, 2017/04/06 16:39:59]

In addition of spang's nice explanation, this code will be used for plane linux.
however, this code should be compiled in old kernel also. that's the main reason
why we have local definition.
https://bugs.chromium.org/p/chromium/issues/detail?id=584248

+#include <linux/dma-buf.h>
+#else
 #include <linux/types.h>
 
-struct local_dma_buf_sync {
+struct dma_buf_sync {
   __u64 flags;
 };
 
-#define LOCAL_DMA_BUF_SYNC_READ (1 << 0)
-#define LOCAL_DMA_BUF_SYNC_WRITE (2 << 0)
-#define LOCAL_DMA_BUF_SYNC_RW \
-  (LOCAL_DMA_BUF_SYNC_READ | LOCAL_DMA_BUF_SYNC_WRITE)
-#define LOCAL_DMA_BUF_SYNC_START (0 << 2)
-#define LOCAL_DMA_BUF_SYNC_END (1 << 2)
-
-#define LOCAL_DMA_BUF_BASE 'b'
-#define LOCAL_DMA_BUF_IOCTL_SYNC \
-  _IOW(LOCAL_DMA_BUF_BASE, 0, struct local_dma_buf_sync)
+#define DMA_BUF_SYNC_READ (1 << 0)
+#define DMA_BUF_SYNC_WRITE (2 << 0)
+#define DMA_BUF_SYNC_RW (DMA_BUF_SYNC_READ | DMA_BUF_SYNC_WRITE)
+#define DMA_BUF_SYNC_START (0 << 2)
+#define DMA_BUF_SYNC_END (1 << 2)
 
-#else
-#include <linux/dma-buf.h>
+#define DMA_BUF_BASE 'b'
+#define DMA_BUF_IOCTL_SYNC _IOW(DMA_BUF_BASE, 0, struct dma_buf_sync)
 #endif
 
 namespace gfx {
@@ -44,24 +43,24 @@ namespace gfx {
 namespace {
 
 void PrimeSyncStart(int dmabuf_fd) {
-  struct local_dma_buf_sync sync_start = {0};
+  struct dma_buf_sync sync_start = {0};
 
-  sync_start.flags = LOCAL_DMA_BUF_SYNC_START | LOCAL_DMA_BUF_SYNC_RW;
+  sync_start.flags = DMA_BUF_SYNC_START | DMA_BUF_SYNC_RW;
 #if DCHECK_IS_ON()
   int rv =
 #endif
-      drmIoctl(dmabuf_fd, LOCAL_DMA_BUF_IOCTL_SYNC, &sync_start);
+      HANDLE_EINTR(ioctl(dmabuf_fd, DMA_BUF_IOCTL_SYNC, &sync_start));
   DPLOG_IF(ERROR, rv) << "Failed DMA_BUF_SYNC_START";
 }
 
 void PrimeSyncEnd(int dmabuf_fd) {
-  struct local_dma_buf_sync sync_end = {0};
+  struct dma_buf_sync sync_end = {0};
 
-  sync_end.flags = LOCAL_DMA_BUF_SYNC_END | LOCAL_DMA_BUF_SYNC_RW;
+  sync_end.flags = DMA_BUF_SYNC_END | DMA_BUF_SYNC_RW;
 #if DCHECK_IS_ON()
   int rv =
 #endif
-      drmIoctl(dmabuf_fd, LOCAL_DMA_BUF_IOCTL_SYNC, &sync_end);
+      HANDLE_EINTR(ioctl(dmabuf_fd, DMA_BUF_IOCTL_SYNC, &sync_end));

[marcheu, 2017/04/06 22:47:10]

HANDLE_EINTR can still return an error in non-debug builds, can you always
handle this error? I don't know why drmIoctl wasn't checked before, but
especially in a world where we have weird dmabuf crashes it's not a good idea.

[gurchetansingh, 2017/04/06 22:52:02]

Not checking the return value was added b/c of crbug.com/705189. Since that bug
has been resolved, we can check the return code again.

[dshwang, 2017/04/07 01:32:20]

In addition, haswell dump lots of err message:
https://bugs.chromium.org/p/chromium/issues/detail?id=641392
It's why reveman landed https://codereview.chromium.org/2774583002/

When dma-buf sync ioctl is stable enough, I'll remove dcheck guard.

   DPLOG_IF(ERROR, rv) << "Failed DMA_BUF_SYNC_END";
 }