CHOON.NET Forums

Re: [Xen-devel] [PATCH] libxl: Introduce LIBXL_DOMAIN_TYPE_INVALID to make gcc happy

Dario Faggioli — Fri, 18 May 2012 22:53:13 +0800

On Fri, 2012-05-18 at 15:39 +0100, Ian Campbell wrote:
> > Which actually seems to be right:
> >
> > $ grep LIBXL_DOMAIN_TYPE_INVALID tools/* -R
> > tools/libxl/libxl_dm.c: case LIBXL_DOMAIN_TYPE_INVALID:
> > tools/libxl/libxl_dm.c: case LIBXL_DOMAIN_TYPE_INVALID:
> > tools/libxl/libxl_dom.c: return LIBXL_DOMAIN_TYPE_INVALID;
> > tools/libxl/libxl_dom.c: return LIBXL_DOMAIN_TYPE_INVALID;
> > tools/libxl/libxl.c: case LIBXL_DOMAIN_TYPE_INVALID:
> >
> > Am I missing something?
>
> This should be defined in tools/libxl/libxl_types.idl but the patch
> doesn't seem to add it.
>
Yep, I'm adding it myself with the attached patch, but I'm now getting
this:

_libxl_types.c: In function ‘libxl_domain_build_info_dispose’:
_libxl_types.c:91:5: error: enumeration value ‘LIBXL_DOMAIN_TYPE_INVALID’ not handled in switch [-Werror=switch]
_libxl_types.c: In function ‘libxl_domain_build_info_init_type’:
_libxl_types.c:284:5: error: enumeration value ‘LIBXL_DOMAIN_TYPE_INVALID’ not handled in switch [-Werror=switch]
testidl.c: In function ‘libxl_domain_build_info_rand_init’:
testidl.c:366:5: error: enumeration value ‘LIBXL_DOMAIN_TYPE_INVALID’ not handled in switch [-Werror=switch]
_libxl_types.c: In function ‘libxl_domain_build_info_gen_json’:
_libxl_types.c:1713:5: error: enumeration value ‘LIBXL_DOMAIN_TYPE_INVALID’ not handled in switch [-Werror=switch]
cc1: all warnings being treated as errors

:-O

Dario

--
<> (Raistlin Majere)
-----------------------------------------------------------------
Dario Faggioli, Ph.D, [retis.sssup.it]
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
[lists.xen.org]

Re: [RFC v2 PATCH 2/4] block: add queue runtime pm callbacks

Lin Ming — Fri, 18 May 2012 22:53:12 +0800

On Thu, 2012-05-17 at 14:29 -0400, Alan Stern wrote:

[ ... ]

>
> I may have left some parts out from this brief description. Hopefully
> you'll be able to figure out the general idea and get it to work.

Thanks for the detail idea.

Let me look at it closely first.

>
> Alan Stern
>

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

[PATCH] drivers/staging/line6/config.h: Delete unused header

Johannes Thumshirn — Fri, 18 May 2012 22:53:06 +0800

Delete unused header file drivers/staging/line6/config.h

Signed-off-by: Johannes Thumshirn
---
drivers/staging/line6/config.h | 43 ----------------------------------------
1 files changed, 0 insertions(+), 43 deletions(-)
delete mode 100644 drivers/staging/line6/config.h

diff --git a/drivers/staging/line6/config.h b/drivers/staging/line6/config.h
deleted file mode 100644
index 2493491..0000000
--- a/drivers/staging/line6/config.h
+++ /dev/null
@@ -1,43 +0,0 @@
-/*
- * Line6 Linux USB driver - 0.8.0
- *
- * Copyright (C) 2004-2009 Markus Grabner (grabner@icg.tugraz.at)
- *
- * This program is free software; you can redistribute it and/or
- * modify it under the terms of the GNU General Public License as
- * published by the Free Software Foundation, version 2.
- *
- */
-
-#ifndef CONFIG_H
-#define CONFIG_H
-
-
-#ifdef CONFIG_USB_DEBUG
-#define DEBUG 1
-#endif
-
-
-/*
- * Development tools.
- */
-#define DO_DEBUG_MESSAGES 0
-#define DO_DUMP_URB_SEND DO_DEBUG_MESSAGES
-#define DO_DUMP_URB_RECEIVE DO_DEBUG_MESSAGES
-#define DO_DUMP_PCM_SEND 0
-#define DO_DUMP_PCM_RECEIVE 0
-#define DO_DUMP_MIDI_SEND DO_DEBUG_MESSAGES
-#define DO_DUMP_MIDI_RECEIVE DO_DEBUG_MESSAGES
-#define DO_DUMP_ANY (DO_DUMP_URB_SEND || DO_DUMP_URB_RECEIVE || \
- DO_DUMP_PCM_SEND || DO_DUMP_PCM_RECEIVE || \
- DO_DUMP_MIDI_SEND || DO_DUMP_MIDI_RECEIVE)
-#define CREATE_RAW_FILE 0
-
-#if DO_DEBUG_MESSAGES
-#define DEBUG_MESSAGES(x) (x)
-#else
-#define DEBUG_MESSAGES(x)
-#endif
-
-
-#endif
--
1.7.7.6

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

Re: [Xen-devel] [PATCH v6 05/11] libxl: introduce libxl__device_disk_add

Ian Jackson — Fri, 18 May 2012 22:50:09 +0800

Ian Campbell writes ("Re: [PATCH v6 05/11] libxl: introduce libxl__device_disk_add"):
> On Fri, 2012-05-18 at 15:36 +0100, Ian Jackson wrote:
> > There is no need IMO to move the
> > internal function to libxl_internal.c.
>
> I've always thought the existence libxl_internal.c was just begging for
> people to group functions along the wrong criteria anyway...

Yes.

> The whole concept of what lives where inside tools/libxl/libxl*.c is all
> pretty ad-hoc anyway.

Quite.

Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
[lists.xen.org]

Re: [RESEND,PATCH] DCA, x86: fix invalid memory access in DCA core

Jiang Liu — Fri, 18 May 2012 22:50:08 +0800

Hi Maciej,
Thanks for your help!
I think your patch is correct but I could only test it on next Monday.
My previous patch may cause NULL pointer dereference when calling
dca_sysfs_remove_provider(dca) the second time for the same DCA device.
I'm just curious why the NULL pointer dereference issue hasn't been triggered
during tests, will ping my team member about that.
Thanks!
Gerry

On 05/18/2012 10:04 PM, Sosnowski, Maciej wrote:
> On Mon, May 07, 2012 5:58 PM, Jiang Liu wrote:
>>
>> From: Jiang Liu
>>
>> When unregister_dca_providers() is called, it will remove all registered
>> providers from the dca_providrers list by calling list_del(&dca->node).
>> list_del(node) poisons node->next and node->prev as 0xDEADBEEF and
>> 0xBEEFDEAD.
>> Later when unregister_dca_provider() is called to remove a DCA provier,
>> it calls list_del(&dca->node) to remove the dca from the list again,
>> but dca->node has already been poisoned, then causes invalid memory
>> access.
>>
>> The solution here is to use list_del_init(&dca->node) instead of
>> list_del(&dca->node) in function unregister_dca_providers(), so it won't
>> cause invalid memory access in unregister_dca_provider() later.
>>
>> ---
>>
>> This issue is triggered when hot-removing IOHs on Intel platforms, which
>> will remove all IOAT devices built in the IOHs.
>>
>> ioatdma 0000:80:16.7: Removing dma and dca services
>> ioatdma 0000:80:16.7: PCI INT D disabled
>> ioatdma 0000:80:16.6: Removing dma and dca services
>> ioatdma 0000:80:16.7: Removing dma and dca services
>> ioatdma 0000:80:16.7: PCI INT D disabled
>> ioatdma 0000:80:16.6: Removing dma and dca services
>> ioatdma 0000:80:16.6: PCI INT C disabled
>> ioatdma 0000:00:16.0: Removing dma and dca services
>> ------------[ cut here ]------------
>> WARNING: at lib/list_debug.c:47 __list_del_entry+0x63/0xd0()
>> Hardware name: System x3850 X5 -[7143O3G]-
>> list_del corruption, ffff880463540bc0->next is LIST_POISON1
>> (dead000000100100)
>> Modules linked in: ebtable_nat ebtables ipt_MASQUERADE iptable_nat
>> nf_nat xt_CHECKSUM iptable_mangle bridge stp llc autofs4 sunrpc
>> cpufreq_ondemand acpi_cpufreq freq_table mperf ipt_REJECT
>> nf_conntrack_ipv4 nf_defrag_ipv4 iptable_filter ip_tables ip6t_REJECT
>> nf_conntrack_ipv6 nf_defrag_ipv6 xt_state nf_conntrack ip6table_filter
>> ip6_tables ipv6 vfat fat vhost_net macvtap macvlan tun kvm_intel kvm uinput
>> microcode pcspkr serio_raw pmcraid sg be2net cdc_ether usbnet mii i2c_i801
>> i2c_core iTCO_wdt iTCO_vendor_support shpchp ioatdma i7core_edac
>> edac_core igb dca e1000e bnx2 ext4 mbcache jbd2 sr_mod cdrom sd_mod
>> crc_t10dif qla2xxx pata_acpi ata_generic ata_piix bfa scsi_transport_fc
>> scsi_tgt megaraid_sas dm_mirror dm_region_hash dm_log dm_mod [last
>> unloaded: scsi_wait_scan]
>> Pid: 10049, comm: bash.sh Not tainted 3.2.0IOAT+ #5
>> Call Trace:
>> [] warn_slowpath_common+0x7f/0xc0
>> [] warn_slowpath_fmt+0x46/0x50
>> [] ? __blocking_notifier_call_chain+0x65/0x80
>> [] __list_del_entry+0x63/0xd0
>> [] list_del+0x11/0x40
>> [] unregister_dca_provider+0x42/0xe0 [dca]
>> [] ioat_remove+0x43/0x67 [ioatdma]
>> [] pci_device_remove+0x52/0x120
>> [] __device_release_driver+0x7c/0xe0
>> [] device_release_driver+0x2d/0x40
>> [] driver_unbind+0xa1/0xc0
>> [] drv_attr_store+0x2c/0x30
>> [] sysfs_write_file+0xef/0x170
>> [] vfs_write+0xc8/0x190
>> [] sys_write+0x51/0x90
>> [] system_call_fastpath+0x16/0x1b
>> ---[ end trace b81b51e7c494ec0d ]---
>> BUG: unable to handle kernel NULL pointer dereference at 0000000000000010
>> IP: [] unregister_dca_provider+0xc0/0xe0 [dca]
>> PGD 1465b48067 PUD 1465035067 PMD 0
>> Oops: 0000 [#1] SMP
>> CPU 57
>> Modules linked in: ebtable_nat ebtables ipt_MASQUERADE iptable_nat
>> nf_nat xt_CHECKSUM iptable_mangle bridge stp llc autofs4 sunrpc
>> cpufreq_ondemand acpi_cpufreq freq_table mperf ipt_REJECT
>> nf_conntrack_ipv4 nf_defrag_ipv4 iptable_filter ip_tables ip6t_REJECT
>> nf_conntrack_ipv6 nf_defrag_ipv6 xt_state nf_conntrack ip6table_filter
>> ip6_tables ipv6 vfat fat vhost_net macvtap macvlan tun kvm_intel kvm uinput
>> microcode pcspkr serio_raw pmcraid sg be2net cdc_ether usbnet mii i2c_i801
>> i2c_core iTCO_wdt iTCO_vendor_support shpchp ioatdma i7core_edac
>> edac_core igb dca e1000e bnx2 ext4 mbcache jbd2 sr_mod cdrom sd_mod
>> crc_t10dif qla2xxx pata_acpi ata_generic ata_piix bfa scsi_transport_fc
>> scsi_tgt megaraid_sas dm_mirror dm_region_hash dm_log dm_mod [last
>> unloaded: scsi_wait_scan]
>>
>> Pid: 10049, comm: bash.sh Tainted: G W 3.2.0IOAT+ #5 IBM System x3850
>> X5 -[7143O3G]-/Node 1, Processor Card
>> RIP: 0010:[] []
>> unregister_dca_provider+0xc0/0xe0 [dca]
>> RSP: 0018:ffff880c4eafbdb8 EFLAGS: 00010046
>> RAX: 0000000000000010 RBX: ffff880463540bc0 RCX: 0000000000002288
>> RDX: ffff881465a51800 RSI: 0000000000000046 RDI: 0000000000000009
>> RBP: ffff880c4eafbdd8 R08: 0000000000000000 R09: 0000000000000000
>> R10: 0000000000000010 R11: 000000000000000b R12: 0000000000000000
>> R13: 0000000000000257 R14: ffff881465abe000 R15: ffff881464199840
>> FS: 00007f91d8314700(0000) GS:ffff88147fd20000(0000)
>> knlGS:0000000000000000
>> CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>> CR2: 0000000000000010 CR3: 0000001457b07000 CR4: 00000000000006e0
>> DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
>> DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
>> Process bash.sh (pid: 10049, threadinfo ffff880c4eafa000, task
>> ffff880c4e3b8af0)
>> Stack:
>> 0000000000000206 ffff88046133a218 ffff881465abe090 ffffffffa0222560
>> ffff880c4eafbdf8 ffffffffa021f87d ffff881465abe090 ffff881465abe208
>> ffff880c4eafbe28 ffffffff8126b1a2 ffff881465abe090 ffffffffa02225c0
>> Call Trace:
>> [] ioat_remove+0x43/0x67 [ioatdma]
>> [] pci_device_remove+0x52/0x120
>> [] __device_release_driver+0x7c/0xe0
>> [] device_release_driver+0x2d/0x40
>> [] driver_unbind+0xa1/0xc0
>> [] drv_attr_store+0x2c/0x30
>> [] sysfs_write_file+0xef/0x170
>> [] vfs_write+0xc8/0x190
>> [] sys_write+0x51/0x90
>> [] system_call_fastpath+0x16/0x1b
>> Code: c7 20 c0 01 a0 e8 51 6c 4d e1 48 89 df e8 c9 05 00 00 48 83 c4 08 5b 41 5c 41
>> 5d c9 c3 66 0f 1f 44 00 00 45 31 e4 49 8d 44 24 10 <49> 39 44 24 10 75 c9 4c 89 e7
>> e8 71 ad 23 e1 4c 89 e7 e8 19 7b
>> RIP [] unregister_dca_provider+0xc0/0xe0 [dca]
>> RSP
>> CR2: 0000000000000010
>> ---[ end trace b81b51e7c494ec0e ]---
>
> Jiang,
>
> Could you verify if the following fixes the issue above?
>
> Thanks,
> Maciej
> ---
>
> drivers/dca/dca-core.c | 5 +++++
> 1 files changed, 5 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/dca/dca-core.c b/drivers/dca/dca-core.c
> index bc6f5fa..819dfda 100644
> --- a/drivers/dca/dca-core.c
> +++ b/drivers/dca/dca-core.c
> @@ -420,6 +420,11 @@ void unregister_dca_provider(struct dca_
>
> raw_spin_lock_irqsave(&dca_lock, flags);
>
> + if (list_empty(&dca_domains)) {
> + raw_spin_unlock_irqrestore(&dca_lock, flags);
> + return;
> + }
> +
> list_del(&dca->node);
>
> pci_rc = dca_pci_rc_from_dev(dev);

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

Re: NVM Mapping API

Matthew Wilcox — Fri, 18 May 2012 22:49:13 +0800

On Fri, May 18, 2012 at 10:03:53AM +0100, James Bottomley wrote:
> On Thu, 2012-05-17 at 14:59 -0400, Matthew Wilcox wrote:
> > On Thu, May 17, 2012 at 10:54:38AM +0100, James Bottomley wrote:
> > > On Wed, 2012-05-16 at 13:35 -0400, Matthew Wilcox wrote:
> > > > I'm not talking about a specific piece of technology, I'm assuming that
> > > > one of the competing storage technologies will eventually make it to
> > > > widespread production usage. Let's assume what we have is DRAM with a
> > > > giant battery on it.
> > > >
> > > > So, while we can use it just as DRAM, we're not taking advantage of the
> > > > persistent aspect of it if we don't have an API that lets us find the
> > > > data we wrote before the last reboot. And that sounds like a filesystem
> > > > to me.
> > >
> > > Well, it sounds like a unix file to me rather than a filesystem (it's a
> > > flat region with a beginning and end and no structure in between).
> >
> > That's true, but I think we want to put a structure on top of it.
> > Presumably there will be multiple independent users, and each will want
> > only a fraction of it.
> >
> > > However, I'm not precluding doing this, I'm merely asking that if it
> > > looks and smells like DRAM with the only additional property being
> > > persistency, shouldn't we begin with the memory APIs and see if we can
> > > add persistency to them?
> >
> > I don't think so. It feels harder to add useful persistent
> > properties to the memory APIs than it does to add memory-like
> > properties to our file APIs, at least partially because for
> > userspace we already have memory properties for our file APIs (ie
> > mmap/msync/munmap/mprotect/mincore/mlock/munlock/mremap).
>
> This is what I don't quite get. At the OS level, it's all memory; we
> just have to flag one region as persistent. This is easy, I'd do it in
> the physical memory map. once this is done, we need either to tell the
> allocators only use volatile, only use persistent, or don't care (I
> presume the latter would only be if you needed the extra ram).
>
> The missing thing is persistent key management of the memory space (so
> if a user or kernel wants 10Mb of persistent space, they get the same
> 10Mb back again across boots).
>
> The reason a memory API looks better to me is because a memory API can
> be used within the kernel. For instance, I want a persistent /var/tmp
> on tmpfs, I just tell tmpfs to allocate it in persistent memory and it
> survives reboots. Likewise, if I want an area to dump panics, I just
> use it ... in fact, I'd probably always place the dmesg buffer in
> persistent memory.
>
> If you start off with a vfs API, it becomes far harder to use it easily
> from within the kernel.
>
> The question, really is all about space management: how many persistent
> spaces would there be. I think, given the use cases above it would be a
> small number (it's basically one for every kernel use and one for ever
> user use ... a filesystem mount counting as one use), so a flat key to
> space management mapping (probably using u32 keys) makes sense, and
> that's similar to our current shared memory API.

So who manages the key space? If we do it based on names, it's easy; all
kernel uses are ".kernel/..." and we manage our own sub-hierarchy within
the namespace. If there's only a u32, somebody has to lay down the rules
about which numbers are used for what things. This isn't quite as ugly
as the initial proposal somebody made to me "We just use the physical
address as the key", and I told them all about how a.out libraries worked.

Nevertheless, I'm not interested in being the Mitch DSouza of NVM.

> > Discussion of use cases is exactly what I want! I think that a
> > non-hierarchical attempt at naming chunks of memory quickly expands
> > into cases where we learn we really do want a hierarchy after all.
>
> OK, so enumerate the uses. I can be persuaded the namespace has to be
> hierarchical if there are orders of magnitude more users than I think
> there will be.

I don't know what the potential use cases might be. I just don't think
the use cases are all that bounded.

> > > Again, this depends on use case. The SYSV shm API has a global flat
> > > keyspace. Perhaps your envisaged use requires a hierarchical key space
> > > and therefore a FS interface looks more natural with the leaves being
> > > divided memory regions?
> >
> > I've really never heard anybody hold up the SYSV shm API as something
> > to be desired before. Indeed, POSIX shared memory is much closer to
> > the filesystem API;
>
> I'm not really ... I was just thinking this needs key -> region mapping
> and SYSV shm does that. The POSIX anonymous memory API needs you to
> map /dev/zero and then pass file descriptors around for sharing. It's
> not clear how you manage a persistent key space with that.

I didn't say "POSIX anonymous memory". I said "POSIX shared memory".
I even pointed you at the right manpage to read if you haven't heard
of it before. The POSIX committee took a look at SYSV shm and said
"This is too ugly". So they invented their own API.

> > the only difference being use of shm_open() and
> > shm_unlink() instead of open() and unlink() [see shm_overview(7)].
>
> The internal kernel API addition is simply a key -> region mapping.
> Once that's done, you need an allocation API for userspace and you're
> done. I bet most userspace uses will be either give me xGB and put a
> tmpfs on it or give me xGB and put a something filesystem on it, but if
> the user wants an xGB mmap'd region, you can give them that as well.
>
> For a vfs interface, you have to do all of this as well, but in a much
> more complex way because the file name becomes the key and the metadata
> becomes the mapping.

You're downplaying the complexity of your own solution while overstating
the complexity of mine. Let's compare, using your suggestion of the
dmesg buffer.

Mine:

struct file *filp = filp_open(".kernel/dmesg", O_RDWR, 0);
if (!IS_ERR(filp))
log_buf = nvm_map(filp, 0, __LOG_BUF_LEN, PAGE_KERNEL);

Yours:

log_buf = nvm_attach(492, NULL, 0); /* Hope nobody else used 492! */

Hm. Doesn't look all that different, does it? I've modelled nvm_attach()
after shmat(). Of course, this ignores the need to be able to sync,
which may vary between different NVM technologies, and the (desired
by some users) ability to change portions of the mapped NVM between
read-only and read-write.

If the extra parameters and extra lines of code hinder adoption, I have
no problems with adding a helper for the simple use cases:

void *nvm_attach(const char *name, int perms)
{
void *mem;
struct file *filp = filp_open(name, perms, 0);
if (IS_ERR(filp))
return NULL;
mem = nvm_map(filp, 0, filp->f_dentry->d_inode->i_size, PAGE_KERNEL);
fput(filp);
return mem;
}

I do think that using numbers to refer to regions of NVM is a complete
non-starter. This was one of the big mistakes of SYSV; one so big that
even POSIX couldn't stomach it.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at [vger.kernel.org]
Please read the FAQ at [www.tux.org]

Re: RCU related performance regression in 3.3

Pascal Chapperon — Fri, 18 May 2012 22:49:07 +0800

Le 18/05/2012 14:14, Paul E. McKenney a écrit :
> On Fri, May 18, 2012 at 01:01:41PM +0200, Pascal Chapperon wrote:
>> Le 15/05/2012 00:32, Paul E. McKenney a écrit :
>>> On Fri, May 04, 2012 at 04:14:42PM -0700, Paul E. McKenney wrote:
>>>> On Fri, May 04, 2012 at 11:41:13PM +0200, Pascal Chapperon wrote:
>>>>> Le 04/05/2012 17:04, Paul E. McKenney a écrit :
>>>>>> On Fri, May 04, 2012 at 04:42:54PM +0200, Pascal Chapperon wrote:
>>>>>>> Le 01/05/2012 17:45, Paul E. McKenney a écrit :
>>>>>>>
>>>>>>>> Here is my RCU_FAST_NO_HZ patch stack on top of v3.4-rc4.
>>>>>>>>
>>>>>>>> Or you can pull branch fnh.2012.05.01a from:
>>>>>>>>
>>>>>>>> git://git.kernel.org/pub/scm/linux/kernel/git/paulmck/linux-rcu.git
>>>>>>>>
>>>>>>>> Thanx, Paul
>>>>>>>>
>>>>>>> I applied your global patch on top of v3.4-rc4. But the slowdown is
>>>>>>> worse than before : boot sequence took 80s instead 20-30s (12s for
>>>>>>> initramfs instead of 2s).
>>>>>>>
>>>>>>> I'll send you rcu tracing log in a second mail.
>>>>>>
>>>>>> Hmmm... Well, I guess I am glad that I finally did something that
>>>>>> had an effect, but I sure wish that the effect had been in the other
>>>>>> direction!
>>>>>>
>>>>>> Just to make sure I understand: the difference between the 20-30s and
>>>>>> the 80s is exactly the patch I sent you?
>>>>>>
>>>>>> Thanx, Paul
>>>>>>
>>>>>>
>>>>> Yes. Exactly same kernel config as in previous results, I applied
>>>>> your patch against v3.4-rc4, and sorry, the result is exactly what I
>>>>> said;
>>>>> I saw that your global patch was quite huge, and addresses things which
>>>>> are not directly related with the initial patch (commit
>>>>> 7cb92499000e3c86dae653077b1465458a039ef6); maybe a side effect?
>>>>>
>>>>> However, I'm ready to try this patch on my smaller laptop which
>>>>> supports well CONFIG_FAST_NO_HZ=y and systemd, if you think it can
>>>>> help ?
>>>>>
>>>>> Another thought: this issue as nothing to do with i7 Hyper-threading
>>>>> capacities ? (as I test core2duo, Pentium ulv in same conditions and I
>>>>> don't encountered any slowdown ?)
>>>>
>>>> Well, one possibility is that your setup starts the jiffies counter
>>>> at some interesting value. The attached patch (also against v3.4-rc4)
>>>> applies a bit more paranoia to the initialization to handle this
>>>> and other possibilities.
>>>
>>> This patchset fixes the problem where RCU_FAST_NO_HZ's timers were
>>> being ignored due to the dyntick-idle code having already calculated
>>> the CPU's wakeup time (which I sent earlier, mistakenly offlist), but
>>> also fixes a botched check in my workaround.
>>>
>>> Could you please try it out? This patch is against 3.4-rc4.
>>>
>>> Thanx, Paul
>>>
>> Hi Paul,
>>
>> < + if (!rcu_cpu_has_nonlazy_callbacks(cpu))
>> ---
>>> + if (rcu_cpu_has_nonlazy_callbacks(cpu))
>>
>> I was a little disappointed by the previous patch (boot sequence still
>> took 72 s.), but this one makes a huge difference ;-)
>> Slowdown during boot or shutdown with CONFIG_RCU_FAST_NO_HZ has
>> disappeared (~ 10 attempts) :
>> # systemd-analyze
>> Startup finished in 1990ms (kernel) + 1174ms (initramfs) + 3121ms
>> (userspace) = 6285ms
>> .
>
> Very good! And thank you very much for all your testing efforts and
> for bearing with me through this!
>
> Does this mean that I can add your Tested-by?

Yes: the results are good and stable, at least for my hardware.
I tried with both a standard fedora 16 kernel configuration and a custom
one (hardware optimized, preempted, etc...) and this on over more 20
attempts.
With or without FAST_NO_HZ makes no difference now.

>
>> Do you want the rcu tracing log for this patch ?
>
> Could you please? Just in case there is some other surprise that
> I should know about that might not be visible. ;-)
>
> Thanx, Paul
I'll send you the logs in a second mail (offlist).

Pascal

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

Re: [ 00/53] 3.2.18-stable review

Steven Rostedt — Fri, 18 May 2012 22:48:08 +0800

On Fri, May 18, 2012 at 03:32:54AM +0100, Ben Hutchings wrote:
> This is the start of the stable review cycle for the 3.2.18 release.
> There are 53 patches in this series, which will be posted as responses
> to this one. If anyone has any issues with these being applied, please
> let me know.
>
> Responses should be made by Sun May 20 13:00:00 UTC 2012.
> Anything received after that time might be too late.
>
> The whole patch series should soon be available in one patch at:
> kernel.org/pub/linux/kernel/v3.x/stable-review/patch-3.2.18-rc1.gz
> and the diffstat can be found below.

$ wget kernel.org/pub/linux/kernel/v3.x/stable-review/patch-3.2.18-rc1.gz
--2012-05-18 10:44:23-- [kernel.org]
Resolving kernel.org (kernel.org)... 149.20.4.69
Connecting to kernel.org (kernel.org)|149.20.4.69|:80... connected.
HTTP request sent, awaiting response... 404 Not Found
2012-05-18 10:44:23 ERROR 404: Not Found.

How much is "soon"? Did you push it to the repo yet and I'm just waiting
on the mirroring? Or do you still need to push it yourself?

-- Steve

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

Re: [Xen-devel] [PATCH v6 10/11] libxl_string_to_backend: add qdisk

Ian Jackson — Fri, 18 May 2012 22:47:06 +0800

Stefano Stabellini writes ("[PATCH v6 10/11] libxl_string_to_backend: add qdisk"):
> Signed-off-by: Stefano Stabellini

Acked-by: Ian Jackson

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
[lists.xen.org]

Re: [Xen-devel] [PATCH v6 11/11] main_blockdetach: destroy the disk on successful removal

Ian Jackson — Fri, 18 May 2012 22:47:05 +0800

Stefano Stabellini writes ("[PATCH v6 11/11] main_blockdetach: destroy the disk on successful removal"):
> main_blockdetach needs to call libxl_device_disk_destroy so that all the
> disk related entries are properly removed from xenstore.
>
> Signed-off-by: Stefano Stabellini

Acked-by: Ian Jackson

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
[lists.xen.org]

Re: [Xen-devel] [PATCH v6 09/11] libxl__device_disk_local_attach: wait for state "connected"

Ian Jackson — Fri, 18 May 2012 22:47:04 +0800

Stefano Stabellini writes ("[PATCH v6 09/11] libxl__device_disk_local_attach: wait for state "connected""):
> In order to make sure that the block device is available and ready to be
> used, wait for state "connected" in the backend before returning.
>
> Signed-off-by: Stefano Stabellini

Acked-by: Ian Jackson

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
[lists.xen.org]

Re: [PATCH 1/2] lib: Proportions with flexible period

Jan Kara — Fri, 18 May 2012 22:46:08 +0800

On Fri 18-05-12 12:43:44, Peter Zijlstra wrote:
> On Tue, 2012-05-15 at 17:43 +0200, Jan Kara wrote:
> > +void __fprop_inc_percpu_max(struct fprop_global *p,
> > + struct fprop_local_percpu *pl, int max_frac)
> > +{
> > + if (unlikely(max_frac < 100)) {
> > + unsigned long numerator, denominator;
> > +
> > + fprop_fraction_percpu(p, pl, &numerator, &denominator);
> > + if (numerator > ((long long)denominator) * max_frac / 100)
> > + return;
>
> Another thing, your fprop_fraction_percpu() can he horribly expensive
> due to using _sum() (and to a lesser degree the retry), remember that
> this function is called for _every_ page written out.
The retry happens only when new period is declared while
fprop_fraction_percpu() is running. So that should be rather exceptional.
Regarding the _sum I agree, luckily that's easy enough to fix.

> Esp. on the mega fast storage (multi-spindle or SSD) they're pushing cpu
> limits as it is with iops, we should be very careful not to make it more
> expensive than absolutely needed.
Yup.

> > + } else
> > + fprop_reflect_period_percpu(p, pl);
> > + __percpu_counter_add(&pl->events, 1, PROP_BATCH);
> > + percpu_counter_add(&p->events, 1);
> > +}

Honza
--
Jan Kara
SUSE Labs, CR
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at [vger.kernel.org]
Please read the FAQ at [www.tux.org]

Re: [Xen-devel] [PATCH v6 05/11] libxl: introduce libxl__device_disk_add

Ian Campbell — Fri, 18 May 2012 22:45:02 +0800

On Fri, 2012-05-18 at 15:36 +0100, Ian Jackson wrote:
> There is no need IMO to move the
> internal function to libxl_internal.c.

I've always thought the existence libxl_internal.c was just begging for
people to group functions along the wrong criteria anyway...

The whole concept of what lives where inside tools/libxl/libxl*.c is all
pretty ad-hoc anyway.

Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
[lists.xen.org]

Re: [Xen-devel] [PATCH v6 08/11] xl/libxl: implement QDISK libxl_device_disk_local_attach

Ian Jackson — Fri, 18 May 2012 22:44:10 +0800

Stefano Stabellini writes ("[PATCH v6 08/11] xl/libxl: implement QDISK libxl_device_disk_local_attach"):
> - Spawn a QEMU instance at boot time to handle disk local attach
> requests.

> Signed-off-by: Stefano Stabellini
> Acked-by:Ian Campbell

Acked-by: Ian Jackson

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
[lists.xen.org]

Re: lockdep false positive in double_lock_balance()?

Steven Rostedt — Fri, 18 May 2012 22:44:08 +0800

On Thu, 2012-05-17 at 21:19 +0200, Peter Zijlstra wrote:

> Something like this should fix it I think..
>
>
> ---
> kernel/sched/rt.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/kernel/sched/rt.c b/kernel/sched/rt.c
> index c5565c3..b649108 100644
> --- a/kernel/sched/rt.c
> +++ b/kernel/sched/rt.c
> @@ -1556,7 +1556,7 @@ static struct rq *find_lock_lowest_rq(struct task_struct *task, struct rq *rq)
> task_running(rq, task) ||
> !task->on_rq)) {
>
> - raw_spin_unlock(&lowest_rq->lock);
> + double_unlock_balance(rq, lowest_rq);

Acked-by: Steven Rostedt

-- Steve

> lowest_rq = NULL;
> break;
> }

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

Re: [Xen-devel] [PATCH v6 07/11] libxl: introduce libxl__alloc_vdev

Ian Jackson — Fri, 18 May 2012 22:43:16 +0800

Stefano Stabellini writes ("[PATCH v6 07/11] libxl: introduce libxl__alloc_vdev"):
> Introduce libxl__alloc_vdev: find a spare virtual block device in the
> domain passed as argument.
...
> +/* Same as in Linux.
> + * The maximum buffer length is 32 bytes.
> + * The code is safe thanks to the upper bound enforced by the caller. */
> +static char *encode_disk_name(char *ptr, unsigned int n)

So this says that encode_disk_name may use up to 32 bytes in *ptr
(including or excluding the trailing nul?)

_Why_ may the maximum buffer length used be 32 bytes ? Also, "maximum
buffer length" is a confusing phrase because it seems to refer to the
/actual/ length of the buffer rather than the amount /used/.

And this...

> + char *ret = libxl__zalloc(gc, 32);

... allocates a 32 byte buffer which we then fill with ...

> + strcpy(ret, "xvd");
> + ptr = encode_disk_name(ret + 3, offset);

3 characters plus the encoded disk name. So possibly using up to 36
bytes.

In fact it seems to me that there could be a much tighter upper bound
on the length of output of encode_disk_name (based on arithmetic) but
it needs to be properly calculated and documented and perhaps put in a
#define.

Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
[lists.xen.org]

Re: [Xen-devel] [PATCH] libxl: Introduce LIBXL_DOMAIN_TYPE_INVALID to make gcc happy

Ian Campbell — Fri, 18 May 2012 22:43:15 +0800

On Fri, 2012-05-18 at 15:30 +0100, Dario Faggioli wrote:
> On Fri, 2012-05-18 at 14:21 +0200, Christoph Egger wrote:
> > Introduce LIBXL_DOMAIN_TYPE_INVALID.
> > Change libxl__domain_type() to return LIBXL_DOMAIN_TYPE_INVALID rather
> > hardcoding -1.
> > Adjust code pieces where gcc 4.5.3 claims that LIBXL_DOMAIN_TYPE_INVALID
> > is not handled.
> >
> > This fixes the build error with gcc 4.5.3 reported here:
> > [lists.xen.org]
> >
> I was also running into that error and thus I applied this patch, which
> brought me here:
>
> libxl.c: In function ‘libxl_primary_console_exec’:
> libxl.c:1233:14: error: ‘LIBXL_DOMAIN_TYPE_INVALID’ undeclared (first use in this function)
> libxl.c:1233:14: note: each undeclared identifier is reported only once for each function it appears in
>
> Which actually seems to be right:
>
> $ grep LIBXL_DOMAIN_TYPE_INVALID tools/* -R
> tools/libxl/libxl_dm.c: case LIBXL_DOMAIN_TYPE_INVALID:
> tools/libxl/libxl_dm.c: case LIBXL_DOMAIN_TYPE_INVALID:
> tools/libxl/libxl_dom.c: return LIBXL_DOMAIN_TYPE_INVALID;
> tools/libxl/libxl_dom.c: return LIBXL_DOMAIN_TYPE_INVALID;
> tools/libxl/libxl.c: case LIBXL_DOMAIN_TYPE_INVALID:
>
> Am I missing something?

This should be defined in tools/libxl/libxl_types.idl but the patch
doesn't seem to add it.

Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
[lists.xen.org]

Re: [PATCH 1/2] lib: Proportions with flexible period

Jan Kara — Fri, 18 May 2012 22:43:14 +0800

On Thu 17-05-12 23:56:45, Peter Zijlstra wrote:
> On Tue, 2012-05-15 at 17:43 +0200, Jan Kara wrote:
> > +void fprop_fraction_percpu(struct fprop_global *p,
> > + struct fprop_local_percpu *pl,
> > + unsigned long *numerator, unsigned long *denominator)
> > +{
> > + unsigned int seq;
> > + s64 den;
> > +
> > + do {
> > + seq = read_seqcount_begin(&p->sequence);
> > + fprop_reflect_period_percpu(p, pl);
> > + *numerator = percpu_counter_read_positive(&pl->events);
> > + den = percpu_counter_read(&p->events);
> > + if (den <= 0)
> > + den = percpu_counter_sum(&p->events);
> > + *denominator = den;
> > + } while (read_seqcount_retry(&p->sequence, seq));
> > +}
>
>
> why not use percpu_counter_read_positive(&p->events) and ditch
> percpu_counter_sum()? That sum can be terribly expensive..
Yes. I'm actually not sure why I used the _sum here... Thanks for
spotting this.

Honza
--
Jan Kara
SUSE Labs, CR
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at [vger.kernel.org]
Please read the FAQ at [www.tux.org]

Re: [PATCH 2/3] x86: x2apic/cluster: Make use of lowest priority delivery mode

Cyrill Gorcunov — Fri, 18 May 2012 22:43:08 +0800

On Fri, May 18, 2012 at 12:26:41PM +0200, Alexander Gordeev wrote:
> Currently x2APIC in logical destination mode delivers interrupts to a
> single CPU, no matter how many CPUs were specified in the destination
> cpumask.
>
> This fix enables delivery of interrupts to multiple CPUs by bit-ORing
> Logical IDs of destination CPUs that have matching Cluster ID.
>
> Because only one cluster could be specified in a message destination
> address, the destination cpumask is tried for a cluster that contains
> maximum number of CPUs matching this cpumask. The CPUs in this cluster
> are selected to receive the interrupts while all other CPUs (in the
> cpumask) are ignored.

Hi Alexander,

I'm a bit confused, we do compose destination id from target cpumask
and send one ipi per _cluster_ with all cpus belonging to this cluster
OR'ed. So if my memory doesn't betray me all specified cpus in cluster
should obtain this message. Thus I'm wondering where do you find that
only one apic obtains ipi? (note i don't have the real physical
machine with x2apic enabled handy to test, so please share
the experience).

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

Re: [Xen-devel] [PATCH v6 05/11] libxl: introduce libxl__device_disk_add

Ian Jackson — Fri, 18 May 2012 22:39:11 +0800

Stefano Stabellini writes ("[PATCH v6 05/11] libxl: introduce libxl__device_disk_add"):
> Introduce libxl__device_disk_add that takes an additional
> xs_transaction_t paramter.
> Implement libxl_device_disk_add using libxl__device_disk_add.

Can't this be done in such a way that the diff isn't "delete this
function completely" followed by "here is a new function" ?

I don't really understand why you want to move the function at all,
TBH. I think keeping the public wrapper and the internal
implementation together is fine. There is no need IMO to move the
internal function to libxl_internal.c.

Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
[lists.xen.org]

Re: [PATCH v6 3/3] gpio: TS-5500 GPIO support

Vivien Didelot — Fri, 18 May 2012 22:39:10 +0800

Hi Grant,

On Thu, 2012-05-17 at 16:59 -0600, Grant Likely wrote:
> On Thu, May 17, 2012 at 3:40 PM, Vivien Didelot
> wrote:
> > On Thu, 2012-05-17 at 15:06 -0600, Grant Likely wrote:
> >> > arch/x86/include/asm/ts5500.h | 62 ++++++++
> >>
> >> Why the separate header file? What will use these defines? I
> >> normally expect driver-specific defines to be in the driver .c file
> >> directly; particularly for things like gpio drivers which should be a
> >> generic interface that doesn't need to export symbols.
> >
> > Should an intermediate driver directly use values for GPIOs instead of
> > these symbols? For example, how should a temperature sensor plugged on
> > this platform refer to inputs and outputs?
>
> Tell me more about this platform. Where does the data about
> connections come from? Is it a purpose-built embedded system?

This is a generic purpose platform, the GPIO bus is exposed on an
external DB25 connector. End-users can use it however they like.

> Is the
> GPIO controller described in ACPI? (probably not since GPIOs were only
> added to ACPI in v5) Does the end-user attach her own hardware to the
> board like the temperature sensor you describe? If so, is that
> hardware driven by kernel drivers or user-space drivers?

Both in fact. For instance, we are connecting an SHT15
humidity/temperature sensor, which already has support in the kernel.

>
> For userspace drivers you can get information about the GPIO number
> assignments from /sys, but it isn't well documented and can probably
> be improved.
>
> If it is kernelspace, then you really need a way to add data about the
> platform to the kernel at runtime. Having it statically compiled in
> isn't a very good solution. I would recommend injecting configuration
> data into the kernel from userspace. You could invent something, but
> that wouldn't be very portable. Xilinx has done some work on this
> using Flattened Device Tree and the firmware loading infrastructure.
> The kernel requests a .dtb (device tree blob) from userspace and uses
> that to configure devices. That may do the job for you. GPIO and
> platform device infrastructure already have FDT support which will
> help you here. I expect it could be done with an ACPI fragment too,
> but I just don't know of anybody having done any work in this area.
>
> That probably isn't the answer you want though since I assume you just
> need to get something that works rather than investing a whole bunch
> of time on generic infrastructure.

Exactly.

> What I would recommend is for your
> platform setup code to use a notifier to wait for the
> BUS_NOTIFY_BOUND_DRIVER event and then register the temperature sensor
> with the correct gpio number at that time (because once you have a
> reference to the gpio controller you can calculate the assigned gpio
> numbers).

Thanks.

I've been working on pushing this code mainline for a while. To
summarize, for you to accept this code, you'd prefer me to move every
symbol into the driver itself (in addition to addressing your and Joe's
other requests), and then we're good?

>
> g.

Thanks,
Vivien.

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

Re: [PATCH 1/2] perf: Add persistent event facilities

Peter Zijlstra — Fri, 18 May 2012 22:38:08 +0800

On Fri, 2012-05-18 at 16:21 +0200, Borislav Petkov wrote:
> On Fri, May 18, 2012 at 04:14:22PM +0200, Peter Zijlstra wrote:
> > On Fri, 2012-05-18 at 16:09 +0200, Borislav Petkov wrote:
> > > What happens here if a second user wants to enable a second set of
> > > persistent events, allocate a second set of per-CPU buffers?
> >
> > I'm not seeing the relation to VM_SHARED here.
>
> Well, in the sense that if mmap fails in the !VM_SHARED case, I can't
> enable any persistent events except the ones which are enabled and so
> I need to allocate another set of resources for that second persistent
> events user.

Right, well, you could actually allow operations on the fd, just not a
second mapping :-)

Anyway.. I'd push that error back to the user. If they need a second
set, let them create it.

Do you really need multiple consumers for your MCE stuff? If so, what
would be the problem of using VM_SHARED?
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at [vger.kernel.org]
Please read the FAQ at [www.tux.org]

Re: [Xen-devel] [PATCH v6 04/11] libxl: move libxl__device_from_disk to libxl_internal.c

Ian Jackson — Fri, 18 May 2012 22:37:03 +0800

Stefano Stabellini writes ("[PATCH v6 04/11] libxl: move libxl__device_from_disk to libxl_internal.c"):
> Signed-off-by: Stefano Stabellini

Acked-by: Ian Jackson

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
[lists.xen.org]

Re: [Xen-devel] [PATCH v6 02/11] libxl: libxl__device_disk_local_attach return a new libxl_device_disk

Ian Jackson — Fri, 18 May 2012 22:37:02 +0800

Stefano Stabellini writes ("[PATCH v6 02/11] libxl: libxl__device_disk_local_attach return a new libxl_device_disk"):
> Introduce a new libxl_device_disk* parameter to
> libxl__device_disk_local_attach, the parameter is allocated by the
> caller. libxl__device_disk_local_attach is going to fill the new disk
> with informations about the new locally attached disk. The new
> libxl_device_disk should be passed to libxl_device_disk_local_detach
> afterwards.

In this declaration:

> @@ -1767,6 +1768,7 @@ struct libxl__bootloader_state {
> libxl__bootloader_console_callback *console_available;
> libxl_domain_build_info *info; /* u.pv.{kernel,ramdisk,cmdline} updated */
> libxl_device_disk *disk;
> + libxl_device_disk tmpdisk;
> uint32_t domid;

We need information about what this "tmpdisk" is. All of the other
parameters here are input parameters, except as otherwise noted in the
comment.

Also I'm not convinced that "tmpdisk" is quite the right name. You
also need to explain the distinction between "disk" and "tmpdisk".

Perhaps:

const libxl_device_disk *disk; /* as specified by user */
libxl_device_disk localdisk;
/* Should be zeroed by caller on entry. Will be filled in by
* bootloader machinery; represents the local attachment of the
* disk for the benefit of the bootloader. Must be detached by
* the caller using libxl__device_disk_local_detach, but only
* after the domain's kernel and initramfs have been loaded into
* memory and the file references disposed of. */

?

The implementation looks sane.

Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
[lists.xen.org]

Re: [PATCH 2/2] block: Convert BDI proportion calculations to flexible proportions

Peter Zijlstra — Fri, 18 May 2012 22:35:08 +0800

On Fri, 2012-05-18 at 16:24 +0200, Jan Kara wrote:
> Yeah, that should be easy enough so I'll try it that way since I presume
> it's nicer to power usage to use deferred timers if it's reasonably
> possible.

Btw, your current scheme also drifts. Since you do jiffes + 3*HZ you
period might actually be longer if the timer got delayed.

If you keep an external jiffies count like:

unsigned long period_jiffies = jiffies;

void my_timer_func()
{
unsigned long delta = jiffies - period_jiffies;
unsigned long periods = delta / 3*HZ;

age(periods);

period_jiffies += 3*HZ * periods;
mod_timer(&my_timer, period_jiffies);
}

it all works without drift (+- bugs of course).
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at [vger.kernel.org]
Please read the FAQ at [www.tux.org]

Re: [Xen-devel] [PATCH] libxl: Introduce LIBXL_DOMAIN_TYPE_INVALID to make gcc happy

Dario Faggioli — Fri, 18 May 2012 22:34:10 +0800

On Fri, 2012-05-18 at 14:21 +0200, Christoph Egger wrote:
> Introduce LIBXL_DOMAIN_TYPE_INVALID.
> Change libxl__domain_type() to return LIBXL_DOMAIN_TYPE_INVALID rather
> hardcoding -1.
> Adjust code pieces where gcc 4.5.3 claims that LIBXL_DOMAIN_TYPE_INVALID
> is not handled.
>
> This fixes the build error with gcc 4.5.3 reported here:
> [lists.xen.org]
>
I was also running into that error and thus I applied this patch, which
brought me here:

libxl.c: In function ‘libxl_primary_console_exec’:
libxl.c:1233:14: error: ‘LIBXL_DOMAIN_TYPE_INVALID’ undeclared (first use in this function)
libxl.c:1233:14: note: each undeclared identifier is reported only once for each function it appears in

Which actually seems to be right:

$ grep LIBXL_DOMAIN_TYPE_INVALID tools/* -R
tools/libxl/libxl_dm.c: case LIBXL_DOMAIN_TYPE_INVALID:
tools/libxl/libxl_dm.c: case LIBXL_DOMAIN_TYPE_INVALID:
tools/libxl/libxl_dom.c: return LIBXL_DOMAIN_TYPE_INVALID;
tools/libxl/libxl_dom.c: return LIBXL_DOMAIN_TYPE_INVALID;
tools/libxl/libxl.c: case LIBXL_DOMAIN_TYPE_INVALID:

Am I missing something?

Dario

--
<> (Raistlin Majere)
-----------------------------------------------------------------
Dario Faggioli, Ph.D, [retis.sssup.it]
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
[lists.xen.org]

Re: [PATCH 1/2] lib: Proportions with flexible period

Jan Kara — Fri, 18 May 2012 22:34:09 +0800

On Thu 17-05-12 23:35:12, Peter Zijlstra wrote:
> On Tue, 2012-05-15 at 17:43 +0200, Jan Kara wrote:
> > + if (numerator > ((long long)denominator) * max_frac / 100)
>
> Does that even compile on 32bit archs?
>
> Operator precedence is *,/ left-to-right, so that's:
>
> long long t1 = (long long)denom * max_frac
> long long t2 = t1 / 100;
>
> Which is a 64bit signed division.
>
> There's a reason I used that max_prop_frac thing you removed, it avoids
> having to do the division at all and allows a mult and shift instead.
Yeah, I misunderstood it's purpose when I read the code originally. I'll
put it back to avoid the division since this is a hot path.

Honza
--
Jan Kara
SUSE Labs, CR
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at [vger.kernel.org]
Please read the FAQ at [www.tux.org]

Re: [PATCH v24b] RAS: Add a tracepoint for reporting memory controller events

Mauro Carvalho Chehab — Fri, 18 May 2012 22:33:09 +0800

Em 18-05-2012 11:05, Borislav Petkov escreveu:
> On Fri, May 18, 2012 at 10:23:39AM -0300, Mauro Carvalho Chehab wrote:
>> Em 18-05-2012 09:43, Borislav Petkov escreveu:
>>
>> (offensive/ironic comments skipped)
>>
>>> you fail to see that I'm trying to make this as generic as possible and
>>> not fit it into anything so that EVERYONE can use it.
>>
>> Tracepoints are generic enough. Everyone can use it. If need new fields are
>> needed, just add a new tracepoint.
>
> This is exactly what I'm talking about - this is wrong on so many levels
> that it ain't even funny! You can add as many tracepoints if you want
> to to your pet project but in the kernel we add one tracepoint for one
> thing which fits all users.
>
> I want to see you be successful at adding a new tracepoint
> to, say, prepare_task_switch() in kernel/sched/core.c because
> trace_sched_switch() does not fit your needs.

Ok, but you won't use trace_sched_switch() as a memory tracepoint, as
they represent different things.

Memory errors are different than CPU errors. So, their tracepoints
will be different.

> Right, and this is why I'm asking you to have the following tracepoint proto:
>
> + TP_PROTO(const unsigned int err_type,
> + const unsigned int mc_index,
> + const char *error_msg,
> + const char *label,
> + const char *location,
> + const char *detail)
>
> where detail contains all the crap one driver adds for technical people
> to pinpoint where the error is.
>
> And not have _TWO_ detail arguments!

And what I'm saying is that this should be, instead:

+ TP_PROTO(const unsigned int err_type,
+ const unsigned int mc_index,
+ const char *error_msg,
+ const char *label,
+ int layer0,
+ int layer1,
+ int layer2,
+ unsigned long pfn,
+ unsigned long offset,
+ unsigned long grain,
+ unsigned long syndrome,
+ const char *driver_detail),

So, having just one detail argument, filled by the driver, and not
folding "location" and core "details" into strings, but keeping as they
are.

>
> Btw, the output looks like this here:
>
> <...>-2723 [001] .N.. 89.107045: mc_event: Corrected error: on memory stick "unknown memory" (mc:0 csrow:3 channel:1 page:0x5bac7 offset:0x388 grain:0 syndrome:0xfc5b driver:amd64_edac)
>
> Come to think of it, the "driver:amd64_edac" is not really needed
> because on every single system there's only one EDAC driver running and
> I don't think the fact that we're telling in the tracepoint who detected
> the error is meaningfull information.
>
> Which means, you can remove the EDAC_MOD_STR argument you're passing to
> edac_mc_handle_error() and have one less argument.

That's what I said you, but you didn't seem to agree, as I understood that
you've required to keep "amd64_edac" at the trace, due to:
[markmail.org].

If you're ok, I'll remove EDAC_MOD_STR argument from the amd64_edac calls
on a separate patch (with can be merged latter with the patch that converted
amd64_edac to the new function calls).

>
>>>> Also, doing that will avoid the extra effort of joining everything into
>>>> a single string, and then breaking them back into their individual fields
>>>> on userspace.
>>>
>>> I'm being told this is very easy to do in userspace in your garden
>>> variety language.
>>
>> Well, ask the one that told you that to write a parser that will get all
>> EDAC error reports on all drivers, on several kernel versions using dmesg.
>
> Nobody is talking about dmesg - I'm talking about the "const char
> *detail" string in the tracepoint above and how I want it to be a single
> char * so that userspace can read it out and fumble with it the way it
> sees fit.
>
> Having "detail" and "other_detail" is simply misleading and unneeded and
> a clear sign that this ABI is not designed properly yet.
>
> That's it.

I agree with that.

It follows a version removing the "core_detail" parameter from the trace.

I removed the changes at amd64_edac from it. I'll address them on a latter
patch, removing the EDAC_MOD_STR argument from the calls, after we've
done with this patch.

Regards,
Mauro

-

RAS: Add a tracepoint for reporting memory controller events

From: Mauro Carvalho Chehab

Add a new tracepoint-based hardware events report method for
reporting Memory Controller events.

Part of the description bellow is shamelessly copied from Tony
Luck's notes about the Hardware Error BoF during LPC 2010 [1].
Tony, thanks for your notes and discussions to generate the
h/w error reporting requirements.

[1] [lwn.net]

We have several subsystems & methods for reporting hardware errors:

1) EDAC ("Error Detection and Correction"). In its original form
this consisted of a platform specific driver that read topology
information and error counts from chipset registers and reported
the results via a sysfs interface.

2) mcelog - x86 specific decoding of machine check bank registers
reporting in binary form via /dev/mcelog. Recent additions make use
of the APEI extensions that were documented in version 4.0a of the
ACPI specification to acquire more information about errors without
having to rely reading chipset registers directly. A user level
programs decodes into somewhat human readable format.

3) drivers/edac/mce_amd.c - this driver hooks into the mcelog path and
decodes errors reported via machine check bank registers in AMD
processors to the console log using printk();

Each of these mechanisms has a band of followers ... and none
of them appear to meet all the needs of all users.

As part of a RAS subsystem, let's encapsulate the memory error hardware
events into a trace facility.

The tracepoint printk will be displayed like:

mc_event: (Corrected|Uncorrected|Fatal) error:[error msg] on memory stick "[label]" ([location] [edac_mc detail] [driver_detail])

Where:
[error msg] is the driver-specific error message
(e. g. "memory read", "bus error", ...);
[location] is the location in terms of memory controller and
branch/channel/slot, channel/slot or csrow/channel;
[label] is the memory stick label;
[edac_mc detail] describes the address location of the error
and the syndrome;
[driver detail] is driver-specifig error message details,
when needed/provided (e. g. "area:DMA", ...)

For example:

mc_event: Corrected error:memory read on memory stick "DIMM_1A" (mc:0 channel:0 slot:0 page:0x586b6e offset:0xa66 grain:32 syndrome:0x0 area:DMA)

Of course, any userspace tools meant to handle errors should not parse
the above data. They should, instead, use the binary fields provided by
the tracepoint, mapping them directly into their Management Information
Base.

NOTE: The original patch was providing an additional mechanism for
MCA-based trace events that also contained MCA error register data.
However, as no agreement was reached so far for the MCA-based trace
events, for now, let's add events only for memory errors.
A latter patch is planned to change the tracepoint, for those types
of event.

Cc: Aristeu Rozanski
Cc: Doug Thompson
Cc: Steven Rostedt
Cc: Frederic Weisbecker
Cc: Ingo Molnar
Signed-off-by: Mauro Carvalho Chehab

diff --git a/drivers/edac/edac_core.h b/drivers/edac/edac_core.h
index f06ce9a..eee7360 100644
--- a/drivers/edac/edac_core.h
+++ b/drivers/edac/edac_core.h
@@ -468,7 +468,7 @@ void edac_mc_handle_error(const enum hw_event_mc_err_type type,
const int layer2,
const char *msg,
const char *other_detail,
- const void *mcelog);
+ const void *arch_log);

/*
* edac_device APIs
diff --git a/drivers/edac/edac_mc.c b/drivers/edac/edac_mc.c
index 10f3750..5006461 100644
--- a/drivers/edac/edac_mc.c
+++ b/drivers/edac/edac_mc.c
@@ -33,6 +33,10 @@
#include "edac_core.h"
#include "edac_module.h"

+#define CREATE_TRACE_POINTS
+#define TRACE_INCLUDE_PATH ../../include/ras
+#include
+
/* lock to memory controller's control array */
static DEFINE_MUTEX(mem_ctls_mutex);
static LIST_HEAD(mc_devices);
@@ -384,6 +388,7 @@ struct mem_ctl_info *edac_mc_alloc(unsigned mc_num,
* which will perform kobj unregistration and the actual free
* will occur during the kobject callback operation
*/
+
return mci;
}
EXPORT_SYMBOL_GPL(edac_mc_alloc);
@@ -909,12 +914,12 @@ static void edac_ce_error(struct mem_ctl_info *mci,
if (edac_mc_get_log_ce()) {
if (other_detail && *other_detail)
edac_mc_printk(mci, KERN_WARNING,
- "CE %s on %s (%s%s - %s)\n",
+ "CE %s on %s (%s %s - %s)\n",
msg, label, location,
detail, other_detail);
else
edac_mc_printk(mci, KERN_WARNING,
- "CE %s on %s (%s%s)\n",
+ "CE %s on %s (%s %s)\n",
msg, label, location,
detail);
}
@@ -953,12 +958,12 @@ static void edac_ue_error(struct mem_ctl_info *mci,
if (edac_mc_get_log_ue()) {
if (other_detail && *other_detail)
edac_mc_printk(mci, KERN_WARNING,
- "UE %s on %s (%s%s - %s)\n",
+ "UE %s on %s (%s %s - %s)\n",
msg, label, location, detail,
other_detail);
else
edac_mc_printk(mci, KERN_WARNING,
- "UE %s on %s (%s%s)\n",
+ "UE %s on %s (%s %s)\n",
msg, label, location, detail);
}

@@ -975,6 +980,27 @@ static void edac_ue_error(struct mem_ctl_info *mci,
}

#define OTHER_LABEL " or "
+
+/**
+ * edac_mc_handle_error - reports a memory event to userspace
+ *
+ * @type: severity of the error (CE/UE/Fatal)
+ * @mci: a struct mem_ctl_info pointer
+ * @page_frame_number: mem page where the error occurred
+ * @offset_in_page: offset of the error inside the page
+ * @syndrome: ECC syndrome
+ * @layer0: Memory layer0 position
+ * @layer1: Memory layer2 position
+ * @layer2: Memory layer3 position
+ * @msg: Message meaningful to the end users that
+ * explains the event
+ * @other_detail: Technical details about the event that
+ * may help hardware manufacturers and
+ * EDAC developers to analyse the event
+ * @arch_log: Architecture-specific struct that can
+ * be used to add extended information to the
+ * tracepoint, like dumping MCE registers.
+ */
void edac_mc_handle_error(const enum hw_event_mc_err_type type,
struct mem_ctl_info *mci,
const unsigned long page_frame_number,
@@ -985,7 +1011,7 @@ void edac_mc_handle_error(const enum hw_event_mc_err_type type,
const int layer2,
const char *msg,
const char *other_detail,
- const void *mcelog)
+ const void *arch_log)
{
/* FIXME: too much for stack: move it to some pre-alocated area */
char detail[80], location[80];
@@ -1120,6 +1146,13 @@ void edac_mc_handle_error(const enum hw_event_mc_err_type type,
edac_layer_name[mci->layers.type],
pos);
}
+ if (p > location)
+ *(p - 1) = '\0';
+
+ /* Report the error via the trace interface */
+ trace_mc_event(type, mci->mc_idx, msg, label, layer0, layer1, layer2,
+ page_frame_number, offset_in_page, grain, syndrome,
+ other_detail);

/* Memory type dependent details about the error */
if (type == HW_EVENT_ERR_CORRECTED) {
diff --git a/include/ras/ras_event.h b/include/ras/ras_event.h
new file mode 100644
index 0000000..65835df
--- /dev/null
+++ b/include/ras/ras_event.h
@@ -0,0 +1,100 @@
+#undef TRACE_SYSTEM
+#define TRACE_SYSTEM ras
+#define TRACE_INCLUDE_FILE ras_event
+
+#if !defined(_TRACE_HW_EVENT_MC_H) || defined(TRACE_HEADER_MULTI_READ)
+#define _TRACE_HW_EVENT_MC_H
+
+#include
+#include
+#include
+
+/*
+ * Hardware Events Report
+ *
+ * Those events are generated when hardware detected a corrected or
+ * uncorrected event, and are meant to replace the current API to report
+ * errors defined on both EDAC and MCE subsystems.
+ *
+ * FIXME: Add events for handling memory errors originated from the
+ * MCE subsystem.
+ */
+
+/*
+ * Hardware-independent Memory Controller specific events
+ */
+
+/*
+ * Default error mechanisms for Memory Controller errors (CE and UE)
+ */
+TRACE_EVENT(mc_event,
+
+ TP_PROTO(const unsigned int err_type,
+ const unsigned int mc_index,
+ const char *error_msg,
+ const char *label,
+ int layer0,
+ int layer1,
+ int layer2,
+ unsigned long pfn,
+ unsigned long offset,
+ unsigned long grain,
+ unsigned long syndrome,
+ const char *driver_detail),
+
+ TP_ARGS(err_type, mc_index, error_msg, label, layer0, layer1, layer2,
+ pfn, offset, grain, syndrome, driver_detail),
+
+ TP_STRUCT__entry(
+ __field( unsigned int, err_type )
+ __field( unsigned int, mc_index )
+ __string( msg, error_msg )
+ __string( label, label )
+ __field( int, layer0 )
+ __field( int, layer1 )
+ __field( int, layer2 )
+ __field( int, pfn )
+ __field( int, offset )
+ __field( int, grain )
+ __field( int, syndrome )
+ __string( driver_detail, driver_detail )
+ ),
+
+ TP_fast_assign(
+ __entry->err_type = err_type;
+ __entry->mc_index = mc_index;
+ __assign_str(msg, error_msg);
+ __assign_str(label, label);
+ __entry->layer0 = layer0;
+ __entry->layer1 = layer1;
+ __entry->layer2 = layer2;
+ __entry->pfn = pfn;
+ __entry->offset = offset;
+ __entry->grain = grain;
+ __entry->syndrome = syndrome;
+ __assign_str(driver_detail, driver_detail);
+ ),
+
+ TP_printk("%s error:%s%s on memory stick \"%s\" (mc:%d location:%d:%d:%d page:0x%08x offset:0x%08x grain:%d syndrome:0x%08x%s%s)",
+ (__entry->err_type == HW_EVENT_ERR_CORRECTED) ? "Corrected" :
+ ((__entry->err_type == HW_EVENT_ERR_FATAL) ?
+ "Fatal" : "Uncorrected"),
+ ((char *)__get_str(msg))[0] ? " " : "",
+ __get_str(msg),
+ __get_str(label),
+ __entry->mc_index,
+ __entry->layer0,
+ __entry->layer1,
+ __entry->layer2,
+ __entry->pfn,
+ __entry->offset,
+ __entry->grain,
+ __entry->syndrome,
+ ((char *)__get_str(driver_detail))[0] ? " " : "",
+ __get_str(driver_detail))
+);
+
+#endif /* _TRACE_HW_EVENT_MC_H */
+
+/* This part must be outside protection */
+#include
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at [vger.kernel.org]
Please read the FAQ at [www.tux.org]

Re: [RESEND,PATCH] DCA, x86: fix invalid memory access in DCA core

Jiang Liu — Fri, 18 May 2012 22:32:09 +0800

On 05/18/2012 10:10 PM, Sosnowski, Maciej wrote:
> On Thu, May 07, May 10, 2012 3:59 AM, Jiang Liu wrote:
>>
>> Hi Maciej,
>> I feel we may also need to tune the multiple IOH support in DCA.
>> Multiple IOH support is disabled for CB3.0 devices, how about CB3.1 devices
>> in Ivrbridge or SandyBridge? Does the hardware limitation still exist? Or
>> could we support multiple IOHs with IvyBridge and SandyBridge?
>> If multiple IOH is supported, I think we should move the logic to
>> disable multiple IOH support for CB3.0 from DCA core into ioatdma. I have
>> also prepared two patches for that two.
>> Thanks!
>>
>
> At this point I do not think we would need to tune multiple IOH for DCA.
> The limitation you mention applies only to CB3.0. I do not think DCA is supported
> with Sandy Bridge / Ivy Bridge regardless of multi-IOH case but let me confirm it
> yet.
It seems that Intel introduces DDIO technology for IvyBridge. Does it replace DCA
technology on new platforms?
Thanks!

>
> Thanks,
> Maciej

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

Re: [Xen-devel] [PATCH v6 01/11] libxl: make libxl_device_disk_local_attach/detach internal functions

Ian Jackson — Fri, 18 May 2012 22:28:03 +0800

Stefano Stabellini writes ("[PATCH v6 01/11] libxl: make libxl_device_disk_local_attach/detach internal functions"):
> Changes in v5:
>
> - remove _hidden from the implementation.
>
> Signed-off-by: Stefano Stabellini
> Acked-by: Ian Campbell

Acked-by: Ian Jackson

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
[lists.xen.org]