linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] io_uring: Remove CONFIG_EXPERT
@ 2021-05-26 22:34 Justin M. Forbes
  2021-05-26 22:55 ` Pavel Begunkov
  2021-05-27 13:43 ` Jens Axboe
  0 siblings, 2 replies; 9+ messages in thread
From: Justin M. Forbes @ 2021-05-26 22:34 UTC (permalink / raw)
  To: axboe, asml.silence, io-uring, linux-kernel; +Cc: Justin M. Forbes

While IO_URING has been in fairly heavy development, it is hidden behind
CONFIG_EXPERT with a default of on.  It has been long enough now that I
think we should remove EXPERT and allow users and distros to decide how
they want this config option set without jumping through hoops.

Signed-off-by: Justin M. Forbes <jforbes@fedoraproject.org>
---
 init/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/init/Kconfig b/init/Kconfig
index 1ea12c64e4c9..0decca696bf7 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -1621,7 +1621,7 @@ config AIO
 	  this option saves about 7k.

 config IO_URING
-	bool "Enable IO uring support" if EXPERT
+	bool "Enable IO uring support"
 	select IO_WQ
 	default y
 	help
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH] io_uring: Remove CONFIG_EXPERT
  2021-05-26 22:34 [PATCH] io_uring: Remove CONFIG_EXPERT Justin M. Forbes
@ 2021-05-26 22:55 ` Pavel Begunkov
  2021-05-27 13:43 ` Jens Axboe
  1 sibling, 0 replies; 9+ messages in thread
From: Pavel Begunkov @ 2021-05-26 22:55 UTC (permalink / raw)
  To: Justin M. Forbes, axboe, io-uring, linux-kernel

On 5/26/21 11:34 PM, Justin M. Forbes wrote:
> While IO_URING has been in fairly heavy development, it is hidden behind
> CONFIG_EXPERT with a default of on.  It has been long enough now that I
> think we should remove EXPERT and allow users and distros to decide how
> they want this config option set without jumping through hoops.

Acked-by: Pavel Begunkov <asml.silence@gmail.com>


> Signed-off-by: Justin M. Forbes <jforbes@fedoraproject.org>
> ---
>  init/Kconfig | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/init/Kconfig b/init/Kconfig
> index 1ea12c64e4c9..0decca696bf7 100644
> --- a/init/Kconfig
> +++ b/init/Kconfig
> @@ -1621,7 +1621,7 @@ config AIO
>  	  this option saves about 7k.
> 
>  config IO_URING
> -	bool "Enable IO uring support" if EXPERT
> +	bool "Enable IO uring support"
>  	select IO_WQ
>  	default y
>  	help
> 

-- 
Pavel Begunkov

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] io_uring: Remove CONFIG_EXPERT
  2021-05-26 22:34 [PATCH] io_uring: Remove CONFIG_EXPERT Justin M. Forbes
  2021-05-26 22:55 ` Pavel Begunkov
@ 2021-05-27 13:43 ` Jens Axboe
  2021-05-27 14:12   ` Justin Forbes
  1 sibling, 1 reply; 9+ messages in thread
From: Jens Axboe @ 2021-05-27 13:43 UTC (permalink / raw)
  To: Justin M. Forbes, asml.silence, io-uring, linux-kernel

On 5/26/21 4:34 PM, Justin M. Forbes wrote:
> While IO_URING has been in fairly heavy development, it is hidden behind
> CONFIG_EXPERT with a default of on.  It has been long enough now that I
> think we should remove EXPERT and allow users and distros to decide how
> they want this config option set without jumping through hoops.

The whole point of EXPERT is to ensure that it doesn't get turned off
"by accident". It's a core feature, and something that more and more
apps or libraries are relying on. It's not something I intended to ever
go away, just like it would never go away for eg futex or epoll support.

-- 
Jens Axboe


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] io_uring: Remove CONFIG_EXPERT
  2021-05-27 13:43 ` Jens Axboe
@ 2021-05-27 14:12   ` Justin Forbes
  2021-05-27 14:19     ` Jens Axboe
  0 siblings, 1 reply; 9+ messages in thread
From: Justin Forbes @ 2021-05-27 14:12 UTC (permalink / raw)
  To: Jens Axboe; +Cc: asml.silence, io-uring, Linux Kernel Mailing List

On Thu, May 27, 2021 at 8:43 AM Jens Axboe <axboe@kernel.dk> wrote:
>
> On 5/26/21 4:34 PM, Justin M. Forbes wrote:
> > While IO_URING has been in fairly heavy development, it is hidden behind
> > CONFIG_EXPERT with a default of on.  It has been long enough now that I
> > think we should remove EXPERT and allow users and distros to decide how
> > they want this config option set without jumping through hoops.
>
> The whole point of EXPERT is to ensure that it doesn't get turned off
> "by accident". It's a core feature, and something that more and more
> apps or libraries are relying on. It's not something I intended to ever
> go away, just like it would never go away for eg futex or epoll support.
>

I am not arguing with that, I don't expect it will go away. I
certainly do not have an issue with it defaulting to on, and I didn't
even submit this with intention to turn it off for default Fedora. I
do think that there are cases where people might not wish it turned on
at this point in time. Hiding it behind EXPERT makes it much more
difficult than it needs to be.  There are plenty of config options
that are largely expected default and not hidden behind EXPERT.

Justin


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] io_uring: Remove CONFIG_EXPERT
  2021-05-27 14:12   ` Justin Forbes
@ 2021-05-27 14:19     ` Jens Axboe
  2021-05-27 16:01       ` Justin Forbes
  0 siblings, 1 reply; 9+ messages in thread
From: Jens Axboe @ 2021-05-27 14:19 UTC (permalink / raw)
  To: Justin Forbes; +Cc: asml.silence, io-uring, Linux Kernel Mailing List

On 5/27/21 8:12 AM, Justin Forbes wrote:
> On Thu, May 27, 2021 at 8:43 AM Jens Axboe <axboe@kernel.dk> wrote:
>>
>> On 5/26/21 4:34 PM, Justin M. Forbes wrote:
>>> While IO_URING has been in fairly heavy development, it is hidden behind
>>> CONFIG_EXPERT with a default of on.  It has been long enough now that I
>>> think we should remove EXPERT and allow users and distros to decide how
>>> they want this config option set without jumping through hoops.
>>
>> The whole point of EXPERT is to ensure that it doesn't get turned off
>> "by accident". It's a core feature, and something that more and more
>> apps or libraries are relying on. It's not something I intended to ever
>> go away, just like it would never go away for eg futex or epoll support.
>>
> 
> I am not arguing with that, I don't expect it will go away. I
> certainly do not have an issue with it defaulting to on, and I didn't
> even submit this with intention to turn it off for default Fedora. I
> do think that there are cases where people might not wish it turned on
> at this point in time. Hiding it behind EXPERT makes it much more
> difficult than it needs to be.  There are plenty of config options
> that are largely expected default and not hidden behind EXPERT.

Right there are, but not really core kernel features like the ones
I mentioned. Hence my argument for why it's correct as-is and I
don't think it should be changed.

-- 
Jens Axboe


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] io_uring: Remove CONFIG_EXPERT
  2021-05-27 14:19     ` Jens Axboe
@ 2021-05-27 16:01       ` Justin Forbes
  2021-05-28  7:59         ` David Laight
  2021-06-03 15:43         ` Justin Forbes
  0 siblings, 2 replies; 9+ messages in thread
From: Justin Forbes @ 2021-05-27 16:01 UTC (permalink / raw)
  To: Jens Axboe; +Cc: asml.silence, io-uring, Linux Kernel Mailing List

On Thu, May 27, 2021 at 9:19 AM Jens Axboe <axboe@kernel.dk> wrote:
>
> On 5/27/21 8:12 AM, Justin Forbes wrote:
> > On Thu, May 27, 2021 at 8:43 AM Jens Axboe <axboe@kernel.dk> wrote:
> >>
> >> On 5/26/21 4:34 PM, Justin M. Forbes wrote:
> >>> While IO_URING has been in fairly heavy development, it is hidden behind
> >>> CONFIG_EXPERT with a default of on.  It has been long enough now that I
> >>> think we should remove EXPERT and allow users and distros to decide how
> >>> they want this config option set without jumping through hoops.
> >>
> >> The whole point of EXPERT is to ensure that it doesn't get turned off
> >> "by accident". It's a core feature, and something that more and more
> >> apps or libraries are relying on. It's not something I intended to ever
> >> go away, just like it would never go away for eg futex or epoll support.
> >>
> >
> > I am not arguing with that, I don't expect it will go away. I
> > certainly do not have an issue with it defaulting to on, and I didn't
> > even submit this with intention to turn it off for default Fedora. I
> > do think that there are cases where people might not wish it turned on
> > at this point in time. Hiding it behind EXPERT makes it much more
> > difficult than it needs to be.  There are plenty of config options
> > that are largely expected default and not hidden behind EXPERT.
>
> Right there are, but not really core kernel features like the ones
> I mentioned. Hence my argument for why it's correct as-is and I
> don't think it should be changed.
>

Honestly, this is fair, and I understand your concerns behind it. I
think my real issue is that there is no simple way to override one
EXPERT setting without having to set them all.  It would be nice if
expert were a "visible if" menu, setting defaults if not selected,
which allows direct override with a config file. Perhaps I will try to
fix this in kbuild.

Justin


^ permalink raw reply	[flat|nested] 9+ messages in thread

* RE: [PATCH] io_uring: Remove CONFIG_EXPERT
  2021-05-27 16:01       ` Justin Forbes
@ 2021-05-28  7:59         ` David Laight
  2021-06-03 15:43         ` Justin Forbes
  1 sibling, 0 replies; 9+ messages in thread
From: David Laight @ 2021-05-28  7:59 UTC (permalink / raw)
  To: 'Justin Forbes', Jens Axboe
  Cc: asml.silence, io-uring, Linux Kernel Mailing List

From: Justin Forbes
> Sent: 27 May 2021 17:01
> 
> On Thu, May 27, 2021 at 9:19 AM Jens Axboe <axboe@kernel.dk> wrote:
> >
> > On 5/27/21 8:12 AM, Justin Forbes wrote:
> > > On Thu, May 27, 2021 at 8:43 AM Jens Axboe <axboe@kernel.dk> wrote:
> > >>
> > >> On 5/26/21 4:34 PM, Justin M. Forbes wrote:
> > >>> While IO_URING has been in fairly heavy development, it is hidden behind
> > >>> CONFIG_EXPERT with a default of on.  It has been long enough now that I
> > >>> think we should remove EXPERT and allow users and distros to decide how
> > >>> they want this config option set without jumping through hoops.
> > >>
> > >> The whole point of EXPERT is to ensure that it doesn't get turned off
> > >> "by accident". It's a core feature, and something that more and more
> > >> apps or libraries are relying on. It's not something I intended to ever
> > >> go away, just like it would never go away for eg futex or epoll support.
> > >>
> > >
> > > I am not arguing with that, I don't expect it will go away. I
> > > certainly do not have an issue with it defaulting to on, and I didn't
> > > even submit this with intention to turn it off for default Fedora. I
> > > do think that there are cases where people might not wish it turned on
> > > at this point in time. Hiding it behind EXPERT makes it much more
> > > difficult than it needs to be.  There are plenty of config options
> > > that are largely expected default and not hidden behind EXPERT.
> >
> > Right there are, but not really core kernel features like the ones
> > I mentioned. Hence my argument for why it's correct as-is and I
> > don't think it should be changed.
> >
> 
> Honestly, this is fair, and I understand your concerns behind it. I
> think my real issue is that there is no simple way to override one
> EXPERT setting without having to set them all.  It would be nice if
> expert were a "visible if" menu, setting defaults if not selected,
> which allows direct override with a config file. Perhaps I will try to
> fix this in kbuild.

Someone might want to disable things like IO_URING for an embedded
system just to remove code that might have possible attack vectors
and isn't required by the subset of kernel features they need.

If turning on 'EXPERT' makes extra config options visible this is ok.
But if that changes the defaults it gets to be a real PITA.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] io_uring: Remove CONFIG_EXPERT
  2021-05-27 16:01       ` Justin Forbes
  2021-05-28  7:59         ` David Laight
@ 2021-06-03 15:43         ` Justin Forbes
  2021-06-03 15:46           ` Jens Axboe
  1 sibling, 1 reply; 9+ messages in thread
From: Justin Forbes @ 2021-06-03 15:43 UTC (permalink / raw)
  To: Justin Forbes
  Cc: Jens Axboe, asml.silence, io-uring, Linux Kernel Mailing List

On Thu, May 27, 2021 at 11:01 AM Justin Forbes <jforbes@redhat.com> wrote:
>
> On Thu, May 27, 2021 at 9:19 AM Jens Axboe <axboe@kernel.dk> wrote:
> >
> > On 5/27/21 8:12 AM, Justin Forbes wrote:
> > > On Thu, May 27, 2021 at 8:43 AM Jens Axboe <axboe@kernel.dk> wrote:
> > >>
> > >> On 5/26/21 4:34 PM, Justin M. Forbes wrote:
> > >>> While IO_URING has been in fairly heavy development, it is hidden behind
> > >>> CONFIG_EXPERT with a default of on.  It has been long enough now that I
> > >>> think we should remove EXPERT and allow users and distros to decide how
> > >>> they want this config option set without jumping through hoops.
> > >>
> > >> The whole point of EXPERT is to ensure that it doesn't get turned off
> > >> "by accident". It's a core feature, and something that more and more
> > >> apps or libraries are relying on. It's not something I intended to ever
> > >> go away, just like it would never go away for eg futex or epoll support.
> > >>
> > >
> > > I am not arguing with that, I don't expect it will go away. I
> > > certainly do not have an issue with it defaulting to on, and I didn't
> > > even submit this with intention to turn it off for default Fedora. I
> > > do think that there are cases where people might not wish it turned on
> > > at this point in time. Hiding it behind EXPERT makes it much more
> > > difficult than it needs to be.  There are plenty of config options
> > > that are largely expected default and not hidden behind EXPERT.
> >
> > Right there are, but not really core kernel features like the ones
> > I mentioned. Hence my argument for why it's correct as-is and I
> > don't think it should be changed.
> >
>
> Honestly, this is fair, and I understand your concerns behind it. I
> think my real issue is that there is no simple way to override one
> EXPERT setting without having to set them all.  It would be nice if
> expert were a "visible if" menu, setting defaults if not selected,
> which allows direct override with a config file. Perhaps I will try to
> fix this in kbuild.
>

So it turns out that untangling this in kbuild is very difficult
without getting very unexpected results. Given the audit and security
discussions around io_uring lately, I am inclined to believe the
proper action is still to remove 'if EXPERT'.

Justin

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] io_uring: Remove CONFIG_EXPERT
  2021-06-03 15:43         ` Justin Forbes
@ 2021-06-03 15:46           ` Jens Axboe
  0 siblings, 0 replies; 9+ messages in thread
From: Jens Axboe @ 2021-06-03 15:46 UTC (permalink / raw)
  To: Justin Forbes, Justin Forbes
  Cc: asml.silence, io-uring, Linux Kernel Mailing List

On 6/3/21 9:43 AM, Justin Forbes wrote:
> On Thu, May 27, 2021 at 11:01 AM Justin Forbes <jforbes@redhat.com> wrote:
>>
>> On Thu, May 27, 2021 at 9:19 AM Jens Axboe <axboe@kernel.dk> wrote:
>>>
>>> On 5/27/21 8:12 AM, Justin Forbes wrote:
>>>> On Thu, May 27, 2021 at 8:43 AM Jens Axboe <axboe@kernel.dk> wrote:
>>>>>
>>>>> On 5/26/21 4:34 PM, Justin M. Forbes wrote:
>>>>>> While IO_URING has been in fairly heavy development, it is hidden behind
>>>>>> CONFIG_EXPERT with a default of on.  It has been long enough now that I
>>>>>> think we should remove EXPERT and allow users and distros to decide how
>>>>>> they want this config option set without jumping through hoops.
>>>>>
>>>>> The whole point of EXPERT is to ensure that it doesn't get turned off
>>>>> "by accident". It's a core feature, and something that more and more
>>>>> apps or libraries are relying on. It's not something I intended to ever
>>>>> go away, just like it would never go away for eg futex or epoll support.
>>>>>
>>>>
>>>> I am not arguing with that, I don't expect it will go away. I
>>>> certainly do not have an issue with it defaulting to on, and I didn't
>>>> even submit this with intention to turn it off for default Fedora. I
>>>> do think that there are cases where people might not wish it turned on
>>>> at this point in time. Hiding it behind EXPERT makes it much more
>>>> difficult than it needs to be.  There are plenty of config options
>>>> that are largely expected default and not hidden behind EXPERT.
>>>
>>> Right there are, but not really core kernel features like the ones
>>> I mentioned. Hence my argument for why it's correct as-is and I
>>> don't think it should be changed.
>>>
>>
>> Honestly, this is fair, and I understand your concerns behind it. I
>> think my real issue is that there is no simple way to override one
>> EXPERT setting without having to set them all.  It would be nice if
>> expert were a "visible if" menu, setting defaults if not selected,
>> which allows direct override with a config file. Perhaps I will try to
>> fix this in kbuild.
>>
> 
> So it turns out that untangling this in kbuild is very difficult
> without getting very unexpected results. Given the audit and security
> discussions around io_uring lately, I am inclined to believe the
> proper action is still to remove 'if EXPERT'.

I'm still going to disagree with that. In terms of security, io_uring
is in a much better spot than it was in the past. Arguably it should have
been easier to turn off when it went in, but that ship has sailed a long
time ago and the need now isn't really there imho. The lack of audit
isn't new and is something that'll be solved for the next release.

-- 
Jens Axboe


^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2021-06-03 15:47 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-26 22:34 [PATCH] io_uring: Remove CONFIG_EXPERT Justin M. Forbes
2021-05-26 22:55 ` Pavel Begunkov
2021-05-27 13:43 ` Jens Axboe
2021-05-27 14:12   ` Justin Forbes
2021-05-27 14:19     ` Jens Axboe
2021-05-27 16:01       ` Justin Forbes
2021-05-28  7:59         ` David Laight
2021-06-03 15:43         ` Justin Forbes
2021-06-03 15:46           ` Jens Axboe

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).