Re: [PATCH 1/1] x86/hyper-v: Fix 'set but not used' warnings
On Sat, Dec 22, 2018 at 12:06:58AM +, Michael Kelley wrote: In these two cases, a value returned by rdmsr() or rdmsrl() is ignored. Indicate that ignoring the value is intentional, so that with the W=1 compilation option no warning is generated. Signed-off-by: Michael Kelley Queued up, thank you. -- Thanks, Sasha ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH] staging: android: ion: add buffer flag update ioctl
Hi laura:
>-Original Message-
>From: Laura Abbott [mailto:labb...@redhat.com]
>Sent: Friday, December 21, 2018 4:50 AM
>To: Zengtao (B) ; sumit.sem...@linaro.org
>Cc: Greg Kroah-Hartman ; Arve Hjønnevåg
>; Todd Kjos ; Martijn Coenen
>; Joel Fernandes ;
>de...@driverdev.osuosl.org; dri-de...@lists.freedesktop.org;
>linaro-mm-...@lists.linaro.org; linux-ker...@vger.kernel.org
>Subject: Re: [PATCH] staging: android: ion: add buffer flag update ioctl
>
>On 12/19/18 5:39 PM, Zengtao (B) wrote:
>> Hi laura:
>>
>>> -Original Message-
>>> From: Laura Abbott [mailto:labb...@redhat.com]
>>> Sent: Thursday, December 20, 2018 2:10 AM
>>> To: Zengtao (B) ;
>sumit.sem...@linaro.org
>>> Cc: Greg Kroah-Hartman ; Arve
>Hjønnevåg
>>> ; Todd Kjos ; Martijn
>Coenen
>>> ; Joel Fernandes ;
>>> de...@driverdev.osuosl.org; dri-de...@lists.freedesktop.org;
>>> linaro-mm-...@lists.linaro.org; linux-ker...@vger.kernel.org
>>> Subject: Re: [PATCH] staging: android: ion: add buffer flag update
>>> ioctl
>>>
>>> On 12/19/18 9:19 AM, Zeng Tao wrote:
In some usecases, the buffer cached attribute is not determined at
allocation time, it's determined just before the real cpu mapping.
And from the memory view of point, a buffer should not have the
>>> cached
attribute util is really mapped by the cpu. So in this patch, we
introduced the new ioctl command to target the requirement.
>>>
>>> This is racy and error prone. Can you explain more what problem you
>>> are trying to solve?
>>
>> My use case is like this:
>> 1. There are two process A and B, A takes case of ion buffer allocation,
>and
>> pass the buffer fd to B, then B maps and uses it.
>> 2. Process B need to map the buffer with different cached attribute
>> for different use case, for example, if the buffer is used for pure
>> software algorithm, then we need to map it as cached, otherwise
>> non-cached, and B needs to deal with both cases.
>> And unfortunately the mmap syscall takes no cached flags and we can't
>> decide the cache attribute when we are doing the mmap, so I introduce
>> new the ioctl even though I think the solution is not as good.
>>
>>
>
>Thanks for the explanation, this was about the use case I expected.
>I'm pretty sure I had this exact problem once upon a time and we didn't
>come up with a solution. I'd still like to get rid of uncached buffers in
>general and just use cached buffers (see
>http://driverdev.linuxdriverproject.org/pipermail/driverdev-devel/2018-N
>ovember/128842.html)
>What's your usecase for uncached buffers?
Some buffers are only used by HW, in this case, we tend to use uncached buffers.
But sometimes the SW need to read few buffer contents for debug purpose and
no synchronization is needed.
>
>>>
Signed-off-by: Zeng Tao
---
drivers/staging/android/ion/ion-ioctl.c | 4
drivers/staging/android/ion/ion.c | 17
>+
drivers/staging/android/ion/ion.h | 1 +
drivers/staging/android/uapi/ion.h | 22
>>> ++
4 files changed, 44 insertions(+)
diff --git a/drivers/staging/android/ion/ion-ioctl.c
b/drivers/staging/android/ion/ion-ioctl.c
index a8d3cc4..60bb702 100644
--- a/drivers/staging/android/ion/ion-ioctl.c
+++ b/drivers/staging/android/ion/ion-ioctl.c
@@ -12,6 +12,7 @@
union ion_ioctl_arg {
struct ion_allocation_data allocation;
+ struct ion_buffer_flag_data update;
struct ion_heap_query query;
};
@@ -83,6 +84,9 @@ long ion_ioctl(struct file *filp, unsigned int
cmd, unsigned long arg)
break;
}
+ case ION_IOC_BUFFER_UPDATE:
+ ret = ion_buffer_update(data.update.fd, data.update.flags);
+ break;
case ION_IOC_HEAP_QUERY:
ret = ion_query_heaps();
break;
diff --git a/drivers/staging/android/ion/ion.c
b/drivers/staging/android/ion/ion.c
index 9907332..f1404dc 100644
--- a/drivers/staging/android/ion/ion.c
+++ b/drivers/staging/android/ion/ion.c
@@ -436,6 +436,23 @@ int ion_alloc(size_t len, unsigned int
>>> heap_id_mask, unsigned int flags)
return fd;
}
+int ion_buffer_update(unsigned int fd, unsigned int flags) {
+ struct dma_buf *dmabuf;
+ struct ion_buffer *buffer;
+
+ dmabuf = dma_buf_get(fd);
+
+ if (!dmabuf)
+ return -EINVAL;
+
+ buffer = dmabuf->priv;
+ buffer->flags = flags;
+ dma_buf_put(dmabuf);
+
+ return 0;
+}
+
int ion_query_heaps(struct ion_heap_query *query)
{
struct ion_device *dev = internal_dev; diff --git
a/drivers/staging/android/ion/ion.h
b/drivers/staging/android/ion/ion.h
index c006fc1..99bf9ab 100644
---
[PATCH] rtl8723bs/ioctl_linux: Add a security check to copy_from_user()
Currently, the return value of copy_from_user is not checked.
extra is assigned to u32wps_start irrespective of these failures.
Signed-off-by: Aditya Pakki
---
drivers/staging/rtl8723bs/os_dep/ioctl_linux.c | 9 +++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/rtl8723bs/os_dep/ioctl_linux.c
b/drivers/staging/rtl8723bs/os_dep/ioctl_linux.c
index b8631baf128d..9992caa8c839 100644
--- a/drivers/staging/rtl8723bs/os_dep/ioctl_linux.c
+++ b/drivers/staging/rtl8723bs/os_dep/ioctl_linux.c
@@ -2577,14 +2577,19 @@ static int rtw_wps_start(struct net_device *dev,
struct adapter *padapter = (struct adapter *)rtw_netdev_priv(dev);
struct iw_point *pdata = >data;
u32 u32wps_start = 0;
-unsigned int uintRet = 0;
if ((true == padapter->bDriverStopped) ||(true
==padapter->bSurpriseRemoved) || (NULL == pdata)) {
ret = -EINVAL;
goto exit;
}
- uintRet = copy_from_user((void*)_start, pdata->pointer, 4);
+ ret = copy_from_user((void *)_start, pdata->pointer, 4);
+
+ if (ret) {
+ ret = -EINVAL;
+ goto exit;
+ }
+
if (u32wps_start == 0)
u32wps_start = *extra;
--
2.17.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] dt-bindings: net: dsa: ksz9477: fix indentation for switch spi bindings
Le 12/21/18 à 11:39 PM, Sergio Paracuellos a écrit : > Switch bindings for spi managed mode are using spaces instead of tabs. > Fix them to get a file with a proper kernel indentation style. > > Signed-off-by: Sergio Paracuellos Reviewed-by: Florian Fainelli -- Florian ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v1 1/2] binderfs: implement "max" mount option
Since binderfs can be mounted by userns root in non-initial user namespaces
some precautions are in order. First, a way to set a maximum on the number
of binder devices that can be allocated per binderfs instance and second, a
way to reserve a reasonable chunk of binderfs devices for the initial ipc
namespace.
A first approach as seen in [1] used sysctls similiar to devpts but was
shown to be flawed (cf. [2] and [3]) since some aspects were unneeded. This
is an alternative approach which avoids sysctls completely and instead
switches to a single mount option.
Starting with this commit binderfs instances can be mounted with a limit on
the number of binder devices that can be allocated. The max= mount
option serves as a per-instance limit. If max= is set then only
number of binder devices can be allocated in this binderfs
instance.
This allows to safely bind-mount binderfs instances into unprivileged user
namespaces since userns root in a non-initial user namespace cannot change
the mount option as long as it does not own the mount namespace the
binderfs mount was created in and hence cannot drain the host of minor
device numbers
[1]: https://lore.kernel.org/lkml/20181221133909.18794-1-christ...@brauner.io/
[2]; https://lore.kernel.org/lkml/20181221163316.ga8...@kroah.com/
[3]:
https://lore.kernel.org/lkml/cahrssex+gdvw4fkkk8oznair9g5icjlyodo8hykv3o0o1jt...@mail.gmail.com/
[4]: https://lore.kernel.org/lkml/20181221192044.5yvfnuri7gdop...@brauner.io/
Cc: Todd Kjos
Cc: Greg Kroah-Hartman
Signed-off-by: Christian Brauner
---
v1:
- split reserving devices for the binderfs instance in the initial ipc
namespace into a separate patch (cf. [1])
- s/
- use simple int instead of atomic_t for counting devices since we're
incrementing and decrementing under a mutex anyway (cf. [1])
- correctly use match_int() (cf. [2])
v0:
- patch introduced
[1]: https://lore.kernel.org/lkml/20181223112944.gc27...@kroah.com/
[2]: https://lore.kernel.org/lkml/20181223135755.sqnv5ave62jtj...@brauner.io/
---
drivers/android/binderfs.c | 101 +++--
1 file changed, 96 insertions(+), 5 deletions(-)
diff --git a/drivers/android/binderfs.c b/drivers/android/binderfs.c
index 7496b10532aa..873adda064ac 100644
--- a/drivers/android/binderfs.c
+++ b/drivers/android/binderfs.c
@@ -20,6 +20,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -46,6 +47,24 @@ static dev_t binderfs_dev;
static DEFINE_MUTEX(binderfs_minors_mutex);
static DEFINE_IDA(binderfs_minors);
+/**
+ * binderfs_mount_opts - mount options for binderfs
+ * @max: maximum number of allocatable binderfs binder devices
+ */
+struct binderfs_mount_opts {
+ int max;
+};
+
+enum {
+ Opt_max,
+ Opt_err
+};
+
+static const match_table_t tokens = {
+ { Opt_max, "max=%d" },
+ { Opt_err, NULL }
+};
+
/**
* binderfs_info - information about a binderfs mount
* @ipc_ns: The ipc namespace the binderfs mount belongs to.
@@ -55,13 +74,16 @@ static DEFINE_IDA(binderfs_minors);
* created.
* @root_gid: gid that needs to be used when a new binder device is
* created.
+ * @mount_opts: The mount options in use.
+ * @device_count: The current number of allocated binder devices.
*/
struct binderfs_info {
struct ipc_namespace *ipc_ns;
struct dentry *control_dentry;
kuid_t root_uid;
kgid_t root_gid;
-
+ struct binderfs_mount_opts mount_opts;
+ int device_count;
};
static inline struct binderfs_info *BINDERFS_I(const struct inode *inode)
@@ -110,10 +132,16 @@ static int binderfs_binder_device_create(struct inode
*ref_inode,
/* Reserve new minor number for the new device. */
mutex_lock(_minors_mutex);
- minor = ida_alloc_max(_minors, BINDERFS_MAX_MINOR, GFP_KERNEL);
+ if (++info->device_count <= info->mount_opts.max)
+ minor = ida_alloc_max(_minors, BINDERFS_MAX_MINOR,
+ GFP_KERNEL);
+ else
+ minor = -ENOSPC;
mutex_unlock(_minors_mutex);
- if (minor < 0)
+ if (minor < 0) {
+ --info->device_count;
return minor;
+ }
ret = -ENOMEM;
device = kzalloc(sizeof(*device), GFP_KERNEL);
@@ -187,6 +215,7 @@ static int binderfs_binder_device_create(struct inode
*ref_inode,
kfree(name);
kfree(device);
mutex_lock(_minors_mutex);
+ --info->device_count;
ida_free(_minors, minor);
mutex_unlock(_minors_mutex);
iput(inode);
@@ -232,6 +261,7 @@ static long binder_ctl_ioctl(struct file *file, unsigned
int cmd,
static void binderfs_evict_inode(struct inode *inode)
{
struct binder_device *device = inode->i_private;
+ struct binderfs_info *info = BINDERFS_I(inode);
clear_inode(inode);
@@ -239,6 +269,7 @@ static void binderfs_evict_inode(struct inode
[PATCH v1 2/2] binderfs: reserve devices for initial mount
The binderfs instance in the initial ipc namespace will always have a reserve of 4 binder devices unless explicitly capped by specifying a lower value via the "max" mount option. This ensures when binder devices are removed (on accident or on purpose) they can always be recreated without risking that all minor numbers have already been used up. Cc: Todd Kjos Cc: Greg Kroah-Hartman Signed-off-by: Christian Brauner --- v1: - patch introduced v0: - patch not present --- drivers/android/binderfs.c | 7 ++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/drivers/android/binderfs.c b/drivers/android/binderfs.c index 873adda064ac..aa635c7ea727 100644 --- a/drivers/android/binderfs.c +++ b/drivers/android/binderfs.c @@ -40,6 +40,8 @@ #define INODE_OFFSET 3 #define INTSTRLEN 21 #define BINDERFS_MAX_MINOR (1U << MINORBITS) +/* Ensure that the initial ipc namespace always has a devices available. */ +#define BINDERFS_MAX_MINOR_CAPPED (BINDERFS_MAX_MINOR - 4) static struct vfsmount *binderfs_mnt; @@ -129,11 +131,14 @@ static int binderfs_binder_device_create(struct inode *ref_inode, struct inode *inode = NULL; struct super_block *sb = ref_inode->i_sb; struct binderfs_info *info = sb->s_fs_info; + bool use_reserve = (info->ipc_ns == _ipc_ns); /* Reserve new minor number for the new device. */ mutex_lock(_minors_mutex); if (++info->device_count <= info->mount_opts.max) - minor = ida_alloc_max(_minors, BINDERFS_MAX_MINOR, + minor = ida_alloc_max(_minors, + use_reserve ? BINDERFS_MAX_MINOR : + BINDERFS_MAX_MINOR_CAPPED, GFP_KERNEL); else minor = -ENOSPC; -- 2.19.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] Staging: iio: adt7316: Add regmap support
Both i2c and spi drivers have functions for reading and writing
to/from registers. Remove this redundant and common code by using
regmap API.
Also remove multi_read and multi_write functions from i2c
and spi driver as they are not being used anywhere.
Signed-off-by: Shreeya Patel
---
drivers/staging/iio/addac/adt7316-i2c.c | 101 ++--
drivers/staging/iio/addac/adt7316-spi.c | 94 +++
drivers/staging/iio/addac/adt7316.c | 147
drivers/staging/iio/addac/adt7316.h | 15 +--
4 files changed, 103 insertions(+), 254 deletions(-)
diff --git a/drivers/staging/iio/addac/adt7316-i2c.c
b/drivers/staging/iio/addac/adt7316-i2c.c
index ac91163656b5..435b65845174 100644
--- a/drivers/staging/iio/addac/adt7316-i2c.c
+++ b/drivers/staging/iio/addac/adt7316-i2c.c
@@ -12,105 +12,28 @@
#include
#include
#include
+#include
#include "adt7316.h"
-/*
- * adt7316 register access by I2C
- */
-static int adt7316_i2c_read(void *client, u8 reg, u8 *data)
-{
- struct i2c_client *cl = client;
- int ret;
-
- ret = i2c_smbus_write_byte(cl, reg);
- if (ret < 0) {
- dev_err(>dev, "I2C fail to select reg\n");
- return ret;
- }
-
- ret = i2c_smbus_read_byte(client);
-
- if (!ret)
- return -EIO;
-
- if (ret < 0) {
- dev_err(>dev, "I2C read error\n");
- return ret;
- }
-
- *data = ret;
-
- return 0;
-}
-
-static int adt7316_i2c_write(void *client, u8 reg, u8 data)
-{
- struct i2c_client *cl = client;
- int ret = 0;
-
- ret = i2c_smbus_write_byte_data(cl, reg, data);
- if (ret < 0)
- dev_err(>dev, "I2C write error\n");
-
- return ret;
-}
-
-static int adt7316_i2c_multi_read(void *client, u8 reg, u8 count, u8 *data)
-{
- struct i2c_client *cl = client;
- int i, ret = 0;
-
- if (count > ADT7316_REG_MAX_ADDR)
- count = ADT7316_REG_MAX_ADDR;
-
- for (i = 0; i < count; i++) {
- ret = adt7316_i2c_read(cl, reg, [i]);
- if (ret < 0) {
- dev_err(>dev, "I2C multi read error\n");
- return ret;
- }
- }
-
- return 0;
-}
-
-static int adt7316_i2c_multi_write(void *client, u8 reg, u8 count, u8 *data)
-{
- struct i2c_client *cl = client;
- int i, ret = 0;
-
- if (count > ADT7316_REG_MAX_ADDR)
- count = ADT7316_REG_MAX_ADDR;
-
- for (i = 0; i < count; i++) {
- ret = adt7316_i2c_write(cl, reg, data[i]);
- if (ret < 0) {
- dev_err(>dev, "I2C multi write error\n");
- return ret;
- }
- }
-
- return 0;
-}
-
/*
* device probe and remove
*/
-
static int adt7316_i2c_probe(struct i2c_client *client,
const struct i2c_device_id *id)
{
- struct adt7316_bus bus = {
- .client = client,
- .irq = client->irq,
- .read = adt7316_i2c_read,
- .write = adt7316_i2c_write,
- .multi_read = adt7316_i2c_multi_read,
- .multi_write = adt7316_i2c_multi_write,
- };
+ struct regmap *regmap;
+
+ regmap = devm_regmap_init_i2c(client, _regmap_config);
+
+ if (IS_ERR(regmap)) {
+ dev_err(>dev, "Error initializing i2c regmap: %ld\n",
+ PTR_ERR(regmap));
+ return PTR_ERR(regmap);
+ }
- return adt7316_probe(>dev, , id->name);
+ return adt7316_probe(>dev, regmap, id ? id->name : NULL,
+client->irq);
}
static const struct i2c_device_id adt7316_i2c_id[] = {
diff --git a/drivers/staging/iio/addac/adt7316-spi.c
b/drivers/staging/iio/addac/adt7316-spi.c
index e75827e326a6..9e3decb5cb77 100644
--- a/drivers/staging/iio/addac/adt7316-spi.c
+++ b/drivers/staging/iio/addac/adt7316-spi.c
@@ -11,79 +11,12 @@
#include
#include
#include
+#include
#include
#include "adt7316.h"
#define ADT7316_SPI_MAX_FREQ_HZ500
-#define ADT7316_SPI_CMD_READ 0x91
-#define ADT7316_SPI_CMD_WRITE 0x90
-
-/*
- * adt7316 register access by SPI
- */
-
-static int adt7316_spi_multi_read(void *client, u8 reg, u8 count, u8 *data)
-{
- struct spi_device *spi_dev = client;
- u8 cmd[2];
- int ret = 0;
-
- if (count > ADT7316_REG_MAX_ADDR)
- count = ADT7316_REG_MAX_ADDR;
-
- cmd[0] = ADT7316_SPI_CMD_WRITE;
- cmd[1] = reg;
-
- ret = spi_write(spi_dev, cmd, 2);
- if (ret < 0) {
- dev_err(_dev->dev, "SPI fail to select reg\n");
- return ret;
- }
-
- cmd[0] = ADT7316_SPI_CMD_READ;
-
- ret = spi_write_then_read(spi_dev, cmd, 1, data, count);
- if (ret < 0) {
- dev_err(_dev->dev, "SPI read data error\n");
Re: [PATCH] binderfs: implement "max" mount option
On Sat, Dec 22, 2018 at 10:18:06PM +0100, Christian Brauner wrote:
> Since binderfs can be mounted by userns root in non-initial user namespaces
> some precautions are in order. First, a way to set a maximum on the number
> of binder devices that can be allocated per binderfs instance and second, a
> way to reserve a reasonable chunk of binderfs devices for the initial ipc
> namespace.
> A first approach as seen in [1] used sysctls similiar to devpts but was
> shown to be flawed (cf. [2] and [3]) since some aspects were unneeded. This
> is an alternative approach which avoids sysctls completely and instead
> switches to a single mount option.
>
> Starting with this commit binderfs instances can be mounted with a limit on
> the number of binder devices that can be allocated. The max= mount
> option serves as a per-instance limit. If max= is set then only
> number of binder devices can be allocated in this binderfs
> instance.
> Additionally, the binderfs instance in the initial ipc namespace will
> always have a reserve of at least 1024 binder devices unless explicitly
> capped via max=.
>
> This allows to safely bind-mount binderfs instances into unprivileged user
> namespaces since userns root in a non-initial user namespace cannot change
> the mount option as long as it does not own the mount namespace the
> binderfs mount was created in and hence cannot drain the host of minor
> device numbers
>
> [1]: https://lore.kernel.org/lkml/20181221133909.18794-1-christ...@brauner.io/
> [2]; https://lore.kernel.org/lkml/20181221163316.ga8...@kroah.com/
> [3]:
> https://lore.kernel.org/lkml/cahrssex+gdvw4fkkk8oznair9g5icjlyodo8hykv3o0o1jt...@mail.gmail.com/
> [4]: https://lore.kernel.org/lkml/20181221192044.5yvfnuri7gdop...@brauner.io/
>
> Cc: Todd Kjos
> Cc: Greg Kroah-Hartman
> Signed-off-by: Christian Brauner
> ---
> drivers/android/binderfs.c | 109 +++--
> 1 file changed, 104 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/android/binderfs.c b/drivers/android/binderfs.c
> index 7496b10532aa..93aee00994d4 100644
> --- a/drivers/android/binderfs.c
> +++ b/drivers/android/binderfs.c
> @@ -20,6 +20,7 @@
> #include
> #include
> #include
> +#include
> #include
> #include
> #include
> @@ -39,6 +40,11 @@
> #define INODE_OFFSET 3
> #define INTSTRLEN 21
> #define BINDERFS_MAX_MINOR (1U << MINORBITS)
> +/*
> + * Ensure that the initial ipc namespace always has a good chunk of devices
> + * available.
> + */
> +#define BINDERFS_MAX_MINOR_CAPPED (BINDERFS_MAX_MINOR - 1024)
>
> static struct vfsmount *binderfs_mnt;
>
> @@ -46,6 +52,24 @@ static dev_t binderfs_dev;
> static DEFINE_MUTEX(binderfs_minors_mutex);
> static DEFINE_IDA(binderfs_minors);
>
> +/**
> + * binderfs_mount_opts - mount options for binderfs
> + * @max: maximum number of allocatable binderfs binder devices
> + */
> +struct binderfs_mount_opts {
> + int max;
> +};
> +
> +enum {
> + Opt_max,
> + Opt_err
> +};
> +
> +static const match_table_t tokens = {
> + { Opt_max, "max=%d" },
> + { Opt_err, NULL }
> +};
> +
> /**
> * binderfs_info - information about a binderfs mount
> * @ipc_ns: The ipc namespace the binderfs mount belongs to.
> @@ -55,13 +79,16 @@ static DEFINE_IDA(binderfs_minors);
> * created.
> * @root_gid: gid that needs to be used when a new binder device is
> * created.
> + * @mount_opts: The mount options in use.
> + * @device_count: The current number of allocated binder devices.
> */
> struct binderfs_info {
> struct ipc_namespace *ipc_ns;
> struct dentry *control_dentry;
> kuid_t root_uid;
> kgid_t root_gid;
> -
> + struct binderfs_mount_opts mount_opts;
> + atomic_t device_count;
> };
>
> static inline struct binderfs_info *BINDERFS_I(const struct inode *inode)
> @@ -107,13 +134,22 @@ static int binderfs_binder_device_create(struct inode
> *ref_inode,
> struct inode *inode = NULL;
> struct super_block *sb = ref_inode->i_sb;
> struct binderfs_info *info = sb->s_fs_info;
> + bool use_reserve = (info->ipc_ns == _ipc_ns);
>
> /* Reserve new minor number for the new device. */
> mutex_lock(_minors_mutex);
> - minor = ida_alloc_max(_minors, BINDERFS_MAX_MINOR, GFP_KERNEL);
> + if (atomic_inc_return(>device_count) < info->mount_opts.max)
> + minor = ida_alloc_max(_minors,
> + use_reserve ? BINDERFS_MAX_MINOR :
> + BINDERFS_MAX_MINOR_CAPPED,
> + GFP_KERNEL);
> + else
> + minor = -ENOSPC;
> mutex_unlock(_minors_mutex);
> - if (minor < 0)
> + if (minor < 0) {
> + atomic_dec(>device_count);
> return minor;
> + }
>
> ret = -ENOMEM;
> device = kzalloc(sizeof(*device), GFP_KERNEL);
> @@ -187,6 +223,7 @@ static
Re: [PATCH] binderfs: implement "max" mount option
On Sun, Dec 23, 2018 at 12:29:44PM +0100, Greg KH wrote:
> On Sat, Dec 22, 2018 at 10:18:06PM +0100, Christian Brauner wrote:
> > Since binderfs can be mounted by userns root in non-initial user namespaces
> > some precautions are in order. First, a way to set a maximum on the number
> > of binder devices that can be allocated per binderfs instance and second, a
> > way to reserve a reasonable chunk of binderfs devices for the initial ipc
> > namespace.
> > A first approach as seen in [1] used sysctls similiar to devpts but was
> > shown to be flawed (cf. [2] and [3]) since some aspects were unneeded. This
> > is an alternative approach which avoids sysctls completely and instead
> > switches to a single mount option.
> >
> > Starting with this commit binderfs instances can be mounted with a limit on
> > the number of binder devices that can be allocated. The max= mount
> > option serves as a per-instance limit. If max= is set then only
> > number of binder devices can be allocated in this binderfs
> > instance.
>
> Ok, this is fine, but why such a big default? You only need 4 to run a
> modern android system, and anyone using binder outside of android is
> really too crazy to ever be using it in a container :)
>
> > Additionally, the binderfs instance in the initial ipc namespace will
> > always have a reserve of at least 1024 binder devices unless explicitly
> > capped via max=.
>
> Again, why so many? And why wouldn't that initial ipc namespace already
> have their device nodes created _before_ anything else is mounted?
Right, my issue is with re-creating devices, like if binderfs gets
unmounted or if devices get removed via rm. But we can lower the number
to 4 (see below).
>
> Some comments on the patch below:
Thanks!
>
> > +/*
> > + * Ensure that the initial ipc namespace always has a good chunk of devices
> > + * available.
> > + */
> > +#define BINDERFS_MAX_MINOR_CAPPED (BINDERFS_MAX_MINOR - 1024)
>
> Again that seems crazy big, how about splitting this into two different
> patches, one for the max= stuff, and one for this "reserve some minors"
> thing, so we can review them separately.
Yes, let's do that. I will also lower this to 4 reserved devices.
>
> >
> > static struct vfsmount *binderfs_mnt;
> >
> > @@ -46,6 +52,24 @@ static dev_t binderfs_dev;
> > static DEFINE_MUTEX(binderfs_minors_mutex);
> > static DEFINE_IDA(binderfs_minors);
> >
> > +/**
> > + * binderfs_mount_opts - mount options for binderfs
> > + * @max: maximum number of allocatable binderfs binder devices
> > + */
> > +struct binderfs_mount_opts {
> > + int max;
> > +};
> > +
> > +enum {
> > + Opt_max,
> > + Opt_err
> > +};
> > +
> > +static const match_table_t tokens = {
> > + { Opt_max, "max=%d" },
> > + { Opt_err, NULL }
> > +};
> > +
> > /**
> > * binderfs_info - information about a binderfs mount
> > * @ipc_ns: The ipc namespace the binderfs mount belongs to.
> > @@ -55,13 +79,16 @@ static DEFINE_IDA(binderfs_minors);
> > * created.
> > * @root_gid: gid that needs to be used when a new binder device is
> > * created.
> > + * @mount_opts: The mount options in use.
> > + * @device_count: The current number of allocated binder devices.
> > */
> > struct binderfs_info {
> > struct ipc_namespace *ipc_ns;
> > struct dentry *control_dentry;
> > kuid_t root_uid;
> > kgid_t root_gid;
> > -
> > + struct binderfs_mount_opts mount_opts;
> > + atomic_t device_count;
>
> Why atomic?
>
> You should already have the lock held every time this is accessed,
> so no need to use an atomic value, just use an int.
>
> > /* Reserve new minor number for the new device. */
> > mutex_lock(_minors_mutex);
> > - minor = ida_alloc_max(_minors, BINDERFS_MAX_MINOR, GFP_KERNEL);
> > + if (atomic_inc_return(>device_count) < info->mount_opts.max)
>
> No need for atomic, see, your lock is held :)
Habit, to be honest.
Thanks, fixed version to follow in a bit.
Christian
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] binderfs: implement "max" mount option
On Sat, Dec 22, 2018 at 10:18:06PM +0100, Christian Brauner wrote:
> Since binderfs can be mounted by userns root in non-initial user namespaces
> some precautions are in order. First, a way to set a maximum on the number
> of binder devices that can be allocated per binderfs instance and second, a
> way to reserve a reasonable chunk of binderfs devices for the initial ipc
> namespace.
> A first approach as seen in [1] used sysctls similiar to devpts but was
> shown to be flawed (cf. [2] and [3]) since some aspects were unneeded. This
> is an alternative approach which avoids sysctls completely and instead
> switches to a single mount option.
>
> Starting with this commit binderfs instances can be mounted with a limit on
> the number of binder devices that can be allocated. The max= mount
> option serves as a per-instance limit. If max= is set then only
> number of binder devices can be allocated in this binderfs
> instance.
Ok, this is fine, but why such a big default? You only need 4 to run a
modern android system, and anyone using binder outside of android is
really too crazy to ever be using it in a container :)
> Additionally, the binderfs instance in the initial ipc namespace will
> always have a reserve of at least 1024 binder devices unless explicitly
> capped via max=.
Again, why so many? And why wouldn't that initial ipc namespace already
have their device nodes created _before_ anything else is mounted?
Some comments on the patch below:
> +/*
> + * Ensure that the initial ipc namespace always has a good chunk of devices
> + * available.
> + */
> +#define BINDERFS_MAX_MINOR_CAPPED (BINDERFS_MAX_MINOR - 1024)
Again that seems crazy big, how about splitting this into two different
patches, one for the max= stuff, and one for this "reserve some minors"
thing, so we can review them separately.
>
> static struct vfsmount *binderfs_mnt;
>
> @@ -46,6 +52,24 @@ static dev_t binderfs_dev;
> static DEFINE_MUTEX(binderfs_minors_mutex);
> static DEFINE_IDA(binderfs_minors);
>
> +/**
> + * binderfs_mount_opts - mount options for binderfs
> + * @max: maximum number of allocatable binderfs binder devices
> + */
> +struct binderfs_mount_opts {
> + int max;
> +};
> +
> +enum {
> + Opt_max,
> + Opt_err
> +};
> +
> +static const match_table_t tokens = {
> + { Opt_max, "max=%d" },
> + { Opt_err, NULL }
> +};
> +
> /**
> * binderfs_info - information about a binderfs mount
> * @ipc_ns: The ipc namespace the binderfs mount belongs to.
> @@ -55,13 +79,16 @@ static DEFINE_IDA(binderfs_minors);
> * created.
> * @root_gid: gid that needs to be used when a new binder device is
> * created.
> + * @mount_opts: The mount options in use.
> + * @device_count: The current number of allocated binder devices.
> */
> struct binderfs_info {
> struct ipc_namespace *ipc_ns;
> struct dentry *control_dentry;
> kuid_t root_uid;
> kgid_t root_gid;
> -
> + struct binderfs_mount_opts mount_opts;
> + atomic_t device_count;
Why atomic?
You should already have the lock held every time this is accessed,
so no need to use an atomic value, just use an int.
> /* Reserve new minor number for the new device. */
> mutex_lock(_minors_mutex);
> - minor = ida_alloc_max(_minors, BINDERFS_MAX_MINOR, GFP_KERNEL);
> + if (atomic_inc_return(>device_count) < info->mount_opts.max)
No need for atomic, see, your lock is held :)
thanks,
greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: mt29f_spinand: add SPDX identifiers
This satisfies a checkpatch warning and is the preferred method for notating the license. The SPDX identifier is a legally binding shorthand, which can be used instead of the full boiler plate text. Signed-off-by: Michael Straube --- drivers/staging/mt29f_spinand/mt29f_spinand.c | 11 +-- drivers/staging/mt29f_spinand/mt29f_spinand.h | 12 ++-- 2 files changed, 3 insertions(+), 20 deletions(-) diff --git a/drivers/staging/mt29f_spinand/mt29f_spinand.c b/drivers/staging/mt29f_spinand/mt29f_spinand.c index def8a1f57d1c..8630f7012957 100644 --- a/drivers/staging/mt29f_spinand/mt29f_spinand.c +++ b/drivers/staging/mt29f_spinand/mt29f_spinand.c @@ -1,17 +1,8 @@ +// SPDX-License-Identifier: GPL-2.0+ /* * Copyright (c) 2003-2013 Broadcom Corporation * * Copyright (c) 2009-2010 Micron Technology, Inc. - * - * This program is free software; you can redistribute it and/or - * modify it under the terms of the GNU General Public License - * as published by the Free Software Foundation; either version 2 - * of the License, or (at your option) any later version. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU General Public License for more details. */ #include diff --git a/drivers/staging/mt29f_spinand/mt29f_spinand.h b/drivers/staging/mt29f_spinand/mt29f_spinand.h index 457dc7ffdaf1..38837cc6421c 100644 --- a/drivers/staging/mt29f_spinand/mt29f_spinand.h +++ b/drivers/staging/mt29f_spinand/mt29f_spinand.h @@ -1,17 +1,9 @@ -/*- +/* SPDX-License-Identifier: GPL-2.0 */ +/* * Copyright 2013 Broadcom Corporation * * Copyright (c) 2009-2010 Micron Technology, Inc. * - * This software is licensed under the terms of the GNU General Public - * License version 2, as published by the Free Software Foundation, and - * may be copied, distributed, and modified under those terms. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU General Public License for more details. - * * Henry Pan * * based on nand.h -- 2.20.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: mt7621-pci: use PCI definitions instead of hardcode values
Setting up ports to enable PCI_COMMAND_MASTER is using '0x4' as a
hardcore value and '0x4' also for PCI_COMMAND register instead
of use definitions from linux pci system headers. Replace both.
Signed-off-by: Sergio Paracuellos
---
drivers/staging/mt7621-pci/pci-mt7621.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/mt7621-pci/pci-mt7621.c
b/drivers/staging/mt7621-pci/pci-mt7621.c
index 31310b6fb7db..e720524ba5a3 100644
--- a/drivers/staging/mt7621-pci/pci-mt7621.c
+++ b/drivers/staging/mt7621-pci/pci-mt7621.c
@@ -701,8 +701,9 @@ static void mt7621_pcie_enable_ports(struct mt7621_pcie
*pcie)
}
for (slot = 0; slot < num_slots_enabled; slot++) {
- val = read_config(pcie, slot, 0x4);
- write_config(pcie, slot, 0x4, val | 0x4);
+ val = read_config(pcie, slot, PCI_COMMAND);
+ val |= PCI_COMMAND_MASTER;
+ write_config(pcie, slot, PCI_COMMAND, val);
/* configure RC FTS number to 250 when it leaves L0s */
val = read_config(pcie, slot, PCIE_FTS_NUM);
val &= ~PCIE_FTS_NUM_MASK;
--
2.19.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
