Linux Hotplug development
 help / color / mirror / Atom feed
* Re: Early-boot kernel panics from udev-165/extras/ata_id/ata_id.c
From: John Stanley @ 2011-01-18 21:48 UTC (permalink / raw)
  To: linux-hotplug
In-Reply-To: <4D263BF6.6050305@verizon.net>

Fantastic... looks promising, test-identify-packet is running w/o issue, 
and boots, so far, as well. I will further test with a number of warm 
and cold boots over the next several days to see if anything crops up. 
I'm very grateful for your time and efforts with this, thanks again,
John

On 01/18/2011 10:09 AM, Tejun Heo wrote:
> Hello,
>
> Can you please test whether the following patch fixes the problem?
>
> Thanks.
>
> diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
> index 5defc74..9d46731 100644
> --- a/drivers/ata/libata-scsi.c
> +++ b/drivers/ata/libata-scsi.c
> @@ -1099,9 +1099,9 @@ static int ata_scsi_dev_config(struct scsi_device *sdev,
>   		struct request_queue *q = sdev->request_queue;
>   		void *buf;
>
> -		/* set the min alignment and padding */
> -		blk_queue_update_dma_alignment(sdev->request_queue,
> -					       ATA_DMA_PAD_SZ - 1);
> +		sdev->sector_size = ATA_SECT_SIZE;
> +
> +		/* set DMA padding */
>   		blk_queue_update_dma_pad(sdev->request_queue,
>   					 ATA_DMA_PAD_SZ - 1);
>
> @@ -1115,13 +1115,18 @@ static int ata_scsi_dev_config(struct scsi_device *sdev,
>
>   		blk_queue_dma_drain(q, atapi_drain_needed, buf, ATAPI_MAX_DRAIN);
>   	} else {
> -		/* ATA devices must be sector aligned */
>   		sdev->sector_size = ata_id_logical_sector_size(dev->id);
> -		blk_queue_update_dma_alignment(sdev->request_queue,
> -					       sdev->sector_size - 1);
>   		sdev->manage_start_stop = 1;
>   	}
>
> +	/*
> +	 * ata_pio_sectors() expect sector alignment on buffers.  ATAPI
> +	 * devices also need the alignment as IDENTIFY_PACKET is executed
> +	 * as ATA_PROT_PIO.
> +	 */
> +	blk_queue_update_dma_alignment(sdev->request_queue,
> +				       sdev->sector_size - 1);
> +
>   	if (dev->flags&  ATA_DFLAG_AN)
>   		set_bit(SDEV_EVT_MEDIA_CHANGE, sdev->supported_events);
>
>

^ permalink raw reply

* Re: udev minimum requirement
From: Darcoux Christine @ 2011-01-18 21:56 UTC (permalink / raw)
  To: linux-hotplug
In-Reply-To: <AANLkTikdw11L1adphyv4hDk-aG9vn82V80X0zyvZ+Lkc@mail.gmail.com>

2011/1/18 Kay Sievers <kay.sievers@vrfy.org>:
> On Tue, Jan 18, 2011 at 18:25, Darcoux Christine <bouloumag@gmail.com> wrote:
>> I am a little confused about the minimum kernel version need for udev .
>> Is there a contradiction between these two sentences from the README file ?
>>
>> "The upstream udev project's set of default rules may require a most recent
>> kernel release to work properly. This is currently version 2.6.31."
>
> That's what we usually test and should work out-of-the-box.
>
>> and
>>
>> "Requirements:
>>  - Version 2.6.27 of the Linux kernel"
>
> That's the first version of the old kernels that has all the
> requirement to just run udev. It may fail completely with older
> versions. But nobody really tests that, I guess, at least not
> constantly. Rules may be missing. Many possible issues could be worked
> around, are might not be to find in the udev source tree.
>
>> Does it means that the **real** minimum requirement is kernel 2.6.31 ?
>> If not, what will works with 2.6.31 that will not work with 2.6.27 ?
>
> Nobody really knows, I guess, because we usually don't run old kernels
> on new userspace tools. We only make sure that new kernels can run on
> old userspace, not the other way around.
>
> Kay
>

(sorry I did not replyed to the list)
Thank you Kay for the explanations. May I suggest to include them in
the readme ?

Cheers

^ permalink raw reply

* Re: Early-boot kernel panics from udev-165/extras/ata_id/ata_id.c
From: Brad Price @ 2011-01-19  2:07 UTC (permalink / raw)
  To: linux-hotplug
In-Reply-To: <4D263BF6.6050305@verizon.net>

Hi, I'm also seeing this issue. I applied the libata patch and so far it 
seem to solve the issue. I've done many boots, and run "sg_sat_identify 
-p /dev/dvd" in a loop 10k times with no kernel panics. I don't think I 
could execute that command 20 times without a kernel panic before this.

Thanks!
Brad

^ permalink raw reply

* Re: About volume/disk blocks/size properties
From: Karel Zak @ 2011-01-19 13:38 UTC (permalink / raw)
  To: linux-hotplug
In-Reply-To: <AANLkTi=zsM5nY+J9X7-Soe_5Ajn1Qek6TEhWgpWj-ra2@mail.gmail.com>

On Mon, Jan 10, 2011 at 12:07:41AM +0100, José Félix Ontañón wrote:
> El día 8 de enero de 2011 18:26, Kay Sievers <kay.sievers@vrfy.org> escribió:
> > 2011/1/8 José Félix Ontañón <felixonta@gmail.com>:
> >> I wonder... are there any reasons for not being convenient to export
> >> some properties refering the volume size of a disk device?
> >> Although I'm concerned about the
> >> don't-turning-udev-into-a-hal-like-system, I still perceiving the
> >> volume.size or volume.block_size properties as very useful for user
> >> space apps.
> >>
> >> On the one hand, it could be easy for user space apps to get the
> >> size/blocks for partition devices,
> >
> >  $ grep . /sys/class/block/*/size
> >  /sys/class/block/sda1/size:41943040
> >  /sys/class/block/sda2/size:41943040
> >  /sys/class/block/sda3/size:115343360
> >  /sys/class/block/sda4/size:50835456
> >  /sys/class/block/sda/size:250069680
> >  /sys/class/block/sr0/size:2097151
> >
> >> but on the other hand you need root
> >> privs to get this info for disk devices, am i wrong?
> >
> > Everybody can do that.
> >
> > Kay
> >
> 
> Oh! Much better! Sorry for messing up.
> 
> It seems the size file give us the number of sectors, isn't?
> In order to get the bytes of the disk/partition ... is it right to
> multiply /sys/block/<disk>/queue/logical_block_size times
> /sys/block/<disk>/size ??

 The latest util-linux version (2.19-rc1) contains a new "lsblk" util,
 for example:

$ lsblk -b
NAME                 MAJ:MIN RM         SIZE RO MOUNTPOINT
sda                    8:0    0 100030242816  0 
├─sda1                 8:1    0    106896384  0 
├─sda2                 8:2    0         1024  0 
├─sda3                 8:3    0   2410007040  0 [SWAP]
├─sda4                 8:4    0  81783959040  0 /
├─sda5                 8:5    0  10742183424  0 
│ └─kzak-home (dm-0) 253:0    0  10741655040  0 /home/kzak
└─sda6                 8:6    0   4984487424  0 /boot

 more at http://karelzak.blogspot.com/2010/12/lsblk8.html

    Karel

-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com

^ permalink raw reply

* how to suppress write_net_rules (prevent alteration of my net rules)
From: John Lumby @ 2011-01-19 16:58 UTC (permalink / raw)
  To: linux-hotplug


Is there any supported way of telling udev never to create, append or alter my /etc/udev/rules.d/70-persistent-net.rules ?
It appears that by default a script called /lib/udev/write_net_rules will always do so,
although I can't find this documented anywhere other than inside the  /etc/udev/rules.d/70-persistent-net.rules.

The reason I want to prevent this is that I have one special requirement:
  that any net interface that is named eth0 by the kernel must never be renamed.
(This is for ensuring that running root over nfs works without hiccup).
I can specify this restriction in my /etc/udev/rules.d/60-net.rules by 
 SUBSYSTEM="net", ....  , KERNEL!="eth0", NAME="eth[n]"
which works   --   but then along comes /lib/udev/write_net_rules and creates or appends 
/etc/udev/rules.d/70-persistent-net.rules with conflicting specifications.

So I want to stop it from doing that.    The only way I have found is to edit this script
and change the string "/etc/udev/rules.d/70-persistent-net.rule" to "/tmp/70-persistent-net.rules"
but I suppose that change gets wiped out whenever I upgrade the software.

Is there any properly supported way of specifying this?

John Lumby
 		 	   		  

^ permalink raw reply

* Re: how to suppress write_net_rules (prevent alteration of my net rules)
From: Kay Sievers @ 2011-01-19 17:24 UTC (permalink / raw)
  To: linux-hotplug
In-Reply-To: <COL116-W182958BE507DDC0AD7E74DA3F60@phx.gbl>

On Wed, Jan 19, 2011 at 17:58, John Lumby <johnlumby@hotmail.com> wrote:
>
> Is there any supported way of telling udev never to create, append or alter my /etc/udev/rules.d/70-persistent-net.rules ?
> It appears that by default a script called /lib/udev/write_net_rules will always do so,
> although I can't find this documented anywhere other than inside the  /etc/udev/rules.d/70-persistent-net.rules.
>
> The reason I want to prevent this is that I have one special requirement:
>   that any net interface that is named eth0 by the kernel must never be renamed.
> (This is for ensuring that running root over nfs works without hiccup).
> I can specify this restriction in my /etc/udev/rules.d/60-net.rules by
>  SUBSYSTEM="net", ....  , KERNEL!="eth0", NAME="eth[n]"
> which works   --   but then along comes /lib/udev/write_net_rules and creates or appends
> /etc/udev/rules.d/70-persistent-net.rules with conflicting specifications.
>
> So I want to stop it from doing that.    The only way I have found is to edit this script
> and change the string "/etc/udev/rules.d/70-persistent-net.rule" to "/tmp/70-persistent-net.rules"
> but I suppose that change gets wiped out whenever I upgrade the software.
>
> Is there any properly supported way of specifying this?

Rules in /etc over-rule rules in /lib with the same name.

echo "# disable net-generator rules in /lib/udev" > \
  /etc/udev/rules.d/75-persistent-net-generator.rules

Kay

^ permalink raw reply

* Re: how to suppress write_net_rules (prevent alteration of my net rules)
From: Andrey Borzenkov @ 2011-01-19 18:31 UTC (permalink / raw)
  To: linux-hotplug
In-Reply-To: <COL116-W182958BE507DDC0AD7E74DA3F60@phx.gbl>

On Wed, Jan 19, 2011 at 7:58 PM, John Lumby <johnlumby@hotmail.com> wrote:
>
> Is there any supported way of telling udev never to create, append or alter my /etc/udev/rules.d/70-persistent-net.rules ?
> It appears that by default a script called /lib/udev/write_net_rules will always do so,
> although I can't find this documented anywhere other than inside the  /etc/udev/rules.d/70-persistent-net.rules.
>
> The reason I want to prevent this is that I have one special requirement:
>   that any net interface that is named eth0 by the kernel must never be renamed.
> (This is for ensuring that running root over nfs works without hiccup).
> I can specify this restriction in my /etc/udev/rules.d/60-net.rules by
>  SUBSYSTEM="net", ....  , KERNEL!="eth0", NAME="eth[n]"
> which works   --   but then along comes /lib/udev/write_net_rules and creates or appends
> /etc/udev/rules.d/70-persistent-net.rules with conflicting specifications.
>
> So I want to stop it from doing that.    The only way I have found is to edit this script
> and change the string "/etc/udev/rules.d/70-persistent-net.rule" to "/tmp/70-persistent-net.rules"
> but I suppose that change gets wiped out whenever I upgrade the software.
>
> Is there any properly supported way of specifying this?
>

/lib/udev/rules.d/75-persistent-net-generator.rules should not call
write_net_rules if NAME is already set, so

KERNEL="eth0", NAME="eth0"

may do the trick.

^ permalink raw reply

* Re: how to suppress write_net_rules (prevent alteration of my net
From: Marco d'Itri @ 2011-01-19 18:37 UTC (permalink / raw)
  To: linux-hotplug
In-Reply-To: <COL116-W182958BE507DDC0AD7E74DA3F60@phx.gbl>

On Jan 19, Andrey Borzenkov <arvidjaar@gmail.com> wrote:

> /lib/udev/rules.d/75-persistent-net-generator.rules should not call
> write_net_rules if NAME is already set, so
> 
> KERNEL="eth0", NAME="eth0"
This is how it should work indeed.

-- 
ciao,
Marco

^ permalink raw reply

* RE: how to suppress write_net_rules (prevent alteration of my net
From: John Lumby @ 2011-01-19 20:03 UTC (permalink / raw)
  To: linux-hotplug
In-Reply-To: <COL116-W182958BE507DDC0AD7E74DA3F60@phx.gbl>


Thanks everyone for the quick response.    Useful education for me but, as far as I can tell,
neither of those approaches quite achieves what I'm looking for.

I should explain a bit more about my root-over-nfs setup.    To simplify,  I have two computers,   C1 and C2,
and each machine has two ethernet NICs with MAC addresses   M1a,  M1b,   and M2a , M2b  respectively.
C1  has M1a and M1b

C2  has M2a and M2b

On each machine there is a linux installation which might run on either machine,
either locally or by mounting root over nfs from the other.

I would ideally like a single content for each of my systems' 60-net.rules,  naming all 4 NICs,
and assigning a unique eth[n] name to each one but *only* if the kernel did not already name it eth0
(which is what the kernel does if told to configure a network interface itself for nfs-mount purposes)
In other words,  I want all 4 interfaces to have a consistent name except when in use for mounting root over nfs.

So I would have content looking like


# mobo 8168
SUBSYSTEM="net", ACTION="add", ATTR{address}="[M1a]", ... KERNEL!="eth0", NAME="eth1"

# PCI via-rhine
SUBSYSTEM="net", ACTION="add", ATTR{address}="[M1b]", ... KERNEL!="eth0", NAME="eth2"

# mobo yukon2
SUBSYSTEM="net", ACTION="add", ATTR{address}="[M2a]",  ... KERNEL!="eth0", NAME="eth3"

#  e1000
SUBSYSTEM="net", ACTION="add", ATTR{address}="[M2b]",  ... KERNEL!="eth0", NAME="eth4"

And that's what I've done and,  in the absence of write_net_rules ,  it works.

Now  if I may ask questions about your specific suggestions:

Kay Sievers wrote:
> 
> Rules in /etc over-rule rules in /lib with the same name.
> 
> echo "# disable net-generator rules in /lib/udev" > \
>   /etc/udev/rules.d/75-persistent-net-generator.rules
>
That looks perfect  -  except  ...    I already have a  /etc/udev/rules.d/75-persistent-net-generator.rules
and at the top it says:
# do not edit this file, it will be overwritten on update

Is that true?     If so  -  although I can see this would work,   I'm no better off than editing the /lib/udev/write_net_rules am I?

Andrey Borzenkov  wrote:
>
> /lib/udev/rules.d/75-persistent-net-generator.rules should not call
> write_net_rules if NAME is already set, so
>
> KERNEL="eth0", NAME="eth0"
>

I think I see how this would prevent the renaming but how can I achieve the renaming I *do* want?
It seems to me that adding KERNEL="eth0", NAME="eth0" would prevent udev from doing anything when
KERNEL!="eth0",  wouldn't it?   I didn't try it but that's my reading of the man page  -
there is no syntax for parenthesising conditions e.g.

( KERNEL="eth0", NAME="eth0" ) || (  KERNEL!="eth0", NAME="eth1" )
is there?

I have one more question  -   is there any master user-specified profile or config file associated with udev?
Something which orchestrates all the other parts?


Cheers,   John

>
> --
> ciao,
> Marco
> --
> To unsubscribe from this list: send the line "unsubscribe linux-hotplug" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
 		 	   		  

^ permalink raw reply

* Re: Early-boot kernel panics from udev-165/extras/ata_id/ata_id.c
From: John Stanley @ 2011-01-19 20:20 UTC (permalink / raw)
  To: linux-hotplug
In-Reply-To: <4D263BF6.6050305@verizon.net>

Ok, looks like this fixes it. So would this patch go in 2.6.37.1 ?

John

On 01/18/2011 10:09 AM, Tejun Heo wrote:
> Hello,
>
> Can you please test whether the following patch fixes the problem?
>
> Thanks.
>
> diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
> index 5defc74..9d46731 100644
> --- a/drivers/ata/libata-scsi.c
> +++ b/drivers/ata/libata-scsi.c
> @@ -1099,9 +1099,9 @@ static int ata_scsi_dev_config(struct scsi_device *sdev,
>   		struct request_queue *q = sdev->request_queue;
>   		void *buf;
>
> -		/* set the min alignment and padding */
> -		blk_queue_update_dma_alignment(sdev->request_queue,
> -					       ATA_DMA_PAD_SZ - 1);
> +		sdev->sector_size = ATA_SECT_SIZE;
> +
> +		/* set DMA padding */
>   		blk_queue_update_dma_pad(sdev->request_queue,
>   					 ATA_DMA_PAD_SZ - 1);
>
> @@ -1115,13 +1115,18 @@ static int ata_scsi_dev_config(struct scsi_device *sdev,
>
>   		blk_queue_dma_drain(q, atapi_drain_needed, buf, ATAPI_MAX_DRAIN);
>   	} else {
> -		/* ATA devices must be sector aligned */
>   		sdev->sector_size = ata_id_logical_sector_size(dev->id);
> -		blk_queue_update_dma_alignment(sdev->request_queue,
> -					       sdev->sector_size - 1);
>   		sdev->manage_start_stop = 1;
>   	}
>
> +	/*
> +	 * ata_pio_sectors() expect sector alignment on buffers.  ATAPI
> +	 * devices also need the alignment as IDENTIFY_PACKET is executed
> +	 * as ATA_PROT_PIO.
> +	 */
> +	blk_queue_update_dma_alignment(sdev->request_queue,
> +				       sdev->sector_size - 1);
> +
>   	if (dev->flags&  ATA_DFLAG_AN)
>   		set_bit(SDEV_EVT_MEDIA_CHANGE, sdev->supported_events);
>
>

^ permalink raw reply

* Re: About volume/disk blocks/size properties
From: José Félix Ontañón @ 2011-01-19 22:34 UTC (permalink / raw)
  To: linux-hotplug
In-Reply-To: <AANLkTi=zsM5nY+J9X7-Soe_5Ajn1Qek6TEhWgpWj-ra2@mail.gmail.com>

El día 19 de enero de 2011 14:38, Karel Zak <kzak@redhat.com> escribió:
> On Mon, Jan 10, 2011 at 12:07:41AM +0100, José Félix Ontañón wrote:
>> El día 8 de enero de 2011 18:26, Kay Sievers <kay.sievers@vrfy.org> escribió:
>> > 2011/1/8 José Félix Ontañón <felixonta@gmail.com>:
>> >> I wonder... are there any reasons for not being convenient to export
>> >> some properties refering the volume size of a disk device?
>> >> Although I'm concerned about the
>> >> don't-turning-udev-into-a-hal-like-system, I still perceiving the
>> >> volume.size or volume.block_size properties as very useful for user
>> >> space apps.
>> >>
>> >> On the one hand, it could be easy for user space apps to get the
>> >> size/blocks for partition devices,
>> >
>> >  $ grep . /sys/class/block/*/size
>> >  /sys/class/block/sda1/size:41943040
>> >  /sys/class/block/sda2/size:41943040
>> >  /sys/class/block/sda3/size:115343360
>> >  /sys/class/block/sda4/size:50835456
>> >  /sys/class/block/sda/size:250069680
>> >  /sys/class/block/sr0/size:2097151
>> >
>> >> but on the other hand you need root
>> >> privs to get this info for disk devices, am i wrong?
>> >
>> > Everybody can do that.
>> >
>> > Kay
>> >
>>
>> Oh! Much better! Sorry for messing up.
>>
>> It seems the size file give us the number of sectors, isn't?
>> In order to get the bytes of the disk/partition ... is it right to
>> multiply /sys/block/<disk>/queue/logical_block_size times
>> /sys/block/<disk>/size ??
>
>  The latest util-linux version (2.19-rc1) contains a new "lsblk" util,
>  for example:
>
> $ lsblk -b
> NAME                 MAJ:MIN RM         SIZE RO MOUNTPOINT
> sda                    8:0    0 100030242816  0
> ├─sda1                 8:1    0    106896384  0
> ├─sda2                 8:2    0         1024  0
> ├─sda3                 8:3    0   2410007040  0 [SWAP]
> ├─sda4                 8:4    0  81783959040  0 /
> ├─sda5                 8:5    0  10742183424  0
> │ └─kzak-home (dm-0) 253:0    0  10741655040  0 /home/kzak
> └─sda6                 8:6    0   4984487424  0 /boot
>
>  more at http://karelzak.blogspot.com/2010/12/lsblk8.html
>
>    Karel

Thaks Karel, i'll check that improvement on util-linux for taking
ideas about how you access /proc information.

Cheers!

> --
>  Karel Zak  <kzak@redhat.com>
>  http://karelzak.blogspot.com
>



-- 
http://fontanon.org

^ permalink raw reply

* Re: Proper way to get USB class codes
From: José Félix Ontañón @ 2011-01-19 22:55 UTC (permalink / raw)
  To: linux-hotplug
In-Reply-To: <AANLkTi=GV6=BkEHM7fi=AOprNzBMUaigzPYgH=1vAJWN@mail.gmail.com>

El día 15 de enero de 2011 19:26, José Félix Ontañón
<felixonta@gmail.com> escribió:
> Hello everybody,
> I've got some doubts and problems getting USB class codes via udev and
> i'll be very grateful if someone could clarify.
>
> First of all, using udevadm i've realized some usb devices left the
> class/subclass/protocol attributes with 00:
>
>    ATTR{bDeviceClass}="00"
>    ATTR{bDeviceSubClass}="00"
>    ATTR{bDeviceProtocol}="00"
>
> Is that an issue on the usb device itself or an issue on udev reading
> usb device/interface descriptor?
>
> Second, i've some doubts about when should I use the
> ID_USB_INTERFACES, TYPE or INTERFACE property: in which cases should I
> use one or another? Why the TYPE property sometimes appear as 0/0/0
> even when the ID_USB_INTERFACES and INTERFACE properties are filled?
>
> Thanks for sharing your thoughs.

Hello everyone,

I've been doing some tests and the fact is I think this could be an
udev bug: HAL reports class/subclass/protocol for all USB devices with
no failures at all. Here the same example as above but using HAL:

  usb.interface.class = 11  (0xb)  (int)
  usb.interface.subclass = 0  (0x0)  (int)
  usb.interface.protocol = 0  (0x0)  (int)

After had been looking through bugzilla.kernel.org I realized there
weren't any bug reported about it. Does anyone think, like me, this
could be a bug? Would somebody tell me if i can perform some more
tests before reporting the bug myself?

Thanks in advance.

-- 
http://fontanon.org

^ permalink raw reply

* [PATCH V2] tracing, perf : add cpu hotplug trace events
From: Vincent Guittot @ 2011-01-20  8:25 UTC (permalink / raw)
  To: linux-kernel, linux-hotplug, fweisbec, rostedt, amit.kucheria

Please find below a new proposal for adding trace events for cpu hotplug.
The goal is to measure the latency of each part (kernel, architecture)
and also to trace the cpu hotplug activity with other power events. I
have tested these traces events on an arm platform.

Changes since previous version:
-Use cpu_hotplug for trace name
-Define traces for kernel core and arch parts only
-Use DECLARE_EVENT_CLASS and DEFINE_EVENT
-Use proper indentation

Subject: [PATCH] cpu hotplug tracepoint

this patch adds new events for cpu hotplug tracing
 * plug/unplug sequence
 * core and architecture latency measurements

Signed-off-by: Vincent Guittot <vincent.guittot@linaro.com>
---
 include/trace/events/cpu_hotplug.h |  117 ++++++++++++++++++++++++++++++++++++
 1 files changed, 117 insertions(+), 0 deletions(-)
 create mode 100644 include/trace/events/cpu_hotplug.h

diff --git a/include/trace/events/cpu_hotplug.h
b/include/trace/events/cpu_hotplug.h
new file mode 100644
index 0000000..efccc70
--- /dev/null
+++ b/include/trace/events/cpu_hotplug.h
@@ -0,0 +1,117 @@
+#undef TRACE_SYSTEM
+#define TRACE_SYSTEM cpu_hotplug
+
+#if !defined(_TRACE_CPU_HOTPLUG_H) || defined(TRACE_HEADER_MULTI_READ)
+#define _TRACE_CPU_HOTPLUG_H
+
+#include <linux/tracepoint.h>
+
+DECLARE_EVENT_CLASS(cpu_hotplug,
+
+	TP_PROTO(unsigned int cpuid),
+
+	TP_ARGS(cpuid),
+
+	TP_STRUCT__entry(
+		__field(	unsigned int,	cpuid	)
+	),
+
+	TP_fast_assign(
+		__entry->cpuid = cpuid;
+	),
+
+	TP_printk("cpuid=%u", __entry->cpuid)
+);
+
+/* Core function of cpu hotplug */
+
+DEFINE_EVENT(cpu_hotplug, cpu_hotplug_down_start,
+
+	TP_PROTO(unsigned int cpuid),
+
+	TP_ARGS(cpuid)
+);
+
+DEFINE_EVENT(cpu_hotplug, cpu_hotplug_down_end,
+
+	TP_PROTO(unsigned int cpuid),
+
+	TP_ARGS(cpuid)
+);
+
+DEFINE_EVENT(cpu_hotplug, cpu_hotplug_up_start,
+
+	TP_PROTO(unsigned int cpuid),
+
+	TP_ARGS(cpuid)
+);
+
+DEFINE_EVENT(cpu_hotplug, cpu_hotplug_up_end,
+
+	TP_PROTO(unsigned int cpuid),
+
+	TP_ARGS(cpuid)
+);
+
+/* Architecture function for cpu hotplug */
+
+DEFINE_EVENT(cpu_hotplug, cpu_hotplug_arch_disable_start,
+
+	TP_PROTO(unsigned int cpuid),
+
+	TP_ARGS(cpuid)
+);
+
+DEFINE_EVENT(cpu_hotplug, cpu_hotplug_arch_disable_end,
+
+	TP_PROTO(unsigned int cpuid),
+
+	TP_ARGS(cpuid)
+);
+
+DEFINE_EVENT(cpu_hotplug, cpu_hotplug_arch_die_start,
+
+	TP_PROTO(unsigned int cpuid),
+
+	TP_ARGS(cpuid)
+);
+
+DEFINE_EVENT(cpu_hotplug, cpu_hotplug_arch_die_end,
+
+	TP_PROTO(unsigned int cpuid),
+
+	TP_ARGS(cpuid)
+);
+
+DEFINE_EVENT(cpu_hotplug, cpu_hotplug_arch_wait_die_start,
+
+	TP_PROTO(unsigned int cpuid),
+
+	TP_ARGS(cpuid)
+);
+
+DEFINE_EVENT(cpu_hotplug, cpu_hotplug_arch_wait_die_end,
+
+	TP_PROTO(unsigned int cpuid),
+
+	TP_ARGS(cpuid)
+);
+
+DEFINE_EVENT(cpu_hotplug, cpu_hotplug_arch_up_start,
+
+	TP_PROTO(unsigned int cpuid),
+
+	TP_ARGS(cpuid)
+);
+
+DEFINE_EVENT(cpu_hotplug, cpu_hotplug_arch_up_end,
+
+	TP_PROTO(unsigned int cpuid),
+
+	TP_ARGS(cpuid)
+);
+
+#endif /* _TRACE_CPU_HOTPLUG_H */
+
+/* This part must be outside protection */
+#include <trace/define_trace.h>
-- 
1.7.1

^ permalink raw reply related

* [PATCH #upstream-fixes] libata: set queue DMA alignment to sector
From: Tejun Heo @ 2011-01-20 12:59 UTC (permalink / raw)
  To: Jeff Garzik, John Stanley, linux-ide
  Cc: Hannes Reinecke, Robby Workman, Greg KH, linux-hotplug, stable
In-Reply-To: <4D374796.1010009@verizon.net>

ata_pio_sectors() expects buffer for each sector to be contained in a
single page; otherwise, it ends up overrunning the first page.  This
is achieved by setting queue DMA alignment.  If sector_size is smaller
than PAGE_SIZE and all buffers are sector_size aligned, buffer for
each sector is always contained in a single page.

This wasn't applied to ATAPI devices but IDENTIFY_PACKET is executed
as ATA_PROT_PIO and thus uses ata_pio_sectors().  Newer versions of
udev issue IDENTIFY_PACKET with unaligned buffer triggering the
problem and causing oops.

This patch fixes the problem by setting sdev->sector_size to
ATA_SECT_SIZE on ATATPI devices and always setting DMA alignment to
sector_size.  While at it, add a warning for the unlikely but still
possible scenario where sector_size is larger than PAGE_SIZE, in which
case the alignment wouldn't be enough.

Signed-off-by: Tejun Heo <tj@kernel.org>
Reported-by: John Stanley <jpsinthemix@verizon.net>
Tested-by: John Stanley <jpsinthemix@verizon.net>
Cc: stable@kernel.org
---
 drivers/ata/libata-scsi.c |   24 ++++++++++++++++++------
 1 file changed, 18 insertions(+), 6 deletions(-)

diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index 5defc74..600f635 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -1099,9 +1099,9 @@ static int ata_scsi_dev_config(struct scsi_device *sdev,
 		struct request_queue *q = sdev->request_queue;
 		void *buf;
 
-		/* set the min alignment and padding */
-		blk_queue_update_dma_alignment(sdev->request_queue,
-					       ATA_DMA_PAD_SZ - 1);
+		sdev->sector_size = ATA_SECT_SIZE;
+
+		/* set DMA padding */
 		blk_queue_update_dma_pad(sdev->request_queue,
 					 ATA_DMA_PAD_SZ - 1);
 
@@ -1115,13 +1115,25 @@ static int ata_scsi_dev_config(struct scsi_device *sdev,
 
 		blk_queue_dma_drain(q, atapi_drain_needed, buf, ATAPI_MAX_DRAIN);
 	} else {
-		/* ATA devices must be sector aligned */
 		sdev->sector_size = ata_id_logical_sector_size(dev->id);
-		blk_queue_update_dma_alignment(sdev->request_queue,
-					       sdev->sector_size - 1);
 		sdev->manage_start_stop = 1;
 	}
 
+	/*
+	 * ata_pio_sectors() expects buffer for each sector to not cross
+	 * page boundary.  Enforce it by requiring buffers to be sector
+	 * aligned, which works iff sector_size is not larger than
+	 * PAGE_SIZE.  ATAPI devices also need the alignment as
+	 * IDENTIFY_PACKET is executed as ATA_PROT_PIO.
+	 */
+	if (sdev->sector_size > PAGE_SIZE)
+		ata_dev_printk(dev, KERN_WARNING,
+			"sector_size=%u > PAGE_SIZE, PIO may malfunction\n",
+			sdev->sector_size);
+
+	blk_queue_update_dma_alignment(sdev->request_queue,
+				       sdev->sector_size - 1);
+
 	if (dev->flags & ATA_DFLAG_AN)
 		set_bit(SDEV_EVT_MEDIA_CHANGE, sdev->supported_events);
 

^ permalink raw reply related

* Re: [PATCH V2] tracing, perf : add cpu hotplug trace events
From: Frederic Weisbecker @ 2011-01-20 16:11 UTC (permalink / raw)
  To: Vincent Guittot; +Cc: linux-kernel, linux-hotplug, rostedt, amit.kucheria
In-Reply-To: <AANLkTinNoYbDe5huY4_Q6YidbWxqXqVWbVP2_7o-=P3W@mail.gmail.com>

On Thu, Jan 20, 2011 at 09:25:54AM +0100, Vincent Guittot wrote:
> Please find below a new proposal for adding trace events for cpu hotplug.
> The goal is to measure the latency of each part (kernel, architecture)
> and also to trace the cpu hotplug activity with other power events. I
> have tested these traces events on an arm platform.
> 
> Changes since previous version:
> -Use cpu_hotplug for trace name
> -Define traces for kernel core and arch parts only
> -Use DECLARE_EVENT_CLASS and DEFINE_EVENT
> -Use proper indentation
> 
> Subject: [PATCH] cpu hotplug tracepoint
> 
> this patch adds new events for cpu hotplug tracing
>  * plug/unplug sequence
>  * core and architecture latency measurements
> 
> Signed-off-by: Vincent Guittot <vincent.guittot@linaro.com>
> ---
>  include/trace/events/cpu_hotplug.h |  117 ++++++++++++++++++++++++++++++++++++

Note we can't apply new tracepoints if they are not inserted in the code.

> +DEFINE_EVENT(cpu_hotplug, cpu_hotplug_arch_wait_die_start,
> +
> +	TP_PROTO(unsigned int cpuid),
> +
> +	TP_ARGS(cpuid)
> +);
> +
> +DEFINE_EVENT(cpu_hotplug, cpu_hotplug_arch_wait_die_end,
> +
> +	TP_PROTO(unsigned int cpuid),
> +
> +	TP_ARGS(cpuid)
> +);

What is wait die, compared to die for example?

We need those tracepoints to be applied to get that reviewable in practice.

Thanks.

^ permalink raw reply

* Re: Proper way to get USB class codes
From: Greg KH @ 2011-01-20 17:52 UTC (permalink / raw)
  To: linux-hotplug
In-Reply-To: <AANLkTi=GV6=BkEHM7fi=AOprNzBMUaigzPYgH=1vAJWN@mail.gmail.com>

On Sat, Jan 15, 2011 at 07:26:28PM +0100, José Félix Ontañón wrote:
> Hello everybody,
> I've got some doubts and problems getting USB class codes via udev and
> i'll be very grateful if someone could clarify.
> 
> First of all, using udevadm i've realized some usb devices left the
> class/subclass/protocol attributes with 00:
> 
>     ATTR{bDeviceClass}="00"
>     ATTR{bDeviceSubClass}="00"
>     ATTR{bDeviceProtocol}="00"
> 
> Is that an issue on the usb device itself or an issue on udev reading
> usb device/interface descriptor?

No, that's normal, why wouldn't it be?  Those attributes are properly
defined for these devices in the interface values for those fields, not
the main device values.

> Second, i've some doubts about when should I use the
> ID_USB_INTERFACES, TYPE or INTERFACE property: in which cases should I
> use one or another? Why the TYPE property sometimes appear as 0/0/0
> even when the ID_USB_INTERFACES and INTERFACE properties are filled?

That's normal, and to be expected.  Please read the usb.org specs for
details if you really are curious.

thanks,

greg k-h

^ permalink raw reply

* Re: how to suppress write_net_rules (prevent alteration of my net rules)
From: Andrey Borzenkov @ 2011-01-20 18:50 UTC (permalink / raw)
  To: linux-hotplug
In-Reply-To: <COL116-W182958BE507DDC0AD7E74DA3F60@phx.gbl>

On Wed, Jan 19, 2011 at 11:03 PM, John Lumby <johnlumby@hotmail.com> wrote:
>> Rules in /etc over-rule rules in /lib with the same name.
>>
>> echo "# disable net-generator rules in /lib/udev" > \
>>   /etc/udev/rules.d/75-persistent-net-generator.rules
>>
> That looks perfect  -  except  ...    I already have a  /etc/udev/rules.d/75-persistent-net-generator.rules
> and at the top it says:
> # do not edit this file, it will be overwritten on update
>
> Is that true?     If so  -  although I can see this would work,   I'm no better off than editing the /lib/udev/write_net_rules am I?
>

Either you have really old udev version or your distribution installs
standard rules under /etc instead of under /lib. In both cases you
probably need to ask your distro about it.

> Andrey Borzenkov  wrote:
>>
>> /lib/udev/rules.d/75-persistent-net-generator.rules should not call
>> write_net_rules if NAME is already set, so
>>
>> KERNEL="eth0", NAME="eth0"
>>
>
> I think I see how this would prevent the renaming but how can I achieve the renaming I *do* want?
> It seems to me that adding KERNEL="eth0", NAME="eth0" would prevent udev from doing anything when
> KERNEL!="eth0",  wouldn't it?

No, it won't.

>  I didn't try it but that's my reading of the man page  -
> there is no syntax for parenthesising conditions e.g.
>
> ( KERNEL="eth0", NAME="eth0" ) || (  KERNEL!="eth0", NAME="eth1" )
> is there?
>

Just continue to use the same rules for KERNEL != "eth0" as you use now.

> I have one more question  -   is there any master user-specified profile or config file associated with udev?
> Something which orchestrates all the other parts?

Current udev reads /etc/udev/rules.d, /lib/udev/rules.d and one more
directory under /dev for rules generated when root was still
read-only. This is hardcoded.

^ permalink raw reply

* udev-165 build problem (and workaround)
From: Thomas Koeller @ 2011-01-20 23:58 UTC (permalink / raw)
  To: linux-hotplug

Hi,

I am building udev-165 on a system that has an older udev version (151)
installed.

The build step that creates 'extras/gudev/GUdev-1.0.gir' fails because
it uses the installed 'libgudev-1.0.so' instead of the one just built,
and that library does not provide 'g_udev_enumerator_get_type'. I
noticed that the Makefile tries to address this issue by adding
'$(top_builddir)/extras/gudev' to LD_LIBRARY_PATH, however, that does
not seem to work. Btw., shouldn't that be '$(top_builddir)/extras/gudev/.libs'?
Anyway, I tried that, too, and it did not work either.

I have finally been able to work around the problem by using
'make LD_PRELOAD=$PWD/extras/gudev/.libs/libgudev-1.0.so'.

Thomas


^ permalink raw reply

* Re: [PATCH V2] tracing, perf : add cpu hotplug trace events
From: Vincent Guittot @ 2011-01-21  8:43 UTC (permalink / raw)
  To: Frederic Weisbecker; +Cc: linux-kernel, linux-hotplug, rostedt, amit.kucheria
In-Reply-To: <20110120161101.GA17218@nowhere>

On 20 January 2011 17:11, Frederic Weisbecker <fweisbec@gmail.com> wrote:
> On Thu, Jan 20, 2011 at 09:25:54AM +0100, Vincent Guittot wrote:
>> Please find below a new proposal for adding trace events for cpu hotplug.
>> The goal is to measure the latency of each part (kernel, architecture)
>> and also to trace the cpu hotplug activity with other power events. I
>> have tested these traces events on an arm platform.
>>
>> Changes since previous version:
>> -Use cpu_hotplug for trace name
>> -Define traces for kernel core and arch parts only
>> -Use DECLARE_EVENT_CLASS and DEFINE_EVENT
>> -Use proper indentation
>>
>> Subject: [PATCH] cpu hotplug tracepoint
>>
>> this patch adds new events for cpu hotplug tracing
>>  * plug/unplug sequence
>>  * core and architecture latency measurements
>>
>> Signed-off-by: Vincent Guittot <vincent.guittot@linaro.com>
>> ---
>>  include/trace/events/cpu_hotplug.h |  117 ++++++++++++++++++++++++++++++++++++
>
> Note we can't apply new tracepoints if they are not inserted in the code.

I agree, i just want to have 1st feedbacks on the tracepoint interface
before providing a patch which inserts the trace in the code.

>
>> +DEFINE_EVENT(cpu_hotplug, cpu_hotplug_arch_wait_die_start,
>> +
>> +     TP_PROTO(unsigned int cpuid),
>> +
>> +     TP_ARGS(cpuid)
>> +);
>> +
>> +DEFINE_EVENT(cpu_hotplug, cpu_hotplug_arch_wait_die_end,
>> +
>> +     TP_PROTO(unsigned int cpuid),
>> +
>> +     TP_ARGS(cpuid)
>> +);
>
> What is wait die, compared to die for example?
>

The arch_wait_die is used to trace the process which waits for the cpu
to die (__cpu_die) and the arch_die is used to trace when the cpu dies
(cpu_die)

> We need those tracepoints to be applied to get that reviewable in practice.
>

Ok, I'm going to send a proposal

> Thanks.
>

^ permalink raw reply

* how to disable a device in a udev rule
From: Pádraig Brady @ 2011-01-21 11:17 UTC (permalink / raw)
  To: linux-hotplug

I notice one can no longer ignore a device with a udev rule, since:
http://odin3.kernel.org/git-lewiemann/?p=linux/hotplug/udev.git;a=commit;hÍae488a

I would like to:

SUBSYSTEM="usb" ATTRS{idVendor}="174f" ATTRS{idProduct}="8a34" ATTRS{busnum}="2", OPTIONS:="ignore_device"

What's the best way to achieve this now?
Blacklist the module and manually load it in an init script?

cheers,
Pádraig.

^ permalink raw reply

* [PATCH] udevd.c: set selinux context on /lib/udev/devices copy
From: harald @ 2011-01-21 15:41 UTC (permalink / raw)
  To: linux-hotplug

From: Harald Hoyer <harald@redhat.com>

Set the selinux context on /lib/udev/devices copy, even if the file
already exists.
---
 libudev/libudev-private.h         |    2 ++
 libudev/libudev-selinux-private.c |   28 ++++++++++++++++++++++++++++
 udev/udevd.c                      |    1 +
 3 files changed, 31 insertions(+), 0 deletions(-)

diff --git a/libudev/libudev-private.h b/libudev/libudev-private.h
index f95be53..de6735c 100644
--- a/libudev/libudev-private.h
+++ b/libudev/libudev-private.h
@@ -233,6 +233,7 @@ unsigned long long usec_monotonic(void);
 static inline void udev_selinux_init(struct udev *udev) {}
 static inline void udev_selinux_exit(struct udev *udev) {}
 static inline void udev_selinux_lsetfilecon(struct udev *udev, const char *file, unsigned int mode) {}
+static inline void udev_selinux_lsetfileconat(struct udev *udev, int dfd, const char *file, unsigned int mode) {}
 static inline void udev_selinux_setfscreatecon(struct udev *udev, const char *file, unsigned int mode) {}
 static inline void udev_selinux_setfscreateconat(struct udev *udev, int dfd, const char *file, unsigned int mode) {}
 static inline void udev_selinux_resetfscreatecon(struct udev *udev) {}
@@ -240,6 +241,7 @@ static inline void udev_selinux_resetfscreatecon(struct udev *udev) {}
 void udev_selinux_init(struct udev *udev);
 void udev_selinux_exit(struct udev *udev);
 void udev_selinux_lsetfilecon(struct udev *udev, const char *file, unsigned int mode);
+void udev_selinux_lsetfileconat(struct udev *udev, int dfd, const char *file, unsigned int mode);
 void udev_selinux_setfscreatecon(struct udev *udev, const char *file, unsigned int mode);
 void udev_selinux_setfscreateconat(struct udev *udev, int dfd, const char *file, unsigned int mode);
 void udev_selinux_resetfscreatecon(struct udev *udev);
diff --git a/libudev/libudev-selinux-private.c b/libudev/libudev-selinux-private.c
index cb06a28..f482ea8 100644
--- a/libudev/libudev-selinux-private.c
+++ b/libudev/libudev-selinux-private.c
@@ -59,6 +59,34 @@ void udev_selinux_lsetfilecon(struct udev *udev, const char *file, unsigned int
 	freecon(scontext);
 }
 
+void udev_selinux_lsetfileconat(struct udev *udev, int dfd, const char *file, unsigned int mode)
+{
+	char filename[UTIL_PATH_SIZE];
+
+	if (!selinux_enabled)
+		return;
+
+	/* resolve relative filename */
+	if (file[0] != '/') {
+		char procfd[UTIL_PATH_SIZE];
+		char target[UTIL_PATH_SIZE];
+		ssize_t len;
+
+		snprintf(procfd, sizeof(procfd), "/proc/%u/fd/%u", getpid(), dfd);
+		len = readlink(procfd, target, sizeof(target));
+		if (len <= 0 || len = sizeof(target))
+			return;
+		target[len] = '\0';
+
+		util_strscpyl(filename, sizeof(filename), target, "/", file, NULL);
+		file = filename;
+	}
+
+	udev_selinux_lsetfilecon(udev, file, mode)
+}
+
+
+
 void udev_selinux_setfscreatecon(struct udev *udev, const char *file, unsigned int mode)
 {
 	security_context_t scontext = NULL;
diff --git a/udev/udevd.c b/udev/udevd.c
index aa2e365..8ebefd0 100644
--- a/udev/udevd.c
+++ b/udev/udevd.c
@@ -851,6 +851,7 @@ static int copy_dir(struct udev *udev, DIR *dir_from, DIR *dir_to, int maxdepth)
 				fchmodat(dirfd(dir_to), dent->d_name, stats.st_mode & 0777, 0);
 				fchownat(dirfd(dir_to), dent->d_name, stats.st_uid, stats.st_gid, 0);
 			} else {
+				udev_selinux_lsetfileconat(udev, dirfd(dir_to), dent->d_name, stats.st_mode & 0777);
 				utimensat(dirfd(dir_to), dent->d_name, NULL, 0);
 			}
 			udev_selinux_resetfscreatecon(udev);

^ permalink raw reply related

* Re: [PATCH V2] tracing, perf : add cpu hotplug trace events
From: Frederic Weisbecker @ 2011-01-21 16:44 UTC (permalink / raw)
  To: Vincent Guittot; +Cc: linux-kernel, linux-hotplug, rostedt, amit.kucheria
In-Reply-To: <AANLkTinhX6UwQA0HFhuu5=jbq84woVhqtahuzA5hEo2o@mail.gmail.com>

On Fri, Jan 21, 2011 at 09:43:18AM +0100, Vincent Guittot wrote:
> On 20 January 2011 17:11, Frederic Weisbecker <fweisbec@gmail.com> wrote:
> > On Thu, Jan 20, 2011 at 09:25:54AM +0100, Vincent Guittot wrote:
> >> Please find below a new proposal for adding trace events for cpu hotplug.
> >> The goal is to measure the latency of each part (kernel, architecture)
> >> and also to trace the cpu hotplug activity with other power events. I
> >> have tested these traces events on an arm platform.
> >>
> >> Changes since previous version:
> >> -Use cpu_hotplug for trace name
> >> -Define traces for kernel core and arch parts only
> >> -Use DECLARE_EVENT_CLASS and DEFINE_EVENT
> >> -Use proper indentation
> >>
> >> Subject: [PATCH] cpu hotplug tracepoint
> >>
> >> this patch adds new events for cpu hotplug tracing
> >>  * plug/unplug sequence
> >>  * core and architecture latency measurements
> >>
> >> Signed-off-by: Vincent Guittot <vincent.guittot@linaro.com>
> >> ---
> >>  include/trace/events/cpu_hotplug.h |  117 ++++++++++++++++++++++++++++++++++++
> >
> > Note we can't apply new tracepoints if they are not inserted in the code.
> 
> I agree, i just want to have 1st feedbacks on the tracepoint interface
> before providing a patch which inserts the trace in the code.
> 
> >
> >> +DEFINE_EVENT(cpu_hotplug, cpu_hotplug_arch_wait_die_start,
> >> +
> >> +     TP_PROTO(unsigned int cpuid),
> >> +
> >> +     TP_ARGS(cpuid)
> >> +);
> >> +
> >> +DEFINE_EVENT(cpu_hotplug, cpu_hotplug_arch_wait_die_end,
> >> +
> >> +     TP_PROTO(unsigned int cpuid),
> >> +
> >> +     TP_ARGS(cpuid)
> >> +);
> >
> > What is wait die, compared to die for example?
> >
> 
> The arch_wait_die is used to trace the process which waits for the cpu
> to die (__cpu_die) and the arch_die is used to trace when the cpu dies
> (cpu_die)

I still can't find the difference.

Having:

trace_cpu_hotplug_arch_die_start(cpu)
__cpu_die();
trace_cpu_hotplug_arch_die_end(cpu)

Is not enough to get both the information that a cpu dies
and the time took to do so?

^ permalink raw reply

* Re: [PATCH V2] tracing, perf : add cpu hotplug trace events
From: Steven Rostedt @ 2011-01-21 17:28 UTC (permalink / raw)
  To: Vincent Guittot
  Cc: Frederic Weisbecker, linux-kernel, linux-hotplug, amit.kucheria
In-Reply-To: <AANLkTinhX6UwQA0HFhuu5=jbq84woVhqtahuzA5hEo2o@mail.gmail.com>

On Fri, 2011-01-21 at 09:43 +0100, Vincent Guittot wrote:
> On 20 January 2011 17:11, Frederic Weisbecker <fweisbec@gmail.com> wrote:

> >
> >> +DEFINE_EVENT(cpu_hotplug, cpu_hotplug_arch_wait_die_start,
> >> +
> >> +     TP_PROTO(unsigned int cpuid),
> >> +
> >> +     TP_ARGS(cpuid)
> >> +);
> >> +
> >> +DEFINE_EVENT(cpu_hotplug, cpu_hotplug_arch_wait_die_end,
> >> +
> >> +     TP_PROTO(unsigned int cpuid),
> >> +
> >> +     TP_ARGS(cpuid)
> >> +);
> >
> > What is wait die, compared to die for example?
> >
> 
> The arch_wait_die is used to trace the process which waits for the cpu
> to die (__cpu_die) and the arch_die is used to trace when the cpu dies
> (cpu_die)

Shouldn't that be "cpu_hotplug_arch_task_wait_die" then? If it is a task
that is waiting?

-- Steve

> 
> > We need those tracepoints to be applied to get that reviewable in practice.
> >
> 
> Ok, I'm going to send a proposal
> 
> > Thanks.
> >



^ permalink raw reply

* Re: [PATCH V2] tracing, perf : add cpu hotplug trace events
From: Vincent Guittot @ 2011-01-21 17:41 UTC (permalink / raw)
  To: Frederic Weisbecker; +Cc: linux-kernel, linux-hotplug, rostedt, amit.kucheria
In-Reply-To: <20110121164404.GA2520@nowhere>

On 21 January 2011 17:44, Frederic Weisbecker <fweisbec@gmail.com> wrote:
> On Fri, Jan 21, 2011 at 09:43:18AM +0100, Vincent Guittot wrote:
>> On 20 January 2011 17:11, Frederic Weisbecker <fweisbec@gmail.com> wrote:
>> > On Thu, Jan 20, 2011 at 09:25:54AM +0100, Vincent Guittot wrote:
>> >> Please find below a new proposal for adding trace events for cpu hotplug.
>> >> The goal is to measure the latency of each part (kernel, architecture)
>> >> and also to trace the cpu hotplug activity with other power events. I
>> >> have tested these traces events on an arm platform.
>> >>
>> >> Changes since previous version:
>> >> -Use cpu_hotplug for trace name
>> >> -Define traces for kernel core and arch parts only
>> >> -Use DECLARE_EVENT_CLASS and DEFINE_EVENT
>> >> -Use proper indentation
>> >>
>> >> Subject: [PATCH] cpu hotplug tracepoint
>> >>
>> >> this patch adds new events for cpu hotplug tracing
>> >>  * plug/unplug sequence
>> >>  * core and architecture latency measurements
>> >>
>> >> Signed-off-by: Vincent Guittot <vincent.guittot@linaro.com>
>> >> ---
>> >>  include/trace/events/cpu_hotplug.h |  117 ++++++++++++++++++++++++++++++++++++
>> >
>> > Note we can't apply new tracepoints if they are not inserted in the code.
>>
>> I agree, i just want to have 1st feedbacks on the tracepoint interface
>> before providing a patch which inserts the trace in the code.
>>
>> >
>> >> +DEFINE_EVENT(cpu_hotplug, cpu_hotplug_arch_wait_die_start,
>> >> +
>> >> +     TP_PROTO(unsigned int cpuid),
>> >> +
>> >> +     TP_ARGS(cpuid)
>> >> +);
>> >> +
>> >> +DEFINE_EVENT(cpu_hotplug, cpu_hotplug_arch_wait_die_end,
>> >> +
>> >> +     TP_PROTO(unsigned int cpuid),
>> >> +
>> >> +     TP_ARGS(cpuid)
>> >> +);
>> >
>> > What is wait die, compared to die for example?
>> >
>>
>> The arch_wait_die is used to trace the process which waits for the cpu
>> to die (__cpu_die) and the arch_die is used to trace when the cpu dies
>> (cpu_die)
>
> I still can't find the difference.
>
> Having:
>
> trace_cpu_hotplug_arch_die_start(cpu)
> __cpu_die();
> trace_cpu_hotplug_arch_die_end(cpu)
>
> Is not enough to get both the information that a cpu dies
> and the time took to do so?
>

it's quite interesting to trace the cpu_die function because the cpu
really dies in this one. The __cpu_die function can't return if the
cpu fails to die in the very last step and then wake up. But this
could be detected with some cpu_die traces.

for a normal use case we have something like :
cpu 0 enters __cpu_die
cpu 1 enters cpu_die
cpu1 acks that it is going to died
cpu0 returns from __cpu_die

if the cpu 1 fails to die at the very last step, we could have:
cpu 0 enters __cpu_die
cpu 1 enters cpu_idle --> cpu_die
cpu1 leaves cpu_die because of some issues and comes back into cpu_idle.
cpu0 returns from __cpu_die after a timeout or an error ack

Then, cpu_die traces can be used with power traces for profiling the
cpu power state. May be, the power.h trace file is a better place for
the cpu_die traces ?

^ permalink raw reply

* Re: [PATCH V2] tracing, perf : add cpu hotplug trace events
From: Frederic Weisbecker @ 2011-01-22  2:42 UTC (permalink / raw)
  To: Vincent Guittot; +Cc: linux-kernel, linux-hotplug, rostedt, amit.kucheria
In-Reply-To: <AANLkTi=Tx3DYczwf9U=QS9XGs6C8sjKwVZr+RLuo781V@mail.gmail.com>

On Fri, Jan 21, 2011 at 06:41:58PM +0100, Vincent Guittot wrote:
> On 21 January 2011 17:44, Frederic Weisbecker <fweisbec@gmail.com> wrote:
> > On Fri, Jan 21, 2011 at 09:43:18AM +0100, Vincent Guittot wrote:
> >> On 20 January 2011 17:11, Frederic Weisbecker <fweisbec@gmail.com> wrote:
> >> > On Thu, Jan 20, 2011 at 09:25:54AM +0100, Vincent Guittot wrote:
> >> >> Please find below a new proposal for adding trace events for cpu hotplug.
> >> >> The goal is to measure the latency of each part (kernel, architecture)
> >> >> and also to trace the cpu hotplug activity with other power events. I
> >> >> have tested these traces events on an arm platform.
> >> >>
> >> >> Changes since previous version:
> >> >> -Use cpu_hotplug for trace name
> >> >> -Define traces for kernel core and arch parts only
> >> >> -Use DECLARE_EVENT_CLASS and DEFINE_EVENT
> >> >> -Use proper indentation
> >> >>
> >> >> Subject: [PATCH] cpu hotplug tracepoint
> >> >>
> >> >> this patch adds new events for cpu hotplug tracing
> >> >>  * plug/unplug sequence
> >> >>  * core and architecture latency measurements
> >> >>
> >> >> Signed-off-by: Vincent Guittot <vincent.guittot@linaro.com>
> >> >> ---
> >> >>  include/trace/events/cpu_hotplug.h |  117 ++++++++++++++++++++++++++++++++++++
> >> >
> >> > Note we can't apply new tracepoints if they are not inserted in the code.
> >>
> >> I agree, i just want to have 1st feedbacks on the tracepoint interface
> >> before providing a patch which inserts the trace in the code.
> >>
> >> >
> >> >> +DEFINE_EVENT(cpu_hotplug, cpu_hotplug_arch_wait_die_start,
> >> >> +
> >> >> +     TP_PROTO(unsigned int cpuid),
> >> >> +
> >> >> +     TP_ARGS(cpuid)
> >> >> +);
> >> >> +
> >> >> +DEFINE_EVENT(cpu_hotplug, cpu_hotplug_arch_wait_die_end,
> >> >> +
> >> >> +     TP_PROTO(unsigned int cpuid),
> >> >> +
> >> >> +     TP_ARGS(cpuid)
> >> >> +);
> >> >
> >> > What is wait die, compared to die for example?
> >> >
> >>
> >> The arch_wait_die is used to trace the process which waits for the cpu
> >> to die (__cpu_die) and the arch_die is used to trace when the cpu dies
> >> (cpu_die)
> >
> > I still can't find the difference.
> >
> > Having:
> >
> > trace_cpu_hotplug_arch_die_start(cpu)
> > __cpu_die();
> > trace_cpu_hotplug_arch_die_end(cpu)
> >
> > Is not enough to get both the information that a cpu dies
> > and the time took to do so?
> >
> 
> it's quite interesting to trace the cpu_die function because the cpu
> really dies in this one.

Note in case of success, you have barely the same time between die and
wait_die, the difference will reside in some completion wait/polling,
noise, mostly. Probably most of the time unnoticeable and irrelevant.

Plus if you opt for this scheme, you need to put your die hook into
every architectures, while otherwise a simple trace_cpu_die_start()
trace_cpu_die_stop() pair around __cpu_die() call in the generic code
is enough.

> The __cpu_die function can't return if the
> cpu fails to die in the very last step and then wake up. But this
> could be detected with some cpu_die traces.
>
> 
> for a normal use case we have something like :
> cpu 0 enters __cpu_die
> cpu 1 enters cpu_die
> cpu1 acks that it is going to died
> cpu0 returns from __cpu_die
> 
> if the cpu 1 fails to die at the very last step, we could have:
> cpu 0 enters __cpu_die
> cpu 1 enters cpu_idle --> cpu_die
> cpu1 leaves cpu_die because of some issues and comes back into cpu_idle.
> cpu0 returns from __cpu_die after a timeout or an error ack

If it fails in the hardware level, you'll certainly notice in your
power profiling because a CPU is not supposed to take seconds to
die. Especially with a such visual tool like pytimechart, it will
be obvious.

For the details, that's something that must be found in syslogs and
that's it.

I don't think it's a good idea to handle such buggy and unexpected case at
the tracepoint level. You don't want to profile bugs, you want to debug them.
So it doesn't belong to this space IMHO.

> Then, cpu_die traces can be used with power traces for profiling the
> cpu power state. May be, the power.h trace file is a better place for
> the cpu_die traces ?

Hmm, this should probably stay inside the cpu hotplug tracepoint family,
this is where people will seek them in the first place.

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox