From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jani Nikula Subject: Re: [Intel-gfx] [PATCH] drm/i915: Hold the table lock whilst walking the file's idr and counting the objects in debugfs Date: Tue, 24 Jun 2014 16:18:43 +0300 Message-ID: <87r42e8mz0.fsf@intel.com> References: <1402995384-981-1-git-send-email-chris@chris-wilson.co.uk> <20140617093502.GJ5821@phenom.ffwll.local> Mime-Version: 1.0 Content-Type: text/plain Return-path: In-Reply-To: <20140617093502.GJ5821@phenom.ffwll.local> Sender: stable-owner@vger.kernel.org To: Daniel Vetter , Chris Wilson Cc: Sam Jansen , intel-gfx@lists.freedesktop.org, stable@vger.kernel.org List-Id: intel-gfx@lists.freedesktop.org On Tue, 17 Jun 2014, Daniel Vetter wrote: > On Tue, Jun 17, 2014 at 09:56:24AM +0100, Chris Wilson wrote: >> Fixes an issue whereby we may race with the table updates (before the >> core takes the struct_mutex) and so risk dereferencing a stale pointer in >> the iterator for /debugfs/.../i915_gem_objects. For example, >> >> [ 1524.757545] BUG: unable to handle kernel paging request at f53af748 >> [ 1524.757572] IP: [] per_file_stats+0x12/0x100 >> [ 1524.757599] *pdpt = 0000000001b13001 *pde = 00000000379fb067 *pte = 80000000353af060 >> [ 1524.757621] Oops: 0000 [#1] SMP DEBUG_PAGEALLOC >> [ 1524.757637] Modules linked in: ctr ccm arc4 ath9k ath9k_common ath9k_hw ath snd_hda_codec_conexant mac80211 snd_hda_codec_generic snd_hda_intel snd_hda_controller snd_hda_codec bnep snd_hwdep rfcomm snd_pcm gpio_ich dell_wmi sparse_keymap snd_seq_midi hid_multitouch uvcvideo snd_seq_midi_event dell_laptop snd_rawmidi dcdbas snd_seq videobuf2_vmalloc videobuf2_memops videobuf2_core usbhid videodev snd_seq_device coretemp snd_timer hid joydev kvm_intel cfg80211 ath3k kvm btusb bluetooth serio_raw snd microcode soundcore lpc_ich wmi mac_hid parport_pc ppdev lp parport psmouse ahci libahci >> [ 1524.757825] CPU: 3 PID: 1911 Comm: intel-gpu-overl Tainted: G W OE 3.15.0-rc3+ #96 >> [ 1524.757840] Hardware name: Dell Inc. Inspiron 1090/Inspiron 1090, BIOS A06 08/23/2011 >> [ 1524.757855] task: f52f36c0 ti: f4cbc000 task.ti: f4cbc000 >> [ 1524.757869] EIP: 0060:[] EFLAGS: 00210202 CPU: 3 >> [ 1524.757884] EIP is at per_file_stats+0x12/0x100 >> [ 1524.757896] EAX: 0000002d EBX: 00000000 ECX: f4cbdefc EDX: f53af700 >> [ 1524.757909] ESI: c1406970 EDI: f53af700 EBP: f4cbde6c ESP: f4cbde5c >> [ 1524.757922] DS: 007b ES: 007b FS: 00d8 GS: 0033 SS: 0068 >> [ 1524.757934] CR0: 80050033 CR2: f53af748 CR3: 356af000 CR4: 000007f0 >> [ 1524.757945] Stack: >> [ 1524.757957] f4cbdefc 00000000 c1406970 f53af700 f4cbdea8 c12e5f15 f4cbdefc c1406970 >> [ 1524.757993] 0000ffff f4cbde90 0000002d f5dc5cd0 e4e80438 c1181d59 f4cbded8 f4d89900 >> [ 1524.758027] f5631b40 e5131074 c1903f37 f4cbdf28 c14068e6 f52648a0 c1927748 c1903f37 >> [ 1524.758062] Call Trace: >> [ 1524.758084] [] ? i915_gem_object_info+0x510/0x510 >> [ 1524.758106] [] idr_for_each+0xa5/0x100 >> [ 1524.758126] [] ? i915_gem_object_info+0x510/0x510 >> [ 1524.758148] [] ? seq_vprintf+0x29/0x50 >> [ 1524.758168] [] i915_gem_object_info+0x486/0x510 >> [ 1524.758189] [] seq_read+0xd6/0x380 >> [ 1524.758208] [] ? final_putname+0x1d/0x40 >> [ 1524.758227] [] ? seq_hlist_next_percpu+0x90/0x90 >> [ 1524.758246] [] vfs_read+0x82/0x150 >> [ 1524.758265] [] SyS_read+0x46/0x90 >> [ 1524.758285] [] sysenter_do_call+0x12/0x22 >> [ 1524.758298] Code: f5 8f 2a 00 83 c4 6c 31 c0 5b 5e 5f 5d c3 8d 74 26 00 8d bc 27 00 00 00 00 55 89 e5 57 56 53 83 ec 04 3e 8d 74 26 00 83 41 04 01 <8b> 42 48 01 41 08 8b 42 4c 89 d7 85 c0 75 07 8b 42 60 85 c0 74 >> [ 1524.758461] EIP: [] per_file_stats+0x12/0x100 SS:ESP 0068:f4cbde5c >> [ 1524.758485] CR2: 00000000f53af748 >> >> Reported-by: Sam Jansen >> Signed-off-by: Chris Wilson >> Cc: Sam Jansen >> Cc: stable@vger.kernel.org > > Reviewed-by: Daniel Vetter Pushed to -fixes, thanks for the patch and review. BR, Jani. >> --- >> drivers/gpu/drm/i915/i915_debugfs.c | 2 ++ >> 1 file changed, 2 insertions(+) >> >> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c >> index a8b81407338a..d2610a5f4efe 100644 >> --- a/drivers/gpu/drm/i915/i915_debugfs.c >> +++ b/drivers/gpu/drm/i915/i915_debugfs.c >> @@ -446,7 +446,9 @@ static int i915_gem_object_info(struct seq_file *m, void* data) >> >> memset(&stats, 0, sizeof(stats)); >> stats.file_priv = file->driver_priv; >> + spin_lock(&file->table_lock); >> idr_for_each(&file->object_idr, per_file_stats, &stats); >> + spin_unlock(&file->table_lock); >> /* >> * Although we have a valid reference on file->pid, that does >> * not guarantee that the task_struct who called get_pid() is >> -- >> 2.0.0 >> >> _______________________________________________ >> Intel-gfx mailing list >> Intel-gfx@lists.freedesktop.org >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx > > -- > Daniel Vetter > Software Engineer, Intel Corporation > +41 (0) 79 365 57 48 - http://blog.ffwll.ch > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Jani Nikula, Intel Open Source Technology Center