Re: [Intel-gfx] [PATCH] file, i915: fix file reference for mmap_singleton()

2023-10-26 Thread Jann Horn
On Wed, Oct 25, 2023 at 2:01 PM Christian Brauner  wrote:
> Today we got a report at [1] for rcu stalls on the i915 testsuite in [2]
> due to the conversion of files to SLAB_TYPSSAFE_BY_RCU. Afaict,
> get_file_rcu() goes into an infinite loop trying to carefully verify
> that i915->gem.mmap_singleton hasn't changed - see the splat below.
>
> So I stared at this code to figure out what it actually does. It seems
> that the i915->gem.mmap_singleton pointer itself never had rcu semantics.
>
> The i915->gem.mmap_singleton is replaced in
> file->f_op->release::singleton_release():
>
> static int singleton_release(struct inode *inode, struct file *file)
> {
> struct drm_i915_private *i915 = file->private_data;
>
> cmpxchg(>gem.mmap_singleton, file, NULL);
> drm_dev_put(>drm);
>
> return 0;
> }
>
> The cmpxchg() is ordered against a concurrent update of
> i915->gem.mmap_singleton from mmap_singleton(). IOW, when
> mmap_singleton() fails to get a reference on i915->gem.mmap_singleton
> via mmap_singleton():
>
> rcu_read_lock();
> file = get_file_rcu(>gem.mmap_singleton);
> rcu_read_unlock();
>
> mmap_singleton() allocates a new file via anon_inode_getfile() and does
>
> smp_store_mb(i915->gem.mmap_singleton, file);
>
> So, afaiu, then what happens in the case of this bug is that at some
> point fput() is called and drops the file->f_count to zero but obviously
> leaving the pointer in i915->gem.mmap_singleton in tact until
> file->f_op->release::singleton_release() is called.
>
> Now, there might be possibly larger delays until
> file->f_op->release::singleton_release() is called and
> i915->gem.mmap_singleton is set to NULL?
>
> Say concurrently another task hits mmap_singleton() and does:
>
> rcu_read_lock();
> file = get_file_rcu(>gem.mmap_singleton);
> rcu_read_unlock();
>
> When get_file_rcu() fails to get a reference via atomic_inc_not_zero()
> it will try the reload from i915->gem.mmap_singleton assuming it has
> comparable semantics as __fget_files_rcu() that also reloads on
> atomic_inc_not_zero() failure.
>
> But since i915->gem.mmap_singleton doesn't have proper rcu semantics it
> reloads the same pointer again, trying the same atomic_inc_not_zero()
> again and doing so until file->f_op->release::singleton_release() of the
> old file has been called.
>
> So, in contrast to __fget_files_rcu() here we want to not retry when
> atomic_inc_not_zero() has failed. We only want to retry in case we
> managed to get a reference but the pointer did change on reload.
[...]
> Link: [1]: 
> https://lore.kernel.org/intel-gfx/sj1pr11mb6129cb39eed831784c331bafb9...@sj1pr11mb6129.namprd11.prod.outlook.com
> Link: [2]: 
> https://intel-gfx-ci.01.org/tree/linux-next/next-20231013/bat-dg2-11/igt@i915_selftest@l...@mman.html#dmesg-warnings10963
> Signed-off-by: Christian Brauner 

Patch makes sense to me. I'm not sure why you changed EAGAIN to
EINVAL, and it's obviously a bit ugly, but it looks like a valid fix
for what the SLUB_TYPESAFE_BY_RCU conversion broke.

Reviewed-by: Jann Horn 


But as a side note, I am a bit confused about how the concurrency of
the existing code is supposed to work... it looks like two parallel
calls to mmap_singleton() can end up returning different pointers, and
then the singleton is not actually a singleton anymore? If that part
of the existing code is wrong even before the SLAB_TYPESAFE_BY_RCU
conversion, we might later have to open-code get_file_rcu() in
mmap_singleton(), so that we can do a cmpxchg at the end that checks
whether the i915->gem.mmap_singleton pointer is still the same?

Like:

static struct file *mmap_singleton(struct drm_i915_private *i915)
{
struct file *file, *new_file;

rcu_read_lock();
file = READ_ONCE(i915->gem.mmap_singleton);
if (file && atomic_long_inc_not_zero(>f_count)) {
rcu_read_unlock();
return file;
}
rcu_read_unlock();

new_file = anon_inode_getfile("i915.gem", _fops,
i915, O_RDWR);
if (IS_ERR(new_file))
return new_file;

/* Everyone shares a single global address space */
new_file->f_mapping = i915->drm.anon_inode->i_mapping;

if (try_cmpxchg(>gem.mmap_singleton, , new_file)) {
// something with drm_dev refcount ?
return new_file;
}

// something with drm_dev refcount ?
fput(new_file);

return file;
}

It would be nice to get some i915 maintainer's comment on how the
singleton stuff is supposed to work.

> ---
>
> Jann, Linus,
>
> Since you've been quite involved, can you check that what I'm babbling
> here makes sense? If this isn't the fix then I would have to drop the
> SLAB_TYPESAFE_BY_RCU conversion patch for now since I'd like to send PRs
> by the end of the week.
>
> This is on top of
> 

Re: [Intel-gfx] [PATCH] file, i915: fix file reference for mmap_singleton()

2023-10-26 Thread Tvrtko Ursulin



On 25/10/2023 16:23, Jann Horn wrote:

On Wed, Oct 25, 2023 at 2:01 PM Christian Brauner  wrote:

Today we got a report at [1] for rcu stalls on the i915 testsuite in [2]
due to the conversion of files to SLAB_TYPSSAFE_BY_RCU. Afaict,
get_file_rcu() goes into an infinite loop trying to carefully verify
that i915->gem.mmap_singleton hasn't changed - see the splat below.

So I stared at this code to figure out what it actually does. It seems
that the i915->gem.mmap_singleton pointer itself never had rcu semantics.

The i915->gem.mmap_singleton is replaced in
file->f_op->release::singleton_release():

 static int singleton_release(struct inode *inode, struct file *file)
 {
 struct drm_i915_private *i915 = file->private_data;

 cmpxchg(>gem.mmap_singleton, file, NULL);
 drm_dev_put(>drm);

 return 0;
 }

The cmpxchg() is ordered against a concurrent update of
i915->gem.mmap_singleton from mmap_singleton(). IOW, when
mmap_singleton() fails to get a reference on i915->gem.mmap_singleton
via mmap_singleton():

 rcu_read_lock();
 file = get_file_rcu(>gem.mmap_singleton);
 rcu_read_unlock();

mmap_singleton() allocates a new file via anon_inode_getfile() and does

 smp_store_mb(i915->gem.mmap_singleton, file);

So, afaiu, then what happens in the case of this bug is that at some
point fput() is called and drops the file->f_count to zero but obviously
leaving the pointer in i915->gem.mmap_singleton in tact until
file->f_op->release::singleton_release() is called.

Now, there might be possibly larger delays until
file->f_op->release::singleton_release() is called and
i915->gem.mmap_singleton is set to NULL?

Say concurrently another task hits mmap_singleton() and does:

 rcu_read_lock();
 file = get_file_rcu(>gem.mmap_singleton);
 rcu_read_unlock();

When get_file_rcu() fails to get a reference via atomic_inc_not_zero()
it will try the reload from i915->gem.mmap_singleton assuming it has
comparable semantics as __fget_files_rcu() that also reloads on
atomic_inc_not_zero() failure.

But since i915->gem.mmap_singleton doesn't have proper rcu semantics it
reloads the same pointer again, trying the same atomic_inc_not_zero()
again and doing so until file->f_op->release::singleton_release() of the
old file has been called.

So, in contrast to __fget_files_rcu() here we want to not retry when
atomic_inc_not_zero() has failed. We only want to retry in case we
managed to get a reference but the pointer did change on reload.

[...]

Link: [1]: 
https://lore.kernel.org/intel-gfx/sj1pr11mb6129cb39eed831784c331bafb9...@sj1pr11mb6129.namprd11.prod.outlook.com
Link: [2]: 
https://intel-gfx-ci.01.org/tree/linux-next/next-20231013/bat-dg2-11/igt@i915_selftest@l...@mman.html#dmesg-warnings10963
Signed-off-by: Christian Brauner 


Patch makes sense to me. I'm not sure why you changed EAGAIN to
EINVAL, and it's obviously a bit ugly, but it looks like a valid fix
for what the SLUB_TYPESAFE_BY_RCU conversion broke.

Reviewed-by: Jann Horn 


But as a side note, I am a bit confused about how the concurrency of
the existing code is supposed to work... it looks like two parallel
calls to mmap_singleton() can end up returning different pointers, and
then the singleton is not actually a singleton anymore? If that part
of the existing code is wrong even before the SLAB_TYPESAFE_BY_RCU
conversion, we might later have to open-code get_file_rcu() in
mmap_singleton(), so that we can do a cmpxchg at the end that checks
whether the i915->gem.mmap_singleton pointer is still the same?

Like:

static struct file *mmap_singleton(struct drm_i915_private *i915)
{
 struct file *file, *new_file;

 rcu_read_lock();
 file = READ_ONCE(i915->gem.mmap_singleton);
 if (file && atomic_long_inc_not_zero(>f_count)) {
 rcu_read_unlock();
 return file;
 }
 rcu_read_unlock();

 new_file = anon_inode_getfile("i915.gem", _fops,
i915, O_RDWR);
 if (IS_ERR(new_file))
 return new_file;

 /* Everyone shares a single global address space */
 new_file->f_mapping = i915->drm.anon_inode->i_mapping;

 if (try_cmpxchg(>gem.mmap_singleton, , new_file)) {
 // something with drm_dev refcount ?
 return new_file;
 }

 // something with drm_dev refcount ?
 fput(new_file);

 return file;
}

It would be nice to get some i915 maintainer's comment on how the
singleton stuff is supposed to work.


I see that the story has mostly been unraveled by now elsewhere in the 
thread, and yes, upon some historical digging and looking at the code I 
agree that the name singleton is confusing/misleading.


And although race can happen and we can end up with more than one 
anonymous inode, I don't think there is any real harm which would 
warrant complicating 

Re: [Intel-gfx] [PATCH] file, i915: fix file reference for mmap_singleton()

2023-10-25 Thread Linus Torvalds
On Wed, 25 Oct 2023 at 10:36, Christian Brauner  wrote:
>
> I did think that the loop didn't really matter for this case but since
> it seemingly does paper over the weird semantics here let's drop it.
> Anyway, pushed to:
>
> https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git/commit/?h=vfs.misc

LGTM.

We could then make the i915 mmap_singleton() do the proper loop over
the whole thing. Not quite the way Jann suggested - which would not
increment the file refcount properly, but instead of the current

smp_store_mb(i915->gem.mmap_singleton, file);
drm_dev_get(>drm);
return file;

it could do something much more complicated like

drm_dev_get(>drm);
if (cmpxchg(>gem.mmap_singleton, NULL, file) == NULL)
return file;

// mmap_singleton wasn't NULL: it might be an old one in the
// process of being torn down (with a zero refcount), or a new
// one that was just installed that we should use instead
fput(file);
file = READ_ONCE(i915->gem.mmap_singleton);
if (!file)
goto repeat;
// Is it valid? Just try again
if (atomic_read(>f_count))
goto repeat;

// We have a file pointer, but it's in the process of being torn
// down but gem.mmap_singleton hasn't been cleared yet. Yield to
// make progress.
sched_yield();
goto repeat;

which is disgusting, but would probably work.

Note the "probably work". I'm handwaving: "something like the above".

Presumably not even worth doing - I assume a correct client always
just does a single mmap() before starting work.

   Linus


Re: [Intel-gfx] [PATCH] file, i915: fix file reference for mmap_singleton()

2023-10-25 Thread Christian Brauner
On Wed, Oct 25, 2023 at 08:52:57AM -1000, Linus Torvalds wrote:
> On Wed, 25 Oct 2023 at 02:01, Christian Brauner  wrote:
> >
> > rcu_read_lock();
> > -   file = get_file_rcu(>gem.mmap_singleton);
> > +   file = get_file_rcu_once(>gem.mmap_singleton);
> > rcu_read_unlock();
> > if (file)
> > return file;
> 
> Honestly, the get_file_rcu_once() function seems a bit pointless.
> 
> The above doesn't want a loop at all. Yet your "once" still does loop,
> because "even get_file_rcu_once() is trying to deal with the whole
> "the pointer itself changed".
> 
> And the i915 code is actually designed to just depend on the atomicity
> of the mmap_singleton access itself, in how it uses
> 
> cmpxchg(>gem.mmap_singleton, file, NULL);
> ...
> file = READ_ONCE(i915->gem.mmap_singleton);
> 
> and literally says "I'll remove my singleton as it is released". The
> important part there is that the 'map_singleton' pointer itself isn't
> actually reference-counted - it's the reverse, where
> reference-counting *other* instances will then auto-clear it.
> 
> And that's what then makes that get_file_rcu() model not work with it,
> because get_file_rcu() kind of assumes that the argument it gets is
> *part* of the reference counting, not a cached *result* of the
> reference counting that gets cleared as a result of the ref going down
> to zero.
> 
> I may explain my objections badly, but hopefully you get what I mean.

No, I get it. I was on the fence how to deal with this because I
honestly find this model here extremely weird and very unintuitive to
begin with with the pointer being NULLed during release and replaced
that way.

> 
> And I think that also means that using that new get_file_rcu_once()
> may be hiding the actual problem, but it's still conceptually wrong,
> because it still has that conceptual model of "the pointer I'm getting
> is part of the reference counting", when it really isn't.
> 
> So I think we'd actually be better off with something that is more
> clearly different from get_file_rcu() in naming, so make that
> conceptual difference clearer. Make it be something like
> "get_active_file(struct file **)", and make the implementation be just
> exactly what your current __get_file_rcu() is with no loops at all.
> 
> Then thew i915 code ends up being
> 
> rcu_read_lock();
> file = get_active_file(>gem.mmap_singleton);
> rcu_read_unlock();
> 
> if (!IS_ERR_OR_NULL(file))
> return file;
> 
>.. create new mmap_singleton ..
> 
> and that's it.
> 
> If you don't want to expose __get_file_rcu() as-is, you could maybe just do
> 
>   struct file *get_active_file(struct file **fp)
>   {
> struct file *file;
> rcu_read_lock();
> file = __get_file_rcu(fp);
> rcu_read_unlock();
> return file;
>   }
> 
> and then the i916 code would just drop the RCU locking that it has no
> business even knowing about.

Yeah, fair idea. I just want this fixed so we don't have to drop. Pushed
to vfs.misc with your suggested changes for get_file_active() with rcu
hidden in that function. Double-check and yell if something looks wrong:

> The old i915 code is already racy, in that it's called a "singleton",
> but if you have multiple concurrent callers to mmap_singleton(), they
> all might see a NULL file at first, and then they all create
> *separate* new "singleton" files, and they *all* do that
> 
> smp_store_mb(i915->gem.mmap_singleton, file);
> 
> and one random case of them happens to win the race and set *its* file
> as "THE singleton" file.

Yeah, I think that's all really weird but whatever.

> Am I missing something?

No, I don't think so. I thought you might have a good opinion here.

I did think that the loop didn't really matter for this case but since
it seemingly does paper over the weird semantics here let's drop it.
Anyway, pushed to:

https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git/commit/?h=vfs.misc

and appended here. Please yell, if something's still off.
>From 61d4fb0b349ec1b33119913c3b0bd109de30142c Mon Sep 17 00:00:00 2001
From: Christian Brauner 
Date: Wed, 25 Oct 2023 12:14:37 +0200
Subject: [PATCH] file, i915: fix file reference for mmap_singleton()

Today we got a report at [1] for rcu stalls on the i915 testsuite in [2]
due to the conversion of files to SLAB_TYPSSAFE_BY_RCU. Afaict,
get_file_rcu() goes into an infinite loop trying to carefully verify
that i915->gem.mmap_singleton hasn't changed - see the splat below.

So I stared at this code to figure out what it actually does. It seems
that the i915->gem.mmap_singleton pointer itself never had rcu semantics.

The i915->gem.mmap_singleton is replaced in
file->f_op->release::singleton_release():

static int singleton_release(struct inode *inode, struct file *file)
{
struct drm_i915_private *i915 = file->private_data;


Re: [Intel-gfx] [PATCH] file, i915: fix file reference for mmap_singleton()

2023-10-25 Thread Linus Torvalds
On Wed, 25 Oct 2023 at 02:01, Christian Brauner  wrote:
>
> rcu_read_lock();
> -   file = get_file_rcu(>gem.mmap_singleton);
> +   file = get_file_rcu_once(>gem.mmap_singleton);
> rcu_read_unlock();
> if (file)
> return file;

Honestly, the get_file_rcu_once() function seems a bit pointless.

The above doesn't want a loop at all. Yet your "once" still does loop,
because "even get_file_rcu_once() is trying to deal with the whole
"the pointer itself changed".

And the i915 code is actually designed to just depend on the atomicity
of the mmap_singleton access itself, in how it uses

cmpxchg(>gem.mmap_singleton, file, NULL);
...
file = READ_ONCE(i915->gem.mmap_singleton);

and literally says "I'll remove my singleton as it is released". The
important part there is that the 'map_singleton' pointer itself isn't
actually reference-counted - it's the reverse, where
reference-counting *other* instances will then auto-clear it.

And that's what then makes that get_file_rcu() model not work with it,
because get_file_rcu() kind of assumes that the argument it gets is
*part* of the reference counting, not a cached *result* of the
reference counting that gets cleared as a result of the ref going down
to zero.

I may explain my objections badly, but hopefully you get what I mean.

And I think that also means that using that new get_file_rcu_once()
may be hiding the actual problem, but it's still conceptually wrong,
because it still has that conceptual model of "the pointer I'm getting
is part of the reference counting", when it really isn't.

So I think we'd actually be better off with something that is more
clearly different from get_file_rcu() in naming, so make that
conceptual difference clearer. Make it be something like
"get_active_file(struct file **)", and make the implementation be just
exactly what your current __get_file_rcu() is with no loops at all.

Then thew i915 code ends up being

rcu_read_lock();
file = get_active_file(>gem.mmap_singleton);
rcu_read_unlock();

if (!IS_ERR_OR_NULL(file))
return file;

   .. create new mmap_singleton ..

and that's it.

If you don't want to expose __get_file_rcu() as-is, you could maybe just do

  struct file *get_active_file(struct file **fp)
  {
struct file *file;
rcu_read_lock();
file = __get_file_rcu(fp);
rcu_read_unlock();
return file;
  }

and then the i916 code would just drop the RCU locking that it has no
business even knowing about.

I realize that my complaints are a bit conceptual, and that the
practical end result is pretty much the same, but I do think that it
is worth noting this conceptual difference between "file pointer is
ref-counted" and "file counter is *controlled* by ref-counting".

Add a comment to the effect at get_active_file() users.

The old i915 code is already racy, in that it's called a "singleton",
but if you have multiple concurrent callers to mmap_singleton(), they
all might see a NULL file at first, and then they all create
*separate* new "singleton" files, and they *all* do that

smp_store_mb(i915->gem.mmap_singleton, file);

and one random case of them happens to win the race and set *its* file
as "THE singleton" file.

So your "let's loop if it changes" is not fixing anything as-is, and
it's just actually hiding what is going on.

If the i915 code wants to be consistent and really have just one
singleton, it needs to do the looping with some cmpxchg or whatever
itself. Doing the loop in some get_file_rcu_once() function for when
the file pointer changed isn't going to fix the race.

Am I missing something?

  Linus


[Intel-gfx] [PATCH] file, i915: fix file reference for mmap_singleton()

2023-10-25 Thread Christian Brauner
Today we got a report at [1] for rcu stalls on the i915 testsuite in [2]
due to the conversion of files to SLAB_TYPSSAFE_BY_RCU. Afaict,
get_file_rcu() goes into an infinite loop trying to carefully verify
that i915->gem.mmap_singleton hasn't changed - see the splat below.

So I stared at this code to figure out what it actually does. It seems
that the i915->gem.mmap_singleton pointer itself never had rcu semantics.

The i915->gem.mmap_singleton is replaced in
file->f_op->release::singleton_release():

static int singleton_release(struct inode *inode, struct file *file)
{
struct drm_i915_private *i915 = file->private_data;

cmpxchg(>gem.mmap_singleton, file, NULL);
drm_dev_put(>drm);

return 0;
}

The cmpxchg() is ordered against a concurrent update of
i915->gem.mmap_singleton from mmap_singleton(). IOW, when
mmap_singleton() fails to get a reference on i915->gem.mmap_singleton
via mmap_singleton():

rcu_read_lock();
file = get_file_rcu(>gem.mmap_singleton);
rcu_read_unlock();

mmap_singleton() allocates a new file via anon_inode_getfile() and does

smp_store_mb(i915->gem.mmap_singleton, file);

So, afaiu, then what happens in the case of this bug is that at some
point fput() is called and drops the file->f_count to zero but obviously
leaving the pointer in i915->gem.mmap_singleton in tact until
file->f_op->release::singleton_release() is called.

Now, there might be possibly larger delays until
file->f_op->release::singleton_release() is called and
i915->gem.mmap_singleton is set to NULL?

Say concurrently another task hits mmap_singleton() and does:

rcu_read_lock();
file = get_file_rcu(>gem.mmap_singleton);
rcu_read_unlock();

When get_file_rcu() fails to get a reference via atomic_inc_not_zero()
it will try the reload from i915->gem.mmap_singleton assuming it has
comparable semantics as __fget_files_rcu() that also reloads on
atomic_inc_not_zero() failure.

But since i915->gem.mmap_singleton doesn't have proper rcu semantics it
reloads the same pointer again, trying the same atomic_inc_not_zero()
again and doing so until file->f_op->release::singleton_release() of the
old file has been called.

So, in contrast to __fget_files_rcu() here we want to not retry when
atomic_inc_not_zero() has failed. We only want to retry in case we
managed to get a reference but the pointer did change on reload.

<3> [511.395679] rcu: INFO: rcu_preempt detected stalls on CPUs/tasks:
<3> [511.395716] rcu:   Tasks blocked on level-1 rcu_node (CPUs 0-9): P6238
<3> [511.395934] rcu:   (detected by 16, t=65002 jiffies, g=123977, q=439 
ncpus=20)
<6> [511.395944] task:i915_selftest   state:R  running task stack:10568 
pid:6238  tgid:6238  ppid:1001   flags:0x4002
<6> [511.395962] Call Trace:
<6> [511.395966]  
<6> [511.395974]  ? __schedule+0x3a8/0xd70
<6> [511.395995]  ? asm_sysvec_apic_timer_interrupt+0x1a/0x20
<6> [511.396003]  ? lockdep_hardirqs_on+0xc3/0x140
<6> [511.396013]  ? asm_sysvec_apic_timer_interrupt+0x1a/0x20
<6> [511.396029]  ? get_file_rcu+0x10/0x30
<6> [511.396039]  ? get_file_rcu+0x10/0x30
<6> [511.396046]  ? i915_gem_object_mmap+0xbc/0x450 [i915]
<6> [511.396509]  ? i915_gem_mmap+0x272/0x480 [i915]
<6> [511.396903]  ? mmap_region+0x253/0xb60
<6> [511.396925]  ? do_mmap+0x334/0x5c0
<6> [511.396939]  ? vm_mmap_pgoff+0x9f/0x1c0
<6> [511.396949]  ? rcu_is_watching+0x11/0x50
<6> [511.396962]  ? igt_mmap_offset+0xfc/0x110 [i915]
<6> [511.397376]  ? __igt_mmap+0xb3/0x570 [i915]
<6> [511.397762]  ? igt_mmap+0x11e/0x150 [i915]
<6> [511.398139]  ? __trace_bprintk+0x76/0x90
<6> [511.398156]  ? __i915_subtests+0xbf/0x240 [i915]
<6> [511.398586]  ? __pfx___i915_live_setup+0x10/0x10 [i915]
<6> [511.399001]  ? __pfx___i915_live_teardown+0x10/0x10 [i915]
<6> [511.399433]  ? __run_selftests+0xbc/0x1a0 [i915]
<6> [511.399875]  ? i915_live_selftests+0x4b/0x90 [i915]
<6> [511.400308]  ? i915_pci_probe+0x106/0x200 [i915]
<6> [511.400692]  ? pci_device_probe+0x95/0x120
<6> [511.400704]  ? really_probe+0x164/0x3c0
<6> [511.400715]  ? __pfx___driver_attach+0x10/0x10
<6> [511.400722]  ? __driver_probe_device+0x73/0x160
<6> [511.400731]  ? driver_probe_device+0x19/0xa0
<6> [511.400741]  ? __driver_attach+0xb6/0x180
<6> [511.400749]  ? __pfx___driver_attach+0x10/0x10
<6> [511.400756]  ? bus_for_each_dev+0x77/0xd0
<6> [511.400770]  ? bus_add_driver+0x114/0x210
<6> [511.400781]  ? driver_register+0x5b/0x110
<6> [511.400791]  ? i915_init+0x23/0xc0 [i915]
<6> [511.401153]  ? __pfx_i915_init+0x10/0x10 [i915]
<6> [511.401503]  ? do_one_initcall+0x57/0x270
<6> [511.401515]  ? rcu_is_watching+0x11/0x50
<6> [511.401521]  ? kmalloc_trace+0xa3/0xb0
<6> [511.401532]  ? do_init_module+0x5f/0x210
<6> [511.401544]  ? load_module+0x1d00/0x1f60
<6> [511.401581]  ? init_module_from_file+0x86/0xd0
<6> [511.401590]  ? init_module_from_file+0x86/0xd0
<6> [511.401613]  ? idempotent_init_module+0x17c/0x230