Re: [PATCH v10 1/8] sched/core: Add __might_sleep_precision()

[Boqun Feng <boqun.feng@gmail.com>]

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