Re: [PATCH] kernel/params.c: fix the module name length in param_sysfs_builtin
On Jan 23, 2008 7:13 AM, Jan Engelhardt <[EMAIL PROTECTED]> wrote:
> But nf..dada_compat.c gets linked into nf_conntrack_ipv4.ko,
> and that is what is used in /sys/module - and it fits the 20.
> Any place where nf_conntrack_l3proto_ipv4_compat would still be used?
there is a module named nf_conntrack_proto_icmp.ko, length 23. and you
can find all them by:
$ make allmodconfig && make modules
$ find -name '*.ko' -printf '%f\n' |gawk '{print length($0), $0}' |sort -n
...
24 dvb-usb-af9005-remote.ko
24 dvb-usb-dibusb-common.ko
25 nf_conntrack_proto_gre.ko
26 nf_conntrack_netbios_ns.ko
26 nf_conntrack_proto_sctp.ko
29 nf_conntrack_proto_udplite.ko
so currently tha max length of module name is 26 (in
nf_conntrack_proto_udplite), but still no any length limit to module
names in Documentation/, so we have to prepare reserved space for
modules later, or mark MODULE_NAME_LEN as the modules' name length
limit in Documentation/?
Simply speaking, MODULE_NAME_LEN does the better job.
>
--
Denis Cheng
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] kernel/params.c: fix the module name length in param_sysfs_builtin
On Jan 23, 2008 7:13 AM, Jan Engelhardt [EMAIL PROTECTED] wrote:
But nf..dada_compat.c gets linked into nf_conntrack_ipv4.ko,
and that is what is used in /sys/module - and it fits the 20.
Any place where nf_conntrack_l3proto_ipv4_compat would still be used?
there is a module named nf_conntrack_proto_icmp.ko, length 23. and you
can find all them by:
$ make allmodconfig make modules
$ find -name '*.ko' -printf '%f\n' |gawk '{print length($0), $0}' |sort -n
...
24 dvb-usb-af9005-remote.ko
24 dvb-usb-dibusb-common.ko
25 nf_conntrack_proto_gre.ko
26 nf_conntrack_netbios_ns.ko
26 nf_conntrack_proto_sctp.ko
29 nf_conntrack_proto_udplite.ko
so currently tha max length of module name is 26 (in
nf_conntrack_proto_udplite), but still no any length limit to module
names in Documentation/, so we have to prepare reserved space for
modules later, or mark MODULE_NAME_LEN as the modules' name length
limit in Documentation/?
Simply speaking, MODULE_NAME_LEN does the better job.
--
Denis Cheng
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] kernel/params.c: fix the module name length in param_sysfs_builtin
From: Denis Cheng <[EMAIL PROTECTED]>
Date: Sat, 19 Jan 2008 13:29:51 +0800
Subject: [PATCH] kernel/params.c: fix the module name length in
param_sysfs_builtin
the original code use KOBJ_NAME_LEN for built-in module name length,
that's defined to 20 in linux/kobject.h, but this is not enough appearntly,
many module names are longer than this;
#define KOBJ_NAME_LEN 20
another macro is MODULE_NAME_LEN defined in linux/module.h, I think this is
enough for module names:
#define MODULE_NAME_LEN (64 - sizeof(unsigned long))
Signed-off-by: Denis Cheng <[EMAIL PROTECTED]>
---
kernel/params.c |8 +++-
1 files changed, 3 insertions(+), 5 deletions(-)
diff --git a/kernel/params.c b/kernel/params.c
index 7686417..a085b40 100644
--- a/kernel/params.c
+++ b/kernel/params.c
@@ -376,8 +376,6 @@ int param_get_string(char *buffer, struct kernel_param *kp)
extern struct kernel_param __start___param[], __stop___param[];
-#define MAX_KBUILD_MODNAME KOBJ_NAME_LEN
-
struct param_attribute
{
struct module_attribute mattr;
@@ -588,7 +586,7 @@ static void __init param_sysfs_builtin(void)
{
struct kernel_param *kp, *kp_begin = NULL;
unsigned int i, name_len, count = 0;
- char modname[MAX_KBUILD_MODNAME + 1] = "";
+ char modname[MODULE_NAME_LEN + 1] = "";
for (i=0; i < __stop___param - __start___param; i++) {
char *dot;
@@ -596,12 +594,12 @@ static void __init param_sysfs_builtin(void)
kp = &__start___param[i];
max_name_len =
- min_t(size_t, MAX_KBUILD_MODNAME, strlen(kp->name));
+ min_t(size_t, MODULE_NAME_LEN, strlen(kp->name));
dot = memchr(kp->name, '.', max_name_len);
if (!dot) {
DEBUGP("couldn't find period in first %d characters "
- "of %s\n", MAX_KBUILD_MODNAME, kp->name);
+ "of %s\n", MODULE_NAME_LEN, kp->name);
continue;
}
name_len = dot - kp->name;
--
1.5.3.5
--
Denis Cheng
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] module: add modinfo support for all built-in modules
On Jan 16, 2008 8:25 PM, Rusty Russell <[EMAIL PROTECTED]> wrote:
> I'd love to see patches. module_parm showed it's possible, if messy.
>
> Thanks!
> Rusty.
here's the patch, I added .modinfo section to the vmlinux, to collect
built-in module information.
I have just define __MODULE_INFO to another meaning while
CONFIG_MODULES undefined
(modules compiled built-in), instead of nothing; and so the
MODULE_LICENSE, MODULE_AUTHOR,
MODULE_DESCRIPTION's meaning also changed, each macro would define one
struct kernel_modinfo
entry in the .modinfo section of vmlinux; and one __initcall converts
all these information to read-only
files under /sys/modules//...
but the MODULE_PARM_DESC macro is still different:
it generates entries with the same tag, that would confuse
sys_create_group, so I skipped them in the __initcall,
since the parameters had been in /sys/modules/<>/parameters/(with perm
non-zero) or didn't appear(with perm 0);
I think the parameter description might be only useful for external
module files, not needed in memory(under /sys/module/),
so a better solution is define MODULE_PARM_DESC to nothing while
CONFIG_MODULES undefined.
Another possible defect is that it compares two modname with
(km->modname != modname),
that depends on a gcc feature: keep same constant string only one copy
in the image,
this did work on my test machines, but I'm not sure it's standard or
not; and if not, I would change it to strcmp.
Apperantly this approach will increase the kernel image size. on a
moderate system(with 1.8MB bzImage),
this patch would increase vmlinux 46KB and after compression increase
bzImage 9.2KB.
and in the increment of vmlinux, the .modinfo section occupied 5.3KB
and others are constant strings.
However, the iscsid can now work well when scsi_transport_iscsi module
built-in without the problem refered in my former email.
please give comments.
>From 50831a260b1ad2c8b495854a58408c1fbc75a3fe Mon Sep 17 00:00:00 2001
From: Denis Cheng <[EMAIL PROTECTED]>
Date: Fri, 18 Jan 2008 16:37:35 +0800
Subject: [PATCH] module: add modinfo support for all built-in modules
the current modinfo support is for external modules only, it provided module
information under /sys/module//, such as verion, ...;
now some application(such as iscsid of open-iscsi) has been designed to
use this module information; but built-in modules don't have modinfo support,
so these apps would break if modules they depend on are compiled built-in.
this patch add modinfo support for all built-in modules, so now no matter
whether modules they depends on are built-in or external, modules' information
could always be accessed from /sys/module//version, apps won't break.
Signed-off-by: Denis Cheng <[EMAIL PROTECTED]>
---
include/asm-generic/vmlinux.lds.h |7 ++
include/linux/moduleparam.h | 18 -
kernel/module.c | 147 +
3 files changed, 170 insertions(+), 2 deletions(-)
diff --git a/include/asm-generic/vmlinux.lds.h
b/include/asm-generic/vmlinux.lds.h
index 9f584cc..896f0fe 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -137,6 +137,13 @@
VMLINUX_SYMBOL(__start___param) = .;\
*(__param) \
VMLINUX_SYMBOL(__stop___param) = .; \
+ } \
+ \
+ /* Built-in module information. */ \
+ .modinfo : AT(ADDR(.modinfo) - LOAD_OFFSET) { \
+ VMLINUX_SYMBOL(__start___modinfo) = .; \
+ *(.modinfo) \
+ VMLINUX_SYMBOL(__stop___modinfo) = .; \
VMLINUX_SYMBOL(__end_rodata) = .; \
} \
\
diff --git a/include/linux/moduleparam.h b/include/linux/moduleparam.h
index 13410b2..86ddbd4 100644
--- a/include/linux/moduleparam.h
+++ b/include/linux/moduleparam.h
@@ -13,16 +13,30 @@
#define MODULE_PARAM_PREFIX KBUILD_MODNAME "."
#endif
-#ifdef MODULE
#define ___module_cat(a,b) __mod_ ## a ## b
#define __module_cat(a,b) ___module_cat(a,b)
+
+#ifdef MODULE
#define __MODULE_INFO(tag, name, info) \
static const char __module_cat(name,__LINE__)[]
\
__attribute_used__ \
__attribute__((section(".modinfo"),unused)) = __stringify(tag) "=" info
#else /* !MODULE */
-#define __MODULE_INFO(tag, name, info)
+struct kernel_modinfo {
+ char *modname;
+ char *tag;
+ char *info;
+};
+#define
[PATCH] module: add modinfo support for all built-in modules
On Jan 16, 2008 8:25 PM, Rusty Russell [EMAIL PROTECTED] wrote:
I'd love to see patches. module_parm showed it's possible, if messy.
Thanks!
Rusty.
here's the patch, I added .modinfo section to the vmlinux, to collect
built-in module information.
I have just define __MODULE_INFO to another meaning while
CONFIG_MODULES undefined
(modules compiled built-in), instead of nothing; and so the
MODULE_LICENSE, MODULE_AUTHOR,
MODULE_DESCRIPTION's meaning also changed, each macro would define one
struct kernel_modinfo
entry in the .modinfo section of vmlinux; and one __initcall converts
all these information to read-only
files under /sys/modules/module-name/...
but the MODULE_PARM_DESC macro is still different:
it generates entries with the same tag, that would confuse
sys_create_group, so I skipped them in the __initcall,
since the parameters had been in /sys/modules//parameters/(with perm
non-zero) or didn't appear(with perm 0);
I think the parameter description might be only useful for external
module files, not needed in memory(under /sys/module/),
so a better solution is define MODULE_PARM_DESC to nothing while
CONFIG_MODULES undefined.
Another possible defect is that it compares two modname with
(km-modname != modname),
that depends on a gcc feature: keep same constant string only one copy
in the image,
this did work on my test machines, but I'm not sure it's standard or
not; and if not, I would change it to strcmp.
Apperantly this approach will increase the kernel image size. on a
moderate system(with 1.8MB bzImage),
this patch would increase vmlinux 46KB and after compression increase
bzImage 9.2KB.
and in the increment of vmlinux, the .modinfo section occupied 5.3KB
and others are constant strings.
However, the iscsid can now work well when scsi_transport_iscsi module
built-in without the problem refered in my former email.
please give comments.
From 50831a260b1ad2c8b495854a58408c1fbc75a3fe Mon Sep 17 00:00:00 2001
From: Denis Cheng [EMAIL PROTECTED]
Date: Fri, 18 Jan 2008 16:37:35 +0800
Subject: [PATCH] module: add modinfo support for all built-in modules
the current modinfo support is for external modules only, it provided module
information under /sys/module/XYZ/, such as verion, ...;
now some application(such as iscsid of open-iscsi) has been designed to
use this module information; but built-in modules don't have modinfo support,
so these apps would break if modules they depend on are compiled built-in.
this patch add modinfo support for all built-in modules, so now no matter
whether modules they depends on are built-in or external, modules' information
could always be accessed from /sys/module/XYZ/version, apps won't break.
Signed-off-by: Denis Cheng [EMAIL PROTECTED]
---
include/asm-generic/vmlinux.lds.h |7 ++
include/linux/moduleparam.h | 18 -
kernel/module.c | 147 +
3 files changed, 170 insertions(+), 2 deletions(-)
diff --git a/include/asm-generic/vmlinux.lds.h
b/include/asm-generic/vmlinux.lds.h
index 9f584cc..896f0fe 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -137,6 +137,13 @@
VMLINUX_SYMBOL(__start___param) = .;\
*(__param) \
VMLINUX_SYMBOL(__stop___param) = .; \
+ } \
+ \
+ /* Built-in module information. */ \
+ .modinfo : AT(ADDR(.modinfo) - LOAD_OFFSET) { \
+ VMLINUX_SYMBOL(__start___modinfo) = .; \
+ *(.modinfo) \
+ VMLINUX_SYMBOL(__stop___modinfo) = .; \
VMLINUX_SYMBOL(__end_rodata) = .; \
} \
\
diff --git a/include/linux/moduleparam.h b/include/linux/moduleparam.h
index 13410b2..86ddbd4 100644
--- a/include/linux/moduleparam.h
+++ b/include/linux/moduleparam.h
@@ -13,16 +13,30 @@
#define MODULE_PARAM_PREFIX KBUILD_MODNAME .
#endif
-#ifdef MODULE
#define ___module_cat(a,b) __mod_ ## a ## b
#define __module_cat(a,b) ___module_cat(a,b)
+
+#ifdef MODULE
#define __MODULE_INFO(tag, name, info) \
static const char __module_cat(name,__LINE__)[]
\
__attribute_used__ \
__attribute__((section(.modinfo),unused)) = __stringify(tag) = info
#else /* !MODULE */
-#define __MODULE_INFO(tag, name, info)
+struct kernel_modinfo {
+ char *modname;
+ char *tag;
+ char *info;
+};
+#define
[PATCH] kernel/params.c: fix the module name length in param_sysfs_builtin
From: Denis Cheng [EMAIL PROTECTED]
Date: Sat, 19 Jan 2008 13:29:51 +0800
Subject: [PATCH] kernel/params.c: fix the module name length in
param_sysfs_builtin
the original code use KOBJ_NAME_LEN for built-in module name length,
that's defined to 20 in linux/kobject.h, but this is not enough appearntly,
many module names are longer than this;
#define KOBJ_NAME_LEN 20
another macro is MODULE_NAME_LEN defined in linux/module.h, I think this is
enough for module names:
#define MODULE_NAME_LEN (64 - sizeof(unsigned long))
Signed-off-by: Denis Cheng [EMAIL PROTECTED]
---
kernel/params.c |8 +++-
1 files changed, 3 insertions(+), 5 deletions(-)
diff --git a/kernel/params.c b/kernel/params.c
index 7686417..a085b40 100644
--- a/kernel/params.c
+++ b/kernel/params.c
@@ -376,8 +376,6 @@ int param_get_string(char *buffer, struct kernel_param *kp)
extern struct kernel_param __start___param[], __stop___param[];
-#define MAX_KBUILD_MODNAME KOBJ_NAME_LEN
-
struct param_attribute
{
struct module_attribute mattr;
@@ -588,7 +586,7 @@ static void __init param_sysfs_builtin(void)
{
struct kernel_param *kp, *kp_begin = NULL;
unsigned int i, name_len, count = 0;
- char modname[MAX_KBUILD_MODNAME + 1] = ;
+ char modname[MODULE_NAME_LEN + 1] = ;
for (i=0; i __stop___param - __start___param; i++) {
char *dot;
@@ -596,12 +594,12 @@ static void __init param_sysfs_builtin(void)
kp = __start___param[i];
max_name_len =
- min_t(size_t, MAX_KBUILD_MODNAME, strlen(kp-name));
+ min_t(size_t, MODULE_NAME_LEN, strlen(kp-name));
dot = memchr(kp-name, '.', max_name_len);
if (!dot) {
DEBUGP(couldn't find period in first %d characters
- of %s\n, MAX_KBUILD_MODNAME, kp-name);
+ of %s\n, MODULE_NAME_LEN, kp-name);
continue;
}
name_len = dot - kp-name;
--
1.5.3.5
--
Denis Cheng
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[RFC on MODULE SUPPORT] hello, Rusty, Should we provide module information even if the kernel module compiled built-in with bzImage?
hello, Rusty: I encountered a problem when modules compiled built-in with bzImage: open-iscsi is an iSCSI software, it has a userspace daemon(iscsid) and a userspace mani tool(iscsiadm) and a kernel module (scsi_transport_iscsi), recently the kernel module has been accepted into the official kernel release; since the module licensed with GPL, it could be compiled as built-in, but when I compiled the module within the bzImage, the problem appeared: tux ~ # iscsid -f iscsid: Missing or Invalid version from /sys/module/scsi_transport_iscsi/version. Make sure a up to date scsi_transport_iscsi module is loaded and a up todate version of iscsid is running. Exiting... this is just because iscsid hope there's an external module could be under /sys/module, and read the kernel module's version information, but if the module compiled built-in, all its module information discarded and it doesn't appeared under /sys/module/, that would break iscsid. Now the problem is: Should we provide module information under /sys/module//... even if the module compiled built-in with bzImage? Or just this module(scsi_transport_iscsi) should be marked with [M] only? if the former solution is preferred, I would be happy to work on MODULE_INFO-like macros improvements with CONFIG_MODULE undefined. -- Denis -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[RFC on MODULE SUPPORT] hello, Rusty, Should we provide module information even if the kernel module compiled built-in with bzImage?
hello, Rusty: I encountered a problem when modules compiled built-in with bzImage: open-iscsi is an iSCSI software, it has a userspace daemon(iscsid) and a userspace mani tool(iscsiadm) and a kernel module (scsi_transport_iscsi), recently the kernel module has been accepted into the official kernel release; since the module licensed with GPL, it could be compiled as built-in, but when I compiled the module within the bzImage, the problem appeared: tux ~ # iscsid -f iscsid: Missing or Invalid version from /sys/module/scsi_transport_iscsi/version. Make sure a up to date scsi_transport_iscsi module is loaded and a up todate version of iscsid is running. Exiting... this is just because iscsid hope there's an external module could be under /sys/module, and read the kernel module's version information, but if the module compiled built-in, all its module information discarded and it doesn't appeared under /sys/module/, that would break iscsid. Now the problem is: Should we provide module information under /sys/module/module-name/... even if the module compiled built-in with bzImage? Or just this module(scsi_transport_iscsi) should be marked with [M] only? if the former solution is preferred, I would be happy to work on MODULE_INFO-like macros improvements with CONFIG_MODULE undefined. -- Denis -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] PROC_FS: get and set the smp affinity of tasks by read-write /proc//smp_affinity
On Jan 10, 2008 8:33 AM, Andrew Morton <[EMAIL PROTECTED]> wrote:
> On Fri, 4 Jan 2008 15:03:41 +0800
> Denis Cheng <[EMAIL PROTECTED]> wrote:
>
> > this adds a read-write /proc//smp_affinity entry,
> > just like what /proc/irq//smp_affinity does,
> > so now we can get and set the affinity of tasks by procfs,
> > this is especially useful used in shell scripts.
> >
> > this also adds a read-write /proc//tasks//smp_affinity
> > for the same purpose.
>
> Why not use /usr/bin/taskset?
I know /usr/bin/tasklet is another way to set smp affinity of a task,
it uses the sched_{set,get}affinity system call binary interface, but
add a /proc//smp_affinity could give a new choice; and this keeps
consistency with /proc/irq//smp_affinity, will be familiar to
most people.
Another way, the sysctl system call binary interface has been marked
deprecated recently, only /proc/sys/ operating interface left; I
wonder this this would be a tendency of virtual filesystem interface
to replace binary interface? In my opinion, vfs text interface is
always better than system call binary interface.
>
--
Denis Cheng
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] PROC_FS: get and set the smp affinity of tasks by read-write /proc/pid/smp_affinity
On Jan 10, 2008 8:33 AM, Andrew Morton [EMAIL PROTECTED] wrote:
On Fri, 4 Jan 2008 15:03:41 +0800
Denis Cheng [EMAIL PROTECTED] wrote:
this adds a read-write /proc/pid/smp_affinity entry,
just like what /proc/irq/irq/smp_affinity does,
so now we can get and set the affinity of tasks by procfs,
this is especially useful used in shell scripts.
this also adds a read-write /proc/pid/tasks/tid/smp_affinity
for the same purpose.
Why not use /usr/bin/taskset?
I know /usr/bin/tasklet is another way to set smp affinity of a task,
it uses the sched_{set,get}affinity system call binary interface, but
add a /proc/pid/smp_affinity could give a new choice; and this keeps
consistency with /proc/irq/irq/smp_affinity, will be familiar to
most people.
Another way, the sysctl system call binary interface has been marked
deprecated recently, only /proc/sys/ operating interface left; I
wonder this this would be a tendency of virtual filesystem interface
to replace binary interface? In my opinion, vfs text interface is
always better than system call binary interface.
--
Denis Cheng
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] PROC_FS: get and set the smp affinity of tasks by read-write /proc//smp_affinity
>From d2be88406fdc1d28a7cf0b1a13ca761d625820a5 Mon Sep 17 00:00:00 2001
From: Denis Cheng <[EMAIL PROTECTED]>
Date: Fri, 4 Jan 2008 14:50:54 +0800
Subject: [PATCH] PROC_FS: get and set the smp affinity of tasks by
read-write /proc//smp_affinity
this adds a read-write /proc//smp_affinity entry,
just like what /proc/irq//smp_affinity does,
so now we can get and set the affinity of tasks by procfs,
this is especially useful used in shell scripts.
this also adds a read-write /proc//tasks//smp_affinity
for the same purpose.
Cc: Eli M Dow <[EMAIL PROTECTED]>
Signed-off-by: Denis Cheng <[EMAIL PROTECTED]>
---
length check copied from kernel/irq/proc.c, now 'page' buffer couldn't
be overrun,
Another way, although Documentation/filesystems/proc.txt is heavily outdated,
I'll add introduction of smp_affinity.
fs/proc/base.c | 65
1 files changed, 65 insertions(+), 0 deletions(-)
diff --git a/fs/proc/base.c b/fs/proc/base.c
index 7411bfb..d768d66 100644
--- a/fs/proc/base.c
+++ b/fs/proc/base.c
@@ -1083,6 +1083,65 @@ static const struct file_operations
proc_pid_sched_operations = {
#endif
+#ifdef CONFIG_SMP
+static ssize_t smp_affinity_read(struct file *file,
+ char __user *buf, size_t count, loff_t *ppos)
+{
+ cpumask_t mask;
+ char *page;
+ ssize_t length;
+ pid_t pid = pid_nr(proc_pid(file->f_dentry->d_inode));
+
+ length = sched_getaffinity(pid, );
+ if (length < 0)
+ return length;
+
+ if (count > PROC_BLOCK_SIZE)
+ count = PROC_BLOCK_SIZE;
+
+ page = (char *)__get_free_page(GFP_TEMPORARY);
+ if (!page) {
+ length = -ENOMEM;
+ goto out;
+ }
+
+ length = cpumask_scnprintf(page, count, mask);
+ if (count - length < 2) {
+ length = -EINVAL;
+ goto out_free_page;
+ }
+ length += sprintf(page + length, "\n");
+ length = simple_read_from_buffer(buf, count, ppos, page, length);
+
+out_free_page:
+ free_page((unsigned long)page);
+out:
+ return length;
+}
+
+static ssize_t smp_affinity_write(struct file *file,
+ const char __user *buf, size_t count, loff_t *ppos)
+{
+ cpumask_t mask;
+ int ret;
+ pid_t pid = pid_nr(proc_pid(file->f_dentry->d_inode));
+
+ ret = cpumask_parse_user(buf, count, mask);
+ if (ret < 0)
+ return ret;
+ ret = sched_setaffinity(pid, mask);
+ if (ret < 0)
+ return ret;
+
+ return count;
+}
+
+static const struct file_operations proc_smp_affinity_operations = {
+ .read = smp_affinity_read,
+ .write = smp_affinity_write,
+};
+#endif
+
static void *proc_pid_follow_link(struct dentry *dentry, struct nameidata *nd)
{
struct inode *inode = dentry->d_inode;
@@ -2230,6 +2289,9 @@ static const struct pid_entry tgid_base_stuff[] = {
#ifdef CONFIG_SCHEDSTATS
INF("schedstat", S_IRUGO, pid_schedstat),
#endif
+#ifdef CONFIG_SMP
+ REG("smp_affinity", S_IRUGO|S_IWUSR, smp_affinity),
+#endif
#ifdef CONFIG_PROC_PID_CPUSET
REG("cpuset", S_IRUGO, cpuset),
#endif
@@ -2555,6 +2617,9 @@ static const struct pid_entry tid_base_stuff[] = {
#ifdef CONFIG_SCHEDSTATS
INF("schedstat", S_IRUGO, pid_schedstat),
#endif
+#ifdef CONFIG_SMP
+ REG("smp_affinity", S_IRUGO|S_IWUSR, smp_affinity),
+#endif
#ifdef CONFIG_PROC_PID_CPUSET
REG("cpuset",S_IRUGO, cpuset),
#endif
--
1.5.3.5
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] PROC_FS: get and set the smp affinity of tasks by read-write /proc/pid/smp_affinity
From d2be88406fdc1d28a7cf0b1a13ca761d625820a5 Mon Sep 17 00:00:00 2001
From: Denis Cheng [EMAIL PROTECTED]
Date: Fri, 4 Jan 2008 14:50:54 +0800
Subject: [PATCH] PROC_FS: get and set the smp affinity of tasks by
read-write /proc/pid/smp_affinity
this adds a read-write /proc/pid/smp_affinity entry,
just like what /proc/irq/irq/smp_affinity does,
so now we can get and set the affinity of tasks by procfs,
this is especially useful used in shell scripts.
this also adds a read-write /proc/pid/tasks/tid/smp_affinity
for the same purpose.
Cc: Eli M Dow [EMAIL PROTECTED]
Signed-off-by: Denis Cheng [EMAIL PROTECTED]
---
length check copied from kernel/irq/proc.c, now 'page' buffer couldn't
be overrun,
Another way, although Documentation/filesystems/proc.txt is heavily outdated,
I'll add introduction of smp_affinity.
fs/proc/base.c | 65
1 files changed, 65 insertions(+), 0 deletions(-)
diff --git a/fs/proc/base.c b/fs/proc/base.c
index 7411bfb..d768d66 100644
--- a/fs/proc/base.c
+++ b/fs/proc/base.c
@@ -1083,6 +1083,65 @@ static const struct file_operations
proc_pid_sched_operations = {
#endif
+#ifdef CONFIG_SMP
+static ssize_t smp_affinity_read(struct file *file,
+ char __user *buf, size_t count, loff_t *ppos)
+{
+ cpumask_t mask;
+ char *page;
+ ssize_t length;
+ pid_t pid = pid_nr(proc_pid(file-f_dentry-d_inode));
+
+ length = sched_getaffinity(pid, mask);
+ if (length 0)
+ return length;
+
+ if (count PROC_BLOCK_SIZE)
+ count = PROC_BLOCK_SIZE;
+
+ page = (char *)__get_free_page(GFP_TEMPORARY);
+ if (!page) {
+ length = -ENOMEM;
+ goto out;
+ }
+
+ length = cpumask_scnprintf(page, count, mask);
+ if (count - length 2) {
+ length = -EINVAL;
+ goto out_free_page;
+ }
+ length += sprintf(page + length, \n);
+ length = simple_read_from_buffer(buf, count, ppos, page, length);
+
+out_free_page:
+ free_page((unsigned long)page);
+out:
+ return length;
+}
+
+static ssize_t smp_affinity_write(struct file *file,
+ const char __user *buf, size_t count, loff_t *ppos)
+{
+ cpumask_t mask;
+ int ret;
+ pid_t pid = pid_nr(proc_pid(file-f_dentry-d_inode));
+
+ ret = cpumask_parse_user(buf, count, mask);
+ if (ret 0)
+ return ret;
+ ret = sched_setaffinity(pid, mask);
+ if (ret 0)
+ return ret;
+
+ return count;
+}
+
+static const struct file_operations proc_smp_affinity_operations = {
+ .read = smp_affinity_read,
+ .write = smp_affinity_write,
+};
+#endif
+
static void *proc_pid_follow_link(struct dentry *dentry, struct nameidata *nd)
{
struct inode *inode = dentry-d_inode;
@@ -2230,6 +2289,9 @@ static const struct pid_entry tgid_base_stuff[] = {
#ifdef CONFIG_SCHEDSTATS
INF(schedstat, S_IRUGO, pid_schedstat),
#endif
+#ifdef CONFIG_SMP
+ REG(smp_affinity, S_IRUGO|S_IWUSR, smp_affinity),
+#endif
#ifdef CONFIG_PROC_PID_CPUSET
REG(cpuset, S_IRUGO, cpuset),
#endif
@@ -2555,6 +2617,9 @@ static const struct pid_entry tid_base_stuff[] = {
#ifdef CONFIG_SCHEDSTATS
INF(schedstat, S_IRUGO, pid_schedstat),
#endif
+#ifdef CONFIG_SMP
+ REG(smp_affinity, S_IRUGO|S_IWUSR, smp_affinity),
+#endif
#ifdef CONFIG_PROC_PID_CPUSET
REG(cpuset,S_IRUGO, cpuset),
#endif
--
1.5.3.5
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: Since sysfs_mount is static and used only in sysfs_init function, it could be just an automatic variable.
On Dec 2, 2007 12:48 PM, Greg KH <[EMAIL PROTECTED]> wrote:
...
> > and where is a detailed explaination on kern_mount? could someone give
> > some comments or documentation pointers on this?
>
> See the patches that Eric Biederman just posted to lkml for why this
> structure is a static pointer this way right now, it's in preparation
> for future patches.
I have checked commit 7d0c7d676cc066413e1583b5af9fba8011972d41 by Eric
W. Biederman,
http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=7d0c7d676cc066413e1583b5af9fba8011972d41
which just make sysfs_mount from externally visible to static that
could be only used in one c file,
but I mean that the static variable is still on kernel bss section,
this consumes a pointer (4 or 8 bytes) memory,
through a grep from fs/sysfs/, it appears that the variable
sysfs_mount is only used in the sysfs_init function,
$ grep -RsInw sysfs_mount fs/sysfs/
fs/sysfs/mount.c:25:static struct vfsmount *sysfs_mount;
fs/sysfs/mount.c:101: sysfs_mount = kern_mount(_fs_type);
fs/sysfs/mount.c:102: if (IS_ERR(sysfs_mount)) {
fs/sysfs/mount.c:104: err = PTR_ERR(sysfs_mount);
fs/sysfs/mount.c:105: sysfs_mount = NULL;
we could mark this variable an automatic one, which scope is just in
this function, thus created and destroyed with the stack,
this approach does not consume a pointer on kernel bss section,
Why not do this?
--
Denis Cheng
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Since sysfs_mount is static and used only in sysfs_init function, it could be just an automatic variable.
---
and I still have questions about this code:
1. Why here kern_mount is needed?
Or the first time user space `mount -t sysfs` won't do that?
2. If root executes many mounts to mount sysfs on /sys and many other places,
are there many instances of struct vfsmount those have only
mnt_mountpoint different?
For most common case, mount a virtual filesystem(proc, sysfs, ...) on
multiple mounting point,
how to handle it more efficiently?
and where is a detailed explaination on kern_mount? could someone give
some comments or documentation pointers on this?
diff --git a/fs/sysfs/mount.c b/fs/sysfs/mount.c
index 7416826..add0dda 100644
--- a/fs/sysfs/mount.c
+++ b/fs/sysfs/mount.c
@@ -22,7 +22,6 @@
/* Random magic number */
#define SYSFS_MAGIC 0x62656572
-static struct vfsmount *sysfs_mount;
struct super_block * sysfs_sb = NULL;
struct kmem_cache *sysfs_dir_cachep;
@@ -98,11 +97,10 @@ int __init sysfs_init(void)
err = register_filesystem(_fs_type);
if (!err) {
- sysfs_mount = kern_mount(_fs_type);
+ struct vfsmount *sysfs_mount = kern_mount(_fs_type);
if (IS_ERR(sysfs_mount)) {
printk(KERN_ERR "sysfs: could not mount!\n");
err = PTR_ERR(sysfs_mount);
- sysfs_mount = NULL;
unregister_filesystem(_fs_type);
goto out_err;
}
--
Denis Cheng
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Since sysfs_mount is static and used only in sysfs_init function, it could be just an automatic variable.
---
and I still have questions about this code:
1. Why here kern_mount is needed?
Or the first time user space `mount -t sysfs` won't do that?
2. If root executes many mounts to mount sysfs on /sys and many other places,
are there many instances of struct vfsmount those have only
mnt_mountpoint different?
For most common case, mount a virtual filesystem(proc, sysfs, ...) on
multiple mounting point,
how to handle it more efficiently?
and where is a detailed explaination on kern_mount? could someone give
some comments or documentation pointers on this?
diff --git a/fs/sysfs/mount.c b/fs/sysfs/mount.c
index 7416826..add0dda 100644
--- a/fs/sysfs/mount.c
+++ b/fs/sysfs/mount.c
@@ -22,7 +22,6 @@
/* Random magic number */
#define SYSFS_MAGIC 0x62656572
-static struct vfsmount *sysfs_mount;
struct super_block * sysfs_sb = NULL;
struct kmem_cache *sysfs_dir_cachep;
@@ -98,11 +97,10 @@ int __init sysfs_init(void)
err = register_filesystem(sysfs_fs_type);
if (!err) {
- sysfs_mount = kern_mount(sysfs_fs_type);
+ struct vfsmount *sysfs_mount = kern_mount(sysfs_fs_type);
if (IS_ERR(sysfs_mount)) {
printk(KERN_ERR sysfs: could not mount!\n);
err = PTR_ERR(sysfs_mount);
- sysfs_mount = NULL;
unregister_filesystem(sysfs_fs_type);
goto out_err;
}
--
Denis Cheng
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: Since sysfs_mount is static and used only in sysfs_init function, it could be just an automatic variable.
On Dec 2, 2007 12:48 PM, Greg KH [EMAIL PROTECTED] wrote:
...
and where is a detailed explaination on kern_mount? could someone give
some comments or documentation pointers on this?
See the patches that Eric Biederman just posted to lkml for why this
structure is a static pointer this way right now, it's in preparation
for future patches.
I have checked commit 7d0c7d676cc066413e1583b5af9fba8011972d41 by Eric
W. Biederman,
http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=7d0c7d676cc066413e1583b5af9fba8011972d41
which just make sysfs_mount from externally visible to static that
could be only used in one c file,
but I mean that the static variable is still on kernel bss section,
this consumes a pointer (4 or 8 bytes) memory,
through a grep from fs/sysfs/, it appears that the variable
sysfs_mount is only used in the sysfs_init function,
$ grep -RsInw sysfs_mount fs/sysfs/
fs/sysfs/mount.c:25:static struct vfsmount *sysfs_mount;
fs/sysfs/mount.c:101: sysfs_mount = kern_mount(sysfs_fs_type);
fs/sysfs/mount.c:102: if (IS_ERR(sysfs_mount)) {
fs/sysfs/mount.c:104: err = PTR_ERR(sysfs_mount);
fs/sysfs/mount.c:105: sysfs_mount = NULL;
we could mark this variable an automatic one, which scope is just in
this function, thus created and destroyed with the stack,
this approach does not consume a pointer on kernel bss section,
Why not do this?
--
Denis Cheng
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] [RESEND] crypto test: use print_hex_dump from kernel.h instead
On Nov 29, 2007 7:13 PM, Herbert Xu <[EMAIL PROTECTED]> wrote:
...
> > uninlining this function shrinks crypto/tcrypt.o's .text from 20,009 bytes
> > down to 19,701.
> >
> > inlining is almost always wrong.
>
> I agree. Please do as Andrew suggests and resubmit.
inline disabled.
Cc: Randy Dunlap <[EMAIL PROTECTED]>
Signed-off-by: Denis Cheng <[EMAIL PROTECTED]>
---
diff --git a/crypto/tcrypt.c b/crypto/tcrypt.c
index 24141fb..13efc72 100644
--- a/crypto/tcrypt.c
+++ b/crypto/tcrypt.c
@@ -83,10 +83,9 @@ static char *check[] = {
static void hexdump(unsigned char *buf, unsigned int len)
{
- while (len--)
- printk("%02x", *buf++);
-
- printk("\n");
+ print_hex_dump(KERN_CONT, "", DUMP_PREFIX_OFFSET,
+ 16, 1,
+ buf, len, false);
}
static void tcrypt_complete(struct crypto_async_request *req, int err)
--
Denis Cheng
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] [RESEND] crypto test: use print_hex_dump from kernel.h instead
On Nov 29, 2007 7:13 PM, Herbert Xu [EMAIL PROTECTED] wrote:
...
uninlining this function shrinks crypto/tcrypt.o's .text from 20,009 bytes
down to 19,701.
inlining is almost always wrong.
I agree. Please do as Andrew suggests and resubmit.
inline disabled.
Cc: Randy Dunlap [EMAIL PROTECTED]
Signed-off-by: Denis Cheng [EMAIL PROTECTED]
---
diff --git a/crypto/tcrypt.c b/crypto/tcrypt.c
index 24141fb..13efc72 100644
--- a/crypto/tcrypt.c
+++ b/crypto/tcrypt.c
@@ -83,10 +83,9 @@ static char *check[] = {
static void hexdump(unsigned char *buf, unsigned int len)
{
- while (len--)
- printk(%02x, *buf++);
-
- printk(\n);
+ print_hex_dump(KERN_CONT, , DUMP_PREFIX_OFFSET,
+ 16, 1,
+ buf, len, false);
}
static void tcrypt_complete(struct crypto_async_request *req, int err)
--
Denis Cheng
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] [RESEND] crypto test: use print_hex_dump from kernel.h instead
On Nov 27, 2007 10:58 AM, Richard Knutsson <[EMAIL PROTECTED]> wrote:
...
> > + print_hex_dump(KERN_CONT, "", DUMP_PREFIX_OFFSET,
> > + 16, 1,
> > + buf, len, 0);
> >
> Not important, but why use '0' instead of 'false'?
after read http://lkml.org/lkml/2006/7/27/281, I agreed with you.
this is refreshed patch against the lastest cryptodev tree.
Cc: Randy Dunlap <[EMAIL PROTECTED]>
Signed-off-by: Denis Cheng <[EMAIL PROTECTED]>
---
crypto/tcrypt.c |9 -
1 files changed, 4 insertions(+), 5 deletions(-)
diff --git a/crypto/tcrypt.c b/crypto/tcrypt.c
index 1e12b86..ae762c2 100644
--- a/crypto/tcrypt.c
+++ b/crypto/tcrypt.c
@@ -87,12 +87,11 @@ static char *check[] = {
"camellia", "seed", "salsa20", NULL
};
-static void hexdump(unsigned char *buf, unsigned int len)
+static inline void hexdump(unsigned char *buf, unsigned int len)
{
- while (len--)
- printk("%02x", *buf++);
-
- printk("\n");
+ print_hex_dump(KERN_CONT, "", DUMP_PREFIX_OFFSET,
+ 16, 1,
+ buf, len, false);
}
static void tcrypt_complete(struct crypto_async_request *req, int err)
--
Denis Cheng
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2] ide-scsi: use print_hex_dump from
On Nov 26, 2007 3:41 PM, Joe Perches <[EMAIL PROTECTED]> wrote:
> On Mon, 2007-11-26 at 15:16 +0800, Denis Cheng wrote:
> > diff --git a/drivers/scsi/ide-scsi.c b/drivers/scsi/ide-scsi.c
> > index 8d0244c..8f3fc1d 100644
> > --- a/drivers/scsi/ide-scsi.c
> > +++ b/drivers/scsi/ide-scsi.c
> > @@ -282,7 +272,7 @@ static int idescsi_check_condition(ide_drive_t *drive,
> > struct request *failed_co
> > pc->scsi_cmd = ((idescsi_pc_t *) failed_command->special)->scsi_cmd;
> > if (test_bit(IDESCSI_LOG_CMD, >log)) {
> > printk ("ide-scsi: %s: queue cmd = ", drive->name);
> > - hexdump(pc->c, 6);
> > + print_hex_dump(KERN_DEBUG, "", DUMP_PREFIX_OFFSET, 16, 1,
> > pc->c, 6, 1);
> > }
> > rq->rq_disk = scsi->disk;
> > return ide_do_drive_cmd(drive, rq, ide_preempt);
>
> Hi Denis.
>
> These aren't really equivalent. You need to look at the
> line above to determine if a KERN_ prefix needs to be
> used at all.
>
> You should probably use print_hex_dump_bytes here.
I know this is different from the original hexdump in ide-scsi.c, I
just want to tell someone that there's a good implementation of
hexdump in kernel.h, and I think the default KERN_DEBUG and
print_hex_dump is more informative and has better output. However,
anyone want more precise control on debug message could make her/his
improvements with print_hex_dump.
>
> cheers, Joe
>
>
--
Denis Cheng
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2] ide-scsi: use print_hex_dump from linux/kernel.h
On Nov 26, 2007 3:41 PM, Joe Perches [EMAIL PROTECTED] wrote:
On Mon, 2007-11-26 at 15:16 +0800, Denis Cheng wrote:
diff --git a/drivers/scsi/ide-scsi.c b/drivers/scsi/ide-scsi.c
index 8d0244c..8f3fc1d 100644
--- a/drivers/scsi/ide-scsi.c
+++ b/drivers/scsi/ide-scsi.c
@@ -282,7 +272,7 @@ static int idescsi_check_condition(ide_drive_t *drive,
struct request *failed_co
pc-scsi_cmd = ((idescsi_pc_t *) failed_command-special)-scsi_cmd;
if (test_bit(IDESCSI_LOG_CMD, scsi-log)) {
printk (ide-scsi: %s: queue cmd = , drive-name);
- hexdump(pc-c, 6);
+ print_hex_dump(KERN_DEBUG, , DUMP_PREFIX_OFFSET, 16, 1,
pc-c, 6, 1);
}
rq-rq_disk = scsi-disk;
return ide_do_drive_cmd(drive, rq, ide_preempt);
Hi Denis.
These aren't really equivalent. You need to look at the
line above to determine if a KERN_ prefix needs to be
used at all.
You should probably use print_hex_dump_bytes here.
I know this is different from the original hexdump in ide-scsi.c, I
just want to tell someone that there's a good implementation of
hexdump in kernel.h, and I think the default KERN_DEBUG and
print_hex_dump is more informative and has better output. However,
anyone want more precise control on debug message could make her/his
improvements with print_hex_dump.
cheers, Joe
--
Denis Cheng
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] [RESEND] crypto test: use print_hex_dump from kernel.h instead
On Nov 27, 2007 10:58 AM, Richard Knutsson [EMAIL PROTECTED] wrote:
...
+ print_hex_dump(KERN_CONT, , DUMP_PREFIX_OFFSET,
+ 16, 1,
+ buf, len, 0);
Not important, but why use '0' instead of 'false'?
after read http://lkml.org/lkml/2006/7/27/281, I agreed with you.
this is refreshed patch against the lastest cryptodev tree.
Cc: Randy Dunlap [EMAIL PROTECTED]
Signed-off-by: Denis Cheng [EMAIL PROTECTED]
---
crypto/tcrypt.c |9 -
1 files changed, 4 insertions(+), 5 deletions(-)
diff --git a/crypto/tcrypt.c b/crypto/tcrypt.c
index 1e12b86..ae762c2 100644
--- a/crypto/tcrypt.c
+++ b/crypto/tcrypt.c
@@ -87,12 +87,11 @@ static char *check[] = {
camellia, seed, salsa20, NULL
};
-static void hexdump(unsigned char *buf, unsigned int len)
+static inline void hexdump(unsigned char *buf, unsigned int len)
{
- while (len--)
- printk(%02x, *buf++);
-
- printk(\n);
+ print_hex_dump(KERN_CONT, , DUMP_PREFIX_OFFSET,
+ 16, 1,
+ buf, len, false);
}
static void tcrypt_complete(struct crypto_async_request *req, int err)
--
Denis Cheng
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] [sysfs]: make readlink result shorter when the symlink and its target shared some base sysfs subdirectory
On 10/31/07, Greg KH <[EMAIL PROTECTED]> wrote:
> On Wed, Oct 31, 2007 at 06:34:20PM +0800, Denis Cheng wrote:
> > this is especially useful after /sys/slab introduced, for example:
> >
> > $ ls -l /sys/slab/mm_struct
> > lrwxrwxrwx 1 root root 0 2007-10-31 17:40 /sys/slab/mm_struct -> :448
> >
> > instead of:
> >
> > $ ls -l /sys/slab/mm_struct
> > lrwxrwxrwx 1 root root 0 2007-10-31 17:40 /sys/slab/mm_struct ->
> > ../slab/:448
> >
> > Signed-off-by: Denis Cheng <[EMAIL PROTECTED]>
>
> As pretty as this change is, it's not really necessary, right?
I don't think so.
Suppose to create a symlink on the disk, say /usr/src/linux, that
points to /usr/src/linux-2.6.23, the best way is:
# cd /usr/src/
# ln -s linux-2.6.23 linux
# ls -l linux
# ls -l linux
lrwxrwxrwx 1 root root 14 2007-10-16 19:21 linux -> linux-2.6.23
other than:
# cd /usr/src/
# ln -s /usr/src/linux-2.6.23 linux
or
# ln -s ../../usr/src/linux-2.6.23 linux
# ls -l linux
lrwxrwxrwx 1 root root 14 2007-10-16 19:21 linux -> ../../usr/src/linux-2.6.23
Anyone know this, since sysfs is also a filesystem, it should conform
the perfect way.
For another point, consider the code in fs/sysfs/symlink.c:
static int sysfs_get_target_path(struct sysfs_dirent * parent_sd,
struct sysfs_dirent * target_sd, char *path)
{
...
size = object_path_length(target_sd) + depth * 3 - 1;
if (size > PATH_MAX)
return -ENAMETOOLONG;
Since having longer readlink result would consume more memory on the
output parameter path, that is error prone to return -ENAMETOOLONG; we
just need the shorter readlink result.
>
> Is there any other place in /sys that would benefit from this?
Yes. there are already some other symlinks those are also not crossing
top subdirectory of /sys, they would benefit from this patch:
I have found all of them by this little shell:
$ find /sys -type l -printf '%p -> %l -> ' -exec readlink -f '{}' \; | gawk '{
split($1, a, "/");
split($5, b, "/");
if (a[3] == b[3])
print;
}'
that will print many lines like:
...
/sys/block/hdd/subsystem -> ../../block -> /sys/block
/sys/module/snd_mixer_oss/holders/snd_pcm_oss ->
../../../module/snd_pcm_oss -> /sys/module/snd_pcm_oss
/sys/class/sound/audio/subsystem -> ../../../class/sound -> /sys/class/sound
/sys/class/pci_bus/:00/subsystem -> ../../../class/pci_bus ->
/sys/class/pci_bus
...
>
> thanks,
>
> greg k-h
>
--
Denis Cheng
Linux Application Developer
"One of my most productive days was throwing away 1000 lines of code."
- Ken Thompson.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] [sysfs]: make readlink result shorter when the symlink and its target shared some base sysfs subdirectory
On 10/31/07, Greg KH [EMAIL PROTECTED] wrote:
On Wed, Oct 31, 2007 at 06:34:20PM +0800, Denis Cheng wrote:
this is especially useful after /sys/slab introduced, for example:
$ ls -l /sys/slab/mm_struct
lrwxrwxrwx 1 root root 0 2007-10-31 17:40 /sys/slab/mm_struct - :448
instead of:
$ ls -l /sys/slab/mm_struct
lrwxrwxrwx 1 root root 0 2007-10-31 17:40 /sys/slab/mm_struct -
../slab/:448
Signed-off-by: Denis Cheng [EMAIL PROTECTED]
As pretty as this change is, it's not really necessary, right?
I don't think so.
Suppose to create a symlink on the disk, say /usr/src/linux, that
points to /usr/src/linux-2.6.23, the best way is:
# cd /usr/src/
# ln -s linux-2.6.23 linux
# ls -l linux
# ls -l linux
lrwxrwxrwx 1 root root 14 2007-10-16 19:21 linux - linux-2.6.23
other than:
# cd /usr/src/
# ln -s /usr/src/linux-2.6.23 linux
or
# ln -s ../../usr/src/linux-2.6.23 linux
# ls -l linux
lrwxrwxrwx 1 root root 14 2007-10-16 19:21 linux - ../../usr/src/linux-2.6.23
Anyone know this, since sysfs is also a filesystem, it should conform
the perfect way.
For another point, consider the code in fs/sysfs/symlink.c:
static int sysfs_get_target_path(struct sysfs_dirent * parent_sd,
struct sysfs_dirent * target_sd, char *path)
{
...
size = object_path_length(target_sd) + depth * 3 - 1;
if (size PATH_MAX)
return -ENAMETOOLONG;
Since having longer readlink result would consume more memory on the
output parameter path, that is error prone to return -ENAMETOOLONG; we
just need the shorter readlink result.
Is there any other place in /sys that would benefit from this?
Yes. there are already some other symlinks those are also not crossing
top subdirectory of /sys, they would benefit from this patch:
I have found all of them by this little shell:
$ find /sys -type l -printf '%p - %l - ' -exec readlink -f '{}' \; | gawk '{
split($1, a, /);
split($5, b, /);
if (a[3] == b[3])
print;
}'
that will print many lines like:
...
/sys/block/hdd/subsystem - ../../block - /sys/block
/sys/module/snd_mixer_oss/holders/snd_pcm_oss -
../../../module/snd_pcm_oss - /sys/module/snd_pcm_oss
/sys/class/sound/audio/subsystem - ../../../class/sound - /sys/class/sound
/sys/class/pci_bus/:00/subsystem - ../../../class/pci_bus -
/sys/class/pci_bus
...
thanks,
greg k-h
--
Denis Cheng
Linux Application Developer
One of my most productive days was throwing away 1000 lines of code.
- Ken Thompson.
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/5] update boot spec to 2.07
On 10/3/07, Rusty Russell <[EMAIL PROTECTED]> wrote: > Proposed updates for version 2.07 of the boot protocol. This includes: > ... > Signed-off-by: Jeremy Fitzhardinge < [EMAIL PROTECTED]> > Signed-off-by: Rusty Russell <[EMAIL PROTECTED]> > Cc: "Eric W. Biederman" < [EMAIL PROTECTED]> > Cc: H. Peter Anvin <[EMAIL PROTECTED]> > Cc: Vivek Goyal <[EMAIL PROTECTED] > > > --- > Documentation/i386/boot.txt| 34 Sugguestion is you also add an entry to the header of the file (Documentation/i386/boot.txt): something like this: diff --git a/Documentation/i386/boot.txt b/Documentation/i386/boot.txt index fc49b79..645bbd7 100644 --- a/Documentation/i386/boot.txt +++ b/Documentation/i386/boot.txt @@ -42,6 +42,9 @@ Protocol 2.05:(Kernel 2.6.20) Make protected mode kernel relocatable. Protocol 2.06: (Kernel 2.6.22) Added a field that contains the size of the boot command line +Protocol 2.07: (Kernel 2.6.23) Added two fields hardware_subarch and + hardware_subarch_data to describe the paravirtualized + environment. MEMORY LAYOUT -- Denis Cheng Linux Application Developer "One of my most productive days was throwing away 1000 lines of code." - Ken Thompson. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/5] update boot spec to 2.07
On 10/3/07, Rusty Russell [EMAIL PROTECTED] wrote: Proposed updates for version 2.07 of the boot protocol. This includes: ... Signed-off-by: Jeremy Fitzhardinge [EMAIL PROTECTED] Signed-off-by: Rusty Russell [EMAIL PROTECTED] Cc: Eric W. Biederman [EMAIL PROTECTED] Cc: H. Peter Anvin [EMAIL PROTECTED] Cc: Vivek Goyal [EMAIL PROTECTED] --- Documentation/i386/boot.txt| 34 Sugguestion is you also add an entry to the header of the file (Documentation/i386/boot.txt): something like this: diff --git a/Documentation/i386/boot.txt b/Documentation/i386/boot.txt index fc49b79..645bbd7 100644 --- a/Documentation/i386/boot.txt +++ b/Documentation/i386/boot.txt @@ -42,6 +42,9 @@ Protocol 2.05:(Kernel 2.6.20) Make protected mode kernel relocatable. Protocol 2.06: (Kernel 2.6.22) Added a field that contains the size of the boot command line +Protocol 2.07: (Kernel 2.6.23) Added two fields hardware_subarch and + hardware_subarch_data to describe the paravirtualized + environment. MEMORY LAYOUT -- Denis Cheng Linux Application Developer One of my most productive days was throwing away 1000 lines of code. - Ken Thompson. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/3] netlink: the temp variable name max is ambiguous
On 9/17/07, David Miller <[EMAIL PROTECTED]> wrote:
> From: Denis Cheng <[EMAIL PROTECTED]>
> Date: Sun, 2 Sep 2007 03:45:58 +0800
>
> > with the macro max provided by , so changed its name to a
> > more proper one: limit
> >
> > Signed-off-by: Denis Cheng <[EMAIL PROTECTED]>
>
> Not strictly necessary because CPP knows to differentiate between
> 'max(' and plain 'max' when evaluating if a CPP macro should be
> expanded or not.
I also know the GNU CPP is intelligent, but people are often not.
I just think the avoidance to use human ambiguous names could give
more readability.
>
> Nonetheless, applied to net-2.6.24, thanks.
>
--
Denis Cheng
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] net/: all net/ cleanup with ARRAY_SIZE
On 9/17/07, David Miller <[EMAIL PROTECTED]> wrote: > From: Denis Cheng <[EMAIL PROTECTED]> > Date: Sun, 2 Sep 2007 18:30:17 +0800 > > > Signed-off-by: Denis Cheng <[EMAIL PROTECTED]> > > You already submitted the net/ipv4/af_inet.c case > seperately, so I had to remove it from this patch for > it to apply properly. > > Please keep your patches straight to avoid problems > like this. I just can say sorry. But at that time, I'm not sure the former specific patch to net/ipv4/af_inet.c would be applied, and then I realized that change should be done with every subsystem in the kernel source, so I regenerate a new patch for the whole net/ subsystem; In this situation, I think I should give an announcement to make the former patch deprecated, shouldn't it? However, I'll be more cautious with patches. > > Thans. Thanks for applying. > -- Denis Cheng - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/3] netlink: the temp variable name max is ambiguous
On 9/17/07, David Miller [EMAIL PROTECTED] wrote:
From: Denis Cheng [EMAIL PROTECTED]
Date: Sun, 2 Sep 2007 03:45:58 +0800
with the macro max provided by linux/kernel.h, so changed its name to a
more proper one: limit
Signed-off-by: Denis Cheng [EMAIL PROTECTED]
Not strictly necessary because CPP knows to differentiate between
'max(' and plain 'max' when evaluating if a CPP macro should be
expanded or not.
I also know the GNU CPP is intelligent, but people are often not.
I just think the avoidance to use human ambiguous names could give
more readability.
Nonetheless, applied to net-2.6.24, thanks.
--
Denis Cheng
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] net/: all net/ cleanup with ARRAY_SIZE
On 9/17/07, David Miller [EMAIL PROTECTED] wrote: From: Denis Cheng [EMAIL PROTECTED] Date: Sun, 2 Sep 2007 18:30:17 +0800 Signed-off-by: Denis Cheng [EMAIL PROTECTED] You already submitted the net/ipv4/af_inet.c case seperately, so I had to remove it from this patch for it to apply properly. Please keep your patches straight to avoid problems like this. I just can say sorry. But at that time, I'm not sure the former specific patch to net/ipv4/af_inet.c would be applied, and then I realized that change should be done with every subsystem in the kernel source, so I regenerate a new patch for the whole net/ subsystem; In this situation, I think I should give an announcement to make the former patch deprecated, shouldn't it? However, I'll be more cautious with patches. Thans. Thanks for applying. -- Denis Cheng - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] net/ipv4/af_inet.c: use ARRAY_SIZE macro from kernel.h instead
On 9/2/07, Robert P. J. Day <[EMAIL PROTECTED]> wrote:
> On Sun, 2 Sep 2007, Denis Cheng wrote:
>
> > Signed-off-by: Denis Cheng <[EMAIL PROTECTED]>
> > ---
> > net/ipv4/af_inet.c |2 +-
> > 1 files changed, 1 insertions(+), 1 deletions(-)
> >
> > diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
> > index e681034..d5e8b67 100644
> > --- a/net/ipv4/af_inet.c
> > +++ b/net/ipv4/af_inet.c
> > @@ -939,7 +939,7 @@ static struct inet_protosw inetsw_array[] =
> > }
> > };
> >
> > -#define INETSW_ARRAY_LEN (sizeof(inetsw_array) / sizeof(struct
> > inet_protosw))
> > +#define INETSW_ARRAY_LEN ARRAY_SIZE(inetsw_array)
> >
> > void inet_register_protosw(struct inet_protosw *p)
> > {
>
> denis:
>
> if you're planning on doing this ARRAY_SIZE cleanup fairly rigorously,
> here's an overview of what you're looking (based on a fairly dumb
> scanning script that undoubtedly generates some false positives). of
> course, the respective subsystem maintainers are welcome to deal with
> them first, of course.
>
> p.s. and when you submit those patches, it's necessary to submit them
> to only the appropriate subsystem mailing lists, not to the LKML in
> general.
I didn't realize that there's so many places to switch to ARRAY_SIZE,
so now I wonder is this cleaning work valuable to the whole kernel tree?
or we can keep the current state and just encourage new code to use ARRAY_SIZE?
--
Denis
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/3] netlink: use the macro min(x,y) provided by instead
On 9/2/07, David Newall <[EMAIL PROTECTED]> wrote: > Denis Cheng wrote > > + order = get_bitmask_order(min(max, (unsigned long)UINT_MAX)) - 1; > > > > Why doesn't this clash with the max define, also in linux/kernel.h? They indeed don't clash, the cpp included by gcc is intelligent enough, it know the function-style definition of max in kernel.h, that's different from the auto variable max here, so they don't clash with each other, But I think the variable name "max" here is ambiguous, I changed it to "limit", see my following patch [PATCH 2/3]. -- Denis Cheng - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/3] netlink: use the macro min(x,y) provided by linux/kernel.h instead
On 9/2/07, David Newall [EMAIL PROTECTED] wrote: Denis Cheng wrote + order = get_bitmask_order(min(max, (unsigned long)UINT_MAX)) - 1; Why doesn't this clash with the max define, also in linux/kernel.h? They indeed don't clash, the cpp included by gcc is intelligent enough, it know the function-style definition of max in kernel.h, that's different from the auto variable max here, so they don't clash with each other, But I think the variable name max here is ambiguous, I changed it to limit, see my following patch [PATCH 2/3]. -- Denis Cheng - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [Cluster-devel] Re: [gfs2][RFC] readdir caused ls process into D (uninterruptible) state, under testing with Samba 3.0.25
On 8/17/07, Steven Whitehouse <[EMAIL PROTECTED]> wrote: ... > > the stack trace of the 'D' state `ls`: > > > > === > > lsD F89B83F8 2200 12018 1 (NOTLB) > >f3eeadd4 0082 f6a425c0 f89b83f8 f3eead9c f6a425d4 f6f32d80 > > f573a93c > >0001 f89b83f3 c40a2030 c3fa9fa0 c40aaa70 c40aab7c > > 0e89 > >b2a4b036 02e4 c40a2030 f3eeae1c c3f85e98 f8e11e09 > > f8e11e0e > > Call Trace: > > [] gdlm_bast+0x0/0x93 [lock_dlm] > > [] gdlm_ast+0x0/0x5 [lock_dlm] > > [] holder_wait+0x0/0x8 [gfs2] > > [] holder_wait+0x5/0x8 [gfs2] > This function doesn't exist in recent kernels, so I > guess you are using an older kernel. Which version is it? Sorry for the late, The kernel I'm testing is 2.6.21.7, just because our testing cluster suite is from the last month when cluster-2.01 from here didn't come out, ftp://sources.redhat.com/pub/cluster/releases/ So now we were keeping testing on kernel 2.6.21.y series, just for its stability, I don't know how about the stability of 2.6.22.y, I haven't tested it yet. So the problem I said has been fixed in later kernel after 2.6.22, please feel free to let me know. > > > [] __wait_on_bit+0x2c/0x51 > > [] out_of_line_wait_on_bit+0x6f/0x77 > > [] holder_wait+0x0/0x8 [gfs2] > > [] wake_bit_function+0x0/0x3c > > [] wake_bit_function+0x0/0x3c > > [] wait_on_holder+0x3c/0x40 [gfs2] > > [] glock_wait_internal+0x81/0x1a3 [gfs2] > > [] gfs2_glock_nq+0x5e/0x79 [gfs2] > > [] gfs2_getattr+0x72/0xb5 [gfs2] > > [] gfs2_getattr+0x6b/0xb5 [gfs2] > > [] do_path_lookup+0x17a/0x1c3 > > [] gfs2_getattr+0x0/0xb5 [gfs2] > > [] vfs_getattr+0x3e/0x51 > > [] vfs_lstat_fd+0x2b/0x3d > > [] do_path_lookup+0x17a/0x1c3 > > [] mntput_no_expire+0x11/0x6e > > [] sys_lstat64+0xf/0x23 > > [] sys_symlinkat+0x81/0xb5 > > [] sysenter_past_esp+0x5d/0x81 > > [] __ipv6_addr_type+0x88/0xb8 > > > > the system is still running, so the mormal 'R' and 'S' state process > > are ignored, But it turns out that it's not the readdir's fault from > > this call trace, but gdlm_bast's problem in lock_dlm module. > > > Yes, it does look a bit odd. There was a bug fix (which has only very > recently made it into Linus' kernel as of the last GFS2 pull a few days > ago) which fixes a problem in the DLM, although this doesn't look like > that, at least at first sight. > > The other thing which you can check is the glock state which you can > find in /sys/kernel/debug/gfs2//glocks on each node. The list is > usually quite large, so its best to just email a url where it can be > found. That will tell you which processes own which locks and thus what > is holding the lock which is causing the problem. Likewise there is also > a debugfs file which contains the locks from the DLM's point of view > too. I'll try it. Thanks. > > Steve. > > > -- Denis Cheng - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [Cluster-devel] Re: [gfs2][RFC] readdir caused ls process into D (uninterruptible) state, under testing with Samba 3.0.25
On 8/17/07, Steven Whitehouse [EMAIL PROTECTED] wrote: ... the stack trace of the 'D' state `ls`: === lsD F89B83F8 2200 12018 1 (NOTLB) f3eeadd4 0082 f6a425c0 f89b83f8 f3eead9c f6a425d4 f6f32d80 f573a93c 0001 f89b83f3 c40a2030 c3fa9fa0 c40aaa70 c40aab7c 0e89 b2a4b036 02e4 c40a2030 f3eeae1c c3f85e98 f8e11e09 f8e11e0e Call Trace: [f89b83f8] gdlm_bast+0x0/0x93 [lock_dlm] [f89b83f3] gdlm_ast+0x0/0x5 [lock_dlm] [f8e11e09] holder_wait+0x0/0x8 [gfs2] [f8e11e0e] holder_wait+0x5/0x8 [gfs2] This function doesn't exist in recent kernels, so I guess you are using an older kernel. Which version is it? Sorry for the late, The kernel I'm testing is 2.6.21.7, just because our testing cluster suite is from the last month when cluster-2.01 from here didn't come out, ftp://sources.redhat.com/pub/cluster/releases/ So now we were keeping testing on kernel 2.6.21.y series, just for its stability, I don't know how about the stability of 2.6.22.y, I haven't tested it yet. So the problem I said has been fixed in later kernel after 2.6.22, please feel free to let me know. [c0303adf] __wait_on_bit+0x2c/0x51 [c0303b73] out_of_line_wait_on_bit+0x6f/0x77 [f8e11e09] holder_wait+0x0/0x8 [gfs2] [c012dd7d] wake_bit_function+0x0/0x3c [c012dd7d] wake_bit_function+0x0/0x3c [f8e11e4d] wait_on_holder+0x3c/0x40 [gfs2] [f8e12a9a] glock_wait_internal+0x81/0x1a3 [gfs2] [f8e12d64] gfs2_glock_nq+0x5e/0x79 [gfs2] [f8e1fc02] gfs2_getattr+0x72/0xb5 [gfs2] [f8e1fbfb] gfs2_getattr+0x6b/0xb5 [gfs2] [c0166946] do_path_lookup+0x17a/0x1c3 [f8e1fb90] gfs2_getattr+0x0/0xb5 [gfs2] [c0161f92] vfs_getattr+0x3e/0x51 [c016201e] vfs_lstat_fd+0x2b/0x3d [c0166946] do_path_lookup+0x17a/0x1c3 [c0171e40] mntput_no_expire+0x11/0x6e [c016260b] sys_lstat64+0xf/0x23 [c01681a0] sys_symlinkat+0x81/0xb5 [c01030b8] sysenter_past_esp+0x5d/0x81 [c030] __ipv6_addr_type+0x88/0xb8 the system is still running, so the mormal 'R' and 'S' state process are ignored, But it turns out that it's not the readdir's fault from this call trace, but gdlm_bast's problem in lock_dlm module. Yes, it does look a bit odd. There was a bug fix (which has only very recently made it into Linus' kernel as of the last GFS2 pull a few days ago) which fixes a problem in the DLM, although this doesn't look like that, at least at first sight. The other thing which you can check is the glock state which you can find in /sys/kernel/debug/gfs2/fsname/glocks on each node. The list is usually quite large, so its best to just email a url where it can be found. That will tell you which processes own which locks and thus what is holding the lock which is causing the problem. Likewise there is also a debugfs file which contains the locks from the DLM's point of view too. I'll try it. Thanks. Steve. -- Denis Cheng - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [Cluster-devel] Re: [gfs2][RFC] readdir caused ls process into D (uninterruptible) state, under testing with Samba 3.0.25
On 8/16/07, Steven Whitehouse <[EMAIL PROTECTED]> wrote: > Hi, > > On Thu, 2007-08-16 at 16:20 +0800, 程任全 wrote: > > It seems that gfs2 cannot work well with Samba, > > > > I'm using the gfs2 and the new cluster suite(cman with openais), > > > > 1. the testing environment is that 1 iscsi target and 2 cluster node, > > 2. the two nodes both used iscsi initiator connect to the target, > > 3. they're using the same physical iscsi disk, > > 4. run LVM2 on top of the same iscsi disk, > > 5. on the same lv (logical volume), I created a gfs2 filesystem, > > 6. mount the gfs2 system to a same path under 2 nodes, > > 7. start samba to shared the gfs2 mounting pointer on the 2 nodes, > > > > now test with windows client, when two or above clients connects to the > > samba, > > everything is still normal; but when heavy writers or readers start, > > the samba server daemon changed to D state, that's uninterruptible in > > the kernel, > > I wonder that's a problem of gfs2? > > > Which version of gfs2 are you using? GFS2 doesn't support leases which I > know that Samba uses, however only relatively recent kernels have been > able to report that fact via the VFS. > > > then I start a simple ls command on the gfs2 mouting point: > > $ ls /mnt/gfs2 > > the ls process is also changed to D state, > > > > I think it's problems about readdir implementation in gfs2, and I want > > to fix it, someone could give me some pointers? > > > Can you get a stack trace? echo 't' >/proc/sysrq-trigger > That should show where Samba is getting stuck, > > Steve. the stack trace of the 'D' state `ls`: === lsD F89B83F8 2200 12018 1 (NOTLB) f3eeadd4 0082 f6a425c0 f89b83f8 f3eead9c f6a425d4 f6f32d80 f573a93c 0001 f89b83f3 c40a2030 c3fa9fa0 c40aaa70 c40aab7c 0e89 b2a4b036 02e4 c40a2030 f3eeae1c c3f85e98 f8e11e09 f8e11e0e Call Trace: [] gdlm_bast+0x0/0x93 [lock_dlm] [] gdlm_ast+0x0/0x5 [lock_dlm] [] holder_wait+0x0/0x8 [gfs2] [] holder_wait+0x5/0x8 [gfs2] [] __wait_on_bit+0x2c/0x51 [] out_of_line_wait_on_bit+0x6f/0x77 [] holder_wait+0x0/0x8 [gfs2] [] wake_bit_function+0x0/0x3c [] wake_bit_function+0x0/0x3c [] wait_on_holder+0x3c/0x40 [gfs2] [] glock_wait_internal+0x81/0x1a3 [gfs2] [] gfs2_glock_nq+0x5e/0x79 [gfs2] [] gfs2_getattr+0x72/0xb5 [gfs2] [] gfs2_getattr+0x6b/0xb5 [gfs2] [] do_path_lookup+0x17a/0x1c3 [] gfs2_getattr+0x0/0xb5 [gfs2] [] vfs_getattr+0x3e/0x51 [] vfs_lstat_fd+0x2b/0x3d [] do_path_lookup+0x17a/0x1c3 [] mntput_no_expire+0x11/0x6e [] sys_lstat64+0xf/0x23 [] sys_symlinkat+0x81/0xb5 [] sysenter_past_esp+0x5d/0x81 [] __ipv6_addr_type+0x88/0xb8 the system is still running, so the mormal 'R' and 'S' state process are ignored, But it turns out that it's not the readdir's fault from this call trace, but gdlm_bast's problem in lock_dlm module. > > > -- Denis Cheng - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [Cluster-devel] Re: [gfs2][RFC] readdir caused ls process into D (uninterruptible) state, under testing with Samba 3.0.25
On 8/16/07, Steven Whitehouse [EMAIL PROTECTED] wrote: Hi, On Thu, 2007-08-16 at 16:20 +0800, 程任全 wrote: It seems that gfs2 cannot work well with Samba, I'm using the gfs2 and the new cluster suite(cman with openais), 1. the testing environment is that 1 iscsi target and 2 cluster node, 2. the two nodes both used iscsi initiator connect to the target, 3. they're using the same physical iscsi disk, 4. run LVM2 on top of the same iscsi disk, 5. on the same lv (logical volume), I created a gfs2 filesystem, 6. mount the gfs2 system to a same path under 2 nodes, 7. start samba to shared the gfs2 mounting pointer on the 2 nodes, now test with windows client, when two or above clients connects to the samba, everything is still normal; but when heavy writers or readers start, the samba server daemon changed to D state, that's uninterruptible in the kernel, I wonder that's a problem of gfs2? Which version of gfs2 are you using? GFS2 doesn't support leases which I know that Samba uses, however only relatively recent kernels have been able to report that fact via the VFS. then I start a simple ls command on the gfs2 mouting point: $ ls /mnt/gfs2 the ls process is also changed to D state, I think it's problems about readdir implementation in gfs2, and I want to fix it, someone could give me some pointers? Can you get a stack trace? echo 't' /proc/sysrq-trigger That should show where Samba is getting stuck, Steve. the stack trace of the 'D' state `ls`: === lsD F89B83F8 2200 12018 1 (NOTLB) f3eeadd4 0082 f6a425c0 f89b83f8 f3eead9c f6a425d4 f6f32d80 f573a93c 0001 f89b83f3 c40a2030 c3fa9fa0 c40aaa70 c40aab7c 0e89 b2a4b036 02e4 c40a2030 f3eeae1c c3f85e98 f8e11e09 f8e11e0e Call Trace: [f89b83f8] gdlm_bast+0x0/0x93 [lock_dlm] [f89b83f3] gdlm_ast+0x0/0x5 [lock_dlm] [f8e11e09] holder_wait+0x0/0x8 [gfs2] [f8e11e0e] holder_wait+0x5/0x8 [gfs2] [c0303adf] __wait_on_bit+0x2c/0x51 [c0303b73] out_of_line_wait_on_bit+0x6f/0x77 [f8e11e09] holder_wait+0x0/0x8 [gfs2] [c012dd7d] wake_bit_function+0x0/0x3c [c012dd7d] wake_bit_function+0x0/0x3c [f8e11e4d] wait_on_holder+0x3c/0x40 [gfs2] [f8e12a9a] glock_wait_internal+0x81/0x1a3 [gfs2] [f8e12d64] gfs2_glock_nq+0x5e/0x79 [gfs2] [f8e1fc02] gfs2_getattr+0x72/0xb5 [gfs2] [f8e1fbfb] gfs2_getattr+0x6b/0xb5 [gfs2] [c0166946] do_path_lookup+0x17a/0x1c3 [f8e1fb90] gfs2_getattr+0x0/0xb5 [gfs2] [c0161f92] vfs_getattr+0x3e/0x51 [c016201e] vfs_lstat_fd+0x2b/0x3d [c0166946] do_path_lookup+0x17a/0x1c3 [c0171e40] mntput_no_expire+0x11/0x6e [c016260b] sys_lstat64+0xf/0x23 [c01681a0] sys_symlinkat+0x81/0xb5 [c01030b8] sysenter_past_esp+0x5d/0x81 [c030] __ipv6_addr_type+0x88/0xb8 the system is still running, so the mormal 'R' and 'S' state process are ignored, But it turns out that it's not the readdir's fault from this call trace, but gdlm_bast's problem in lock_dlm module. -- Denis Cheng - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] gfs2: better code for translating characters
On 8/13/07, Denis Cheng <[EMAIL PROTECTED]> wrote: > the original code could work, but I think this code could work better. > > Signed-off-by: Denis Cheng <[EMAIL PROTECTED]> > --- > fs/gfs2/ops_fstype.c |3 ++- > 1 files changed, 2 insertions(+), 1 deletions(-) > > diff --git a/fs/gfs2/ops_fstype.c b/fs/gfs2/ops_fstype.c > index cf5aa50..b9a7759 100644 > --- a/fs/gfs2/ops_fstype.c > +++ b/fs/gfs2/ops_fstype.c > @@ -145,7 +145,8 @@ static int init_names(struct gfs2_sbd *sdp, int silent) > snprintf(sdp->sd_proto_name, GFS2_FSNAME_LEN, "%s", proto); > snprintf(sdp->sd_table_name, GFS2_FSNAME_LEN, "%s", table); > > - while ((table = strchr(sdp->sd_table_name, '/'))) > + table = sdp->sd_table_name; > + while ((table = strchr(table, '/'))) > *table = '_'; Sorry, I don't know what the while loop really means, what's the common case that slash character exists? if the '/' appears multiple, the latter code would be better; however, if slash appears rarely, the original would still be better. > > out: > -- > 1.5.2.2 > > -- Denis Cheng Linux Application Developer "One of my most productive days was throwing away 1000 lines of code." - Ken Thompson. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] gfs2: better code for translating characters
On 8/13/07, Denis Cheng [EMAIL PROTECTED] wrote: the original code could work, but I think this code could work better. Signed-off-by: Denis Cheng [EMAIL PROTECTED] --- fs/gfs2/ops_fstype.c |3 ++- 1 files changed, 2 insertions(+), 1 deletions(-) diff --git a/fs/gfs2/ops_fstype.c b/fs/gfs2/ops_fstype.c index cf5aa50..b9a7759 100644 --- a/fs/gfs2/ops_fstype.c +++ b/fs/gfs2/ops_fstype.c @@ -145,7 +145,8 @@ static int init_names(struct gfs2_sbd *sdp, int silent) snprintf(sdp-sd_proto_name, GFS2_FSNAME_LEN, %s, proto); snprintf(sdp-sd_table_name, GFS2_FSNAME_LEN, %s, table); - while ((table = strchr(sdp-sd_table_name, '/'))) + table = sdp-sd_table_name; + while ((table = strchr(table, '/'))) *table = '_'; Sorry, I don't know what the while loop really means, what's the common case that slash character exists? if the '/' appears multiple, the latter code would be better; however, if slash appears rarely, the original would still be better. out: -- 1.5.2.2 -- Denis Cheng Linux Application Developer One of my most productive days was throwing away 1000 lines of code. - Ken Thompson. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] net/sched: mark "NET_CLS_RSVP6 depends on IPV6"
On 8/1/07, Gabriel C <[EMAIL PROTECTED]> wrote: > I thought the same but ... see > > http://marc.info/?l=linux-kernel=118440958516205=2 > http://marc.info/?l=linux-netdev=118442747709230=2 That sounds good. Thanks. -- Denis - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] fs/gfs2: mark struct *_operations const
On 7/31/07, Steven Whitehouse <[EMAIL PROTECTED]> wrote: > Hi, > > On Tue, 2007-07-31 at 13:46 +0800, Denis Cheng wrote: > > these struct *_operations are all method tables, thus should be const. > > > > Signed-off-by: Denis Cheng <[EMAIL PROTECTED]> > > --- > > fs/gfs2/eaops.c|8 > > fs/gfs2/eaops.h|4 ++-- > > fs/gfs2/glock.c|2 +- > > fs/gfs2/ops_dentry.c |3 +-- > > fs/gfs2/ops_dentry.h |2 +- > > fs/gfs2/ops_vm.c |4 ++-- > > fs/gfs2/ops_vm.h |4 ++-- > > include/linux/dcache.h |2 +- > > include/linux/mm.h |2 +- > > 9 files changed, 15 insertions(+), 16 deletions(-) > > > > In general this looks good, however where you have made changes in the > two include files dcache.h and mm.h be aware that other filesystems also > use these and I suspect there are more places to change than just gfs2. > Can you do a test build with all filesystems enabled to ensure that > you've got all the places which can then be marked const? OCFS2, to take > one example, has a vm_operations_struct which would need to be updated > on that basis at least. > > In fact if you can break this up into a patch which affects only gfs2 > (which I can apply right away) and a patch which affects the core, plus > updates for the various filesytems that would probably make things > easier from the merging point of view. Since the latter would affect > multiple filesystems, it would make sense to push it through -mm rather > than for me to put it in my git tree, > > Steve. > I think so. But I've tested it under x86(pentium4) with allyesconfig and allmodconfig, the compilication process of the kernel looked very quiet. $ make mrproper $ make allyesconfig $ make fs/ $ make mrproper $ make allmodconfig $ make fs/ and the ocfs2 didn't complain. However, I'll split it into two patches and resend them later. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] fs/gfs2: mark struct *_operations const
On 7/31/07, Steven Whitehouse [EMAIL PROTECTED] wrote: Hi, On Tue, 2007-07-31 at 13:46 +0800, Denis Cheng wrote: these struct *_operations are all method tables, thus should be const. Signed-off-by: Denis Cheng [EMAIL PROTECTED] --- fs/gfs2/eaops.c|8 fs/gfs2/eaops.h|4 ++-- fs/gfs2/glock.c|2 +- fs/gfs2/ops_dentry.c |3 +-- fs/gfs2/ops_dentry.h |2 +- fs/gfs2/ops_vm.c |4 ++-- fs/gfs2/ops_vm.h |4 ++-- include/linux/dcache.h |2 +- include/linux/mm.h |2 +- 9 files changed, 15 insertions(+), 16 deletions(-) In general this looks good, however where you have made changes in the two include files dcache.h and mm.h be aware that other filesystems also use these and I suspect there are more places to change than just gfs2. Can you do a test build with all filesystems enabled to ensure that you've got all the places which can then be marked const? OCFS2, to take one example, has a vm_operations_struct which would need to be updated on that basis at least. In fact if you can break this up into a patch which affects only gfs2 (which I can apply right away) and a patch which affects the core, plus updates for the various filesytems that would probably make things easier from the merging point of view. Since the latter would affect multiple filesystems, it would make sense to push it through -mm rather than for me to put it in my git tree, Steve. I think so. But I've tested it under x86(pentium4) with allyesconfig and allmodconfig, the compilication process of the kernel looked very quiet. $ make mrproper $ make allyesconfig $ make fs/ $ make mrproper $ make allmodconfig $ make fs/ and the ocfs2 didn't complain. However, I'll split it into two patches and resend them later. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] net/sched: mark NET_CLS_RSVP6 depends on IPV6
On 8/1/07, Gabriel C [EMAIL PROTECTED] wrote: I thought the same but ... see http://marc.info/?l=linux-kernelm=118440958516205w=2 http://marc.info/?l=linux-netdevm=118442747709230w=2 That sounds good. Thanks. -- Denis - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC] fs/super.c: Why alloc_super use a static variable default_op?
On 7/25/07, Al Viro <[EMAIL PROTECTED]> wrote:
On Wed, Jul 25, 2007 at 12:29:17PM +0800, rae l wrote:
> But is it valuable? Compared to a waste of sizeof(struct super_block)
> bytes memory.
It's less that struct super_block, actually.
> When some code want to refer fs_type->s_op, it almost always want to
> refer some function pointer in s_op with fs_type->s_op->***, but all
> pointers in default_op are all NULLs, what about this scenario?
Yes, and? You still need one test instead of two. Which gets you
more than 21 words used by that sucker, only in .text instead of .bss.
> and if you do grep s_op in the source code, you will found nowhere
> will want to test s_op or dependent on s_op not NULL.
What? fs/inode.c:
if (sb->s_op->alloc_inode)
inode = sb->s_op->alloc_inode(sb);
else
inode = (struct inode *) kmem_cache_alloc(inode_cachep,
GFP_KERNEL);
and the same goes everywhere else. Of course we don't check for
sb->s_op not being NULL - that's exactly why we are safe skipping such
tests.
Oh, Thank you.
But there are also many other subsystems will do
fs/dcache.c:
void dput(struct dentry *dentry)
if (dentry->d_op && dentry->d_op->d_delete) {
Do you think it's worth optimizing it with a static d_op filled?
we can add a static variable to d_alloc and set its initial d_op to
this static variable?
struct dentry *d_alloc(struct dentry * parent, const struct qstr *name)
--
Denis Cheng
Linux Application Developer
"One of my most productive days was throwing away 1000 lines of code."
- Ken Thompson.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC] fs/super.c: Why alloc_super use a static variable default_op?
On 7/25/07, Al Viro <[EMAIL PROTECTED]> wrote: On Wed, Jul 25, 2007 at 11:48:35AM +0800, rae l wrote: > Why alloc_super use a static variable default_op? > the static struct super_operations default_op is just all zeros, and > just referenced as the initial value of a new allocated super_block, > what does it for? So that we would not have to care about ->s_op *ever* being NULL. But is it valuable? Compared to a waste of sizeof(struct super_block) bytes memory. When some code want to refer fs_type->s_op, it almost always want to refer some function pointer in s_op with fs_type->s_op->***, but all pointers in default_op are all NULLs, what about this scenario? and if you do grep s_op in the source code, you will found nowhere will want to test s_op or dependent on s_op not NULL. So my opinion is to remove default_ops, just keep new allocated s_op NULL. -- Denis Cheng Linux Application Developer "One of my most productive days was throwing away 1000 lines of code." - Ken Thompson. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[RFC] fs/super.c: Why alloc_super use a static variable default_op?
Why alloc_super use a static variable default_op?
the static struct super_operations default_op is just all zeros, and
just referenced as the initial value of a new allocated super_block,
what does it for?
the filesystem dependent code such as ext2_fill_super would fill this
field eventually,
and after carefully checked, it seems no one filesystem would need a
all zero default_op,
as the command output in the kernel source tree:
$ grep -RInw s_op fs/
You could check all the use of s_op.
/**
* alloc_super - create new superblock
* @type: filesystem type superblock should belong to
*
* Allocates and initializes a new super_block. alloc_super()
* returns a pointer new superblock or %NULL if allocation had failed.
*/
static struct super_block *alloc_super(struct file_system_type *type)
{
struct super_block *s = kzalloc(sizeof(struct super_block), GFP_USER);
static struct super_operations default_op;
if (s) {
...
s->s_op = _op;
s->s_time_gran = 10;
}
out:
return s;
}
--
Denis Cheng
Linux Application Developer
"One of my most productive days was throwing away 1000 lines of code."
- Ken Thompson.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[RFC] fs/super.c: Why alloc_super use a static variable default_op?
Why alloc_super use a static variable default_op?
the static struct super_operations default_op is just all zeros, and
just referenced as the initial value of a new allocated super_block,
what does it for?
the filesystem dependent code such as ext2_fill_super would fill this
field eventually,
and after carefully checked, it seems no one filesystem would need a
all zero default_op,
as the command output in the kernel source tree:
$ grep -RInw s_op fs/
You could check all the use of s_op.
/**
* alloc_super - create new superblock
* @type: filesystem type superblock should belong to
*
* Allocates and initializes a new struct super_block. alloc_super()
* returns a pointer new superblock or %NULL if allocation had failed.
*/
static struct super_block *alloc_super(struct file_system_type *type)
{
struct super_block *s = kzalloc(sizeof(struct super_block), GFP_USER);
static struct super_operations default_op;
if (s) {
...
s-s_op = default_op;
s-s_time_gran = 10;
}
out:
return s;
}
--
Denis Cheng
Linux Application Developer
One of my most productive days was throwing away 1000 lines of code.
- Ken Thompson.
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC] fs/super.c: Why alloc_super use a static variable default_op?
On 7/25/07, Al Viro [EMAIL PROTECTED] wrote: On Wed, Jul 25, 2007 at 11:48:35AM +0800, rae l wrote: Why alloc_super use a static variable default_op? the static struct super_operations default_op is just all zeros, and just referenced as the initial value of a new allocated super_block, what does it for? So that we would not have to care about -s_op *ever* being NULL. But is it valuable? Compared to a waste of sizeof(struct super_block) bytes memory. When some code want to refer fs_type-s_op, it almost always want to refer some function pointer in s_op with fs_type-s_op-***, but all pointers in default_op are all NULLs, what about this scenario? and if you do grep s_op in the source code, you will found nowhere will want to test s_op or dependent on s_op not NULL. So my opinion is to remove default_ops, just keep new allocated s_op NULL. -- Denis Cheng Linux Application Developer One of my most productive days was throwing away 1000 lines of code. - Ken Thompson. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC] fs/super.c: Why alloc_super use a static variable default_op?
On 7/25/07, Al Viro [EMAIL PROTECTED] wrote:
On Wed, Jul 25, 2007 at 12:29:17PM +0800, rae l wrote:
But is it valuable? Compared to a waste of sizeof(struct super_block)
bytes memory.
It's less that struct super_block, actually.
When some code want to refer fs_type-s_op, it almost always want to
refer some function pointer in s_op with fs_type-s_op-***, but all
pointers in default_op are all NULLs, what about this scenario?
Yes, and? You still need one test instead of two. Which gets you
more than 21 words used by that sucker, only in .text instead of .bss.
and if you do grep s_op in the source code, you will found nowhere
will want to test s_op or dependent on s_op not NULL.
What? fs/inode.c:
if (sb-s_op-alloc_inode)
inode = sb-s_op-alloc_inode(sb);
else
inode = (struct inode *) kmem_cache_alloc(inode_cachep,
GFP_KERNEL);
and the same goes everywhere else. Of course we don't check for
sb-s_op not being NULL - that's exactly why we are safe skipping such
tests.
Oh, Thank you.
But there are also many other subsystems will do
fs/dcache.c:
void dput(struct dentry *dentry)
if (dentry-d_op dentry-d_op-d_delete) {
Do you think it's worth optimizing it with a static d_op filled?
we can add a static variable to d_alloc and set its initial d_op to
this static variable?
struct dentry *d_alloc(struct dentry * parent, const struct qstr *name)
--
Denis Cheng
Linux Application Developer
One of my most productive days was throwing away 1000 lines of code.
- Ken Thompson.
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] run scripts/Lindent on it to match Documentation/CodingStyle
On 7/21/07, Simon Arlott <[EMAIL PROTECTED]> wrote: Changing the code to fix a utility bug is madness. I think it's been fixed too... Now I also think it's the utility's bug, that hardly do nothing on indent the source. -- Denis Cheng Linux Application Developer "One of my most productive days was throwing away 1000 lines of code." - Ken Thompson. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] run scripts/Lindent on it to match Documentation/CodingStyle
On 7/21/07, Simon Arlott [EMAIL PROTECTED] wrote: Changing the code to fix a utility bug is madness. I think it's been fixed too... Now I also think it's the utility's bug, that hardly do nothing on indent the source. -- Denis Cheng Linux Application Developer One of my most productive days was throwing away 1000 lines of code. - Ken Thompson. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] fs/fuse/dev.c: use zero_user_page instead
On 7/20/07, Dave Young <[EMAIL PROTECTED]> wrote:
> - if (page && zeroing && count < PAGE_SIZE) {
> - void *mapaddr = kmap_atomic(page, KM_USER1);
> - memset(mapaddr, 0, PAGE_SIZE);
> - kunmap_atomic(mapaddr, KM_USER1);
> - }
> + if (page && zeroing && count < PAGE_SIZE)
> + zero_user_page(page, 0, PAGE_SIZE, KM_USER1);
Why clear all page? IMHO,only count bytes need to be cleared.
The original one is memset(mapaddr, 0, PAGE_SIZE), namely clear the whole page.
--
Denis Cheng
Linux Application Developer
"One of my most productive days was throwing away 1000 lines of code."
- Ken Thompson.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] fs/fuse/dev.c: use zero_user_page instead
On 7/20/07, Dave Young [EMAIL PROTECTED] wrote:
- if (page zeroing count PAGE_SIZE) {
- void *mapaddr = kmap_atomic(page, KM_USER1);
- memset(mapaddr, 0, PAGE_SIZE);
- kunmap_atomic(mapaddr, KM_USER1);
- }
+ if (page zeroing count PAGE_SIZE)
+ zero_user_page(page, 0, PAGE_SIZE, KM_USER1);
Why clear all page? IMHO,only count bytes need to be cleared.
The original one is memset(mapaddr, 0, PAGE_SIZE), namely clear the whole page.
--
Denis Cheng
Linux Application Developer
One of my most productive days was throwing away 1000 lines of code.
- Ken Thompson.
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] net/core: merge the content of dev_mcast.c into dev.c
On 7/20/07, David Miller <[EMAIL PROTECTED]> wrote: Please don't quote a big huge patch just to say one sentence that doesn't apply to any particular specific part of a patch. That's wastes bandwidth, annoys people you might actually want a response from, and is bad netiquette in general. Thanks. -- Denis Cheng Linux Application Developer "One of my most productive days was throwing away 1000 lines of code." - Ken Thompson. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] net/core: merge the content of dev_mcast.c into dev.c
On 7/18/07, Denis Cheng <[EMAIL PROTECTED]> wrote:
- removed three function declarations from header file to mark them static,
- reduced one file
Signed-off-by: Denis Cheng <[EMAIL PROTECTED]>
---
this one is just merging by concatenating, and I'll try to adjust some
function definitions' order to make it more readable.
include/linux/netdevice.h |3 -
net/core/Makefile |2 +-
net/core/dev.c| 239 +-
net/core/dev_mcast.c | 255 -
4 files changed, 237 insertions(+), 262 deletions(-)
delete mode 100644 net/core/dev_mcast.c
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 9820ca1..ca68c58 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -1091,15 +1091,12 @@ extern int register_netdev(struct
net_device *dev);
extern voidunregister_netdev(struct net_device *dev);
/* Functions used for secondary unicast and multicast support */
extern voiddev_set_rx_mode(struct net_device *dev);
-extern void__dev_set_rx_mode(struct net_device *dev);
extern int dev_unicast_delete(struct net_device *dev, void *addr,
int alen);
extern int dev_unicast_add(struct net_device *dev, void *addr, int
alen);
extern int dev_mc_delete(struct net_device *dev, void *addr, int
alen, int all);
extern int dev_mc_add(struct net_device *dev, void *addr, int
alen, int newonly);
extern int dev_mc_sync(struct net_device *to, struct net_device
*from);
extern voiddev_mc_unsync(struct net_device *to, struct net_device
*from);
-extern int __dev_addr_delete(struct dev_addr_list **list, int
*count, void *addr, int alen, int all);
-extern int __dev_addr_add(struct dev_addr_list **list, int *count,
void *addr, int alen, int newonly);
extern voiddev_set_promiscuity(struct net_device *dev, int inc);
extern voiddev_set_allmulti(struct net_device *dev, int inc);
extern voidnetdev_state_change(struct net_device *dev);
diff --git a/net/core/Makefile b/net/core/Makefile
index 4751613..54d28dd 100644
--- a/net/core/Makefile
+++ b/net/core/Makefile
@@ -7,7 +7,7 @@ obj-y := sock.o request_sock.o skbuff.o iovec.o datagram.o
stream.o scm.o \
obj-$(CONFIG_SYSCTL) += sysctl_net_core.o
-obj-y += dev.o ethtool.o dev_mcast.o dst.o netevent.o \
+obj-y += dev.o ethtool.o dst.o netevent.o \
neighbour.o rtnetlink.o utils.o link_watch.o filter.o
obj-$(CONFIG_XFRM) += flow.o
diff --git a/net/core/dev.c b/net/core/dev.c
index 6357f54..16842af 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -18,6 +18,7 @@
* Alexey Kuznetsov <[EMAIL PROTECTED]>
* Adam Sulmicki <[EMAIL PROTECTED]>
* Pekka Riikonen <[EMAIL PROTECTED]>
+ * Denis Cheng <[EMAIL PROTECTED]>
*
* Changes:
* D.J. Barrow : Fixed bug where dev->refcnt gets set
@@ -70,6 +71,32 @@
* indefinitely on dev->refcnt
* J Hadi Salim: - Backlog queue sampling
* - netif_rx() feedback
+ * Denis Cheng : Merge dev_mcast.c into it
+ */
+
+/*
+ * The original information in dev_mcast.c:
+ *
+ * Linux NET3: Multicast List maintenance.
+ *
+ * Authors:
+ * Tim Kordas <[EMAIL PROTECTED]>
+ * Richard Underwood <[EMAIL PROTECTED]>
+ *
+ * Stir fried together from the IP multicast and CAP patches above
+ * Alan Cox <[EMAIL PROTECTED]>
+ *
+ * Fixes:
+ * Alan Cox: Update the device on a real delete
+ * rather than any time but...
+ * Alan Cox: IFF_ALLMULTI support.
+ * Alan Cox: New format set_multicast_list() calls.
+ * Gleb Natapov: Remove dev_mc_lock.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version
+ * 2 of the License, or (at your option) any later version.
*/
#include
@@ -2622,7 +2649,7 @@ void dev_set_allmulti(struct net_device *dev, int inc)
* filtering it is put in promiscous mode while unicast addresses
* are present.
*/
-void __dev_set_rx_mode(struct net_device *dev)
+static void __dev_set_rx_mode(struct net_device *dev)
{
/* dev_open will call this function so the list will stay sane. */
if (!(dev->flags_UP))
@@ -2657,7 +2684,7 @@ void dev_set_rx_mode(struct net_device *dev)
netif_tx_unlock_bh(dev);
}
-int __dev_addr_delete(struct dev_addr_list **list, int *count,
+static int __dev_addr_delete(struct
Re: [PATCH 2/2] nbd: change a parameter's type to remove a memcpy call
On 7/20/07, Paul Clements <[EMAIL PROTECTED]> wrote: Denis Cheng wrote: > this memcpy looks so strange, in fact it's merely a pointer dereference, > so I change the parameter's type to refer it more directly, > this could make the memcpy not needed anymore. > > in the function nbd_read_stat where nbd_find_request is only once called, > the parameter served should be transformed accordingly. This is really a matter of preference. The generated code ends up being about the same, I think, while your patch makes the call to nbd_find_request kind of obtuse. Also, the memcpy's are balanced between send_req and find_request, so you can quickly see how the data is being transferred (from req into handle, and then back again). Your patch makes this less clear, at least to me. With one explicit memcpy stripped out, I think it's more clear to nbd_find_request. In nbd_read_stat, the cast to (struct request **) is not apparent, I must admit; but I think the best solution is declaring other few structs to make it clear, it's due to the lack of description of nbd client and server communication protocol. BTW, I think the nbd driver needs a clear documentation (its main site http://nbd.sourceforge.net/ does not give it): 1. When nbd_find_request is needed to call, the 8 byte memory of char handle[8] field in struct nbd_reply actually stores a pointer (struct request *), that pointer is received from the network. Since a pointer is only meaningful to the host, transfering it over the network will be unreliable, I don't think it's a good design, -- Denis Cheng Linux Application Developer "One of my most productive days was throwing away 1000 lines of code." - Ken Thompson. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] nbd: use list_for_each_entry_safe to make it more consolidated and readable
On 7/19/07, rae l <[EMAIL PROTECTED]> wrote:
On 7/19/07, Paul Clements <[EMAIL PROTECTED]> wrote:
> Could you name "n" as "tmp" (as in the previous code) so that it's clear
> that's only a temporary variable. Other than that, this looks good.
Sure. I just use the name "n" as in the declaration of
list_for_each_entry_safe in the header file
I'll resend it a little later.
Signed-off-by: Denis Cheng <[EMAIL PROTECTED]>
---
drivers/block/nbd.c |6 ++
1 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c
index c129510..86639c0 100644
--- a/drivers/block/nbd.c
+++ b/drivers/block/nbd.c
@@ -237,8 +237,7 @@ error_out:
static struct request *nbd_find_request(struct nbd_device *lo, char *handle)
{
- struct request *req;
- struct list_head *tmp;
+ struct request *req, *tmp;
struct request *xreq;
int err;
@@ -249,8 +248,7 @@ static struct request *nbd_find_request(struct
nbd_device *lo, char *handle)
goto out;
spin_lock(>queue_lock);
- list_for_each(tmp, >queue_head) {
- req = list_entry(tmp, struct request, queuelist);
+ list_for_each_entry_safe(req, tmp, >queue_head, queuelist) {
if (req != xreq)
continue;
list_del_init(>queuelist);
--
Denis Cheng
Linux Application Developer
"One of my most productive days was throwing away 1000 lines of code."
- Ken Thompson.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] nbd: use list_for_each_entry_safe to make it more consolidated and readable
On 7/19/07, Paul Clements <[EMAIL PROTECTED]> wrote: Could you name "n" as "tmp" (as in the previous code) so that it's clear that's only a temporary variable. Other than that, this looks good. Sure. I just use the name "n" as in the declaration of list_for_each_entry_safe in the header file I'll resend it a little later. Thanks, Paul -- Denis Cheng Linux Application Developer "One of my most productive days was throwing away 1000 lines of code." - Ken Thompson. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] nbd: use list_for_each_entry_safe to make it more consolidated and readable
On 7/19/07, Paul Clements [EMAIL PROTECTED] wrote: Could you name n as tmp (as in the previous code) so that it's clear that's only a temporary variable. Other than that, this looks good. Sure. I just use the name n as in the declaration of list_for_each_entry_safe in the header file linux/list.h I'll resend it a little later. Thanks, Paul -- Denis Cheng Linux Application Developer One of my most productive days was throwing away 1000 lines of code. - Ken Thompson. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] nbd: use list_for_each_entry_safe to make it more consolidated and readable
On 7/19/07, rae l [EMAIL PROTECTED] wrote:
On 7/19/07, Paul Clements [EMAIL PROTECTED] wrote:
Could you name n as tmp (as in the previous code) so that it's clear
that's only a temporary variable. Other than that, this looks good.
Sure. I just use the name n as in the declaration of
list_for_each_entry_safe in the header file linux/list.h
I'll resend it a little later.
Signed-off-by: Denis Cheng [EMAIL PROTECTED]
---
drivers/block/nbd.c |6 ++
1 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c
index c129510..86639c0 100644
--- a/drivers/block/nbd.c
+++ b/drivers/block/nbd.c
@@ -237,8 +237,7 @@ error_out:
static struct request *nbd_find_request(struct nbd_device *lo, char *handle)
{
- struct request *req;
- struct list_head *tmp;
+ struct request *req, *tmp;
struct request *xreq;
int err;
@@ -249,8 +248,7 @@ static struct request *nbd_find_request(struct
nbd_device *lo, char *handle)
goto out;
spin_lock(lo-queue_lock);
- list_for_each(tmp, lo-queue_head) {
- req = list_entry(tmp, struct request, queuelist);
+ list_for_each_entry_safe(req, tmp, lo-queue_head, queuelist) {
if (req != xreq)
continue;
list_del_init(req-queuelist);
--
Denis Cheng
Linux Application Developer
One of my most productive days was throwing away 1000 lines of code.
- Ken Thompson.
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2] nbd: change a parameter's type to remove a memcpy call
On 7/20/07, Paul Clements [EMAIL PROTECTED] wrote: Denis Cheng wrote: this memcpy looks so strange, in fact it's merely a pointer dereference, so I change the parameter's type to refer it more directly, this could make the memcpy not needed anymore. in the function nbd_read_stat where nbd_find_request is only once called, the parameter served should be transformed accordingly. This is really a matter of preference. The generated code ends up being about the same, I think, while your patch makes the call to nbd_find_request kind of obtuse. Also, the memcpy's are balanced between send_req and find_request, so you can quickly see how the data is being transferred (from req into handle, and then back again). Your patch makes this less clear, at least to me. With one explicit memcpy stripped out, I think it's more clear to nbd_find_request. In nbd_read_stat, the cast to (struct request **) is not apparent, I must admit; but I think the best solution is declaring other few structs to make it clear, it's due to the lack of description of nbd client and server communication protocol. BTW, I think the nbd driver needs a clear documentation (its main site http://nbd.sourceforge.net/ does not give it): 1. When nbd_find_request is needed to call, the 8 byte memory of char handle[8] field in struct nbd_reply actually stores a pointer (struct request *), that pointer is received from the network. Since a pointer is only meaningful to the host, transfering it over the network will be unreliable, I don't think it's a good design, -- Denis Cheng Linux Application Developer One of my most productive days was throwing away 1000 lines of code. - Ken Thompson. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] net/core: merge the content of dev_mcast.c into dev.c
On 7/18/07, Denis Cheng [EMAIL PROTECTED] wrote:
- removed three function declarations from header file to mark them static,
- reduced one file
Signed-off-by: Denis Cheng [EMAIL PROTECTED]
---
this one is just merging by concatenating, and I'll try to adjust some
function definitions' order to make it more readable.
include/linux/netdevice.h |3 -
net/core/Makefile |2 +-
net/core/dev.c| 239 +-
net/core/dev_mcast.c | 255 -
4 files changed, 237 insertions(+), 262 deletions(-)
delete mode 100644 net/core/dev_mcast.c
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 9820ca1..ca68c58 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -1091,15 +1091,12 @@ extern int register_netdev(struct
net_device *dev);
extern voidunregister_netdev(struct net_device *dev);
/* Functions used for secondary unicast and multicast support */
extern voiddev_set_rx_mode(struct net_device *dev);
-extern void__dev_set_rx_mode(struct net_device *dev);
extern int dev_unicast_delete(struct net_device *dev, void *addr,
int alen);
extern int dev_unicast_add(struct net_device *dev, void *addr, int
alen);
extern int dev_mc_delete(struct net_device *dev, void *addr, int
alen, int all);
extern int dev_mc_add(struct net_device *dev, void *addr, int
alen, int newonly);
extern int dev_mc_sync(struct net_device *to, struct net_device
*from);
extern voiddev_mc_unsync(struct net_device *to, struct net_device
*from);
-extern int __dev_addr_delete(struct dev_addr_list **list, int
*count, void *addr, int alen, int all);
-extern int __dev_addr_add(struct dev_addr_list **list, int *count,
void *addr, int alen, int newonly);
extern voiddev_set_promiscuity(struct net_device *dev, int inc);
extern voiddev_set_allmulti(struct net_device *dev, int inc);
extern voidnetdev_state_change(struct net_device *dev);
diff --git a/net/core/Makefile b/net/core/Makefile
index 4751613..54d28dd 100644
--- a/net/core/Makefile
+++ b/net/core/Makefile
@@ -7,7 +7,7 @@ obj-y := sock.o request_sock.o skbuff.o iovec.o datagram.o
stream.o scm.o \
obj-$(CONFIG_SYSCTL) += sysctl_net_core.o
-obj-y += dev.o ethtool.o dev_mcast.o dst.o netevent.o \
+obj-y += dev.o ethtool.o dst.o netevent.o \
neighbour.o rtnetlink.o utils.o link_watch.o filter.o
obj-$(CONFIG_XFRM) += flow.o
diff --git a/net/core/dev.c b/net/core/dev.c
index 6357f54..16842af 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -18,6 +18,7 @@
* Alexey Kuznetsov [EMAIL PROTECTED]
* Adam Sulmicki [EMAIL PROTECTED]
* Pekka Riikonen [EMAIL PROTECTED]
+ * Denis Cheng [EMAIL PROTECTED]
*
* Changes:
* D.J. Barrow : Fixed bug where dev-refcnt gets set
@@ -70,6 +71,32 @@
* indefinitely on dev-refcnt
* J Hadi Salim: - Backlog queue sampling
* - netif_rx() feedback
+ * Denis Cheng : Merge dev_mcast.c into it
+ */
+
+/*
+ * The original information in dev_mcast.c:
+ *
+ * Linux NET3: Multicast List maintenance.
+ *
+ * Authors:
+ * Tim Kordas [EMAIL PROTECTED]
+ * Richard Underwood [EMAIL PROTECTED]
+ *
+ * Stir fried together from the IP multicast and CAP patches above
+ * Alan Cox [EMAIL PROTECTED]
+ *
+ * Fixes:
+ * Alan Cox: Update the device on a real delete
+ * rather than any time but...
+ * Alan Cox: IFF_ALLMULTI support.
+ * Alan Cox: New format set_multicast_list() calls.
+ * Gleb Natapov: Remove dev_mc_lock.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version
+ * 2 of the License, or (at your option) any later version.
*/
#include asm/uaccess.h
@@ -2622,7 +2649,7 @@ void dev_set_allmulti(struct net_device *dev, int inc)
* filtering it is put in promiscous mode while unicast addresses
* are present.
*/
-void __dev_set_rx_mode(struct net_device *dev)
+static void __dev_set_rx_mode(struct net_device *dev)
{
/* dev_open will call this function so the list will stay sane. */
if (!(dev-flagsIFF_UP))
@@ -2657,7 +2684,7 @@ void dev_set_rx_mode(struct net_device *dev)
netif_tx_unlock_bh(dev);
}
-int __dev_addr_delete(struct dev_addr_list **list, int *count,
+static int __dev_addr_delete(struct
Re: [PATCH 1/2] net/core: merge the content of dev_mcast.c into dev.c
On 7/20/07, David Miller [EMAIL PROTECTED] wrote: Please don't quote a big huge patch just to say one sentence that doesn't apply to any particular specific part of a patch. That's wastes bandwidth, annoys people you might actually want a response from, and is bad netiquette in general. Thanks. -- Denis Cheng Linux Application Developer One of my most productive days was throwing away 1000 lines of code. - Ken Thompson. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2] net/core: some functions' definition order adjustment for readability
On 7/18/07, Patrick McHardy <[EMAIL PROTECTED]> wrote: This could be done in the patch moving it .. anyways, What? Denis Cheng wrote: > +#ifdef CONFIG_PROC_FS > +static void *dev_mc_seq_start(struct seq_file *seq, loff_t *pos) If you're interested in doing more work, it would be nice to generalize the seq-file functions for unicast and multicast address lists and add /proc/net/dev_unicast or something like that. Eh, there is already a dev_multicast file but lack of dev_unicast, but is dev_unicast really useful? OTOH we could also export this using rtnetlink. The main reason why I didn't do that is that it can only be read, not changed, but this is also true for statistics etc. Any opinions on this? how to do that? -- Denis Cheng Linux Application Developer "One of my most productive days was throwing away 1000 lines of code." - Ken Thompson. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/3] [net/core] move dev_mc_discard from dev_mcast.c to dev.c
On 7/18/07, David Miller <[EMAIL PROTECTED]> wrote: From: Denis Cheng <[EMAIL PROTECTED]> Date: Wed, 18 Jul 2007 10:41:03 +0800 > Because this function is only called by unregister_netdevice, > this moving could make this non-global function static, > and also remove its declaration in netdevice.h; > > Any further, function __dev_addr_discard is also just called by > dev_mc_discard and dev_unicast_discard, keeping this two functions > both in one c file could make __dev_addr_discard also static > and remove its declaration in netdevice.h; > > Futhermore, the sequential call to dev_unicast_discard and then > dev_mc_discard in unregister_netdevice have a similar mechanism that: > (netif_tx_lock_bh / __dev_addr_discard / netif_tx_unlock_bh), > they should merged into one to eliminate duplicates in acquiring and > releasing the dev->_xmit_lock, this would be done in my following patch. > > Signed-off-by: Denis Cheng <[EMAIL PROTECTED]> Patch applied, thanks. Thanks for applying, too. And then the dev_mcast.c is now only 256 lines long(versus dev.c 4052 lines), just left a few multicast related functions definition and "dev_mcast" procfs code, I have an idea to merge all code dev_mcast.c into dev.c, that would: - remove two functions (__dev_addr_delete, __dev_set_rx_mode) from netdevice.h, and then tag them static, those two are also defined in dev.c and only called from dev_mcast.c, - reducing one file would benefit the compilation process. All in one word, I don't think the single file dev_mcast.c is needed anymore. -- Denis Cheng Linux Application Developer "One of my most productive days was throwing away 1000 lines of code." - Ken Thompson. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/3] [net/core] move dev_mc_discard from dev_mcast.c to dev.c
On 7/18/07, David Miller [EMAIL PROTECTED] wrote: From: Denis Cheng [EMAIL PROTECTED] Date: Wed, 18 Jul 2007 10:41:03 +0800 Because this function is only called by unregister_netdevice, this moving could make this non-global function static, and also remove its declaration in netdevice.h; Any further, function __dev_addr_discard is also just called by dev_mc_discard and dev_unicast_discard, keeping this two functions both in one c file could make __dev_addr_discard also static and remove its declaration in netdevice.h; Futhermore, the sequential call to dev_unicast_discard and then dev_mc_discard in unregister_netdevice have a similar mechanism that: (netif_tx_lock_bh / __dev_addr_discard / netif_tx_unlock_bh), they should merged into one to eliminate duplicates in acquiring and releasing the dev-_xmit_lock, this would be done in my following patch. Signed-off-by: Denis Cheng [EMAIL PROTECTED] Patch applied, thanks. Thanks for applying, too. And then the dev_mcast.c is now only 256 lines long(versus dev.c 4052 lines), just left a few multicast related functions definition and dev_mcast procfs code, I have an idea to merge all code dev_mcast.c into dev.c, that would: - remove two functions (__dev_addr_delete, __dev_set_rx_mode) from netdevice.h, and then tag them static, those two are also defined in dev.c and only called from dev_mcast.c, - reducing one file would benefit the compilation process. All in one word, I don't think the single file dev_mcast.c is needed anymore. -- Denis Cheng Linux Application Developer One of my most productive days was throwing away 1000 lines of code. - Ken Thompson. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2] net/core: some functions' definition order adjustment for readability
On 7/18/07, Patrick McHardy [EMAIL PROTECTED] wrote: This could be done in the patch moving it .. anyways, What? Denis Cheng wrote: +#ifdef CONFIG_PROC_FS +static void *dev_mc_seq_start(struct seq_file *seq, loff_t *pos) If you're interested in doing more work, it would be nice to generalize the seq-file functions for unicast and multicast address lists and add /proc/net/dev_unicast or something like that. Eh, there is already a dev_multicast file but lack of dev_unicast, but is dev_unicast really useful? OTOH we could also export this using rtnetlink. The main reason why I didn't do that is that it can only be read, not changed, but this is also true for statistics etc. Any opinions on this? how to do that? -- Denis Cheng Linux Application Developer One of my most productive days was throwing away 1000 lines of code. - Ken Thompson. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] replace kmem_cache_alloc with kmem_cache_zalloc to remove some following zero initializations.
On 7/13/07, Kirill Korotaev <[EMAIL PROTECTED]> wrote: This doesn't look worth zeroing half of the struct when it is initialized to non-zeros then. But why? My reason to think it's better and faster is that: 1. the code will be shorter if it calls zalloc and then removes the NULL and zero initilization; 2. in the assembly code objdumped, many mov operations reduced, such as: movl $0,0x40(%ebp) ... this style of zero initialization occupies 7 bytes per line (i386), and then multiply 7 lines, 3. the only change is that calls to kmem_cache_zalloc other than kmem_cache_alloc, it's just an extra memset is called, as we all know the memset implimentation is string operation, that's rather fast. Denis Cheng wrote: >>From 4d87e14b67890f06885a76b5792ca034de2e9d06 Mon Sep 17 00:00:00 2001 > From: Denis Cheng <[EMAIL PROTECTED]> > Date: Thu, 12 Jul 2007 11:53:58 +0800 > Subject: [PATCH] replace kmem_cache_alloc with kmem_cache_zalloc to > remove some following zero initializations. > > Signed-off-by: Denis Cheng <[EMAIL PROTECTED]> > --- > fs/dcache.c | 12 ++-- > 1 files changed, 2 insertions(+), 10 deletions(-) > > diff --git a/fs/dcache.c b/fs/dcache.c > index 0e73aa0..8c559b2 100644 > --- a/fs/dcache.c > +++ b/fs/dcache.c > @@ -898,7 +898,7 @@ struct dentry *d_alloc(struct dentry * parent, const > struct qstr *name) > struct dentry *dentry; > char *dname; > > - dentry = kmem_cache_alloc(dentry_cache, GFP_KERNEL); > + dentry = kmem_cache_zalloc(dentry_cache, GFP_KERNEL); > if (!dentry) > return NULL; > > @@ -921,15 +921,7 @@ struct dentry *d_alloc(struct dentry * parent, > const struct qstr *name) > atomic_set(>d_count, 1); > dentry->d_flags = DCACHE_UNHASHED; > spin_lock_init(>d_lock); > - dentry->d_inode = NULL; > - dentry->d_parent = NULL; > - dentry->d_sb = NULL; > - dentry->d_op = NULL; > - dentry->d_fsdata = NULL; > - dentry->d_mounted = 0; > -#ifdef CONFIG_PROFILING > - dentry->d_cookie = NULL; > -#endif > + > INIT_HLIST_NODE(>d_hash); > INIT_LIST_HEAD(>d_lru); > INIT_LIST_HEAD(>d_subdirs); -- Denis Cheng Linux Application Developer "One of my most productive days was throwing away 1000 lines of code." - Ken Thompson. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] replace kmem_cache_alloc with kmem_cache_zalloc to remove some following zero initializations.
On 7/13/07, Kirill Korotaev [EMAIL PROTECTED] wrote: This doesn't look worth zeroing half of the struct when it is initialized to non-zeros then. But why? My reason to think it's better and faster is that: 1. the code will be shorter if it calls zalloc and then removes the NULL and zero initilization; 2. in the assembly code objdumped, many mov operations reduced, such as: movl $0,0x40(%ebp) ... this style of zero initialization occupies 7 bytes per line (i386), and then multiply 7 lines, 3. the only change is that calls to kmem_cache_zalloc other than kmem_cache_alloc, it's just an extra memset is called, as we all know the memset implimentation is string operation, that's rather fast. Denis Cheng wrote: From 4d87e14b67890f06885a76b5792ca034de2e9d06 Mon Sep 17 00:00:00 2001 From: Denis Cheng [EMAIL PROTECTED] Date: Thu, 12 Jul 2007 11:53:58 +0800 Subject: [PATCH] replace kmem_cache_alloc with kmem_cache_zalloc to remove some following zero initializations. Signed-off-by: Denis Cheng [EMAIL PROTECTED] --- fs/dcache.c | 12 ++-- 1 files changed, 2 insertions(+), 10 deletions(-) diff --git a/fs/dcache.c b/fs/dcache.c index 0e73aa0..8c559b2 100644 --- a/fs/dcache.c +++ b/fs/dcache.c @@ -898,7 +898,7 @@ struct dentry *d_alloc(struct dentry * parent, const struct qstr *name) struct dentry *dentry; char *dname; - dentry = kmem_cache_alloc(dentry_cache, GFP_KERNEL); + dentry = kmem_cache_zalloc(dentry_cache, GFP_KERNEL); if (!dentry) return NULL; @@ -921,15 +921,7 @@ struct dentry *d_alloc(struct dentry * parent, const struct qstr *name) atomic_set(dentry-d_count, 1); dentry-d_flags = DCACHE_UNHASHED; spin_lock_init(dentry-d_lock); - dentry-d_inode = NULL; - dentry-d_parent = NULL; - dentry-d_sb = NULL; - dentry-d_op = NULL; - dentry-d_fsdata = NULL; - dentry-d_mounted = 0; -#ifdef CONFIG_PROFILING - dentry-d_cookie = NULL; -#endif + INIT_HLIST_NODE(dentry-d_hash); INIT_LIST_HEAD(dentry-d_lru); INIT_LIST_HEAD(dentry-d_subdirs); -- Denis Cheng Linux Application Developer One of my most productive days was throwing away 1000 lines of code. - Ken Thompson. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Does the kernel HPET support has problems or the hwclock from util-linux?
On 7/3/07, Luca Tettamanti <[EMAIL PROTECTED]> wrote: rae l <[EMAIL PROTECTED]> ha scritto: > from this address, I know util-linux-2.12r is the latest: > http://www.kernel.org/pub/linux/utils/util-linux/util-linux-2.12r.lsm > > My Dell OptiPlex 320 has 4 HPET timers and no RTC, so the execution of > hwclock has errors: > > [EMAIL PROTECTED] ~ $ /sbin/hwclock --show > select() to /dev/rtc to wait for clock tick timed out > [EMAIL PROTECTED] ~ $ /sbin/hwclock --version > hwclock from util-linux-2.12r I think that the problem is that HPET and the CMOS RTC (list in CC) share the same interrupt line. I suppose that you should enable CONFIG_HPET_RTC_IRQ (my hardware has the same "feature"); in this way /dev/rtc correcly reports that it cannot deliver the interrupt (when HPET is enabled) and hwclock uses direct ISA access. I searched the CONFIG_HPET_RTC_IRQ, it's conflict with HPET_EMULATE_RTC, that I've enabled. so while HPET_RTC_IRQ is the good way, EMULATE_RTC is a bad way. Luca -- Windows NT crashed. I'm the Blue Screen of Death. No one hears your screams. -- Denis Cheng Linux Application Developer - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Does the kernel HPET support has problems or the hwclock from util-linux?
On 7/3/07, Luca Tettamanti [EMAIL PROTECTED] wrote: rae l [EMAIL PROTECTED] ha scritto: from this address, I know util-linux-2.12r is the latest: http://www.kernel.org/pub/linux/utils/util-linux/util-linux-2.12r.lsm My Dell OptiPlex 320 has 4 HPET timers and no RTC, so the execution of hwclock has errors: [EMAIL PROTECTED] ~ $ /sbin/hwclock --show select() to /dev/rtc to wait for clock tick timed out [EMAIL PROTECTED] ~ $ /sbin/hwclock --version hwclock from util-linux-2.12r I think that the problem is that HPET and the CMOS RTC (list in CC) share the same interrupt line. I suppose that you should enable CONFIG_HPET_RTC_IRQ (my hardware has the same feature); in this way /dev/rtc correcly reports that it cannot deliver the interrupt (when HPET is enabled) and hwclock uses direct ISA access. I searched the CONFIG_HPET_RTC_IRQ, it's conflict with HPET_EMULATE_RTC, that I've enabled. so while HPET_RTC_IRQ is the good way, EMULATE_RTC is a bad way. Luca -- Windows NT crashed. I'm the Blue Screen of Death. No one hears your screams. -- Denis Cheng Linux Application Developer - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Does the kernel HPET support has problems or the hwclock from util-linux?
from this address, I know util-linux-2.12r is the latest: http://www.kernel.org/pub/linux/utils/util-linux/util-linux-2.12r.lsm My Dell OptiPlex 320 has 4 HPET timers and no RTC, so the execution of hwclock has errors: [EMAIL PROTECTED] ~ $ /sbin/hwclock --show select() to /dev/rtc to wait for clock tick timed out [EMAIL PROTECTED] ~ $ /sbin/hwclock --version hwclock from util-linux-2.12r I wonder is there (kernel.org) a specific description of hwclock and util-linux? -- Denis Cheng Linux Application Developer - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Does the kernel HPET support has problems or the hwclock from util-linux?
from this address, I know util-linux-2.12r is the latest: http://www.kernel.org/pub/linux/utils/util-linux/util-linux-2.12r.lsm My Dell OptiPlex 320 has 4 HPET timers and no RTC, so the execution of hwclock has errors: [EMAIL PROTECTED] ~ $ /sbin/hwclock --show select() to /dev/rtc to wait for clock tick timed out [EMAIL PROTECTED] ~ $ /sbin/hwclock --version hwclock from util-linux-2.12r I wonder is there (kernel.org) a specific description of hwclock and util-linux? -- Denis Cheng Linux Application Developer - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] trivial: the memset operation on a automatic array variable should be optimized out by data initialization
On 6/23/07, Oleg Verych <[EMAIL PROTECTED]> wrote:
Why not just show actual objdump output on code (maybe with different
oxygen atoms used in gcc), rather than *talking* about optimization and
standards, hm?
here is the objdump output of the two object files:
As you could see, the older one used 0x38 bytes stack space while the
new one used 0x28 bytes,
and the object code is two bytes less,
I think all these benefits are the gcc's __builtin_memset optimization
than the explicit call to memset.
$ objdump -d /tmp/init.orig.o|grep -A23 -nw ''
525:0395 :
526- 395: 48 83 ec 38 sub$0x38,%rsp
527- 399: 48 8d 54 24 10 lea0x10(%rsp),%rdx
528- 39e: fc cld
529- 39f: 31 c0 xor%eax,%eax
530- 3a1: 48 89 d7mov%rdx,%rdi
531- 3a4: ab stos %eax,%es:(%rdi)
532- 3a5: ab stos %eax,%es:(%rdi)
533- 3a6: ab stos %eax,%es:(%rdi)
534- 3a7: ab stos %eax,%es:(%rdi)
535- 3a8: ab stos %eax,%es:(%rdi)
536- 3a9: 48 89 7c 24 08 mov%rdi,0x8(%rsp)
537- 3ae: ab stos %eax,%es:(%rdi)
538- 3af: 48 c7 44 24 10 00 10movq $0x1000,0x10(%rsp)
539- 3b6: 00 00
540- 3b8: 48 c7 44 24 18 00 00movq $0x10,0x18(%rsp)
541- 3bf: 10 00
542- 3c1: 48 8b 05 00 00 00 00mov0(%rip),%rax#
3c8
543- 3c8: 48 89 44 24 20 mov%rax,0x20(%rsp)
544- 3cd: 48 89 d7mov%rdx,%rdi
545- 3d0: e8 00 00 00 00 callq 3d5
546- 3d5: 48 83 c4 38 add$0x38,%rsp
547- 3d9: c3 retq
548-
$ objdump -d /tmp/init.new.o|grep -A23 -nw ''
525:0395 :
526- 395: 48 83 ec 28 sub$0x28,%rsp
527- 399: 48 89 e7mov%rsp,%rdi
528- 39c: fc cld
529- 39d: 31 c0 xor%eax,%eax
530- 39f: ab stos %eax,%es:(%rdi)
531- 3a0: ab stos %eax,%es:(%rdi)
532- 3a1: ab stos %eax,%es:(%rdi)
533- 3a2: ab stos %eax,%es:(%rdi)
534- 3a3: ab stos %eax,%es:(%rdi)
535- 3a4: ab stos %eax,%es:(%rdi)
536- 3a5: 48 c7 04 24 00 10 00movq $0x1000,(%rsp)
537- 3ac: 00
538- 3ad: 48 c7 44 24 08 00 00movq $0x10,0x8(%rsp)
539- 3b4: 10 00
540- 3b6: 48 8b 05 00 00 00 00mov0(%rip),%rax#
3bd
541- 3bd: 48 89 44 24 10 mov%rax,0x10(%rsp)
542- 3c2: 48 89 e7mov%rsp,%rdi
543- 3c5: e8 00 00 00 00 callq 3ca
544- 3ca: 48 83 c4 28 add$0x28,%rsp
545- 3ce: c3 retq
546-
547-03cf :
548- 3cf: 41 56 push %r14
I bet, that will be a key for success. And if you are interested in such
optimizations, why not to grep whole source tree for this kind of
things? I'm not sure one function in arch/x86_64 is only such ``unoptimized''.
And after doing that maybe you will see, that "{}" initializer can be
applied not only to integer values (you did init with of *long int*,
with *int*, btw), but to structs and others.
with '{}' initializer, gcc will fill its memory with zeros.
to other potential points to be optimized, I only see this trivial as
the first point, I wonder how people gives comments on this; and if
this optimization can be tested correctly, this can be done as an
optimization example and I'll try others.
Ahh, one more thing about _optimizing_ your time, i.e. not wasting one.
Add to CC list people, who already did reply on you patch. Otherwise
you are showing your disrespect for them and hiding from further
discussion.
Thank you, I know it and I've already subscribed the linux kernel
mailing list(linux-kernel@vger.kernel.org) so that I won't miss any
further discussion about it.
I think you do not, but Linux development not have an automatic system
for patch tracking, so you are on your own with your text editor and
e-mail client on this. Please take care for your time.
What about that?
Do you mean something such as git by "an automatic system"?
--
frenzy
-o--=O`C
#oo'L O
<___=E M
--
Denis Cheng
Linux Application Developer
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] trivial: the memset operation on a automatic array variable should be optimized out by data initialization
On 6/23/07, Oleg Verych [EMAIL PROTECTED] wrote:
Why not just show actual objdump output on code (maybe with different
oxygen atoms used in gcc), rather than *talking* about optimization and
standards, hm?
here is the objdump output of the two object files:
As you could see, the older one used 0x38 bytes stack space while the
new one used 0x28 bytes,
and the object code is two bytes less,
I think all these benefits are the gcc's __builtin_memset optimization
than the explicit call to memset.
$ objdump -d /tmp/init.orig.o|grep -A23 -nw 'paging_init'
525:0395 paging_init:
526- 395: 48 83 ec 38 sub$0x38,%rsp
527- 399: 48 8d 54 24 10 lea0x10(%rsp),%rdx
528- 39e: fc cld
529- 39f: 31 c0 xor%eax,%eax
530- 3a1: 48 89 d7mov%rdx,%rdi
531- 3a4: ab stos %eax,%es:(%rdi)
532- 3a5: ab stos %eax,%es:(%rdi)
533- 3a6: ab stos %eax,%es:(%rdi)
534- 3a7: ab stos %eax,%es:(%rdi)
535- 3a8: ab stos %eax,%es:(%rdi)
536- 3a9: 48 89 7c 24 08 mov%rdi,0x8(%rsp)
537- 3ae: ab stos %eax,%es:(%rdi)
538- 3af: 48 c7 44 24 10 00 10movq $0x1000,0x10(%rsp)
539- 3b6: 00 00
540- 3b8: 48 c7 44 24 18 00 00movq $0x10,0x18(%rsp)
541- 3bf: 10 00
542- 3c1: 48 8b 05 00 00 00 00mov0(%rip),%rax#
3c8 paging_init+0x33
543- 3c8: 48 89 44 24 20 mov%rax,0x20(%rsp)
544- 3cd: 48 89 d7mov%rdx,%rdi
545- 3d0: e8 00 00 00 00 callq 3d5 paging_init+0x40
546- 3d5: 48 83 c4 38 add$0x38,%rsp
547- 3d9: c3 retq
548-
$ objdump -d /tmp/init.new.o|grep -A23 -nw 'paging_init'
525:0395 paging_init:
526- 395: 48 83 ec 28 sub$0x28,%rsp
527- 399: 48 89 e7mov%rsp,%rdi
528- 39c: fc cld
529- 39d: 31 c0 xor%eax,%eax
530- 39f: ab stos %eax,%es:(%rdi)
531- 3a0: ab stos %eax,%es:(%rdi)
532- 3a1: ab stos %eax,%es:(%rdi)
533- 3a2: ab stos %eax,%es:(%rdi)
534- 3a3: ab stos %eax,%es:(%rdi)
535- 3a4: ab stos %eax,%es:(%rdi)
536- 3a5: 48 c7 04 24 00 10 00movq $0x1000,(%rsp)
537- 3ac: 00
538- 3ad: 48 c7 44 24 08 00 00movq $0x10,0x8(%rsp)
539- 3b4: 10 00
540- 3b6: 48 8b 05 00 00 00 00mov0(%rip),%rax#
3bd paging_init+0x28
541- 3bd: 48 89 44 24 10 mov%rax,0x10(%rsp)
542- 3c2: 48 89 e7mov%rsp,%rdi
543- 3c5: e8 00 00 00 00 callq 3ca paging_init+0x35
544- 3ca: 48 83 c4 28 add$0x28,%rsp
545- 3ce: c3 retq
546-
547-03cf alloc_low_page:
548- 3cf: 41 56 push %r14
I bet, that will be a key for success. And if you are interested in such
optimizations, why not to grep whole source tree for this kind of
things? I'm not sure one function in arch/x86_64 is only such ``unoptimized''.
And after doing that maybe you will see, that {} initializer can be
applied not only to integer values (you did init with of *long int*,
with *int*, btw), but to structs and others.
with '{}' initializer, gcc will fill its memory with zeros.
to other potential points to be optimized, I only see this trivial as
the first point, I wonder how people gives comments on this; and if
this optimization can be tested correctly, this can be done as an
optimization example and I'll try others.
Ahh, one more thing about _optimizing_ your time, i.e. not wasting one.
Add to CC list people, who already did reply on you patch. Otherwise
you are showing your disrespect for them and hiding from further
discussion.
Thank you, I know it and I've already subscribed the linux kernel
mailing list(linux-kernel@vger.kernel.org) so that I won't miss any
further discussion about it.
I think you do not, but Linux development not have an automatic system
for patch tracking, so you are on your own with your text editor and
e-mail client on this. Please take care for your time.
What about that?
Do you mean something such as git by an automatic system?
--
frenzy
-o--=O`C
#oo'L O
___=E M
--
Denis Cheng
Linux Application Developer
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] trivial: the memset operation on a automatic array variable should be optimized out by data initialization
On 6/19/07, Jeremy Fitzhardinge <[EMAIL PROTECTED]> wrote: Denis Cheng wrote: > From: Denis Cheng <[EMAIL PROTECTED]> > > the explicit memset call could be optimized out by data initialization, > thus all the fill working can be done by the compiler implicitly. > How does the generated code change? Does gcc do something stupid like statically allocate a prototype structure full of zeros, and then memcpy it in? Or does it generate a series of explicit assignments for each member? Or does it generate a memset anyway? Seems to me that this gives gcc the opportunity to be more stupid, and the only right answer is what we're doing anyway. J Technically speaking, C standard guarantees the data be initialized correctly; just from the point view of code style, let the compiler selects how to initialize will be better, this could let the compiler has more optimization points. -- Denis Cheng Linux Application Developer - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] trivial: the memset operation on a automatic array variable should be optimized out by data initialization
On 6/19/07, Jeremy Fitzhardinge [EMAIL PROTECTED] wrote: Denis Cheng wrote: From: Denis Cheng [EMAIL PROTECTED] the explicit memset call could be optimized out by data initialization, thus all the fill working can be done by the compiler implicitly. How does the generated code change? Does gcc do something stupid like statically allocate a prototype structure full of zeros, and then memcpy it in? Or does it generate a series of explicit assignments for each member? Or does it generate a memset anyway? Seems to me that this gives gcc the opportunity to be more stupid, and the only right answer is what we're doing anyway. J Technically speaking, C standard guarantees the data be initialized correctly; just from the point view of code style, let the compiler selects how to initialize will be better, this could let the compiler has more optimization points. -- Denis Cheng Linux Application Developer - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] lib: Replace calls to __get_free_pages() with __get_dma_pages().
On 6/6/07, Andrew Morton <[EMAIL PROTECTED]> wrote:
On Tue, 5 Jun 2007 16:58:57 -0400 (EDT)
"Robert P. J. Day" <[EMAIL PROTECTED]> wrote:
> Replace a couple calls to __get_free_pages() with the corresponding
> calls to __get_dma_pages().
>
> Signed-off-by: Robert P. J. Day <[EMAIL PROTECTED]>
>
> ---
>
> that's the lot of them.
>
> diff --git a/lib/swiotlb.c b/lib/swiotlb.c
> index 10c13ad..8fc38dc 100644
> --- a/lib/swiotlb.c
> +++ b/lib/swiotlb.c
> @@ -201,8 +201,7 @@ swiotlb_late_init_with_default_size(size_t default_size)
> bytes = io_tlb_nslabs << IO_TLB_SHIFT;
>
> while ((SLABS_PER_PAGE << order) > IO_TLB_MIN_SLABS) {
> - io_tlb_start = (char *)__get_free_pages(GFP_DMA | __GFP_NOWARN,
> - order);
> + io_tlb_start = (char *)__get_dma_pages(__GFP_NOWARN, order);
__get_dma_pages() is just pointless obfuscation. I think it'd be better to
go the other way: open-code the GFP_DMA at all callsites then send
__get_dma_pages() bitbucketwards.
thus __get_free_pages(GFP_DMA ...) can do better, I don't think
__get_dma_pages is needed.
--
Denis Cheng
Linux Application Developer
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] lib: Replace calls to __get_free_pages() with __get_dma_pages().
On 6/6/07, Andrew Morton [EMAIL PROTECTED] wrote:
On Tue, 5 Jun 2007 16:58:57 -0400 (EDT)
Robert P. J. Day [EMAIL PROTECTED] wrote:
Replace a couple calls to __get_free_pages() with the corresponding
calls to __get_dma_pages().
Signed-off-by: Robert P. J. Day [EMAIL PROTECTED]
---
that's the lot of them.
diff --git a/lib/swiotlb.c b/lib/swiotlb.c
index 10c13ad..8fc38dc 100644
--- a/lib/swiotlb.c
+++ b/lib/swiotlb.c
@@ -201,8 +201,7 @@ swiotlb_late_init_with_default_size(size_t default_size)
bytes = io_tlb_nslabs IO_TLB_SHIFT;
while ((SLABS_PER_PAGE order) IO_TLB_MIN_SLABS) {
- io_tlb_start = (char *)__get_free_pages(GFP_DMA | __GFP_NOWARN,
- order);
+ io_tlb_start = (char *)__get_dma_pages(__GFP_NOWARN, order);
__get_dma_pages() is just pointless obfuscation. I think it'd be better to
go the other way: open-code the GFP_DMA at all callsites then send
__get_dma_pages() bitbucketwards.
thus __get_free_pages(GFP_DMA ...) can do better, I don't think
__get_dma_pages is needed.
--
Denis Cheng
Linux Application Developer
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] since the definition of dst_discard_in and dst_discard_out are the same,
On 6/2/07, Denis Cheng <[EMAIL PROTECTED]> wrote:
they should merged into one
this patch I have checked, it's not corrupted, I wonder someone can
give some comment on this?
Signed-off-by: Denis Cheng <[EMAIL PROTECTED]>
---
net/core/dst.c | 17 -
1 files changed, 4 insertions(+), 13 deletions(-)
diff --git a/net/core/dst.c b/net/core/dst.c
index 764bccb..c6a0587 100644
--- a/net/core/dst.c
+++ b/net/core/dst.c
@@ -111,13 +111,7 @@ out:
spin_unlock(_lock);
}
-static int dst_discard_in(struct sk_buff *skb)
-{
- kfree_skb(skb);
- return 0;
-}
-
-static int dst_discard_out(struct sk_buff *skb)
+static int dst_discard(struct sk_buff *skb)
{
kfree_skb(skb);
return 0;
@@ -138,8 +132,7 @@ void * dst_alloc(struct dst_ops * ops)
dst->ops = ops;
dst->lastuse = jiffies;
dst->path = dst;
- dst->input = dst_discard_in;
- dst->output = dst_discard_out;
+ dst->input = dst->output = dst_discard;
#if RT_CACHE_DEBUG >= 2
atomic_inc(_total);
#endif
@@ -153,8 +146,7 @@ static void ___dst_free(struct dst_entry * dst)
protocol module is unloaded.
*/
if (dst->dev == NULL || !(dst->dev->flags_UP)) {
- dst->input = dst_discard_in;
- dst->output = dst_discard_out;
+ dst->input = dst->output = dst_discard;
}
dst->obsolete = 2;
}
@@ -242,8 +234,7 @@ static inline void dst_ifdown(struct dst_entry *dst, struct
net_device *dev,
return;
if (!unregister) {
- dst->input = dst_discard_in;
- dst->output = dst_discard_out;
+ dst->input = dst->output = dst_discard;
} else {
dst->dev = _dev;
dev_hold(_dev);
--
1.4.4.2
--
Denis Cheng
Linux Application Developer
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] since the definition of dst_discard_in and dst_discard_out are the same,
On 6/2/07, Denis Cheng [EMAIL PROTECTED] wrote:
they should merged into one
this patch I have checked, it's not corrupted, I wonder someone can
give some comment on this?
Signed-off-by: Denis Cheng [EMAIL PROTECTED]
---
net/core/dst.c | 17 -
1 files changed, 4 insertions(+), 13 deletions(-)
diff --git a/net/core/dst.c b/net/core/dst.c
index 764bccb..c6a0587 100644
--- a/net/core/dst.c
+++ b/net/core/dst.c
@@ -111,13 +111,7 @@ out:
spin_unlock(dst_lock);
}
-static int dst_discard_in(struct sk_buff *skb)
-{
- kfree_skb(skb);
- return 0;
-}
-
-static int dst_discard_out(struct sk_buff *skb)
+static int dst_discard(struct sk_buff *skb)
{
kfree_skb(skb);
return 0;
@@ -138,8 +132,7 @@ void * dst_alloc(struct dst_ops * ops)
dst-ops = ops;
dst-lastuse = jiffies;
dst-path = dst;
- dst-input = dst_discard_in;
- dst-output = dst_discard_out;
+ dst-input = dst-output = dst_discard;
#if RT_CACHE_DEBUG = 2
atomic_inc(dst_total);
#endif
@@ -153,8 +146,7 @@ static void ___dst_free(struct dst_entry * dst)
protocol module is unloaded.
*/
if (dst-dev == NULL || !(dst-dev-flagsIFF_UP)) {
- dst-input = dst_discard_in;
- dst-output = dst_discard_out;
+ dst-input = dst-output = dst_discard;
}
dst-obsolete = 2;
}
@@ -242,8 +234,7 @@ static inline void dst_ifdown(struct dst_entry *dst, struct
net_device *dev,
return;
if (!unregister) {
- dst-input = dst_discard_in;
- dst-output = dst_discard_out;
+ dst-input = dst-output = dst_discard;
} else {
dst-dev = loopback_dev;
dev_hold(loopback_dev);
--
1.4.4.2
--
Denis Cheng
Linux Application Developer
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [git patch] since the definition of dst_discard_in and dst_discard_out are the same,,they should merged into one
On 5/31/07, David Miller <[EMAIL PROTECTED]> wrote: Your email client has changed all of the tab characters into spaces, corrupting the patch. Please configure your email client to not perform any text formatting. Please test this, by emailing the patch to yourself and trying to apply it, before resending. Thank you. Oh, as this, I'll try. A lot of thanks. -- Denis Cheng - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [git patch] since the definition of dst_discard_in and dst_discard_out are the same,,they should merged into one
On 5/31/07, David Miller [EMAIL PROTECTED] wrote: Your email client has changed all of the tab characters into spaces, corrupting the patch. Please configure your email client to not perform any text formatting. Please test this, by emailing the patch to yourself and trying to apply it, before resending. Thank you. Oh, as this, I'll try. A lot of thanks. -- Denis Cheng - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] merge dst_discard in & out into one, removed a duplicate function
On 5/28/07, Jan Engelhardt <[EMAIL PROTECTED]> wrote:
Uhm, just replace every invocation of dst_discard_in/_out() directly
by dst_discard ... don't add macros for that.
merge dst_discard in & out into one, this removed a duplicate function.
Signed-off-by: Denis Cheng <[EMAIL PROTECTED]>
---
Is there anybody also found this duplicate? I found this duplcate had
existed from linux-2.2 linux-2.4 to the latest kernel tree, you could
check it in the lxr site conveniently:
http://lxr.linux.no/source/net/core/dst.c?v=2.2.26
http://lxr.linux.no/source/net/core/dst.c?v=2.4.18
http://lxr.linux.no/source/net/core/dst.c
With little difference, the 2.2 and 2.4 version use two different
function name: dst_discard and dst_blackhole, but the definition of
them are the same, so they are same substantially. Additionally, the
two same function are two local(static) functions in a single file
dst.c, I don't know why there should exist two same functions. I
wonder what the consideration of this design is, because I'm just a
kernel newbie; this problem has been confusing me for many days, so if
you know why, please drop me some directions.
diff --git a/net/core/dst.c b/net/core/dst.c
index 764bccb..c6a0587 100644
--- a/net/core/dst.c
+++ b/net/core/dst.c
@@ -111,13 +111,7 @@ out:
spin_unlock(_lock);
}
-static int dst_discard_in(struct sk_buff *skb)
-{
- kfree_skb(skb);
- return 0;
-}
-
-static int dst_discard_out(struct sk_buff *skb)
+static int dst_discard(struct sk_buff *skb)
{
kfree_skb(skb);
return 0;
@@ -138,8 +132,7 @@ void * dst_alloc(struct dst_ops * ops)
dst->ops = ops;
dst->lastuse = jiffies;
dst->path = dst;
- dst->input = dst_discard_in;
- dst->output = dst_discard_out;
+ dst->input = dst->output = dst_discard;
#if RT_CACHE_DEBUG >= 2
atomic_inc(_total);
#endif
@@ -153,8 +146,7 @@ static void ___dst_free(struct dst_entry * dst)
protocol module is unloaded.
*/
if (dst->dev == NULL || !(dst->dev->flags_UP)) {
- dst->input = dst_discard_in;
- dst->output = dst_discard_out;
+ dst->input = dst->output = dst_discard;
}
dst->obsolete = 2;
}
@@ -242,8 +234,7 @@ static inline void dst_ifdown(struct dst_entry
*dst, struct net_device *dev,
return;
if (!unregister) {
- dst->input = dst_discard_in;
- dst->output = dst_discard_out;
+ dst->input = dst->output = dst_discard;
} else {
dst->dev = _dev;
dev_hold(_dev);
--
Denis Cheng
Linux Application Developer
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] merge dst_discard in out into one, removed a duplicate function
On 5/28/07, Jan Engelhardt [EMAIL PROTECTED] wrote:
Uhm, just replace every invocation of dst_discard_in/_out() directly
by dst_discard ... don't add macros for that.
merge dst_discard in out into one, this removed a duplicate function.
Signed-off-by: Denis Cheng [EMAIL PROTECTED]
---
Is there anybody also found this duplicate? I found this duplcate had
existed from linux-2.2 linux-2.4 to the latest kernel tree, you could
check it in the lxr site conveniently:
http://lxr.linux.no/source/net/core/dst.c?v=2.2.26
http://lxr.linux.no/source/net/core/dst.c?v=2.4.18
http://lxr.linux.no/source/net/core/dst.c
With little difference, the 2.2 and 2.4 version use two different
function name: dst_discard and dst_blackhole, but the definition of
them are the same, so they are same substantially. Additionally, the
two same function are two local(static) functions in a single file
dst.c, I don't know why there should exist two same functions. I
wonder what the consideration of this design is, because I'm just a
kernel newbie; this problem has been confusing me for many days, so if
you know why, please drop me some directions.
diff --git a/net/core/dst.c b/net/core/dst.c
index 764bccb..c6a0587 100644
--- a/net/core/dst.c
+++ b/net/core/dst.c
@@ -111,13 +111,7 @@ out:
spin_unlock(dst_lock);
}
-static int dst_discard_in(struct sk_buff *skb)
-{
- kfree_skb(skb);
- return 0;
-}
-
-static int dst_discard_out(struct sk_buff *skb)
+static int dst_discard(struct sk_buff *skb)
{
kfree_skb(skb);
return 0;
@@ -138,8 +132,7 @@ void * dst_alloc(struct dst_ops * ops)
dst-ops = ops;
dst-lastuse = jiffies;
dst-path = dst;
- dst-input = dst_discard_in;
- dst-output = dst_discard_out;
+ dst-input = dst-output = dst_discard;
#if RT_CACHE_DEBUG = 2
atomic_inc(dst_total);
#endif
@@ -153,8 +146,7 @@ static void ___dst_free(struct dst_entry * dst)
protocol module is unloaded.
*/
if (dst-dev == NULL || !(dst-dev-flagsIFF_UP)) {
- dst-input = dst_discard_in;
- dst-output = dst_discard_out;
+ dst-input = dst-output = dst_discard;
}
dst-obsolete = 2;
}
@@ -242,8 +234,7 @@ static inline void dst_ifdown(struct dst_entry
*dst, struct net_device *dev,
return;
if (!unregister) {
- dst-input = dst_discard_in;
- dst-output = dst_discard_out;
+ dst-input = dst-output = dst_discard;
} else {
dst-dev = loopback_dev;
dev_hold(loopback_dev);
--
Denis Cheng
Linux Application Developer
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] merge dst_discard in & out into one, this decrements the vmlinux image by 21 bytes under i386 arch.
On 5/28/07, Jan Engelhardt <[EMAIL PROTECTED]> wrote:
Uhm, just replace every invocation of dst_discard_in/_out() directly
by dst_discard ... don't add macros for that.
so that, the trival patch changed to this:
because the definition of dst_discard_in and dst_discard_out are the
same, so they merged into one.
Signed-off-by: Denis Cheng <[EMAIL PROTECTED]>
---
diff --git a/net/core/dst.c b/net/core/dst.c
index 764bccb..c6a0587 100644
--- a/net/core/dst.c
+++ b/net/core/dst.c
@@ -111,13 +111,7 @@ out:
spin_unlock(_lock);
}
-static int dst_discard_in(struct sk_buff *skb)
-{
- kfree_skb(skb);
- return 0;
-}
-
-static int dst_discard_out(struct sk_buff *skb)
+static int dst_discard(struct sk_buff *skb)
{
kfree_skb(skb);
return 0;
@@ -138,8 +132,7 @@ void * dst_alloc(struct dst_ops * ops)
dst->ops = ops;
dst->lastuse = jiffies;
dst->path = dst;
- dst->input = dst_discard_in;
- dst->output = dst_discard_out;
+ dst->input = dst->output = dst_discard;
#if RT_CACHE_DEBUG >= 2
atomic_inc(_total);
#endif
@@ -153,8 +146,7 @@ static void ___dst_free(struct dst_entry * dst)
protocol module is unloaded.
*/
if (dst->dev == NULL || !(dst->dev->flags_UP)) {
- dst->input = dst_discard_in;
- dst->output = dst_discard_out;
+ dst->input = dst->output = dst_discard;
}
dst->obsolete = 2;
}
@@ -242,8 +234,7 @@ static inline void dst_ifdown(struct dst_entry
*dst, struct net_device *dev,
return;
if (!unregister) {
- dst->input = dst_discard_in;
- dst->output = dst_discard_out;
+ dst->input = dst->output = dst_discard;
} else {
dst->dev = _dev;
dev_hold(_dev);
--
Denis Cheng
Linux Application Developer
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] merge dst_discard in out into one, this decrements the vmlinux image by 21 bytes under i386 arch.
On 5/28/07, Jan Engelhardt [EMAIL PROTECTED] wrote:
Uhm, just replace every invocation of dst_discard_in/_out() directly
by dst_discard ... don't add macros for that.
so that, the trival patch changed to this:
because the definition of dst_discard_in and dst_discard_out are the
same, so they merged into one.
Signed-off-by: Denis Cheng [EMAIL PROTECTED]
---
diff --git a/net/core/dst.c b/net/core/dst.c
index 764bccb..c6a0587 100644
--- a/net/core/dst.c
+++ b/net/core/dst.c
@@ -111,13 +111,7 @@ out:
spin_unlock(dst_lock);
}
-static int dst_discard_in(struct sk_buff *skb)
-{
- kfree_skb(skb);
- return 0;
-}
-
-static int dst_discard_out(struct sk_buff *skb)
+static int dst_discard(struct sk_buff *skb)
{
kfree_skb(skb);
return 0;
@@ -138,8 +132,7 @@ void * dst_alloc(struct dst_ops * ops)
dst-ops = ops;
dst-lastuse = jiffies;
dst-path = dst;
- dst-input = dst_discard_in;
- dst-output = dst_discard_out;
+ dst-input = dst-output = dst_discard;
#if RT_CACHE_DEBUG = 2
atomic_inc(dst_total);
#endif
@@ -153,8 +146,7 @@ static void ___dst_free(struct dst_entry * dst)
protocol module is unloaded.
*/
if (dst-dev == NULL || !(dst-dev-flagsIFF_UP)) {
- dst-input = dst_discard_in;
- dst-output = dst_discard_out;
+ dst-input = dst-output = dst_discard;
}
dst-obsolete = 2;
}
@@ -242,8 +234,7 @@ static inline void dst_ifdown(struct dst_entry
*dst, struct net_device *dev,
return;
if (!unregister) {
- dst-input = dst_discard_in;
- dst-output = dst_discard_out;
+ dst-input = dst-output = dst_discard;
} else {
dst-dev = loopback_dev;
dev_hold(loopback_dev);
--
Denis Cheng
Linux Application Developer
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] merge dst_discard in & out into one, this decrements the vmlinux image by 21 bytes under i386 arch.
From: Denis Cheng
thus the definition of dst_discard_in and dst_discard_out is the same,
we can define a dst_discard function and map the _in and _out to it,
this can reduce space in vmlinux.
Signed-off-by: Denis Cheng <[EMAIL PROTECTED]>
---
diff --git a/net/core/dst.c b/net/core/dst.c
index 764bccb..daa0439 100644
--- a/net/core/dst.c
+++ b/net/core/dst.c
@@ -111,17 +111,14 @@ out:
spin_unlock(_lock);
}
-static int dst_discard_in(struct sk_buff *skb)
+static int dst_discard(struct sk_buff *skb)
{
kfree_skb(skb);
return 0;
}
-static int dst_discard_out(struct sk_buff *skb)
-{
- kfree_skb(skb);
- return 0;
-}
+#define dst_discard_in dst_discard
+#define dst_discard_outdst_discard
void * dst_alloc(struct dst_ops * ops)
{
--
Denis Cheng
Linux Application Developer
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Block device elevator: use list_for_each_entry() instead of list_for_each()
it makes sense to what it does.
On 5/28/07, Matthias Kaehlcke <[EMAIL PROTECTED]> wrote:
Use list_for_each_entry() instead of list_for_each() in the block
device elevator
Signed-off-by: Matthias Kaehlcke <[EMAIL PROTECTED]>
--
diff --git a/block/elevator.c b/block/elevator.c
index ce866eb..4769a25 100644
--- a/block/elevator.c
+++ b/block/elevator.c
@@ -112,12 +112,8 @@ static inline int elv_try_merge(struct request *__rq,
struct bio *bio)
static struct elevator_type *elevator_find(const char *name)
{
struct elevator_type *e;
- struct list_head *entry;
-
- list_for_each(entry, _list) {
-
- e = list_entry(entry, struct elevator_type, list);
+ list_for_each_entry(e, _list, list) {
if (!strcmp(e->elevator_name, name))
return e;
}
@@ -1116,14 +1112,11 @@ ssize_t elv_iosched_show(request_queue_t *q, char *name)
{
elevator_t *e = q->elevator;
struct elevator_type *elv = e->elevator_type;
- struct list_head *entry;
+ struct elevator_type *__e;
int len = 0;
spin_lock(_list_lock);
- list_for_each(entry, _list) {
- struct elevator_type *__e;
-
- __e = list_entry(entry, struct elevator_type, list);
+ list_for_each_entry(__e, _list, list) {
if (!strcmp(elv->elevator_name, __e->elevator_name))
len += sprintf(name+len, "[%s] ", elv->elevator_name);
else
--
Matthias Kaehlcke
Linux Application Developer
Barcelona
Usually when people are sad, they don't do anything. They just cry over
their condition. But when they get angry, they bring about a change
(Malcolm X)
.''`.
using free software / Debian GNU/Linux | http://debian.org : :' :
`. `'`
gpg --keyserver pgp.mit.edu --recv-keys 47D8E5D4 `-
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
--
Denis Cheng
Linux Application Developer
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Block device elevator: use list_for_each_entry() instead of list_for_each()
it makes sense to what it does.
On 5/28/07, Matthias Kaehlcke [EMAIL PROTECTED] wrote:
Use list_for_each_entry() instead of list_for_each() in the block
device elevator
Signed-off-by: Matthias Kaehlcke [EMAIL PROTECTED]
--
diff --git a/block/elevator.c b/block/elevator.c
index ce866eb..4769a25 100644
--- a/block/elevator.c
+++ b/block/elevator.c
@@ -112,12 +112,8 @@ static inline int elv_try_merge(struct request *__rq,
struct bio *bio)
static struct elevator_type *elevator_find(const char *name)
{
struct elevator_type *e;
- struct list_head *entry;
-
- list_for_each(entry, elv_list) {
-
- e = list_entry(entry, struct elevator_type, list);
+ list_for_each_entry(e, elv_list, list) {
if (!strcmp(e-elevator_name, name))
return e;
}
@@ -1116,14 +1112,11 @@ ssize_t elv_iosched_show(request_queue_t *q, char *name)
{
elevator_t *e = q-elevator;
struct elevator_type *elv = e-elevator_type;
- struct list_head *entry;
+ struct elevator_type *__e;
int len = 0;
spin_lock(elv_list_lock);
- list_for_each(entry, elv_list) {
- struct elevator_type *__e;
-
- __e = list_entry(entry, struct elevator_type, list);
+ list_for_each_entry(__e, elv_list, list) {
if (!strcmp(elv-elevator_name, __e-elevator_name))
len += sprintf(name+len, [%s] , elv-elevator_name);
else
--
Matthias Kaehlcke
Linux Application Developer
Barcelona
Usually when people are sad, they don't do anything. They just cry over
their condition. But when they get angry, they bring about a change
(Malcolm X)
.''`.
using free software / Debian GNU/Linux | http://debian.org : :' :
`. `'`
gpg --keyserver pgp.mit.edu --recv-keys 47D8E5D4 `-
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
--
Denis Cheng
Linux Application Developer
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] merge dst_discard in out into one, this decrements the vmlinux image by 21 bytes under i386 arch.
From: Denis Cheng
thus the definition of dst_discard_in and dst_discard_out is the same,
we can define a dst_discard function and map the _in and _out to it,
this can reduce space in vmlinux.
Signed-off-by: Denis Cheng [EMAIL PROTECTED]
---
diff --git a/net/core/dst.c b/net/core/dst.c
index 764bccb..daa0439 100644
--- a/net/core/dst.c
+++ b/net/core/dst.c
@@ -111,17 +111,14 @@ out:
spin_unlock(dst_lock);
}
-static int dst_discard_in(struct sk_buff *skb)
+static int dst_discard(struct sk_buff *skb)
{
kfree_skb(skb);
return 0;
}
-static int dst_discard_out(struct sk_buff *skb)
-{
- kfree_skb(skb);
- return 0;
-}
+#define dst_discard_in dst_discard
+#define dst_discard_outdst_discard
void * dst_alloc(struct dst_ops * ops)
{
--
Denis Cheng
Linux Application Developer
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
