linux-kernel - Re: [PATCH v2 2/2] Improve container_notify

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]

Date:	Thu, 25 Oct 2012 09:31:42 +0800
From:	Tang Chen <tangchen@...fujitsu.com>
To:	Toshi Kani <toshi.kani@...com>
CC:	yinghai@...nel.org, bhelgaas@...gle.com, lenb@...nel.org,
	jiang.liu@...wei.com, izumi.taku@...fujitsu.com,
	isimatu.yasuaki@...fujitsu.com, mihailm@...allels.com,
	linux-acpi@...r.kernel.org, linux-pci@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 2/2] Improve container_notify_cb() to support container
 hot-remove.

Hi Toshi,

On 10/25/2012 01:14 AM, Toshi Kani wrote:
> On Wed, 2012-10-24 at 14:05 +0800, Tang Chen wrote:
>> +static int container_device_remove(struct acpi_device *device)
>> +{
>> +	int ret;
>> +	struct acpi_eject_event *ej_event;
>> +
>> +	/* stop container device at first */
>> +	ret = acpi_bus_trim(device, 0);
>
> Hi Tang,
>
> Why do you need to call acpi_bus_trim(device,0) to stop the container
> device first?

This issue was introduced by Lu Yinghai, I think he could give a better
answer than me. :)
Please refer to the following url:

http://www.spinics.net/lists/linux-pci/msg17667.html

However, this is not applied into the pci tree yet.

>
>> +	printk(KERN_WARNING "acpi_bus_trim stop return %x\n", ret);
>
> Do you need this message in the normal case?  If so, I'd suggest to use
> pr_debug().
>
>> +	if (ret)
>> +		return ret;
>> +
>> +	/* event originated from ACPI eject notification */
>> +	device->flags.eject_pending = 1;
>
> You do not need to set the eject_pending flag when the handler calls
> acpi_bus_hot_remove_device().  It was set before because the handler did
> not initiate the hot-remove operation.

I just set it to keep the logic the same as before.
And thanks for telling me this. :)

>
...
>> +		printk(KERN_WARNING "Container driver received %s event\n",
>> +			"ACPI_NOTIFY_EJECT_REQUEST");
>
> Same as other comment.  Suggest to use pr_debug().

OK. :)

>
>> +
>> +		if (!present || ACPI_FAILURE(status) || !device)
>> +			break;
>> +
>> +		result = container_device_remove(device);
>> +		if (result) {
>> +			printk(KERN_WARNING "Failed to remove container\n");
>
> Please use pr_warn().
>
> Thanks,
> -Toshi
>
>> +			break;
>>   		}
>> -		break;
>> +
>> +		return;
>>
>>   	default:
>>   		/* non-hotplug event; possibly handled by other handler */
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to majordomo@...r.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

--
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/