* [PATCH 1/3] lib/vsprintf.c: Add %pU - ptr to a UUID/GUID
From: Joe Perches @ 2009-09-27 5:57 UTC (permalink / raw)
To: linux-kernel, netdev, Greg KH
In-Reply-To: <cover.1254030722.git.joe@perches.com>
UUID/GUIDs are somewhat common in kernel source.
Standardize the printed style of UUID/GUIDs by using
another extension to %p.
%pU: 01020304:0506:0708:090a:0b0c0d0e0f10
%pUr: 04030201:0605:0807:0a09:0b0c0d0e0f10
%pU[r]X:Use upper case hex
Signed-off-by: Joe Perches <joe@perches.com>
---
lib/vsprintf.c | 58 +++++++++++++++++++++++++++++++++++++++++++++++++++++++-
1 files changed, 57 insertions(+), 1 deletions(-)
diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index b91839e..68a49bb 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -790,6 +790,53 @@ static char *ip4_addr_string(char *buf, char *end, const u8 *addr,
return string(buf, end, ip4_addr, spec);
}
+static char *uuid_string(char *buf, char *end, const u8 *addr,
+ struct printf_spec spec, const char *fmt)
+{
+ char uuid[sizeof("xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx")];
+ char *p = uuid;
+ int i;
+ static const u8 r[16] = {3,2,1,0,5,4,7,6,8,9,10,11,12,13,14,15};
+ static const u8 n[16] = {0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15};
+ const u8 *index = n;
+ bool uc = false;
+
+ while (isalnum(*(++fmt))) {
+ switch (*fmt) {
+ case 'r':
+ index = r;
+ break;
+ case 'X':
+ uc = true;
+ break;
+ }
+ }
+
+ for (i = 0; i < 16; i++) {
+ p = pack_hex_byte(p, addr[index[i]]);
+ switch (i) {
+ case 3:
+ case 5:
+ case 7:
+ case 9:
+ *p++ = '-';
+ break;
+ }
+ }
+
+ *p = 0;
+
+ if (uc) {
+ p = uuid;
+ while (*p) {
+ *p = toupper(*p);
+ p++;
+ }
+ }
+
+ return string(buf, end, uuid, spec);
+}
+
/*
* Show a '%p' thing. A kernel extension is that the '%p' is followed
* by an extra set of alphanumeric characters that are extended format
@@ -814,6 +861,13 @@ static char *ip4_addr_string(char *buf, char *end, const u8 *addr,
* IPv4 uses dot-separated decimal with leading 0's (010.123.045.006)
* - 'I6c' for IPv6 addresses printed as specified by
* http://www.ietf.org/id/draft-kawamura-ipv6-text-representation-03.txt
+ * - 'U' For a 16 byte UUID/GUID, it prints the UUID/GUID in the form
+ * "xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx"
+ * Options for %pU are:
+ * 'X' use upper case hex digits
+ * 'r' use LE byte order for U32 and U16s equivalents. Use indices:
+ * [3][2][1][0]-[5][4]-[7][6]-[9][8]-[10]...[15]
+ *
* Note: The difference between 'S' and 'F' is that on ia64 and ppc64
* function pointers are really function descriptors, which contain a
* pointer to the real address.
@@ -828,9 +882,9 @@ static char *pointer(const char *fmt, char *buf, char *end, void *ptr,
case 'F':
case 'f':
ptr = dereference_function_descriptor(ptr);
- case 's':
/* Fallthrough */
case 'S':
+ case 's':
return symbol_string(buf, end, ptr, spec, *fmt);
case 'R':
return resource_string(buf, end, ptr, spec);
@@ -853,6 +907,8 @@ static char *pointer(const char *fmt, char *buf, char *end, void *ptr,
return ip4_addr_string(buf, end, ptr, spec, fmt);
}
break;
+ case 'U':
+ return uuid_string(buf, end, ptr, spec, fmt);
}
spec.flags |= SMALL;
if (spec.field_width == -1) {
--
1.6.3.1.10.g659a0.dirty
^ permalink raw reply related
* [PATCH 2/3] treewide: use %pU to print UUID/GUIDs
From: Joe Perches @ 2009-09-27 5:57 UTC (permalink / raw)
To: linux-kernel, netdev, Greg KH
In-Reply-To: <cover.1254030722.git.joe@perches.com>
Converted individual GUID/UUID printing functions
to use the new %pU[Xr] in lib/vsprintf.c
Signed-off-by: Joe Perches <joe@perches.com>
---
drivers/char/random.c | 10 +---
drivers/firmware/dmi_scan.c | 5 +--
drivers/md/md.c | 16 ++------
drivers/media/video/uvc/uvc_ctrl.c | 69 ++++++++++++++++------------------
drivers/media/video/uvc/uvc_driver.c | 7 +--
drivers/media/video/uvc/uvcvideo.h | 10 -----
fs/gfs2/sys.c | 16 +------
fs/ubifs/debug.c | 9 +---
fs/ubifs/super.c | 7 +---
fs/xfs/xfs_log_recover.c | 14 ++-----
include/linux/efi.h | 6 +--
11 files changed, 54 insertions(+), 115 deletions(-)
diff --git a/drivers/char/random.c b/drivers/char/random.c
index 04b505e..7104df9 100644
--- a/drivers/char/random.c
+++ b/drivers/char/random.c
@@ -1245,12 +1245,8 @@ static int proc_do_uuid(ctl_table *table, int write,
if (uuid[8] == 0)
generate_random_uuid(uuid);
- sprintf(buf, "%02x%02x%02x%02x-%02x%02x-%02x%02x-%02x%02x-"
- "%02x%02x%02x%02x%02x%02x",
- uuid[0], uuid[1], uuid[2], uuid[3],
- uuid[4], uuid[5], uuid[6], uuid[7],
- uuid[8], uuid[9], uuid[10], uuid[11],
- uuid[12], uuid[13], uuid[14], uuid[15]);
+ sprintf(buf, "%pU", uuid);
+
fake_table.data = buf;
fake_table.maxlen = sizeof(buf);
@@ -1350,7 +1346,7 @@ ctl_table random_table[] = {
/********************************************************************
*
- * Random funtions for networking
+ * Random functions for networking
*
********************************************************************/
diff --git a/drivers/firmware/dmi_scan.c b/drivers/firmware/dmi_scan.c
index 938100f..c0deabb 100644
--- a/drivers/firmware/dmi_scan.c
+++ b/drivers/firmware/dmi_scan.c
@@ -169,10 +169,7 @@ static void __init dmi_save_uuid(const struct dmi_header *dm, int slot, int inde
if (!s)
return;
- sprintf(s,
- "%02X%02X%02X%02X-%02X%02X-%02X%02X-%02X%02X-%02X%02X%02X%02X%02X%02X",
- d[0], d[1], d[2], d[3], d[4], d[5], d[6], d[7],
- d[8], d[9], d[10], d[11], d[12], d[13], d[14], d[15]);
+ sprintf(s, "%pUX", d);
dmi_ident[slot] = s;
}
diff --git a/drivers/md/md.c b/drivers/md/md.c
index 26ba42a..68b52d7 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -1813,15 +1813,11 @@ static void print_sb_1(struct mdp_superblock_1 *sb)
uuid = sb->set_uuid;
printk(KERN_INFO
- "md: SB: (V:%u) (F:0x%08x) Array-ID:<%02x%02x%02x%02x"
- ":%02x%02x:%02x%02x:%02x%02x:%02x%02x%02x%02x%02x%02x>\n"
+ "md: SB: (V:%u) (F:0x%08x) Array-ID:<%pU>\n"
"md: Name: \"%s\" CT:%llu\n",
le32_to_cpu(sb->major_version),
le32_to_cpu(sb->feature_map),
- uuid[0], uuid[1], uuid[2], uuid[3],
- uuid[4], uuid[5], uuid[6], uuid[7],
- uuid[8], uuid[9], uuid[10], uuid[11],
- uuid[12], uuid[13], uuid[14], uuid[15],
+ uuid,
sb->set_name,
(unsigned long long)le64_to_cpu(sb->ctime)
& MD_SUPERBLOCK_1_TIME_SEC_MASK);
@@ -1830,8 +1826,7 @@ static void print_sb_1(struct mdp_superblock_1 *sb)
printk(KERN_INFO
"md: L%u SZ%llu RD:%u LO:%u CS:%u DO:%llu DS:%llu SO:%llu"
" RO:%llu\n"
- "md: Dev:%08x UUID: %02x%02x%02x%02x:%02x%02x:%02x%02x:%02x%02x"
- ":%02x%02x%02x%02x%02x%02x\n"
+ "md: Dev:%08x UUID: %pU\n"
"md: (F:0x%08x) UT:%llu Events:%llu ResyncOffset:%llu CSUM:0x%08x\n"
"md: (MaxDev:%u) \n",
le32_to_cpu(sb->level),
@@ -1844,10 +1839,7 @@ static void print_sb_1(struct mdp_superblock_1 *sb)
(unsigned long long)le64_to_cpu(sb->super_offset),
(unsigned long long)le64_to_cpu(sb->recovery_offset),
le32_to_cpu(sb->dev_number),
- uuid[0], uuid[1], uuid[2], uuid[3],
- uuid[4], uuid[5], uuid[6], uuid[7],
- uuid[8], uuid[9], uuid[10], uuid[11],
- uuid[12], uuid[13], uuid[14], uuid[15],
+ uuid,
sb->devflags,
(unsigned long long)le64_to_cpu(sb->utime) & MD_SUPERBLOCK_1_TIME_SEC_MASK,
(unsigned long long)le64_to_cpu(sb->events),
diff --git a/drivers/media/video/uvc/uvc_ctrl.c b/drivers/media/video/uvc/uvc_ctrl.c
index c3225a5..2959e46 100644
--- a/drivers/media/video/uvc/uvc_ctrl.c
+++ b/drivers/media/video/uvc/uvc_ctrl.c
@@ -1093,8 +1093,8 @@ int uvc_xu_ctrl_query(struct uvc_video_chain *chain,
if (!found) {
uvc_trace(UVC_TRACE_CONTROL,
- "Control " UVC_GUID_FORMAT "/%u not found.\n",
- UVC_GUID_ARGS(entity->extension.guidExtensionCode),
+ "Control %pUr/%u not found.\n",
+ entity->extension.guidExtensionCode,
xctrl->selector);
return -EINVAL;
}
@@ -1171,9 +1171,9 @@ int uvc_ctrl_resume_device(struct uvc_device *dev)
(ctrl->info->flags & UVC_CONTROL_RESTORE) == 0)
continue;
- printk(KERN_INFO "restoring control " UVC_GUID_FORMAT
- "/%u/%u\n", UVC_GUID_ARGS(ctrl->info->entity),
- ctrl->info->index, ctrl->info->selector);
+ printk(KERN_INFO "restoring control %pUr/%u/%u\n",
+ ctrl->info->entity,
+ ctrl->info->index, ctrl->info->selector);
ctrl->dirty = 1;
}
@@ -1228,46 +1228,43 @@ static void uvc_ctrl_add_ctrl(struct uvc_device *dev,
dev->intfnum, info->selector, (__u8 *)&size, 2);
if (ret < 0) {
uvc_trace(UVC_TRACE_CONTROL, "GET_LEN failed on "
- "control " UVC_GUID_FORMAT "/%u (%d).\n",
- UVC_GUID_ARGS(info->entity), info->selector,
- ret);
+ "control %pUr/%u (%d).\n",
+ info->entity, info->selector, ret);
return;
}
if (info->size != le16_to_cpu(size)) {
- uvc_trace(UVC_TRACE_CONTROL, "Control " UVC_GUID_FORMAT
- "/%u size doesn't match user supplied "
- "value.\n", UVC_GUID_ARGS(info->entity),
- info->selector);
+ uvc_trace(UVC_TRACE_CONTROL,
+ "Control %pUr/%u size doesn't match user supplied value.\n",
+ info->entity, info->selector);
return;
}
ret = uvc_query_ctrl(dev, UVC_GET_INFO, ctrl->entity->id,
dev->intfnum, info->selector, &inf, 1);
if (ret < 0) {
- uvc_trace(UVC_TRACE_CONTROL, "GET_INFO failed on "
- "control " UVC_GUID_FORMAT "/%u (%d).\n",
- UVC_GUID_ARGS(info->entity), info->selector,
- ret);
+ uvc_trace(UVC_TRACE_CONTROL,
+ "GET_INFO failed on control %pUr/%u (%d).\n",
+ info->entity, info->selector, ret);
return;
}
flags = info->flags;
if (((flags & UVC_CONTROL_GET_CUR) && !(inf & (1 << 0))) ||
((flags & UVC_CONTROL_SET_CUR) && !(inf & (1 << 1)))) {
- uvc_trace(UVC_TRACE_CONTROL, "Control "
- UVC_GUID_FORMAT "/%u flags don't match "
- "supported operations.\n",
- UVC_GUID_ARGS(info->entity), info->selector);
+ uvc_trace(UVC_TRACE_CONTROL,
+ "Control %pUr/%u flags don't match supported operations.\n",
+ info->entity, info->selector);
return;
}
}
ctrl->info = info;
ctrl->data = kmalloc(ctrl->info->size * UVC_CTRL_NDATA, GFP_KERNEL);
- uvc_trace(UVC_TRACE_CONTROL, "Added control " UVC_GUID_FORMAT "/%u "
- "to device %s entity %u\n", UVC_GUID_ARGS(ctrl->info->entity),
- ctrl->info->selector, dev->udev->devpath, entity->id);
+ uvc_trace(UVC_TRACE_CONTROL,
+ "Added control %pUr/%u to device %s entity %u\n",
+ ctrl->info->entity, ctrl->info->selector,
+ dev->udev->devpath, entity->id);
}
/*
@@ -1293,17 +1290,16 @@ int uvc_ctrl_add_info(struct uvc_control_info *info)
continue;
if (ctrl->selector == info->selector) {
- uvc_trace(UVC_TRACE_CONTROL, "Control "
- UVC_GUID_FORMAT "/%u is already defined.\n",
- UVC_GUID_ARGS(info->entity), info->selector);
+ uvc_trace(UVC_TRACE_CONTROL,
+ "Control %pUr/%u is already defined.\n",
+ info->entity, info->selector);
ret = -EEXIST;
goto end;
}
if (ctrl->index == info->index) {
- uvc_trace(UVC_TRACE_CONTROL, "Control "
- UVC_GUID_FORMAT "/%u would overwrite index "
- "%d.\n", UVC_GUID_ARGS(info->entity),
- info->selector, info->index);
+ uvc_trace(UVC_TRACE_CONTROL,
+ "Control %pUr/%u would overwrite index %d.\n",
+ info->entity, info->selector, info->index);
ret = -EEXIST;
goto end;
}
@@ -1344,10 +1340,9 @@ int uvc_ctrl_add_mapping(struct uvc_control_mapping *mapping)
continue;
if (info->size * 8 < mapping->size + mapping->offset) {
- uvc_trace(UVC_TRACE_CONTROL, "Mapping '%s' would "
- "overflow control " UVC_GUID_FORMAT "/%u\n",
- mapping->name, UVC_GUID_ARGS(info->entity),
- info->selector);
+ uvc_trace(UVC_TRACE_CONTROL,
+ "Mapping '%s' would overflow control %pUr/%u\n",
+ mapping->name, info->entity, info->selector);
ret = -EOVERFLOW;
goto end;
}
@@ -1366,9 +1361,9 @@ int uvc_ctrl_add_mapping(struct uvc_control_mapping *mapping)
mapping->ctrl = info;
list_add_tail(&mapping->list, &info->mappings);
- uvc_trace(UVC_TRACE_CONTROL, "Adding mapping %s to control "
- UVC_GUID_FORMAT "/%u.\n", mapping->name,
- UVC_GUID_ARGS(info->entity), info->selector);
+ uvc_trace(UVC_TRACE_CONTROL,
+ "Adding mapping %s to control %pUr/%u.\n",
+ mapping->name, info->entity, info->selector);
ret = 0;
break;
diff --git a/drivers/media/video/uvc/uvc_driver.c b/drivers/media/video/uvc/uvc_driver.c
index 8756be5..647d0a2 100644
--- a/drivers/media/video/uvc/uvc_driver.c
+++ b/drivers/media/video/uvc/uvc_driver.c
@@ -328,11 +328,10 @@ static int uvc_parse_format(struct uvc_device *dev,
sizeof format->name);
format->fcc = fmtdesc->fcc;
} else {
- uvc_printk(KERN_INFO, "Unknown video format "
- UVC_GUID_FORMAT "\n",
- UVC_GUID_ARGS(&buffer[5]));
+ uvc_printk(KERN_INFO, "Unknown video format %pUr\n",
+ &buffer[5]);
snprintf(format->name, sizeof format->name,
- UVC_GUID_FORMAT, UVC_GUID_ARGS(&buffer[5]));
+ "%pUr", &Buffer[5]);
format->fcc = 0;
}
diff --git a/drivers/media/video/uvc/uvcvideo.h b/drivers/media/video/uvc/uvcvideo.h
index e7958aa..9f4a437 100644
--- a/drivers/media/video/uvc/uvcvideo.h
+++ b/drivers/media/video/uvc/uvcvideo.h
@@ -555,16 +555,6 @@ extern unsigned int uvc_trace_param;
#define uvc_printk(level, msg...) \
printk(level "uvcvideo: " msg)
-#define UVC_GUID_FORMAT "%02x%02x%02x%02x-%02x%02x-%02x%02x-%02x%02x-" \
- "%02x%02x%02x%02x%02x%02x"
-#define UVC_GUID_ARGS(guid) \
- (guid)[3], (guid)[2], (guid)[1], (guid)[0], \
- (guid)[5], (guid)[4], \
- (guid)[7], (guid)[6], \
- (guid)[8], (guid)[9], \
- (guid)[10], (guid)[11], (guid)[12], \
- (guid)[13], (guid)[14], (guid)[15]
-
/* --------------------------------------------------------------------------
* Internal functions.
*/
diff --git a/fs/gfs2/sys.c b/fs/gfs2/sys.c
index 4463297..56901be 100644
--- a/fs/gfs2/sys.c
+++ b/fs/gfs2/sys.c
@@ -85,11 +85,7 @@ static ssize_t uuid_show(struct gfs2_sbd *sdp, char *buf)
buf[0] = '\0';
if (!gfs2_uuid_valid(uuid))
return 0;
- return snprintf(buf, PAGE_SIZE, "%02X%02X%02X%02X-%02X%02X-"
- "%02X%02X-%02X%02X-%02X%02X%02X%02X%02X%02X\n",
- uuid[0], uuid[1], uuid[2], uuid[3], uuid[4], uuid[5],
- uuid[6], uuid[7], uuid[8], uuid[9], uuid[10], uuid[11],
- uuid[12], uuid[13], uuid[14], uuid[15]);
+ return snprintf(buf, PAGE_SIZE, "%pUX\n", uuid);
}
static ssize_t freeze_show(struct gfs2_sbd *sdp, char *buf)
@@ -573,14 +569,8 @@ static int gfs2_uevent(struct kset *kset, struct kobject *kobj,
add_uevent_var(env, "LOCKPROTO=%s", sdp->sd_proto_name);
if (!sdp->sd_args.ar_spectator)
add_uevent_var(env, "JOURNALID=%u", sdp->sd_lockstruct.ls_jid);
- if (gfs2_uuid_valid(uuid)) {
- add_uevent_var(env, "UUID=%02X%02X%02X%02X-%02X%02X-%02X%02X-"
- "%02X%02X-%02X%02X%02X%02X%02X%02X",
- uuid[0], uuid[1], uuid[2], uuid[3], uuid[4],
- uuid[5], uuid[6], uuid[7], uuid[8], uuid[9],
- uuid[10], uuid[11], uuid[12], uuid[13],
- uuid[14], uuid[15]);
- }
+ if (gfs2_uuid_valid(uuid))
+ add_uevent_var(env, "UUID=%pUX", uuid);
return 0;
}
diff --git a/fs/ubifs/debug.c b/fs/ubifs/debug.c
index dbc093a..b16779e 100644
--- a/fs/ubifs/debug.c
+++ b/fs/ubifs/debug.c
@@ -350,13 +350,8 @@ void dbg_dump_node(const struct ubifs_info *c, const void *node)
le32_to_cpu(sup->fmt_version));
printk(KERN_DEBUG "\ttime_gran %u\n",
le32_to_cpu(sup->time_gran));
- printk(KERN_DEBUG "\tUUID %02X%02X%02X%02X-%02X%02X"
- "-%02X%02X-%02X%02X-%02X%02X%02X%02X%02X%02X\n",
- sup->uuid[0], sup->uuid[1], sup->uuid[2], sup->uuid[3],
- sup->uuid[4], sup->uuid[5], sup->uuid[6], sup->uuid[7],
- sup->uuid[8], sup->uuid[9], sup->uuid[10], sup->uuid[11],
- sup->uuid[12], sup->uuid[13], sup->uuid[14],
- sup->uuid[15]);
+ printk(KERN_DEBUG "\tUUID %pUX\n",
+ sup->uuid);
break;
}
case UBIFS_MST_NODE:
diff --git a/fs/ubifs/super.c b/fs/ubifs/super.c
index 333e181..7d59ab7 100644
--- a/fs/ubifs/super.c
+++ b/fs/ubifs/super.c
@@ -1393,12 +1393,7 @@ static int mount_ubifs(struct ubifs_info *c)
c->leb_size, c->leb_size >> 10);
dbg_msg("data journal heads: %d",
c->jhead_cnt - NONDATA_JHEADS_CNT);
- dbg_msg("UUID: %02X%02X%02X%02X-%02X%02X"
- "-%02X%02X-%02X%02X-%02X%02X%02X%02X%02X%02X",
- c->uuid[0], c->uuid[1], c->uuid[2], c->uuid[3],
- c->uuid[4], c->uuid[5], c->uuid[6], c->uuid[7],
- c->uuid[8], c->uuid[9], c->uuid[10], c->uuid[11],
- c->uuid[12], c->uuid[13], c->uuid[14], c->uuid[15]);
+ dbg_msg("UUID: %pUX", c->uuid);
dbg_msg("big_lpt %d", c->big_lpt);
dbg_msg("log LEBs: %d (%d - %d)",
c->log_lebs, UBIFS_LOG_LNUM, c->log_last);
diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c
index 1099395..3b8e3df 100644
--- a/fs/xfs/xfs_log_recover.c
+++ b/fs/xfs/xfs_log_recover.c
@@ -225,16 +225,10 @@ xlog_header_check_dump(
xfs_mount_t *mp,
xlog_rec_header_t *head)
{
- int b;
-
- cmn_err(CE_DEBUG, "%s: SB : uuid = ", __func__);
- for (b = 0; b < 16; b++)
- cmn_err(CE_DEBUG, "%02x", ((__uint8_t *)&mp->m_sb.sb_uuid)[b]);
- cmn_err(CE_DEBUG, ", fmt = %d\n", XLOG_FMT);
- cmn_err(CE_DEBUG, " log : uuid = ");
- for (b = 0; b < 16; b++)
- cmn_err(CE_DEBUG, "%02x", ((__uint8_t *)&head->h_fs_uuid)[b]);
- cmn_err(CE_DEBUG, ", fmt = %d\n", be32_to_cpu(head->h_fmt));
+ cmn_err(CE_DEBUG, "%s: SB : uuid = %pU, fmt = %d\n",
+ __func__, &mp->m_sb.sb_uuid, XLOG_FMT);
+ cmn_err(CE_DEBUG, " log : uuid = %pU, fmt = %d\n",
+ &head->h_fs_uuid, be32_to_cpu(head->h_fmt));
}
#else
#define xlog_header_check_dump(mp, head)
diff --git a/include/linux/efi.h b/include/linux/efi.h
index ce4581f..dd85b39 100644
--- a/include/linux/efi.h
+++ b/include/linux/efi.h
@@ -280,11 +280,7 @@ efi_guidcmp (efi_guid_t left, efi_guid_t right)
static inline char *
efi_guid_unparse(efi_guid_t *guid, char *out)
{
- sprintf(out, "%02x%02x%02x%02x-%02x%02x-%02x%02x-%02x%02x-%02x%02x%02x%02x%02x%02x",
- guid->b[3], guid->b[2], guid->b[1], guid->b[0],
- guid->b[5], guid->b[4], guid->b[7], guid->b[6],
- guid->b[8], guid->b[9], guid->b[10], guid->b[11],
- guid->b[12], guid->b[13], guid->b[14], guid->b[15]);
+ sprintf(out, "%pUr", guid->b);
return out;
}
--
1.6.3.1.10.g659a0.dirty
^ permalink raw reply related
* Re: [PATCH] ax25: Fix ax25_cb refcounting in ax25_ctl_ioctl
From: Ralf Baechle DL5RB @ 2009-09-27 7:23 UTC (permalink / raw)
To: Jarek Poplawski
Cc: David Miller, Bernard Pidoux F6BVP, Bernard Pidoux,
Linux Netdev List, linux-hams
In-Reply-To: <20090925183504.GA3307@del.dom.local>
On Fri, Sep 25, 2009 at 08:35:04PM +0200, Jarek Poplawski wrote:
> > > This bug isn't responsible for these oopses here, but looks quite
> > > obviously. (I'm not sure if it's easy to test/hit with the common
> > > tools.)
> >
> > The issue your patch fixes is obvious enough.
>
> Yes, with new code there would be no doubt. But here, if you know it's
> worked for some time, you wonder if you're not blind. |-)
Most of of the ioctls are used by AX.25 userland which does error
checking on user supplied values so userland will never attempt invalid
ioctl calls. So no surprise this went unnoticed.
Ralf
^ permalink raw reply
* Re: [PATCHv5 3/3] vhost_net: a kernel-level virtio server
From: Michael S. Tsirkin @ 2009-09-27 7:43 UTC (permalink / raw)
To: Ira W. Snyder
Cc: netdev, virtualization, kvm, linux-kernel, mingo, linux-mm, akpm,
hpa, gregory.haskins, Rusty Russell, s.hetze
In-Reply-To: <20090925170158.GA16014@ovro.caltech.edu>
On Fri, Sep 25, 2009 at 10:01:58AM -0700, Ira W. Snyder wrote:
> > + case VHOST_SET_VRING_KICK:
> > + r = copy_from_user(&f, argp, sizeof f);
> > + if (r < 0)
> > + break;
> > + eventfp = f.fd == -1 ? NULL : eventfd_fget(f.fd);
> > + if (IS_ERR(eventfp))
> > + return PTR_ERR(eventfp);
> > + if (eventfp != vq->kick) {
> > + pollstop = filep = vq->kick;
> > + pollstart = vq->kick = eventfp;
> > + } else
> > + filep = eventfp;
> > + break;
> > + case VHOST_SET_VRING_CALL:
> > + r = copy_from_user(&f, argp, sizeof f);
> > + if (r < 0)
> > + break;
> > + eventfp = f.fd == -1 ? NULL : eventfd_fget(f.fd);
> > + if (IS_ERR(eventfp))
> > + return PTR_ERR(eventfp);
> > + if (eventfp != vq->call) {
> > + filep = vq->call;
> > + ctx = vq->call_ctx;
> > + vq->call = eventfp;
> > + vq->call_ctx = eventfp ?
> > + eventfd_ctx_fileget(eventfp) : NULL;
> > + } else
> > + filep = eventfp;
> > + break;
> > + case VHOST_SET_VRING_ERR:
> > + r = copy_from_user(&f, argp, sizeof f);
> > + if (r < 0)
> > + break;
> > + eventfp = f.fd == -1 ? NULL : eventfd_fget(f.fd);
> > + if (IS_ERR(eventfp))
> > + return PTR_ERR(eventfp);
> > + if (eventfp != vq->error) {
> > + filep = vq->error;
> > + vq->error = eventfp;
> > + ctx = vq->error_ctx;
> > + vq->error_ctx = eventfp ?
> > + eventfd_ctx_fileget(eventfp) : NULL;
> > + } else
> > + filep = eventfp;
> > + break;
>
> I'm not sure how these eventfd's save a trip to userspace.
>
> AFAICT, eventfd's cannot be used to signal another part of the kernel,
> they can only be used to wake up userspace.
Yes, they can. See irqfd code in virt/kvm/eventfd.c.
> In my system, when an IRQ for kick() comes in, I have an eventfd which
> gets signalled to notify userspace. When I want to send a call(), I have
> to use a special ioctl(), just like lguest does.
>
> Doesn't this mean that for call(), vhost is just going to signal an
> eventfd to wake up userspace, which is then going to call ioctl(), and
> then we're back in kernelspace. Seems like a wasted userspace
> round-trip.
>
> Or am I mis-reading this code?
Yes. Kernel can poll eventfd and deliver an interrupt directly
without involving userspace.
> PS - you can see my current code at:
> http://www.mmarray.org/~iws/virtio-phys/
>
> Thanks,
> Ira
>
> > + default:
> > + r = -ENOIOCTLCMD;
> > + }
> > +
> > + if (pollstop && vq->handle_kick)
> > + vhost_poll_stop(&vq->poll);
> > +
> > + if (ctx)
> > + eventfd_ctx_put(ctx);
> > + if (filep)
> > + fput(filep);
> > +
> > + if (pollstart && vq->handle_kick)
> > + vhost_poll_start(&vq->poll, vq->kick);
> > +
> > + mutex_unlock(&vq->mutex);
> > +
> > + if (pollstop && vq->handle_kick)
> > + vhost_poll_flush(&vq->poll);
> > + return 0;
> > +}
> > +
> > +long vhost_dev_ioctl(struct vhost_dev *d, unsigned int ioctl, unsigned long arg)
> > +{
> > + void __user *argp = (void __user *)arg;
> > + long r;
> > +
> > + mutex_lock(&d->mutex);
> > + /* If you are not the owner, you can become one */
> > + if (ioctl == VHOST_SET_OWNER) {
> > + r = vhost_dev_set_owner(d);
> > + goto done;
> > + }
> > +
> > + /* You must be the owner to do anything else */
> > + r = vhost_dev_check_owner(d);
> > + if (r)
> > + goto done;
> > +
> > + switch (ioctl) {
> > + case VHOST_SET_MEM_TABLE:
> > + r = vhost_set_memory(d, argp);
> > + break;
> > + default:
> > + r = vhost_set_vring(d, ioctl, argp);
> > + break;
> > + }
> > +done:
> > + mutex_unlock(&d->mutex);
> > + return r;
> > +}
> > +
> > +static const struct vhost_memory_region *find_region(struct vhost_memory *mem,
> > + __u64 addr, __u32 len)
> > +{
> > + struct vhost_memory_region *reg;
> > + int i;
> > + /* linear search is not brilliant, but we really have on the order of 6
> > + * regions in practice */
> > + for (i = 0; i < mem->nregions; ++i) {
> > + reg = mem->regions + i;
> > + if (reg->guest_phys_addr <= addr &&
> > + reg->guest_phys_addr + reg->memory_size - 1 >= addr)
> > + return reg;
> > + }
> > + return NULL;
> > +}
> > +
> > +int translate_desc(struct vhost_dev *dev, u64 addr, u32 len,
> > + struct iovec iov[], int iov_size)
> > +{
> > + const struct vhost_memory_region *reg;
> > + struct vhost_memory *mem;
> > + struct iovec *_iov;
> > + u64 s = 0;
> > + int ret = 0;
> > +
> > + rcu_read_lock();
> > +
> > + mem = rcu_dereference(dev->memory);
> > + while ((u64)len > s) {
> > + u64 size;
> > + if (ret >= iov_size) {
> > + ret = -ENOBUFS;
> > + break;
> > + }
> > + reg = find_region(mem, addr, len);
> > + if (!reg) {
> > + ret = -EFAULT;
> > + break;
> > + }
> > + _iov = iov + ret;
> > + size = reg->memory_size - addr + reg->guest_phys_addr;
> > + _iov->iov_len = min((u64)len, size);
> > + _iov->iov_base = (void *)
> > + (reg->userspace_addr + addr - reg->guest_phys_addr);
> > + s += size;
> > + addr += size;
> > + ++ret;
> > + }
> > +
> > + rcu_read_unlock();
> > + return ret;
> > +}
> > +
> > +/* Each buffer in the virtqueues is actually a chain of descriptors. This
> > + * function returns the next descriptor in the chain, or vq->vring.num if we're
> > + * at the end. */
> > +static unsigned next_desc(struct vhost_virtqueue *vq, struct vring_desc *desc)
> > +{
> > + unsigned int next;
> > +
> > + /* If this descriptor says it doesn't chain, we're done. */
> > + if (!(desc->flags & VRING_DESC_F_NEXT))
> > + return vq->num;
> > +
> > + /* Check they're not leading us off end of descriptors. */
> > + next = desc->next;
> > + /* Make sure compiler knows to grab that: we don't want it changing! */
> > + /* We will use the result as an index in an array, so most
> > + * architectures only need a compiler barrier here. */
> > + read_barrier_depends();
> > +
> > + if (next >= vq->num) {
> > + vq_err(vq, "Desc next is %u > %u", next, vq->num);
> > + return vq->num;
> > + }
> > +
> > + return next;
> > +}
> > +
> > +/* This looks in the virtqueue and for the first available buffer, and converts
> > + * it to an iovec for convenient access. Since descriptors consist of some
> > + * number of output then some number of input descriptors, it's actually two
> > + * iovecs, but we pack them into one and note how many of each there were.
> > + *
> > + * This function returns the descriptor number found, or vq->num (which
> > + * is never a valid descriptor number) if none was found. */
> > +unsigned vhost_get_vq_desc(struct vhost_dev *dev, struct vhost_virtqueue *vq,
> > + struct iovec iov[],
> > + unsigned int *out_num, unsigned int *in_num)
> > +{
> > + struct vring_desc desc;
> > + unsigned int i, head;
> > + u16 last_avail_idx;
> > + int ret;
> > +
> > + /* Check it isn't doing very strange things with descriptor numbers. */
> > + last_avail_idx = vq->last_avail_idx;
> > + if (get_user(vq->avail_idx, &vq->avail->idx)) {
> > + vq_err(vq, "Failed to access avail idx at %p\n",
> > + &vq->avail->idx);
> > + return vq->num;
> > + }
> > +
> > + if ((u16)(vq->avail_idx - last_avail_idx) > vq->num) {
> > + vq_err(vq, "Guest moved used index from %u to %u",
> > + last_avail_idx, vq->avail_idx);
> > + return vq->num;
> > + }
> > +
> > + /* If there's nothing new since last we looked, return invalid. */
> > + if (vq->avail_idx == last_avail_idx)
> > + return vq->num;
> > +
> > + /* Grab the next descriptor number they're advertising, and increment
> > + * the index we've seen. */
> > + if (get_user(head, &vq->avail->ring[last_avail_idx % vq->num])) {
> > + vq_err(vq, "Failed to read head: idx %d address %p\n",
> > + last_avail_idx,
> > + &vq->avail->ring[last_avail_idx % vq->num]);
> > + return vq->num;
> > + }
> > +
> > + /* If their number is silly, that's an error. */
> > + if (head >= vq->num) {
> > + vq_err(vq, "Guest says index %u > %u is available",
> > + head, vq->num);
> > + return vq->num;
> > + }
> > +
> > + vq->last_avail_idx++;
> > +
> > + /* When we start there are none of either input nor output. */
> > + *out_num = *in_num = 0;
> > +
> > + i = head;
> > + do {
> > + unsigned iov_count = *in_num + *out_num;
> > + if (copy_from_user(&desc, vq->desc + i, sizeof desc)) {
> > + vq_err(vq, "Failed to get descriptor: idx %d addr %p\n",
> > + i, vq->desc + i);
> > + return vq->num;
> > + }
> > + ret = translate_desc(dev, desc.addr, desc.len, iov + iov_count,
> > + VHOST_NET_MAX_SG - iov_count);
> > + if (ret < 0) {
> > + vq_err(vq, "Translation failure %d descriptor idx %d\n",
> > + ret, i);
> > + return vq->num;
> > + }
> > + /* If this is an input descriptor, increment that count. */
> > + if (desc.flags & VRING_DESC_F_WRITE)
> > + *in_num += ret;
> > + else {
> > + /* If it's an output descriptor, they're all supposed
> > + * to come before any input descriptors. */
> > + if (*in_num) {
> > + vq_err(vq, "Descriptor has out after in: "
> > + "idx %d\n", i);
> > + return vq->num;
> > + }
> > + *out_num += ret;
> > + }
> > + } while ((i = next_desc(vq, &desc)) != vq->num);
> > + return head;
> > +}
> > +
> > +/* Reverse the effect of vhost_get_vq_desc. Useful for error handling. */
> > +void vhost_discard_vq_desc(struct vhost_virtqueue *vq)
> > +{
> > + vq->last_avail_idx--;
> > +}
> > +
> > +/* After we've used one of their buffers, we tell them about it. We'll then
> > + * want to send them an interrupt, using vq->call. */
> > +int vhost_add_used(struct vhost_virtqueue *vq,
> > + unsigned int head, int len)
> > +{
> > + struct vring_used_elem *used;
> > +
> > + /* The virtqueue contains a ring of used buffers. Get a pointer to the
> > + * next entry in that used ring. */
> > + used = &vq->used->ring[vq->last_used_idx % vq->num];
> > + if (put_user(head, &used->id)) {
> > + vq_err(vq, "Failed to write used id");
> > + return -EFAULT;
> > + }
> > + if (put_user(len, &used->len)) {
> > + vq_err(vq, "Failed to write used len");
> > + return -EFAULT;
> > + }
> > + /* Make sure buffer is written before we update index. */
> > + wmb();
> > + if (put_user(vq->last_used_idx + 1, &vq->used->idx)) {
> > + vq_err(vq, "Failed to increment used idx");
> > + return -EFAULT;
> > + }
> > + vq->last_used_idx++;
> > + return 0;
> > +}
> > +
> > +/* This actually sends the interrupt for this virtqueue */
> > +void vhost_trigger_irq(struct vhost_dev *dev, struct vhost_virtqueue *vq)
> > +{
> > + __u16 flags = 0;
> > + if (get_user(flags, &vq->avail->flags)) {
> > + vq_err(vq, "Failed to get flags");
> > + return;
> > + }
> > +
> > + /* If they don't want an interrupt, don't send one, unless empty. */
> > + if ((flags & VRING_AVAIL_F_NO_INTERRUPT) &&
> > + (!vhost_has_feature(dev, VIRTIO_F_NOTIFY_ON_EMPTY) ||
> > + vq->avail_idx != vq->last_avail_idx))
> > + return;
> > +
> > + /* Send the Guest an interrupt tell them we used something up. */
> > + if (vq->call_ctx)
> > + eventfd_signal(vq->call_ctx, 1);
> > +}
> > +
> > +/* And here's the combo meal deal. Supersize me! */
> > +void vhost_add_used_and_trigger(struct vhost_dev *dev,
> > + struct vhost_virtqueue *vq,
> > + unsigned int head, int len)
> > +{
> > + vhost_add_used(vq, head, len);
> > + vhost_trigger_irq(dev, vq);
> > +}
> > +
> > +/* OK, now we need to know about added descriptors. */
> > +bool vhost_notify(struct vhost_virtqueue *vq)
> > +{
> > + int r;
> > + if (!(vq->used_flags & VRING_USED_F_NO_NOTIFY))
> > + return false;
> > + vq->used_flags &= ~VRING_USED_F_NO_NOTIFY;
> > + r = put_user(vq->used_flags, &vq->used->flags);
> > + if (r)
> > + vq_err(vq, "Failed to disable notification: %d\n", r);
> > + /* They could have slipped one in as we were doing that: make
> > + * sure it's written, tell caller it needs to check again. */
> > + mb();
> > + return true;
> > +}
> > +
> > +/* We don't need to be notified again. */
> > +void vhost_no_notify(struct vhost_virtqueue *vq)
> > +{
> > + int r;
> > + if (vq->used_flags & VRING_USED_F_NO_NOTIFY)
> > + return;
> > + vq->used_flags |= VRING_USED_F_NO_NOTIFY;
> > + r = put_user(vq->used_flags, &vq->used->flags);
> > + if (r)
> > + vq_err(vq, "Failed to enable notification: %d\n", r);
> > +}
> > +
> > +int vhost_init(void)
> > +{
> > + vhost_workqueue = create_workqueue("vhost");
> > + if (!vhost_workqueue)
> > + return -ENOMEM;
> > + return 0;
> > +}
> > +
> > +void vhost_cleanup(void)
> > +{
> > + destroy_workqueue(vhost_workqueue);
> > +}
> > diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
> > new file mode 100644
> > index 0000000..8e13d06
> > --- /dev/null
> > +++ b/drivers/vhost/vhost.h
> > @@ -0,0 +1,122 @@
> > +#ifndef _VHOST_H
> > +#define _VHOST_H
> > +
> > +#include <linux/eventfd.h>
> > +#include <linux/vhost.h>
> > +#include <linux/mm.h>
> > +#include <linux/mutex.h>
> > +#include <linux/workqueue.h>
> > +#include <linux/poll.h>
> > +#include <linux/file.h>
> > +#include <linux/skbuff.h>
> > +#include <linux/uio.h>
> > +#include <linux/virtio_config.h>
> > +
> > +struct vhost_device;
> > +
> > +enum {
> > + VHOST_NET_MAX_SG = MAX_SKB_FRAGS + 2,
> > +};
> > +
> > +/* Poll a file (eventfd or socket) */
> > +/* Note: there's nothing vhost specific about this structure. */
> > +struct vhost_poll {
> > + poll_table table;
> > + wait_queue_head_t *wqh;
> > + wait_queue_t wait;
> > + /* struct which will handle all actual work. */
> > + struct work_struct work;
> > + unsigned long mask;
> > +};
> > +
> > +void vhost_poll_init(struct vhost_poll *poll, work_func_t func,
> > + unsigned long mask);
> > +void vhost_poll_start(struct vhost_poll *poll, struct file *file);
> > +void vhost_poll_stop(struct vhost_poll *poll);
> > +void vhost_poll_flush(struct vhost_poll *poll);
> > +
> > +/* The virtqueue structure describes a queue attached to a device. */
> > +struct vhost_virtqueue {
> > + struct vhost_dev *dev;
> > +
> > + /* The actual ring of buffers. */
> > + struct mutex mutex;
> > + unsigned int num;
> > + struct vring_desc __user *desc;
> > + struct vring_avail __user *avail;
> > + struct vring_used __user *used;
> > + struct file *kick;
> > + struct file *call;
> > + struct file *error;
> > + struct eventfd_ctx *call_ctx;
> > + struct eventfd_ctx *error_ctx;
> > +
> > + struct vhost_poll poll;
> > +
> > + /* The routine to call when the Guest pings us, or timeout. */
> > + work_func_t handle_kick;
> > +
> > + /* Last available index we saw. */
> > + u16 last_avail_idx;
> > +
> > + /* Caches available index value from user. */
> > + u16 avail_idx;
> > +
> > + /* Last index we used. */
> > + u16 last_used_idx;
> > +
> > + /* Used flags */
> > + u16 used_flags;
> > +
> > + struct iovec iov[VHOST_NET_MAX_SG];
> > + struct iovec hdr[VHOST_NET_MAX_SG];
> > +};
> > +
> > +struct vhost_dev {
> > + /* Readers use RCU to access memory table pointer.
> > + * Writers use mutex below.*/
> > + struct vhost_memory *memory;
> > + struct mm_struct *mm;
> > + struct vhost_virtqueue *vqs;
> > + int nvqs;
> > + struct mutex mutex;
> > + unsigned acked_features;
> > +};
> > +
> > +long vhost_dev_init(struct vhost_dev *, struct vhost_virtqueue *vqs, int nvqs);
> > +long vhost_dev_check_owner(struct vhost_dev *);
> > +long vhost_dev_reset_owner(struct vhost_dev *);
> > +void vhost_dev_cleanup(struct vhost_dev *);
> > +long vhost_dev_ioctl(struct vhost_dev *, unsigned int ioctl, unsigned long arg);
> > +
> > +unsigned vhost_get_vq_desc(struct vhost_dev *, struct vhost_virtqueue *,
> > + struct iovec iov[],
> > + unsigned int *out_num, unsigned int *in_num);
> > +void vhost_discard_vq_desc(struct vhost_virtqueue *);
> > +
> > +int vhost_add_used(struct vhost_virtqueue *, unsigned int head, int len);
> > +void vhost_trigger_irq(struct vhost_dev *, struct vhost_virtqueue *);
> > +void vhost_add_used_and_trigger(struct vhost_dev *, struct vhost_virtqueue *,
> > + unsigned int head, int len);
> > +void vhost_no_notify(struct vhost_virtqueue *);
> > +bool vhost_notify(struct vhost_virtqueue *);
> > +
> > +int vhost_init(void);
> > +void vhost_cleanup(void);
> > +
> > +#define vq_err(vq, fmt, ...) do { \
> > + pr_debug(pr_fmt(fmt), ##__VA_ARGS__); \
> > + if ((vq)->error_ctx) \
> > + eventfd_signal((vq)->error_ctx, 1);\
> > + } while (0)
> > +
> > +enum {
> > + VHOST_FEATURES = 1 << VIRTIO_F_NOTIFY_ON_EMPTY,
> > +};
> > +
> > +static inline int vhost_has_feature(struct vhost_dev *dev, int bit)
> > +{
> > + return dev->acked_features & (1 << bit);
> > +}
> > +
> > +#endif
> > diff --git a/include/linux/Kbuild b/include/linux/Kbuild
> > index dec2f18..975df9a 100644
> > --- a/include/linux/Kbuild
> > +++ b/include/linux/Kbuild
> > @@ -360,6 +360,7 @@ unifdef-y += uio.h
> > unifdef-y += unistd.h
> > unifdef-y += usbdevice_fs.h
> > unifdef-y += utsname.h
> > +unifdef-y += vhost.h
> > unifdef-y += videodev2.h
> > unifdef-y += videodev.h
> > unifdef-y += virtio_config.h
> > diff --git a/include/linux/miscdevice.h b/include/linux/miscdevice.h
> > index 0521177..781a8bb 100644
> > --- a/include/linux/miscdevice.h
> > +++ b/include/linux/miscdevice.h
> > @@ -30,6 +30,7 @@
> > #define HPET_MINOR 228
> > #define FUSE_MINOR 229
> > #define KVM_MINOR 232
> > +#define VHOST_NET_MINOR 233
> > #define MISC_DYNAMIC_MINOR 255
> >
> > struct device;
> > diff --git a/include/linux/vhost.h b/include/linux/vhost.h
> > new file mode 100644
> > index 0000000..3f441a9
> > --- /dev/null
> > +++ b/include/linux/vhost.h
> > @@ -0,0 +1,101 @@
> > +#ifndef _LINUX_VHOST_H
> > +#define _LINUX_VHOST_H
> > +/* Userspace interface for in-kernel virtio accelerators. */
> > +
> > +/* vhost is used to reduce the number of system calls involved in virtio.
> > + *
> > + * Existing virtio net code is used in the guest without modification.
> > + *
> > + * This header includes interface used by userspace hypervisor for
> > + * device configuration.
> > + */
> > +
> > +#include <linux/types.h>
> > +#include <linux/compiler.h>
> > +#include <linux/ioctl.h>
> > +#include <linux/virtio_config.h>
> > +#include <linux/virtio_ring.h>
> > +
> > +struct vhost_vring_state {
> > + unsigned int index;
> > + unsigned int num;
> > +};
> > +
> > +struct vhost_vring_file {
> > + unsigned int index;
> > + int fd;
> > +};
> > +
> > +struct vhost_vring_addr {
> > + unsigned int index;
> > + unsigned int padding;
> > + __u64 user_addr;
> > +};
> > +
> > +struct vhost_memory_region {
> > + __u64 guest_phys_addr;
> > + __u64 memory_size; /* bytes */
> > + __u64 userspace_addr;
> > + __u64 padding; /* read/write protection? */
> > +};
> > +
> > +struct vhost_memory {
> > + __u32 nregions;
> > + __u32 padding;
> > + struct vhost_memory_region regions[0];
> > +};
> > +
> > +/* ioctls */
> > +
> > +#define VHOST_VIRTIO 0xAF
> > +
> > +/* Features bitmask for forward compatibility. Transport bits are used for
> > + * vhost specific features. */
> > +#define VHOST_GET_FEATURES _IOR(VHOST_VIRTIO, 0x00, __u64)
> > +#define VHOST_ACK_FEATURES _IOW(VHOST_VIRTIO, 0x00, __u64)
> > +
> > +/* Set current process as the (exclusive) owner of this file descriptor. This
> > + * must be called before any other vhost command. Further calls to
> > + * VHOST_OWNER_SET fail until VHOST_OWNER_RESET is called. */
> > +#define VHOST_SET_OWNER _IO(VHOST_VIRTIO, 0x01)
> > +/* Give up ownership, and reset the device to default values.
> > + * Allows subsequent call to VHOST_OWNER_SET to succeed. */
> > +#define VHOST_RESET_OWNER _IO(VHOST_VIRTIO, 0x02)
> > +
> > +/* Set up/modify memory layout */
> > +#define VHOST_SET_MEM_TABLE _IOW(VHOST_VIRTIO, 0x03, struct vhost_memory)
> > +
> > +/* Ring setup. These parameters can not be modified while ring is running
> > + * (bound to a device). */
> > +/* Set number of descriptors in ring */
> > +#define VHOST_SET_VRING_NUM _IOW(VHOST_VIRTIO, 0x10, struct vhost_vring_state)
> > +/* Start of array of descriptors (virtually contiguous) */
> > +#define VHOST_SET_VRING_DESC _IOW(VHOST_VIRTIO, 0x11, struct vhost_vring_addr)
> > +/* Used structure address */
> > +#define VHOST_SET_VRING_USED _IOW(VHOST_VIRTIO, 0x12, struct vhost_vring_addr)
> > +/* Available structure address */
> > +#define VHOST_SET_VRING_AVAIL _IOW(VHOST_VIRTIO, 0x13, struct vhost_vring_addr)
> > +/* Base value where queue looks for available descriptors */
> > +#define VHOST_SET_VRING_BASE _IOW(VHOST_VIRTIO, 0x14, struct vhost_vring_state)
> > +/* Get accessor: reads index, writes value in num */
> > +#define VHOST_GET_VRING_BASE _IOWR(VHOST_VIRTIO, 0x14, struct vhost_vring_state)
> > +
> > +/* The following ioctls use eventfd file descriptors to signal and poll
> > + * for events. */
> > +
> > +/* Set eventfd to poll for added buffers */
> > +#define VHOST_SET_VRING_KICK _IOW(VHOST_VIRTIO, 0x20, struct vhost_vring_file)
> > +/* Set eventfd to signal when buffers have beed used */
> > +#define VHOST_SET_VRING_CALL _IOW(VHOST_VIRTIO, 0x21, struct vhost_vring_file)
> > +/* Set eventfd to signal an error */
> > +#define VHOST_SET_VRING_ERR _IOW(VHOST_VIRTIO, 0x22, struct vhost_vring_file)
> > +
> > +/* VHOST_NET specific defines */
> > +
> > +/* Attach virtio net device to a raw socket. The socket must be already
> > + * bound to an ethernet device, this device will be used for transmit.
> > + * Pass -1 to unbind from the socket and the transmit device.
> > + * This can be used to stop the device (e.g. for migration). */
> > +#define VHOST_NET_SET_SOCKET _IOW(VHOST_VIRTIO, 0x30, int)
> > +
> > +#endif
> > --
> > 1.6.2.5
> > --
> > To unsubscribe from this list: send the line "unsubscribe netdev" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [PATCHv5 3/3] vhost_net: a kernel-level virtio server
From: Avi Kivity @ 2009-09-27 9:43 UTC (permalink / raw)
To: Gregory Haskins
Cc: Ira W. Snyder, Michael S. Tsirkin, netdev, virtualization, kvm,
linux-kernel, mingo, linux-mm, akpm, hpa, Rusty Russell, s.hetze,
alacrityvm-devel
In-Reply-To: <4ABD36E3.9070503@gmail.com>
On 09/26/2009 12:32 AM, Gregory Haskins wrote:
>>>
>>> I realize in retrospect that my choice of words above implies vbus _is_
>>> complete, but this is not what I was saying. What I was trying to
>>> convey is that vbus is _more_ complete. Yes, in either case some kind
>>> of glue needs to be written. The difference is that vbus implements
>>> more of the glue generally, and leaves less required to be customized
>>> for each iteration.
>>>
>>>
>>
>> No argument there. Since you care about non-virt scenarios and virtio
>> doesn't, naturally vbus is a better fit for them as the code stands.
>>
> Thanks for finally starting to acknowledge there's a benefit, at least.
>
I think I've mentioned vbus' finer grained layers as helpful here,
though I doubt the value of this. Hypervisors are added rarely, while
devices and drivers are added (and modified) much more often. I don't
buy the anything-to-anything promise.
> To be more precise, IMO virtio is designed to be a performance oriented
> ring-based driver interface that supports all types of hypervisors (e.g.
> shmem based kvm, and non-shmem based Xen). vbus is designed to be a
> high-performance generic shared-memory interconnect (for rings or
> otherwise) framework for environments where linux is the underpinning
> "host" (physical or virtual). They are distinctly different, but
> complementary (the former addresses the part of the front-end, and
> latter addresses the back-end, and a different part of the front-end).
>
They're not truly complementary since they're incompatible. A 2.6.27
guest, or Windows guest with the existing virtio drivers, won't work
over vbus. Further, non-shmem virtio can't work over vbus. Since
virtio is guest-oriented and host-agnostic, it can't ignore
non-shared-memory hosts (even though it's unlikely virtio will be
adopted there).
> In addition, the kvm-connector used in AlacrityVM's design strives to
> add value and improve performance via other mechanisms, such as dynamic
> allocation, interrupt coalescing (thus reducing exit-ratio, which is a
> serious issue in KVM)
Do you have measurements of inter-interrupt coalescing rates (excluding
intra-interrupt coalescing).
> and priortizable/nestable signals.
>
That doesn't belong in a bus.
> Today there is a large performance disparity between what a KVM guest
> sees and what a native linux application sees on that same host. Just
> take a look at some of my graphs between "virtio", and "native", for
> example:
>
> http://developer.novell.com/wiki/images/b/b7/31-rc4_throughput.png
>
That's a red herring. The problem is not with virtio as an ABI, but
with its implementation in userspace. vhost-net should offer equivalent
performance to vbus.
> A dominant vbus design principle is to try to achieve the same IO
> performance for all "linux applications" whether they be literally
> userspace applications, or things like KVM vcpus or Ira's physical
> boards. It also aims to solve problems not previously expressible with
> current technologies (even virtio), like nested real-time.
>
> And even though you repeatedly insist otherwise, the neat thing here is
> that the two technologies mesh (at least under certain circumstances,
> like when virtio is deployed on a shared-memory friendly linux backend
> like KVM). I hope that my stack diagram below depicts that clearly.
>
Right, when you ignore the points where they don't fit, it's a perfect mesh.
>> But that's not a strong argument for vbus; instead of adding vbus you
>> could make virtio more friendly to non-virt
>>
> Actually, it _is_ a strong argument then because adding vbus is what
> helps makes virtio friendly to non-virt, at least for when performance
> matters.
>
As vhost-net shows, you can do that without vbus and without breaking
compatibility.
>> Right. virtio assumes that it's in a virt scenario and that the guest
>> architecture already has enumeration and hotplug mechanisms which it
>> would prefer to use. That happens to be the case for kvm/x86.
>>
> No, virtio doesn't assume that. It's stack provides the "virtio-bus"
> abstraction and what it does assume is that it will be wired up to
> something underneath. Kvm/x86 conveniently has pci, so the virtio-pci
> adapter was created to reuse much of that facility. For other things
> like lguest and s360, something new had to be created underneath to make
> up for the lack of pci-like support.
>
Right, I was wrong there. But it does allow you to have a 1:1 mapping
between native devices and virtio devices.
>>> So to answer your question, the difference is that the part that has to
>>> be customized in vbus should be a fraction of what needs to be
>>> customized with vhost because it defines more of the stack.
>>>
>> But if you want to use the native mechanisms, vbus doesn't have any
>> added value.
>>
> First of all, thats incorrect. If you want to use the "native"
> mechanisms (via the way the vbus-connector is implemented, for instance)
> you at least still have the benefit that the backend design is more
> broadly re-useable in more environments (like non-virt, for instance),
> because vbus does a proper job of defining the requisite
> layers/abstractions compared to vhost. So it adds value even in that
> situation.
>
Maybe. If vhost-net isn't sufficient I'm sure there will be patches sent.
> Second of all, with PV there is no such thing as "native". It's
> software so it can be whatever we want. Sure, you could argue that the
> guest may have built-in support for something like PCI protocol.
> However, PCI protocol itself isn't suitable for high-performance PV out
> of the can. So you will therefore invariably require new software
> layers on top anyway, even if part of the support is already included.
>
Of course there is such a thing as native, a pci-ready guest has tons of
support built into it that doesn't need to be retrofitted. Since
practically everyone (including Xen) does their paravirt drivers atop
pci, the claim that pci isn't suitable for high performance is incorrect.
> And lastly, why would you _need_ to use the so called "native"
> mechanism? The short answer is, "you don't". Any given system (guest
> or bare-metal) already have a wide-range of buses (try running "tree
> /sys/bus" in Linux). More importantly, the concept of adding new buses
> is widely supported in both the Windows and Linux driver model (and
> probably any other guest-type that matters). Therefore, despite claims
> to the contrary, its not hard or even unusual to add a new bus to the mix.
>
The short answer is "compatibility".
> In summary, vbus is simply one more bus of many, purpose built to
> support high-end IO in a virt-like model, giving controlled access to
> the linux-host underneath it. You can write a high-performance layer
> below the OS bus-model (vbus), or above it (virtio-pci) but either way
> you are modifying the stack to add these capabilities, so we might as
> well try to get this right.
>
> With all due respect, you are making a big deal out of a minor issue.
>
It's not minor to me.
>>> And, as
>>> eluded to in my diagram, both virtio-net and vhost (with some
>>> modifications to fit into the vbus framework) are potentially
>>> complementary, not competitors.
>>>
>>>
>> Only theoretically. The existing installed base would have to be thrown
>> away
>>
> "Thrown away" is pure hyperbole. The installed base, worse case, needs
> to load a new driver for a missing device.
Yes, we all know how fun this is. Especially if the device changed is
your boot disk. You may not care about the pain caused to users, but I
do, so I will continue to insist on compatibility.
>> or we'd need to support both.
>>
>>
>>
> No matter what model we talk about, there's always going to be a "both"
> since the userspace virtio models are probably not going to go away (nor
> should they).
>
virtio allows you to have userspace-only, kernel-only, or
start-with-userspace-and-move-to-kernel-later, all transparent to the
guest. In many cases we'll stick with userspace-only.
>> All this is after kvm has decoded that vbus is addresses. It can't work
>> without someone outside vbus deciding that.
>>
> How the connector message is delivered is really not relevant. Some
> architectures will simply deliver the message point-to-point (like the
> original hypercall design for KVM, or something like Ira's rig), and
> some will need additional demuxing (like pci-bridge/pio based KVM).
> It's an implementation detail of the connector.
>
> However, the real point here is that something needs to establish a
> scoped namespace mechanism, add items to that namespace, and advertise
> the presence of the items to the guest. vbus has this facility built in
> to its stack. vhost doesn't, so it must come from elsewhere.
>
So we have: vbus needs a connector, vhost needs a connector. vbus
doesn't need userspace to program the addresses (but does need userspace
to instantiate the devices and to program the bus address decode), vhost
needs userspace to instantiate the devices and program the addresses.
>>> In fact, it's actually a simpler design to unify things this way because
>>> you avoid splitting the device model up. Consider how painful the vhost
>>> implementation would be if it didn't already have the userspace
>>> virtio-net to fall-back on. This is effectively what we face for new
>>> devices going forward if that model is to persist.
>>>
>>>
>>
>> It doesn't have just virtio-net, it has userspace-based hostplug
>>
> vbus has hotplug too: mkdir and rmdir
>
Does that work from nonprivileged processes? Does it work on Windows?
> As an added bonus, its device-model is modular. A developer can write a
> new device model, compile it, insmod it to the host kernel, hotplug it
> to the running guest with mkdir/ln, and the come back out again
> (hotunplug with rmdir, rmmod, etc). They may do this all without taking
> the guest down, and while eating QEMU based IO solutions for breakfast
> performance wise.
>
> Afaict, qemu can't do either of those things.
>
We've seen that herring before, and it's redder than ever.
>> Refactor instead of duplicating.
>>
> There is no duplicating. vbus has no equivalent today as virtio doesn't
> define these layers.
>
So define them if they're missing.
>>>
>>>
>>>> Use libraries (virtio-shmem.ko, libvhost.so).
>>>>
>>>>
>>> What do you suppose vbus is? vbus-proxy.ko = virtio-shmem.ko, and you
>>> dont need libvhost.so per se since you can just use standard kernel
>>> interfaces (like configfs/sysfs). I could create an .so going forward
>>> for the new ioctl-based interface, I suppose.
>>>
>>>
>> Refactor instead of rewriting.
>>
> There is no rewriting. vbus has no equivalent today as virtio doesn't
> define these layers.
>
> By your own admission, you said if you wanted that capability, use a
> library. What I think you are not understanding is vbus _is_ that
> library. So what is the problem, exactly?
>
It's not compatible. If you were truly worried about code duplication
in virtio, you'd refactor it to remove the duplication, without
affecting existing guests.
>>>> For kvm/x86 pci definitely remains king.
>>>>
>>>>
>>> For full virtualization, sure. I agree. However, we are talking about
>>> PV here. For PV, PCI is not a requirement and is a technical dead-end
>>> IMO.
>>>
>>> KVM seems to be the only virt solution that thinks otherwise (*), but I
>>> believe that is primarily a condition of its maturity. I aim to help
>>> advance things here.
>>>
>>> (*) citation: xen has xenbus, lguest has lguest-bus, vmware has some
>>> vmi-esq thing (I forget what its called) to name a few. Love 'em or
>>> hate 'em, most other hypervisors do something along these lines. I'd
>>> like to try to create one for KVM, but to unify them all (at least for
>>> the Linux-based host designs).
>>>
>>>
>> VMware are throwing VMI away (won't be supported in their new product,
>> and they've sent a patch to rip it off from Linux);
>>
> vmware only cares about x86 iiuc, so probably not a good example.
>
Well, you brought it up. Between you and me, I only care about x86 too.
>> Xen has to tunnel
>> xenbus in pci for full virtualization (which is where Windows is, and
>> where Linux will be too once people realize it's faster). lguest is
>> meant as an example hypervisor, not an attempt to take over the world.
>>
> So pick any other hypervisor, and the situation is often similar.
>
The situation is often pci.
>
>> An right now you can have a guest using pci to access a mix of
>> userspace-emulated devices, userspace-emulated-but-kernel-accelerated
>> virtio devices, and real host devices. All on one dead-end bus. Try
>> that with vbus.
>>
> vbus is not interested in userspace devices. The charter is to provide
> facilities for utilizing the host linux kernel's IO capabilities in the
> most efficient, yet safe, manner possible. Those devices that fit
> outside that charter can ride on legacy mechanisms if that suits them best.
>
vbus isn't, but I am. I would prefer not to have to expose
implementation decisions (kernel vs userspace) to the guest (vbus vs pci).
>>> That won't cut it. For one, creating an eventfd is only part of the
>>> equation. I.e. you need to have originate/terminate somewhere
>>> interesting (and in-kernel, otherwise use tuntap).
>>>
>>>
>> vbus needs the same thing so it cancels out.
>>
> No, it does not. vbus just needs a relatively simple single message
> pipe between the guest and host (think "hypercall tunnel", if you will).
>
That's ioeventfd. So far so similar.
> Per queue/device addressing is handled by the same conceptual namespace
> as the one that would trigger eventfds in the model you mention. And
> that namespace is built in to the vbus stack, and objects are registered
> automatically as they are created.
>
> Contrast that to vhost, which requires some other kernel interface to
> exist, and to be managed manually for each object that is created. Your
> libvhostconfig would need to somehow know how to perform this
> registration operation, and there would have to be something in the
> kernel to receive it, presumably on a per platform basis. Solving this
> problem generally would probably end up looking eerily like vbus,
> because thats what vbus does.
>
vbus devices aren't magically instantiated. Userspace needs to
instantiate them too. Sure, there's less work on the host side since
you're using vbus instead of the native interface, but more work on the
guest side since you're using vbus instead of the native interface.
>> Well, let's see. Can vbus today:
>>
>> - let userspace know which features are available (so it can decide if
>> live migration is possible)
>>
> yes, its in sysfs.
>
>
>> - let userspace limit which features are exposed to the guest (so it can
>> make live migration possible among hosts of different capabilities)
>>
> yes, its in sysfs.
>
Per-device? non-privileged-user capable?
>> - let userspace know which features were negotiated (so it can transfer
>> them to the other host during live migration)
>>
> no, but we can easily add ->save()/->restore() to the model going
> forward, and the negotiated features are just a subcomponent if its
> serialized stream.
>
>
>> - let userspace tell the kernel which features were negotiated (when
>> live migration completes, to avoid requiring the guest to re-negotiate)
>>
> that would be the function of the ->restore() deserializer.
>
>
>> - do all that from an unprivileged process
>>
> yes, in the upcoming alacrityvm v0.3 with the ioctl based control plane.
>
Ah, so you have two control planes.
> Bottom line: vbus isn't done, especially w.r.t. live-migration..but that
> is not an valid argument against the idea if you believe in
> release-early/release-often. kvm wasn't (isn't) done either when it was
> proposed/merged.
>
>
kvm didn't have an existing counterpart in Linux when it was
proposed/merged.
--
Do not meddle in the internals of kernels, for they are subtle and quick to panic.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [PATCH] /proc/net/tcp, overhead removed
From: Eric Dumazet @ 2009-09-27 9:53 UTC (permalink / raw)
To: Yakov Lerner
Cc: linux-kernel, netdev, davem, kuznet, pekkas, jmorris, yoshfuji,
kaber, torvalds
In-Reply-To: <1254000675-8327-1-git-send-email-iler.ml@gmail.com>
Yakov Lerner a écrit :
> /proc/net/tcp does 20,000 sockets in 60-80 milliseconds, with this patch.
>
> The overhead was in tcp_seq_start(). See analysis (3) below.
> The patch is against Linus git tree (1). The patch is small.
>
> ------------ ----------- ------------------------------------
> Before patch After patch 20,000 sockets (10,000 tw + 10,000 estab)(2)
> ------------ ----------- ------------------------------------
> 6 sec 0.06 sec dd bs=1k if=/proc/net/tcp >/dev/null
> 1.5 sec 0.06 sec dd bs=4k if=/proc/net/tcp >/dev/null
>
> 1.9 sec 0.16 sec netstat -4ant >/dev/null
> ------------ ----------- ------------------------------------
>
> This is ~ x25 improvement.
> The new time is not dependent on read blockize.
> Speed of netstat, naturally, improves, too; both -4 and -6.
> /proc/net/tcp6 does 20,000 sockets in 100 millisec.
>
> (1) against git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git
>
> (2) Used 'manysock' utility to stress system with large number of sockets:
> "manysock 10000 10000" - 10,000 tw + 10,000 estab ip4 sockets.
> "manysock -6 10000 10000" - 10,000 tw + 10,000 estab ip6 sockets.
> Found at http://ilerner.3b1.org/manysock/manysock.c
>
> (3) Algorithmic analysis.
> Old algorithm.
>
> During 'cat </proc/net/tcp', tcp_seq_start() is called O(numsockets) times (4).
> On average, every call to tcp_seq_start() scans half the whole hashtable. Ouch.
> This is O(numsockets * hashsize). 95-99% of 'cat </proc/net/tcp' is spent in
> tcp_seq_start()->tcp_get_idx. This overhead is eliminated by new algorithm,
> which is O(numsockets + hashsize).
>
> New algorithm.
>
> New algorithms is O(numsockets + hashsize). We jump to the right
> hash bucket in tcp_seq_start(), without scanning half the hash.
> To jump right to the hash bucket corresponding to *pos in tcp_seq_start(),
> we reuse three pieces of state (st->num, st->bucket, st->sbucket)
> as follows:
> - we check that requested pos >= last seen pos (st->num), the typical case.
> - if so, we jump to bucket st->bucket
> - to arrive to the right item after beginning of st->bucket, we
> keep in st->sbucket the position corresponding to the beginning of
> bucket.
>
> (4) Explanation of O( numsockets * hashsize) of old algorithm.
>
> tcp_seq_start() is called once for every ~7 lines of netstat output
> if readsize is 1kb, or once for every ~28 lines if readsize >= 4kb.
> Since record length of /proc/net/tcp records is 150 bytes, formula for
> number of calls to tcp_seq_start() is
> (numsockets * 150 / min(4096,readsize)).
> Netstat uses 4kb readsize (newer versions), or 1kb (older versions).
> Note that speed of old algorithm does not improve above 4kb blocksize.
>
> Speed of the new algorithm does not depend on blocksize.
>
> Speed of the new algorithm does not perceptibly depend on hashsize (which
> depends on ramsize). Speed of old algorithm drops with bigger hashsize.
>
> (5) Reporting order.
>
> Reporting order is exactly same as before if hash does not change underfoot.
> When hash elements come and go during report, reporting order will be
> same as that of tcpdiag.
>
> Signed-off-by: Yakov Lerner <iler.ml@gmail.com>
> ---
> net/ipv4/tcp_ipv4.c | 26 ++++++++++++++++++++++++--
> 1 files changed, 24 insertions(+), 2 deletions(-)
>
> diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
> index 7cda24b..7d9421a 100644
> --- a/net/ipv4/tcp_ipv4.c
> +++ b/net/ipv4/tcp_ipv4.c
> @@ -1994,13 +1994,14 @@ static inline int empty_bucket(struct tcp_iter_state *st)
> hlist_nulls_empty(&tcp_hashinfo.ehash[st->bucket].twchain);
> }
>
> -static void *established_get_first(struct seq_file *seq)
> +static void *established_get_first_after(struct seq_file *seq, int bucket)
> {
> struct tcp_iter_state *st = seq->private;
> struct net *net = seq_file_net(seq);
> void *rc = NULL;
>
> - for (st->bucket = 0; st->bucket < tcp_hashinfo.ehash_size; ++st->bucket) {
> + for (st->bucket = bucket; st->bucket < tcp_hashinfo.ehash_size;
> + ++st->bucket) {
> struct sock *sk;
> struct hlist_nulls_node *node;
> struct inet_timewait_sock *tw;
> @@ -2036,6 +2037,11 @@ out:
> return rc;
> }
>
> +static void *established_get_first(struct seq_file *seq)
> +{
> + return established_get_first_after(seq, 0);
> +}
> +
> static void *established_get_next(struct seq_file *seq, void *cur)
> {
> struct sock *sk = cur;
> @@ -2045,6 +2051,7 @@ static void *established_get_next(struct seq_file *seq, void *cur)
> struct net *net = seq_file_net(seq);
>
> ++st->num;
> + st->sbucket = st->num;
Hello Yakov
Intention of your patch is very good, but not currently working.
It seems you believe there is at most one entry per hash slot or something like that
Please reboot your test machine with "thash_entries=4096" so that tcp hash
size is 4096, and try to fill 20000 tcp sockets with a test program.
then :
# ss | wc -l
20001
(ok)
# cat /proc/net/tcp | wc -l
22160
(not quite correct ...)
# netstat -tn | wc -l
<never ends>
# dd if=/proc/net/tcp ibs=1024 | wc -l
<never ends>
Please send your next patch on netdev@vger.kernel.org , DaveM only , were netdev people
are reviewing netdev patches, there is no need include other people for first submissions.
Thank you
#include <sys/types.h>
#include <sys/socket.h>
#include <netinet/in.h>
#include <string.h>
int fdlisten;
main()
{
int i;
struct sockaddr_in sockaddr;
fdlisten = socket(AF_INET, SOCK_STREAM, 0);
memset(&sockaddr, 0, sizeof(sockaddr));
sockaddr.sin_family = AF_INET;
sockaddr.sin_port = htons(2222);
if (bind(fdlisten, (struct sockaddr *)&sockaddr, sizeof(sockaddr))== -1) {
perror("bind");
return 1;
}
if (listen(fdlisten, 10)== -1) {
perror("listen");
return 1;
}
if (fork() == 0) {
while (1) {
socklen_t len = sizeof(sockaddr);
int newfd = accept(fdlisten, (struct sockaddr *)&sockaddr, &len);
}
}
for (i = 0 ; i < 10000; i++) {
int fd = socket(AF_INET, SOCK_STREAM, 0);
if (fd == -1) {
perror("socket");
break;
}
connect(fd, (struct sockaddr *)&sockaddr, sizeof(sockaddr));
}
pause();
}
^ permalink raw reply
* Re: [PATCH 1/3] lib/vsprintf.c: Add %pU - ptr to a UUID/GUID
From: Ingo Oeser @ 2009-09-27 10:45 UTC (permalink / raw)
To: Joe Perches; +Cc: linux-kernel, netdev, Greg KH
In-Reply-To: <1507728e0ea3deafa71c481d508a6e9765c92221.1254030722.git.joe@perches.com>
Hi Joe,
On Sunday 27 September 2009, Joe Perches wrote:
> UUID/GUIDs are somewhat common in kernel source.
>
> Standardize the printed style of UUID/GUIDs by using
> another extension to %p.
>
> %pU: 01020304:0506:0708:090a:0b0c0d0e0f10
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ here
> %pUr: 04030201:0605:0807:0a09:0b0c0d0e0f10
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ and here
Code does "01020304-0506-0708-090a-0b0c0d0e0f10".
This is not, what commit promises. Please change the commit message!
Best Regards
Ingo Oeser
^ permalink raw reply
* [bisected] Wireless regression in 2.6.32-git
From: Arjan van de Ven @ 2009-09-27 13:18 UTC (permalink / raw)
To: netdev; +Cc: linux-kernel, linux-wireless
Hi,
With todays git my laptop fails to associate with my access point.
Bisection points to the commit below, and reverting this one commit on
the HEAD of tree also fixes the issue, so I'm pretty confident that this
commit is to blame.
I have a 4965 wifi card in my laptop, and the network I'm trying to
connect to has no encryption. I'm running Fedora 11 as OS.
I would like to kindly request for this commit to be reverted until a
more permanent solution is found (I'm happy to test any patches)..
94f85853324e02c3a32bc3101f090dc9a3f512b4 is first bad commit
commit 94f85853324e02c3a32bc3101f090dc9a3f512b4
Author: Johannes Berg <johannes@sipsolutions.net>
Date: Thu Sep 17 17:15:31 2009 -0700
cfg80211: don't overwrite privacy setting
When cfg80211 is instructed to connect, it always
uses the default WEP key for the privacy setting,
which clearly is wrong when using wpa_supplicant.
Don't overwrite the setting, and rely on it being
false when wpa_supplicant is not running, instead
set it to true when we have keys.
Signed-off-by: Johannes Berg <johannes@sipsolutions.net>
Signed-off-by: John W. Linville <linville@tuxdriver.com>
:040000 040000 27fb46273e88eefee373699eb7e3f2923ac0886b
9518ee3e52c8320613cc5eee5ac54aabf082432f M net
--
Arjan van de Ven Intel Open Source Technology Centre
For development, discussion and tips for power savings,
visit http://www.lesswatts.org
^ permalink raw reply
* Re: [bisected] Wireless regression in 2.6.32-git
From: Maciej Rutecki @ 2009-09-27 13:24 UTC (permalink / raw)
To: Arjan van de Ven; +Cc: netdev, linux-kernel, linux-wireless
In-Reply-To: <20090927151855.29efcc53@infradead.org>
Did You have similar messages in dmesg like this:
http://bugzilla.intellinuxwireless.org/show_bug.cgi?id=2089
(use WPA)
?
I try make sure, that that result of my bisection is correct.
--
Maciej Rutecki
http://www.maciek.unixy.pl
^ permalink raw reply
* Re: [bisected] Wireless regression in 2.6.32-git
From: Arjan van de Ven @ 2009-09-27 13:32 UTC (permalink / raw)
To: Maciej Rutecki
Cc: netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-wireless-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <8db1092f0909270624l4fc46734u6a91f7cc7ffb0c72-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On Sun, 27 Sep 2009 15:24:10 +0200
Maciej Rutecki <maciej.rutecki-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> Did You have similar messages in dmesg like this:
>
> http://bugzilla.intellinuxwireless.org/show_bug.cgi?id=2089
> (use WPA)
nope I don't get a Reason 6 disconnect...
(I get a reason 3 local choice, but that's true for working and
non-working both)
--
Arjan van de Ven Intel Open Source Technology Centre
For development, discussion and tips for power savings,
visit http://www.lesswatts.org
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [2.6.31-git17] WARNING: at kernel/hrtimer.c:648 hres_timers_resume+0x40/0x50()/WARNING: at drivers/base/sys.c:353 __sysdev_resume+0xc3/0xe0()
From: Yong Zhang @ 2009-09-27 14:49 UTC (permalink / raw)
To: Maciej Rutecki
Cc: Linux Kernel Mailing List, Rafael J. Wysocki, clemens,
venkatesh.pallipadi, gregkh, zambrano, davem, netdev
In-Reply-To: <2674af740909270701u6febabdbpc5c772fabc193b32@mail.gmail.com>
Add cc'ed.
On Sun, Sep 27, 2009 at 10:01 PM, Yong Zhang <yong.zhang0@gmail.com> wrote:
> On Sun, Sep 27, 2009 at 6:25 PM, Maciej Rutecki
> <maciej.rutecki@gmail.com> wrote:
>> 2009/9/27 Yong Zhang <yong.zhang0@gmail.com>:
>>>
>>> Oops, TABLE is corrupted. Please use the attachment.
>>
>> kernel/time/timekeeping.c: In function ‘timekeeping_resume’:
>> kernel/time/timekeeping.c:577: error: ‘xtime_locks’ undeclared (first
>> use in this function)
>> kernel/time/timekeeping.c:577: error: (Each undeclared identifier is
>> reported only once
>> kernel/time/timekeeping.c:577: error: for each function it appears
>> in.)
>> make[3]: *** [kernel/time/timekeeping.o] Błąd 1
>> make[2]: *** [kernel/time] Błąd 2
>>
>>
>> I change:
>> write_seqlock(&xtime_locks);
>> to:
>> write_seqlock(&xtime_lock);
>>
>
> Oh, typo error.
>
>> Corrected patch in attachment.
>>
>> First warning has gone, but still I have this:
>> [ 120.868049] SMP alternatives: switching to UP code
>> [ 120.872570] CPU0 attaching NULL sched-domain.
>> [ 120.872574] CPU1 attaching NULL sched-domain.
>> [ 120.872581] CPU0 attaching NULL sched-domain.
>> [ 120.872787] CPU1 is down
>> [ 120.872846] Extended CMOS year: 2000
>> [ 120.872945] PM: Creating hibernation image:
>> [ 120.876009] PM: Need to copy 107120 pages
>> [ 120.872009] PM: Restoring platform NVS memory
>> [ 120.872009] CPU0: Thermal monitoring handled by SMI
>> [ 120.872009] Extended CMOS year: 2000
>> [ 120.872009] ------------[ cut here ]------------
>> [ 120.872009] WARNING: at drivers/base/sys.c:353
>> __sysdev_resume+0xc3/0xe0()
>> [ 120.872009] Hardware name: HP Compaq nx6310 (EY501ES#AKD)
>> [ 120.872009] Interrupts enabled after timekeeping_resume+0x0/0x1c0
>
> Irq is enabled after timekeeping_resume(), the previous patch do something
> sensible. But we even don't know when the irq is enabled. such as in
> timekeeping_resume() or before enter timekeeping_resume(). But it
> seem that this is not due to timekeeping_resume(). Instead I'm afraid
> it's caused by buggy driver.
>
It seem this is cause by b44 dirver. Can you give a try?
From 05ee2f22a7ea065e05bf8b5294d222a3700d2cc8 Mon Sep 17 00:00:00 2001
From: Yong Zhang <yong.zhang0@gmail.com>
Date: Sun, 27 Sep 2009 22:42:41 +0800
Subject: [PATCH] net/b44: keep irq state at suspend_resume
suspend() and resume() assume irq is disabled. So keep the irq
state when do this.
Signed-off-by: Yong Zhang <yong.zhang0@gmail.com>
---
drivers/net/b44.c | 10 ++++++----
1 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/net/b44.c b/drivers/net/b44.c
index e046943..97b467f 100644
--- a/drivers/net/b44.c
+++ b/drivers/net/b44.c
@@ -2258,20 +2258,21 @@ static int b44_suspend(struct ssb_device
*sdev, pm_message_t state)
{
struct net_device *dev = ssb_get_drvdata(sdev);
struct b44 *bp = netdev_priv(dev);
+ unsigned long flags;
if (!netif_running(dev))
return 0;
del_timer_sync(&bp->timer);
- spin_lock_irq(&bp->lock);
+ spin_lock_irqsave(&bp->lock, flags);
b44_halt(bp);
netif_carrier_off(bp->dev);
netif_device_detach(bp->dev);
b44_free_rings(bp);
- spin_unlock_irq(&bp->lock);
+ spin_unlock_irqrestore(&bp->lock, flags);
free_irq(dev->irq, dev);
if (bp->flags & B44_FLAG_WOL_ENABLE) {
@@ -2288,6 +2289,7 @@ static int b44_resume(struct ssb_device *sdev)
struct net_device *dev = ssb_get_drvdata(sdev);
struct b44 *bp = netdev_priv(dev);
int rc = 0;
+ unsigned long flags;
rc = ssb_bus_powerup(sdev->bus, 0);
if (rc) {
@@ -2305,12 +2307,12 @@ static int b44_resume(struct ssb_device *sdev)
return rc;
}
- spin_lock_irq(&bp->lock);
+ spin_lock_irqsave(&bp->lock, flags);
b44_init_rings(bp);
b44_init_hw(bp, B44_FULL_RESET);
netif_device_attach(bp->dev);
- spin_unlock_irq(&bp->lock);
+ spin_unlock_irqrestore(&bp->lock, flags);
b44_enable_ints(bp);
netif_wake_queue(dev);
--
1.6.0.4
> So can you test the follow patch. And show the dmesg info(DEBUG
> level) after WARNING is triggered?
>
> diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
> index fb0f46f..4a00a1a 100644
> --- a/kernel/time/timekeeping.c
> +++ b/kernel/time/timekeeping.c
> @@ -569,6 +569,9 @@ static int timekeeping_resume(struct sys_device *dev)
> unsigned long flags;
> struct timespec ts;
>
> + WARN_ONCE(!irqs_disabled(),
> + KERN_INFO "timekeeping_resume() called with IRQs enabled!");
> +
> read_persistent_clock(&ts);
>
> clocksource_resume();
>
> Thanks,
> Yong
>
>> [ 120.872009] Modules linked in: i915 drm_kms_helper drm i2c_algo_bit
>> i2c_core sco bnep rfcomm l2cap crc16 xt_tcpudp xt_limit xt_state
>> iptable_filter nf_conntrack_ipv4 nf_conntrack nf_defrag_ipv4 ip_tables
>> x_tables aes_i586 aes_generic cbc dm_crypt dm_snapshot dm_mirror
>> dm_region_hash dm_log dm_mod hp_wmi fuse sbp2 loop
>> snd_hda_codec_si3054 snd_hda_codec_analog snd_hda_intel snd_hda_codec
>> snd_pcm_oss snd_mixer_oss snd_pcm snd_seq_dummy arc4 snd_seq_oss ecb
>> snd_seq_midi snd_rawmidi iwl3945 snd_seq_midi_event iwlcore btusb
>> firmware_class snd_seq bluetooth snd_timer mac80211 pcmcia
>> snd_seq_device led_class b44 video ohci1394 backlight ssb rtc_cmos snd
>> yenta_socket cfg80211 intel_agp soundcore rsrc_nonstatic uhci_hcd
>> ehci_hcd rtc_core usbcore psmouse snd_page_alloc agpgart pcmcia_core
>> rfkill rtc_lib ieee1394 sg output evdev serio_raw mii fan button ac
>> battery
>> [ 120.872009] Pid: 3510, comm: pm-hibernate Not tainted 2.6.31-git17
>> #1
>> [ 120.872009] Call Trace:
>> [ 120.872009] [<c030f4a3>] ? __sysdev_resume+0xc3/0xe0
>> [ 120.872009] [<c030f4a3>] ? __sysdev_resume+0xc3/0xe0
>> [ 120.872009] [<c013c3a1>] warn_slowpath_common+0x71/0xc0
>> [ 120.872009] [<c030f4a3>] ? __sysdev_resume+0xc3/0xe0
>> [ 120.872009] [<c013c43b>] warn_slowpath_fmt+0x2b/0x30
>> [ 120.872009] [<c030f4a3>] __sysdev_resume+0xc3/0xe0
>> [ 120.872009] [<c015f810>] ? timekeeping_resume+0x0/0x1c0
>> [ 120.872009] [<c030f50f>] sysdev_resume+0x4f/0xc0
>> [ 120.872009] [<c0176909>] ? hibernate_nvs_restore+0x19/0x60
>> [ 120.872009] [<c0172ac1>] hibernation_snapshot+0x1d1/0x210
>> [ 120.872009] [<c0171b54>] ? freeze_processes+0x44/0xa0
>> [ 120.872009] [<c0172bef>] hibernate+0xef/0x190
>> [ 120.872009] [<c0171590>] ? state_store+0x0/0xc0
>> [ 120.872009] [<c017163b>] state_store+0xab/0xc0
>> [ 120.872009] [<c0171590>] ? state_store+0x0/0xc0
>> [ 120.872009] [<c0285e04>] kobj_attr_store+0x24/0x30
>> [ 120.872009] [<c0223d82>] sysfs_write_file+0xa2/0x100
>> [ 120.872009] [<c01d880c>] vfs_write+0x9c/0x150
>> [ 120.872009] [<c0223ce0>] ? sysfs_write_file+0x0/0x100
>> [ 120.872009] [<c01d8982>] sys_write+0x42/0x70
>> [ 120.872009] [<c0102f04>] sysenter_do_call+0x12/0x22
>> [ 120.872009] ---[ end trace 51d3cc987b340170 ]---
>> [ 120.872009] Enabling non-boot CPUs ...
>> [ 120.872009] SMP alternatives: switching to SMP code
>>
>> Regards
>> --
>> Maciej Rutecki
>> http://www.maciek.unixy.pl
>>
>
^ permalink raw reply related
* Re: [2.6.31-git17] WARNING: at kernel/hrtimer.c:648 hres_timers_resume+0x40/0x50()/WARNING: at drivers/base/sys.c:353 __sysdev_resume+0xc3/0xe0()
From: Rafael J. Wysocki @ 2009-09-27 15:40 UTC (permalink / raw)
To: Yong Zhang
Cc: Maciej Rutecki, Linux Kernel Mailing List, clemens,
venkatesh.pallipadi, gregkh, zambrano, davem, netdev
In-Reply-To: <2674af740909270749u339671a1ia81d5736eeb750fa@mail.gmail.com>
On Sunday 27 September 2009, Yong Zhang wrote:
> Add cc'ed.
>
> On Sun, Sep 27, 2009 at 10:01 PM, Yong Zhang <yong.zhang0@gmail.com> wrote:
> > On Sun, Sep 27, 2009 at 6:25 PM, Maciej Rutecki
> > <maciej.rutecki@gmail.com> wrote:
> >> 2009/9/27 Yong Zhang <yong.zhang0@gmail.com>:
> >>>
> >>> Oops, TABLE is corrupted. Please use the attachment.
> >>
> >> kernel/time/timekeeping.c: In function ‘timekeeping_resume’:
> >> kernel/time/timekeeping.c:577: error: ‘xtime_locks’ undeclared (first
> >> use in this function)
> >> kernel/time/timekeeping.c:577: error: (Each undeclared identifier is
> >> reported only once
> >> kernel/time/timekeeping.c:577: error: for each function it appears
> >> in.)
> >> make[3]: *** [kernel/time/timekeeping.o] Błąd 1
> >> make[2]: *** [kernel/time] Błąd 2
> >>
> >>
> >> I change:
> >> write_seqlock(&xtime_locks);
> >> to:
> >> write_seqlock(&xtime_lock);
> >>
> >
> > Oh, typo error.
> >
> >> Corrected patch in attachment.
> >>
> >> First warning has gone, but still I have this:
> >> [ 120.868049] SMP alternatives: switching to UP code
> >> [ 120.872570] CPU0 attaching NULL sched-domain.
> >> [ 120.872574] CPU1 attaching NULL sched-domain.
> >> [ 120.872581] CPU0 attaching NULL sched-domain.
> >> [ 120.872787] CPU1 is down
> >> [ 120.872846] Extended CMOS year: 2000
> >> [ 120.872945] PM: Creating hibernation image:
> >> [ 120.876009] PM: Need to copy 107120 pages
> >> [ 120.872009] PM: Restoring platform NVS memory
> >> [ 120.872009] CPU0: Thermal monitoring handled by SMI
> >> [ 120.872009] Extended CMOS year: 2000
> >> [ 120.872009] ------------[ cut here ]------------
> >> [ 120.872009] WARNING: at drivers/base/sys.c:353
> >> __sysdev_resume+0xc3/0xe0()
> >> [ 120.872009] Hardware name: HP Compaq nx6310 (EY501ES#AKD)
> >> [ 120.872009] Interrupts enabled after timekeeping_resume+0x0/0x1c0
> >
> > Irq is enabled after timekeeping_resume(), the previous patch do something
> > sensible. But we even don't know when the irq is enabled. such as in
> > timekeeping_resume() or before enter timekeeping_resume(). But it
> > seem that this is not due to timekeeping_resume(). Instead I'm afraid
> > it's caused by buggy driver.
> >
>
> It seem this is cause by b44 dirver. Can you give a try?
>
> From 05ee2f22a7ea065e05bf8b5294d222a3700d2cc8 Mon Sep 17 00:00:00 2001
> From: Yong Zhang <yong.zhang0@gmail.com>
> Date: Sun, 27 Sep 2009 22:42:41 +0800
> Subject: [PATCH] net/b44: keep irq state at suspend_resume
>
> suspend() and resume() assume irq is disabled.
No, it doesn't.
Thanks,
Rafael
> So keep the irq state when do this.
>
> Signed-off-by: Yong Zhang <yong.zhang0@gmail.com>
> ---
> drivers/net/b44.c | 10 ++++++----
> 1 files changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/b44.c b/drivers/net/b44.c
> index e046943..97b467f 100644
> --- a/drivers/net/b44.c
> +++ b/drivers/net/b44.c
> @@ -2258,20 +2258,21 @@ static int b44_suspend(struct ssb_device
> *sdev, pm_message_t state)
> {
> struct net_device *dev = ssb_get_drvdata(sdev);
> struct b44 *bp = netdev_priv(dev);
> + unsigned long flags;
>
> if (!netif_running(dev))
> return 0;
>
> del_timer_sync(&bp->timer);
>
> - spin_lock_irq(&bp->lock);
> + spin_lock_irqsave(&bp->lock, flags);
>
> b44_halt(bp);
> netif_carrier_off(bp->dev);
> netif_device_detach(bp->dev);
> b44_free_rings(bp);
>
> - spin_unlock_irq(&bp->lock);
> + spin_unlock_irqrestore(&bp->lock, flags);
>
> free_irq(dev->irq, dev);
> if (bp->flags & B44_FLAG_WOL_ENABLE) {
> @@ -2288,6 +2289,7 @@ static int b44_resume(struct ssb_device *sdev)
> struct net_device *dev = ssb_get_drvdata(sdev);
> struct b44 *bp = netdev_priv(dev);
> int rc = 0;
> + unsigned long flags;
>
> rc = ssb_bus_powerup(sdev->bus, 0);
> if (rc) {
> @@ -2305,12 +2307,12 @@ static int b44_resume(struct ssb_device *sdev)
> return rc;
> }
>
> - spin_lock_irq(&bp->lock);
> + spin_lock_irqsave(&bp->lock, flags);
>
> b44_init_rings(bp);
> b44_init_hw(bp, B44_FULL_RESET);
> netif_device_attach(bp->dev);
> - spin_unlock_irq(&bp->lock);
> + spin_unlock_irqrestore(&bp->lock, flags);
>
> b44_enable_ints(bp);
> netif_wake_queue(dev);
> > So can you test the follow patch. And show the dmesg info(DEBUG
> > level) after WARNING is triggered?
> >
> > diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
> > index fb0f46f..4a00a1a 100644
> > --- a/kernel/time/timekeeping.c
> > +++ b/kernel/time/timekeeping.c
> > @@ -569,6 +569,9 @@ static int timekeeping_resume(struct sys_device *dev)
> > unsigned long flags;
> > struct timespec ts;
> >
> > + WARN_ONCE(!irqs_disabled(),
> > + KERN_INFO "timekeeping_resume() called with IRQs enabled!");
> > +
> > read_persistent_clock(&ts);
> >
> > clocksource_resume();
> >
> > Thanks,
> > Yong
> >
> >> [ 120.872009] Modules linked in: i915 drm_kms_helper drm i2c_algo_bit
> >> i2c_core sco bnep rfcomm l2cap crc16 xt_tcpudp xt_limit xt_state
> >> iptable_filter nf_conntrack_ipv4 nf_conntrack nf_defrag_ipv4 ip_tables
> >> x_tables aes_i586 aes_generic cbc dm_crypt dm_snapshot dm_mirror
> >> dm_region_hash dm_log dm_mod hp_wmi fuse sbp2 loop
> >> snd_hda_codec_si3054 snd_hda_codec_analog snd_hda_intel snd_hda_codec
> >> snd_pcm_oss snd_mixer_oss snd_pcm snd_seq_dummy arc4 snd_seq_oss ecb
> >> snd_seq_midi snd_rawmidi iwl3945 snd_seq_midi_event iwlcore btusb
> >> firmware_class snd_seq bluetooth snd_timer mac80211 pcmcia
> >> snd_seq_device led_class b44 video ohci1394 backlight ssb rtc_cmos snd
> >> yenta_socket cfg80211 intel_agp soundcore rsrc_nonstatic uhci_hcd
> >> ehci_hcd rtc_core usbcore psmouse snd_page_alloc agpgart pcmcia_core
> >> rfkill rtc_lib ieee1394 sg output evdev serio_raw mii fan button ac
> >> battery
> >> [ 120.872009] Pid: 3510, comm: pm-hibernate Not tainted 2.6.31-git17
> >> #1
> >> [ 120.872009] Call Trace:
> >> [ 120.872009] [<c030f4a3>] ? __sysdev_resume+0xc3/0xe0
> >> [ 120.872009] [<c030f4a3>] ? __sysdev_resume+0xc3/0xe0
> >> [ 120.872009] [<c013c3a1>] warn_slowpath_common+0x71/0xc0
> >> [ 120.872009] [<c030f4a3>] ? __sysdev_resume+0xc3/0xe0
> >> [ 120.872009] [<c013c43b>] warn_slowpath_fmt+0x2b/0x30
> >> [ 120.872009] [<c030f4a3>] __sysdev_resume+0xc3/0xe0
> >> [ 120.872009] [<c015f810>] ? timekeeping_resume+0x0/0x1c0
> >> [ 120.872009] [<c030f50f>] sysdev_resume+0x4f/0xc0
> >> [ 120.872009] [<c0176909>] ? hibernate_nvs_restore+0x19/0x60
> >> [ 120.872009] [<c0172ac1>] hibernation_snapshot+0x1d1/0x210
> >> [ 120.872009] [<c0171b54>] ? freeze_processes+0x44/0xa0
> >> [ 120.872009] [<c0172bef>] hibernate+0xef/0x190
> >> [ 120.872009] [<c0171590>] ? state_store+0x0/0xc0
> >> [ 120.872009] [<c017163b>] state_store+0xab/0xc0
> >> [ 120.872009] [<c0171590>] ? state_store+0x0/0xc0
> >> [ 120.872009] [<c0285e04>] kobj_attr_store+0x24/0x30
> >> [ 120.872009] [<c0223d82>] sysfs_write_file+0xa2/0x100
> >> [ 120.872009] [<c01d880c>] vfs_write+0x9c/0x150
> >> [ 120.872009] [<c0223ce0>] ? sysfs_write_file+0x0/0x100
> >> [ 120.872009] [<c01d8982>] sys_write+0x42/0x70
> >> [ 120.872009] [<c0102f04>] sysenter_do_call+0x12/0x22
> >> [ 120.872009] ---[ end trace 51d3cc987b340170 ]---
> >> [ 120.872009] Enabling non-boot CPUs ...
> >> [ 120.872009] SMP alternatives: switching to SMP code
^ permalink raw reply
* Re: [bisected] Wireless regression in 2.6.32-git
From: Hugh Dickins @ 2009-09-27 16:14 UTC (permalink / raw)
To: Arjan van de Ven
Cc: Johannes Berg, netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-wireless-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20090927151855.29efcc53-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org>
On Sun, 27 Sep 2009, Arjan van de Ven wrote:
>
> With todays git my laptop fails to associate with my access point.
> Bisection points to the commit below, and reverting this one commit on
> the HEAD of tree also fixes the issue, so I'm pretty confident that this
> commit is to blame.
>
> I have a 4965 wifi card in my laptop, and the network I'm trying to
> connect to has no encryption. I'm running Fedora 11 as OS.
>
> I would like to kindly request for this commit to be reverted until a
> more permanent solution is found (I'm happy to test any patches)..
>
> 94f85853324e02c3a32bc3101f090dc9a3f512b4 is first bad commit
> commit 94f85853324e02c3a32bc3101f090dc9a3f512b4
> Author: Johannes Berg <johannes-cdvu00un1VgdHxzADdlk8Q@public.gmane.org>
> Date: Thu Sep 17 17:15:31 2009 -0700
>
> cfg80211: don't overwrite privacy setting
>
> When cfg80211 is instructed to connect, it always
> uses the default WEP key for the privacy setting,
> which clearly is wrong when using wpa_supplicant.
> Don't overwrite the setting, and rely on it being
> false when wpa_supplicant is not running, instead
> set it to true when we have keys.
>
> Signed-off-by: Johannes Berg <johannes-cdvu00un1VgdHxzADdlk8Q@public.gmane.org>
> Signed-off-by: John W. Linville <linville-2XuSBdqkA4R54TAoqtyWWQ@public.gmane.org>
>
> :040000 040000 27fb46273e88eefee373699eb7e3f2923ac0886b
> 9518ee3e52c8320613cc5eee5ac54aabf082432f M net
I've a different problem with wireless that Johannes is investigating
for me on linux-wireless; but here's a patch that he pointed me to
along the way, didn't help my issue but I expect it will help yours...
Subject: cfg80211: don't set privacy w/o key
When wpa_supplicant is used to connect to open networks,
it causes the wdev->wext.keys to point to key memory, but
that key memory is all empty. Only use privacy when there
is a default key to be used.
Signed-off-by: Johannes Berg <johannes-cdvu00un1VgdHxzADdlk8Q@public.gmane.org>
---
net/wireless/wext-sme.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
--- wireless-testing.orig/net/wireless/wext-sme.c 2009-09-24 08:51:14.000000000 +0200
+++ wireless-testing/net/wireless/wext-sme.c 2009-09-24 08:57:01.000000000 +0200
@@ -30,7 +30,8 @@ int cfg80211_mgd_wext_connect(struct cfg
if (wdev->wext.keys) {
wdev->wext.keys->def = wdev->wext.default_key;
wdev->wext.keys->defmgmt = wdev->wext.default_mgmt_key;
- wdev->wext.connect.privacy = true;
+ if (wdev->wext.default_key != -1)
+ wdev->wext.connect.privacy = true;
}
if (!wdev->wext.connect.ssid_len)
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [2.6.31-git17] WARNING: at kernel/hrtimer.c:648 hres_timers_resume+0x40/0x50()/WARNING: at drivers/base/sys.c:353 __sysdev_resume+0xc3/0xe0()
From: Maciej Rutecki @ 2009-09-27 16:16 UTC (permalink / raw)
To: Yong Zhang
Cc: Linux Kernel Mailing List, Rafael J. Wysocki, clemens,
venkatesh.pallipadi, gregkh, zambrano, davem, netdev
In-Reply-To: <2674af740909270749u339671a1ia81d5736eeb750fa@mail.gmail.com>
2009/9/27 Yong Zhang <yong.zhang0@gmail.com>:
>
> It seem this is cause by b44 dirver. Can you give a try?
>
> From 05ee2f22a7ea065e05bf8b5294d222a3700d2cc8 Mon Sep 17 00:00:00 2001
> From: Yong Zhang <yong.zhang0@gmail.com>
> Date: Sun, 27 Sep 2009 22:42:41 +0800
> Subject: [PATCH] net/b44: keep irq state at suspend_resume
>
> suspend() and resume() assume irq is disabled. So keep the irq
> state when do this.
Remove previous patch, add it to clean 2.6.31-git17. It doesn't help.
> --
> 1.6.0.4
>
>> So can you test the follow patch. And show the dmesg info(DEBUG
>> level) after WARNING is triggered?
>>
>> diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
>> index fb0f46f..4a00a1a 100644
Still test it?
Regards
--
Maciej Rutecki
http://www.maciek.unixy.pl
^ permalink raw reply
* Re: [PATCH] ax25: Fix ax25_cb refcounting in ax25_ctl_ioctl
From: Jarek Poplawski @ 2009-09-27 17:10 UTC (permalink / raw)
To: Ralf Baechle DL5RB
Cc: David Miller, Bernard Pidoux F6BVP, Bernard Pidoux,
Linux Netdev List, linux-hams
In-Reply-To: <20090927072319.GA28089@linux-mips.org>
On Sun, Sep 27, 2009 at 08:23:19AM +0100, Ralf Baechle DL5RB wrote:
> On Fri, Sep 25, 2009 at 08:35:04PM +0200, Jarek Poplawski wrote:
>
> > > > This bug isn't responsible for these oopses here, but looks quite
> > > > obviously. (I'm not sure if it's easy to test/hit with the common
> > > > tools.)
> > >
> > > The issue your patch fixes is obvious enough.
> >
> > Yes, with new code there would be no doubt. But here, if you know it's
> > worked for some time, you wonder if you're not blind. |-)
>
> Most of of the ioctls are used by AX.25 userland which does error
> checking on user supplied values so userland will never attempt invalid
> ioctl calls. So no surprise this went unnoticed.
In this case valid calls (return 0) were affected too, so I guess the
whole thing is rarely used.
Jarek P.
^ permalink raw reply
* Re: [2.6.31-git17] WARNING: at kernel/hrtimer.c:648 hres_timers_resume+0x40/0x50()/WARNING: at drivers/base/sys.c:353 __sysdev_resume+0xc3/0xe0()
From: Rafael J. Wysocki @ 2009-09-27 18:17 UTC (permalink / raw)
To: Maciej Rutecki
Cc: Yong Zhang, Linux Kernel Mailing List, clemens,
venkatesh.pallipadi, gregkh, zambrano, davem, netdev
In-Reply-To: <8db1092f0909270916k79d0c369ub93b5e8ddf66c979@mail.gmail.com>
On Sunday 27 September 2009, Maciej Rutecki wrote:
> 2009/9/27 Yong Zhang <yong.zhang0@gmail.com>:
> >
> > It seem this is cause by b44 dirver. Can you give a try?
> >
> > From 05ee2f22a7ea065e05bf8b5294d222a3700d2cc8 Mon Sep 17 00:00:00 2001
> > From: Yong Zhang <yong.zhang0@gmail.com>
> > Date: Sun, 27 Sep 2009 22:42:41 +0800
> > Subject: [PATCH] net/b44: keep irq state at suspend_resume
> >
> > suspend() and resume() assume irq is disabled. So keep the irq
> > state when do this.
>
> Remove previous patch, add it to clean 2.6.31-git17. It doesn't help.
It couldn't. The problem is elswhere, but I haven't found it yet.
Thanks,
Rafael
^ permalink raw reply
* [PATCH 1/2] net: introduce NETDEV_PRE_INIT notifier
From: Johannes Berg @ 2009-09-27 18:26 UTC (permalink / raw)
To: netdev; +Cc: linux-wireless
For various purposes including a wireless extensions
bugfix, we need to hook into the netdev creation at
a point before netdev_register_kobject(). It seems
more generic, however, to have it even earlier. This
will also ease doing the dev type assignment that
Marcel was working on generically.
Signed-off-by: Johannes Berg <johannes-cdvu00un1VgdHxzADdlk8Q@public.gmane.org>
---
include/linux/notifier.h | 1 +
net/core/dev.c | 5 +++++
2 files changed, 6 insertions(+)
--- wireless-testing.orig/include/linux/notifier.h 2009-09-27 15:09:10.000000000 +0200
+++ wireless-testing/include/linux/notifier.h 2009-09-27 15:10:19.000000000 +0200
@@ -199,6 +199,7 @@ static inline int notifier_to_errno(int
#define NETDEV_FEAT_CHANGE 0x000B
#define NETDEV_BONDING_FAILOVER 0x000C
#define NETDEV_PRE_UP 0x000D
+#define NETDEV_PRE_INIT 0x000E
#define SYS_DOWN 0x0001 /* Notify of system down */
#define SYS_RESTART SYS_DOWN
--- wireless-testing.orig/net/core/dev.c 2009-09-27 15:09:56.000000000 +0200
+++ wireless-testing/net/core/dev.c 2009-09-27 15:11:40.000000000 +0200
@@ -4734,6 +4734,11 @@ int register_netdevice(struct net_device
dev->iflink = -1;
+ ret = call_netdevice_notifiers(NETDEV_PRE_INIT, dev);
+ ret = notifier_to_errno(ret);
+ if (ret)
+ goto out;
+
/* Init, if this function is available */
if (dev->netdev_ops->ndo_init) {
ret = dev->netdev_ops->ndo_init(dev);
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH 2/2] cfg80211: fix wireless handlers assignment
From: Johannes Berg @ 2009-09-27 18:27 UTC (permalink / raw)
To: netdev; +Cc: linux-wireless, Hugh Dickins
The point we assign dev->wireless_handlers at is too
late, we need to do that before netdev_register_kobject()
gets called, so use the new NETDEV_PRE_INIT notifier.
The result of adding wireless_handlers too late is the
disappearance of /sys/class/net/wlan0/wireless which a
bunch of distro scripts still require.
Signed-off-by: Johannes Berg <johannes@sipsolutions.net>
---
This should fix the regression Hugh reported (of course requires the
other patch which unfortunately I forgot to CC you, Hugh, I'll send you
a copy in private).
net/wireless/core.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
--- wireless-testing.orig/net/wireless/core.c 2009-09-27 15:12:20.000000000 +0200
+++ wireless-testing/net/wireless/core.c 2009-09-27 15:12:54.000000000 +0200
@@ -641,6 +641,12 @@ static int cfg80211_netdev_notifier_call
WARN_ON(wdev->iftype == NL80211_IFTYPE_UNSPECIFIED);
switch (state) {
+ case NETDEV_PRE_INIT:
+#ifdef CONFIG_WIRELESS_EXT
+ if (!dev->wireless_handlers)
+ dev->wireless_handlers = &cfg80211_wext_handler;
+#endif
+ break;
case NETDEV_REGISTER:
/*
* NB: cannot take rdev->mtx here because this may be
@@ -666,8 +672,6 @@ static int cfg80211_netdev_notifier_call
wdev->sme_state = CFG80211_SME_IDLE;
mutex_unlock(&rdev->devlist_mtx);
#ifdef CONFIG_WIRELESS_EXT
- if (!dev->wireless_handlers)
- dev->wireless_handlers = &cfg80211_wext_handler;
wdev->wext.default_key = -1;
wdev->wext.default_mgmt_key = -1;
wdev->wext.connect.auth_type = NL80211_AUTHTYPE_AUTOMATIC;
^ permalink raw reply
* Re: [bisected] Wireless regression in 2.6.32-git
From: Arjan van de Ven @ 2009-09-27 18:45 UTC (permalink / raw)
To: Hugh Dickins; +Cc: Johannes Berg, netdev, linux-kernel, linux-wireless
In-Reply-To: <Pine.LNX.4.64.0909271708210.8927@sister.anvils>
On Sun, 27 Sep 2009 17:14:04 +0100 (BST)
Hugh Dickins <hugh.dickins@tiscali.co.uk> wrote:
> On Sun, 27 Sep 2009, Arjan van de Ven wrote:
> >
> > With todays git my laptop fails to associate with my access point.
> > Bisection points to the commit below, and reverting this one commit
> > on the HEAD of tree also fixes the issue, so I'm pretty confident
> > that this commit is to blame.
> >
> > I have a 4965 wifi card in my laptop, and the network I'm trying to
> > connect to has no encryption. I'm running Fedora 11 as OS.
> >
> > I would like to kindly request for this commit to be reverted until
> > a more permanent solution is found (I'm happy to test any patches)..
> >
> > 94f85853324e02c3a32bc3101f090dc9a3f512b4 is first bad commit
> > commit 94f85853324e02c3a32bc3101f090dc9a3f512b4
> > Author: Johannes Berg <johannes@sipsolutions.net>
> > Date: Thu Sep 17 17:15:31 2009 -0700
> >
> > cfg80211: don't overwrite privacy setting
> >
> > When cfg80211 is instructed to connect, it always
> > uses the default WEP key for the privacy setting,
> > which clearly is wrong when using wpa_supplicant.
> > Don't overwrite the setting, and rely on it being
> > false when wpa_supplicant is not running, instead
> > set it to true when we have keys.
> >
> > Signed-off-by: Johannes Berg <johannes@sipsolutions.net>
> > Signed-off-by: John W. Linville <linville@tuxdriver.com>
> >
> > :040000 040000 27fb46273e88eefee373699eb7e3f2923ac0886b
> > 9518ee3e52c8320613cc5eee5ac54aabf082432f M net
>
> I've a different problem with wireless that Johannes is investigating
> for me on linux-wireless; but here's a patch that he pointed me to
> along the way, didn't help my issue but I expect it will help yours...
>
>
> Subject: cfg80211: don't set privacy w/o key
>
> When wpa_supplicant is used to connect to open networks,
> it causes the wdev->wext.keys to point to key memory, but
> that key memory is all empty. Only use privacy when there
> is a default key to be used.
indeed it does
can we get this into mainline soon ?
^ permalink raw reply
* Re: [bisected] Wireless regression in 2.6.32-git
From: Johannes Berg @ 2009-09-27 18:46 UTC (permalink / raw)
To: Arjan van de Ven
Cc: Hugh Dickins, netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-wireless-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20090927204548.3ee3768f-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 461 bytes --]
On Sun, 2009-09-27 at 20:45 +0200, Arjan van de Ven wrote:
> > Subject: cfg80211: don't set privacy w/o key
> >
> > When wpa_supplicant is used to connect to open networks,
> > it causes the wdev->wext.keys to point to key memory, but
> > that key memory is all empty. Only use privacy when there
> > is a default key to be used.
>
>
> indeed it does
>
> can we get this into mainline soon ?
John's on his way home I suppose.
johannes
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 801 bytes --]
^ permalink raw reply
* Re: [PATCH 1/3] lib/vsprintf.c: Add %pU - ptr to a UUID/GUID
From: Joe Perches @ 2009-09-27 19:00 UTC (permalink / raw)
To: Ingo Oeser; +Cc: linux-kernel, netdev, Greg KH
In-Reply-To: <200909271245.34000.ioe-lkml@rameria.de>
On Sun, 2009-09-27 at 12:45 +0200, Ingo Oeser wrote:
> Hi Joe,
Hello Ingo.
> On Sunday 27 September 2009, Joe Perches wrote:
> > UUID/GUIDs are somewhat common in kernel source.
> > Standardize the printed style of UUID/GUIDs by using
> > another extension to %p.
> > %pU: 01020304:0506:0708:090a:0b0c0d0e0f10
> > %pUr: 04030201:0605:0807:0a09:0b0c0d0e0f10
> Code does "01020304-0506-0708-090a-0b0c0d0e0f10".
> This is not, what commit promises. Please change the commit message!
True enough, that can change, no worries.
Does anyone have comments like:
1 what a stupid idea
2 how unnecessary
3 meh
4 bloat alert! linux is supposed to be lean
5 ok idea, bad implementation
6 sure, why not
^ permalink raw reply
* Re: [PATCH] ax25: Fix ax25_cb refcounting in ax25_ctl_ioctl
From: Jarek Poplawski @ 2009-09-27 19:02 UTC (permalink / raw)
To: Ralf Baechle DL5RB
Cc: David Miller, Bernard Pidoux F6BVP, Bernard Pidoux,
Linux Netdev List, linux-hams
In-Reply-To: <20090927171029.GA3297@del.dom.local>
On Sun, Sep 27, 2009 at 07:10:29PM +0200, Jarek Poplawski wrote:
...
> In this case valid calls (return 0) were affected too, so I guess the
> whole thing is rarely used.
And, after all, there would be only a little, invisible memory leak.
Jarek P.
^ permalink raw reply
* Re: [PATCH 1/2] net: introduce NETDEV_PRE_INIT notifier
From: Marcel Holtmann @ 2009-09-27 20:11 UTC (permalink / raw)
To: Johannes Berg; +Cc: netdev, linux-wireless
In-Reply-To: <1254075999.6583.4.camel-YfaajirXv2244ywRPIzf9A@public.gmane.org>
Hi Johannes,
> For various purposes including a wireless extensions
> bugfix, we need to hook into the netdev creation at
> a point before netdev_register_kobject(). It seems
> more generic, however, to have it even earlier. This
> will also ease doing the dev type assignment that
> Marcel was working on generically.
you are beating me to it. I can only second that this is a good idea for
the dev type assignment. Once Dave acks this I can sent a rebased patch
for the dev type stuff for WiFi.
Regards
Marcel
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 2/2] cfg80211: fix wireless handlers assignment
From: Luis R. Rodriguez @ 2009-09-27 20:30 UTC (permalink / raw)
To: Johannes Berg; +Cc: netdev, linux-wireless, Hugh Dickins
In-Reply-To: <1254076075.6583.6.camel-YfaajirXv2244ywRPIzf9A@public.gmane.org>
On Sun, Sep 27, 2009 at 11:27 AM, Johannes Berg
<johannes-cdvu00un1VgdHxzADdlk8Q@public.gmane.org> wrote:
> The point we assign dev->wireless_handlers at is too
> late, we need to do that before netdev_register_kobject()
> gets called, so use the new NETDEV_PRE_INIT notifier.
> The result of adding wireless_handlers too late is the
> disappearance of /sys/class/net/wlan0/wireless which a
> bunch of distro scripts still require.
>
> Signed-off-by: Johannes Berg <johannes-cdvu00un1VgdHxzADdlk8Q@public.gmane.org>
> ---
> This should fix the regression Hugh reported (of course requires the
> other patch which unfortunately I forgot to CC you, Hugh, I'll send you
> a copy in private).
Are these stable fixes?
Luis
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 1/3] lib/vsprintf.c: Add %pU - ptr to a UUID/GUID
From: Greg KH @ 2009-09-27 20:53 UTC (permalink / raw)
To: Joe Perches; +Cc: Ingo Oeser, linux-kernel, netdev
In-Reply-To: <1254078046.28232.183.camel@Joe-Laptop.home>
On Sun, Sep 27, 2009 at 12:00:46PM -0700, Joe Perches wrote:
> On Sun, 2009-09-27 at 12:45 +0200, Ingo Oeser wrote:
> > Hi Joe,
>
> Hello Ingo.
>
> > On Sunday 27 September 2009, Joe Perches wrote:
> > > UUID/GUIDs are somewhat common in kernel source.
> > > Standardize the printed style of UUID/GUIDs by using
> > > another extension to %p.
> > > %pU: 01020304:0506:0708:090a:0b0c0d0e0f10
> > > %pUr: 04030201:0605:0807:0a09:0b0c0d0e0f10
> > Code does "01020304-0506-0708-090a-0b0c0d0e0f10".
> > This is not, what commit promises. Please change the commit message!
>
> True enough, that can change, no worries.
>
> Does anyone have comments like:
>
> 1 what a stupid idea
> 2 how unnecessary
> 3 meh
> 4 bloat alert! linux is supposed to be lean
> 5 ok idea, bad implementation
> 6 sure, why not
Sure, why not :)
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox