Add __might_sleep_precision(), Rust friendly version of
__might_sleep(), which takes a pointer to a string with the length
instead of a null-terminated string.

Rust's core::panic::Location::file(), which gives the file name of a
caller, doesn't provide a null-terminated
string. __might_sleep_precision() uses a precision specifier in the
printk format, which specifies the length of a string; a string
doesn't need to be a null-terminated.

Modify __might_sleep() to call __might_sleep_precision() but the
impact should be negligible. strlen() isn't called in a normal case;
it's called only when printing the error (sleeping function called
from invalid context).

Note that Location::file() providing a null-terminated string for
better C interoperability is under discussion [1].

Link: https://github.com/rust-lang/libs-team/issues/466 [1]
Reviewed-by: Alice Ryhl <aliceryhl@google.com>
Co-developed-by: Boqun Feng <boqun.feng@gmail.com>
Signed-off-by: Boqun Feng <boqun.feng@gmail.com>
Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>
---
 include/linux/kernel.h |  2 ++
 kernel/sched/core.c    | 55 ++++++++++++++++++++++++++----------------
 2 files changed, 36 insertions(+), 21 deletions(-)

diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index be2e8c0a187e..086ee1dc447e 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -87,6 +87,7 @@ extern int dynamic_might_resched(void);
 #ifdef CONFIG_DEBUG_ATOMIC_SLEEP
 extern void __might_resched(const char *file, int line, unsigned int offsets);
 extern void __might_sleep(const char *file, int line);
+extern void __might_sleep_precision(const char *file, int len, int line);
 extern void __cant_sleep(const char *file, int line, int preempt_offset);
 extern void __cant_migrate(const char *file, int line);
 
@@ -145,6 +146,7 @@ extern void __cant_migrate(const char *file, int line);
   static inline void __might_resched(const char *file, int line,
 				     unsigned int offsets) { }
 static inline void __might_sleep(const char *file, int line) { }
+static inline void __might_sleep_precision(const char *file, int len, int line) { }
 # define might_sleep() do { might_resched(); } while (0)
 # define cant_sleep() do { } while (0)
 # define cant_migrate()		do { } while (0)
diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 165c90ba64ea..d308f2a8692e 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -8678,24 +8678,6 @@ void __init sched_init(void)
 
 #ifdef CONFIG_DEBUG_ATOMIC_SLEEP
 
-void __might_sleep(const char *file, int line)
-{
-	unsigned int state = get_current_state();
-	/*
-	 * Blocking primitives will set (and therefore destroy) current->state,
-	 * since we will exit with TASK_RUNNING make sure we enter with it,
-	 * otherwise we will destroy state.
-	 */
-	WARN_ONCE(state != TASK_RUNNING && current->task_state_change,
-			"do not call blocking ops when !TASK_RUNNING; "
-			"state=%x set at [<%p>] %pS\n", state,
-			(void *)current->task_state_change,
-			(void *)current->task_state_change);
-
-	__might_resched(file, line, 0);
-}
-EXPORT_SYMBOL(__might_sleep);
-
 static void print_preempt_disable_ip(int preempt_offset, unsigned long ip)
 {
 	if (!IS_ENABLED(CONFIG_DEBUG_PREEMPT))
@@ -8717,7 +8699,8 @@ static inline bool resched_offsets_ok(unsigned int offsets)
 	return nested == offsets;
 }
 
-void __might_resched(const char *file, int line, unsigned int offsets)
+static void __might_resched_precision(const char *file, int len, int line,
+				      unsigned int offsets)
 {
 	/* Ratelimiting timestamp: */
 	static unsigned long prev_jiffy;
@@ -8740,8 +8723,10 @@ void __might_resched(const char *file, int line, unsigned int offsets)
 	/* Save this before calling printk(), since that will clobber it: */
 	preempt_disable_ip = get_preempt_disable_ip(current);
 
-	pr_err("BUG: sleeping function called from invalid context at %s:%d\n",
-	       file, line);
+	if (len < 0)
+		len = strlen(file);
+	pr_err("BUG: sleeping function called from invalid context at %.*s:%d\n",
+	       len, file, line);
 	pr_err("in_atomic(): %d, irqs_disabled(): %d, non_block: %d, pid: %d, name: %s\n",
 	       in_atomic(), irqs_disabled(), current->non_block_count,
 	       current->pid, current->comm);
@@ -8766,8 +8751,36 @@ void __might_resched(const char *file, int line, unsigned int offsets)
 	dump_stack();
 	add_taint(TAINT_WARN, LOCKDEP_STILL_OK);
 }
+
+void __might_resched(const char *file, int line, unsigned int offsets)
+{
+	__might_resched_precision(file, -1, line, offsets);
+}
 EXPORT_SYMBOL(__might_resched);
 
+void __might_sleep_precision(const char *file, int len, int line)
+{
+	unsigned int state = get_current_state();
+	/*
+	 * Blocking primitives will set (and therefore destroy) current->state,
+	 * since we will exit with TASK_RUNNING make sure we enter with it,
+	 * otherwise we will destroy state.
+	 */
+	WARN_ONCE(state != TASK_RUNNING && current->task_state_change,
+			"do not call blocking ops when !TASK_RUNNING; "
+			"state=%x set at [<%p>] %pS\n", state,
+			(void *)current->task_state_change,
+			(void *)current->task_state_change);
+
+	__might_resched_precision(file, len, line, 0);
+}
+
+void __might_sleep(const char *file, int line)
+{
+	__might_sleep_precision(file, -1, line);
+}
+EXPORT_SYMBOL(__might_sleep);
+
 void __cant_sleep(const char *file, int line, int preempt_offset)
 {
 	static unsigned long prev_jiffy;
-- 
2.43.0

On Fri,  7 Feb 2025 22:26:16 +0900
FUJITA Tomonori <fujita.tomonori@gmail.com> wrote:

> Add __might_sleep_precision(), Rust friendly version of
> __might_sleep(), which takes a pointer to a string with the length
> instead of a null-terminated string.
> 
> Rust's core::panic::Location::file(), which gives the file name of a
> caller, doesn't provide a null-terminated
> string. __might_sleep_precision() uses a precision specifier in the
> printk format, which specifies the length of a string; a string
> doesn't need to be a null-terminated.
> 
> Modify __might_sleep() to call __might_sleep_precision() but the
> impact should be negligible. strlen() isn't called in a normal case;
> it's called only when printing the error (sleeping function called
> from invalid context).
> 
> Note that Location::file() providing a null-terminated string for
> better C interoperability is under discussion [1].
> 
> Link: https://github.com/rust-lang/libs-team/issues/466 [1]
> Reviewed-by: Alice Ryhl <aliceryhl@google.com>
> Co-developed-by: Boqun Feng <boqun.feng@gmail.com>
> Signed-off-by: Boqun Feng <boqun.feng@gmail.com>
> Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>
> ---
>  include/linux/kernel.h |  2 ++
>  kernel/sched/core.c    | 55 ++++++++++++++++++++++++++----------------
>  2 files changed, 36 insertions(+), 21 deletions(-)
> 
> diff --git a/include/linux/kernel.h b/include/linux/kernel.h
> index be2e8c0a187e..086ee1dc447e 100644
> --- a/include/linux/kernel.h
> +++ b/include/linux/kernel.h
> @@ -87,6 +87,7 @@ extern int dynamic_might_resched(void);
>  #ifdef CONFIG_DEBUG_ATOMIC_SLEEP
>  extern void __might_resched(const char *file, int line, unsigned int offsets);
>  extern void __might_sleep(const char *file, int line);
> +extern void __might_sleep_precision(const char *file, int len, int line);
>  extern void __cant_sleep(const char *file, int line, int preempt_offset);
>  extern void __cant_migrate(const char *file, int line);
>  
> @@ -145,6 +146,7 @@ extern void __cant_migrate(const char *file, int line);
>    static inline void __might_resched(const char *file, int line,
>  				     unsigned int offsets) { }
>  static inline void __might_sleep(const char *file, int line) { }
> +static inline void __might_sleep_precision(const char *file, int len, int line) { }
>  # define might_sleep() do { might_resched(); } while (0)
>  # define cant_sleep() do { } while (0)
>  # define cant_migrate()		do { } while (0)
> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> index 165c90ba64ea..d308f2a8692e 100644
> --- a/kernel/sched/core.c
> +++ b/kernel/sched/core.c
> @@ -8678,24 +8678,6 @@ void __init sched_init(void)
>  
>  #ifdef CONFIG_DEBUG_ATOMIC_SLEEP
>  
> -void __might_sleep(const char *file, int line)
> -{
> -	unsigned int state = get_current_state();
> -	/*
> -	 * Blocking primitives will set (and therefore destroy) current->state,
> -	 * since we will exit with TASK_RUNNING make sure we enter with it,
> -	 * otherwise we will destroy state.
> -	 */
> -	WARN_ONCE(state != TASK_RUNNING && current->task_state_change,
> -			"do not call blocking ops when !TASK_RUNNING; "
> -			"state=%x set at [<%p>] %pS\n", state,
> -			(void *)current->task_state_change,
> -			(void *)current->task_state_change);
> -
> -	__might_resched(file, line, 0);
> -}
> -EXPORT_SYMBOL(__might_sleep);
> -
>  static void print_preempt_disable_ip(int preempt_offset, unsigned long ip)
>  {
>  	if (!IS_ENABLED(CONFIG_DEBUG_PREEMPT))
> @@ -8717,7 +8699,8 @@ static inline bool resched_offsets_ok(unsigned int offsets)
>  	return nested == offsets;
>  }
>  
> -void __might_resched(const char *file, int line, unsigned int offsets)
> +static void __might_resched_precision(const char *file, int len, int line,

For clarity that ought to be file_len.

> +				      unsigned int offsets)
>  {
>  	/* Ratelimiting timestamp: */
>  	static unsigned long prev_jiffy;
> @@ -8740,8 +8723,10 @@ void __might_resched(const char *file, int line, unsigned int offsets)
>  	/* Save this before calling printk(), since that will clobber it: */
>  	preempt_disable_ip = get_preempt_disable_ip(current);
>  
> -	pr_err("BUG: sleeping function called from invalid context at %s:%d\n",
> -	       file, line);
> +	if (len < 0)
> +		len = strlen(file);

No need for strlen(), just use a big number instead of -1.
Anything bigger than a sane upper limit on the filename length will do.

	David

> +	pr_err("BUG: sleeping function called from invalid context at %.*s:%d\n",
> +	       len, file, line);
>  	pr_err("in_atomic(): %d, irqs_disabled(): %d, non_block: %d, pid: %d, name: %s\n",
>  	       in_atomic(), irqs_disabled(), current->non_block_count,
>  	       current->pid, current->comm);
> @@ -8766,8 +8751,36 @@ void __might_resched(const char *file, int line, unsigned int offsets)
>  	dump_stack();
>  	add_taint(TAINT_WARN, LOCKDEP_STILL_OK);
>  }
> +
> +void __might_resched(const char *file, int line, unsigned int offsets)
> +{
> +	__might_resched_precision(file, -1, line, offsets);
> +}
>  EXPORT_SYMBOL(__might_resched);
>  
> +void __might_sleep_precision(const char *file, int len, int line)
> +{
> +	unsigned int state = get_current_state();
> +	/*
> +	 * Blocking primitives will set (and therefore destroy) current->state,
> +	 * since we will exit with TASK_RUNNING make sure we enter with it,
> +	 * otherwise we will destroy state.
> +	 */
> +	WARN_ONCE(state != TASK_RUNNING && current->task_state_change,
> +			"do not call blocking ops when !TASK_RUNNING; "
> +			"state=%x set at [<%p>] %pS\n", state,
> +			(void *)current->task_state_change,
> +			(void *)current->task_state_change);
> +
> +	__might_resched_precision(file, len, line, 0);
> +}
> +
> +void __might_sleep(const char *file, int line)
> +{
> +	__might_sleep_precision(file, -1, line);
> +}
> +EXPORT_SYMBOL(__might_sleep);
> +
>  void __cant_sleep(const char *file, int line, int preempt_offset)
>  {
>  	static unsigned long prev_jiffy;

On Fri, 7 Feb 2025 18:12:58 +0000
David Laight <david.laight.linux@gmail.com> wrote:

>>  static void print_preempt_disable_ip(int preempt_offset, unsigned long ip)
>>  {
>>  	if (!IS_ENABLED(CONFIG_DEBUG_PREEMPT))
>> @@ -8717,7 +8699,8 @@ static inline bool resched_offsets_ok(unsigned int offsets)
>>  	return nested == offsets;
>>  }
>>  
>> -void __might_resched(const char *file, int line, unsigned int offsets)
>> +static void __might_resched_precision(const char *file, int len, int line,
> 
> For clarity that ought to be file_len.

Yeah, I'll update.

>> +				      unsigned int offsets)
>>  {
>>  	/* Ratelimiting timestamp: */
>>  	static unsigned long prev_jiffy;
>> @@ -8740,8 +8723,10 @@ void __might_resched(const char *file, int line, unsigned int offsets)
>>  	/* Save this before calling printk(), since that will clobber it: */
>>  	preempt_disable_ip = get_preempt_disable_ip(current);
>>  
>> -	pr_err("BUG: sleeping function called from invalid context at %s:%d\n",
>> -	       file, line);
>> +	if (len < 0)
>> +		len = strlen(file);
> 
> No need for strlen(), just use a big number instead of -1.
> Anything bigger than a sane upper limit on the filename length will do.

Ah, that's right. Just passing the maximum precision (1<<15-1) works.

The precision specifies the maximum length. vsnprintf() always
iterates through a string until it reaches the maximum length or
encounters the null terminator. So strlen() here is useless.

Alice and Boqun, the above change is fine? Can I keep the tags?

On Sat, Feb 8, 2025 at 4:01 AM FUJITA Tomonori
<fujita.tomonori@gmail.com> wrote:
>
> On Fri, 7 Feb 2025 18:12:58 +0000
> David Laight <david.laight.linux@gmail.com> wrote:
>
> >>  static void print_preempt_disable_ip(int preempt_offset, unsigned long ip)
> >>  {
> >>      if (!IS_ENABLED(CONFIG_DEBUG_PREEMPT))
> >> @@ -8717,7 +8699,8 @@ static inline bool resched_offsets_ok(unsigned int offsets)
> >>      return nested == offsets;
> >>  }
> >>
> >> -void __might_resched(const char *file, int line, unsigned int offsets)
> >> +static void __might_resched_precision(const char *file, int len, int line,
> >
> > For clarity that ought to be file_len.
>
> Yeah, I'll update.
>
> >> +                                  unsigned int offsets)
> >>  {
> >>      /* Ratelimiting timestamp: */
> >>      static unsigned long prev_jiffy;
> >> @@ -8740,8 +8723,10 @@ void __might_resched(const char *file, int line, unsigned int offsets)
> >>      /* Save this before calling printk(), since that will clobber it: */
> >>      preempt_disable_ip = get_preempt_disable_ip(current);
> >>
> >> -    pr_err("BUG: sleeping function called from invalid context at %s:%d\n",
> >> -           file, line);
> >> +    if (len < 0)
> >> +            len = strlen(file);
> >
> > No need for strlen(), just use a big number instead of -1.
> > Anything bigger than a sane upper limit on the filename length will do.
>
> Ah, that's right. Just passing the maximum precision (1<<15-1) works.
>
> The precision specifies the maximum length. vsnprintf() always
> iterates through a string until it reaches the maximum length or
> encounters the null terminator. So strlen() here is useless.
>
> Alice and Boqun, the above change is fine? Can I keep the tags?

I'd probably like a comment explaining the meaning of this constant
somewhere, but sure ok with me.

Alice

On Mon, Feb 10, 2025 at 10:41:00AM +0100, Alice Ryhl wrote:
> On Sat, Feb 8, 2025 at 4:01 AM FUJITA Tomonori
> <fujita.tomonori@gmail.com> wrote:
> >
> > On Fri, 7 Feb 2025 18:12:58 +0000
> > David Laight <david.laight.linux@gmail.com> wrote:
> >
> > >>  static void print_preempt_disable_ip(int preempt_offset, unsigned long ip)
> > >>  {
> > >>      if (!IS_ENABLED(CONFIG_DEBUG_PREEMPT))
> > >> @@ -8717,7 +8699,8 @@ static inline bool resched_offsets_ok(unsigned int offsets)
> > >>      return nested == offsets;
> > >>  }
> > >>
> > >> -void __might_resched(const char *file, int line, unsigned int offsets)
> > >> +static void __might_resched_precision(const char *file, int len, int line,
> > >
> > > For clarity that ought to be file_len.
> >
> > Yeah, I'll update.
> >
> > >> +                                  unsigned int offsets)
> > >>  {
> > >>      /* Ratelimiting timestamp: */
> > >>      static unsigned long prev_jiffy;
> > >> @@ -8740,8 +8723,10 @@ void __might_resched(const char *file, int line, unsigned int offsets)
> > >>      /* Save this before calling printk(), since that will clobber it: */
> > >>      preempt_disable_ip = get_preempt_disable_ip(current);
> > >>
> > >> -    pr_err("BUG: sleeping function called from invalid context at %s:%d\n",
> > >> -           file, line);
> > >> +    if (len < 0)
> > >> +            len = strlen(file);
> > >
> > > No need for strlen(), just use a big number instead of -1.
> > > Anything bigger than a sane upper limit on the filename length will do.
> >
> > Ah, that's right. Just passing the maximum precision (1<<15-1) works.
> >
> > The precision specifies the maximum length. vsnprintf() always
> > iterates through a string until it reaches the maximum length or
> > encounters the null terminator. So strlen() here is useless.
> >
> > Alice and Boqun, the above change is fine? Can I keep the tags?
> 
> I'd probably like a comment explaining the meaning of this constant
> somewhere, but sure ok with me.
> 

Agreed. The code should be fine but need some comments.

Regards,
Boqun

> Alice

On Sun, 16 Feb 2025 17:51:32 -0800
Boqun Feng <boqun.feng@gmail.com> wrote:

>> > >> +                                  unsigned int offsets)
>> > >>  {
>> > >>      /* Ratelimiting timestamp: */
>> > >>      static unsigned long prev_jiffy;
>> > >> @@ -8740,8 +8723,10 @@ void __might_resched(const char *file, int line, unsigned int offsets)
>> > >>      /* Save this before calling printk(), since that will clobber it: */
>> > >>      preempt_disable_ip = get_preempt_disable_ip(current);
>> > >>
>> > >> -    pr_err("BUG: sleeping function called from invalid context at %s:%d\n",
>> > >> -           file, line);
>> > >> +    if (len < 0)
>> > >> +            len = strlen(file);
>> > >
>> > > No need for strlen(), just use a big number instead of -1.
>> > > Anything bigger than a sane upper limit on the filename length will do.
>> >
>> > Ah, that's right. Just passing the maximum precision (1<<15-1) works.
>> >
>> > The precision specifies the maximum length. vsnprintf() always
>> > iterates through a string until it reaches the maximum length or
>> > encounters the null terminator. So strlen() here is useless.
>> >
>> > Alice and Boqun, the above change is fine? Can I keep the tags?
>> 
>> I'd probably like a comment explaining the meaning of this constant
>> somewhere, but sure ok with me.
>> 
> 
> Agreed. The code should be fine but need some comments.

Of course, I'll add some in the next version.

Thanks!