You can subscribe to this list here.
| 2009 |
Jan
|
Feb
|
Mar
|
Apr
|
May
(32) |
Jun
(66) |
Jul
(102) |
Aug
(78) |
Sep
(106) |
Oct
(137) |
Nov
(147) |
Dec
(147) |
|---|---|---|---|---|---|---|---|---|---|---|---|---|
| 2010 |
Jan
(71) |
Feb
(139) |
Mar
(86) |
Apr
(76) |
May
(57) |
Jun
(10) |
Jul
(12) |
Aug
(6) |
Sep
(8) |
Oct
(12) |
Nov
(12) |
Dec
(18) |
| 2011 |
Jan
(16) |
Feb
(19) |
Mar
(3) |
Apr
(1) |
May
(16) |
Jun
(17) |
Jul
(74) |
Aug
(22) |
Sep
(18) |
Oct
(24) |
Nov
(21) |
Dec
(30) |
| 2012 |
Jan
(31) |
Feb
(16) |
Mar
(22) |
Apr
(25) |
May
(18) |
Jun
(13) |
Jul
(83) |
Aug
(49) |
Sep
(20) |
Oct
(60) |
Nov
(35) |
Dec
(28) |
| 2013 |
Jan
(39) |
Feb
(61) |
Mar
(35) |
Apr
(21) |
May
(45) |
Jun
(56) |
Jul
(20) |
Aug
(9) |
Sep
(10) |
Oct
(31) |
Nov
(8) |
Dec
(4) |
| 2014 |
Jan
(6) |
Feb
(7) |
Mar
(7) |
Apr
(6) |
May
(4) |
Jun
(8) |
Jul
(5) |
Aug
(2) |
Sep
(4) |
Oct
(4) |
Nov
(11) |
Dec
(5) |
| 2015 |
Jan
(4) |
Feb
(4) |
Mar
(3) |
Apr
(4) |
May
(9) |
Jun
(4) |
Jul
(15) |
Aug
(8) |
Sep
(16) |
Oct
(18) |
Nov
(15) |
Dec
(7) |
| 2016 |
Jan
(20) |
Feb
(9) |
Mar
(15) |
Apr
(24) |
May
(16) |
Jun
(28) |
Jul
(22) |
Aug
(23) |
Sep
(18) |
Oct
(30) |
Nov
(40) |
Dec
(9) |
| 2017 |
Jan
(1) |
Feb
(8) |
Mar
(37) |
Apr
(26) |
May
(25) |
Jun
(46) |
Jul
(24) |
Aug
(9) |
Sep
|
Oct
|
Nov
|
Dec
|
|
From: Karel Z. <kz...@re...> - 2011-07-15 12:48:26
|
On Fri, Jul 15, 2011 at 03:55:02PM +0900, Nao Nishijima wrote:
> > If there is not /dev/foo and /sys/block/foo then the patch introduces
> > a REGRESSION.
> >
> > The names from /proc/partitions are used in many applications
> > (libblkid, fdisk, ...) for many many years. The applications will not
> > work as expected.
> >
>
> I think that it is not a regression when users do not set an alias name.
> Of course, I am going to modify all these utils so that users can use
> alias names.
>
> The purpose of alias names is to unify the name of device which users
> operate and see. Therefore, I think that users would like to get an
> alias name from it, because currently they get a device name form it.
>
> However, for who wants to use alias name only for dmesg, I have an
> enhancement idea that users can choose alias names or device names in
> procfs by sysctl (default is device names).
>From my point of view
dmesg | grep <alias>
seems a little bit problematic, because for example initial messages
for the device will be invisible. In the log will be messages with
original canonical names and another (later) messages with aliases, you
can also change the alias, etc. Seems like a mess. The log should be
consistent...
I think it would be better to use always only canonical names in the
kernel log and translate to something more user-friendly in userspace.
For example if you add some meta-information to the kernel log then we
can improve dmesg(1) to translate the canonical names to aliases.
printk(KERN_INFO "this is info about %{device}s", device);
<5>[105221.774534]{device=sda} this is info about sda
or whatever... (this is a nice topic with many colors for the bike
shed:-)
> I would like to hear your opinion about this.
If you want to modify all the userspace utils then you don't have to
modify /proc/partitions :-)
You can keep the standard names in /proc/partitions (so the file will
be compatible with /sys and /dev) and you can modify all the utils to
translate the canonical device names to aliases. The result will be
the same, except "cat /proc/partitions" -- but I think that instead of
"cat" you can use "lsblk".
If you add only the attribute (alias name) to the /sys then you don't
have to care if all the utils are already modified.
Anyway, I still agree with Kay and Greg -- the proper solution
is to improve printk() and dmesg(1). The aliases could be implemented
in userspace by udev /dev/disk/by-alias/<cool_name> symlinks.
Karel
--
Karel Zak <kz...@re...>
http://karelzak.blogspot.com
|
|
From: Nao N. <nao...@hi...> - 2011-07-15 06:55:25
|
Hi, karel (2011/07/09 4:45), Karel Zak wrote: > On Fri, Jul 08, 2011 at 05:45:47PM +0900, Nao Nishijima wrote: >> This patch series provides an "alias name" of the disk into kernel and procfs >> messages. The user can assign a preferred name to an alias name of the device. >> >> Based on previous discussion (*), I changed patches as follows >> - This is "alias name" >> - An "alias name" is stored in gendisk struct >> - Add document to Documentation/ABI/testing/sysfs-block >> - When the user changes an "alias name", kernel notifies udev >> >> (*) http://marc.info/?l=linux-scsi&m=130812625531219&w=2 >> > > [...] > >> [localhost]# cat /proc/partitions >> major minor #blocks name >> >> 8 0 12582912 foo >> 8 1 12582878 foo1 >> 8 0 8388608 sdb >> 8 1 512000 sdb1 >> 8 2 7875584 sdb2 > > If there is not /dev/foo and /sys/block/foo then the patch introduces > a REGRESSION. > > The names from /proc/partitions are used in many applications > (libblkid, fdisk, ...) for many many years. The applications will not > work as expected. > I think that it is not a regression when users do not set an alias name. Of course, I am going to modify all these utils so that users can use alias names. The purpose of alias names is to unify the name of device which users operate and see. Therefore, I think that users would like to get an alias name from it, because currently they get a device name form it. However, for who wants to use alias name only for dmesg, I have an enhancement idea that users can choose alias names or device names in procfs by sysctl (default is device names). I would like to hear your opinion about this. > It's crazy to assume that all the applications will be improved to > translate the "pretty name" from /proc/partitions by /sys/block/<maj>:<min>. > > Note, it's pretty naive to expect that people will use the pretty > names for printk()/dmesg only. I'm absolutely sure that it's will be > expected (by end-users) in /etc/fstab, mount, df, fdisk, ... > > It's will be necessary to modify all these utils. > Yes, I am going to modify all these utils to support udev persistent names(including alias name). I had looked over some commands where those get a device name from. Most of them get a device name from /proc/partitions, also other commands get it from /etc/fstab, /sys/block/ or own device database. I am going to modify those commands to get a device name from /dev/disk. > The patch is opening Pandora's box :-( > Best regards, -- Nao NISHIJIMA Software Platform Research Dept. Linux Technology Center Hitachi, Ltd., YOKOHAMA Research Laboratory Email: nao...@hi... |
|
From: Américo W. <xiy...@gm...> - 2011-07-13 08:37:22
|
2011/7/11 Seiji Aguchi <sei...@hd...>: > Hi, > Hi, > > [Patch Description] > > For meeting Eric/Vivek's requirements and solving issues of pstore/efivars driver, I propose a following patch. > > - Remove kmsg_dump(KMSG_DUMP_KEXEC) from crash_kexec() It is already removed in -mm, can you rebase your patch against -mm? > - Add kmsg_dump_kexec() to crash_kexec() > kmsg_dump_kexec() moved behind machine_crash_shutdown() so that > we can output kernel messages without getting any locks. > > - Introduce CONFIG_KMSG_DUMP_KEXEC > - CONFIG_KMSG_DUMP_KEXEC depends on CONFIG_PRINTK which is required for kmsg_dumper. > > - CONFIG_KMSG_DUMP_KEXEC=n (default) > Kernel message logging feature doesn't work in kdump path. > > - CONFIG_KMSG_DUMP_KEXEC=y > following static functions are called in kdump path. > - kmsg_dump_kexec(): This is based on kmsg_dump() and just removed spinlock and a function call chain from it. > - do_kmsg_dump_kexec(): This is based on pstore_dump() and just removed mutex lock from it. > > Some people who are not familiar with kexec may add function calls getting spin_lock/mutex_lock in kexec path. > These function calls causes failure of kexec. > So, I suggest replace a call chain with static function calls so that we can keep reliability of kexec. > > - Add efi_kmsg_dump_kexec() which outputs kernel messages into NVRAM with EFI. > This is called by do_kmsg_dump_kexec() statically. > By applying to Matthew Garret's patch below, its kernel messages in NVRAM are visible through /dev/pstore. This looks weird, kmsg_dump_kexec() calls do_kmsg_dump_kexec() which then calls efi_kmsg_dump_kexec(), so, why not just call efi_kmsg_dump_kexec()? since there is no other user of kmsg_dump_kexec(). >From the name CONFIG_KMSG_DUMP_KEXEC, you seem to provide a generic method, but actually only efi_kmsg_dump_kexec() is and can be called. Thanks. |
|
From: Aejota <roc...@ya...> - 2011-07-13 05:46:52
|
<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.0 Transitional//EN"> <HTML><HEAD> <META http-equiv=Content-Type content="text/html; charset=unicode"> <META content="MSHTML 6.00.2900.3268" name=GENERATOR></HEAD> <BODY> <DIV><FONT color=#ffffff>9o1T</FONT>我司<FONT color=#ffffff>Wm0R</FONT>可优<FONT color=#ffffff>9n1S</FONT>惠代<FONT color=#ffffff>n0</FONT>开各<FONT color=#ffffff>RhW</FONT>类[<FONT color=#ff0000>发</FONT><FONT color=#ffffff>Tj9</FONT><FONT color=#ffffff>P2</FONT></DIV> <DIV> </DIV> <DIV><FONT color=#ffffff>Q1<FONT color=#ff0000>剽</FONT></FONT><FONT color=#000000>], </FONT>验<FONT color=#ffffff>0RhWm</FONT>后付<FONT color=#ffffff>1Tj9O</FONT>款。欢<FONT color=#ffffff>Uk9</FONT>迎咨<FONT color=#ffffff>0Q2U</FONT>询! </DIV> <DIV> </DIV> <DIV><FONT color=#ffffff>Q2</FONT>电<FONT color=#ffffff>l0Q1Uj</FONT>话:①③⑤<FONT color=#ffffff> FK</FONT> ⑨0①⑨ <FONT color=#ffffff> MQW</FONT> ⑤①⑤③ </DIV> <DIV> </DIV> <DIV><FONT color=#ffffff> FJ</FONT> 联<FONT color=#ffffff>j9O1Ti</FONT>系人:<FONT color=#ffffff>1Si9n</FONT>陈先<FONT color=#ffffff>9O1S</FONT>生 </DIV> <DIV> </DIV> <DIV><FONT color=#ffffff>9n1S</FONT>Q号:⒈⒎⒊<FONT color=#ffffff> UZEKP</FONT> Ο⒊⒊<FONT color=#ffffff> HL</FONT> ⒍⒍⒈⒐ <FONT color=#ffffff>j9O1Si</FONT></DIV></BODY></HTML> |
|
From: Vivek G. <vg...@re...> - 2011-07-11 22:07:32
|
On Mon, Jul 11, 2011 at 11:47:17AM -0400, Seiji Aguchi wrote: > Hi, > > [Background] > - Requirement in enterprise area > From our support service experience, we always need to detect root causes of OS panic. > Because customers in enterprise area never forgive us if kdump fails and we can't detect > root causes of panic due to lack of materials for investigation. > > That is why I would like to add kmsg_dump() in kdump path. > > [Upstream status] > Discussion about kmsg_dump() in kdump path: > - Vivek and Eric are worried about reliability of existing kmsg_dump(). > - Especially, Vivek would like to remove a RCU function call chain in kdump path > which kernel modules can register their function calls freely. > > Development of a feature capturing Oops: > - Pstore has already incorporated in upstream kernel. > > - Matthew Garrett is developing efivars driver, a feature capturing Oops via EFI > through pstore interface. > > https://lkml.org/lkml/2011/6/7/437 > https://lkml.org/lkml/2011/6/7/438 > https://lkml.org/lkml/2011/6/7/439 > https://lkml.org/lkml/2011/6/7/440 > > - However, these features may not work in kdump path because > there are spin_lock/mutex_lock in pstore_dump()/efi_pstore_write(). > These locks may cause dead lock and kdump may fail as well. I think then right solution is to harden pstore() to be able to called from crashed kernel context where interrupts will not be working, all other cpus are not running and one can afford to rely on taking any locks. Bypassing that just to make sure case of efi work is not going to help. > > [Build Status] > Built this patch on 3.0-rc6 in x86_64. > > [Patch Description] > > For meeting Eric/Vivek's requirements and solving issues of pstore/efivars driver, I propose a following patch. > > - Remove kmsg_dump(KMSG_DUMP_KEXEC) from crash_kexec() > - Add kmsg_dump_kexec() to crash_kexec() > kmsg_dump_kexec() moved behind machine_crash_shutdown() so that > we can output kernel messages without getting any locks. So you are introducing two variants of kmsg dump now? One is kmsg_dump() and other is kmsg_dump_kexec()? Why not just get rid of kmsg_dump() for non panic case and simply always call in crashed kernel case? > > - Introduce CONFIG_KMSG_DUMP_KEXEC > - CONFIG_KMSG_DUMP_KEXEC depends on CONFIG_PRINTK which is required for kmsg_dumper. > > - CONFIG_KMSG_DUMP_KEXEC=n (default) > Kernel message logging feature doesn't work in kdump path. > > - CONFIG_KMSG_DUMP_KEXEC=y > following static functions are called in kdump path. > - kmsg_dump_kexec(): This is based on kmsg_dump() and just removed spinlock and a function call chain from it. > - do_kmsg_dump_kexec(): This is based on pstore_dump() and just removed mutex lock from it. > > Some people who are not familiar with kexec may add function calls getting spin_lock/mutex_lock in kexec path. > These function calls causes failure of kexec. > So, I suggest replace a call chain with static function calls so that we can keep reliability of kexec. > > - Add efi_kmsg_dump_kexec() which outputs kernel messages into NVRAM with EFI. > This is called by do_kmsg_dump_kexec() statically. > By applying to Matthew Garret's patch below, its kernel messages in NVRAM are visible through /dev/pstore. > > https://lkml.org/lkml/2011/6/7/437 > https://lkml.org/lkml/2011/6/7/438 > https://lkml.org/lkml/2011/6/7/439 > https://lkml.org/lkml/2011/6/7/440 > - So with this patch you have removed all other subscribers of kmsg_dump() and only calling your case of efi storage (ramoops, mtdoops?) Thanks Vivek |
|
From: Matthew G. <mj...@re...> - 2011-07-11 17:28:15
|
On Mon, Jul 11, 2011 at 11:47:17AM -0400, Seiji Aguchi wrote: > + > + efi_kmsg_dump_kexec(0, part, hsize + l1_cpy + l2_cpy, > + kmsg_dump_kexec_buf); What if we're using ERST as the storage backend? -- Matthew Garrett | mj...@sr... |
|
From: Seiji A. <sei...@hd...> - 2011-07-11 15:48:40
|
Hi,
[Background]
- Requirement in enterprise area
From our support service experience, we always need to detect root causes of OS panic.
Because customers in enterprise area never forgive us if kdump fails and we can't detect
root causes of panic due to lack of materials for investigation.
That is why I would like to add kmsg_dump() in kdump path.
[Upstream status]
Discussion about kmsg_dump() in kdump path:
- Vivek and Eric are worried about reliability of existing kmsg_dump().
- Especially, Vivek would like to remove a RCU function call chain in kdump path
which kernel modules can register their function calls freely.
Development of a feature capturing Oops:
- Pstore has already incorporated in upstream kernel.
- Matthew Garrett is developing efivars driver, a feature capturing Oops via EFI
through pstore interface.
https://lkml.org/lkml/2011/6/7/437
https://lkml.org/lkml/2011/6/7/438
https://lkml.org/lkml/2011/6/7/439
https://lkml.org/lkml/2011/6/7/440
- However, these features may not work in kdump path because
there are spin_lock/mutex_lock in pstore_dump()/efi_pstore_write().
These locks may cause dead lock and kdump may fail as well.
[Build Status]
Built this patch on 3.0-rc6 in x86_64.
[Patch Description]
For meeting Eric/Vivek's requirements and solving issues of pstore/efivars driver, I propose a following patch.
- Remove kmsg_dump(KMSG_DUMP_KEXEC) from crash_kexec()
- Add kmsg_dump_kexec() to crash_kexec()
kmsg_dump_kexec() moved behind machine_crash_shutdown() so that
we can output kernel messages without getting any locks.
- Introduce CONFIG_KMSG_DUMP_KEXEC
- CONFIG_KMSG_DUMP_KEXEC depends on CONFIG_PRINTK which is required for kmsg_dumper.
- CONFIG_KMSG_DUMP_KEXEC=n (default)
Kernel message logging feature doesn't work in kdump path.
- CONFIG_KMSG_DUMP_KEXEC=y
following static functions are called in kdump path.
- kmsg_dump_kexec(): This is based on kmsg_dump() and just removed spinlock and a function call chain from it.
- do_kmsg_dump_kexec(): This is based on pstore_dump() and just removed mutex lock from it.
Some people who are not familiar with kexec may add function calls getting spin_lock/mutex_lock in kexec path.
These function calls causes failure of kexec.
So, I suggest replace a call chain with static function calls so that we can keep reliability of kexec.
- Add efi_kmsg_dump_kexec() which outputs kernel messages into NVRAM with EFI.
This is called by do_kmsg_dump_kexec() statically.
By applying to Matthew Garret's patch below, its kernel messages in NVRAM are visible through /dev/pstore.
https://lkml.org/lkml/2011/6/7/437
https://lkml.org/lkml/2011/6/7/438
https://lkml.org/lkml/2011/6/7/439
https://lkml.org/lkml/2011/6/7/440
[Test Status]
This patch is tested with lkdtm (Linux Kernel Dump Test Module) on the following environment for proving its reliability.
lkdtm:
http://lwn.net/Articles/198690/
Hardware:
Server: Dell PowerEdge T310
number of logical cpus: 4
Memory: 4GB
The results are followings.
As you can see below, kdump never fails due to this patch.
-----------------------------------------------------------
cpoint_name |cpoint_type|kdump | capture Oops via EFI|
-----------------------------------------------------------
INT_HARDWARE_ENTRY|PANIC | O | O |
|BUG | O | O |
|EXCEPTION | O | O |
|LOOP | O | O |
|OVERFLOW | X | X |
-----------------------------------------------------------
INT_TASKLET_ENTRY |PANIC | O | O |
|BUG | O | O |
|EXCEPTION | O | O |
|LOOP | O | O |
|OVERFLOW | O | O |
-----------------------------------------------------------
FS_DEVRW |PANIC | O | O |
|BUG | O | O |
|EXCEPTION | O | O |
|LOOP | O | O |
|OVERFLOW | O | O |
-----------------------------------------------------------
MEM_SWAPOUT |PANIC | O | O |
|BUG | O | O |
|EXCEPTION | O | O |
|LOOP | O | O |
|OVERFLOW | X | X |
-----------------------------------------------------------
TIMERADD |PANIC | O | O |
|BUG | O | O |
|EXCEPTION | O | O |
|LOOP | O | O |
|OVERFLOW | O | O |
-----------------------------------------------------------
SCSI_DISPATCH_CMD |PANIC | O | O |
|BUG | O | O |
|EXCEPTION | O | O |
|LOOP | O | O |
|OVERFLOW | O | O |
-----------------------------------------------------------
IDE_CORE_CP |PANIC | O | O |
|BUG | O | O |
|EXCEPTION | O | O |
|LOOP | O | O |
|OVERFLOW | O | O |
-----------------------------------------------------------
O:Success
X:Fail (Hard reset happened before calling crash_kexec())
(*)I couldn't test INT_HW_IRQ_EN because loading lkdtm modules fails.
Signed-off-by: Seiji Aguchi <sei...@hd...>
---
arch/x86/platform/efi/efi.c | 99 +++++++++++++++++++++++++++++++++++++++++++
include/linux/efi.h | 5 ++
include/linux/kmsg_dump.h | 8 ++++
init/Kconfig | 6 +++
kernel/kexec.c | 3 +-
kernel/printk.c | 82 +++++++++++++++++++++++++++++++++++
6 files changed, 201 insertions(+), 2 deletions(-)
diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c index 474356b..1dd9800 100644
--- a/arch/x86/platform/efi/efi.c
+++ b/arch/x86/platform/efi/efi.c
@@ -69,6 +69,16 @@ early_param("noefi", setup_noefi); int add_efi_memmap; EXPORT_SYMBOL(add_efi_memmap);
+#define DUMP_NAME_LEN 52
+#define VARIABLE_NAME_LEN 1024
+efi_char16_t variable_name[VARIABLE_NAME_LEN]; efi_guid_t
+linux_efi_crash_guid = LINUX_EFI_CRASH_GUID;
+
+#define KMSG_DUMP_ATTRIBUTE \
+ (EFI_VARIABLE_NON_VOLATILE | \
+ EFI_VARIABLE_BOOTSERVICE_ACCESS | \
+ EFI_VARIABLE_RUNTIME_ACCESS)
+
static int __init setup_add_efi_memmap(char *arg) {
add_efi_memmap = 1;
@@ -711,3 +721,92 @@ u64 efi_mem_attributes(unsigned long phys_addr)
}
return 0;
}
+
+static inline int
+utf16_strncmp(const efi_char16_t *a, const efi_char16_t *b, size_t len)
+{
+ while (1) {
+ if (len == 0)
+ return 0;
+ if (*a < *b)
+ return -1;
+ if (*a > *b)
+ return 1;
+ if (*a == 0) /* implies *b == 0 */
+ return 0;
+ a++;
+ b++;
+ len--;
+ }
+}
+
+static unsigned long
+utf16_strnlen(efi_char16_t *s, size_t maxlength) {
+ unsigned long length = 0;
+
+ while (*s++ != 0 && length < maxlength)
+ length++;
+ return length;
+}
+
+
+static unsigned long
+utf16_strlen(efi_char16_t *s)
+{
+ return utf16_strnlen(s, ~0UL);
+}
+
+u64 efi_kmsg_dump_kexec(int type, int part, size_t size, char *buf) {
+ char name[DUMP_NAME_LEN];
+ char stub_name[DUMP_NAME_LEN];
+ efi_char16_t efi_name[DUMP_NAME_LEN];
+ efi_guid_t vendor_guid;
+ efi_status_t status;
+ unsigned long variable_name_size;
+ int i;
+
+ if (!efi_enabled)
+ return 0;
+
+ /* Don't get lock */
+
+ memset(variable_name, 0, sizeof(variable_name));
+
+ snprintf(stub_name, sizeof(stub_name), "dump-type%u-%u-", type, part);
+ snprintf(name, sizeof(name), "%s%lu", stub_name, get_seconds());
+
+ for (i = 0; i < DUMP_NAME_LEN; i++)
+ efi_name[i] = stub_name[i];
+
+ /*
+ * Clean up any entries with the same name
+ */
+
+ do {
+ variable_name_size = VARIABLE_NAME_LEN;
+ status = efi.get_next_variable(&variable_name_size,
+ variable_name, &vendor_guid);
+ if (status == EFI_SUCCESS) {
+ if (efi_guidcmp(vendor_guid, linux_efi_crash_guid))
+ continue;
+
+ if (utf16_strncmp(efi_name, variable_name,
+ utf16_strlen(efi_name)))
+ continue;
+
+ efi.set_variable(variable_name, &linux_efi_crash_guid,
+ KMSG_DUMP_ATTRIBUTE, 0, NULL);
+ }
+ } while (status != EFI_NOT_FOUND);
+
+ for (i = 0; i < DUMP_NAME_LEN; i++)
+ efi_name[i] = name[i];
+
+ efi.set_variable(efi_name, &linux_efi_crash_guid, KMSG_DUMP_ATTRIBUTE,
+ size, buf);
+
+ return part;
+}
+
diff --git a/include/linux/efi.h b/include/linux/efi.h index e376270..b5f8944 100644
--- a/include/linux/efi.h
+++ b/include/linux/efi.h
@@ -211,6 +211,9 @@ typedef efi_status_t efi_set_virtual_address_map_t (unsigned long memory_map_siz #define UV_SYSTEM_TABLE_GUID \
EFI_GUID( 0x3b13a7d4, 0x633e, 0x11dd, 0x93, 0xec, 0xda, 0x25, 0x56, 0xd8, 0x95, 0x93 )
+#define LINUX_EFI_CRASH_GUID \
+ EFI_GUID( 0xcfc8fc79, 0xbe2e, 0x4ddc, 0x97, 0xf0, 0x9f, 0x98,
+0xbf, 0xe2, 0x98, 0xa0 )
+
typedef struct {
efi_guid_t guid;
unsigned long table;
@@ -398,6 +401,8 @@ static inline void memrange_efi_to_native(u64 *addr, u64 *npages)
*addr &= PAGE_MASK;
}
+extern u64 efi_kmsg_dump_kexec(int type, int part, size_t size, char
+*buf);
+
#if defined(CONFIG_EFI_VARS) || defined(CONFIG_EFI_VARS_MODULE)
/*
* EFI Variable support.
diff --git a/include/linux/kmsg_dump.h b/include/linux/kmsg_dump.h index ee0c952..d8f07e5 100644
--- a/include/linux/kmsg_dump.h
+++ b/include/linux/kmsg_dump.h
@@ -63,4 +63,12 @@ static inline int kmsg_dump_unregister(struct kmsg_dumper *dumper) } #endif
+#ifdef CONFIG_KMSG_DUMP_KEXEC
+void kmsg_dump_kexec(void);
+#else
+static inline void kmsg_dump_kexec(void) { } #endif /*
+CONFIG_KMSG_DUMP_KEXEC */
+
#endif /* _LINUX_KMSG_DUMP_H */
diff --git a/init/Kconfig b/init/Kconfig index 412c21b..8c410b3 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -991,6 +991,12 @@ config PRINTK
very difficult to diagnose system problems, saying N here is
strongly discouraged.
+config KMSG_DUMP_KEXEC
+ bool "Enable support for kmsg_dumper in kexec path"
+ depends on PRINTK
+ help
+ This option enables kmsg_dumper in kexec path.
+
config BUG
bool "BUG() support" if EXPERT
default y
diff --git a/kernel/kexec.c b/kernel/kexec.c index 8d814cb..48cc64e 100644
--- a/kernel/kexec.c
+++ b/kernel/kexec.c
@@ -1079,11 +1079,10 @@ void crash_kexec(struct pt_regs *regs)
if (kexec_crash_image) {
struct pt_regs fixed_regs;
- kmsg_dump(KMSG_DUMP_KEXEC);
-
crash_setup_regs(&fixed_regs, regs);
crash_save_vmcoreinfo();
machine_crash_shutdown(&fixed_regs);
+ kmsg_dump_kexec();
machine_kexec(kexec_crash_image);
}
mutex_unlock(&kexec_mutex);
diff --git a/kernel/printk.c b/kernel/printk.c index 3518539..34983b6 100644
--- a/kernel/printk.c
+++ b/kernel/printk.c
@@ -41,6 +41,7 @@
#include <linux/cpu.h>
#include <linux/notifier.h>
#include <linux/rculist.h>
+#include <linux/efi.h>
#include <asm/uaccess.h>
@@ -1735,4 +1736,85 @@ void kmsg_dump(enum kmsg_dump_reason reason)
dumper->dump(dumper, reason, s1, l1, s2, l2);
rcu_read_unlock();
}
+
+#ifdef CONFIG_KMSG_DUMP_KEXEC
+#define KMSG_DUMP_KEXEC_BUF 128
+char kmsg_dump_kexec_buf[KMSG_DUMP_KEXEC_BUF];
+static unsigned long kmsg_bytes = 10240;
+
+static void do_kmsg_dump_kexec(const char *s1, unsigned long l1, const char *s2,
+ unsigned long l2)
+{
+ unsigned long s1_start, s2_start;
+ unsigned long l1_cpy, l2_cpy;
+ unsigned long size, total = 0;
+ char *dst, *why;
+ int hsize, part = 1;
+ int oops_count;
+
+ why = "Kexec";
+
+ /* Don't get lock */
+
+ oops_count = 1;
+ while (total < kmsg_bytes) {
+ dst = kmsg_dump_kexec_buf;
+ hsize = sprintf(dst, "%s#%d Part%d\n", why, oops_count,
+ part);
+ size = KMSG_DUMP_KEXEC_BUF - hsize;
+ dst += hsize;
+
+ l2_cpy = min(l2, size);
+ l1_cpy = min(l1, size - l2_cpy);
+
+ if (l1_cpy + l2_cpy == 0)
+ break;
+
+ s2_start = l2 - l2_cpy;
+ s1_start = l1 - l1_cpy;
+
+ memcpy(dst, s1 + s1_start, l1_cpy);
+ memcpy(dst + l1_cpy, s2 + s2_start, l2_cpy);
+
+ efi_kmsg_dump_kexec(0, part, hsize + l1_cpy + l2_cpy,
+ kmsg_dump_kexec_buf);
+
+ l1 -= l1_cpy;
+ l2 -= l2_cpy;
+ total += l1_cpy + l2_cpy;
+ part++;
+ }
+}
+
+void kmsg_dump_kexec(void)
+{
+ unsigned long end;
+ unsigned chars;
+ const char *s1, *s2;
+ unsigned long l1, l2;
+
+ /* Theoretically, the log could move on after we do this, but
+ there's not a lot we can do about that. The new messages
+ will overwrite the start of what we dump. */
+ /* Don't get lock */
+ end = log_end & LOG_BUF_MASK;
+ chars = logged_chars;
+
+ if (chars > end) {
+ s1 = log_buf + log_buf_len - chars + end;
+ l1 = chars - end;
+
+ s2 = log_buf;
+ l2 = end;
+ } else {
+ s1 = "";
+ l1 = 0;
+
+ s2 = log_buf + end - chars;
+ l2 = chars;
+ }
+ do_kmsg_dump_kexec(s1, 11, s2, l2);
+
+}
+#endif /* CONFIG_KMSG_DUMP_KEXEC */
#endif
--
1.7.1
|
|
From: Hannes R. <ha...@su...> - 2011-07-11 11:47:33
|
On 07/08/2011 06:38 PM, Kay Sievers wrote:
> On Fri, Jul 8, 2011 at 18:15, Kay Sievers<kay...@vr...> wrote:
>> On Fri, Jul 8, 2011 at 17:54, James Bottomley
>
>>> But hey,
>>> you have the enthusiasm, propose it as a KS topic to get agreement that
>>> we should do it and what the format should be and we can go from there.
>>
>> Sure, I'll do that. If needed, I can even make half or a third of it
>> possible I guess.
>
> Submitted it a minute ago.
>
I'd be willing to working on the design here, as it ties in rather
neatly with my goal of updating the SCSI debugging facilities.
printk() is easy to write, but basically impossible to impose any
type of checking/format whatever.
My all-time favorite here:
printk(KERN_INFO "error 1\n");
At least it got a 'KERN_INFO' thrown in there ...
So first step would be to reach an agreement _what_ printk() et al
is supposed to convey to userland.
Informal only? Informal, but traceable to the origin?
Should the origin be the internal
structure/subsystem/device/whatever generating the error?
Should the origin be persistent across reboots?
And, tied to that, what the supposed audience for printk() is:
Programs? Syslog? Humans?
Once we have an agreement here we can then go about and code the
required pieces. But currently the main problem is that there are
different ideas of what printk() is supposed to do.
Cheers,
Hannes
--
Dr. Hannes Reinecke zSeries & Storage
ha...@su... +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)
|
|
From: Nao N. <nao...@hi...> - 2011-07-09 13:32:45
|
Hi, Joe
Thank you for looking at this patch.
Your patch sounds great. I will test the patch.
Best regards,
(2011/07/09 14:42), Joe Perches wrote:
> Reduce size of code and text of scmd_printk and sd_printk
> macros by converting to the macros to functions and using
> vsprintf extension %pV. This moves the code out-of-line
> and centralizes the code used to emit additional arguments.
>
> Save ~32KB of space in an x86 allyesconfig.
>
> $ size drivers/scsi/built-in.o*
> text data bss dec hex filename
> 5860439 135396 1393024 7388859 70bebb drivers/scsi/built-in.o.new
> 5882368 135396 1395848 7413612 711f6c drivers/scsi/built-in.o.old
> 5887100 135396 1397264 7419760 713770 drivers/scsi/built-in.o.with_patch_1_and_2
>
> Signed-off-by: Joe Perches <jo...@pe...>
>
> ---
>
> Re: [RFC PATCH 2/4] sd: modify printk for alias_name
>
> On Fri, 2011-07-08 at 17:46 +0900, Nao Nishijima wrote:
>> This patch modify sd_printk() and scmd_printk() to use alias_name. If user set
>> an alias_name, those print an alias_name instead of a disk_name.
>
> Instead of larding more function/macros into these
> relatively heavily used logging macros, how about
> converting the logging macros into functions?
>
> drivers/scsi/scsi_lib.c | 26 ++++++++++++++++++++++++++
> drivers/scsi/sd.c | 23 +++++++++++++++++++++++
> drivers/scsi/sd.h | 8 +++-----
> include/scsi/scsi_device.h | 8 +++-----
> 4 files changed, 55 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
> index ec1803a..249c54c 100644
> --- a/drivers/scsi/scsi_lib.c
> +++ b/drivers/scsi/scsi_lib.c
> @@ -2571,3 +2571,29 @@ void scsi_kunmap_atomic_sg(void *virt)
> kunmap_atomic(virt, KM_BIO_SRC_IRQ);
> }
> EXPORT_SYMBOL(scsi_kunmap_atomic_sg);
> +
> +/* Logging utilities */
> +
> +int scmd_printk(const char *prefix, const struct scsi_cmnd *scmd,
> + const char *format, ...)
> +{
> + struct va_format vaf;
> + va_list args;
> + int r;
> +
> + va_start(args, format);
> +
> + vaf.fmt = format;
> + vaf.va = &args;
> +
> + if (scmd->request->rq_disk)
> + r = sdev_printk(prefix, scmd->device, "[%s] %pV",
> + alias_name(scmd->request->rq_disk), &vaf);
> + else
> + r = sdev_printk(prefix, scmd->device, "%pV", &vaf);
> +
> + va_end(args);
> +
> + return r;
> +}
> +EXPORT_SYMBOL(scmd_printk);
> diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
> index 953773c..2251e01 100644
> --- a/drivers/scsi/sd.c
> +++ b/drivers/scsi/sd.c
> @@ -2888,3 +2888,26 @@ static void sd_print_result(struct scsi_disk *sdkp, int result)
> scsi_show_result(result);
> }
>
> +int sd_printk(const char *prefix, const struct scsi_disk *sdsk,
> + const char *format, ...)
> +{
> + struct va_format vaf;
> + va_list args;
> + int r;
> +
> + va_start(args, format);
> +
> + vaf.fmt = format;
> + vaf.va = &args;
> +
> + if (sdsk->disk)
> + r = sdev_printk(prefix, sdsk->device, "[%s] %pV",
> + alias_name(sdsk->disk), &vaf);
> + else
> + r = sdev_printk(prefix, sdsk->device, "%pV", &vaf);
> +
> + va_end(args);
> +
> + return r;
> +}
> +EXPORT_SYMBOL(sd_printk);
> diff --git a/drivers/scsi/sd.h b/drivers/scsi/sd.h
> index 6ad798b..46aa748 100644
> --- a/drivers/scsi/sd.h
> +++ b/drivers/scsi/sd.h
> @@ -88,11 +88,9 @@ static inline struct scsi_disk *scsi_disk(struct gendisk *disk)
> return container_of(disk->private_data, struct scsi_disk, driver);
> }
>
> -#define sd_printk(prefix, sdsk, fmt, a...) \
> - (sdsk)->disk ? \
> - sdev_printk(prefix, (sdsk)->device, "[%s] " fmt, \
> - (sdsk)->disk->disk_name, ##a) : \
> - sdev_printk(prefix, (sdsk)->device, fmt, ##a)
> +extern __attribute__((format (printf, 3, 4)))
> +int sd_printk(const char *prefix, const struct scsi_disk *sdsk,
> + const char *format, ...);
>
> /*
> * A DIF-capable target device can be formatted with different
> diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h
> index dd82e02..c79631b 100644
> --- a/include/scsi/scsi_device.h
> +++ b/include/scsi/scsi_device.h
> @@ -216,11 +216,9 @@ struct scsi_dh_data {
> #define sdev_printk(prefix, sdev, fmt, a...) \
> dev_printk(prefix, &(sdev)->sdev_gendev, fmt, ##a)
>
> -#define scmd_printk(prefix, scmd, fmt, a...) \
> - (scmd)->request->rq_disk ? \
> - sdev_printk(prefix, (scmd)->device, "[%s] " fmt, \
> - (scmd)->request->rq_disk->disk_name, ##a) : \
> - sdev_printk(prefix, (scmd)->device, fmt, ##a)
> +extern __attribute__((format (printf, 3, 4)))
> +int scmd_printk(const char *prefix, const struct scsi_cmnd *scmd,
> + const char *format, ...);
>
> enum scsi_target_state {
> STARGET_CREATED = 1,
--
Nao NISHIJIMA
Software Platform Research Dept. Linux Technology Center
Hitachi, Ltd., YOKOHAMA Research Laboratory
Email: nao...@hi...
|
|
From: Masami H. <mas...@hi...> - 2011-07-09 06:12:05
|
(2011/07/09 1:15), Kay Sievers wrote: > On Fri, Jul 8, 2011 at 17:54, James Bottomley > <Jam...@ha...> wrote: > >> Yes, we did. Everyone agrees structured logging would be the best long >> term solution. However, it's at least 10x the work presented here, plus >> it would be a long process getting everyone to agree. > > Maybe even 100 times :) > >> This looks like a >> good 95% interim solution and it can be removed when structured logging >> makes everything "just work(tm)". > > I don't really see that. I think it does not add any real value on top > of a simple udev logging at device discovery. > > You would see that in the log: > [78441.742634]: udevd: sdc: \ > disk/by-id/ata-INTEL_SSDSA1M080G2GN_CVPO0363030V080EGN \ > disk/by-id/scsi-SATA_INTEL_SSDSA1M08CVPO0363030V080EGN \ > disk/by-path/pci-0000:00:1f.2-scsi-0:0:0:0 \ > disk/by-id/wwn-0x50015179593f3038 > > and know exactly _all_ names and all identifiers of the disk at that time. > > Then you see this: > [78441.742634] storage: really bad things happened to: sdc > > and with the earlier message we have all we need to know without any > pretty-name hacks. Hmm, yes, we CAN do that, if user carefully sets the udev log-level high, and saves udev log every boot or device change. I think the main point of this attempt is to reduce the cost of those operation, as like as users have done with old un*x way. If we can use a same device name for same device and can see the same name on the kernel log, we don't need to pay additional cost for searching and picking correct entry from udev log. Of course, it needs updates on some tools too. > All that can be done today already with a single udev rule, even on > many years old distros. > >> I have also seen a couple of other attempts at structured logging which >> both failed when the people proposing the patches realised how much work >> it actually was, so I'm a bit sceptical we'll ever get there. > > The more paper-over we add the more unlikely it gets. That's what I fear. :) > >> But hey, >> you have the enthusiasm, propose it as a KS topic to get agreement that >> we should do it and what the format should be and we can go from there. > > Sure, I'll do that. If needed, I can even make half or a third of it > possible I guess. BTW, I'm also interested in that structured error events, from the long term view and viewpoint of tracers :) I think we could expand current TRACE_EVENT macro to define those error events. Thank you, -- Masami HIRAMATSU Software Platform Research Dept. Linux Technology Center Hitachi, Ltd., Yokohama Research Laboratory E-mail: mas...@hi... |
|
From: Joe P. <jo...@pe...> - 2011-07-09 05:43:42
|
Reduce size of code and text of scmd_printk and sd_printk
macros by converting to the macros to functions and using
vsprintf extension %pV. This moves the code out-of-line
and centralizes the code used to emit additional arguments.
Save ~32KB of space in an x86 allyesconfig.
$ size drivers/scsi/built-in.o*
text data bss dec hex filename
5860439 135396 1393024 7388859 70bebb drivers/scsi/built-in.o.new
5882368 135396 1395848 7413612 711f6c drivers/scsi/built-in.o.old
5887100 135396 1397264 7419760 713770 drivers/scsi/built-in.o.with_patch_1_and_2
Signed-off-by: Joe Perches <jo...@pe...>
---
Re: [RFC PATCH 2/4] sd: modify printk for alias_name
On Fri, 2011-07-08 at 17:46 +0900, Nao Nishijima wrote:
> This patch modify sd_printk() and scmd_printk() to use alias_name. If user set
> an alias_name, those print an alias_name instead of a disk_name.
Instead of larding more function/macros into these
relatively heavily used logging macros, how about
converting the logging macros into functions?
drivers/scsi/scsi_lib.c | 26 ++++++++++++++++++++++++++
drivers/scsi/sd.c | 23 +++++++++++++++++++++++
drivers/scsi/sd.h | 8 +++-----
include/scsi/scsi_device.h | 8 +++-----
4 files changed, 55 insertions(+), 10 deletions(-)
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index ec1803a..249c54c 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -2571,3 +2571,29 @@ void scsi_kunmap_atomic_sg(void *virt)
kunmap_atomic(virt, KM_BIO_SRC_IRQ);
}
EXPORT_SYMBOL(scsi_kunmap_atomic_sg);
+
+/* Logging utilities */
+
+int scmd_printk(const char *prefix, const struct scsi_cmnd *scmd,
+ const char *format, ...)
+{
+ struct va_format vaf;
+ va_list args;
+ int r;
+
+ va_start(args, format);
+
+ vaf.fmt = format;
+ vaf.va = &args;
+
+ if (scmd->request->rq_disk)
+ r = sdev_printk(prefix, scmd->device, "[%s] %pV",
+ alias_name(scmd->request->rq_disk), &vaf);
+ else
+ r = sdev_printk(prefix, scmd->device, "%pV", &vaf);
+
+ va_end(args);
+
+ return r;
+}
+EXPORT_SYMBOL(scmd_printk);
diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index 953773c..2251e01 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -2888,3 +2888,26 @@ static void sd_print_result(struct scsi_disk *sdkp, int result)
scsi_show_result(result);
}
+int sd_printk(const char *prefix, const struct scsi_disk *sdsk,
+ const char *format, ...)
+{
+ struct va_format vaf;
+ va_list args;
+ int r;
+
+ va_start(args, format);
+
+ vaf.fmt = format;
+ vaf.va = &args;
+
+ if (sdsk->disk)
+ r = sdev_printk(prefix, sdsk->device, "[%s] %pV",
+ alias_name(sdsk->disk), &vaf);
+ else
+ r = sdev_printk(prefix, sdsk->device, "%pV", &vaf);
+
+ va_end(args);
+
+ return r;
+}
+EXPORT_SYMBOL(sd_printk);
diff --git a/drivers/scsi/sd.h b/drivers/scsi/sd.h
index 6ad798b..46aa748 100644
--- a/drivers/scsi/sd.h
+++ b/drivers/scsi/sd.h
@@ -88,11 +88,9 @@ static inline struct scsi_disk *scsi_disk(struct gendisk *disk)
return container_of(disk->private_data, struct scsi_disk, driver);
}
-#define sd_printk(prefix, sdsk, fmt, a...) \
- (sdsk)->disk ? \
- sdev_printk(prefix, (sdsk)->device, "[%s] " fmt, \
- (sdsk)->disk->disk_name, ##a) : \
- sdev_printk(prefix, (sdsk)->device, fmt, ##a)
+extern __attribute__((format (printf, 3, 4)))
+int sd_printk(const char *prefix, const struct scsi_disk *sdsk,
+ const char *format, ...);
/*
* A DIF-capable target device can be formatted with different
diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h
index dd82e02..c79631b 100644
--- a/include/scsi/scsi_device.h
+++ b/include/scsi/scsi_device.h
@@ -216,11 +216,9 @@ struct scsi_dh_data {
#define sdev_printk(prefix, sdev, fmt, a...) \
dev_printk(prefix, &(sdev)->sdev_gendev, fmt, ##a)
-#define scmd_printk(prefix, scmd, fmt, a...) \
- (scmd)->request->rq_disk ? \
- sdev_printk(prefix, (scmd)->device, "[%s] " fmt, \
- (scmd)->request->rq_disk->disk_name, ##a) : \
- sdev_printk(prefix, (scmd)->device, fmt, ##a)
+extern __attribute__((format (printf, 3, 4)))
+int scmd_printk(const char *prefix, const struct scsi_cmnd *scmd,
+ const char *format, ...);
enum scsi_target_state {
STARGET_CREATED = 1,
--
1.7.6.131.g99019
|
|
From: Greg KH <gr...@kr...> - 2011-07-08 20:01:22
|
On Fri, Jul 08, 2011 at 09:45:01PM +0200, Karel Zak wrote: > On Fri, Jul 08, 2011 at 05:45:47PM +0900, Nao Nishijima wrote: > > This patch series provides an "alias name" of the disk into kernel and procfs > > messages. The user can assign a preferred name to an alias name of the device. > > > > Based on previous discussion (*), I changed patches as follows > > - This is "alias name" > > - An "alias name" is stored in gendisk struct > > - Add document to Documentation/ABI/testing/sysfs-block > > - When the user changes an "alias name", kernel notifies udev > > > > (*) http://marc.info/?l=linux-scsi&m=130812625531219&w=2 > > > > [...] > > > [localhost]# cat /proc/partitions > > major minor #blocks name > > > > 8 0 12582912 foo > > 8 1 12582878 foo1 > > 8 0 8388608 sdb > > 8 1 512000 sdb1 > > 8 2 7875584 sdb2 > > If there is not /dev/foo and /sys/block/foo then the patch introduces > a REGRESSION. > > The names from /proc/partitions are used in many applications > (libblkid, fdisk, ...) for many many years. The applications will not > work as expected. > > It's crazy to assume that all the applications will be improved to > translate the "pretty name" from /proc/partitions by /sys/block/<maj>:<min>. > > Note, it's pretty naive to expect that people will use the pretty > names for printk()/dmesg only. I'm absolutely sure that it's will be > expected (by end-users) in /etc/fstab, mount, df, fdisk, ... > > It's will be necessary to modify all these utils. And it would be even easier, to just modify the utilities in the first place to handle the symlink names in /dev/disk that are there today, and leave the kernel alone. With the above information you provided, I don't see how we can accept this patch at all as it will break existing userspace utilities which is not allowed. thanks, greg k-h |
|
From: Karel Z. <kz...@re...> - 2011-07-08 19:45:18
|
On Fri, Jul 08, 2011 at 05:45:47PM +0900, Nao Nishijima wrote: > This patch series provides an "alias name" of the disk into kernel and procfs > messages. The user can assign a preferred name to an alias name of the device. > > Based on previous discussion (*), I changed patches as follows > - This is "alias name" > - An "alias name" is stored in gendisk struct > - Add document to Documentation/ABI/testing/sysfs-block > - When the user changes an "alias name", kernel notifies udev > > (*) http://marc.info/?l=linux-scsi&m=130812625531219&w=2 > [...] > [localhost]# cat /proc/partitions > major minor #blocks name > > 8 0 12582912 foo > 8 1 12582878 foo1 > 8 0 8388608 sdb > 8 1 512000 sdb1 > 8 2 7875584 sdb2 If there is not /dev/foo and /sys/block/foo then the patch introduces a REGRESSION. The names from /proc/partitions are used in many applications (libblkid, fdisk, ...) for many many years. The applications will not work as expected. It's crazy to assume that all the applications will be improved to translate the "pretty name" from /proc/partitions by /sys/block/<maj>:<min>. Note, it's pretty naive to expect that people will use the pretty names for printk()/dmesg only. I'm absolutely sure that it's will be expected (by end-users) in /etc/fstab, mount, df, fdisk, ... It's will be necessary to modify all these utils. The patch is opening Pandora's box :-( Karel -- Karel Zak <kz...@re...> http://karelzak.blogspot.com |
|
From: Kay S. <kay...@vr...> - 2011-07-08 16:38:50
|
On Fri, Jul 8, 2011 at 18:15, Kay Sievers <kay...@vr...> wrote: > On Fri, Jul 8, 2011 at 17:54, James Bottomley >> But hey, >> you have the enthusiasm, propose it as a KS topic to get agreement that >> we should do it and what the format should be and we can go from there. > > Sure, I'll do that. If needed, I can even make half or a third of it > possible I guess. Submitted it a minute ago. Thanks, Kay |
|
From: Greg KH <gr...@kr...> - 2011-07-08 16:33:00
|
On Fri, Jul 08, 2011 at 11:17:36AM -0500, James Bottomley wrote: > On Fri, 2011-07-08 at 09:04 -0700, Greg KH wrote: > > Do you seriously think that this sysfs attribute can be removed someday > > in the future if we get structured logging implemented? It can't, > > sorry, people will start depending on this and their jury-rigged > > implementations will require it no matter what we do otherwise. > > Yes, it can. The way we do that is to make udev the interface to this > and the sysfs file becomes an internal udev implementation, not an > external ABI. No, sorry, you just described the "external" abi in the Documentation/ABI directory. > Any program using it rather than udev that breaks, well, tough, it > used the wrong interface. Again, no, we can't do that as you should well know. And see Kay's response as to how udev will not be using this interface by default anyway, so I don't see how you will ever be able to remove this sysfs file once it is added. greg k-h |
|
From: James B. <Jam...@Ha...> - 2011-07-08 16:17:47
|
On Fri, 2011-07-08 at 09:04 -0700, Greg KH wrote: > On Fri, Jul 08, 2011 at 10:54:11AM -0500, James Bottomley wrote: > > On Fri, 2011-07-08 at 08:47 -0700, Greg KH wrote: > > > On Fri, Jul 08, 2011 at 05:41:36PM +0200, Kay Sievers wrote: > > > > On Fri, Jul 8, 2011 at 16:54, Greg KH <gr...@kr...> wrote: > > > > > On Fri, Jul 08, 2011 at 05:45:47PM +0900, Nao Nishijima wrote: > > > > >> This patch series provides an "alias name" of the disk into kernel and procfs > > > > >> messages. The user can assign a preferred name to an alias name of the device. > > > > >> > > > > >> Based on previous discussion (*), I changed patches as follows > > > > >> - This is "alias name" > > > > >> - An "alias name" is stored in gendisk struct > > > > >> - Add document to Documentation/ABI/testing/sysfs-block > > > > >> - When the user changes an "alias name", kernel notifies udev > > > > >> > > > > >> (*) http://marc.info/?l=linux-scsi&m=130812625531219&w=2 > > > > > > > > > > I don't like it and I don't think it will really solve the root problem > > > > > you are trying to address, but as the patches don't touch any code I > > > > > maintain, there's not much I can do to object to it. > > > > > > > > I can only repeat what I already wrote in detail earlier: > > > > > > > > This approach seems to papers over the problem which emitting and > > > > parsing free-text printk() messages with much-too-dumb tools cause. It > > > > seems to fix the symptoms not the cause. > > > > > > > > You can already write a udev rule today that logs _all_ symlinks of a > > > > device at discovery time, and any later kernel message can safely be > > > > associated with all possible names of the blockdev. No kernel changes > > > > needed, all possible names are covered. That also works good enough > > > > with our current stone-age tools for anybody who is able to scroll > > > > back to the last log udev message in that same log file. > > > > > > > > There can be by-definition no default udev rules assigning a proper > > > > single name to a block device. There is never a valid single name for > > > > a disks, so udev can not ship anything like that in the default setup, > > > > so this stays as a custom hack. > > > > > > > > We absolutely need _structured_ data for logging and error reporting, > > > > not only to solve this problem. Along with the current free-text > > > > printk(), we would be able to attach classifications, device > > > > error/sense data, firmware register dumps and anything > > > > interesting-for-debug to the messages. > > > > > > > > We can't solve that problem in the kernel alone. Structured data from > > > > the kernel will need to feed a smarter userspace logger that can index > > > > and classify messages, merge current userspace data into it, and > > > > provides hooks for the system management to act on critical failures > > > > and raise notifications. > > > > > > > > Structured logging seems like the solution for this and also to many > > > > other problems in this area. Single custom names pushed into the > > > > kernel might cover some rather exotic use cases, but I think, is not > > > > what we are looking for. > > > > > > I totally agree, but hey, no one listens to us :) > > > > Yes, we did. Everyone agrees structured logging would be the best long > > term solution. However, it's at least 10x the work presented here, plus > > it would be a long process getting everyone to agree. This looks like a > > good 95% interim solution and it can be removed when structured logging > > makes everything "just work(tm)". > > Do you seriously think that this sysfs attribute can be removed someday > in the future if we get structured logging implemented? It can't, > sorry, people will start depending on this and their jury-rigged > implementations will require it no matter what we do otherwise. Yes, it can. The way we do that is to make udev the interface to this and the sysfs file becomes an internal udev implementation, not an external ABI. Any program using it rather than udev that breaks, well, tough, it used the wrong interface. > But that's still not the main issue here. > > The main issue that I see is that this really doesn't solve the problem > that is trying to be addressed, and the authors admit that. So for that > very reason alone I feel it should be rejected, but again, it's not my > subsystem to maintain or control. > > best of luck, Thanks, James |
|
From: Kay S. <kay...@vr...> - 2011-07-08 16:15:45
|
On Fri, Jul 8, 2011 at 17:54, James Bottomley <Jam...@ha...> wrote: > Yes, we did. Everyone agrees structured logging would be the best long > term solution. However, it's at least 10x the work presented here, plus > it would be a long process getting everyone to agree. Maybe even 100 times :) > This looks like a > good 95% interim solution and it can be removed when structured logging > makes everything "just work(tm)". I don't really see that. I think it does not add any real value on top of a simple udev logging at device discovery. You would see that in the log: [78441.742634]: udevd: sdc: \ disk/by-id/ata-INTEL_SSDSA1M080G2GN_CVPO0363030V080EGN \ disk/by-id/scsi-SATA_INTEL_SSDSA1M08CVPO0363030V080EGN \ disk/by-path/pci-0000:00:1f.2-scsi-0:0:0:0 \ disk/by-id/wwn-0x50015179593f3038 and know exactly _all_ names and all identifiers of the disk at that time. Then you see this: [78441.742634] storage: really bad things happened to: sdc and with the earlier message we have all we need to know without any pretty-name hacks. All that can be done today already with a single udev rule, even on many years old distros. > I have also seen a couple of other attempts at structured logging which > both failed when the people proposing the patches realised how much work > it actually was, so I'm a bit sceptical we'll ever get there. The more paper-over we add the more unlikely it gets. That's what I fear. :) > But hey, > you have the enthusiasm, propose it as a KS topic to get agreement that > we should do it and what the format should be and we can go from there. Sure, I'll do that. If needed, I can even make half or a third of it possible I guess. Thanks, Kay |
|
From: Kay S. <kay...@vr...> - 2011-07-08 16:06:03
|
On Fri, Jul 8, 2011 at 16:54, Greg KH <gr...@kr...> wrote: > On Fri, Jul 08, 2011 at 05:45:47PM +0900, Nao Nishijima wrote: >> This patch series provides an "alias name" of the disk into kernel and procfs >> messages. The user can assign a preferred name to an alias name of the device. >> >> Based on previous discussion (*), I changed patches as follows >> - This is "alias name" >> - An "alias name" is stored in gendisk struct >> - Add document to Documentation/ABI/testing/sysfs-block >> - When the user changes an "alias name", kernel notifies udev >> >> (*) http://marc.info/?l=linux-scsi&m=130812625531219&w=2 > > I don't like it and I don't think it will really solve the root problem > you are trying to address, but as the patches don't touch any code I > maintain, there's not much I can do to object to it. I can only repeat what I already wrote in detail earlier: This approach seems to papers over the problem which emitting and parsing free-text printk() messages with much-too-dumb tools cause. It seems to fix the symptoms not the cause. You can already write a udev rule today that logs _all_ symlinks of a device at discovery time, and any later kernel message can safely be associated with all possible names of the blockdev. No kernel changes needed, all possible names are covered. That also works good enough with our current stone-age tools for anybody who is able to scroll back to the last log udev message in that same log file. There can be by-definition no default udev rules assigning a proper single name to a block device. There is never a valid single name for a disks, so udev can not ship anything like that in the default setup, so this stays as a custom hack. We absolutely need _structured_ data for logging and error reporting, not only to solve this problem. Along with the current free-text printk(), we would be able to attach classifications, device error/sense data, firmware register dumps and anything interesting-for-debug to the messages. We can't solve that problem in the kernel alone. Structured data from the kernel will need to feed a smarter userspace logger that can index and classify messages, merge current userspace data into it, and provides hooks for the system management to act on critical failures and raise notifications. Structured logging seems like the solution for this and also to many other problems in this area. Single custom names pushed into the kernel might cover some rather exotic use cases, but I think, is not what we are looking for. Kay |
|
From: Greg KH <gr...@kr...> - 2011-07-08 16:04:15
|
On Fri, Jul 08, 2011 at 10:54:11AM -0500, James Bottomley wrote: > On Fri, 2011-07-08 at 08:47 -0700, Greg KH wrote: > > On Fri, Jul 08, 2011 at 05:41:36PM +0200, Kay Sievers wrote: > > > On Fri, Jul 8, 2011 at 16:54, Greg KH <gr...@kr...> wrote: > > > > On Fri, Jul 08, 2011 at 05:45:47PM +0900, Nao Nishijima wrote: > > > >> This patch series provides an "alias name" of the disk into kernel and procfs > > > >> messages. The user can assign a preferred name to an alias name of the device. > > > >> > > > >> Based on previous discussion (*), I changed patches as follows > > > >> - This is "alias name" > > > >> - An "alias name" is stored in gendisk struct > > > >> - Add document to Documentation/ABI/testing/sysfs-block > > > >> - When the user changes an "alias name", kernel notifies udev > > > >> > > > >> (*) http://marc.info/?l=linux-scsi&m=130812625531219&w=2 > > > > > > > > I don't like it and I don't think it will really solve the root problem > > > > you are trying to address, but as the patches don't touch any code I > > > > maintain, there's not much I can do to object to it. > > > > > > I can only repeat what I already wrote in detail earlier: > > > > > > This approach seems to papers over the problem which emitting and > > > parsing free-text printk() messages with much-too-dumb tools cause. It > > > seems to fix the symptoms not the cause. > > > > > > You can already write a udev rule today that logs _all_ symlinks of a > > > device at discovery time, and any later kernel message can safely be > > > associated with all possible names of the blockdev. No kernel changes > > > needed, all possible names are covered. That also works good enough > > > with our current stone-age tools for anybody who is able to scroll > > > back to the last log udev message in that same log file. > > > > > > There can be by-definition no default udev rules assigning a proper > > > single name to a block device. There is never a valid single name for > > > a disks, so udev can not ship anything like that in the default setup, > > > so this stays as a custom hack. > > > > > > We absolutely need _structured_ data for logging and error reporting, > > > not only to solve this problem. Along with the current free-text > > > printk(), we would be able to attach classifications, device > > > error/sense data, firmware register dumps and anything > > > interesting-for-debug to the messages. > > > > > > We can't solve that problem in the kernel alone. Structured data from > > > the kernel will need to feed a smarter userspace logger that can index > > > and classify messages, merge current userspace data into it, and > > > provides hooks for the system management to act on critical failures > > > and raise notifications. > > > > > > Structured logging seems like the solution for this and also to many > > > other problems in this area. Single custom names pushed into the > > > kernel might cover some rather exotic use cases, but I think, is not > > > what we are looking for. > > > > I totally agree, but hey, no one listens to us :) > > Yes, we did. Everyone agrees structured logging would be the best long > term solution. However, it's at least 10x the work presented here, plus > it would be a long process getting everyone to agree. This looks like a > good 95% interim solution and it can be removed when structured logging > makes everything "just work(tm)". Do you seriously think that this sysfs attribute can be removed someday in the future if we get structured logging implemented? It can't, sorry, people will start depending on this and their jury-rigged implementations will require it no matter what we do otherwise. But that's still not the main issue here. The main issue that I see is that this really doesn't solve the problem that is trying to be addressed, and the authors admit that. So for that very reason alone I feel it should be rejected, but again, it's not my subsystem to maintain or control. best of luck, greg k-h |
|
From: James B. <Jam...@Ha...> - 2011-07-08 15:54:22
|
On Fri, 2011-07-08 at 08:47 -0700, Greg KH wrote: > On Fri, Jul 08, 2011 at 05:41:36PM +0200, Kay Sievers wrote: > > On Fri, Jul 8, 2011 at 16:54, Greg KH <gr...@kr...> wrote: > > > On Fri, Jul 08, 2011 at 05:45:47PM +0900, Nao Nishijima wrote: > > >> This patch series provides an "alias name" of the disk into kernel and procfs > > >> messages. The user can assign a preferred name to an alias name of the device. > > >> > > >> Based on previous discussion (*), I changed patches as follows > > >> - This is "alias name" > > >> - An "alias name" is stored in gendisk struct > > >> - Add document to Documentation/ABI/testing/sysfs-block > > >> - When the user changes an "alias name", kernel notifies udev > > >> > > >> (*) http://marc.info/?l=linux-scsi&m=130812625531219&w=2 > > > > > > I don't like it and I don't think it will really solve the root problem > > > you are trying to address, but as the patches don't touch any code I > > > maintain, there's not much I can do to object to it. > > > > I can only repeat what I already wrote in detail earlier: > > > > This approach seems to papers over the problem which emitting and > > parsing free-text printk() messages with much-too-dumb tools cause. It > > seems to fix the symptoms not the cause. > > > > You can already write a udev rule today that logs _all_ symlinks of a > > device at discovery time, and any later kernel message can safely be > > associated with all possible names of the blockdev. No kernel changes > > needed, all possible names are covered. That also works good enough > > with our current stone-age tools for anybody who is able to scroll > > back to the last log udev message in that same log file. > > > > There can be by-definition no default udev rules assigning a proper > > single name to a block device. There is never a valid single name for > > a disks, so udev can not ship anything like that in the default setup, > > so this stays as a custom hack. > > > > We absolutely need _structured_ data for logging and error reporting, > > not only to solve this problem. Along with the current free-text > > printk(), we would be able to attach classifications, device > > error/sense data, firmware register dumps and anything > > interesting-for-debug to the messages. > > > > We can't solve that problem in the kernel alone. Structured data from > > the kernel will need to feed a smarter userspace logger that can index > > and classify messages, merge current userspace data into it, and > > provides hooks for the system management to act on critical failures > > and raise notifications. > > > > Structured logging seems like the solution for this and also to many > > other problems in this area. Single custom names pushed into the > > kernel might cover some rather exotic use cases, but I think, is not > > what we are looking for. > > I totally agree, but hey, no one listens to us :) Yes, we did. Everyone agrees structured logging would be the best long term solution. However, it's at least 10x the work presented here, plus it would be a long process getting everyone to agree. This looks like a good 95% interim solution and it can be removed when structured logging makes everything "just work(tm)". I have also seen a couple of other attempts at structured logging which both failed when the people proposing the patches realised how much work it actually was, so I'm a bit sceptical we'll ever get there. But hey, you have the enthusiasm, propose it as a KS topic to get agreement that we should do it and what the format should be and we can go from there. James |
|
From: Greg KH <gr...@kr...> - 2011-07-08 15:47:46
|
On Fri, Jul 08, 2011 at 05:41:36PM +0200, Kay Sievers wrote: > On Fri, Jul 8, 2011 at 16:54, Greg KH <gr...@kr...> wrote: > > On Fri, Jul 08, 2011 at 05:45:47PM +0900, Nao Nishijima wrote: > >> This patch series provides an "alias name" of the disk into kernel and procfs > >> messages. The user can assign a preferred name to an alias name of the device. > >> > >> Based on previous discussion (*), I changed patches as follows > >> - This is "alias name" > >> - An "alias name" is stored in gendisk struct > >> - Add document to Documentation/ABI/testing/sysfs-block > >> - When the user changes an "alias name", kernel notifies udev > >> > >> (*) http://marc.info/?l=linux-scsi&m=130812625531219&w=2 > > > > I don't like it and I don't think it will really solve the root problem > > you are trying to address, but as the patches don't touch any code I > > maintain, there's not much I can do to object to it. > > I can only repeat what I already wrote in detail earlier: > > This approach seems to papers over the problem which emitting and > parsing free-text printk() messages with much-too-dumb tools cause. It > seems to fix the symptoms not the cause. > > You can already write a udev rule today that logs _all_ symlinks of a > device at discovery time, and any later kernel message can safely be > associated with all possible names of the blockdev. No kernel changes > needed, all possible names are covered. That also works good enough > with our current stone-age tools for anybody who is able to scroll > back to the last log udev message in that same log file. > > There can be by-definition no default udev rules assigning a proper > single name to a block device. There is never a valid single name for > a disks, so udev can not ship anything like that in the default setup, > so this stays as a custom hack. > > We absolutely need _structured_ data for logging and error reporting, > not only to solve this problem. Along with the current free-text > printk(), we would be able to attach classifications, device > error/sense data, firmware register dumps and anything > interesting-for-debug to the messages. > > We can't solve that problem in the kernel alone. Structured data from > the kernel will need to feed a smarter userspace logger that can index > and classify messages, merge current userspace data into it, and > provides hooks for the system management to act on critical failures > and raise notifications. > > Structured logging seems like the solution for this and also to many > other problems in this area. Single custom names pushed into the > kernel might cover some rather exotic use cases, but I think, is not > what we are looking for. I totally agree, but hey, no one listens to us :) greg k-h |
|
From: Greg KH <gr...@kr...> - 2011-07-08 14:57:49
|
On Fri, Jul 08, 2011 at 05:45:47PM +0900, Nao Nishijima wrote: > Hi, > > This patch series provides an "alias name" of the disk into kernel and procfs > messages. The user can assign a preferred name to an alias name of the device. > > Based on previous discussion (*), I changed patches as follows > - This is "alias name" > - An "alias name" is stored in gendisk struct > - Add document to Documentation/ABI/testing/sysfs-block > - When the user changes an "alias name", kernel notifies udev > > (*) http://marc.info/?l=linux-scsi&m=130812625531219&w=2 I don't like it and I don't think it will really solve the root problem you are trying to address, but as the patches don't touch any code I maintain, there's not much I can do to object to it. greg k-h |
|
From: Nao N. <nao...@hi...> - 2011-07-08 09:01:04
|
This patch modify sd_printk() and scmd_printk() to use alias_name. If user set
an alias_name, those print an alias_name instead of a disk_name.
Suggested-by: James Bottomley <Jam...@Ha...>
Suggested-by: Jon Masters <jon...@jo...>
Signed-off-by: Nao Nishijima <nao...@hi...>
Reviewed-by: Masami Hiramatsu <mas...@hi...>
---
drivers/scsi/sd.h | 2 +-
include/scsi/scsi_device.h | 3 ++-
2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/scsi/sd.h b/drivers/scsi/sd.h
index 6ad798b..66c7ae8 100644
--- a/drivers/scsi/sd.h
+++ b/drivers/scsi/sd.h
@@ -91,7 +91,7 @@ static inline struct scsi_disk *scsi_disk(struct gendisk *disk)
#define sd_printk(prefix, sdsk, fmt, a...) \
(sdsk)->disk ? \
sdev_printk(prefix, (sdsk)->device, "[%s] " fmt, \
- (sdsk)->disk->disk_name, ##a) : \
+ alias_name((sdsk)->disk), ##a) : \
sdev_printk(prefix, (sdsk)->device, fmt, ##a)
/*
diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h
index dd82e02..125c05c 100644
--- a/include/scsi/scsi_device.h
+++ b/include/scsi/scsi_device.h
@@ -6,6 +6,7 @@
#include <linux/spinlock.h>
#include <linux/workqueue.h>
#include <linux/blkdev.h>
+#include <linux/genhd.h>
#include <scsi/scsi.h>
#include <asm/atomic.h>
@@ -219,7 +220,7 @@ struct scsi_dh_data {
#define scmd_printk(prefix, scmd, fmt, a...) \
(scmd)->request->rq_disk ? \
sdev_printk(prefix, (scmd)->device, "[%s] " fmt, \
- (scmd)->request->rq_disk->disk_name, ##a) : \
+ alias_name((scmd)->request->rq_disk), ##a) : \
sdev_printk(prefix, (scmd)->device, fmt, ##a)
enum scsi_target_state {
|
|
From: Nao N. <nao...@hi...> - 2011-07-08 09:01:02
|
Hi, This patch series provides an "alias name" of the disk into kernel and procfs messages. The user can assign a preferred name to an alias name of the device. Based on previous discussion (*), I changed patches as follows - This is "alias name" - An "alias name" is stored in gendisk struct - Add document to Documentation/ABI/testing/sysfs-block - When the user changes an "alias name", kernel notifies udev (*) http://marc.info/?l=linux-scsi&m=130812625531219&w=2 How to use: 1. Build and install the kernel with this series, and reboot with the kernel. 2. Check device names. [localhost]# cat /proc/partitions major minor #blocks name 8 0 12582912 sda 8 1 12582878 sda1 8 0 8388608 sdb 8 1 512000 sdb1 8 2 7875584 sdb2 3. Make a script of get alias_name [localhost]# vi /lib/udev/get_alias_name #!/bin/sh -e DEVNAME=`echo $1 | sed -e 's/[0-9]//g'` echo "ALIAS=`cat /sys/block/$DEVNAME/alias_name`" exit 0 And you should set an execute bit, [localhost]# chmod +x /lib/udev/get_alias_name 4. Check disk's id Here is an example to get the serial id and the path of the device. Some devices have not the serial id. Therefore, to identify a device, users need to get the path of the device. [localhost]# udevadm info --query=property --path=/sys/block/sda \ | grep ID_SERIAL= ID_SERIAL=0QEMU_QEMU_HARDDISK_drive-scsi0-0-1 or you can also use the path of the device [localhost]# udevadm info --query=property --path=/sys/block/sr0 \ | grep ID_PATH= ID_PATH=pci-0000:00:01.1-scsi-1:0:0:0 5. Write udev rules as follows (The user assigns "foo" to sda and "bar" to sr0) We use ENV{ID_SERIAL} or ENV{ID_PATH} (get by 3) to identify a disk. And to assign automatically an "alias name", we use ATTR key. If ENV{ALIAS} is empty, we use to get an "alias_name" by get_alias_name script. [localhost]# vi /etc/udev/rules.d/70-alias_name.rules SUBSYSTEM!="block", GOTO="end" # write alias name for sdX KERNEL=="sd*[!0-9]", ACTION=="add", ATTR{alias_name}="foo", \ ENV{ID_SERIAL}=="0QEMU_QEMU_HARDDISK_drive-scsi0-0-1" # write alias name for srX KERNEL=="sr[0-9]", ACTION=="add", ATTR{alias_name}="bar", \ ENV{ID_PATH}=="pci-0000:00:01.1-scsi-1:0:0:0" # make symlink ENV{DEVTYPE}=="disk", ENV{ALIAS}=="?*", SYMLINK+="disk/by-alias/$env{ALIAS}" ENV{DEVTYPE}=="partition", ENV{ALIAS}=="", \ IMPORT{program}="/lib/udev/get_alias_name %k" ENV{DEVTYPE}=="partition", ENV{ALIAS}=="?*", \ SYMLINK+="disk/by-alias/$env{ALIAS}%n" LABEL="end" 6. reboot After reboot, we can see alias name in kernel and procfs messages. [localhost]# ls -l /dev/disk/by-alias/ total 0 lrwxrwxrwx. 1 root root 9 Jul 1 21:21 bar -> ../../sr0 lrwxrwxrwx. 1 root root 9 Jul 1 21:21 foo -> ../../sda lrwxrwxrwx. 1 root root 10 Jul 1 21:21 foo1 -> ../../sda1 [localhost]# dmesg ... sd 2:0:1:0: [sda] Attached SCSI disk alias_name: assigned foo to sda EXT4-fs (foo1): warning: maximal mount count reached, running e2fsck is recommended EXT4-fs (foo1): mounted filesystem with ordered data mode. Opts: (null) ... [localhost]# cat /proc/partitions major minor #blocks name 8 0 12582912 foo 8 1 12582878 foo1 8 0 8388608 sdb 8 1 512000 sdb1 8 2 7875584 sdb2 When a new device is added, the udev appends a new rule manually. In the future, it is appended automatically, as like NIC. TODO: - Modify blkid to show "alias name" Best Regards, --- Nao Nishijima (4): sd: cleanup for alias name fs: modify disk_name() for alias name sd: modify printk for alias_name block: add a new attribute "alias name" in gendisk structure Documentation/ABI/testing/sysfs-block | 15 ++++++ block/genhd.c | 85 +++++++++++++++++++++++++++++++++ drivers/scsi/sd.c | 7 ++- drivers/scsi/sd.h | 2 - fs/partitions/check.c | 6 +- include/linux/genhd.h | 4 ++ include/scsi/scsi_device.h | 3 + 7 files changed, 114 insertions(+), 8 deletions(-) -- Nao Nishijima (nao...@hi...) |
|
From: Nao N. <nao...@hi...> - 2011-07-08 09:01:02
|
This patch allows the user to set an "alias name" of the disk via sysfs interface.
A raw device name of a disk does not always point a same disk each boot-up time.
Therefore, users have to use persistent device names, which udev create to always
access the same disk. However, kernel messages still display device names.
This patch adds a new attribute "alias name" in gendisk structure. And if users
set their preferred name to an "alias name" of the disk, it would be appeared
in kernel messages. A disk can have an "alias name" which length is up to
255bytes. Users can use alphabets, numbers, '-' and '_' in alias name.
Suggested-by: James Bottomley <Jam...@Ha...>
Suggested-by: Jon Masters <jc...@re...>
Signed-off-by: Nao Nishijima <nao...@hi...>
---
Documentation/ABI/testing/sysfs-block | 15 ++++++
block/genhd.c | 85 +++++++++++++++++++++++++++++++++
include/linux/genhd.h | 4 ++
3 files changed, 104 insertions(+), 0 deletions(-)
diff --git a/Documentation/ABI/testing/sysfs-block b/Documentation/ABI/testing/sysfs-block
index c1eb41c..07c4fcd 100644
--- a/Documentation/ABI/testing/sysfs-block
+++ b/Documentation/ABI/testing/sysfs-block
@@ -206,3 +206,18 @@ Description:
when a discarded area is read the discard_zeroes_data
parameter will be set to one. Otherwise it will be 0 and
the result of reading a discarded area is undefined.
+
+What: /sys/block/<disk>/alias_name
+Date: June 2011
+Contact: Nao Nishijima <nao...@hi...>
+Description:
+ A raw device name of a disk does not always point a same disk
+ each boot-up time. Therefore, users have to use persistent
+ device names, which udev creates when the kernel finds a disk,
+ instead of raw device name. However, kernel doesn't show those
+ persistent names on its message (e.g. dmesg).
+ This file can store an alias name of the disk and it would be
+ appeared in kernel messages if it is set. A disk can have an
+ alias name which length is up to 255bytes. Users can use
+ use alphabets, numbers, "-" and "_" in alias name and can
+ change it anytime.
diff --git a/block/genhd.c b/block/genhd.c
index 3608289..4363947 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -19,6 +19,7 @@
#include <linux/mutex.h>
#include <linux/idr.h>
#include <linux/log2.h>
+#include <linux/ctype.h>
#include "blk.h"
@@ -909,6 +910,87 @@ static int __init genhd_device_init(void)
subsys_initcall(genhd_device_init);
+static ssize_t alias_name_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct gendisk *disk = dev_to_disk(dev);
+ ssize_t ret = 0;
+
+ if (disk->alias_name)
+ ret = snprintf(buf, ALIAS_NAME_LEN + 1, "%s\n",
+ disk->alias_name);
+ return ret;
+}
+
+static ssize_t alias_name_store(struct device *dev,
+ struct device_attribute *attr, const char *buf,
+ size_t count)
+{
+ struct gendisk *disk = dev_to_disk(dev);
+ struct disk_part_iter piter;
+ struct hd_struct *part;
+
+ char *new_alias_name;
+ char *envp[] = { NULL, NULL };
+ unsigned char c;
+ int idx;
+ ssize_t ret = count;
+
+ if (!count)
+ return -EINVAL;
+
+ if (count >= ALIAS_NAME_LEN) {
+ printk(KERN_ERR "alias_name: alias name is too long\n");
+ return -EINVAL;
+ }
+
+ /* Validation check */
+ for (idx = 0; idx < count; idx++) {
+ c = buf[idx];
+ if (idx == count - 1 && c == '\n')
+ break;
+ if (!isalnum(c) && c != '_' && c != '-') {
+ printk(KERN_ERR "alias_name: invalid alias name\n");
+ return -EINVAL;
+ }
+ }
+
+ new_alias_name = kasprintf(GFP_KERNEL, "%s", buf);
+ if (!new_alias_name)
+ return -ENOMEM;
+
+ if (new_alias_name[count - 1] == '\n')
+ new_alias_name[count - 1] = '\0';
+
+ envp[0] = kasprintf(GFP_KERNEL, "ALIAS=%s", new_alias_name);
+ if (!envp[0]) {
+ kfree(new_alias_name);
+ return -ENOMEM;
+ }
+
+ if (disk->alias_name[0] != '\0')
+ printk(KERN_INFO "alias_name: assigned %s to %s (previous "
+ "alias_name was %s)\n",
+ new_alias_name, disk->disk_name, disk->alias_name);
+ else
+ printk(KERN_INFO "alias_name: assigned %s to %s\n",
+ new_alias_name, disk->disk_name);
+
+ snprintf(disk->alias_name, ALIAS_NAME_LEN, "%s", new_alias_name);
+
+ kobject_uevent_env(&dev->kobj, KOBJ_CHANGE, envp);
+
+ /* announce possible partitions */
+ disk_part_iter_init(&piter, disk, 0);
+ while ((part = disk_part_iter_next(&piter)))
+ kobject_uevent_env(&part_to_dev(part)->kobj, KOBJ_CHANGE, envp);
+ disk_part_iter_exit(&piter);
+
+ kfree(envp[0]);
+ kfree(new_alias_name);
+ return ret;
+}
+
static ssize_t disk_range_show(struct device *dev,
struct device_attribute *attr, char *buf)
{
@@ -968,6 +1050,8 @@ static ssize_t disk_discard_alignment_show(struct device *dev,
return sprintf(buf, "%d\n", queue_discard_alignment(disk->queue));
}
+static DEVICE_ATTR(alias_name, S_IRUGO|S_IWUSR, alias_name_show,
+ alias_name_store);
static DEVICE_ATTR(range, S_IRUGO, disk_range_show, NULL);
static DEVICE_ATTR(ext_range, S_IRUGO, disk_ext_range_show, NULL);
static DEVICE_ATTR(removable, S_IRUGO, disk_removable_show, NULL);
@@ -990,6 +1074,7 @@ static struct device_attribute dev_attr_fail_timeout =
#endif
static struct attribute *disk_attrs[] = {
+ &dev_attr_alias_name.attr,
&dev_attr_range.attr,
&dev_attr_ext_range.attr,
&dev_attr_removable.attr,
diff --git a/include/linux/genhd.h b/include/linux/genhd.h
index 300d758..a3c219d 100644
--- a/include/linux/genhd.h
+++ b/include/linux/genhd.h
@@ -21,6 +21,8 @@
#define dev_to_part(device) container_of((device), struct hd_struct, __dev)
#define disk_to_dev(disk) (&(disk)->part0.__dev)
#define part_to_dev(part) (&((part)->__dev))
+#define alias_name(disk) ((disk)->alias_name[0] != '\0' \
+ ? (disk)->alias_name : (disk)->disk_name)
extern struct device_type part_type;
extern struct kobject *block_depr;
@@ -58,6 +60,7 @@ enum {
#define DISK_MAX_PARTS 256
#define DISK_NAME_LEN 32
+#define ALIAS_NAME_LEN 256
#include <linux/major.h>
#include <linux/device.h>
@@ -162,6 +165,7 @@ struct gendisk {
* disks that can't be partitioned. */
char disk_name[DISK_NAME_LEN]; /* name of major driver */
+ char alias_name[ALIAS_NAME_LEN];/* alias name of disk */
char *(*devnode)(struct gendisk *gd, mode_t *mode);
unsigned int events; /* supported events */
|