linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Thomas Gleixner <tglx@linutronix.de>
To: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Cc: LKML <linux-kernel@vger.kernel.org>,
	"Linus Torvalds" <torvalds@linux-foundation.org>,
	"Peter Zijlstra" <peterz@infradead.org>,
	"Christophe Leroy" <christophe.leroy@csgroup.eu>,
	"kernel test robot" <lkp@intel.com>,
	"Russell King" <linux@armlinux.org.uk>,
	linux-arm-kernel@lists.infradead.org,
	"Nathan Chancellor" <nathan@kernel.org>,
	"Darren Hart" <dvhart@infradead.org>,
	"Davidlohr Bueso" <dave@stgolabs.net>,
	"André Almeida" <andrealmeid@igalia.com>,
	x86@kernel.org, "Alexander Viro" <viro@zeniv.linux.org.uk>,
	"Christian Brauner" <brauner@kernel.org>,
	"Jan Kara" <jack@suse.cz>,
	linux-fsdevel@vger.kernel.org
Subject: Re: [patch V2 3/6] uaccess: Provide scoped masked user access regions
Date: Fri, 19 Sep 2025 11:10:48 +0200	[thread overview]
Message-ID: <87bjn6959j.ffs@tglx> (raw)
In-Reply-To: <aMwHHkaSECBDjuir@localhost.localdomain>

On Thu, Sep 18 2025 at 09:20, Mathieu Desnoyers wrote:
> On 16-Sep-2025 06:33:13 PM, Thomas Gleixner wrote:
>> These patterns beg for scopes with automatic cleanup. The resulting outcome
>> is:
>>     	scoped_masked_user_read_access(from, return -EFAULT,
>> 		scoped_get_user(val, from); );
>> 	return 0;
>
> I find a few aspects of the proposed API odd:
>
> - Explicitly implementing the error label within a macro parameter,

Which error label are you talking about?

> - Having the scoped code within another macro parameter.

Macros are limited and the whole construct requires a closed scope to
work and to keep the local variables and the jump label local.

> I would rather expect something like this to mimick our expectations
> in C:

I obviously would love to do it more C style as everyone else.
If you can come up with a way to do that in full C I'm all ears :)

> int func(void __user *ptr, size_t len, char *val1, char *val2)
> {
>         int ret;
>
>         scoped_masked_user_read_access(ptr, len, ret) {
>                 scoped_get_user(val1, ptr[0]);
>                 scoped_get_user(val2, ptr[0]);
>         }
>         return ret;
> }
>
> Where:
>
> - ptr is the pointer at the beginning of the range where the userspace
>   access will be done.

That's the case with the proposed interface already, no?

> - len is the length of the range.

The majority of use cases does not need an explicit size because the
size is determined by the data type. So not forcing everyone to write
scope(ptr, sizeof(*ptr), ..) is a good thing, no?

Adding a sized interface on top for the others is straight forward
enough.

> - ret is a variable used as output (set to -EFAULT on error, 0 on
>   success). If the user needs to do something cleverer than
>   get a -EFAULT on error, they can open-code it rather than use
>   the scoped helper.

Just write  "ret = WHATEVER" instead of "return WHATEVER".

> - The scope is presented similarly to a "for ()" loop scope.

It is a loop scope and you can exit it with either return or break.

> Now I have no clue whether preprocessor limitations prevent achieving
> this somehow, or if it would end up generating poor assembler.

There is no real good way to implement it so that the scope local
requirements are fulfilled. The only alternative would be to close the
scope with a bracket after the code:

      scope(....)
        foo(); }

or for multiline:

      scope(....) {
           ....
      ))

The lonely extra bracket screws up reading even more than the code
within the macro parameter because it's imbalanced. It also makes
tooling from editors to code checkers mightily unhappy.

Thanks,

        tglx

  reply	other threads:[~2025-09-19  9:10 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-09-16 16:33 [patch V2 0/6] uaccess: Provide and use scopes for user masked access Thomas Gleixner
2025-09-16 16:33 ` [patch V2 1/6] ARM: uaccess: Implement missing __get_user_asm_dword() Thomas Gleixner
2025-09-16 21:26   ` Russell King (Oracle)
2025-09-17  5:48     ` Thomas Gleixner
2025-09-17  9:41       ` Russell King (Oracle)
2025-09-17 12:35         ` Christophe Leroy
2025-09-17 13:55         ` Thomas Gleixner
2025-09-17 15:17           ` Russell King (Oracle)
2025-09-17 17:14             ` Nathan Chancellor
2025-09-17 17:34               ` Russell King (Oracle)
2025-09-17 19:25                 ` Thomas Gleixner
2025-09-17 18:44             ` Thomas Gleixner
2025-09-19 18:27   ` [patch V2a " Thomas Gleixner
2025-09-16 16:33 ` [patch V2 2/6] kbuild: Disable asm goto on clang < 17 Thomas Gleixner
2025-09-16 18:44   ` Nathan Chancellor
2025-09-16 20:43     ` Thomas Gleixner
2025-09-16 20:56       ` [patch V2a 2/6] kbuild: Disable CC_HAS_ASM_GOTO_OUTPUT on clang < version 17 Thomas Gleixner
2025-09-16 21:50         ` Nathan Chancellor
2025-09-29  9:38         ` Geert Uytterhoeven
2025-09-29 10:08           ` Peter Zijlstra
2025-09-29 10:58             ` Geert Uytterhoeven
2025-09-29 11:04               ` Peter Zijlstra
2025-09-29 11:10                 ` Geert Uytterhoeven
2025-09-29 15:53                   ` Linus Torvalds
2025-10-02 18:47                     ` David Laight
2025-09-29 22:05                 ` Thomas Gleixner
2025-09-16 16:33 ` [patch V2 3/6] uaccess: Provide scoped masked user access regions Thomas Gleixner
2025-09-18 13:20   ` Mathieu Desnoyers
2025-09-19  9:10     ` Thomas Gleixner [this message]
2025-09-16 16:33 ` [patch V2 4/6] futex: Convert to scoped masked user access Thomas Gleixner
2025-09-16 16:33 ` [patch V2 5/6] x86/futex: " Thomas Gleixner
2025-09-16 16:33 ` [patch V2 6/6] select: " Thomas Gleixner

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87bjn6959j.ffs@tglx \
    --to=tglx@linutronix.de \
    --cc=andrealmeid@igalia.com \
    --cc=brauner@kernel.org \
    --cc=christophe.leroy@csgroup.eu \
    --cc=dave@stgolabs.net \
    --cc=dvhart@infradead.org \
    --cc=jack@suse.cz \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@armlinux.org.uk \
    --cc=lkp@intel.com \
    --cc=mathieu.desnoyers@efficios.com \
    --cc=nathan@kernel.org \
    --cc=peterz@infradead.org \
    --cc=torvalds@linux-foundation.org \
    --cc=viro@zeniv.linux.org.uk \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).