From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752733AbdKMWY0 (ORCPT ); Mon, 13 Nov 2017 17:24:26 -0500 Received: from mail-yw0-f195.google.com ([209.85.161.195]:47214 "EHLO mail-yw0-f195.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751350AbdKMWYZ (ORCPT ); Mon, 13 Nov 2017 17:24:25 -0500 X-Google-Smtp-Source: AGs4zMb3/B3X4230KUNvw1PzL9Yt+YuAeybnLFxd5SGV8bufpcuonftxt572YD8bBtuDLwuM+Aib46GTBBAJLJHo0lE= MIME-Version: 1.0 In-Reply-To: <20171113220109.ibgsydspaj73l5d5@dtor-ws> References: <9629c610-26cf-d64d-8351-b90273b09676@infradead.org> <6634ab5b-b21f-120e-2a29-52c824d5dcff@infradead.org> <20171113190400.hayy4iff5wmjy3qt@dtor-ws> <20171113192317.GA27109@kroah.com> <20171113220109.ibgsydspaj73l5d5@dtor-ws> From: Guenter Roeck Date: Mon, 13 Nov 2017 14:24:23 -0800 Message-ID: Subject: Re: drivers/firmware/google/vpd.c: duplicate sysfs file To: Dmitry Torokhov Cc: Greg Kroah-Hartman , Randy Dunlap , LKML , Guenter Roeck , Wei-Ning Huang Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, Nov 13, 2017 at 2:01 PM, Dmitry Torokhov wrote: > On Mon, Nov 13, 2017 at 12:14:41PM -0800, Guenter Roeck wrote: >> The sysfs warning, yes. However, after unbinding and rebinding the >> driver, "cat /sys/firmware/vpd/rw_raw" will result in a crash. >> >> Sequence: >> >> echo vpd > /sys/bus/platform/drivers/vpd/unbind >> echo vpd > /sys/bus/platform/drivers/vpd/bind # <-- nasty message >> >> cat /sys/firmware/vpd/rw_raw # <-- crash > > I'd disable unbind (.suppress_bind_attrs = true) and commented out > unload for now. And then looked into fixing properly. > Fixing properly is easy as long as we retain the platform device; I already have a set of patches doing just that. I have no idea how to implement the non-platform-device variant, if that is what you mean with "properly". Guenter >> >> Guenter >> >> On Mon, Nov 13, 2017 at 11:23 AM, Greg Kroah-Hartman >> wrote: >> > On Mon, Nov 13, 2017 at 11:09:21AM -0800, Guenter Roeck wrote: >> >> On Mon, Nov 13, 2017 at 11:04 AM, Dmitry Torokhov >> >> wrote: >> >> > >> >> > On Mon, Nov 13, 2017 at 10:18:35AM -0800, Randy Dunlap wrote: >> >> > > On 11/13/2017 06:41 AM, Guenter Roeck wrote: >> >> > > > >> >> > > > >> >> > > > On Sun, Nov 12, 2017 at 10:36 PM, Randy Dunlap > wrote: >> >> > > > >> >> > > > sysfs: cannot create duplicate filename '/devices/platform/vpd' >> >> > > > >> >> > > > on the second load of this driver. I.e., >> >> > > > >> >> > > > modprobe vpd-sysfs >> >> > > > rmmod vpd-sysfs >> >> > > > modprobe vpd-sysfs >> >> > > > [boom] >> >> > > > >> >> > > > Neither the platform device nor the platform driver driver are ever unregistered, so this isn't entirely surprising. I'll try to reproduce and send a patch. >> >> > > >> >> > > >> >> > > Seems to be a common theme: >> >> > > >> >> > > google> grep --color=never "platform.*register" *.c >> >> > > coreboot_table-acpi.c: return platform_driver_register(&coreboot_table_acpi_driver); >> >> > > coreboot_table-of.c: return platform_driver_register(&coreboot_table_of_driver); >> >> > >> >> > These are not unloadable (for better or worse) - they do not have >> >> > module_exit() in them. >> >> > >> >> > > >> >> > > gsmi.c: gsmi_dev.pdev = platform_device_register_full(&gsmi_dev_info); >> >> > > gsmi.c: platform_device_unregister(gsmi_dev.pdev); >> >> > > gsmi.c: platform_device_unregister(gsmi_dev.pdev); >> >> > > [looks good] >> >> > > >> >> > > memconsole-coreboot.c: pdev = platform_device_register_simple("memconsole", -1, NULL, 0); >> >> > > memconsole-coreboot.c: platform_driver_register(&memconsole_driver); >> >> > >> >> > Same here: not unloadable. >> >> > >> >> > > >> >> > > vpd.c: pdev = platform_device_register_simple("vpd", -1, NULL, 0); >> >> > > vpd.c: platform_driver_register(&vpd_driver); >> >> > >> >> > Arguably this should not even be a platform driver, there is no hardware >> >> > behind it. I was planning on purring some notifiers into coreboot table >> >> > driver and using notifiers to attach vpd to them. -ENOTIME though. >> >> > >> >> Two options for now: clean it up and make it unloadable, or make it bool >> >> and drop the exit function. Any preference ? >> >> >> >> The problem is easy to reproduce even with the driver is built into >> >> the kernel with a simple unbind/bind sequence. And after the unbind, >> >> it is easy to crash the system since the sysfs attributes are still there. >> > >> > The kernel should not 'crash', just spit out a nasty warning, right? >> > >> > thanks, >> > >> > greg k-h > > -- > Dmitry