This patchset adds the ability for userspace to select a faster handle
generation logic. This is implemented through a new ioctl that enables
certain functionality on a "per-proc" basis, this is required in order
to maintain backwards compatibility.
Cc: Tim Murray <[email protected]>
Cc: Arve Hjønnevåg <[email protected]>
Cc: Alice Ryhl <[email protected]>
Cc: Martijn Coenen <[email protected]>
Cc: Todd Kjos <[email protected]>
Cc: John Stultz <[email protected]>
Carlos Llamas (4):
binder: introduce BINDER_SET_PROC_FLAGS ioctl
binder: migrate ioctl to new PF_SPAM_DETECTION
binder: add support for PF_LARGE_HANDLES
binder: fix max_thread type inconsistency
drivers/android/binder.c | 78 +++++++++++++++++++++++------
drivers/android/binder_internal.h | 10 ++--
include/uapi/linux/android/binder.h | 13 ++++-
3 files changed, 83 insertions(+), 18 deletions(-)
--
2.44.0.683.g7961c838ac-goog
This new ioctl enables userspace to control the individual behavior of
the 'struct binder_proc' instance via flags. The driver validates and
returns the supported subset. Some existing ioctls are migrated to use
these flags in subsequent commits.
Suggested-by: Arve Hjønnevåg <[email protected]>
Signed-off-by: Carlos Llamas <[email protected]>
---
drivers/android/binder.c | 25 +++++++++++++++++++++++++
drivers/android/binder_internal.h | 4 +++-
include/uapi/linux/android/binder.h | 6 ++++++
3 files changed, 34 insertions(+), 1 deletion(-)
diff --git a/drivers/android/binder.c b/drivers/android/binder.c
index bad28cf42010..e0d193bfb237 100644
--- a/drivers/android/binder.c
+++ b/drivers/android/binder.c
@@ -5334,6 +5334,26 @@ static int binder_ioctl_get_extended_error(struct binder_thread *thread,
return 0;
}
+static int binder_ioctl_set_proc_flags(struct binder_proc *proc,
+ u32 __user *user)
+{
+ u32 flags;
+
+ if (get_user(flags, user))
+ return -EFAULT;
+
+ binder_inner_proc_lock(proc);
+ flags &= PF_SUPPORTED_FLAGS_MASK;
+ proc->flags = flags;
+ binder_inner_proc_unlock(proc);
+
+ /* confirm supported flags with user */
+ if (put_user(flags, user))
+ return -EFAULT;
+
+ return 0;
+}
+
static long binder_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
{
int ret;
@@ -5542,6 +5562,11 @@ static long binder_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
if (ret < 0)
goto err;
break;
+ case BINDER_SET_PROC_FLAGS:
+ ret = binder_ioctl_set_proc_flags(proc, ubuf);
+ if (ret < 0)
+ goto err;
+ break;
default:
ret = -EINVAL;
goto err;
diff --git a/drivers/android/binder_internal.h b/drivers/android/binder_internal.h
index 7270d4d22207..a22e64cddbae 100644
--- a/drivers/android/binder_internal.h
+++ b/drivers/android/binder_internal.h
@@ -346,6 +346,8 @@ struct binder_ref {
* @cred struct cred associated with the `struct file`
* in binder_open()
* (invariant after initialized)
+ * @flags: enum proc_flags set via BINDER_SET_PROC_FLAGS.
+ * (protected by @inner_lock)
* @deferred_work_node: element for binder_deferred_list
* (protected by binder_deferred_lock)
* @deferred_work: bitmap of deferred work to perform
@@ -409,6 +411,7 @@ struct binder_proc {
int pid;
struct task_struct *tsk;
const struct cred *cred;
+ u32 flags;
struct hlist_node deferred_work_node;
int deferred_work;
int outstanding_txns;
@@ -417,7 +420,6 @@ struct binder_proc {
bool sync_recv;
bool async_recv;
wait_queue_head_t freeze_wait;
-
struct list_head todo;
struct binder_stats stats;
struct list_head delivered_death;
diff --git a/include/uapi/linux/android/binder.h b/include/uapi/linux/android/binder.h
index d44a8118b2ed..281a8e2e734e 100644
--- a/include/uapi/linux/android/binder.h
+++ b/include/uapi/linux/android/binder.h
@@ -251,6 +251,11 @@ struct binder_extended_error {
__s32 param;
};
+/* Used with BINDER_SET_PROC_FLAGS ioctl */
+enum proc_flags {
+ PF_SUPPORTED_FLAGS_MASK,
+};
+
enum {
BINDER_WRITE_READ = _IOWR('b', 1, struct binder_write_read),
BINDER_SET_IDLE_TIMEOUT = _IOW('b', 3, __s64),
@@ -266,6 +271,7 @@ enum {
BINDER_GET_FROZEN_INFO = _IOWR('b', 15, struct binder_frozen_status_info),
BINDER_ENABLE_ONEWAY_SPAM_DETECTION = _IOW('b', 16, __u32),
BINDER_GET_EXTENDED_ERROR = _IOWR('b', 17, struct binder_extended_error),
+ BINDER_SET_PROC_FLAGS = _IOWR('b', 18, __u32),
};
/*
--
2.44.0.683.g7961c838ac-goog
Carlos Llamas <[email protected]> writes:
> This new ioctl enables userspace to control the individual behavior of
> the 'struct binder_proc' instance via flags. The driver validates and
> returns the supported subset. Some existing ioctls are migrated to use
> these flags in subsequent commits.
>
> Suggested-by: Arve Hjønnevåg <[email protected]>
> Signed-off-by: Carlos Llamas <[email protected]>
> ---
> drivers/android/binder.c | 25 +++++++++++++++++++++++++
> drivers/android/binder_internal.h | 4 +++-
> include/uapi/linux/android/binder.h | 6 ++++++
> 3 files changed, 34 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/android/binder.c b/drivers/android/binder.c
> index bad28cf42010..e0d193bfb237 100644
> --- a/drivers/android/binder.c
> +++ b/drivers/android/binder.c
> @@ -5334,6 +5334,26 @@ static int binder_ioctl_get_extended_error(struct binder_thread *thread,
> return 0;
> }
>
> +static int binder_ioctl_set_proc_flags(struct binder_proc *proc,
> + u32 __user *user)
> +{
> + u32 flags;
> +
> + if (get_user(flags, user))
> + return -EFAULT;
> +
> + binder_inner_proc_lock(proc);
> + flags &= PF_SUPPORTED_FLAGS_MASK;
> + proc->flags = flags;
> + binder_inner_proc_unlock(proc);
> +
> + /* confirm supported flags with user */
> + if (put_user(flags, user))
> + return -EFAULT;
> +
> + return 0;
> +}
I'm just thinking out loud here, but is this the best API for this
ioctl? Using this API, if I want to toggle the oneway-spam-detection
flag, then I can't do so without knowing the value of all other flags,
and I also need to synchronize all calls to this ioctl.
That's fine for the current use-case where these flags are only set
during startup, but are we confident that no future flag will be toggled
while a process is active?
How about these alternatives?
1. Userspace passes two masks, one containing bits to set, and another
containing bits to unset. Userspace returns new value of flags. (If
the same bit is set in both masks, we can fail with EINVAL.)
2. Compare and swap. Userspace passes the expected previous value and
the desired new value. The kernel returns the actual previous value
and updates it only if userspace gave the right previous value.
3. Set or unset only. Userspace passes a boolean and a mask. Boolean
determines whether userspace wants to set or unset the bits set in
the mask.
I don't know what the usual kernel convention is for this kind of
ioctl, so I'm happy with whatever you all think is best.
Alice
On Thu, Apr 18, 2024 at 08:34:47AM +0000, Alice Ryhl wrote:
> Carlos Llamas <[email protected]> writes:
> > This new ioctl enables userspace to control the individual behavior of
> > the 'struct binder_proc' instance via flags. The driver validates and
> > returns the supported subset. Some existing ioctls are migrated to use
> > these flags in subsequent commits.
> >
> > Suggested-by: Arve Hj?nnev?g <[email protected]>
> > Signed-off-by: Carlos Llamas <[email protected]>
> > ---
> > drivers/android/binder.c | 25 +++++++++++++++++++++++++
> > drivers/android/binder_internal.h | 4 +++-
> > include/uapi/linux/android/binder.h | 6 ++++++
> > 3 files changed, 34 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/android/binder.c b/drivers/android/binder.c
> > index bad28cf42010..e0d193bfb237 100644
> > --- a/drivers/android/binder.c
> > +++ b/drivers/android/binder.c
> > @@ -5334,6 +5334,26 @@ static int binder_ioctl_get_extended_error(struct binder_thread *thread,
> > return 0;
> > }
> >
> > +static int binder_ioctl_set_proc_flags(struct binder_proc *proc,
> > + u32 __user *user)
> > +{
> > + u32 flags;
> > +
> > + if (get_user(flags, user))
> > + return -EFAULT;
> > +
> > + binder_inner_proc_lock(proc);
> > + flags &= PF_SUPPORTED_FLAGS_MASK;
> > + proc->flags = flags;
> > + binder_inner_proc_unlock(proc);
> > +
> > + /* confirm supported flags with user */
> > + if (put_user(flags, user))
> > + return -EFAULT;
> > +
> > + return 0;
> > +}
>
> I'm just thinking out loud here, but is this the best API for this
> ioctl? Using this API, if I want to toggle the oneway-spam-detection
> flag, then I can't do so without knowing the value of all other flags,
> and I also need to synchronize all calls to this ioctl.
>
> That's fine for the current use-case where these flags are only set
> during startup, but are we confident that no future flag will be toggled
> while a process is active?
hmmm, this is a very good point. It would probably lead to userspace
having to cache its flags for every binder instance. This is not a good
solution at all.
>
> How about these alternatives?
>
> 1. Userspace passes two masks, one containing bits to set, and another
> containing bits to unset. Userspace returns new value of flags. (If
> the same bit is set in both masks, we can fail with EINVAL.)
>
> 2. Compare and swap. Userspace passes the expected previous value and
> the desired new value. The kernel returns the actual previous value
> and updates it only if userspace gave the right previous value.
>
> 3. Set or unset only. Userspace passes a boolean and a mask. Boolean
> determines whether userspace wants to set or unset the bits set in
> the mask.
>
> I don't know what the usual kernel convention is for this kind of
> ioctl, so I'm happy with whatever you all think is best.
I've never come across these types of alternatives personally. What I've
seen however, is the typical SET/GET ioctl pairs. This is a "simpler"
interface I guess but it has the downside of an extra roundtrip. e.g.
ioctl(fd, BINDER_GET_PROC_FLAGS, &flags);
flags |= BF_LARGE_HANDLES;
ioctl(fd, BINDER_SET_PROC_FLAGS, &flags);
What seems tempting about the SET/GET pair is that we could replace the
BINDER_ENABLE_ONEWAY_SPAM_DETECTION with the SET. Instead of maintaining
legacy code for the "deprecated" ioctl.
wdyt?
I'll have a second look at the alternatives you mentioned. Perhaps I can
reference some other existing ioctl that does something similar.
--
Carlos Llamas
On Sun, Apr 21, 2024 at 1:39 AM Carlos Llamas <[email protected]> wrote:
>
> On Thu, Apr 18, 2024 at 08:34:47AM +0000, Alice Ryhl wrote:
> > Carlos Llamas <[email protected]> writes:
> > > This new ioctl enables userspace to control the individual behavior of
> > > the 'struct binder_proc' instance via flags. The driver validates and
> > > returns the supported subset. Some existing ioctls are migrated to use
> > > these flags in subsequent commits.
> > >
> > > Suggested-by: Arve Hjønnevåg <[email protected]>
> > > Signed-off-by: Carlos Llamas <[email protected]>
> > > ---
> > > drivers/android/binder.c | 25 +++++++++++++++++++++++++
> > > drivers/android/binder_internal.h | 4 +++-
> > > include/uapi/linux/android/binder.h | 6 ++++++
> > > 3 files changed, 34 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/android/binder.c b/drivers/android/binder.c
> > > index bad28cf42010..e0d193bfb237 100644
> > > --- a/drivers/android/binder.c
> > > +++ b/drivers/android/binder.c
> > > @@ -5334,6 +5334,26 @@ static int binder_ioctl_get_extended_error(struct binder_thread *thread,
> > > return 0;
> > > }
> > >
> > > +static int binder_ioctl_set_proc_flags(struct binder_proc *proc,
> > > + u32 __user *user)
> > > +{
> > > + u32 flags;
> > > +
> > > + if (get_user(flags, user))
> > > + return -EFAULT;
> > > +
> > > + binder_inner_proc_lock(proc);
> > > + flags &= PF_SUPPORTED_FLAGS_MASK;
> > > + proc->flags = flags;
> > > + binder_inner_proc_unlock(proc);
> > > +
> > > + /* confirm supported flags with user */
> > > + if (put_user(flags, user))
> > > + return -EFAULT;
> > > +
> > > + return 0;
> > > +}
> >
> > I'm just thinking out loud here, but is this the best API for this
> > ioctl? Using this API, if I want to toggle the oneway-spam-detection
> > flag, then I can't do so without knowing the value of all other flags,
> > and I also need to synchronize all calls to this ioctl.
> >
> > That's fine for the current use-case where these flags are only set
> > during startup, but are we confident that no future flag will be toggled
> > while a process is active?
>
> hmmm, this is a very good point. It would probably lead to userspace
> having to cache its flags for every binder instance. This is not a good
> solution at all.
>
> >
> > How about these alternatives?
> >
> > 1. Userspace passes two masks, one containing bits to set, and another
> > containing bits to unset. Userspace returns new value of flags. (If
> > the same bit is set in both masks, we can fail with EINVAL.)
To add to this one, one could also say that if a bit is set in both,
then the value is toggled.
> > 2. Compare and swap. Userspace passes the expected previous value and
> > the desired new value. The kernel returns the actual previous value
> > and updates it only if userspace gave the right previous value.
> >
> > 3. Set or unset only. Userspace passes a boolean and a mask. Boolean
> > determines whether userspace wants to set or unset the bits set in
> > the mask.
> >
> > I don't know what the usual kernel convention is for this kind of
> > ioctl, so I'm happy with whatever you all think is best.
>
> I've never come across these types of alternatives personally. What I've
> seen however, is the typical SET/GET ioctl pairs. This is a "simpler"
> interface I guess but it has the downside of an extra roundtrip. e.g.
>
> ioctl(fd, BINDER_GET_PROC_FLAGS, &flags);
> flags |= BF_LARGE_HANDLES;
> ioctl(fd, BINDER_SET_PROC_FLAGS, &flags);
>
> What seems tempting about the SET/GET pair is that we could replace the
> BINDER_ENABLE_ONEWAY_SPAM_DETECTION with the SET. Instead of maintaining
> legacy code for the "deprecated" ioctl.
>
> wdyt?
>
> I'll have a second look at the alternatives you mentioned. Perhaps I can
> reference some other existing ioctl that does something similar.
Hmm. I don't think a get/set pair improves the situation much.
Userspace still needs a global mutex for making changes to the flag in
that case. Otherwise, two threads changing two different flags in
parallel could result in a race condition where one of the changes is
lost.
Alice
On Mon, Apr 22, 2024 at 10:56:31AM +0200, Alice Ryhl wrote:
> On Sun, Apr 21, 2024 at 1:39 AM Carlos Llamas <[email protected]> wrote:
> >
> > On Thu, Apr 18, 2024 at 08:34:47AM +0000, Alice Ryhl wrote:
> > > Carlos Llamas <[email protected]> writes:
> > > > This new ioctl enables userspace to control the individual behavior of
> > > > the 'struct binder_proc' instance via flags. The driver validates and
> > > > returns the supported subset. Some existing ioctls are migrated to use
> > > > these flags in subsequent commits.
> > > >
> > > > Suggested-by: Arve Hjønnevåg <[email protected]>
> > > > Signed-off-by: Carlos Llamas <[email protected]>
> > > > ---
> > > > drivers/android/binder.c | 25 +++++++++++++++++++++++++
> > > > drivers/android/binder_internal.h | 4 +++-
> > > > include/uapi/linux/android/binder.h | 6 ++++++
> > > > 3 files changed, 34 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/android/binder.c b/drivers/android/binder.c
> > > > index bad28cf42010..e0d193bfb237 100644
> > > > --- a/drivers/android/binder.c
> > > > +++ b/drivers/android/binder.c
> > > > @@ -5334,6 +5334,26 @@ static int binder_ioctl_get_extended_error(struct binder_thread *thread,
> > > > return 0;
> > > > }
> > > >
> > > > +static int binder_ioctl_set_proc_flags(struct binder_proc *proc,
> > > > + u32 __user *user)
> > > > +{
> > > > + u32 flags;
> > > > +
> > > > + if (get_user(flags, user))
> > > > + return -EFAULT;
> > > > +
> > > > + binder_inner_proc_lock(proc);
> > > > + flags &= PF_SUPPORTED_FLAGS_MASK;
> > > > + proc->flags = flags;
> > > > + binder_inner_proc_unlock(proc);
> > > > +
> > > > + /* confirm supported flags with user */
> > > > + if (put_user(flags, user))
> > > > + return -EFAULT;
> > > > +
> > > > + return 0;
> > > > +}
> > >
> > > I'm just thinking out loud here, but is this the best API for this
> > > ioctl? Using this API, if I want to toggle the oneway-spam-detection
> > > flag, then I can't do so without knowing the value of all other flags,
> > > and I also need to synchronize all calls to this ioctl.
> > >
> > > That's fine for the current use-case where these flags are only set
> > > during startup, but are we confident that no future flag will be toggled
> > > while a process is active?
> >
> > hmmm, this is a very good point. It would probably lead to userspace
> > having to cache its flags for every binder instance. This is not a good
> > solution at all.
> >
> > >
> > > How about these alternatives?
> > >
> > > 1. Userspace passes two masks, one containing bits to set, and another
> > > containing bits to unset. Userspace returns new value of flags. (If
> > > the same bit is set in both masks, we can fail with EINVAL.)
>
> To add to this one, one could also say that if a bit is set in both,
> then the value is toggled.
>
> > > 2. Compare and swap. Userspace passes the expected previous value and
> > > the desired new value. The kernel returns the actual previous value
> > > and updates it only if userspace gave the right previous value.
> > >
> > > 3. Set or unset only. Userspace passes a boolean and a mask. Boolean
> > > determines whether userspace wants to set or unset the bits set in
> > > the mask.
> > >
> > > I don't know what the usual kernel convention is for this kind of
> > > ioctl, so I'm happy with whatever you all think is best.
> >
> > I've never come across these types of alternatives personally. What I've
> > seen however, is the typical SET/GET ioctl pairs. This is a "simpler"
> > interface I guess but it has the downside of an extra roundtrip. e.g.
> >
> > ioctl(fd, BINDER_GET_PROC_FLAGS, &flags);
> > flags |= BF_LARGE_HANDLES;
> > ioctl(fd, BINDER_SET_PROC_FLAGS, &flags);
> >
> > What seems tempting about the SET/GET pair is that we could replace the
> > BINDER_ENABLE_ONEWAY_SPAM_DETECTION with the SET. Instead of maintaining
> > legacy code for the "deprecated" ioctl.
> >
> > wdyt?
> >
> > I'll have a second look at the alternatives you mentioned. Perhaps I can
> > reference some other existing ioctl that does something similar.
>
> Hmm. I don't think a get/set pair improves the situation much.
> Userspace still needs a global mutex for making changes to the flag in
> that case. Otherwise, two threads changing two different flags in
> parallel could result in a race condition where one of the changes is
> lost.
I'm not sure this would ever be a problem, libbinder currently has a
mutex for this kind of things already and it seems unlikely that these
process-wide flags would be toggled outside of initial config. However,
it is worth discussing for sure as things can change.
I'm mainly concern about overloading what should be a very simple ioctl.
With that said, I think one more option that is fairly simple/common is
a SET/CLEAR ioctl pair. A little adaption from your 3rd option I think?
I would be fine with that.
Unfortunately, this would require new ioctl IDs so we would still need
to maintain the ONEWAY_SPAM thing. I suppose that's ok.
Sounds good?
--
Carlos Llamas
On Tue, Apr 23, 2024 at 12:48 AM Carlos Llamas <[email protected]> wrote:
>
> On Mon, Apr 22, 2024 at 10:56:31AM +0200, Alice Ryhl wrote:
> > On Sun, Apr 21, 2024 at 1:39 AM Carlos Llamas <[email protected]> wrote:
> > >
> > > On Thu, Apr 18, 2024 at 08:34:47AM +0000, Alice Ryhl wrote:
> > > > Carlos Llamas <[email protected]> writes:
> > > > > This new ioctl enables userspace to control the individual behavior of
> > > > > the 'struct binder_proc' instance via flags. The driver validates and
> > > > > returns the supported subset. Some existing ioctls are migrated to use
> > > > > these flags in subsequent commits.
> > > > >
> > > > > Suggested-by: Arve Hjønnevåg <[email protected]>
> > > > > Signed-off-by: Carlos Llamas <[email protected]>
> > > > > ---
> > > > > drivers/android/binder.c | 25 +++++++++++++++++++++++++
> > > > > drivers/android/binder_internal.h | 4 +++-
> > > > > include/uapi/linux/android/binder.h | 6 ++++++
> > > > > 3 files changed, 34 insertions(+), 1 deletion(-)
> > > > >
> > > > > diff --git a/drivers/android/binder.c b/drivers/android/binder.c
> > > > > index bad28cf42010..e0d193bfb237 100644
> > > > > --- a/drivers/android/binder.c
> > > > > +++ b/drivers/android/binder.c
> > > > > @@ -5334,6 +5334,26 @@ static int binder_ioctl_get_extended_error(struct binder_thread *thread,
> > > > > return 0;
> > > > > }
> > > > >
> > > > > +static int binder_ioctl_set_proc_flags(struct binder_proc *proc,
> > > > > + u32 __user *user)
> > > > > +{
> > > > > + u32 flags;
> > > > > +
> > > > > + if (get_user(flags, user))
> > > > > + return -EFAULT;
> > > > > +
> > > > > + binder_inner_proc_lock(proc);
> > > > > + flags &= PF_SUPPORTED_FLAGS_MASK;
> > > > > + proc->flags = flags;
> > > > > + binder_inner_proc_unlock(proc);
> > > > > +
> > > > > + /* confirm supported flags with user */
> > > > > + if (put_user(flags, user))
> > > > > + return -EFAULT;
> > > > > +
> > > > > + return 0;
> > > > > +}
> > > >
> > > > I'm just thinking out loud here, but is this the best API for this
> > > > ioctl? Using this API, if I want to toggle the oneway-spam-detection
> > > > flag, then I can't do so without knowing the value of all other flags,
> > > > and I also need to synchronize all calls to this ioctl.
> > > >
> > > > That's fine for the current use-case where these flags are only set
> > > > during startup, but are we confident that no future flag will be toggled
> > > > while a process is active?
> > >
> > > hmmm, this is a very good point. It would probably lead to userspace
> > > having to cache its flags for every binder instance. This is not a good
> > > solution at all.
> > >
> > > >
> > > > How about these alternatives?
> > > >
> > > > 1. Userspace passes two masks, one containing bits to set, and another
> > > > containing bits to unset. Userspace returns new value of flags. (If
> > > > the same bit is set in both masks, we can fail with EINVAL.)
> >
> > To add to this one, one could also say that if a bit is set in both,
> > then the value is toggled.
> >
> > > > 2. Compare and swap. Userspace passes the expected previous value and
> > > > the desired new value. The kernel returns the actual previous value
> > > > and updates it only if userspace gave the right previous value.
> > > >
> > > > 3. Set or unset only. Userspace passes a boolean and a mask. Boolean
> > > > determines whether userspace wants to set or unset the bits set in
> > > > the mask.
> > > >
> > > > I don't know what the usual kernel convention is for this kind of
> > > > ioctl, so I'm happy with whatever you all think is best.
> > >
> > > I've never come across these types of alternatives personally. What I've
> > > seen however, is the typical SET/GET ioctl pairs. This is a "simpler"
> > > interface I guess but it has the downside of an extra roundtrip. e.g.
> > >
> > > ioctl(fd, BINDER_GET_PROC_FLAGS, &flags);
> > > flags |= BF_LARGE_HANDLES;
> > > ioctl(fd, BINDER_SET_PROC_FLAGS, &flags);
> > >
> > > What seems tempting about the SET/GET pair is that we could replace the
> > > BINDER_ENABLE_ONEWAY_SPAM_DETECTION with the SET. Instead of maintaining
> > > legacy code for the "deprecated" ioctl.
> > >
> > > wdyt?
> > >
> > > I'll have a second look at the alternatives you mentioned. Perhaps I can
> > > reference some other existing ioctl that does something similar.
> >
> > Hmm. I don't think a get/set pair improves the situation much.
> > Userspace still needs a global mutex for making changes to the flag in
> > that case. Otherwise, two threads changing two different flags in
> > parallel could result in a race condition where one of the changes is
> > lost.
>
> I'm not sure this would ever be a problem, libbinder currently has a
> mutex for this kind of things already and it seems unlikely that these
> process-wide flags would be toggled outside of initial config. However,
> it is worth discussing for sure as things can change.
>
> I'm mainly concern about overloading what should be a very simple ioctl.
> With that said, I think one more option that is fairly simple/common is
> a SET/CLEAR ioctl pair. A little adaption from your 3rd option I think?
> I would be fine with that.
>
> Unfortunately, this would require new ioctl IDs so we would still need
> to maintain the ONEWAY_SPAM thing. I suppose that's ok.
As long as your decision has taken the things I mentioned into
account, I'm happy with whatever you think is best. If you think a
GET/SET pair is reasonable because binder already has a mutex, then
that's fine with me.
Alice