Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH v2] PCI: Reprogram bridge prefetch registers on resume
From: Daniel Drake @ 2018-09-12  9:37 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Bjorn Helgaas, Linux PCI, Linux Upstreaming Team, nouveau,
	Linux PM, Peter Wu, kherbst, Rafael Wysocki, Keith Busch,
	Mika Westerberg, Jon Derrick, Thomas Martitz, David Miller,
	Heiner Kallweit, netdev, nic_swsd, rchang
In-Reply-To: <CAJZ5v0j6VcFbO_R3fGLi2Ec1DmWsVA-0mzDG3LrA-3D5CrsPbw@mail.gmail.com>

On Wed, Sep 12, 2018 at 5:05 PM, Rafael J. Wysocki <rafael@kernel.org> wrote:
> Passing the extra bool around is somewhat clumsy, but I haven't found
> a cleaner way to do it, so

Thanks, according to your suggestion (which I plan to follow after
this) we will remove this parameter for v4.20+ and have all the
register writes be forced regardless of current value.

Daniel

^ permalink raw reply

* Re: [PATCH net-next 0/8] bnxt_en: devlink param updates
From: Jakub Kicinski @ 2018-09-12  9:50 UTC (permalink / raw)
  To: Vasundhara Volam
  Cc: David Miller, michael.chan@broadcom.com, Netdev, alexander.duyck
In-Reply-To: <CAACQVJr+6uNeyQ=RUF280V39=AYjJD8EJpun0iO4ECaXVt9MMA@mail.gmail.com>

On Wed, 12 Sep 2018 12:09:37 +0530, Vasundhara Volam wrote:
> On Tue, Sep 11, 2018 at 5:04 PM Jakub Kicinski wrote:
> > On Tue, 11 Sep 2018 14:14:57 +0530, Vasundhara Volam wrote:  
> > > This patchset adds support for 4 generic and 1 driver-specific devlink
> > > parameters.
> > >
> > > Also, this patchset adds support to return proper error code if
> > > HWRM_NVM_GET/SET_VARIABLE commands return error code
> > > HWRM_ERR_CODE_RESOURCE_ACCESS_DENIED.
> > >
> > > Vasundhara Volam (8):
> > >   devlink: Add generic parameter hw_tc_offload  
> >
> > Much like Jiri, I can't help but wonder why do you need this?  
>
> There is a request from our customer for a way to toggle tc_offload
> feature in our adapter.

Vasundhara, again, we don't need to know who asked you to do this, but
_why_.  What problem are you solving?  What is the customer trying to
achieve?

> > >   devlink: Add generic parameter ignore_ari
> > >   devlink: Add generic parameter msix_vec_per_pf_max
> > >   devlink: Add generic parameter msix_vec_per_pf_min  
> >
> > IMHO more structured API would be preferable if possible.  The string
> > keys won't scale if you want to set the parameters per PF, and
> > creating more structured API for PCIe which is a relatively slow
> > moving HW spec seems tractable.  
>
> Sorry, could you please suggest an example? We will try to adapt.

My thinking was that the same way devlink device has ports, it should
have PCIe functions as objects which then have attributes.  Instead of
making everything a string-identified device attribute.  But I'm not
dead set on this if others don't think its a good idea.

^ permalink raw reply

* [PATCH v2 02/17] compat_ioctl: move drivers to generic_compat_ioctl_ptrarg
From: Arnd Bergmann @ 2018-09-12 15:01 UTC (permalink / raw)
  To: viro
  Cc: kvm, Alexander Shishkin, Jarkko Sakkinen, virtualization,
	Benjamin Tissoires, linux-mtd, Peter Huewe, linux1394-devel,
	devel, Jason Gunthorpe, Marek Vasut, linux-input, Tomas Winkler,
	Arnd Bergmann, Jiri Kosina, Alex Williamson, OGAWA Hirofumi,
	Artem Bityutskiy, Greg Kroah-Hartman, linux-usb, linux-kernel,
	Sudip Mukherjee, Stefan Richter
In-Reply-To: <20180912150142.157913-1-arnd@arndb.de>

Each of these drivers has a copy of the same trivial helper function to
convert the pointer argument and then call the native ioctl handler.

We now have a generic implementation of that, so use it.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/char/ppdev.c              | 12 +---------
 drivers/char/tpm/tpm_vtpm_proxy.c | 12 +---------
 drivers/firewire/core-cdev.c      | 12 +---------
 drivers/hid/usbhid/hiddev.c       | 11 +--------
 drivers/hwtracing/stm/core.c      | 12 +---------
 drivers/misc/mei/main.c           | 22 +----------------
 drivers/mtd/ubi/cdev.c            | 36 +++-------------------------
 drivers/net/tap.c                 | 12 +---------
 drivers/staging/pi433/pi433_if.c  | 12 +---------
 drivers/usb/core/devio.c          | 16 +------------
 drivers/vfio/vfio.c               | 39 +++----------------------------
 drivers/vhost/net.c               | 12 +---------
 drivers/vhost/scsi.c              | 12 +---------
 drivers/vhost/test.c              | 12 +---------
 drivers/vhost/vsock.c             | 12 +---------
 fs/fat/file.c                     | 13 +----------
 16 files changed, 20 insertions(+), 237 deletions(-)

diff --git a/drivers/char/ppdev.c b/drivers/char/ppdev.c
index 1ae77b41050a..c38a62457cf0 100644
--- a/drivers/char/ppdev.c
+++ b/drivers/char/ppdev.c
@@ -674,14 +674,6 @@ static long pp_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 	return ret;
 }
 
-#ifdef CONFIG_COMPAT
-static long pp_compat_ioctl(struct file *file, unsigned int cmd,
-			    unsigned long arg)
-{
-	return pp_ioctl(file, cmd, (unsigned long)compat_ptr(arg));
-}
-#endif
-
 static int pp_open(struct inode *inode, struct file *file)
 {
 	unsigned int minor = iminor(inode);
@@ -790,9 +782,7 @@ static const struct file_operations pp_fops = {
 	.write		= pp_write,
 	.poll		= pp_poll,
 	.unlocked_ioctl	= pp_ioctl,
-#ifdef CONFIG_COMPAT
-	.compat_ioctl   = pp_compat_ioctl,
-#endif
+	.compat_ioctl   = generic_compat_ioctl_ptrarg,
 	.open		= pp_open,
 	.release	= pp_release,
 };
diff --git a/drivers/char/tpm/tpm_vtpm_proxy.c b/drivers/char/tpm/tpm_vtpm_proxy.c
index 87a0ce47f201..a170f5ca7416 100644
--- a/drivers/char/tpm/tpm_vtpm_proxy.c
+++ b/drivers/char/tpm/tpm_vtpm_proxy.c
@@ -678,20 +678,10 @@ static long vtpmx_fops_ioctl(struct file *f, unsigned int ioctl,
 	}
 }
 
-#ifdef CONFIG_COMPAT
-static long vtpmx_fops_compat_ioctl(struct file *f, unsigned int ioctl,
-					  unsigned long arg)
-{
-	return vtpmx_fops_ioctl(f, ioctl, (unsigned long)compat_ptr(arg));
-}
-#endif
-
 static const struct file_operations vtpmx_fops = {
 	.owner = THIS_MODULE,
 	.unlocked_ioctl = vtpmx_fops_ioctl,
-#ifdef CONFIG_COMPAT
-	.compat_ioctl = vtpmx_fops_compat_ioctl,
-#endif
+	.compat_ioctl = generic_compat_ioctl_ptrarg,
 	.llseek = noop_llseek,
 };
 
diff --git a/drivers/firewire/core-cdev.c b/drivers/firewire/core-cdev.c
index d8e185582642..2acc0c9ddf94 100644
--- a/drivers/firewire/core-cdev.c
+++ b/drivers/firewire/core-cdev.c
@@ -1659,14 +1659,6 @@ static long fw_device_op_ioctl(struct file *file,
 	return dispatch_ioctl(file->private_data, cmd, (void __user *)arg);
 }
 
-#ifdef CONFIG_COMPAT
-static long fw_device_op_compat_ioctl(struct file *file,
-				      unsigned int cmd, unsigned long arg)
-{
-	return dispatch_ioctl(file->private_data, cmd, compat_ptr(arg));
-}
-#endif
-
 static int fw_device_op_mmap(struct file *file, struct vm_area_struct *vma)
 {
 	struct client *client = file->private_data;
@@ -1808,7 +1800,5 @@ const struct file_operations fw_device_ops = {
 	.mmap		= fw_device_op_mmap,
 	.release	= fw_device_op_release,
 	.poll		= fw_device_op_poll,
-#ifdef CONFIG_COMPAT
-	.compat_ioctl	= fw_device_op_compat_ioctl,
-#endif
+	.compat_ioctl	= generic_compat_ioctl_ptrarg,
 };
diff --git a/drivers/hid/usbhid/hiddev.c b/drivers/hid/usbhid/hiddev.c
index 23872d08308c..73a168f97024 100644
--- a/drivers/hid/usbhid/hiddev.c
+++ b/drivers/hid/usbhid/hiddev.c
@@ -845,13 +845,6 @@ static long hiddev_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 	return r;
 }
 
-#ifdef CONFIG_COMPAT
-static long hiddev_compat_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
-{
-	return hiddev_ioctl(file, cmd, (unsigned long)compat_ptr(arg));
-}
-#endif
-
 static const struct file_operations hiddev_fops = {
 	.owner =	THIS_MODULE,
 	.read =		hiddev_read,
@@ -861,9 +854,7 @@ static const struct file_operations hiddev_fops = {
 	.release =	hiddev_release,
 	.unlocked_ioctl =	hiddev_ioctl,
 	.fasync =	hiddev_fasync,
-#ifdef CONFIG_COMPAT
-	.compat_ioctl	= hiddev_compat_ioctl,
-#endif
+	.compat_ioctl	= generic_compat_ioctl_ptrarg,
 	.llseek		= noop_llseek,
 };
 
diff --git a/drivers/hwtracing/stm/core.c b/drivers/hwtracing/stm/core.c
index 10bcb5d73f90..3f5cbb948781 100644
--- a/drivers/hwtracing/stm/core.c
+++ b/drivers/hwtracing/stm/core.c
@@ -651,23 +651,13 @@ stm_char_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 	return err;
 }
 
-#ifdef CONFIG_COMPAT
-static long
-stm_char_compat_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
-{
-	return stm_char_ioctl(file, cmd, (unsigned long)compat_ptr(arg));
-}
-#else
-#define stm_char_compat_ioctl	NULL
-#endif
-
 static const struct file_operations stm_fops = {
 	.open		= stm_char_open,
 	.release	= stm_char_release,
 	.write		= stm_char_write,
 	.mmap		= stm_char_mmap,
 	.unlocked_ioctl	= stm_char_ioctl,
-	.compat_ioctl	= stm_char_compat_ioctl,
+	.compat_ioctl	= generic_compat_ioctl_ptrarg,
 	.llseek		= no_llseek,
 };
 
diff --git a/drivers/misc/mei/main.c b/drivers/misc/mei/main.c
index 4d77a6ae183a..a26b645e432f 100644
--- a/drivers/misc/mei/main.c
+++ b/drivers/misc/mei/main.c
@@ -535,24 +535,6 @@ static long mei_ioctl(struct file *file, unsigned int cmd, unsigned long data)
 	return rets;
 }
 
-/**
- * mei_compat_ioctl - the compat IOCTL function
- *
- * @file: pointer to file structure
- * @cmd: ioctl command
- * @data: pointer to mei message structure
- *
- * Return: 0 on success , <0 on error
- */
-#ifdef CONFIG_COMPAT
-static long mei_compat_ioctl(struct file *file,
-			unsigned int cmd, unsigned long data)
-{
-	return mei_ioctl(file, cmd, (unsigned long)compat_ptr(data));
-}
-#endif
-
-
 /**
  * mei_poll - the poll function
  *
@@ -855,9 +837,7 @@ static const struct file_operations mei_fops = {
 	.owner = THIS_MODULE,
 	.read = mei_read,
 	.unlocked_ioctl = mei_ioctl,
-#ifdef CONFIG_COMPAT
-	.compat_ioctl = mei_compat_ioctl,
-#endif
+	.compat_ioctl = generic_compat_ioctl_ptrarg,
 	.open = mei_open,
 	.release = mei_release,
 	.write = mei_write,
diff --git a/drivers/mtd/ubi/cdev.c b/drivers/mtd/ubi/cdev.c
index 22547d7a84ea..2aad1da86acc 100644
--- a/drivers/mtd/ubi/cdev.c
+++ b/drivers/mtd/ubi/cdev.c
@@ -1061,36 +1061,6 @@ static long ctrl_cdev_ioctl(struct file *file, unsigned int cmd,
 	return err;
 }
 
-#ifdef CONFIG_COMPAT
-static long vol_cdev_compat_ioctl(struct file *file, unsigned int cmd,
-				  unsigned long arg)
-{
-	unsigned long translated_arg = (unsigned long)compat_ptr(arg);
-
-	return vol_cdev_ioctl(file, cmd, translated_arg);
-}
-
-static long ubi_cdev_compat_ioctl(struct file *file, unsigned int cmd,
-				  unsigned long arg)
-{
-	unsigned long translated_arg = (unsigned long)compat_ptr(arg);
-
-	return ubi_cdev_ioctl(file, cmd, translated_arg);
-}
-
-static long ctrl_cdev_compat_ioctl(struct file *file, unsigned int cmd,
-				   unsigned long arg)
-{
-	unsigned long translated_arg = (unsigned long)compat_ptr(arg);
-
-	return ctrl_cdev_ioctl(file, cmd, translated_arg);
-}
-#else
-#define vol_cdev_compat_ioctl  NULL
-#define ubi_cdev_compat_ioctl  NULL
-#define ctrl_cdev_compat_ioctl NULL
-#endif
-
 /* UBI volume character device operations */
 const struct file_operations ubi_vol_cdev_operations = {
 	.owner          = THIS_MODULE,
@@ -1101,7 +1071,7 @@ const struct file_operations ubi_vol_cdev_operations = {
 	.write          = vol_cdev_write,
 	.fsync		= vol_cdev_fsync,
 	.unlocked_ioctl = vol_cdev_ioctl,
-	.compat_ioctl   = vol_cdev_compat_ioctl,
+	.compat_ioctl   = generic_compat_ioctl_ptrarg,
 };
 
 /* UBI character device operations */
@@ -1109,13 +1079,13 @@ const struct file_operations ubi_cdev_operations = {
 	.owner          = THIS_MODULE,
 	.llseek         = no_llseek,
 	.unlocked_ioctl = ubi_cdev_ioctl,
-	.compat_ioctl   = ubi_cdev_compat_ioctl,
+	.compat_ioctl   = generic_compat_ioctl_ptrarg,
 };
 
 /* UBI control character device operations */
 const struct file_operations ubi_ctrl_cdev_operations = {
 	.owner          = THIS_MODULE,
 	.unlocked_ioctl = ctrl_cdev_ioctl,
-	.compat_ioctl   = ctrl_cdev_compat_ioctl,
+	.compat_ioctl   = generic_compat_ioctl_ptrarg,
 	.llseek		= no_llseek,
 };
diff --git a/drivers/net/tap.c b/drivers/net/tap.c
index f0f7cd977667..720deb07f2b4 100644
--- a/drivers/net/tap.c
+++ b/drivers/net/tap.c
@@ -1124,14 +1124,6 @@ static long tap_ioctl(struct file *file, unsigned int cmd,
 	}
 }
 
-#ifdef CONFIG_COMPAT
-static long tap_compat_ioctl(struct file *file, unsigned int cmd,
-			     unsigned long arg)
-{
-	return tap_ioctl(file, cmd, (unsigned long)compat_ptr(arg));
-}
-#endif
-
 static const struct file_operations tap_fops = {
 	.owner		= THIS_MODULE,
 	.open		= tap_open,
@@ -1141,9 +1133,7 @@ static const struct file_operations tap_fops = {
 	.poll		= tap_poll,
 	.llseek		= no_llseek,
 	.unlocked_ioctl	= tap_ioctl,
-#ifdef CONFIG_COMPAT
-	.compat_ioctl	= tap_compat_ioctl,
-#endif
+	.compat_ioctl	= generic_compat_ioctl_ptrarg,
 };
 
 static int tap_sendmsg(struct socket *sock, struct msghdr *m,
diff --git a/drivers/staging/pi433/pi433_if.c b/drivers/staging/pi433/pi433_if.c
index c85a805a1243..9e4caf7ad384 100644
--- a/drivers/staging/pi433/pi433_if.c
+++ b/drivers/staging/pi433/pi433_if.c
@@ -945,16 +945,6 @@ pi433_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
 	return retval;
 }
 
-#ifdef CONFIG_COMPAT
-static long
-pi433_compat_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
-{
-	return pi433_ioctl(filp, cmd, (unsigned long)compat_ptr(arg));
-}
-#else
-#define pi433_compat_ioctl NULL
-#endif /* CONFIG_COMPAT */
-
 /*-------------------------------------------------------------------------*/
 
 static int pi433_open(struct inode *inode, struct file *filp)
@@ -1111,7 +1101,7 @@ static const struct file_operations pi433_fops = {
 	.write =	pi433_write,
 	.read =		pi433_read,
 	.unlocked_ioctl = pi433_ioctl,
-	.compat_ioctl = pi433_compat_ioctl,
+	.compat_ioctl = generic_compat_ioctl_ptrarg,
 	.open =		pi433_open,
 	.release =	pi433_release,
 	.llseek =	no_llseek,
diff --git a/drivers/usb/core/devio.c b/drivers/usb/core/devio.c
index 6ce77b33da61..269e0befba2d 100644
--- a/drivers/usb/core/devio.c
+++ b/drivers/usb/core/devio.c
@@ -2553,18 +2553,6 @@ static long usbdev_ioctl(struct file *file, unsigned int cmd,
 	return ret;
 }
 
-#ifdef CONFIG_COMPAT
-static long usbdev_compat_ioctl(struct file *file, unsigned int cmd,
-			unsigned long arg)
-{
-	int ret;
-
-	ret = usbdev_do_ioctl(file, cmd, compat_ptr(arg));
-
-	return ret;
-}
-#endif
-
 /* No kernel lock - fine */
 static __poll_t usbdev_poll(struct file *file,
 				struct poll_table_struct *wait)
@@ -2588,9 +2576,7 @@ const struct file_operations usbdev_file_operations = {
 	.read =		  usbdev_read,
 	.poll =		  usbdev_poll,
 	.unlocked_ioctl = usbdev_ioctl,
-#ifdef CONFIG_COMPAT
-	.compat_ioctl =   usbdev_compat_ioctl,
-#endif
+	.compat_ioctl =   generic_compat_ioctl_ptrarg,
 	.mmap =           usbdev_mmap,
 	.open =		  usbdev_open,
 	.release =	  usbdev_release,
diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c
index 64833879f75d..79f08a99602d 100644
--- a/drivers/vfio/vfio.c
+++ b/drivers/vfio/vfio.c
@@ -1200,15 +1200,6 @@ static long vfio_fops_unl_ioctl(struct file *filep,
 	return ret;
 }
 
-#ifdef CONFIG_COMPAT
-static long vfio_fops_compat_ioctl(struct file *filep,
-				   unsigned int cmd, unsigned long arg)
-{
-	arg = (unsigned long)compat_ptr(arg);
-	return vfio_fops_unl_ioctl(filep, cmd, arg);
-}
-#endif	/* CONFIG_COMPAT */
-
 static int vfio_fops_open(struct inode *inode, struct file *filep)
 {
 	struct vfio_container *container;
@@ -1291,9 +1282,7 @@ static const struct file_operations vfio_fops = {
 	.read		= vfio_fops_read,
 	.write		= vfio_fops_write,
 	.unlocked_ioctl	= vfio_fops_unl_ioctl,
-#ifdef CONFIG_COMPAT
-	.compat_ioctl	= vfio_fops_compat_ioctl,
-#endif
+	.compat_ioctl	= generic_compat_ioctl_ptrarg,
 	.mmap		= vfio_fops_mmap,
 };
 
@@ -1572,15 +1561,6 @@ static long vfio_group_fops_unl_ioctl(struct file *filep,
 	return ret;
 }
 
-#ifdef CONFIG_COMPAT
-static long vfio_group_fops_compat_ioctl(struct file *filep,
-					 unsigned int cmd, unsigned long arg)
-{
-	arg = (unsigned long)compat_ptr(arg);
-	return vfio_group_fops_unl_ioctl(filep, cmd, arg);
-}
-#endif	/* CONFIG_COMPAT */
-
 static int vfio_group_fops_open(struct inode *inode, struct file *filep)
 {
 	struct vfio_group *group;
@@ -1636,9 +1616,7 @@ static int vfio_group_fops_release(struct inode *inode, struct file *filep)
 static const struct file_operations vfio_group_fops = {
 	.owner		= THIS_MODULE,
 	.unlocked_ioctl	= vfio_group_fops_unl_ioctl,
-#ifdef CONFIG_COMPAT
-	.compat_ioctl	= vfio_group_fops_compat_ioctl,
-#endif
+	.compat_ioctl	= generic_compat_ioctl_ptrarg,
 	.open		= vfio_group_fops_open,
 	.release	= vfio_group_fops_release,
 };
@@ -1703,24 +1681,13 @@ static int vfio_device_fops_mmap(struct file *filep, struct vm_area_struct *vma)
 	return device->ops->mmap(device->device_data, vma);
 }
 
-#ifdef CONFIG_COMPAT
-static long vfio_device_fops_compat_ioctl(struct file *filep,
-					  unsigned int cmd, unsigned long arg)
-{
-	arg = (unsigned long)compat_ptr(arg);
-	return vfio_device_fops_unl_ioctl(filep, cmd, arg);
-}
-#endif	/* CONFIG_COMPAT */
-
 static const struct file_operations vfio_device_fops = {
 	.owner		= THIS_MODULE,
 	.release	= vfio_device_fops_release,
 	.read		= vfio_device_fops_read,
 	.write		= vfio_device_fops_write,
 	.unlocked_ioctl	= vfio_device_fops_unl_ioctl,
-#ifdef CONFIG_COMPAT
-	.compat_ioctl	= vfio_device_fops_compat_ioctl,
-#endif
+	.compat_ioctl	= generic_compat_ioctl_ptrarg,
 	.mmap		= vfio_device_fops_mmap,
 };
 
diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
index 4e656f89cb22..e9624350f6a5 100644
--- a/drivers/vhost/net.c
+++ b/drivers/vhost/net.c
@@ -1535,14 +1535,6 @@ static long vhost_net_ioctl(struct file *f, unsigned int ioctl,
 	}
 }
 
-#ifdef CONFIG_COMPAT
-static long vhost_net_compat_ioctl(struct file *f, unsigned int ioctl,
-				   unsigned long arg)
-{
-	return vhost_net_ioctl(f, ioctl, (unsigned long)compat_ptr(arg));
-}
-#endif
-
 static ssize_t vhost_net_chr_read_iter(struct kiocb *iocb, struct iov_iter *to)
 {
 	struct file *file = iocb->ki_filp;
@@ -1578,9 +1570,7 @@ static const struct file_operations vhost_net_fops = {
 	.write_iter     = vhost_net_chr_write_iter,
 	.poll           = vhost_net_chr_poll,
 	.unlocked_ioctl = vhost_net_ioctl,
-#ifdef CONFIG_COMPAT
-	.compat_ioctl   = vhost_net_compat_ioctl,
-#endif
+	.compat_ioctl   = generic_compat_ioctl_ptrarg,
 	.open           = vhost_net_open,
 	.llseek		= noop_llseek,
 };
diff --git a/drivers/vhost/scsi.c b/drivers/vhost/scsi.c
index c24bb690680b..9180d38de353 100644
--- a/drivers/vhost/scsi.c
+++ b/drivers/vhost/scsi.c
@@ -1495,21 +1495,11 @@ vhost_scsi_ioctl(struct file *f,
 	}
 }
 
-#ifdef CONFIG_COMPAT
-static long vhost_scsi_compat_ioctl(struct file *f, unsigned int ioctl,
-				unsigned long arg)
-{
-	return vhost_scsi_ioctl(f, ioctl, (unsigned long)compat_ptr(arg));
-}
-#endif
-
 static const struct file_operations vhost_scsi_fops = {
 	.owner          = THIS_MODULE,
 	.release        = vhost_scsi_release,
 	.unlocked_ioctl = vhost_scsi_ioctl,
-#ifdef CONFIG_COMPAT
-	.compat_ioctl	= vhost_scsi_compat_ioctl,
-#endif
+	.compat_ioctl	= generic_compat_ioctl_ptrarg,
 	.open           = vhost_scsi_open,
 	.llseek		= noop_llseek,
 };
diff --git a/drivers/vhost/test.c b/drivers/vhost/test.c
index 40589850eb33..0b185b4712fb 100644
--- a/drivers/vhost/test.c
+++ b/drivers/vhost/test.c
@@ -298,21 +298,11 @@ static long vhost_test_ioctl(struct file *f, unsigned int ioctl,
 	}
 }
 
-#ifdef CONFIG_COMPAT
-static long vhost_test_compat_ioctl(struct file *f, unsigned int ioctl,
-				   unsigned long arg)
-{
-	return vhost_test_ioctl(f, ioctl, (unsigned long)compat_ptr(arg));
-}
-#endif
-
 static const struct file_operations vhost_test_fops = {
 	.owner          = THIS_MODULE,
 	.release        = vhost_test_release,
 	.unlocked_ioctl = vhost_test_ioctl,
-#ifdef CONFIG_COMPAT
-	.compat_ioctl   = vhost_test_compat_ioctl,
-#endif
+	.compat_ioctl   = generic_compat_ioctl_ptrarg,
 	.open           = vhost_test_open,
 	.llseek		= noop_llseek,
 };
diff --git a/drivers/vhost/vsock.c b/drivers/vhost/vsock.c
index 34bc3ab40c6d..83c60f3a9c09 100644
--- a/drivers/vhost/vsock.c
+++ b/drivers/vhost/vsock.c
@@ -699,23 +699,13 @@ static long vhost_vsock_dev_ioctl(struct file *f, unsigned int ioctl,
 	}
 }
 
-#ifdef CONFIG_COMPAT
-static long vhost_vsock_dev_compat_ioctl(struct file *f, unsigned int ioctl,
-					 unsigned long arg)
-{
-	return vhost_vsock_dev_ioctl(f, ioctl, (unsigned long)compat_ptr(arg));
-}
-#endif
-
 static const struct file_operations vhost_vsock_fops = {
 	.owner          = THIS_MODULE,
 	.open           = vhost_vsock_dev_open,
 	.release        = vhost_vsock_dev_release,
 	.llseek		= noop_llseek,
 	.unlocked_ioctl = vhost_vsock_dev_ioctl,
-#ifdef CONFIG_COMPAT
-	.compat_ioctl   = vhost_vsock_dev_compat_ioctl,
-#endif
+	.compat_ioctl   = generic_compat_ioctl_ptrarg,
 };
 
 static struct miscdevice vhost_vsock_misc = {
diff --git a/fs/fat/file.c b/fs/fat/file.c
index 4f3d72fb1e60..c52c9e9ca36b 100644
--- a/fs/fat/file.c
+++ b/fs/fat/file.c
@@ -171,15 +171,6 @@ long fat_generic_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
 	}
 }
 
-#ifdef CONFIG_COMPAT
-static long fat_generic_compat_ioctl(struct file *filp, unsigned int cmd,
-				      unsigned long arg)
-
-{
-	return fat_generic_ioctl(filp, cmd, (unsigned long)compat_ptr(arg));
-}
-#endif
-
 static int fat_file_release(struct inode *inode, struct file *filp)
 {
 	if ((filp->f_mode & FMODE_WRITE) &&
@@ -209,9 +200,7 @@ const struct file_operations fat_file_operations = {
 	.mmap		= generic_file_mmap,
 	.release	= fat_file_release,
 	.unlocked_ioctl	= fat_generic_ioctl,
-#ifdef CONFIG_COMPAT
-	.compat_ioctl	= fat_generic_compat_ioctl,
-#endif
+	.compat_ioctl	= generic_compat_ioctl_ptrarg,
 	.fsync		= fat_file_fsync,
 	.splice_read	= generic_file_splice_read,
 	.fallocate	= fat_fallocate,
-- 
2.18.0


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

^ permalink raw reply related

* Re: general protection fault in tcp_cleanup_ulp
From: syzbot @ 2018-09-12 10:00 UTC (permalink / raw)
  To: davem, edumazet, kuznet, linux-kernel, netdev, syzkaller-bugs,
	yoshfuji
In-Reply-To: <00000000000006602605752ffa1a@google.com>

syzbot has found a reproducer for the following crash on:

HEAD commit:    28619527b8a7 Merge git://git.kernel.org/pub/scm/linux/kern..
git tree:       bpf
console output: https://syzkaller.appspot.com/x/log.txt?x=127f5b49400000
kernel config:  https://syzkaller.appspot.com/x/.config?x=8f59875069d721b6
dashboard link: https://syzkaller.appspot.com/bug?extid=0b3ccd4f62dac2cf3a7d
compiler:       gcc (GCC) 8.0.1 20180413 (experimental)
syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=13537269400000

IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: syzbot+0b3ccd4f62dac2cf3a7d@syzkaller.appspotmail.com

kasan: CONFIG_KASAN_INLINE enabled
kasan: GPF could be caused by NULL-ptr deref or user memory access
general protection fault: 0000 [#1] PREEMPT SMP KASAN
CPU: 0 PID: 18627 Comm: syz-executor0 Not tainted 4.19.0-rc2+ #51
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS  
Google 01/01/2011
RIP: 0010:tcp_cleanup_ulp+0xbe/0x140 net/ipv4/tcp_ulp.c:131
Code: 02 00 0f 85 8a 00 00 00 49 8b 9c 24 88 06 00 00 e8 b7 43 ec fa 48 8d  
7b 38 48 b8 00 00 00 00 00 fc ff df 48 89 fa 48 c1 ea 03 <80> 3c 02 00 75  
51 48 8b 7b 38 e8 b3 a2 e0 fa 4c 89 ea 48 b8 00 00
RSP: 0018:ffff8801b92f76c0 EFLAGS: 00010202
RAX: dffffc0000000000 RBX: 0000000000000000 RCX: 1ffff10037e84da6
RDX: 0000000000000007 RSI: ffffffff86928b79 RDI: 0000000000000038
RBP: ffff8801b92f76e0 R08: 0000000000000000 R09: ffffed003b5c4732
R10: ffffed003b5c4732 R11: ffff8801dae23993 R12: ffff8801be338f40
R13: ffff8801be3395c8 R14: ffffffff81938ae0 R15: ffff8801b92f7800
FS:  0000000001a06940(0000) GS:ffff8801dae00000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00007f4d5d4b8db8 CR3: 00000001bc421000 CR4: 00000000001406f0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
Call Trace:
  tcp_v4_destroy_sock+0x161/0x990 net/ipv4/tcp_ipv4.c:1978
  tcp_v6_destroy_sock+0x15/0x20 net/ipv6/tcp_ipv6.c:1762
  inet_csk_destroy_sock+0x19f/0x440 net/ipv4/inet_connection_sock.c:835
  tcp_close+0xba7/0x1300 net/ipv4/tcp.c:2477
  bpf_tcp_close+0x93c/0x10c0 kernel/bpf/sockmap.c:384
  inet_release+0x104/0x1f0 net/ipv4/af_inet.c:428
  inet6_release+0x50/0x70 net/ipv6/af_inet6.c:457
  __sock_release+0xd7/0x250 net/socket.c:579
  sock_close+0x19/0x20 net/socket.c:1139
  __fput+0x385/0xa30 fs/file_table.c:278
  ____fput+0x15/0x20 fs/file_table.c:309
  task_work_run+0x1e8/0x2a0 kernel/task_work.c:113
  tracehook_notify_resume include/linux/tracehook.h:193 [inline]
  exit_to_usermode_loop+0x318/0x380 arch/x86/entry/common.c:166
  prepare_exit_to_usermode arch/x86/entry/common.c:197 [inline]
  syscall_return_slowpath arch/x86/entry/common.c:268 [inline]
  do_syscall_64+0x6be/0x820 arch/x86/entry/common.c:293
  entry_SYSCALL_64_after_hwframe+0x49/0xbe
RIP: 0033:0x410e91
Code: 75 14 b8 03 00 00 00 0f 05 48 3d 01 f0 ff ff 0f 83 34 19 00 00 c3 48  
83 ec 08 e8 0a fc ff ff 48 89 04 24 b8 03 00 00 00 0f 05 <48> 8b 3c 24 48  
89 c2 e8 53 fc ff ff 48 89 d0 48 83 c4 08 48 3d 01
RSP: 002b:00007ffd70bafde0 EFLAGS: 00000293 ORIG_RAX: 0000000000000003
RAX: 0000000000000000 RBX: 0000000000000006 RCX: 0000000000410e91
RDX: 0000000000000000 RSI: 0000000000731160 RDI: 0000000000000005
RBP: 0000000000000000 R08: 0000000000000000 R09: ffffffffffffffff
R10: 00000000009300a0 R11: 0000000000000293 R12: 0000000000000008
R13: 00000000000dc3fa R14: 0000000000000875 R15: badc0ffeebadface
Modules linked in:
Dumping ftrace buffer:
    (ftrace buffer empty)
---[ end trace d9e209a7bdcaec58 ]---
RIP: 0010:tcp_cleanup_ulp+0xbe/0x140 net/ipv4/tcp_ulp.c:131
Code: 02 00 0f 85 8a 00 00 00 49 8b 9c 24 88 06 00 00 e8 b7 43 ec fa 48 8d  
7b 38 48 b8 00 00 00 00 00 fc ff df 48 89 fa 48 c1 ea 03 <80> 3c 02 00 75  
51 48 8b 7b 38 e8 b3 a2 e0 fa 4c 89 ea 48 b8 00 00
RSP: 0018:ffff8801b92f76c0 EFLAGS: 00010202
RAX: dffffc0000000000 RBX: 0000000000000000 RCX: 1ffff10037e84da6
RDX: 0000000000000007 RSI: ffffffff86928b79 RDI: 0000000000000038
RBP: ffff8801b92f76e0 R08: 0000000000000000 R09: ffffed003b5c4732
R10: ffffed003b5c4732 R11: ffff8801dae23993 R12: ffff8801be338f40
R13: ffff8801be3395c8 R14: ffffffff81938ae0 R15: ffff8801b92f7800
FS:  0000000001a06940(0000) GS:ffff8801dae00000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00007f4d5d4b8db8 CR3: 00000001bc421000 CR4: 00000000001406f0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400

^ permalink raw reply

* Re: [PATCH v2] ethernet: hnae: drop adhoc assert() macros
From: Igor Stoppa @ 2018-09-12 15:04 UTC (permalink / raw)
  To: David Miller
  Cc: igor.stoppa, huangdaode, yisen.zhuang, salil.mehta, netdev,
	linux-kernel
In-Reply-To: <20180911.230716.1048852895308106303.davem@davemloft.net>

On 12/09/18 09:07, David Miller wrote:
> From: Igor Stoppa <igor.stoppa@gmail.com>
> Date: Sat,  8 Sep 2018 18:01:42 +0300
> 
>> Replace assert() with a less misleading test_condition() using WARN()
>> Drop one check which had bitrotted and didn't compile anymore.
>>
>> Signed-off-by: Igor Stoppa <igor.stoppa@huawei.com>
> 
> I'm still kind of not happy about this.
> 
> Make the driver use kernel interfaces like WARN_ON_ONCE()
> etc. directly instead of defining alias CPP macros private to the
> driver.
> 
> If it needs to be conditional upon DEBUG, we have pr_debug() and
> the likes as well.

WARN_ON_ONCE() will display the error message only once, but from its 
implementation it looks like it will evaluate the condition anyways, 
every time, unless the compiler can do some magic and somehow know that 
the condition was true already onece (/include/asm-generic/bug.h:146)

pr_debug() otoh, will print or not print, depending on 
CONFIG_DYNAMIC_DEBUG being set or not (/include/linux/printk.h:399)

but the logic we are trying to clean up here seems a bit different:

if (in_debug_mode && error_condition_is_verified)
	generate_warning()

Which, I guess, is meant to completely remove *any* test outside of 
debug mode.

That is why I used the test_condition() macro, instead of using directly 
WARN_ON_ONCE().  Admittedly, it probably would have been better to 
depend on CONFIG_DYNAMIC_DEBUG.

Neither using WARN_ON_ONCE(), nor pr_debug() seem to replicate the 
initial behavior of the code that is currently in the tree, with assert().

How would you suggest that I keep the same behavior, without the helper 
macro? I am probably missing it, but I do not see some facility that 
would support the double condition I described above.

--
thanks, igor

^ permalink raw reply

* [PATCH v2 05/17] compat_ioctl: move more drivers to generic_compat_ioctl_ptrarg
From: Arnd Bergmann @ 2018-09-12 15:08 UTC (permalink / raw)
  To: viro-RmSDqhL/yNMiFSDQTTA3OLVCufUGDwFn
  Cc: linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
	linux-iio-u79uwXL29TY76Z2rM5mHXA,
	linux-pci-u79uwXL29TY76Z2rM5mHXA,
	linux-remoteproc-u79uwXL29TY76Z2rM5mHXA,
	linux-nvme-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	platform-driver-x86-u79uwXL29TY76Z2rM5mHXA,
	sparclinux-u79uwXL29TY76Z2rM5mHXA,
	devel-gWbeCf7V1WCQmaza687I9mD2FQJk+8+b,
	linux-scsi-u79uwXL29TY76Z2rM5mHXA,
	linux-nvdimm-hn68Rpc1hR1g9hUCZPvPmw,
	linux-rdma-u79uwXL29TY76Z2rM5mHXA,
	qat-linux-ral2JQCrhuEAvxtiuMwx3w,
	amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
	linux-input-u79uwXL29TY76Z2rM5mHXA,
	linux-media-u79uwXL29TY76Z2rM5mHXA, Arnd Bergmann,
	linaro-mm-sig-cunTk1MwBs8s++Sfvej+rw,
	dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
	ceph-devel-u79uwXL29TY76Z2rM5mHXA, Greg Kroah-Hartman,
	linux-usb-u79uwXL29TY76Z2rM5mHXA,
	linux-wireless-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-crypto-u79uwXL29TY76Z2rM5mHXA,
	netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-fsdevel-u79uwXL29TY76Z2rM5mHXA,
	linuxppc-dev-uLR06cmDAlY/bJ5BZ2RsiQ, David S. Miller,
	linux-btrfs-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20180912150142.157913-1-arnd-r2nGTMty4D4@public.gmane.org>

The .ioctl and .compat_ioctl file operations have the same prototype so
they can both point to the same function, which works great almost all
the time when all the commands are compatible.

One exception is the s390 architecture, where a compat pointer is only
31 bit wide, and converting it into a 64-bit pointer requires calling
compat_ptr(). Most drivers here will ever run in s390, but since we now
have a generic helper for it, it's easy enough to use it consistently.

I double-checked all these drivers to ensure that all ioctl arguments
are used as pointers or are ignored, but are not interpreted as integer
values.

Signed-off-by: Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>
---
 drivers/android/binder.c                    | 2 +-
 drivers/crypto/qat/qat_common/adf_ctl_drv.c | 2 +-
 drivers/dma-buf/dma-buf.c                   | 4 +---
 drivers/dma-buf/sw_sync.c                   | 2 +-
 drivers/dma-buf/sync_file.c                 | 2 +-
 drivers/gpu/drm/amd/amdkfd/kfd_chardev.c    | 2 +-
 drivers/hid/hidraw.c                        | 4 +---
 drivers/iio/industrialio-core.c             | 2 +-
 drivers/infiniband/core/uverbs_main.c       | 4 ++--
 drivers/media/rc/lirc_dev.c                 | 4 +---
 drivers/mfd/cros_ec_dev.c                   | 4 +---
 drivers/misc/vmw_vmci/vmci_host.c           | 2 +-
 drivers/nvdimm/bus.c                        | 4 ++--
 drivers/nvme/host/core.c                    | 2 +-
 drivers/pci/switch/switchtec.c              | 2 +-
 drivers/platform/x86/wmi.c                  | 2 +-
 drivers/rpmsg/rpmsg_char.c                  | 4 ++--
 drivers/sbus/char/display7seg.c             | 2 +-
 drivers/sbus/char/envctrl.c                 | 4 +---
 drivers/scsi/3w-xxxx.c                      | 4 +---
 drivers/scsi/cxlflash/main.c                | 2 +-
 drivers/scsi/esas2r/esas2r_main.c           | 2 +-
 drivers/scsi/pmcraid.c                      | 4 +---
 drivers/staging/android/ion/ion.c           | 4 +---
 drivers/staging/vme/devices/vme_user.c      | 2 +-
 drivers/tee/tee_core.c                      | 2 +-
 drivers/usb/class/cdc-wdm.c                 | 2 +-
 drivers/usb/class/usbtmc.c                  | 4 +---
 drivers/video/fbdev/ps3fb.c                 | 2 +-
 drivers/virt/fsl_hypervisor.c               | 2 +-
 fs/btrfs/super.c                            | 2 +-
 fs/ceph/dir.c                               | 2 +-
 fs/ceph/file.c                              | 2 +-
 fs/fuse/dev.c                               | 2 +-
 fs/notify/fanotify/fanotify_user.c          | 2 +-
 fs/userfaultfd.c                            | 2 +-
 net/rfkill/core.c                           | 2 +-
 37 files changed, 40 insertions(+), 58 deletions(-)

diff --git a/drivers/android/binder.c b/drivers/android/binder.c
index d58763b6b009..d2464f5759f8 100644
--- a/drivers/android/binder.c
+++ b/drivers/android/binder.c
@@ -5576,7 +5576,7 @@ static const struct file_operations binder_fops = {
 	.owner = THIS_MODULE,
 	.poll = binder_poll,
 	.unlocked_ioctl = binder_ioctl,
-	.compat_ioctl = binder_ioctl,
+	.compat_ioctl = generic_compat_ioctl_ptrarg,
 	.mmap = binder_mmap,
 	.open = binder_open,
 	.flush = binder_flush,
diff --git a/drivers/crypto/qat/qat_common/adf_ctl_drv.c b/drivers/crypto/qat/qat_common/adf_ctl_drv.c
index abc7a7f64d64..8ff77a70addc 100644
--- a/drivers/crypto/qat/qat_common/adf_ctl_drv.c
+++ b/drivers/crypto/qat/qat_common/adf_ctl_drv.c
@@ -68,7 +68,7 @@ static long adf_ctl_ioctl(struct file *fp, unsigned int cmd, unsigned long arg);
 static const struct file_operations adf_ctl_ops = {
 	.owner = THIS_MODULE,
 	.unlocked_ioctl = adf_ctl_ioctl,
-	.compat_ioctl = adf_ctl_ioctl,
+	.compat_ioctl = generic_compat_ioctl_ptrarg,
 };
 
 struct adf_ctl_drv_info {
diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
index 13884474d158..a6d7dc4cf7e9 100644
--- a/drivers/dma-buf/dma-buf.c
+++ b/drivers/dma-buf/dma-buf.c
@@ -325,9 +325,7 @@ static const struct file_operations dma_buf_fops = {
 	.llseek		= dma_buf_llseek,
 	.poll		= dma_buf_poll,
 	.unlocked_ioctl	= dma_buf_ioctl,
-#ifdef CONFIG_COMPAT
-	.compat_ioctl	= dma_buf_ioctl,
-#endif
+	.compat_ioctl	= generic_compat_ioctl_ptrarg,
 };
 
 /*
diff --git a/drivers/dma-buf/sw_sync.c b/drivers/dma-buf/sw_sync.c
index 53c1d6d36a64..bc810506d487 100644
--- a/drivers/dma-buf/sw_sync.c
+++ b/drivers/dma-buf/sw_sync.c
@@ -419,5 +419,5 @@ const struct file_operations sw_sync_debugfs_fops = {
 	.open           = sw_sync_debugfs_open,
 	.release        = sw_sync_debugfs_release,
 	.unlocked_ioctl = sw_sync_ioctl,
-	.compat_ioctl	= sw_sync_ioctl,
+	.compat_ioctl	= generic_compat_ioctl_ptrarg,
 };
diff --git a/drivers/dma-buf/sync_file.c b/drivers/dma-buf/sync_file.c
index 35dd06479867..1c64ed60c658 100644
--- a/drivers/dma-buf/sync_file.c
+++ b/drivers/dma-buf/sync_file.c
@@ -488,5 +488,5 @@ static const struct file_operations sync_file_fops = {
 	.release = sync_file_release,
 	.poll = sync_file_poll,
 	.unlocked_ioctl = sync_file_ioctl,
-	.compat_ioctl = sync_file_ioctl,
+	.compat_ioctl = generic_compat_ioctl_ptrarg,
 };
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
index 297b36c26a05..1d7b1e3c3ebe 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
@@ -47,7 +47,7 @@ static const char kfd_dev_name[] = "kfd";
 static const struct file_operations kfd_fops = {
 	.owner = THIS_MODULE,
 	.unlocked_ioctl = kfd_ioctl,
-	.compat_ioctl = kfd_ioctl,
+	.compat_ioctl = generic_compat_ioctl_ptrarg,
 	.open = kfd_open,
 	.mmap = kfd_mmap,
 };
diff --git a/drivers/hid/hidraw.c b/drivers/hid/hidraw.c
index 4a44e48e08b2..e44b64812850 100644
--- a/drivers/hid/hidraw.c
+++ b/drivers/hid/hidraw.c
@@ -476,9 +476,7 @@ static const struct file_operations hidraw_ops = {
 	.release =      hidraw_release,
 	.unlocked_ioctl = hidraw_ioctl,
 	.fasync =	hidraw_fasync,
-#ifdef CONFIG_COMPAT
-	.compat_ioctl   = hidraw_ioctl,
-#endif
+	.compat_ioctl   = generic_compat_ioctl_ptrarg,
 	.llseek =	noop_llseek,
 };
 
diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
index a062cfddc5af..22844b94b0e9 100644
--- a/drivers/iio/industrialio-core.c
+++ b/drivers/iio/industrialio-core.c
@@ -1630,7 +1630,7 @@ static const struct file_operations iio_buffer_fileops = {
 	.owner = THIS_MODULE,
 	.llseek = noop_llseek,
 	.unlocked_ioctl = iio_ioctl,
-	.compat_ioctl = iio_ioctl,
+	.compat_ioctl = generic_compat_ioctl_ptrarg,
 };
 
 static int iio_check_unique_scan_index(struct iio_dev *indio_dev)
diff --git a/drivers/infiniband/core/uverbs_main.c b/drivers/infiniband/core/uverbs_main.c
index 823beca448e1..f4755c1c9cfa 100644
--- a/drivers/infiniband/core/uverbs_main.c
+++ b/drivers/infiniband/core/uverbs_main.c
@@ -930,7 +930,7 @@ static const struct file_operations uverbs_fops = {
 	.release = ib_uverbs_close,
 	.llseek	 = no_llseek,
 	.unlocked_ioctl = ib_uverbs_ioctl,
-	.compat_ioctl = ib_uverbs_ioctl,
+	.compat_ioctl = generic_compat_ioctl_ptrarg,
 };
 
 static const struct file_operations uverbs_mmap_fops = {
@@ -941,7 +941,7 @@ static const struct file_operations uverbs_mmap_fops = {
 	.release = ib_uverbs_close,
 	.llseek	 = no_llseek,
 	.unlocked_ioctl = ib_uverbs_ioctl,
-	.compat_ioctl = ib_uverbs_ioctl,
+	.compat_ioctl = generic_compat_ioctl_ptrarg,
 };
 
 static struct ib_client uverbs_client = {
diff --git a/drivers/media/rc/lirc_dev.c b/drivers/media/rc/lirc_dev.c
index f862f1b7f996..077209f414ed 100644
--- a/drivers/media/rc/lirc_dev.c
+++ b/drivers/media/rc/lirc_dev.c
@@ -730,9 +730,7 @@ static const struct file_operations lirc_fops = {
 	.owner		= THIS_MODULE,
 	.write		= ir_lirc_transmit_ir,
 	.unlocked_ioctl	= ir_lirc_ioctl,
-#ifdef CONFIG_COMPAT
-	.compat_ioctl	= ir_lirc_ioctl,
-#endif
+	.compat_ioctl	= generic_compat_ioctl_ptrarg,
 	.read		= ir_lirc_read,
 	.poll		= ir_lirc_poll,
 	.open		= ir_lirc_open,
diff --git a/drivers/mfd/cros_ec_dev.c b/drivers/mfd/cros_ec_dev.c
index 999dac752bcc..35a04bcf55da 100644
--- a/drivers/mfd/cros_ec_dev.c
+++ b/drivers/mfd/cros_ec_dev.c
@@ -258,9 +258,7 @@ static const struct file_operations fops = {
 	.release = ec_device_release,
 	.read = ec_device_read,
 	.unlocked_ioctl = ec_device_ioctl,
-#ifdef CONFIG_COMPAT
-	.compat_ioctl = ec_device_ioctl,
-#endif
+	.compat_ioctl = generic_compat_ioctl_ptrarg,
 };
 
 static void cros_ec_sensors_register(struct cros_ec_dev *ec)
diff --git a/drivers/misc/vmw_vmci/vmci_host.c b/drivers/misc/vmw_vmci/vmci_host.c
index 83e0c95d20a4..1308f889e53b 100644
--- a/drivers/misc/vmw_vmci/vmci_host.c
+++ b/drivers/misc/vmw_vmci/vmci_host.c
@@ -983,7 +983,7 @@ static const struct file_operations vmuser_fops = {
 	.release	= vmci_host_close,
 	.poll		= vmci_host_poll,
 	.unlocked_ioctl	= vmci_host_unlocked_ioctl,
-	.compat_ioctl	= vmci_host_unlocked_ioctl,
+	.compat_ioctl	= generic_compat_ioctl_ptrarg,
 };
 
 static struct miscdevice vmci_host_miscdev = {
diff --git a/drivers/nvdimm/bus.c b/drivers/nvdimm/bus.c
index 8aae6dcc839f..7449cbc55df7 100644
--- a/drivers/nvdimm/bus.c
+++ b/drivers/nvdimm/bus.c
@@ -1133,7 +1133,7 @@ static const struct file_operations nvdimm_bus_fops = {
 	.owner = THIS_MODULE,
 	.open = nd_open,
 	.unlocked_ioctl = nd_ioctl,
-	.compat_ioctl = nd_ioctl,
+	.compat_ioctl = generic_compat_ioctl_ptrarg,
 	.llseek = noop_llseek,
 };
 
@@ -1141,7 +1141,7 @@ static const struct file_operations nvdimm_fops = {
 	.owner = THIS_MODULE,
 	.open = nd_open,
 	.unlocked_ioctl = nvdimm_ioctl,
-	.compat_ioctl = nvdimm_ioctl,
+	.compat_ioctl = generic_compat_ioctl_ptrarg,
 	.llseek = noop_llseek,
 };
 
diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index dd8ec1dd9219..2d986f573a29 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -2579,7 +2579,7 @@ static const struct file_operations nvme_dev_fops = {
 	.owner		= THIS_MODULE,
 	.open		= nvme_dev_open,
 	.unlocked_ioctl	= nvme_dev_ioctl,
-	.compat_ioctl	= nvme_dev_ioctl,
+	.compat_ioctl	= generic_compat_ioctl_ptrarg,
 };
 
 static ssize_t nvme_sysfs_reset(struct device *dev,
diff --git a/drivers/pci/switch/switchtec.c b/drivers/pci/switch/switchtec.c
index 9940cc70f38b..4296919c784e 100644
--- a/drivers/pci/switch/switchtec.c
+++ b/drivers/pci/switch/switchtec.c
@@ -967,7 +967,7 @@ static const struct file_operations switchtec_fops = {
 	.read = switchtec_dev_read,
 	.poll = switchtec_dev_poll,
 	.unlocked_ioctl = switchtec_dev_ioctl,
-	.compat_ioctl = switchtec_dev_ioctl,
+	.compat_ioctl = generic_compat_ioctl_ptrarg,
 };
 
 static void link_event_work(struct work_struct *work)
diff --git a/drivers/platform/x86/wmi.c b/drivers/platform/x86/wmi.c
index 04791ea5d97b..e4d0697e07d6 100644
--- a/drivers/platform/x86/wmi.c
+++ b/drivers/platform/x86/wmi.c
@@ -886,7 +886,7 @@ static const struct file_operations wmi_fops = {
 	.read		= wmi_char_read,
 	.open		= wmi_char_open,
 	.unlocked_ioctl	= wmi_ioctl,
-	.compat_ioctl	= wmi_ioctl,
+	.compat_ioctl	= generic_compat_ioctl_ptrarg,
 };
 
 static int wmi_dev_probe(struct device *dev)
diff --git a/drivers/rpmsg/rpmsg_char.c b/drivers/rpmsg/rpmsg_char.c
index a76b963a7e50..02aefb2b2d47 100644
--- a/drivers/rpmsg/rpmsg_char.c
+++ b/drivers/rpmsg/rpmsg_char.c
@@ -285,7 +285,7 @@ static const struct file_operations rpmsg_eptdev_fops = {
 	.write = rpmsg_eptdev_write,
 	.poll = rpmsg_eptdev_poll,
 	.unlocked_ioctl = rpmsg_eptdev_ioctl,
-	.compat_ioctl = rpmsg_eptdev_ioctl,
+	.compat_ioctl = generic_compat_ioctl_ptrarg,
 };
 
 static ssize_t name_show(struct device *dev, struct device_attribute *attr,
@@ -446,7 +446,7 @@ static const struct file_operations rpmsg_ctrldev_fops = {
 	.open = rpmsg_ctrldev_open,
 	.release = rpmsg_ctrldev_release,
 	.unlocked_ioctl = rpmsg_ctrldev_ioctl,
-	.compat_ioctl = rpmsg_ctrldev_ioctl,
+	.compat_ioctl = generic_compat_ioctl_ptrarg,
 };
 
 static void rpmsg_ctrldev_release_device(struct device *dev)
diff --git a/drivers/sbus/char/display7seg.c b/drivers/sbus/char/display7seg.c
index 5c8ed7350a04..064fe7247eb2 100644
--- a/drivers/sbus/char/display7seg.c
+++ b/drivers/sbus/char/display7seg.c
@@ -155,7 +155,7 @@ static long d7s_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 static const struct file_operations d7s_fops = {
 	.owner =		THIS_MODULE,
 	.unlocked_ioctl =	d7s_ioctl,
-	.compat_ioctl =		d7s_ioctl,
+	.compat_ioctl =		generic_compat_ioctl_ptrarg,
 	.open =			d7s_open,
 	.release =		d7s_release,
 	.llseek = noop_llseek,
diff --git a/drivers/sbus/char/envctrl.c b/drivers/sbus/char/envctrl.c
index 56e962a01493..a26665ccea56 100644
--- a/drivers/sbus/char/envctrl.c
+++ b/drivers/sbus/char/envctrl.c
@@ -714,9 +714,7 @@ static const struct file_operations envctrl_fops = {
 	.owner =		THIS_MODULE,
 	.read =			envctrl_read,
 	.unlocked_ioctl =	envctrl_ioctl,
-#ifdef CONFIG_COMPAT
-	.compat_ioctl =		envctrl_ioctl,
-#endif
+	.compat_ioctl =		generic_compat_ioctl_ptrarg,
 	.open =			envctrl_open,
 	.release =		envctrl_release,
 	.llseek =		noop_llseek,
diff --git a/drivers/scsi/3w-xxxx.c b/drivers/scsi/3w-xxxx.c
index 471366945bd4..86c9f22a152f 100644
--- a/drivers/scsi/3w-xxxx.c
+++ b/drivers/scsi/3w-xxxx.c
@@ -1047,9 +1047,7 @@ static int tw_chrdev_open(struct inode *inode, struct file *file)
 static const struct file_operations tw_fops = {
 	.owner		= THIS_MODULE,
 	.unlocked_ioctl	= tw_chrdev_ioctl,
-#ifdef CONFIG_COMPAT
-	.compat_ioctl   = tw_chrdev_ioctl,
-#endif
+	.compat_ioctl   = generic_compat_ioctl_ptrarg,
 	.open		= tw_chrdev_open,
 	.release	= NULL,
 	.llseek		= noop_llseek,
diff --git a/drivers/scsi/cxlflash/main.c b/drivers/scsi/cxlflash/main.c
index 6637116529aa..d968efeb50e8 100644
--- a/drivers/scsi/cxlflash/main.c
+++ b/drivers/scsi/cxlflash/main.c
@@ -3596,7 +3596,7 @@ static const struct file_operations cxlflash_chr_fops = {
 	.owner          = THIS_MODULE,
 	.open           = cxlflash_chr_open,
 	.unlocked_ioctl	= cxlflash_chr_ioctl,
-	.compat_ioctl	= cxlflash_chr_ioctl,
+	.compat_ioctl	= generic_compat_ioctl_ptrarg,
 };
 
 /**
diff --git a/drivers/scsi/esas2r/esas2r_main.c b/drivers/scsi/esas2r/esas2r_main.c
index c07118617d89..95142292e702 100644
--- a/drivers/scsi/esas2r/esas2r_main.c
+++ b/drivers/scsi/esas2r/esas2r_main.c
@@ -614,7 +614,7 @@ static int __init esas2r_init(void)
 
 /* Handle ioctl calls to "/proc/scsi/esas2r/ATTOnode" */
 static const struct file_operations esas2r_proc_fops = {
-	.compat_ioctl	= esas2r_proc_ioctl,
+	.compat_ioctl	= generic_compat_ioctl_ptrarg,
 	.unlocked_ioctl = esas2r_proc_ioctl,
 };
 
diff --git a/drivers/scsi/pmcraid.c b/drivers/scsi/pmcraid.c
index 4e86994e10e8..8a8c73d3bdad 100644
--- a/drivers/scsi/pmcraid.c
+++ b/drivers/scsi/pmcraid.c
@@ -3999,9 +3999,7 @@ static const struct file_operations pmcraid_fops = {
 	.open = pmcraid_chr_open,
 	.fasync = pmcraid_chr_fasync,
 	.unlocked_ioctl = pmcraid_chr_ioctl,
-#ifdef CONFIG_COMPAT
-	.compat_ioctl = pmcraid_chr_ioctl,
-#endif
+	.compat_ioctl = generic_compat_ioctl_ptrarg,
 	.llseek = noop_llseek,
 };
 
diff --git a/drivers/staging/android/ion/ion.c b/drivers/staging/android/ion/ion.c
index 99073325b0c0..ef727c235392 100644
--- a/drivers/staging/android/ion/ion.c
+++ b/drivers/staging/android/ion/ion.c
@@ -484,9 +484,7 @@ int ion_query_heaps(struct ion_heap_query *query)
 static const struct file_operations ion_fops = {
 	.owner          = THIS_MODULE,
 	.unlocked_ioctl = ion_ioctl,
-#ifdef CONFIG_COMPAT
-	.compat_ioctl	= ion_ioctl,
-#endif
+	.compat_ioctl	= generic_compat_ioctl_ptrarg,
 };
 
 static int debug_shrink_set(void *data, u64 val)
diff --git a/drivers/staging/vme/devices/vme_user.c b/drivers/staging/vme/devices/vme_user.c
index 6a33aaa1a49f..568700ffd2f2 100644
--- a/drivers/staging/vme/devices/vme_user.c
+++ b/drivers/staging/vme/devices/vme_user.c
@@ -494,7 +494,7 @@ static const struct file_operations vme_user_fops = {
 	.write = vme_user_write,
 	.llseek = vme_user_llseek,
 	.unlocked_ioctl = vme_user_unlocked_ioctl,
-	.compat_ioctl = vme_user_unlocked_ioctl,
+	.compat_ioctl = generic_compat_ioctl_ptrarg,
 	.mmap = vme_user_mmap,
 };
 
diff --git a/drivers/tee/tee_core.c b/drivers/tee/tee_core.c
index dd46b758852a..cb79f28be894 100644
--- a/drivers/tee/tee_core.c
+++ b/drivers/tee/tee_core.c
@@ -670,7 +670,7 @@ static const struct file_operations tee_fops = {
 	.open = tee_open,
 	.release = tee_release,
 	.unlocked_ioctl = tee_ioctl,
-	.compat_ioctl = tee_ioctl,
+	.compat_ioctl = generic_compat_ioctl_ptrarg,
 };
 
 static void tee_release_device(struct device *dev)
diff --git a/drivers/usb/class/cdc-wdm.c b/drivers/usb/class/cdc-wdm.c
index bec581fb7c63..6e4998c8e64f 100644
--- a/drivers/usb/class/cdc-wdm.c
+++ b/drivers/usb/class/cdc-wdm.c
@@ -724,7 +724,7 @@ static const struct file_operations wdm_fops = {
 	.release =	wdm_release,
 	.poll =		wdm_poll,
 	.unlocked_ioctl = wdm_ioctl,
-	.compat_ioctl = wdm_ioctl,
+	.compat_ioctl = generic_compat_ioctl_ptrarg,
 	.llseek =	noop_llseek,
 };
 
diff --git a/drivers/usb/class/usbtmc.c b/drivers/usb/class/usbtmc.c
index 83ffa5a14c3d..d5da47c4c462 100644
--- a/drivers/usb/class/usbtmc.c
+++ b/drivers/usb/class/usbtmc.c
@@ -1460,9 +1460,7 @@ static const struct file_operations fops = {
 	.open		= usbtmc_open,
 	.release	= usbtmc_release,
 	.unlocked_ioctl	= usbtmc_ioctl,
-#ifdef CONFIG_COMPAT
-	.compat_ioctl	= usbtmc_ioctl,
-#endif
+	.compat_ioctl	= generic_compat_ioctl_ptrarg,
 	.fasync         = usbtmc_fasync,
 	.poll           = usbtmc_poll,
 	.llseek		= default_llseek,
diff --git a/drivers/video/fbdev/ps3fb.c b/drivers/video/fbdev/ps3fb.c
index 5ed2db39d823..f9f8ffaf1c4a 100644
--- a/drivers/video/fbdev/ps3fb.c
+++ b/drivers/video/fbdev/ps3fb.c
@@ -949,7 +949,7 @@ static struct fb_ops ps3fb_ops = {
 	.fb_mmap	= ps3fb_mmap,
 	.fb_blank	= ps3fb_blank,
 	.fb_ioctl	= ps3fb_ioctl,
-	.fb_compat_ioctl = ps3fb_ioctl
+	.fb_compat_ioctl = generic_compat_ioctl_ptrarg,
 };
 
 static const struct fb_fix_screeninfo ps3fb_fix = {
diff --git a/drivers/virt/fsl_hypervisor.c b/drivers/virt/fsl_hypervisor.c
index 8ba726e600e9..406b7e492214 100644
--- a/drivers/virt/fsl_hypervisor.c
+++ b/drivers/virt/fsl_hypervisor.c
@@ -703,7 +703,7 @@ static const struct file_operations fsl_hv_fops = {
 	.poll = fsl_hv_poll,
 	.read = fsl_hv_read,
 	.unlocked_ioctl = fsl_hv_ioctl,
-	.compat_ioctl = fsl_hv_ioctl,
+	.compat_ioctl = generic_compat_ioctl_ptrarg,
 };
 
 static struct miscdevice fsl_hv_misc_dev = {
diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
index 6601c9aa5e35..2b5a8ad86305 100644
--- a/fs/btrfs/super.c
+++ b/fs/btrfs/super.c
@@ -2352,7 +2352,7 @@ static const struct super_operations btrfs_super_ops = {
 static const struct file_operations btrfs_ctl_fops = {
 	.open = btrfs_control_open,
 	.unlocked_ioctl	 = btrfs_control_ioctl,
-	.compat_ioctl = btrfs_control_ioctl,
+	.compat_ioctl = generic_compat_ioctl_ptrarg,
 	.owner	 = THIS_MODULE,
 	.llseek = noop_llseek,
 };
diff --git a/fs/ceph/dir.c b/fs/ceph/dir.c
index da73f29d7faa..eb869fe6774d 100644
--- a/fs/ceph/dir.c
+++ b/fs/ceph/dir.c
@@ -1489,7 +1489,7 @@ const struct file_operations ceph_dir_fops = {
 	.open = ceph_open,
 	.release = ceph_release,
 	.unlocked_ioctl = ceph_ioctl,
-	.compat_ioctl = ceph_ioctl,
+	.compat_ioctl = generic_compat_ioctl_ptrarg,
 	.fsync = ceph_fsync,
 	.lock = ceph_lock,
 	.flock = ceph_flock,
diff --git a/fs/ceph/file.c b/fs/ceph/file.c
index 92ab20433682..85094042cfac 100644
--- a/fs/ceph/file.c
+++ b/fs/ceph/file.c
@@ -1842,7 +1842,7 @@ const struct file_operations ceph_file_fops = {
 	.splice_read = generic_file_splice_read,
 	.splice_write = iter_file_splice_write,
 	.unlocked_ioctl = ceph_ioctl,
-	.compat_ioctl	= ceph_ioctl,
+	.compat_ioctl	= generic_compat_ioctl_ptrarg,
 	.fallocate	= ceph_fallocate,
 };
 
diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c
index 11ea2c4a38ab..a6d4a24963ed 100644
--- a/fs/fuse/dev.c
+++ b/fs/fuse/dev.c
@@ -2258,7 +2258,7 @@ const struct file_operations fuse_dev_operations = {
 	.release	= fuse_dev_release,
 	.fasync		= fuse_dev_fasync,
 	.unlocked_ioctl = fuse_dev_ioctl,
-	.compat_ioctl   = fuse_dev_ioctl,
+	.compat_ioctl   = generic_compat_ioctl_ptrarg,
 };
 EXPORT_SYMBOL_GPL(fuse_dev_operations);
 
diff --git a/fs/notify/fanotify/fanotify_user.c b/fs/notify/fanotify/fanotify_user.c
index 69054886915b..fc4193b384cf 100644
--- a/fs/notify/fanotify/fanotify_user.c
+++ b/fs/notify/fanotify/fanotify_user.c
@@ -447,7 +447,7 @@ static const struct file_operations fanotify_fops = {
 	.fasync		= NULL,
 	.release	= fanotify_release,
 	.unlocked_ioctl	= fanotify_ioctl,
-	.compat_ioctl	= fanotify_ioctl,
+	.compat_ioctl	= generic_compat_ioctl_ptrarg,
 	.llseek		= noop_llseek,
 };
 
diff --git a/fs/userfaultfd.c b/fs/userfaultfd.c
index bfa0ec69f924..bc9118b58a8a 100644
--- a/fs/userfaultfd.c
+++ b/fs/userfaultfd.c
@@ -1878,7 +1878,7 @@ static const struct file_operations userfaultfd_fops = {
 	.poll		= userfaultfd_poll,
 	.read		= userfaultfd_read,
 	.unlocked_ioctl = userfaultfd_ioctl,
-	.compat_ioctl	= userfaultfd_ioctl,
+	.compat_ioctl	= generic_compat_ioctl_ptrarg,
 	.llseek		= noop_llseek,
 };
 
diff --git a/net/rfkill/core.c b/net/rfkill/core.c
index 1355f5ca8d22..ba68b53f58ab 100644
--- a/net/rfkill/core.c
+++ b/net/rfkill/core.c
@@ -1323,7 +1323,7 @@ static const struct file_operations rfkill_fops = {
 	.release	= rfkill_fop_release,
 #ifdef CONFIG_RFKILL_INPUT
 	.unlocked_ioctl	= rfkill_fop_ioctl,
-	.compat_ioctl	= rfkill_fop_ioctl,
+	.compat_ioctl	= generic_compat_ioctl_ptrarg,
 #endif
 	.llseek		= no_llseek,
 };
-- 
2.18.0

^ permalink raw reply related

* Re: [PATCH v2] PCI: Reprogram bridge prefetch registers on resume
From: Peter Wu @ 2018-09-12 10:06 UTC (permalink / raw)
  To: Daniel Drake
  Cc: bhelgaas, linux-pci, linux, nouveau, linux-pm, kherbst,
	andy.shevchenko, rafael.j.wysocki, keith.busch, mika.westerberg,
	jonathan.derrick, kugel, davem, hkallweit1, netdev, nic_swsd,
	rchang
In-Reply-To: <20180912064523.9599-1-drake@endlessm.com>

On Wed, Sep 12, 2018 at 02:45:23PM +0800, Daniel Drake wrote:
> On 38+ Intel-based Asus products, the nvidia GPU becomes unusable
> after S3 suspend/resume. The affected products include multiple
> generations of nvidia GPUs and Intel SoCs. After resume, nouveau logs
> many errors such as:
> 
>     fifo: fault 00 [READ] at 0000005555555000 engine 00 [GR] client 04 [HUB/FE] reason 4a [] on channel -1 [007fa91000 unknown]
>     DRM: failed to idle channel 0 [DRM]
> 
> Similarly, the nvidia proprietary driver also fails after resume
> (black screen, 100% CPU usage in Xorg process). We shipped a sample
> to Nvidia for diagnosis, and their response indicated that it's a
> problem with the parent PCI bridge (on the Intel SoC), not the GPU.
> 
> Runtime suspend/resume works fine, only S3 suspend is affected.
> 
> We found a workaround: on resume, rewrite the Intel PCI bridge
> 'Prefetchable Base Upper 32 Bits' register (PCI_PREF_BASE_UPPER32). In
> the cases that I checked, this register has value 0 and we just have to
> rewrite that value.
> 
> Linux already saves and restores PCI config space during suspend/resume,
> but this register was being skipped because upon resume, it already
> has value 0 (the correct, pre-suspend value).
> 
> Intel appear to have previously acknowledged this behaviour and the
> requirement to rewrite this register.
> https://bugzilla.kernel.org/show_bug.cgi?id=116851#c23
> 
> Based on that, rewrite the prefetch register values even when that
> appears unnecessary.
> 
> We have confirmed this solution on all the affected models we have
> in-hands (X542UQ, UX533FD, X530UN, V272UN).
> 
> Additionally, this solves an issue where r8169 MSI-X interrupts were
> broken after S3 suspend/resume on Asus X441UAR. This issue was recently
> worked around in commit 7bb05b85bc2d ("r8169: don't use MSI-X on
> RTL8106e"). It also fixes the same issue on RTL6186evl/8111evl on an
> Aimfor-tech laptop that we had not yet patched. I suspect it will also
> fix the issue that was worked around in commit 7c53a722459c ("r8169:
> don't use MSI-X on RTL8168g").
> 
> Thomas Martitz reports that this change also solves an issue where
> the AMD Radeon Polaris 10 GPU on the HP Zbook 14u G5 is unresponsive
> after S3 suspend/resume.
> 
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=201069
> Signed-off-by: Daniel Drake <drake@endlessm.com>
> ---
> 
> Notes:
>     Replaces patch:
>        PCI: add prefetch quirk to work around Asus/Nvidia suspend issues
>     
>     Some of the more verbose info was moved to bugzilla:
>     https://bugzilla.kernel.org/show_bug.cgi?id=201069
>     
>     This patch is aimed at v4.19 (and maybe v4.18-stable); we may follow
>     up with more intrusive improvements for v4.20+.
>     
>     v2: reimplement the register restore within the existing
>         pci_restore_config_space() code.
> 
>  drivers/pci/pci.c | 31 +++++++++++++++++++++----------
>  1 file changed, 21 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index 29ff9619b5fa..e1704100e72d 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -1289,13 +1289,15 @@ int pci_save_state(struct pci_dev *dev)
>  EXPORT_SYMBOL(pci_save_state);
>  
>  static void pci_restore_config_dword(struct pci_dev *pdev, int offset,
> -				     u32 saved_val, int retry)
> +				     u32 saved_val, int retry, bool force)
>  {
>  	u32 val;
>  
> -	pci_read_config_dword(pdev, offset, &val);
> -	if (val == saved_val)
> -		return;
> +	if (!force) {
> +		pci_read_config_dword(pdev, offset, &val);
> +		if (val == saved_val)
> +			return;
> +	}
>  
>  	for (;;) {
>  		pci_dbg(pdev, "restoring config space at offset %#x (was %#x, writing %#x)\n",

"val" is uninitialized and used in this debug message.

To avoid adding another parameter and changing unrelated lines below,
what about:

    force = pdev->header_type == PCI_HEADER_TYPE_BRIDGE &&
        0x24 <= start && start <= 0x2c;

(or start == 0x28 || start == 0x24 || start == 0x28 if you want to be
very explicit). This would reduce in a smaller diff for this temporary
solution.

Kind regards,
Peter

> @@ -1313,25 +1315,34 @@ static void pci_restore_config_dword(struct pci_dev *pdev, int offset,
>  }
>  
>  static void pci_restore_config_space_range(struct pci_dev *pdev,
> -					   int start, int end, int retry)
> +					   int start, int end, int retry,
> +					   bool force)
>  {
>  	int index;
>  
>  	for (index = end; index >= start; index--)
>  		pci_restore_config_dword(pdev, 4 * index,
>  					 pdev->saved_config_space[index],
> -					 retry);
> +					 retry, force);
>  }
>  
>  static void pci_restore_config_space(struct pci_dev *pdev)
>  {
>  	if (pdev->hdr_type == PCI_HEADER_TYPE_NORMAL) {
> -		pci_restore_config_space_range(pdev, 10, 15, 0);
> +		pci_restore_config_space_range(pdev, 10, 15, 0, false);
>  		/* Restore BARs before the command register. */
> -		pci_restore_config_space_range(pdev, 4, 9, 10);
> -		pci_restore_config_space_range(pdev, 0, 3, 0);
> +		pci_restore_config_space_range(pdev, 4, 9, 10, false);
> +		pci_restore_config_space_range(pdev, 0, 3, 0, false);
> +	} else if (pdev->hdr_type == PCI_HEADER_TYPE_BRIDGE) {
> +		pci_restore_config_space_range(pdev, 12, 15, 0, false);
> +		/* Force rewriting of prefetch registers to avoid
> +		 * S3 resume issues on Intel PCI bridges that occur when
> +		 * these registers are not explicitly written.
> +		 */
> +		pci_restore_config_space_range(pdev, 9, 11, 0, true);
> +		pci_restore_config_space_range(pdev, 0, 8, 0, false);
>  	} else {
> -		pci_restore_config_space_range(pdev, 0, 15, 0);
> +		pci_restore_config_space_range(pdev, 0, 15, 0, false);
>  	}
>  }
>  
> -- 
> 2.17.1
> 

^ permalink raw reply

* [PATCH v2 11/17] compat_ioctl: remove isdn ioctl translation
From: Arnd Bergmann @ 2018-09-12 15:13 UTC (permalink / raw)
  To: viro
  Cc: linux-fsdevel, Arnd Bergmann, Karsten Keil, Paul Bolle,
	Randy Dunlap, David S. Miller, netdev, linux-kernel,
	gigaset307x-common
In-Reply-To: <20180912151422.571531-1-arnd@arndb.de>

Neither the old isdn4linux interface nor the newer mISDN stack
ever had working 32-bit compat mode as far as I can tell.

However, the CAPI stack and the Gigaset ISDN driver (implemented
on top of either CAPI or i4l) have some ioctl commands that are
correctly listed in fs/compat_ioctl.c.

We can trivially move all of those into the two corresponding
files that implement the native handlers by adding a compat_ioctl
redirect to that.

I did notice that treating CAPI_MANUFACTURER_CMD() as compatible
is broken, so I'm also adding a handler for that, realizing that
in all likelyhood, nobody is ever going to call it.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/isdn/capi/capi.c         | 31 +++++++++++++++++++++++++++++++
 drivers/isdn/gigaset/interface.c | 11 +++++++++++
 fs/compat_ioctl.c                | 22 ----------------------
 3 files changed, 42 insertions(+), 22 deletions(-)

diff --git a/drivers/isdn/capi/capi.c b/drivers/isdn/capi/capi.c
index ef5560b848ab..4851dc3941eb 100644
--- a/drivers/isdn/capi/capi.c
+++ b/drivers/isdn/capi/capi.c
@@ -942,6 +942,34 @@ capi_unlocked_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 	return ret;
 }
 
+#ifdef CONFIG_COMPAT
+static long
+capi_compat_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
+{
+	int ret;
+
+	if (cmd == CAPI_MANUFACTURER_CMD) {
+		struct {
+			unsigned long cmd;
+			compat_uptr_t data;
+		} mcmd32;
+
+		if (!capable(CAP_SYS_ADMIN))
+			return -EPERM;
+		if (copy_from_user(&mcmd32, compat_ptr(arg), sizeof(mcmd32)))
+			return -EFAULT;
+
+		mutex_lock(&capi_mutex);
+		ret = capi20_manufacturer(mcmd32.cmd, compat_ptr(mcmd32.data));
+		mutex_unlock(&capi_mutex);
+
+		return ret;
+	}
+
+	return capi_unlocked_ioctl(file, cmd, (unsigned long)compat_ptr(arg));
+}
+#endif
+
 static int capi_open(struct inode *inode, struct file *file)
 {
 	struct capidev *cdev;
@@ -988,6 +1016,9 @@ static const struct file_operations capi_fops =
 	.write		= capi_write,
 	.poll		= capi_poll,
 	.unlocked_ioctl	= capi_unlocked_ioctl,
+#ifdef CONFIG_COMPAT
+	.compat_ioctl	= capi_compat_ioctl,
+#endif
 	.open		= capi_open,
 	.release	= capi_release,
 };
diff --git a/drivers/isdn/gigaset/interface.c b/drivers/isdn/gigaset/interface.c
index 600c79b030cd..e346756b0b45 100644
--- a/drivers/isdn/gigaset/interface.c
+++ b/drivers/isdn/gigaset/interface.c
@@ -233,6 +233,14 @@ static int if_ioctl(struct tty_struct *tty,
 	return retval;
 }
 
+#ifdef CONFIG_COMPAT
+static long if_compat_ioctl(struct tty_struct *tty,
+		     unsigned int cmd, unsigned long arg)
+{
+	return if_ioctl(tty, cmd, (unsigned long)compat_ptr(arg));
+}
+#endif
+
 static int if_tiocmget(struct tty_struct *tty)
 {
 	struct cardstate *cs = tty->driver_data;
@@ -472,6 +480,9 @@ static const struct tty_operations if_ops = {
 	.open =			if_open,
 	.close =		if_close,
 	.ioctl =		if_ioctl,
+#ifdef CONFIG_COMPAT
+	.compat_ioctl =		if_compat_ioctl,
+#endif
 	.write =		if_write,
 	.write_room =		if_write_room,
 	.chars_in_buffer =	if_chars_in_buffer,
diff --git a/fs/compat_ioctl.c b/fs/compat_ioctl.c
index c28fd9f6b1e8..379e04647f83 100644
--- a/fs/compat_ioctl.c
+++ b/fs/compat_ioctl.c
@@ -60,9 +60,6 @@
 #include <net/bluetooth/hci_sock.h>
 #include <net/bluetooth/rfcomm.h>
 
-#include <linux/capi.h>
-#include <linux/gigaset_dev.h>
-
 #ifdef CONFIG_BLOCK
 #include <linux/cdrom.h>
 #include <linux/fd.h>
@@ -840,25 +837,6 @@ COMPATIBLE_IOCTL(HIDPCONNADD)
 COMPATIBLE_IOCTL(HIDPCONNDEL)
 COMPATIBLE_IOCTL(HIDPGETCONNLIST)
 COMPATIBLE_IOCTL(HIDPGETCONNINFO)
-/* CAPI */
-COMPATIBLE_IOCTL(CAPI_REGISTER)
-COMPATIBLE_IOCTL(CAPI_GET_MANUFACTURER)
-COMPATIBLE_IOCTL(CAPI_GET_VERSION)
-COMPATIBLE_IOCTL(CAPI_GET_SERIAL)
-COMPATIBLE_IOCTL(CAPI_GET_PROFILE)
-COMPATIBLE_IOCTL(CAPI_MANUFACTURER_CMD)
-COMPATIBLE_IOCTL(CAPI_GET_ERRCODE)
-COMPATIBLE_IOCTL(CAPI_INSTALLED)
-COMPATIBLE_IOCTL(CAPI_GET_FLAGS)
-COMPATIBLE_IOCTL(CAPI_SET_FLAGS)
-COMPATIBLE_IOCTL(CAPI_CLR_FLAGS)
-COMPATIBLE_IOCTL(CAPI_NCCI_OPENCOUNT)
-COMPATIBLE_IOCTL(CAPI_NCCI_GETUNIT)
-/* Siemens Gigaset */
-COMPATIBLE_IOCTL(GIGASET_REDIR)
-COMPATIBLE_IOCTL(GIGASET_CONFIG)
-COMPATIBLE_IOCTL(GIGASET_BRKCHARS)
-COMPATIBLE_IOCTL(GIGASET_VERSION)
 /* Misc. */
 COMPATIBLE_IOCTL(0x41545900)		/* ATYIO_CLKR */
 COMPATIBLE_IOCTL(0x41545901)		/* ATYIO_CLKW */
-- 
2.18.0

^ permalink raw reply related

* [PATCH RFC 0/2] Flow sorted receive skb lists
From: Steffen Klassert @ 2018-09-12 10:23 UTC (permalink / raw)
  To: netdev; +Cc: Steffen Klassert

This patchset consists of two patches. Patch 1 adds support for flow
sorted rx skb lists for IPv4. This means that it sorts the skb list so
that packets from the same flow can to travel together through the stack.

The second patch of this pachset is just a hack that disables GRO and does
skb list receive instead. I don't have a NIC whose driver supports to build
skb lists to be received by netif_receive_skb_list(), so I used this hack
to test patch 1.

This is early stage work, so it might be not complete and still buggy.
I send this now because I want to hear some comments on the approach
itself before I spend more time to work on this.

Forwarding performance measurements:

I used used my IPsec forwarding test setup for this:

           ------------         ------------
        -->| router 1 |-------->| router 2 |--
        |  ------------         ------------  |
        |                                     |
        |       --------------------          |
        --------|Spirent Testcenter|<----------
                --------------------

net-next (September 6th):

Single stream UDP frame size 1460 Bytes: 1.258.446 fps.

Single stream UDP frame size 64 Bytes: 1.250.000 fps.

----------------------------------------------------------------------

net-next (September 6th) + patch 2 (list receive):

Single stream UDP frame size 1460 Bytes: 1.469.595 fps (+16%).

Single stream UDP frame size 64 Bytes: 1.502.976 fps  (+20%).

----------------------------------------------------------------------

net-next (September 6th) + patch 1 + patch 2 (flow sorted list receive):

Single stream UDP frame size 1460 Bytes: 2.525.338 fps (+100%).

Single stream UDP frame size 64 Bytes: 2.541.881 fps (+103%).

-----------------------------------------------------------------------

^ permalink raw reply

* [PATCH RFC 2/2] net: Hack to enable skb list receive in the napi layer.
From: Steffen Klassert @ 2018-09-12 10:23 UTC (permalink / raw)
  To: netdev; +Cc: Steffen Klassert
In-Reply-To: <20180912102330.24790-1-steffen.klassert@secunet.com>

This patch was used to test patch ("net: Support flow sorted skb lists
for IPv4.") It is just a hack that disables GRO and does skb list
receive instead. Not for merging!

Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
 include/linux/netdevice.h |  5 ++++-
 net/core/dev.c            | 23 ++++++++++++++---------
 net/ipv4/ip_input.c       |  1 +
 3 files changed, 19 insertions(+), 10 deletions(-)

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index e2b3bd750c98..6518ceeda05d 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -333,7 +333,10 @@ struct napi_struct {
 	int			poll_owner;
 #endif
 	struct net_device	*dev;
-	struct gro_list		gro_hash[GRO_HASH_BUCKETS];
+	union {
+		struct list_head	rx_list;
+		struct gro_list		gro_hash[GRO_HASH_BUCKETS];
+	};
 	struct sk_buff		*skb;
 	struct hrtimer		timer;
 	struct list_head	dev_list;
diff --git a/net/core/dev.c b/net/core/dev.c
index 147da35d7380..68e00727c657 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -5470,6 +5470,10 @@ static enum gro_result dev_gro_receive(struct napi_struct *napi, struct sk_buff
 	int same_flow;
 	int grow;
 
+	list_add_tail(&skb->list, &napi->rx_list);
+	ret = GRO_CONSUMED;
+	goto out;
+
 	if (netif_elide_gro(skb->dev))
 		goto normal;
 
@@ -5559,7 +5563,7 @@ static enum gro_result dev_gro_receive(struct napi_struct *napi, struct sk_buff
 	} else if (test_bit(hash, &napi->gro_bitmask)) {
 		__clear_bit(hash, &napi->gro_bitmask);
 	}
-
+out:
 	return ret;
 
 normal:
@@ -5958,7 +5962,7 @@ bool napi_complete_done(struct napi_struct *n, int work_done)
 				 NAPIF_STATE_IN_BUSY_POLL)))
 		return false;
 
-	if (n->gro_bitmask) {
+	if (!list_empty(&n->rx_list)) {
 		unsigned long timeout = 0;
 
 		if (work_done)
@@ -5967,8 +5971,10 @@ bool napi_complete_done(struct napi_struct *n, int work_done)
 		if (timeout)
 			hrtimer_start(&n->timer, ns_to_ktime(timeout),
 				      HRTIMER_MODE_REL_PINNED);
-		else
-			napi_gro_flush(n, false);
+		else {
+			netif_receive_skb_list(&n->rx_list);
+			INIT_LIST_HEAD(&n->rx_list);
+		}
 	}
 	if (unlikely(!list_empty(&n->poll_list))) {
 		/* If n->poll_list is not empty, we need to mask irqs */
@@ -6189,6 +6195,7 @@ void netif_napi_add(struct net_device *dev, struct napi_struct *napi,
 		    int (*poll)(struct napi_struct *, int), int weight)
 {
 	INIT_LIST_HEAD(&napi->poll_list);
+	INIT_LIST_HEAD(&napi->rx_list);
 	hrtimer_init(&napi->timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL_PINNED);
 	napi->timer.function = napi_watchdog;
 	init_gro_hash(napi);
@@ -6289,11 +6296,9 @@ static int napi_poll(struct napi_struct *n, struct list_head *repoll)
 		goto out_unlock;
 	}
 
-	if (n->gro_bitmask) {
-		/* flush too old packets
-		 * If HZ < 1000, flush all packets.
-		 */
-		napi_gro_flush(n, HZ >= 1000);
+	if (!list_empty(&n->rx_list)) {
+		netif_receive_skb_list(&n->rx_list);
+		INIT_LIST_HEAD(&n->rx_list);
 	}
 
 	/* Some drivers may have called napi_schedule
diff --git a/net/ipv4/ip_input.c b/net/ipv4/ip_input.c
index bf710bf95fea..847965f83a2f 100644
--- a/net/ipv4/ip_input.c
+++ b/net/ipv4/ip_input.c
@@ -729,6 +729,7 @@ void ip_list_rcv(struct list_head *head, struct packet_type *pt,
 		}
 		list_add_tail(&skb->list, &sublist);
 	}
+
 	/* dispatch final sublist */
 	ip_sublist_rcv(&sublist, curr_dev, curr_net);
 }
-- 
2.17.1

^ permalink raw reply related

* [PATCH RFC 1/2] net: Support flow sorted RX skb lists for IPv4.
From: Steffen Klassert @ 2018-09-12 10:23 UTC (permalink / raw)
  To: netdev; +Cc: Steffen Klassert
In-Reply-To: <20180912102330.24790-1-steffen.klassert@secunet.com>

This patch sorts RX skb lists into separate flows, using
a flow dissector, at the IP input layer. Packets of the
same flow are chained at the frag_list pointer of the first
skb of this flow.

After ip_list_rcv_finish() the skb list has this layout:

|---------|        |---------|        |---------|
|flow 1   |        |flow 1   |        |flow 1   |
|---------|        |---------|        |---------|
|frag_list|<-\     |frag_list|        |frag_list|
|---------|   \    |---------|        |---------|
|next     |<-\ \---|next     |<-------|next     |
|---------|   \    |---------|        |---------|
              |
              |
              |    |---------|        |---------|        |---------|
              |    |flow 2   |        |flow 2   |        |flow 2   |
              |    |---------|        |---------|        |---------|
              |    |frag_list|<-\     |frag_list|        |frag_list|
              |    |---------|   \    |---------|        |---------|
              |----|next     |<-\ \---|next     |<-------|next     |
                   |---------|   \    |---------|        |---------|
                                 |
                                 |
                                 |    |---------|        |---------|       |---------|
                                 |    |flow 3   |        |flow 3   |       |flow 3   |
                                 |    |---------|        |---------|       |---------|
                                 |    |frag_list|<-\     |frag_list|       |frag_list|
                                 |    |---------|   \    |---------|       |---------|
                                 |----|next     |    \---|next     |<------|next     |
                                      |---------|        |---------|       |---------|

With this approach route lookups etc. are done just for one
representative packet of a given flow instead for each packet.

ip_sublist_rcv_finish() splits these lists then into:

|---------|        |---------|        |---------|
|flow 1   |        |flow 1   |        |flow 1   |
|---------|        |---------|        |---------|
|frag_list|<-\     |frag_list|        |frag_list|
|---------|   \    |---------|        |---------|
|next     |    \---|next     |<-------|next     |
|---------|        |---------|        |---------|

Packets of the same flow can still travel together after that point.

On input, this is plumbed through to ip_local_deliver_finish(),
here the skb chain is split back into single packets.

My hope is that this can be plumbed through to the sockets
receive queue. I have a patch for UDP, but it has still
problems with UDP encapsulaion, so it is not included here.

On forward, the skb chain can travel together to the TX path.
__skb_gso_segment() will build a standard skb list from this.

For now, this is only enabled if the receiving device allows
forwarding, as the forwarding path has currently the most gain
from this.

Known issues:

- I don't have a NIC whose driver supports to build skb lists
  to be received by netif_receive_skb_list(). To test this
  codepath I used a hack that builds skb lists at the napi
  layer.

- Performance measurements were done with this hack, so I don't
  know if these measurements are really meaningful.

- This is early stage work, so the functional tests are only
  done on a basic level, it might be still buggy.

- This still uses the skb->next, skb->prev pointers to build
  skb lists. So needs to be converted to standard list handling
  at some point.

Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
 include/linux/skbuff.h    |   5 ++
 net/core/dev.c            |  45 +++++++++++-
 net/core/flow_dissector.c |  40 +++++++++++
 net/core/skbuff.c         |  52 ++++++++++++++
 net/ipv4/ip_input.c       | 139 ++++++++++++++++++++++++++++++++++----
 net/ipv4/ip_output.c      |   3 +-
 6 files changed, 270 insertions(+), 14 deletions(-)

diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 17a13e4785fc..d070d073a1dc 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -575,6 +575,8 @@ enum {
 	SKB_GSO_UDP = 1 << 16,
 
 	SKB_GSO_UDP_L4 = 1 << 17,
+
+	SKB_GSO_FRAGLIST = 1 << 18,
 };
 
 #if BITS_PER_LONG > 32
@@ -1226,6 +1228,8 @@ skb_flow_dissect_flow_keys_basic(const struct sk_buff *skb,
 				  data, proto, nhoff, hlen, flags);
 }
 
+u32 skb_flow_keys_rx_digest(struct sk_buff *skb, struct flow_keys_digest *digest);
+
 void
 skb_flow_dissect_tunnel_info(const struct sk_buff *skb,
 			     struct flow_dissector *flow_dissector,
@@ -3302,6 +3306,7 @@ int skb_shift(struct sk_buff *tgt, struct sk_buff *skb, int shiftlen);
 void skb_scrub_packet(struct sk_buff *skb, bool xnet);
 bool skb_gso_validate_network_len(const struct sk_buff *skb, unsigned int mtu);
 bool skb_gso_validate_mac_len(const struct sk_buff *skb, unsigned int len);
+void skb_segment_list(struct sk_buff *skb);
 struct sk_buff *skb_segment(struct sk_buff *skb, netdev_features_t features);
 struct sk_buff *skb_vlan_untag(struct sk_buff *skb);
 int skb_ensure_writable(struct sk_buff *skb, int write_len);
diff --git a/net/core/dev.c b/net/core/dev.c
index ca78dc5a79a3..147da35d7380 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -2998,6 +2998,34 @@ static inline bool skb_needs_check(struct sk_buff *skb, bool tx_path)
 	return skb->ip_summed == CHECKSUM_NONE;
 }
 
+static void skb_segment_list_ip(struct sk_buff *skb)
+{
+	unsigned int tnl_hlen = 0;
+	struct sk_buff *nskb;
+	int id;
+
+	id = ntohs(ip_hdr(skb)->id);
+	skb_segment_list(skb);
+
+	tnl_hlen = skb_tnl_header_len(skb);
+
+	nskb = skb->next;
+
+	do {
+		skb_push(nskb, skb_network_header(nskb) - skb_mac_header(nskb));
+		skb_headers_offset_update(nskb, skb_headroom(nskb) - skb_headroom(skb));
+		skb_copy_from_linear_data_offset(skb, -tnl_hlen,
+						 nskb->data - tnl_hlen,
+						 skb_transport_header(nskb) -
+						 skb_mac_header(nskb) +
+						 tnl_hlen);
+
+		ip_hdr(nskb)->id = htons(++id);
+		ip_send_check(ip_hdr(nskb));
+		nskb = nskb->next;
+	} while (nskb);
+}
+
 /**
  *	__skb_gso_segment - Perform segmentation on skb.
  *	@skb: buffer to segment
@@ -3016,6 +3044,21 @@ struct sk_buff *__skb_gso_segment(struct sk_buff *skb,
 {
 	struct sk_buff *segs;
 
+	if (skb_shinfo(skb)->gso_type & SKB_GSO_FRAGLIST) {
+		int dummy;
+
+		if (skb_network_protocol(skb, &dummy) != htons(ETH_P_IP))
+			return ERR_PTR(-EINVAL);
+
+		skb_segment_list_ip(skb);
+
+		if (skb_needs_linearize(skb, features) &&
+		    __skb_linearize(skb))
+			return ERR_PTR(-EINVAL);
+
+		return skb;
+	}
+
 	if (unlikely(skb_needs_check(skb, tx_path))) {
 		int err;
 
@@ -3289,7 +3332,7 @@ static struct sk_buff *validate_xmit_skb(struct sk_buff *skb, struct net_device
 		segs = skb_gso_segment(skb, features);
 		if (IS_ERR(segs)) {
 			goto out_kfree_skb;
-		} else if (segs) {
+		} else if (segs && segs != skb) {
 			consume_skb(skb);
 			skb = segs;
 		}
diff --git a/net/core/flow_dissector.c b/net/core/flow_dissector.c
index ce9eeeb7c024..8ca7e09dca5e 100644
--- a/net/core/flow_dissector.c
+++ b/net/core/flow_dissector.c
@@ -1211,6 +1211,46 @@ static inline u32 ___skb_get_hash(const struct sk_buff *skb,
 	return __flow_hash_from_keys(keys, keyval);
 }
 
+struct _flow_keys_rx_digest_data {
+	__be16	n_proto;
+	u8	ip_proto;
+	u8	poff;
+	__be32	ports;
+	__be32	src;
+	__be32	dst;
+};
+
+u32 skb_flow_keys_rx_digest(struct sk_buff *skb, struct flow_keys_digest *digest)
+{
+	struct flow_keys keys;
+	struct _flow_keys_rx_digest_data *data =
+	    (struct _flow_keys_rx_digest_data *)digest;
+	struct flow_keys_basic *bkeys;
+	u32 poff;
+
+	__flow_hash_secret_init();
+
+	skb_flow_dissect_flow_keys(skb, &keys,
+				   FLOW_DISSECTOR_F_STOP_AT_FLOW_LABEL);
+
+	bkeys = (struct flow_keys_basic *)&keys;
+	poff = __skb_get_poff(skb, skb->data, bkeys, skb_headlen(skb));
+	if (poff > 255)
+		poff = 0;
+
+	BUILD_BUG_ON(sizeof(*data) > sizeof(*digest));
+
+	data->n_proto = keys.basic.n_proto;
+	data->ip_proto = keys.basic.ip_proto;
+	data->ports = keys.ports.ports;
+	data->poff = poff;
+	data->src = keys.addrs.v4addrs.src;
+	data->dst = keys.addrs.v4addrs.dst;
+
+	return poff;
+}
+EXPORT_SYMBOL(skb_flow_keys_rx_digest);
+
 struct _flow_keys_digest_data {
 	__be16	n_proto;
 	u8	ip_proto;
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index c996c09d095f..8f725a78dc93 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -3495,6 +3495,58 @@ static inline skb_frag_t skb_head_frag_to_page_desc(struct sk_buff *frag_skb)
 	return head_frag;
 }
 
+void skb_segment_list(struct sk_buff *skb)
+{
+	struct sk_buff *list_skb = skb_shinfo(skb)->frag_list;
+	unsigned int delta_truesize = 0;
+	unsigned int delta_len = 0;
+	struct sk_buff *tail = NULL;
+	struct sk_buff *nskb;
+
+
+	skb_shinfo(skb)->frag_list = NULL;
+
+	do {
+		nskb = list_skb;
+		list_skb = list_skb->next;
+
+		if (!tail)
+			skb->next = nskb;
+		else
+			tail->next = nskb;
+
+		tail = nskb;
+
+		delta_len += nskb->len;
+		delta_truesize += nskb->truesize;
+
+		if (!secpath_exists(nskb))
+			nskb->sp = secpath_get(skb->sp);
+
+		memcpy(nskb->cb, skb->cb, sizeof(skb->cb));
+
+		nskb->tstamp = skb->tstamp;
+		nskb->dev = skb->dev;
+		nskb->queue_mapping = skb->queue_mapping;
+
+		nskb->mac_len = skb->mac_len;
+		nskb->mac_header = skb->mac_header;
+		nskb->transport_header = skb->transport_header;
+		nskb->network_header = skb->network_header;
+		skb_dst_copy(nskb, skb);
+
+	} while (list_skb);
+
+	skb->truesize = skb->truesize - delta_truesize;
+	skb->data_len = skb->data_len - delta_len;
+	skb->len = skb->len - delta_len;
+
+	skb_gso_reset(skb);
+
+	skb->prev = tail;
+}
+EXPORT_SYMBOL_GPL(skb_segment_list);
+
 /**
  *	skb_segment - Perform protocol segmentation on skb.
  *	@head_skb: buffer to segment
diff --git a/net/ipv4/ip_input.c b/net/ipv4/ip_input.c
index 3196cf58f418..bf710bf95fea 100644
--- a/net/ipv4/ip_input.c
+++ b/net/ipv4/ip_input.c
@@ -190,14 +190,20 @@ bool ip_call_ra_chain(struct sk_buff *skb)
 
 static int ip_local_deliver_finish(struct net *net, struct sock *sk, struct sk_buff *skb)
 {
-	__skb_pull(skb, skb_network_header_len(skb));
+	if (skb_shinfo(skb)->gso_type & SKB_GSO_FRAGLIST)
+		skb_segment_list(skb);
 
 	rcu_read_lock();
-	{
+	do {
 		int protocol = ip_hdr(skb)->protocol;
 		const struct net_protocol *ipprot;
+		struct sk_buff *nskb = skb->next;
 		int raw;
 
+		skb->next = NULL;
+
+		__skb_pull(skb, skb_network_header_len(skb));
+
 	resubmit:
 		raw = raw_local_deliver(skb, protocol);
 
@@ -208,7 +214,7 @@ static int ip_local_deliver_finish(struct net *net, struct sock *sk, struct sk_b
 			if (!ipprot->no_policy) {
 				if (!xfrm4_policy_check(NULL, XFRM_POLICY_IN, skb)) {
 					kfree_skb(skb);
-					goto out;
+					continue;
 				}
 				nf_reset(skb);
 			}
@@ -231,8 +237,8 @@ static int ip_local_deliver_finish(struct net *net, struct sock *sk, struct sk_b
 				consume_skb(skb);
 			}
 		}
-	}
- out:
+		skb = nskb;
+	} while (skb);
 	rcu_read_unlock();
 
 	return 0;
@@ -403,6 +409,10 @@ static int ip_rcv_finish(struct net *net, struct sock *sk, struct sk_buff *skb)
 {
 	int ret;
 
+	/* Remove any debris in the socket control block */
+	memset(IPCB(skb), 0, sizeof(struct inet_skb_parm));
+	IPCB(skb)->iif = skb->skb_iif;
+
 	/* if ingress device is enslaved to an L3 master device pass the
 	 * skb to its handler for processing
 	 */
@@ -416,10 +426,108 @@ static int ip_rcv_finish(struct net *net, struct sock *sk, struct sk_buff *skb)
 	return ret;
 }
 
+struct dissect_skb_cb {
+	struct sk_buff *last;
+	struct	flow_keys_digest keys;
+};
+
+static inline struct dissect_skb_cb *dissect_skb_cb(const struct sk_buff *skb) {
+	return (struct dissect_skb_cb *)skb->cb;
+}
+
+static void ip_sublist_rcv(struct list_head *head, struct net_device *dev,
+			   struct net *net);
+
+static struct sk_buff *ip_flow_dissect(struct sk_buff *skb, struct list_head *rx_list)
+{
+	unsigned int maclen = skb->dev->hard_header_len;
+	const struct iphdr *iph  = ip_hdr(skb);
+	unsigned int gso_type = 0;
+	struct sk_buff *p;
+	u32 poff;
+
+	if (*(u8 *)iph != 0x45)
+		goto out;
+
+	if (ip_is_fragment(iph))
+		goto out;
+
+	dissect_skb_cb(skb)->last = NULL;
+	poff = skb_flow_keys_rx_digest(skb, &dissect_skb_cb(skb)->keys);
+	if (!poff)
+		goto out;
+
+	switch (iph->protocol) {
+	case IPPROTO_TCP:
+		gso_type = SKB_GSO_TCPV4;
+		break;
+	case IPPROTO_UDP:
+		gso_type = SKB_GSO_UDP_L4;
+		break;
+	default:
+		goto out;
+	}
+
+	list_for_each_entry(p, rx_list, list) {
+		unsigned long diffs;
+
+		diffs = (unsigned long)p->dev ^ (unsigned long)skb->dev;
+		diffs |= p->vlan_tci ^ skb->vlan_tci;
+		diffs |= skb_metadata_dst_cmp(p, skb);
+		diffs |= skb_metadata_differs(p, skb);
+		if (maclen == ETH_HLEN)
+			diffs |= compare_ether_header(skb_mac_header(p),
+						      skb_mac_header(skb));
+		else if (!diffs)
+			diffs = memcmp(skb_mac_header(p),
+				       skb_mac_header(skb),
+				       maclen);
+
+		if (diffs)
+			continue;
+
+		if (memcmp(&dissect_skb_cb(p)->keys,
+			   &dissect_skb_cb(skb)->keys,
+			   sizeof(dissect_skb_cb(skb)->keys)))
+			continue;
+
+		if (p->len != skb->len) {
+			if (!list_empty(rx_list))
+				ip_sublist_rcv(rx_list, p->dev, dev_net(p->dev));
+			INIT_LIST_HEAD(rx_list);
+			goto out;
+	}
+
+		skb->next = NULL;
+		skb->prev = NULL;
+
+		if (!dissect_skb_cb(p)->last) {
+			skb_shinfo(p)->gso_size = p->len - poff;
+			skb_shinfo(p)->gso_type |= (SKB_GSO_FRAGLIST | gso_type);
+			skb_shinfo(p)->frag_list = skb;
+			skb_shinfo(p)->gso_segs = 1;
+		} else {
+			dissect_skb_cb(p)->last->next = skb;
+		}
+
+		dissect_skb_cb(p)->last = skb;
+
+		skb_shinfo(p)->gso_segs++;
+		p->data_len += skb->len;
+		p->truesize += skb->truesize;
+		p->len += skb->len;
+
+		return NULL;
+	}
+
+out:
+	return skb;
+}
+
 /*
  * 	Main IP Receive routine.
  */
-static struct sk_buff *ip_rcv_core(struct sk_buff *skb, struct net *net)
+static struct sk_buff *ip_rcv_core(struct list_head *head, struct sk_buff *skb, struct net *net)
 {
 	const struct iphdr *iph;
 	u32 len;
@@ -491,13 +599,14 @@ static struct sk_buff *ip_rcv_core(struct sk_buff *skb, struct net *net)
 
 	skb->transport_header = skb->network_header + iph->ihl*4;
 
-	/* Remove any debris in the socket control block */
-	memset(IPCB(skb), 0, sizeof(struct inet_skb_parm));
-	IPCB(skb)->iif = skb->skb_iif;
-
 	/* Must drop socket now because of tproxy. */
 	skb_orphan(skb);
 
+	if (IN_DEV_FORWARD(__in_dev_get_rcu(skb->dev))) {
+		if (head)
+			return ip_flow_dissect(skb, head);
+	}
+
 	return skb;
 
 csum_error:
@@ -518,9 +627,10 @@ int ip_rcv(struct sk_buff *skb, struct net_device *dev, struct packet_type *pt,
 {
 	struct net *net = dev_net(dev);
 
-	skb = ip_rcv_core(skb, net);
+	skb = ip_rcv_core(NULL, skb, net);
 	if (skb == NULL)
 		return NET_RX_DROP;
+
 	return NF_HOOK(NFPROTO_IPV4, NF_INET_PRE_ROUTING,
 		       net, NULL, skb, dev, NULL,
 		       ip_rcv_finish);
@@ -552,6 +662,11 @@ static void ip_list_rcv_finish(struct net *net, struct sock *sk,
 		struct dst_entry *dst;
 
 		list_del(&skb->list);
+
+		/* Remove any debris in the socket control block */
+		memset(IPCB(skb), 0, sizeof(struct inet_skb_parm));
+		IPCB(skb)->iif = skb->skb_iif;
+
 		/* if ingress device is enslaved to an L3 master device pass the
 		 * skb to its handler for processing
 		 */
@@ -599,7 +714,7 @@ void ip_list_rcv(struct list_head *head, struct packet_type *pt,
 		struct net *net = dev_net(dev);
 
 		list_del(&skb->list);
-		skb = ip_rcv_core(skb, net);
+		skb = ip_rcv_core(&sublist, skb, net);
 		if (skb == NULL)
 			continue;
 
diff --git a/net/ipv4/ip_output.c b/net/ipv4/ip_output.c
index 9c4e72e9c60a..00d8a2576266 100644
--- a/net/ipv4/ip_output.c
+++ b/net/ipv4/ip_output.c
@@ -272,7 +272,8 @@ static int ip_finish_output_gso(struct net *net, struct sock *sk,
 		return -ENOMEM;
 	}
 
-	consume_skb(skb);
+	if (segs != skb)
+		consume_skb(skb);
 
 	do {
 		struct sk_buff *nskb = segs->next;
-- 
2.17.1

^ permalink raw reply related

* Re: [PATCH v2 02/17] compat_ioctl: move drivers to generic_compat_ioctl_ptrarg
From: Jason Gunthorpe @ 2018-09-12 15:33 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: viro, linux-fsdevel, Sudip Mukherjee, Greg Kroah-Hartman,
	Peter Huewe, Jarkko Sakkinen, Stefan Richter, Jiri Kosina,
	Benjamin Tissoires, Alexander Shishkin, Tomas Winkler,
	Artem Bityutskiy, Marek Vasut, David S. Miller, Alex Williamson,
	OGAWA Hirofumi, linux-kernel, linux-integrity, linux1394-devel
In-Reply-To: <20180912150142.157913-2-arnd@arndb.de>

On Wed, Sep 12, 2018 at 05:01:03PM +0200, Arnd Bergmann wrote:
> Each of these drivers has a copy of the same trivial helper function to
> convert the pointer argument and then call the native ioctl handler.
> 
> We now have a generic implementation of that, so use it.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
>  drivers/char/ppdev.c              | 12 +---------
>  drivers/char/tpm/tpm_vtpm_proxy.c | 12 +---------
>  drivers/firewire/core-cdev.c      | 12 +---------
>  drivers/hid/usbhid/hiddev.c       | 11 +--------
>  drivers/hwtracing/stm/core.c      | 12 +---------
>  drivers/misc/mei/main.c           | 22 +----------------
>  drivers/mtd/ubi/cdev.c            | 36 +++-------------------------
>  drivers/net/tap.c                 | 12 +---------
>  drivers/staging/pi433/pi433_if.c  | 12 +---------
>  drivers/usb/core/devio.c          | 16 +------------
>  drivers/vfio/vfio.c               | 39 +++----------------------------
>  drivers/vhost/net.c               | 12 +---------
>  drivers/vhost/scsi.c              | 12 +---------
>  drivers/vhost/test.c              | 12 +---------
>  drivers/vhost/vsock.c             | 12 +---------
>  fs/fat/file.c                     | 13 +----------
>  16 files changed, 20 insertions(+), 237 deletions(-)
> 

> diff --git a/drivers/char/tpm/tpm_vtpm_proxy.c b/drivers/char/tpm/tpm_vtpm_proxy.c
> index 87a0ce47f201..a170f5ca7416 100644
> +++ b/drivers/char/tpm/tpm_vtpm_proxy.c
> @@ -678,20 +678,10 @@ static long vtpmx_fops_ioctl(struct file *f, unsigned int ioctl,
>  	}
>  }
>  
> -#ifdef CONFIG_COMPAT
> -static long vtpmx_fops_compat_ioctl(struct file *f, unsigned int ioctl,
> -					  unsigned long arg)
> -{
> -	return vtpmx_fops_ioctl(f, ioctl, (unsigned long)compat_ptr(arg));
> -}
> -#endif
> -
>  static const struct file_operations vtpmx_fops = {
>  	.owner = THIS_MODULE,
>  	.unlocked_ioctl = vtpmx_fops_ioctl,
> -#ifdef CONFIG_COMPAT
> -	.compat_ioctl = vtpmx_fops_compat_ioctl,
> -#endif
> +	.compat_ioctl = generic_compat_ioctl_ptrarg,
>  	.llseek = noop_llseek,
>  };

For vtpm:

Reviewed-by: Jason Gunthorpe <jgg@mellanox.com>

Arnd, would you consider including a patch as part of/after this
series to make compat_ioctl in drivers/infiniband/core/uverbs_main.c
use this as well?  Looks like a bug too?

Thanks,
Jason

^ permalink raw reply

* Re: KMSAN: uninit-value in pppoe_rcv
From: Alexander Potapenko @ 2018-09-12 10:38 UTC (permalink / raw)
  To: syzbot+f5f6080811c849739212; +Cc: LKML, mostrows, Networking, syzkaller-bugs
In-Reply-To: <0000000000004624c30575a9fd40@google.com>

On Wed, Sep 12, 2018 at 12:24 PM syzbot
<syzbot+f5f6080811c849739212@syzkaller.appspotmail.com> wrote:
>
> Hello,
>
> syzbot found the following crash on:
>
> HEAD commit:    d2d741e5d189 kmsan: add initialization for shmem pages
> git tree:       https://github.com/google/kmsan.git/master
> console output: https://syzkaller.appspot.com/x/log.txt?x=1465fc37800000
> kernel config:  https://syzkaller.appspot.com/x/.config?x=48f9de3384bcd0f
> dashboard link: https://syzkaller.appspot.com/bug?extid=f5f6080811c849739212
> compiler:       clang version 7.0.0 (trunk 329391)
> syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=14d6e607800000
> C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=10a15b5b800000
>
> IMPORTANT: if you fix the bug, please add the following tag to the commit:
> Reported-by: syzbot+f5f6080811c849739212@syzkaller.appspotmail.com
>
> IPVS: ftp: loaded support on port[0] = 21
> ==================================================================
> BUG: KMSAN: uninit-value in __get_item drivers/net/ppp/pppoe.c:172 [inline]
> BUG: KMSAN: uninit-value in get_item drivers/net/ppp/pppoe.c:236 [inline]
> BUG: KMSAN: uninit-value in pppoe_rcv+0xcef/0x10e0
> drivers/net/ppp/pppoe.c:450
> CPU: 0 PID: 4543 Comm: syz-executor355 Not tainted 4.16.0+ #87
> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
> Google 01/01/2011
> Call Trace:
>   __dump_stack lib/dump_stack.c:17 [inline]
>   dump_stack+0x185/0x1d0 lib/dump_stack.c:53
>   kmsan_report+0x142/0x240 mm/kmsan/kmsan.c:1067
>   __msan_warning_32+0x6c/0xb0 mm/kmsan/kmsan_instr.c:683
>   __get_item drivers/net/ppp/pppoe.c:172 [inline]
>   get_item drivers/net/ppp/pppoe.c:236 [inline]
>   pppoe_rcv+0xcef/0x10e0 drivers/net/ppp/pppoe.c:450
>   __netif_receive_skb_core+0x47df/0x4a90 net/core/dev.c:4562
>   __netif_receive_skb net/core/dev.c:4627 [inline]
>   netif_receive_skb_internal+0x49d/0x630 net/core/dev.c:4701
>   netif_receive_skb+0x230/0x240 net/core/dev.c:4725
>   tun_rx_batched drivers/net/tun.c:1555 [inline]
>   tun_get_user+0x740f/0x7c60 drivers/net/tun.c:1962
>   tun_chr_write_iter+0x1d4/0x330 drivers/net/tun.c:1990
>   call_write_iter include/linux/fs.h:1782 [inline]
>   new_sync_write fs/read_write.c:469 [inline]
>   __vfs_write+0x7fb/0x9f0 fs/read_write.c:482
>   vfs_write+0x463/0x8d0 fs/read_write.c:544
>   SYSC_write+0x172/0x360 fs/read_write.c:589
>   SyS_write+0x55/0x80 fs/read_write.c:581
>   do_syscall_64+0x309/0x430 arch/x86/entry/common.c:287
>   entry_SYSCALL_64_after_hwframe+0x3d/0xa2
> RIP: 0033:0x4447c9
> RSP: 002b:00007fff64c8fc28 EFLAGS: 00000297 ORIG_RAX: 0000000000000001
> RAX: ffffffffffffffda RBX: 0000000000000000 RCX: 00000000004447c9
> RDX: 000000000000fd87 RSI: 0000000020000600 RDI: 0000000000000004
> RBP: 00000000006cf018 R08: 00007fff64c8fda8 R09: 00007fff00006bda
> R10: 0000000000005fe7 R11: 0000000000000297 R12: 00000000004020d0
> R13: 0000000000402160 R14: 0000000000000000 R15: 0000000000000000
>
> Uninit was created at:
>   kmsan_save_stack_with_flags mm/kmsan/kmsan.c:278 [inline]
>   kmsan_internal_poison_shadow+0xb8/0x1b0 mm/kmsan/kmsan.c:188
>   kmsan_kmalloc+0x94/0x100 mm/kmsan/kmsan.c:314
>   kmsan_slab_alloc+0x11/0x20 mm/kmsan/kmsan.c:321
>   slab_post_alloc_hook mm/slab.h:445 [inline]
>   slab_alloc_node mm/slub.c:2737 [inline]
>   __kmalloc_node_track_caller+0xaed/0x11c0 mm/slub.c:4369
>   __kmalloc_reserve net/core/skbuff.c:138 [inline]
>   __alloc_skb+0x2cf/0x9f0 net/core/skbuff.c:206
>   alloc_skb include/linux/skbuff.h:984 [inline]
>   alloc_skb_with_frags+0x1d4/0xb20 net/core/skbuff.c:5234
>   sock_alloc_send_pskb+0xb56/0x1190 net/core/sock.c:2085
>   tun_alloc_skb drivers/net/tun.c:1532 [inline]
>   tun_get_user+0x2242/0x7c60 drivers/net/tun.c:1829
>   tun_chr_write_iter+0x1d4/0x330 drivers/net/tun.c:1990
>   call_write_iter include/linux/fs.h:1782 [inline]
>   new_sync_write fs/read_write.c:469 [inline]
>   __vfs_write+0x7fb/0x9f0 fs/read_write.c:482
>   vfs_write+0x463/0x8d0 fs/read_write.c:544
>   SYSC_write+0x172/0x360 fs/read_write.c:589
>   SyS_write+0x55/0x80 fs/read_write.c:581
>   do_syscall_64+0x309/0x430 arch/x86/entry/common.c:287
>   entry_SYSCALL_64_after_hwframe+0x3d/0xa2
> ==================================================================
I did a little digging before sending the bug upstream.
If I add memset(obj, 0xfe, size) to __kmalloc_reserve(), these 0xfe
bytes are visible in __get_item() at the place where KMSAN reports an
error.

The problem is somehow related to tun_get_user() creating a fragmented
sk_buff - when I change the call to tun_alloc_skb() so that it
allocates a single buffer the bug goes away.

>
> ---
> This bug is generated by a bot. It may contain errors.
> See https://goo.gl/tpsmEJ for more information about syzbot.
> syzbot engineers can be reached at syzkaller@googlegroups.com.
>
> syzbot will keep track of this bug report. See:
> https://goo.gl/tpsmEJ#bug-status-tracking for how to communicate with
> syzbot.
> syzbot can test patches for this bug, for details see:
> https://goo.gl/tpsmEJ#testing-patches
>
> --
> You received this message because you are subscribed to the Google Groups "syzkaller-bugs" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to syzkaller-bugs+unsubscribe@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/syzkaller-bugs/0000000000004624c30575a9fd40%40google.com.
> For more options, visit https://groups.google.com/d/optout.



-- 
Alexander Potapenko
Software Engineer

Google Germany GmbH
Erika-Mann-Straße, 33
80636 München

Geschäftsführer: Paul Manicle, Halimah DeLaine Prado
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg

^ permalink raw reply

* Re: [PATCH v2 05/17] compat_ioctl: move more drivers to generic_compat_ioctl_ptrarg
From: Jason Gunthorpe @ 2018-09-12 15:56 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: viro, linux-fsdevel, Greg Kroah-Hartman, David S. Miller, devel,
	linux-kernel, qat-linux, linux-crypto, linux-media, dri-devel,
	linaro-mm-sig, amd-gfx, linux-input, linux-iio, linux-rdma,
	linux-nvdimm, linux-nvme, linux-pci, platform-driver-x86,
	linux-remoteproc, sparclinux, linux-scsi, linux-usb, linux-fbdev,
	linuxppc-dev, linux-btrfs
In-Reply-To: <20180912151134.436719-1-arnd@arndb.de>

On Wed, Sep 12, 2018 at 05:08:52PM +0200, Arnd Bergmann wrote:
> The .ioctl and .compat_ioctl file operations have the same prototype so
> they can both point to the same function, which works great almost all
> the time when all the commands are compatible.
> 
> One exception is the s390 architecture, where a compat pointer is only
> 31 bit wide, and converting it into a 64-bit pointer requires calling
> compat_ptr(). Most drivers here will ever run in s390, but since we now
> have a generic helper for it, it's easy enough to use it consistently.
> 
> I double-checked all these drivers to ensure that all ioctl arguments
> are used as pointers or are ignored, but are not interpreted as integer
> values.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
>  drivers/android/binder.c                    | 2 +-
>  drivers/crypto/qat/qat_common/adf_ctl_drv.c | 2 +-
>  drivers/dma-buf/dma-buf.c                   | 4 +---
>  drivers/dma-buf/sw_sync.c                   | 2 +-
>  drivers/dma-buf/sync_file.c                 | 2 +-
>  drivers/gpu/drm/amd/amdkfd/kfd_chardev.c    | 2 +-
>  drivers/hid/hidraw.c                        | 4 +---
>  drivers/iio/industrialio-core.c             | 2 +-
>  drivers/infiniband/core/uverbs_main.c       | 4 ++--
>  drivers/media/rc/lirc_dev.c                 | 4 +---
>  drivers/mfd/cros_ec_dev.c                   | 4 +---
>  drivers/misc/vmw_vmci/vmci_host.c           | 2 +-
>  drivers/nvdimm/bus.c                        | 4 ++--
>  drivers/nvme/host/core.c                    | 2 +-
>  drivers/pci/switch/switchtec.c              | 2 +-
>  drivers/platform/x86/wmi.c                  | 2 +-
>  drivers/rpmsg/rpmsg_char.c                  | 4 ++--
>  drivers/sbus/char/display7seg.c             | 2 +-
>  drivers/sbus/char/envctrl.c                 | 4 +---
>  drivers/scsi/3w-xxxx.c                      | 4 +---
>  drivers/scsi/cxlflash/main.c                | 2 +-
>  drivers/scsi/esas2r/esas2r_main.c           | 2 +-
>  drivers/scsi/pmcraid.c                      | 4 +---
>  drivers/staging/android/ion/ion.c           | 4 +---
>  drivers/staging/vme/devices/vme_user.c      | 2 +-
>  drivers/tee/tee_core.c                      | 2 +-
>  drivers/usb/class/cdc-wdm.c                 | 2 +-
>  drivers/usb/class/usbtmc.c                  | 4 +---
>  drivers/video/fbdev/ps3fb.c                 | 2 +-
>  drivers/virt/fsl_hypervisor.c               | 2 +-
>  fs/btrfs/super.c                            | 2 +-
>  fs/ceph/dir.c                               | 2 +-
>  fs/ceph/file.c                              | 2 +-
>  fs/fuse/dev.c                               | 2 +-
>  fs/notify/fanotify/fanotify_user.c          | 2 +-
>  fs/userfaultfd.c                            | 2 +-
>  net/rfkill/core.c                           | 2 +-
>  37 files changed, 40 insertions(+), 58 deletions(-)

> diff --git a/drivers/infiniband/core/uverbs_main.c b/drivers/infiniband/core/uverbs_main.c
> index 823beca448e1..f4755c1c9cfa 100644
> +++ b/drivers/infiniband/core/uverbs_main.c
> @@ -930,7 +930,7 @@ static const struct file_operations uverbs_fops = {
>  	.release = ib_uverbs_close,
>  	.llseek	 = no_llseek,
>  	.unlocked_ioctl = ib_uverbs_ioctl,
> -	.compat_ioctl = ib_uverbs_ioctl,
> +	.compat_ioctl = generic_compat_ioctl_ptrarg,
>  };
>  
>  static const struct file_operations uverbs_mmap_fops = {
> @@ -941,7 +941,7 @@ static const struct file_operations uverbs_mmap_fops = {
>  	.release = ib_uverbs_close,
>  	.llseek	 = no_llseek,
>  	.unlocked_ioctl = ib_uverbs_ioctl,
> -	.compat_ioctl = ib_uverbs_ioctl,
> +	.compat_ioctl = generic_compat_ioctl_ptrarg,
>  };
>  
>  static struct ib_client uverbs_client = {

For uverbs:

Acked-by: Jason Gunthorpe <jgg@mellanox.com>

It is very strange, this patch did not appear in the RDMA patchworks,
I almost missed it  :|

Jason

^ permalink raw reply

* Re: [PATCH v2 05/17] compat_ioctl: move more drivers to generic_compat_ioctl_ptrarg
From: Daniel Vetter @ 2018-09-12 15:58 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Linux Fbdev development list, linux-iio-u79uwXL29TY76Z2rM5mHXA,
	Linux PCI, linux-remoteproc-u79uwXL29TY76Z2rM5mHXA, dri-devel,
	platform-driver-x86-u79uwXL29TY76Z2rM5mHXA,
	sparclinux-u79uwXL29TY76Z2rM5mHXA, driverdevel,
	linux-scsi-u79uwXL29TY76Z2rM5mHXA,
	linux-nvdimm-hn68Rpc1hR1g9hUCZPvPmw,
	linux-rdma-u79uwXL29TY76Z2rM5mHXA,
	qat-linux-ral2JQCrhuEAvxtiuMwx3w, amd-gfx list,
	linux-input-u79uwXL29TY76Z2rM5mHXA,
	open list:DMA BUFFER SHARING FRAMEWORK,
	moderated list:DMA BUFFER SHARING FRAMEWORK,
	linux-nvme-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	ceph-devel-u79uwXL29TY76Z2rM5mHXA, Greg Kroah-Hartman, USB list,
	linux-wireless-u79uwXL29TY76Z2rM5mHXA, Linux Kernel Mailing List,
	lin
In-Reply-To: <20180912151134.436719-1-arnd-r2nGTMty4D4@public.gmane.org>

On Wed, Sep 12, 2018 at 5:08 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> The .ioctl and .compat_ioctl file operations have the same prototype so
> they can both point to the same function, which works great almost all
> the time when all the commands are compatible.
>
> One exception is the s390 architecture, where a compat pointer is only
> 31 bit wide, and converting it into a 64-bit pointer requires calling
> compat_ptr(). Most drivers here will ever run in s390, but since we now
> have a generic helper for it, it's easy enough to use it consistently.
>
> I double-checked all these drivers to ensure that all ioctl arguments
> are used as pointers or are ignored, but are not interpreted as integer
> values.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>

At least for the drm and dma-buf bits.
-Daniel

> ---
>  drivers/android/binder.c                    | 2 +-
>  drivers/crypto/qat/qat_common/adf_ctl_drv.c | 2 +-
>  drivers/dma-buf/dma-buf.c                   | 4 +---
>  drivers/dma-buf/sw_sync.c                   | 2 +-
>  drivers/dma-buf/sync_file.c                 | 2 +-
>  drivers/gpu/drm/amd/amdkfd/kfd_chardev.c    | 2 +-
>  drivers/hid/hidraw.c                        | 4 +---
>  drivers/iio/industrialio-core.c             | 2 +-
>  drivers/infiniband/core/uverbs_main.c       | 4 ++--
>  drivers/media/rc/lirc_dev.c                 | 4 +---
>  drivers/mfd/cros_ec_dev.c                   | 4 +---
>  drivers/misc/vmw_vmci/vmci_host.c           | 2 +-
>  drivers/nvdimm/bus.c                        | 4 ++--
>  drivers/nvme/host/core.c                    | 2 +-
>  drivers/pci/switch/switchtec.c              | 2 +-
>  drivers/platform/x86/wmi.c                  | 2 +-
>  drivers/rpmsg/rpmsg_char.c                  | 4 ++--
>  drivers/sbus/char/display7seg.c             | 2 +-
>  drivers/sbus/char/envctrl.c                 | 4 +---
>  drivers/scsi/3w-xxxx.c                      | 4 +---
>  drivers/scsi/cxlflash/main.c                | 2 +-
>  drivers/scsi/esas2r/esas2r_main.c           | 2 +-
>  drivers/scsi/pmcraid.c                      | 4 +---
>  drivers/staging/android/ion/ion.c           | 4 +---
>  drivers/staging/vme/devices/vme_user.c      | 2 +-
>  drivers/tee/tee_core.c                      | 2 +-
>  drivers/usb/class/cdc-wdm.c                 | 2 +-
>  drivers/usb/class/usbtmc.c                  | 4 +---
>  drivers/video/fbdev/ps3fb.c                 | 2 +-
>  drivers/virt/fsl_hypervisor.c               | 2 +-
>  fs/btrfs/super.c                            | 2 +-
>  fs/ceph/dir.c                               | 2 +-
>  fs/ceph/file.c                              | 2 +-
>  fs/fuse/dev.c                               | 2 +-
>  fs/notify/fanotify/fanotify_user.c          | 2 +-
>  fs/userfaultfd.c                            | 2 +-
>  net/rfkill/core.c                           | 2 +-
>  37 files changed, 40 insertions(+), 58 deletions(-)
>
> diff --git a/drivers/android/binder.c b/drivers/android/binder.c
> index d58763b6b009..d2464f5759f8 100644
> --- a/drivers/android/binder.c
> +++ b/drivers/android/binder.c
> @@ -5576,7 +5576,7 @@ static const struct file_operations binder_fops = {
>         .owner = THIS_MODULE,
>         .poll = binder_poll,
>         .unlocked_ioctl = binder_ioctl,
> -       .compat_ioctl = binder_ioctl,
> +       .compat_ioctl = generic_compat_ioctl_ptrarg,
>         .mmap = binder_mmap,
>         .open = binder_open,
>         .flush = binder_flush,
> diff --git a/drivers/crypto/qat/qat_common/adf_ctl_drv.c b/drivers/crypto/qat/qat_common/adf_ctl_drv.c
> index abc7a7f64d64..8ff77a70addc 100644
> --- a/drivers/crypto/qat/qat_common/adf_ctl_drv.c
> +++ b/drivers/crypto/qat/qat_common/adf_ctl_drv.c
> @@ -68,7 +68,7 @@ static long adf_ctl_ioctl(struct file *fp, unsigned int cmd, unsigned long arg);
>  static const struct file_operations adf_ctl_ops = {
>         .owner = THIS_MODULE,
>         .unlocked_ioctl = adf_ctl_ioctl,
> -       .compat_ioctl = adf_ctl_ioctl,
> +       .compat_ioctl = generic_compat_ioctl_ptrarg,
>  };
>
>  struct adf_ctl_drv_info {
> diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
> index 13884474d158..a6d7dc4cf7e9 100644
> --- a/drivers/dma-buf/dma-buf.c
> +++ b/drivers/dma-buf/dma-buf.c
> @@ -325,9 +325,7 @@ static const struct file_operations dma_buf_fops = {
>         .llseek         = dma_buf_llseek,
>         .poll           = dma_buf_poll,
>         .unlocked_ioctl = dma_buf_ioctl,
> -#ifdef CONFIG_COMPAT
> -       .compat_ioctl   = dma_buf_ioctl,
> -#endif
> +       .compat_ioctl   = generic_compat_ioctl_ptrarg,
>  };
>
>  /*
> diff --git a/drivers/dma-buf/sw_sync.c b/drivers/dma-buf/sw_sync.c
> index 53c1d6d36a64..bc810506d487 100644
> --- a/drivers/dma-buf/sw_sync.c
> +++ b/drivers/dma-buf/sw_sync.c
> @@ -419,5 +419,5 @@ const struct file_operations sw_sync_debugfs_fops = {
>         .open           = sw_sync_debugfs_open,
>         .release        = sw_sync_debugfs_release,
>         .unlocked_ioctl = sw_sync_ioctl,
> -       .compat_ioctl   = sw_sync_ioctl,
> +       .compat_ioctl   = generic_compat_ioctl_ptrarg,
>  };
> diff --git a/drivers/dma-buf/sync_file.c b/drivers/dma-buf/sync_file.c
> index 35dd06479867..1c64ed60c658 100644
> --- a/drivers/dma-buf/sync_file.c
> +++ b/drivers/dma-buf/sync_file.c
> @@ -488,5 +488,5 @@ static const struct file_operations sync_file_fops = {
>         .release = sync_file_release,
>         .poll = sync_file_poll,
>         .unlocked_ioctl = sync_file_ioctl,
> -       .compat_ioctl = sync_file_ioctl,
> +       .compat_ioctl = generic_compat_ioctl_ptrarg,
>  };
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
> index 297b36c26a05..1d7b1e3c3ebe 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
> @@ -47,7 +47,7 @@ static const char kfd_dev_name[] = "kfd";
>  static const struct file_operations kfd_fops = {
>         .owner = THIS_MODULE,
>         .unlocked_ioctl = kfd_ioctl,
> -       .compat_ioctl = kfd_ioctl,
> +       .compat_ioctl = generic_compat_ioctl_ptrarg,
>         .open = kfd_open,
>         .mmap = kfd_mmap,
>  };
> diff --git a/drivers/hid/hidraw.c b/drivers/hid/hidraw.c
> index 4a44e48e08b2..e44b64812850 100644
> --- a/drivers/hid/hidraw.c
> +++ b/drivers/hid/hidraw.c
> @@ -476,9 +476,7 @@ static const struct file_operations hidraw_ops = {
>         .release =      hidraw_release,
>         .unlocked_ioctl = hidraw_ioctl,
>         .fasync =       hidraw_fasync,
> -#ifdef CONFIG_COMPAT
> -       .compat_ioctl   = hidraw_ioctl,
> -#endif
> +       .compat_ioctl   = generic_compat_ioctl_ptrarg,
>         .llseek =       noop_llseek,
>  };
>
> diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
> index a062cfddc5af..22844b94b0e9 100644
> --- a/drivers/iio/industrialio-core.c
> +++ b/drivers/iio/industrialio-core.c
> @@ -1630,7 +1630,7 @@ static const struct file_operations iio_buffer_fileops = {
>         .owner = THIS_MODULE,
>         .llseek = noop_llseek,
>         .unlocked_ioctl = iio_ioctl,
> -       .compat_ioctl = iio_ioctl,
> +       .compat_ioctl = generic_compat_ioctl_ptrarg,
>  };
>
>  static int iio_check_unique_scan_index(struct iio_dev *indio_dev)
> diff --git a/drivers/infiniband/core/uverbs_main.c b/drivers/infiniband/core/uverbs_main.c
> index 823beca448e1..f4755c1c9cfa 100644
> --- a/drivers/infiniband/core/uverbs_main.c
> +++ b/drivers/infiniband/core/uverbs_main.c
> @@ -930,7 +930,7 @@ static const struct file_operations uverbs_fops = {
>         .release = ib_uverbs_close,
>         .llseek  = no_llseek,
>         .unlocked_ioctl = ib_uverbs_ioctl,
> -       .compat_ioctl = ib_uverbs_ioctl,
> +       .compat_ioctl = generic_compat_ioctl_ptrarg,
>  };
>
>  static const struct file_operations uverbs_mmap_fops = {
> @@ -941,7 +941,7 @@ static const struct file_operations uverbs_mmap_fops = {
>         .release = ib_uverbs_close,
>         .llseek  = no_llseek,
>         .unlocked_ioctl = ib_uverbs_ioctl,
> -       .compat_ioctl = ib_uverbs_ioctl,
> +       .compat_ioctl = generic_compat_ioctl_ptrarg,
>  };
>
>  static struct ib_client uverbs_client = {
> diff --git a/drivers/media/rc/lirc_dev.c b/drivers/media/rc/lirc_dev.c
> index f862f1b7f996..077209f414ed 100644
> --- a/drivers/media/rc/lirc_dev.c
> +++ b/drivers/media/rc/lirc_dev.c
> @@ -730,9 +730,7 @@ static const struct file_operations lirc_fops = {
>         .owner          = THIS_MODULE,
>         .write          = ir_lirc_transmit_ir,
>         .unlocked_ioctl = ir_lirc_ioctl,
> -#ifdef CONFIG_COMPAT
> -       .compat_ioctl   = ir_lirc_ioctl,
> -#endif
> +       .compat_ioctl   = generic_compat_ioctl_ptrarg,
>         .read           = ir_lirc_read,
>         .poll           = ir_lirc_poll,
>         .open           = ir_lirc_open,
> diff --git a/drivers/mfd/cros_ec_dev.c b/drivers/mfd/cros_ec_dev.c
> index 999dac752bcc..35a04bcf55da 100644
> --- a/drivers/mfd/cros_ec_dev.c
> +++ b/drivers/mfd/cros_ec_dev.c
> @@ -258,9 +258,7 @@ static const struct file_operations fops = {
>         .release = ec_device_release,
>         .read = ec_device_read,
>         .unlocked_ioctl = ec_device_ioctl,
> -#ifdef CONFIG_COMPAT
> -       .compat_ioctl = ec_device_ioctl,
> -#endif
> +       .compat_ioctl = generic_compat_ioctl_ptrarg,
>  };
>
>  static void cros_ec_sensors_register(struct cros_ec_dev *ec)
> diff --git a/drivers/misc/vmw_vmci/vmci_host.c b/drivers/misc/vmw_vmci/vmci_host.c
> index 83e0c95d20a4..1308f889e53b 100644
> --- a/drivers/misc/vmw_vmci/vmci_host.c
> +++ b/drivers/misc/vmw_vmci/vmci_host.c
> @@ -983,7 +983,7 @@ static const struct file_operations vmuser_fops = {
>         .release        = vmci_host_close,
>         .poll           = vmci_host_poll,
>         .unlocked_ioctl = vmci_host_unlocked_ioctl,
> -       .compat_ioctl   = vmci_host_unlocked_ioctl,
> +       .compat_ioctl   = generic_compat_ioctl_ptrarg,
>  };
>
>  static struct miscdevice vmci_host_miscdev = {
> diff --git a/drivers/nvdimm/bus.c b/drivers/nvdimm/bus.c
> index 8aae6dcc839f..7449cbc55df7 100644
> --- a/drivers/nvdimm/bus.c
> +++ b/drivers/nvdimm/bus.c
> @@ -1133,7 +1133,7 @@ static const struct file_operations nvdimm_bus_fops = {
>         .owner = THIS_MODULE,
>         .open = nd_open,
>         .unlocked_ioctl = nd_ioctl,
> -       .compat_ioctl = nd_ioctl,
> +       .compat_ioctl = generic_compat_ioctl_ptrarg,
>         .llseek = noop_llseek,
>  };
>
> @@ -1141,7 +1141,7 @@ static const struct file_operations nvdimm_fops = {
>         .owner = THIS_MODULE,
>         .open = nd_open,
>         .unlocked_ioctl = nvdimm_ioctl,
> -       .compat_ioctl = nvdimm_ioctl,
> +       .compat_ioctl = generic_compat_ioctl_ptrarg,
>         .llseek = noop_llseek,
>  };
>
> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
> index dd8ec1dd9219..2d986f573a29 100644
> --- a/drivers/nvme/host/core.c
> +++ b/drivers/nvme/host/core.c
> @@ -2579,7 +2579,7 @@ static const struct file_operations nvme_dev_fops = {
>         .owner          = THIS_MODULE,
>         .open           = nvme_dev_open,
>         .unlocked_ioctl = nvme_dev_ioctl,
> -       .compat_ioctl   = nvme_dev_ioctl,
> +       .compat_ioctl   = generic_compat_ioctl_ptrarg,
>  };
>
>  static ssize_t nvme_sysfs_reset(struct device *dev,
> diff --git a/drivers/pci/switch/switchtec.c b/drivers/pci/switch/switchtec.c
> index 9940cc70f38b..4296919c784e 100644
> --- a/drivers/pci/switch/switchtec.c
> +++ b/drivers/pci/switch/switchtec.c
> @@ -967,7 +967,7 @@ static const struct file_operations switchtec_fops = {
>         .read = switchtec_dev_read,
>         .poll = switchtec_dev_poll,
>         .unlocked_ioctl = switchtec_dev_ioctl,
> -       .compat_ioctl = switchtec_dev_ioctl,
> +       .compat_ioctl = generic_compat_ioctl_ptrarg,
>  };
>
>  static void link_event_work(struct work_struct *work)
> diff --git a/drivers/platform/x86/wmi.c b/drivers/platform/x86/wmi.c
> index 04791ea5d97b..e4d0697e07d6 100644
> --- a/drivers/platform/x86/wmi.c
> +++ b/drivers/platform/x86/wmi.c
> @@ -886,7 +886,7 @@ static const struct file_operations wmi_fops = {
>         .read           = wmi_char_read,
>         .open           = wmi_char_open,
>         .unlocked_ioctl = wmi_ioctl,
> -       .compat_ioctl   = wmi_ioctl,
> +       .compat_ioctl   = generic_compat_ioctl_ptrarg,
>  };
>
>  static int wmi_dev_probe(struct device *dev)
> diff --git a/drivers/rpmsg/rpmsg_char.c b/drivers/rpmsg/rpmsg_char.c
> index a76b963a7e50..02aefb2b2d47 100644
> --- a/drivers/rpmsg/rpmsg_char.c
> +++ b/drivers/rpmsg/rpmsg_char.c
> @@ -285,7 +285,7 @@ static const struct file_operations rpmsg_eptdev_fops = {
>         .write = rpmsg_eptdev_write,
>         .poll = rpmsg_eptdev_poll,
>         .unlocked_ioctl = rpmsg_eptdev_ioctl,
> -       .compat_ioctl = rpmsg_eptdev_ioctl,
> +       .compat_ioctl = generic_compat_ioctl_ptrarg,
>  };
>
>  static ssize_t name_show(struct device *dev, struct device_attribute *attr,
> @@ -446,7 +446,7 @@ static const struct file_operations rpmsg_ctrldev_fops = {
>         .open = rpmsg_ctrldev_open,
>         .release = rpmsg_ctrldev_release,
>         .unlocked_ioctl = rpmsg_ctrldev_ioctl,
> -       .compat_ioctl = rpmsg_ctrldev_ioctl,
> +       .compat_ioctl = generic_compat_ioctl_ptrarg,
>  };
>
>  static void rpmsg_ctrldev_release_device(struct device *dev)
> diff --git a/drivers/sbus/char/display7seg.c b/drivers/sbus/char/display7seg.c
> index 5c8ed7350a04..064fe7247eb2 100644
> --- a/drivers/sbus/char/display7seg.c
> +++ b/drivers/sbus/char/display7seg.c
> @@ -155,7 +155,7 @@ static long d7s_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
>  static const struct file_operations d7s_fops = {
>         .owner =                THIS_MODULE,
>         .unlocked_ioctl =       d7s_ioctl,
> -       .compat_ioctl =         d7s_ioctl,
> +       .compat_ioctl =         generic_compat_ioctl_ptrarg,
>         .open =                 d7s_open,
>         .release =              d7s_release,
>         .llseek = noop_llseek,
> diff --git a/drivers/sbus/char/envctrl.c b/drivers/sbus/char/envctrl.c
> index 56e962a01493..a26665ccea56 100644
> --- a/drivers/sbus/char/envctrl.c
> +++ b/drivers/sbus/char/envctrl.c
> @@ -714,9 +714,7 @@ static const struct file_operations envctrl_fops = {
>         .owner =                THIS_MODULE,
>         .read =                 envctrl_read,
>         .unlocked_ioctl =       envctrl_ioctl,
> -#ifdef CONFIG_COMPAT
> -       .compat_ioctl =         envctrl_ioctl,
> -#endif
> +       .compat_ioctl =         generic_compat_ioctl_ptrarg,
>         .open =                 envctrl_open,
>         .release =              envctrl_release,
>         .llseek =               noop_llseek,
> diff --git a/drivers/scsi/3w-xxxx.c b/drivers/scsi/3w-xxxx.c
> index 471366945bd4..86c9f22a152f 100644
> --- a/drivers/scsi/3w-xxxx.c
> +++ b/drivers/scsi/3w-xxxx.c
> @@ -1047,9 +1047,7 @@ static int tw_chrdev_open(struct inode *inode, struct file *file)
>  static const struct file_operations tw_fops = {
>         .owner          = THIS_MODULE,
>         .unlocked_ioctl = tw_chrdev_ioctl,
> -#ifdef CONFIG_COMPAT
> -       .compat_ioctl   = tw_chrdev_ioctl,
> -#endif
> +       .compat_ioctl   = generic_compat_ioctl_ptrarg,
>         .open           = tw_chrdev_open,
>         .release        = NULL,
>         .llseek         = noop_llseek,
> diff --git a/drivers/scsi/cxlflash/main.c b/drivers/scsi/cxlflash/main.c
> index 6637116529aa..d968efeb50e8 100644
> --- a/drivers/scsi/cxlflash/main.c
> +++ b/drivers/scsi/cxlflash/main.c
> @@ -3596,7 +3596,7 @@ static const struct file_operations cxlflash_chr_fops = {
>         .owner          = THIS_MODULE,
>         .open           = cxlflash_chr_open,
>         .unlocked_ioctl = cxlflash_chr_ioctl,
> -       .compat_ioctl   = cxlflash_chr_ioctl,
> +       .compat_ioctl   = generic_compat_ioctl_ptrarg,
>  };
>
>  /**
> diff --git a/drivers/scsi/esas2r/esas2r_main.c b/drivers/scsi/esas2r/esas2r_main.c
> index c07118617d89..95142292e702 100644
> --- a/drivers/scsi/esas2r/esas2r_main.c
> +++ b/drivers/scsi/esas2r/esas2r_main.c
> @@ -614,7 +614,7 @@ static int __init esas2r_init(void)
>
>  /* Handle ioctl calls to "/proc/scsi/esas2r/ATTOnode" */
>  static const struct file_operations esas2r_proc_fops = {
> -       .compat_ioctl   = esas2r_proc_ioctl,
> +       .compat_ioctl   = generic_compat_ioctl_ptrarg,
>         .unlocked_ioctl = esas2r_proc_ioctl,
>  };
>
> diff --git a/drivers/scsi/pmcraid.c b/drivers/scsi/pmcraid.c
> index 4e86994e10e8..8a8c73d3bdad 100644
> --- a/drivers/scsi/pmcraid.c
> +++ b/drivers/scsi/pmcraid.c
> @@ -3999,9 +3999,7 @@ static const struct file_operations pmcraid_fops = {
>         .open = pmcraid_chr_open,
>         .fasync = pmcraid_chr_fasync,
>         .unlocked_ioctl = pmcraid_chr_ioctl,
> -#ifdef CONFIG_COMPAT
> -       .compat_ioctl = pmcraid_chr_ioctl,
> -#endif
> +       .compat_ioctl = generic_compat_ioctl_ptrarg,
>         .llseek = noop_llseek,
>  };
>
> diff --git a/drivers/staging/android/ion/ion.c b/drivers/staging/android/ion/ion.c
> index 99073325b0c0..ef727c235392 100644
> --- a/drivers/staging/android/ion/ion.c
> +++ b/drivers/staging/android/ion/ion.c
> @@ -484,9 +484,7 @@ int ion_query_heaps(struct ion_heap_query *query)
>  static const struct file_operations ion_fops = {
>         .owner          = THIS_MODULE,
>         .unlocked_ioctl = ion_ioctl,
> -#ifdef CONFIG_COMPAT
> -       .compat_ioctl   = ion_ioctl,
> -#endif
> +       .compat_ioctl   = generic_compat_ioctl_ptrarg,
>  };
>
>  static int debug_shrink_set(void *data, u64 val)
> diff --git a/drivers/staging/vme/devices/vme_user.c b/drivers/staging/vme/devices/vme_user.c
> index 6a33aaa1a49f..568700ffd2f2 100644
> --- a/drivers/staging/vme/devices/vme_user.c
> +++ b/drivers/staging/vme/devices/vme_user.c
> @@ -494,7 +494,7 @@ static const struct file_operations vme_user_fops = {
>         .write = vme_user_write,
>         .llseek = vme_user_llseek,
>         .unlocked_ioctl = vme_user_unlocked_ioctl,
> -       .compat_ioctl = vme_user_unlocked_ioctl,
> +       .compat_ioctl = generic_compat_ioctl_ptrarg,
>         .mmap = vme_user_mmap,
>  };
>
> diff --git a/drivers/tee/tee_core.c b/drivers/tee/tee_core.c
> index dd46b758852a..cb79f28be894 100644
> --- a/drivers/tee/tee_core.c
> +++ b/drivers/tee/tee_core.c
> @@ -670,7 +670,7 @@ static const struct file_operations tee_fops = {
>         .open = tee_open,
>         .release = tee_release,
>         .unlocked_ioctl = tee_ioctl,
> -       .compat_ioctl = tee_ioctl,
> +       .compat_ioctl = generic_compat_ioctl_ptrarg,
>  };
>
>  static void tee_release_device(struct device *dev)
> diff --git a/drivers/usb/class/cdc-wdm.c b/drivers/usb/class/cdc-wdm.c
> index bec581fb7c63..6e4998c8e64f 100644
> --- a/drivers/usb/class/cdc-wdm.c
> +++ b/drivers/usb/class/cdc-wdm.c
> @@ -724,7 +724,7 @@ static const struct file_operations wdm_fops = {
>         .release =      wdm_release,
>         .poll =         wdm_poll,
>         .unlocked_ioctl = wdm_ioctl,
> -       .compat_ioctl = wdm_ioctl,
> +       .compat_ioctl = generic_compat_ioctl_ptrarg,
>         .llseek =       noop_llseek,
>  };
>
> diff --git a/drivers/usb/class/usbtmc.c b/drivers/usb/class/usbtmc.c
> index 83ffa5a14c3d..d5da47c4c462 100644
> --- a/drivers/usb/class/usbtmc.c
> +++ b/drivers/usb/class/usbtmc.c
> @@ -1460,9 +1460,7 @@ static const struct file_operations fops = {
>         .open           = usbtmc_open,
>         .release        = usbtmc_release,
>         .unlocked_ioctl = usbtmc_ioctl,
> -#ifdef CONFIG_COMPAT
> -       .compat_ioctl   = usbtmc_ioctl,
> -#endif
> +       .compat_ioctl   = generic_compat_ioctl_ptrarg,
>         .fasync         = usbtmc_fasync,
>         .poll           = usbtmc_poll,
>         .llseek         = default_llseek,
> diff --git a/drivers/video/fbdev/ps3fb.c b/drivers/video/fbdev/ps3fb.c
> index 5ed2db39d823..f9f8ffaf1c4a 100644
> --- a/drivers/video/fbdev/ps3fb.c
> +++ b/drivers/video/fbdev/ps3fb.c
> @@ -949,7 +949,7 @@ static struct fb_ops ps3fb_ops = {
>         .fb_mmap        = ps3fb_mmap,
>         .fb_blank       = ps3fb_blank,
>         .fb_ioctl       = ps3fb_ioctl,
> -       .fb_compat_ioctl = ps3fb_ioctl
> +       .fb_compat_ioctl = generic_compat_ioctl_ptrarg,
>  };
>
>  static const struct fb_fix_screeninfo ps3fb_fix = {
> diff --git a/drivers/virt/fsl_hypervisor.c b/drivers/virt/fsl_hypervisor.c
> index 8ba726e600e9..406b7e492214 100644
> --- a/drivers/virt/fsl_hypervisor.c
> +++ b/drivers/virt/fsl_hypervisor.c
> @@ -703,7 +703,7 @@ static const struct file_operations fsl_hv_fops = {
>         .poll = fsl_hv_poll,
>         .read = fsl_hv_read,
>         .unlocked_ioctl = fsl_hv_ioctl,
> -       .compat_ioctl = fsl_hv_ioctl,
> +       .compat_ioctl = generic_compat_ioctl_ptrarg,
>  };
>
>  static struct miscdevice fsl_hv_misc_dev = {
> diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
> index 6601c9aa5e35..2b5a8ad86305 100644
> --- a/fs/btrfs/super.c
> +++ b/fs/btrfs/super.c
> @@ -2352,7 +2352,7 @@ static const struct super_operations btrfs_super_ops = {
>  static const struct file_operations btrfs_ctl_fops = {
>         .open = btrfs_control_open,
>         .unlocked_ioctl  = btrfs_control_ioctl,
> -       .compat_ioctl = btrfs_control_ioctl,
> +       .compat_ioctl = generic_compat_ioctl_ptrarg,
>         .owner   = THIS_MODULE,
>         .llseek = noop_llseek,
>  };
> diff --git a/fs/ceph/dir.c b/fs/ceph/dir.c
> index da73f29d7faa..eb869fe6774d 100644
> --- a/fs/ceph/dir.c
> +++ b/fs/ceph/dir.c
> @@ -1489,7 +1489,7 @@ const struct file_operations ceph_dir_fops = {
>         .open = ceph_open,
>         .release = ceph_release,
>         .unlocked_ioctl = ceph_ioctl,
> -       .compat_ioctl = ceph_ioctl,
> +       .compat_ioctl = generic_compat_ioctl_ptrarg,
>         .fsync = ceph_fsync,
>         .lock = ceph_lock,
>         .flock = ceph_flock,
> diff --git a/fs/ceph/file.c b/fs/ceph/file.c
> index 92ab20433682..85094042cfac 100644
> --- a/fs/ceph/file.c
> +++ b/fs/ceph/file.c
> @@ -1842,7 +1842,7 @@ const struct file_operations ceph_file_fops = {
>         .splice_read = generic_file_splice_read,
>         .splice_write = iter_file_splice_write,
>         .unlocked_ioctl = ceph_ioctl,
> -       .compat_ioctl   = ceph_ioctl,
> +       .compat_ioctl   = generic_compat_ioctl_ptrarg,
>         .fallocate      = ceph_fallocate,
>  };
>
> diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c
> index 11ea2c4a38ab..a6d4a24963ed 100644
> --- a/fs/fuse/dev.c
> +++ b/fs/fuse/dev.c
> @@ -2258,7 +2258,7 @@ const struct file_operations fuse_dev_operations = {
>         .release        = fuse_dev_release,
>         .fasync         = fuse_dev_fasync,
>         .unlocked_ioctl = fuse_dev_ioctl,
> -       .compat_ioctl   = fuse_dev_ioctl,
> +       .compat_ioctl   = generic_compat_ioctl_ptrarg,
>  };
>  EXPORT_SYMBOL_GPL(fuse_dev_operations);
>
> diff --git a/fs/notify/fanotify/fanotify_user.c b/fs/notify/fanotify/fanotify_user.c
> index 69054886915b..fc4193b384cf 100644
> --- a/fs/notify/fanotify/fanotify_user.c
> +++ b/fs/notify/fanotify/fanotify_user.c
> @@ -447,7 +447,7 @@ static const struct file_operations fanotify_fops = {
>         .fasync         = NULL,
>         .release        = fanotify_release,
>         .unlocked_ioctl = fanotify_ioctl,
> -       .compat_ioctl   = fanotify_ioctl,
> +       .compat_ioctl   = generic_compat_ioctl_ptrarg,
>         .llseek         = noop_llseek,
>  };
>
> diff --git a/fs/userfaultfd.c b/fs/userfaultfd.c
> index bfa0ec69f924..bc9118b58a8a 100644
> --- a/fs/userfaultfd.c
> +++ b/fs/userfaultfd.c
> @@ -1878,7 +1878,7 @@ static const struct file_operations userfaultfd_fops = {
>         .poll           = userfaultfd_poll,
>         .read           = userfaultfd_read,
>         .unlocked_ioctl = userfaultfd_ioctl,
> -       .compat_ioctl   = userfaultfd_ioctl,
> +       .compat_ioctl   = generic_compat_ioctl_ptrarg,
>         .llseek         = noop_llseek,
>  };
>
> diff --git a/net/rfkill/core.c b/net/rfkill/core.c
> index 1355f5ca8d22..ba68b53f58ab 100644
> --- a/net/rfkill/core.c
> +++ b/net/rfkill/core.c
> @@ -1323,7 +1323,7 @@ static const struct file_operations rfkill_fops = {
>         .release        = rfkill_fop_release,
>  #ifdef CONFIG_RFKILL_INPUT
>         .unlocked_ioctl = rfkill_fop_ioctl,
> -       .compat_ioctl   = rfkill_fop_ioctl,
> +       .compat_ioctl   = generic_compat_ioctl_ptrarg,
>  #endif
>         .llseek         = no_llseek,
>  };
> --
> 2.18.0
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel



-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply

* Re: [PATCH net-next v2 2/5] virtio_ring: support creating packed ring
From: Michael S. Tsirkin @ 2018-09-12 16:12 UTC (permalink / raw)
  To: Tiwei Bie
  Cc: jasowang, virtualization, linux-kernel, netdev, virtio-dev, wexu,
	jfreimann
In-Reply-To: <20180912075140.GA22545@debian>

On Wed, Sep 12, 2018 at 03:51:40PM +0800, Tiwei Bie wrote:
> On Mon, Sep 10, 2018 at 10:28:37AM +0800, Tiwei Bie wrote:
> > On Fri, Sep 07, 2018 at 10:03:24AM -0400, Michael S. Tsirkin wrote:
> > > On Wed, Jul 11, 2018 at 10:27:08AM +0800, Tiwei Bie wrote:
> > > > This commit introduces the support for creating packed ring.
> > > > All split ring specific functions are added _split suffix.
> > > > Some necessary stubs for packed ring are also added.
> > > > 
> > > > Signed-off-by: Tiwei Bie <tiwei.bie@intel.com>
> > > 
> [...]
> > > > +
> > > > +/*
> > > > + * The layout for the packed ring is a continuous chunk of memory
> > > > + * which looks like this.
> > > > + *
> > > > + * struct vring_packed {
> > > > + *	// The actual descriptors (16 bytes each)
> > > > + *	struct vring_packed_desc desc[num];
> > > > + *
> > > > + *	// Padding to the next align boundary.
> > > > + *	char pad[];
> > > > + *
> > > > + *	// Driver Event Suppression
> > > > + *	struct vring_packed_desc_event driver;
> > > > + *
> > > > + *	// Device Event Suppression
> > > > + *	struct vring_packed_desc_event device;
> > > > + * };
> > > > + */
> > > 
> > > Why not just allocate event structures separately?
> > > Is it a win to have them share a cache line for some reason?
> 
> Users may call vring_new_virtqueue() with preallocated
> memory to setup the ring, e.g.:
> 
> https://github.com/torvalds/linux/blob/11da3a7f84f1/drivers/s390/virtio/virtio_ccw.c#L513-L522
> https://github.com/torvalds/linux/blob/11da3a7f84f1/drivers/misc/mic/vop/vop_main.c#L306-L307
> 
> Below is the corresponding definition in split ring:
> 
> https://github.com/oasis-tcs/virtio-spec/blob/89dd55f5e606/split-ring.tex#L64-L78
> 
> If my understanding is correct, this is just for legacy
> interfaces, and we won't define layout in packed ring
> and don't need to support vring_new_virtqueue() in packed
> ring. Is it correct? Thanks!

mic doesn't support 1.0 yet but ccw does.

It's probably best to look into converting ccw to
virtio_create_virtqueue, then you can just fail
vring_new_virtqueue for packed.

Cornelia, are there any gotchas to look out for?

> 
> 
> > 
> > Will do that.
> > 
> > > 
> > > > +static inline void vring_init_packed(struct vring_packed *vr, unsigned int num,
> > > > +				     void *p, unsigned long align)
> > > > +{
> > > > +	vr->num = num;
> > > > +	vr->desc = p;
> > > > +	vr->driver = (void *)ALIGN(((uintptr_t)p +
> > > > +		sizeof(struct vring_packed_desc) * num), align);
> > > > +	vr->device = vr->driver + 1;
> > > > +}
> > > 
> > > What's all this about alignment? Where does it come from?
> > 
> > It comes from the `vring_align` parameter of vring_create_virtqueue()
> > and vring_new_virtqueue():
> > 
> > https://github.com/torvalds/linux/blob/a49a9dcce802/drivers/virtio/virtio_ring.c#L1061
> > https://github.com/torvalds/linux/blob/a49a9dcce802/drivers/virtio/virtio_ring.c#L1123
> > 
> > Should I just ignore it in packed ring?
> > 
> > CCW defined this:
> > 
> > #define KVM_VIRTIO_CCW_RING_ALIGN 4096
> > 
> > I'm not familiar with CCW. Currently, in this patch set, packed ring
> > isn't enabled on CCW dues to some legacy accessors are not implemented
> > in packed ring yet.
> > 
> > > 
> > > > +
> > > > +static inline unsigned vring_size_packed(unsigned int num, unsigned long align)
> > > > +{
> > > > +	return ((sizeof(struct vring_packed_desc) * num + align - 1)
> > > > +		& ~(align - 1)) + sizeof(struct vring_packed_desc_event) * 2;
> > > > +}
> > [...]
> > > > @@ -1129,10 +1388,17 @@ struct virtqueue *vring_new_virtqueue(unsigned int index,
> > > >  				      void (*callback)(struct virtqueue *vq),
> > > >  				      const char *name)
> > > >  {
> > > > -	struct vring vring;
> > > > -	vring_init(&vring, num, pages, vring_align);
> > > > -	return __vring_new_virtqueue(index, vring, vdev, weak_barriers, context,
> > > > -				     notify, callback, name);
> > > > +	union vring_union vring;
> > > > +	bool packed;
> > > > +
> > > > +	packed = virtio_has_feature(vdev, VIRTIO_F_RING_PACKED);
> > > > +	if (packed)
> > > > +		vring_init_packed(&vring.vring_packed, num, pages, vring_align);
> > > > +	else
> > > > +		vring_init(&vring.vring_split, num, pages, vring_align);
> > > 
> > > 
> > > vring_init in the UAPI header is more or less a bug.
> > > I'd just stop using it, keep it around for legacy userspace.
> > 
> > Got it. I'd like to do that. Thanks.
> > 
> > > 
> > > > +
> > > > +	return __vring_new_virtqueue(index, vring, packed, vdev, weak_barriers,
> > > > +				     context, notify, callback, name);
> > > >  }
> > > >  EXPORT_SYMBOL_GPL(vring_new_virtqueue);
> > > >  
> > > > @@ -1142,7 +1408,9 @@ void vring_del_virtqueue(struct virtqueue *_vq)
> > > >  
> > > >  	if (vq->we_own_ring) {
> > > >  		vring_free_queue(vq->vq.vdev, vq->queue_size_in_bytes,
> > > > -				 vq->vring.desc, vq->queue_dma_addr);
> > > > +				 vq->packed ? (void *)vq->vring_packed.desc :
> > > > +					      (void *)vq->vring.desc,
> > > > +				 vq->queue_dma_addr);
> > > >  	}
> > > >  	list_del(&_vq->list);
> > > >  	kfree(vq);
> > > > @@ -1184,7 +1452,7 @@ unsigned int virtqueue_get_vring_size(struct virtqueue *_vq)
> > > >  
> > > >  	struct vring_virtqueue *vq = to_vvq(_vq);
> > > >  
> > > > -	return vq->vring.num;
> > > > +	return vq->packed ? vq->vring_packed.num : vq->vring.num;
> > > >  }
> > > >  EXPORT_SYMBOL_GPL(virtqueue_get_vring_size);
> > > >  
> > > > @@ -1227,6 +1495,10 @@ dma_addr_t virtqueue_get_avail_addr(struct virtqueue *_vq)
> > > >  
> > > >  	BUG_ON(!vq->we_own_ring);
> > > >  
> > > > +	if (vq->packed)
> > > > +		return vq->queue_dma_addr + ((char *)vq->vring_packed.driver -
> > > > +				(char *)vq->vring_packed.desc);
> > > > +
> > > >  	return vq->queue_dma_addr +
> > > >  		((char *)vq->vring.avail - (char *)vq->vring.desc);
> > > >  }
> > > > @@ -1238,11 +1510,16 @@ dma_addr_t virtqueue_get_used_addr(struct virtqueue *_vq)
> > > >  
> > > >  	BUG_ON(!vq->we_own_ring);
> > > >  
> > > > +	if (vq->packed)
> > > > +		return vq->queue_dma_addr + ((char *)vq->vring_packed.device -
> > > > +				(char *)vq->vring_packed.desc);
> > > > +
> > > >  	return vq->queue_dma_addr +
> > > >  		((char *)vq->vring.used - (char *)vq->vring.desc);
> > > >  }
> > > >  EXPORT_SYMBOL_GPL(virtqueue_get_used_addr);
> > > >  
> > > > +/* Only available for split ring */
> > > >  const struct vring *virtqueue_get_vring(struct virtqueue *vq)
> > > >  {
> > > >  	return &to_vvq(vq)->vring;
> > > > diff --git a/include/linux/virtio_ring.h b/include/linux/virtio_ring.h
> > > > index fab02133a919..992142b35f55 100644
> > > > --- a/include/linux/virtio_ring.h
> > > > +++ b/include/linux/virtio_ring.h
> > > > @@ -60,6 +60,11 @@ static inline void virtio_store_mb(bool weak_barriers,
> > > >  struct virtio_device;
> > > >  struct virtqueue;
> > > >  
> > > > +union vring_union {
> > > > +	struct vring vring_split;
> > > > +	struct vring_packed vring_packed;
> > > > +};
> > > > +
> > > >  /*
> > > >   * Creates a virtqueue and allocates the descriptor ring.  If
> > > >   * may_reduce_num is set, then this may allocate a smaller ring than
> > > > @@ -79,7 +84,8 @@ struct virtqueue *vring_create_virtqueue(unsigned int index,
> > > >  
> > > >  /* Creates a virtqueue with a custom layout. */
> > > >  struct virtqueue *__vring_new_virtqueue(unsigned int index,
> > > > -					struct vring vring,
> > > > +					union vring_union vring,
> > > > +					bool packed,
> > > >  					struct virtio_device *vdev,
> > > >  					bool weak_barriers,
> > > >  					bool ctx,
> > > > -- 
> > > > 2.18.0

^ permalink raw reply

* Re: [virtio-dev] Re: [PATCH net-next v2 0/5] virtio: support packed ring
From: Michael S. Tsirkin @ 2018-09-12 16:16 UTC (permalink / raw)
  To: Tiwei Bie
  Cc: Jason Wang, virtualization, linux-kernel, netdev, virtio-dev,
	wexu, jfreimann
In-Reply-To: <20180911053726.GA7472@debian>

On Tue, Sep 11, 2018 at 01:37:26PM +0800, Tiwei Bie wrote:
> On Mon, Sep 10, 2018 at 11:33:17AM +0800, Jason Wang wrote:
> > On 2018年09月10日 11:00, Tiwei Bie wrote:
> > > On Fri, Sep 07, 2018 at 09:00:49AM -0400, Michael S. Tsirkin wrote:
> > > > On Fri, Sep 07, 2018 at 09:22:25AM +0800, Tiwei Bie wrote:
> > > > > On Mon, Aug 27, 2018 at 05:00:40PM +0300, Michael S. Tsirkin wrote:
> > > > > > Are there still plans to test the performance with vost pmd?
> > > > > > vhost doesn't seem to show a performance gain ...
> > > > > > 
> > > > > I tried some performance tests with vhost PMD. In guest, the
> > > > > XDP program will return XDP_DROP directly. And in host, testpmd
> > > > > will do txonly fwd.
> > > > > 
> > > > > When burst size is 1 and packet size is 64 in testpmd and
> > > > > testpmd needs to iterate 5 Tx queues (but only the first two
> > > > > queues are enabled) to prepare and inject packets, I got ~12%
> > > > > performance boost (5.7Mpps -> 6.4Mpps). And if the vhost PMD
> > > > > is faster (e.g. just need to iterate the first two queues to
> > > > > prepare and inject packets), then I got similar performance
> > > > > for both rings (~9.9Mpps) (packed ring's performance can be
> > > > > lower, because it's more complicated in driver.)
> > > > > 
> > > > > I think packed ring makes vhost PMD faster, but it doesn't make
> > > > > the driver faster. In packed ring, the ring is simplified, and
> > > > > the handling of the ring in vhost (device) is also simplified,
> > > > > but things are not simplified in driver, e.g. although there is
> > > > > no desc table in the virtqueue anymore, driver still needs to
> > > > > maintain a private desc state table (which is still managed as
> > > > > a list in this patch set) to support the out-of-order desc
> > > > > processing in vhost (device).
> > > > > 
> > > > > I think this patch set is mainly to make the driver have a full
> > > > > functional support for the packed ring, which makes it possible
> > > > > to leverage the packed ring feature in vhost (device). But I'm
> > > > > not sure whether there is any other better idea, I'd like to
> > > > > hear your thoughts. Thanks!
> > > > Just this: Jens seems to report a nice gain with virtio and
> > > > vhost pmd across the board. Try to compare virtio and
> > > > virtio pmd to see what does pmd do better?
> > > The virtio PMD (drivers/net/virtio) in DPDK doesn't need to share
> > > the virtio ring operation code with other drivers and is highly
> > > optimized for network. E.g. in Rx, the Rx burst function won't
> > > chain descs. So the ID management for the Rx ring can be quite
> > > simple and straightforward, we just need to initialize these IDs
> > > when initializing the ring and don't need to change these IDs
> > > in data path anymore (the mergable Rx code in that patch set
> > > assumes the descs will be written back in order, which should be
> > > fixed. I.e., the ID in the desc should be used to index vq->descx[]).
> > > The Tx code in that patch set also assumes the descs will be
> > > written back by device in order, which should be fixed.
> > 
> > Yes it is. I think I've pointed it out in some early version of pmd patch.
> > So I suspect part (or all) of the boost may come from in order feature.
> > 
> > > 
> > > But in kernel virtio driver, the virtio_ring.c is very generic.
> > > The enqueue (virtqueue_add()) and dequeue (virtqueue_get_buf_ctx())
> > > functions need to support all the virtio devices and should be
> > > able to handle all the possible cases that may happen. So although
> > > the packed ring can be very efficient in some cases, currently
> > > the room to optimize the performance in kernel's virtio_ring.c
> > > isn't that much. If we want to take the fully advantage of the
> > > packed ring's efficiency, we need some further e.g. API changes
> > > in virtio_ring.c, which shouldn't be part of this patch set.
> > 
> > Could you please share more thoughts on this e.g how to improve the API?
> > Notice since the API is shared by both split ring and packed ring, it may
> > improve the performance of split ring as well. One can easily imagine a
> > batching API, but it does not have many real users now, the only case is the
> > XDP transmission which can accept an array of XDP frames.
> 
> I don't have detailed thoughts on this yet. But kernel's
> virtio_ring.c is quite generic compared with what we did
> in virtio PMD.

In what way? What are some things that aren't implemented there?

If what you say is true then we should take a careful look
and not supporting these generic things with packed layout.
Once we do support them it will be too late and we won't
be able to get performance back.



> > 
> > > So
> > > I still think this patch set is mainly to make the kernel virtio
> > > driver to have a full functional support of the packed ring, and
> > > we can't expect impressive performance boost with it.
> > 
> > We can only gain when virtio ring layout is the bottleneck. If there're
> > bottlenecks elsewhere, we probably won't see any increasing in the numbers.
> > Vhost-net is an example, and lots of optimizations have proved that virtio
> > ring is not the main bottleneck for the current codes. I suspect it also the
> > case of virtio driver. Did perf tell us any interesting things in virtio
> > driver?
> > 
> > Thanks
> > 
> > > 
> > > > 
> > > > > > On Wed, Jul 11, 2018 at 10:27:06AM +0800, Tiwei Bie wrote:
> > > > > > > Hello everyone,
> > > > > > > 
> > > > > > > This patch set implements packed ring support in virtio driver.
> > > > > > > 
> > > > > > > Some functional tests have been done with Jason's
> > > > > > > packed ring implementation in vhost:
> > > > > > > 
> > > > > > > https://lkml.org/lkml/2018/7/3/33
> > > > > > > 
> > > > > > > Both of ping and netperf worked as expected.
> > > > > > > 
> > > > > > > v1 -> v2:
> > > > > > > - Use READ_ONCE() to read event off_wrap and flags together (Jason);
> > > > > > > - Add comments related to ccw (Jason);
> > > > > > > 
> > > > > > > RFC (v6) -> v1:
> > > > > > > - Avoid extra virtio_wmb() in virtqueue_enable_cb_delayed_packed()
> > > > > > >    when event idx is off (Jason);
> > > > > > > - Fix bufs calculation in virtqueue_enable_cb_delayed_packed() (Jason);
> > > > > > > - Test the state of the desc at used_idx instead of last_used_idx
> > > > > > >    in virtqueue_enable_cb_delayed_packed() (Jason);
> > > > > > > - Save wrap counter (as part of queue state) in the return value
> > > > > > >    of virtqueue_enable_cb_prepare_packed();
> > > > > > > - Refine the packed ring definitions in uapi;
> > > > > > > - Rebase on the net-next tree;
> > > > > > > 
> > > > > > > RFC v5 -> RFC v6:
> > > > > > > - Avoid tracking addr/len/flags when DMA API isn't used (MST/Jason);
> > > > > > > - Define wrap counter as bool (Jason);
> > > > > > > - Use ALIGN() in vring_init_packed() (Jason);
> > > > > > > - Avoid using pointer to track `next` in detach_buf_packed() (Jason);
> > > > > > > - Add comments for barriers (Jason);
> > > > > > > - Don't enable RING_PACKED on ccw for now (noticed by Jason);
> > > > > > > - Refine the memory barrier in virtqueue_poll();
> > > > > > > - Add a missing memory barrier in virtqueue_enable_cb_delayed_packed();
> > > > > > > - Remove the hacks in virtqueue_enable_cb_prepare_packed();
> > > > > > > 
> > > > > > > RFC v4 -> RFC v5:
> > > > > > > - Save DMA addr, etc in desc state (Jason);
> > > > > > > - Track used wrap counter;
> > > > > > > 
> > > > > > > RFC v3 -> RFC v4:
> > > > > > > - Make ID allocation support out-of-order (Jason);
> > > > > > > - Various fixes for EVENT_IDX support;
> > > > > > > 
> > > > > > > RFC v2 -> RFC v3:
> > > > > > > - Split into small patches (Jason);
> > > > > > > - Add helper virtqueue_use_indirect() (Jason);
> > > > > > > - Just set id for the last descriptor of a list (Jason);
> > > > > > > - Calculate the prev in virtqueue_add_packed() (Jason);
> > > > > > > - Fix/improve desc suppression code (Jason/MST);
> > > > > > > - Refine the code layout for XXX_split/packed and wrappers (MST);
> > > > > > > - Fix the comments and API in uapi (MST);
> > > > > > > - Remove the BUG_ON() for indirect (Jason);
> > > > > > > - Some other refinements and bug fixes;
> > > > > > > 
> > > > > > > RFC v1 -> RFC v2:
> > > > > > > - Add indirect descriptor support - compile test only;
> > > > > > > - Add event suppression supprt - compile test only;
> > > > > > > - Move vring_packed_init() out of uapi (Jason, MST);
> > > > > > > - Merge two loops into one in virtqueue_add_packed() (Jason);
> > > > > > > - Split vring_unmap_one() for packed ring and split ring (Jason);
> > > > > > > - Avoid using '%' operator (Jason);
> > > > > > > - Rename free_head -> next_avail_idx (Jason);
> > > > > > > - Add comments for virtio_wmb() in virtqueue_add_packed() (Jason);
> > > > > > > - Some other refinements and bug fixes;
> > > > > > > 
> > > > > > > Thanks!
> > > > > > > 
> > > > > > > Tiwei Bie (5):
> > > > > > >    virtio: add packed ring definitions
> > > > > > >    virtio_ring: support creating packed ring
> > > > > > >    virtio_ring: add packed ring support
> > > > > > >    virtio_ring: add event idx support in packed ring
> > > > > > >    virtio_ring: enable packed ring
> > > > > > > 
> > > > > > >   drivers/s390/virtio/virtio_ccw.c   |   14 +
> > > > > > >   drivers/virtio/virtio_ring.c       | 1365 ++++++++++++++++++++++------
> > > > > > >   include/linux/virtio_ring.h        |    8 +-
> > > > > > >   include/uapi/linux/virtio_config.h |    3 +
> > > > > > >   include/uapi/linux/virtio_ring.h   |   43 +
> > > > > > >   5 files changed, 1157 insertions(+), 276 deletions(-)
> > > > > > > 
> > > > > > > -- 
> > > > > > > 2.18.0
> > > > > > ---------------------------------------------------------------------
> > > > > > To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
> > > > > > For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org
> > > > > > 
> > 

^ permalink raw reply

* Re: [PATCH net-next v2 3/5] virtio_ring: add packed ring support
From: Michael S. Tsirkin @ 2018-09-12 16:22 UTC (permalink / raw)
  To: Tiwei Bie
  Cc: jasowang, virtualization, linux-kernel, netdev, virtio-dev, wexu,
	jfreimann
In-Reply-To: <20180711022711.7090-4-tiwei.bie@intel.com>

On Wed, Jul 11, 2018 at 10:27:09AM +0800, Tiwei Bie wrote:
> This commit introduces the support (without EVENT_IDX) for
> packed ring.
> 
> Signed-off-by: Tiwei Bie <tiwei.bie@intel.com>
> ---
>  drivers/virtio/virtio_ring.c | 495 ++++++++++++++++++++++++++++++++++-
>  1 file changed, 487 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
> index c4f8abc7445a..f317b485ba54 100644
> --- a/drivers/virtio/virtio_ring.c
> +++ b/drivers/virtio/virtio_ring.c
> @@ -55,12 +55,21 @@
>  #define END_USE(vq)
>  #endif
>  
> +#define _VRING_DESC_F_AVAIL(b)	((__u16)(b) << 7)
> +#define _VRING_DESC_F_USED(b)	((__u16)(b) << 15)
> +
>  struct vring_desc_state {
>  	void *data;			/* Data for callback. */
>  	struct vring_desc *indir_desc;	/* Indirect descriptor, if any. */
>  };
>  
>  struct vring_desc_state_packed {
> +	void *data;			/* Data for callback. */
> +	struct vring_packed_desc *indir_desc; /* Indirect descriptor, if any. */
> +	int num;			/* Descriptor list length. */
> +	dma_addr_t addr;		/* Buffer DMA addr. */
> +	u32 len;			/* Buffer length. */
> +	u16 flags;			/* Descriptor flags. */
>  	int next;			/* The next desc state. */
>  };
>  

Idea: how about using data to chain these descriptors?
You can validate it's pointing within an array to distinguish
e.g. on detach.

-- 
MST

^ permalink raw reply

* Re: [PATCH net-next v3 0/7] net: aquantia: implement WOL and EEE support
From: Igor Russkikh @ 2018-09-12 11:42 UTC (permalink / raw)
  To: David Miller; +Cc: netdev
In-Reply-To: <20180911.234820.1747706577550366387.davem@davemloft.net>


>> Discussion outcome regarding driver version bumps was not finished
>> (here https://patchwork.ozlabs.org/patch/954905/)
>> David, could you suggest the best way to proceed on this?
> 
> Having a channel for your driver that is outside of upstream and Linux
> distribution packages creates lots of problems.
> 
> When a user reports a problem with an upstream kernel, that verion
> dictates which driver source was being used.  There is not confusion
> or ambiguity.
> 
> For a distribution kernel, the distributor hashes out which driver
> they published in their kernel package when evaluating a bug reported
> to them.
> 
> None of these two entities is ready to evaluate and handle properly
> your custom scheme.
> 
> So generally I frown against separate distribution schemes.  It is
> in the final analysis an inferior experience for the user because
> you basically narrow all of their support channels for problems
> down to you and you alone.  The whole idea is to make it work the
> opposite way.
> 
> So in the upstream tree, really, the driver version is pretty useless.

Thank you for the comment, David.

I'll pass over these concerns to my company management.

BR, Igor

^ permalink raw reply

* Re: [PATCH] net: dsa: mv88e6xxx: Add MV88E6352 DT compatible
From: Andrew Lunn @ 2018-09-12 12:47 UTC (permalink / raw)
  To: Marek Vasut; +Cc: netdev
In-Reply-To: <b5cdfc7d-6c0a-b7ad-e0dd-f4d196f1811c@denx.de>

On Wed, Sep 12, 2018 at 10:38:41AM +0200, Marek Vasut wrote:
> On 09/12/2018 02:46 AM, Andrew Lunn wrote:
> > On Wed, Sep 12, 2018 at 02:04:54AM +0200, Marek Vasut wrote:
> >> On 09/12/2018 01:32 AM, Andrew Lunn wrote:
> >>>> compatible = "marvell,mv88e6352", "marvell,mv88e6085";
> >>>
> >>> Just "marvell,mv88e6085";
> >>>
> >>> Please take a look at all the other DT files using the Marvell
> >>> chips. You will only ever find "marvell,mv88e6085" or
> >>> "marvell,mv88e6190", because everything is compatible to one of these
> >>> two.
> >>
> >> Well, until you find a difference between those chips which you cannot
> >> discern based solely on the ID register content. Then it's better to
> >> have a compatible to match on which matches the actual chip.
> > 
> > Hi Marek
> > 
> > We have been around this loop before. The problem with putting in a
> > more specific compatible is that nothing is validating it. So it is
> > going to be wrong, simple because people cut/paste, and don't
> > necessary change it. So when we do need to look at it, we cannot look
> > at it, because it is wrong.
> > 
> > I would only add a more specific compatible if and when we need it, it
> > is actually used, and we can verify it is correct.
> 
> You may need it in the future and it may not be possible to adjust the
> DT then. So having a specific compatible and fallback compatible is I
> think the way to go. You can always match on the fallback and if the
> time comes, you can match on the specific one because it's there. In
> addition to that, such a DT would fully correctly describe the hardware,
> unlike one with only a fallback compatible.
> 
> Regarding validation, I don't see other systems using this approach
> (specific + fallback compatible) having a problem with validation.
> What is the trick there ?

The trick is, they actually use the specific version, not the
generic. So it is validated, because if the specific version is wrong,
the device generally does not work.

We have no way to do that in this case. We load the driver based on
the generic version. It then goes and looks at the ID registers, and
from that decides what the device really is.

So far, Marvell have not messed this up. The ID registers have been
correct. I trust the ID registers much more than anything in device
tree. It is baked into silicon.

But you are talking of a theoretical case where the ID register is
wrong. For this to get passed us and out their in the wild, with DT
blown in ROM, that probably means there are a batch with the correct
ID and a batch with incorrect IDs. Those with incorrect IDs are
failing. You are arguing that we should then enable the use of the
specific compatible. And i expect that breaks more devices than it
fixes, because people have cut/pasted the wrong value.

       Andrew

^ permalink raw reply

* Re: [PATCH net-next v3 01/17] asm: simd context helper API
From: Jason A. Donenfeld @ 2018-09-12 18:10 UTC (permalink / raw)
  To: kevin
  Cc: LKML, Netdev, David Miller, Greg Kroah-Hartman, Andrew Lutomirski,
	Thomas Gleixner, Samuel Neves, linux-arch
In-Reply-To: <20180912061433.GA8484@ip-172-31-15-78>

On Wed, Sep 12, 2018 at 8:14 AM Kevin Easton <kevin@guarana.org> wrote:
> Given that it's always supposed to be used like that, mightn't it be
> better if simd_relax() took a pointer to the context, so the call is
> just
>
>     simd_relax(&simd_context);
>
> ?
>
> The inlining means that there won't actually be a pointer dereference in
> the emitted code.
>
> If simd_put() also took a pointer then it could set the context back to
> HAVE_NO_SIMD as well?

That's sort of a neat idea. I guess in this scheme, you'd envision:

   simd_context_t simd_context;

   simd_get(&simd_context);
   simd_relax(&simd_context);
   simd_put(&simd_context);

And this way, if simd_context ever becomes a heavier struct, it can be
modified in place rather than returned by value from the function. On
the other hand, it's a little bit more annoying to type and makes it
harder to do declaration and initialization on the same line.

^ permalink raw reply

* Re: [PATCH v2 05/17] compat_ioctl: move more drivers to generic_compat_ioctl_ptrarg
From: Greg Kroah-Hartman @ 2018-09-12 18:13 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
	linux-iio-u79uwXL29TY76Z2rM5mHXA,
	linux-pci-u79uwXL29TY76Z2rM5mHXA,
	linux-remoteproc-u79uwXL29TY76Z2rM5mHXA,
	linux-nvme-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	platform-driver-x86-u79uwXL29TY76Z2rM5mHXA,
	sparclinux-u79uwXL29TY76Z2rM5mHXA,
	devel-gWbeCf7V1WCQmaza687I9mD2FQJk+8+b,
	linux-scsi-u79uwXL29TY76Z2rM5mHXA,
	linux-nvdimm-hn68Rpc1hR1g9hUCZPvPmw,
	linux-rdma-u79uwXL29TY76Z2rM5mHXA,
	qat-linux-ral2JQCrhuEAvxtiuMwx3w,
	amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
	linux-input-u79uwXL29TY76Z2rM5mHXA,
	linux-media-u79uwXL29TY76Z2rM5mHXA,
	linaro-mm-sig-cunTk1MwBs8s++Sfvej+rw,
	dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
	ceph-devel-u79uwXL29TY76Z2rM5mHXA, netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-usb-u79uwXL29TY76Z2rM5mHXA,
	linux-wireless-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-crypto-u79uwXL29TY76Z2rM5mHXA,
	linux-fsdevel-u79uwXL29TY76Z2rM5mHXA,
	linuxppc-dev-uLR06cmDAlY/bJ5BZ2RsiQ, David S. Miller,
	linux-btrfs-u79uwXL29TY76Z2rM5mHXA,
	viro-RmSDqhL/yNMiFSDQTTA3OLVCufUGDwFn
In-Reply-To: <20180912151134.436719-1-arnd-r2nGTMty4D4@public.gmane.org>

On Wed, Sep 12, 2018 at 05:08:52PM +0200, Arnd Bergmann wrote:
> The .ioctl and .compat_ioctl file operations have the same prototype so
> they can both point to the same function, which works great almost all
> the time when all the commands are compatible.
> 
> One exception is the s390 architecture, where a compat pointer is only
> 31 bit wide, and converting it into a 64-bit pointer requires calling
> compat_ptr(). Most drivers here will ever run in s390, but since we now
> have a generic helper for it, it's easy enough to use it consistently.
> 
> I double-checked all these drivers to ensure that all ioctl arguments
> are used as pointers or are ignored, but are not interpreted as integer
> values.
> 
> Signed-off-by: Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>

Acked-by: Greg Kroah-Hartman <gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org>

^ permalink raw reply

* Re: [PATCH v2 02/17] compat_ioctl: move drivers to generic_compat_ioctl_ptrarg
From: Greg Kroah-Hartman @ 2018-09-12 18:13 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: kvm, Alexander Shishkin, Jarkko Sakkinen, virtualization,
	Benjamin Tissoires, linux-mtd, Peter Huewe, linux1394-devel,
	devel, Jason Gunthorpe, Marek Vasut, linux-input, Tomas Winkler,
	Jiri Kosina, viro, Artem Bityutskiy, netdev, linux-usb,
	linux-kernel, Sudip Mukherjee, Stefan Richter, linux-fsdevel,
	linux-integrity, David S. Miller
In-Reply-To: <20180912150142.157913-2-arnd@arndb.de>

On Wed, Sep 12, 2018 at 05:01:03PM +0200, Arnd Bergmann wrote:
> Each of these drivers has a copy of the same trivial helper function to
> convert the pointer argument and then call the native ioctl handler.
> 
> We now have a generic implementation of that, so use it.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

^ permalink raw reply

* Re: [PATCH] net: dsa: mv88e6xxx: Add MV88E6352 DT compatible
From: Marek Vasut @ 2018-09-12 13:10 UTC (permalink / raw)
  To: Andrew Lunn; +Cc: netdev
In-Reply-To: <20180912124738.GA24595@lunn.ch>

On 09/12/2018 02:47 PM, Andrew Lunn wrote:
> On Wed, Sep 12, 2018 at 10:38:41AM +0200, Marek Vasut wrote:
>> On 09/12/2018 02:46 AM, Andrew Lunn wrote:
>>> On Wed, Sep 12, 2018 at 02:04:54AM +0200, Marek Vasut wrote:
>>>> On 09/12/2018 01:32 AM, Andrew Lunn wrote:
>>>>>> compatible = "marvell,mv88e6352", "marvell,mv88e6085";
>>>>>
>>>>> Just "marvell,mv88e6085";
>>>>>
>>>>> Please take a look at all the other DT files using the Marvell
>>>>> chips. You will only ever find "marvell,mv88e6085" or
>>>>> "marvell,mv88e6190", because everything is compatible to one of these
>>>>> two.
>>>>
>>>> Well, until you find a difference between those chips which you cannot
>>>> discern based solely on the ID register content. Then it's better to
>>>> have a compatible to match on which matches the actual chip.
>>>
>>> Hi Marek
>>>
>>> We have been around this loop before. The problem with putting in a
>>> more specific compatible is that nothing is validating it. So it is
>>> going to be wrong, simple because people cut/paste, and don't
>>> necessary change it. So when we do need to look at it, we cannot look
>>> at it, because it is wrong.
>>>
>>> I would only add a more specific compatible if and when we need it, it
>>> is actually used, and we can verify it is correct.
>>
>> You may need it in the future and it may not be possible to adjust the
>> DT then. So having a specific compatible and fallback compatible is I
>> think the way to go. You can always match on the fallback and if the
>> time comes, you can match on the specific one because it's there. In
>> addition to that, such a DT would fully correctly describe the hardware,
>> unlike one with only a fallback compatible.
>>
>> Regarding validation, I don't see other systems using this approach
>> (specific + fallback compatible) having a problem with validation.
>> What is the trick there ?
> 
> The trick is, they actually use the specific version, not the
> generic. So it is validated, because if the specific version is wrong,
> the device generally does not work.

Many of them use the fallback version to avoid polluting the tables in
the kernel with redundant compat strings. The specific string is only
added into the driver if there is a HW difference which needs to be
dealt with.

> We have no way to do that in this case. We load the driver based on
> the generic version. It then goes and looks at the ID registers, and
> from that decides what the device really is.

And this works fine, until marvell screws it up in one of those chips.

> So far, Marvell have not messed this up. The ID registers have been
> correct. I trust the ID registers much more than anything in device
> tree. It is baked into silicon.

But the DT should correctly describe the hardware, if it doesn't, it's
just broken.

> But you are talking of a theoretical case where the ID register is
> wrong. For this to get passed us and out their in the wild, with DT
> blown in ROM, that probably means there are a batch with the correct
> ID and a batch with incorrect IDs. Those with incorrect IDs are
> failing. You are arguing that we should then enable the use of the
> specific compatible. And i expect that breaks more devices than it
> fixes, because people have cut/pasted the wrong value.

I am arguing you should have both specific compatible which describes
which exact chip is in that device AND a fallback compatible which the
driver matches on.

-- 
Best regards,
Marek Vasut

^ permalink raw reply

* Re: [RFC v2 1/2] netlink: add NLA_REJECT policy type
From: David Miller @ 2018-09-12 18:15 UTC (permalink / raw)
  To: johannes-cdvu00un1VgdHxzADdlk8Q
  Cc: linux-wireless-u79uwXL29TY76Z2rM5mHXA,
	netdev-u79uwXL29TY76Z2rM5mHXA, mkubecek-AlSwsSmVLrQ,
	johannes.berg-ral2JQCrhuEAvxtiuMwx3w
In-Reply-To: <20180912083610.20857-1-johannes-cdvu00un1VgdHxzADdlk8Q@public.gmane.org>

From: Johannes Berg <johannes-cdvu00un1VgdHxzADdlk8Q@public.gmane.org>
Date: Wed, 12 Sep 2018 10:36:09 +0200

> From: Johannes Berg <johannes.berg-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> 
> In some situations some netlink attributes may be used for output
> only (kernel->userspace) or may be reserved for future use. It's
> then helpful to be able to prevent userspace from using them in
> messages sent to the kernel, since they'd otherwise be ignored and
> any future will become impossible if this happens.
> 
> Add NLA_REJECT to the policy which does nothing but reject (with
> EINVAL) validation of any messages containing this attribute.
> Allow for returning a specific extended ACK error message in the
> validation_data pointer.
> 
> While at it fix the indentation of NLA_BITFIELD32 and describe the
> validation_data pointer for it.
> 
> The specific case I have in mind now is a shared nested attribute
> containing request/response data, and it would be pointless and
> potentially confusing to have userspace include response data in
> the messages that actually contain a request.
> 
> Signed-off-by: Johannes Berg <johannes.berg-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>

This looks great, no objections to this idea or the facility.

It does, however, remind me about about the classic problem of how bad
we are at feature support detection because unrecognized attributes are
ignored.

I do really hope we can fully solve that problem some day.

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox