linux-kernel - Re: [RESEND][PATCH] Bluetooth: Make request workqueue freezable

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <830BACF6-994B-4315-B868-D44E667BADB8@holtmann.org>
Date:	Thu, 21 May 2015 14:07:11 +0200
From:	Marcel Holtmann <marcel@...tmann.org>
To:	Takashi Iwai <tiwai@...e.de>
Cc:	Laura Abbott <labbott@...hat.com>,
	Oliver Neukum <oneukum@...e.com>,
	Ming Lei <ming.lei@...onical.com>,
	"David S. Miller" <davem@...emloft.net>,
	Laura Abbott <labbott@...oraproject.org>,
	Johan Hedberg <johan.hedberg@...il.com>,
	"Rafael J. Wysocki" <rafael.j.wysocki@...el.com>,
	"Gustavo F. Padovan" <gustavo@...ovan.org>,
	Alan Stern <stern@...land.harvard.edu>,
	"bluez mailin list (linux-bluetooth@...r.kernel.org)" 
	<linux-bluetooth@...r.kernel.org>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	USB list <linux-usb@...r.kernel.org>,
	netdev <netdev@...r.kernel.org>
Subject: Re: [RESEND][PATCH] Bluetooth: Make request workqueue freezable

Hi Takashi,

>>>>>> The data is cached in RAM.  More specifically, the former loaded
>>>>>> firmware files are reloaded and saved at suspend for each device
>>>>>> object.  See fw_pm_notify() in firmware_class.c.
>>>>> 
>>>>> OK, this may be a stupid idea, but do we know the firmware
>>>>> was successfully loaded in the first place?
>>>>> Also btusb is in the habit of falling back to a generic
>>>>> firmware in some places. It seems to me that caching
>>>>> firmware is conceptually not enough, but we'd also need
>>>>> to record the absence of firmware images.
>>>> 
>>>> in a lot of cases the firmware is optional. The device will operate fine without the firmware. There are a few devices where the firmware is required, but for many it just contains patches.
>>>> 
>>>> It would be nice if we could tell request_firmware() if it is optional or mandatory firmware. Or if it should just cache the status of a missing firmware as well.
>>> 
>>> OK, below is a quick hack to record the failed f/w files, too.
>>> Not sure whether this helps, though.  Proper tests are appreciated.
>>> 
>>> 
>> 
>> This doesn't quite work. We end up with the name on fw_names but
>> the firmware isn't actually on the firmware cache list.
>> 
>> If request_firmware fails to get the firmware from the filesystem,
>> release firmware will be called which is going to free the
>> firmware_buf which has been marked as failed anyway. The only
>> way to make this work would be to always piggy back and increase
>> the ref so it always stays around. But this also marks the firmware
>> as a permanent failure. There would need to be a hook somewhere
>> to force a cache drop, else there would be no way to add new
>> firmware to a running system without a reboot.
>> 
>> Perhaps we split the difference: keep a list of firmware images
>> that failed to load in the past and if one is requested during
>> a time when usermodehelper isn't available, silently return an
>> error? This way, if correct firmware is loaded at a regular time
>> the item can be removed from the list.
> 
> Well, IMO, it's way too much expectation for the generic f/w loader.
> The driver itself must know already which should be really loaded.
> The fact is that it's the driver who calls the function that might not
> work in the resume path.  So the driver can deal with such exceptions
> at best.

I keep repeating myself here. From the driver point of view it goes via probe() callback of the USB driver. So the driver does not know. For the driver it looks like a brand new device. There are platforms that might decide to just kill the power to the USB bus where the Bluetooth controller sits on. It gets the power back on resume. However this means it is a brand new device at that point. So the driver should not have to remember everything.

Regards

Marcel

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/