linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* Should Pstore(ramoops) records customized information?
       [not found] <539E6D4D.5000802@huawei.com>
@ 2014-06-18 10:07 ` Liu hua
  2014-06-18 17:50   ` Luck, Tony
  0 siblings, 1 reply; 10+ messages in thread
From: Liu hua @ 2014-06-18 10:07 UTC (permalink / raw)
  To: anton, ccross, keescook, tony.luck, linux-kernel; +Cc: Wang Nan, peifeiyue

Hi Kees or Colin or Tony or Anton,

We are very interested in Pstore, which provides a mechanism to save information
when machine is going to die. It is much lighter than kdump. So we can deploy it on
our real products. Because our product runs on several platforms (x86,arm and mips),
we prefer ramoops as pstore backend. For ramoops backend, now we can save dmesg,
console and ftrace inforamtion to different memory regions. It is very good, but
can we do something more?

For kmsg_dumper or console, we mixed messages together. So some important meaasges
may be flooded with dispensable ones. Pstore does not provide a way to let us
determine which message to record, which to discard. And

So can we introduce a customized information recording mechanism?

Something like this:

(1) The backend (ramoops) provides servel memory regions staticly. Each region
 is a ring buffer, which does not connect with certain PSTORE_TYPE_ID. So no one
 can modify or use it before allocation.

(2) A pstore user allocs a memory region, pstore will return a pstore_type_id.

        pstore_type_id = alloc_pstroe_region()

(3) This user record certain message to this region.

        psinfo->write(pstore_type_id, ...)


By doing this:

(1) The console and ftrace message recording is also supported. we just need to call
    alloc_pstore_region() before saving such messages.

(2) We can realize a mechanism like black box in aircraft. if we record certain kind of
    messages to a sigle region. We do not need to care other type messages to overlap it.
    we can allways get the latest messages of each type.

(3) Anyone in kernel or modules can use this mechanism, if they alloc a region.


Thanks,
Liu Hua


.





^ permalink raw reply	[flat|nested] 10+ messages in thread

* RE: Should Pstore(ramoops) records customized information?
  2014-06-18 10:07 ` Should Pstore(ramoops) records customized information? Liu hua
@ 2014-06-18 17:50   ` Luck, Tony
  2014-06-19 12:21     ` Liu hua
  0 siblings, 1 reply; 10+ messages in thread
From: Luck, Tony @ 2014-06-18 17:50 UTC (permalink / raw)
  To: Liu hua, anton, ccross, keescook, linux-kernel; +Cc: Wang Nan, peifeiyue

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 1426 bytes --]

> (1) The backend (ramoops) provides servel memory regions staticly. Each region
>  is a ring buffer, which does not connect with certain PSTORE_TYPE_ID. So no one
>  can modify or use it before allocation.
>
> (2) A pstore user allocs a memory region, pstore will return a pstore_type_id.
>
>        pstore_type_id = alloc_pstroe_region()
>
> (3) This user record certain message to this region.
>
>        psinfo->write(pstore_type_id, ...)

Don't you need to match up the number of back-end ring buffer regions
with the number of users in the kernel that call alloc_pstore_region()?

Or do you envision that the backend can create these regions on demand?

Would different users need different sized regions? I think logging of
console messages might be able to work with a smaller ring buffer
than the ftrace logger. So perhaps we need a "size" argument when allocating?

Since these "regions" are in fact "ring buffers", the name of the allocation
routine should make that clear.  So call it "pstore_alloc_ring_buffer()"

After the system hangs/crashes ... how would you like pstore to
name these objects in /sys/fs/pstore/ for applications to pick them
up for analysis?  Maybe pstore_alloc_ring_buffer() needs a "char *name"
argument as well as a size?

-Tony
ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: Should Pstore(ramoops) records customized information?
  2014-06-18 17:50   ` Luck, Tony
@ 2014-06-19 12:21     ` Liu hua
  2014-06-19 23:42       ` Luck, Tony
  0 siblings, 1 reply; 10+ messages in thread
From: Liu hua @ 2014-06-19 12:21 UTC (permalink / raw)
  To: Luck, Tony, anton, ccross, keescook, linux-kernel; +Cc: Wang Nan, peifeiyue

On 2014/6/19 1:50, Luck, Tony wrote:

Hi Tony,

Thanks for you reply.

>> (1) The backend (ramoops) provides servel memory regions staticly. Each region
>>  is a ring buffer, which does not connect with certain PSTORE_TYPE_ID. So no one
>>  can modify or use it before allocation.
>>
>> (2) A pstore user allocs a memory region, pstore will return a pstore_type_id.
>>
>>        pstore_type_id = alloc_pstroe_region()
>>
>> (3) This user record certain message to this region.
>>
>>        psinfo->write(pstore_type_id, ...)
> 
> Don't you need to match up the number of back-end ring buffer regions
> with the number of users in the kernel that call alloc_pstore_region()?
> 
> Or do you envision that the backend can create these regions on demand?

I have do some experiments on ramoops. This may be realizable.

This idea comes from real products'demands . Becasue ram is rather cheap and
we do not need to add new hardware, product engineers want to record several
kinds of messages into reserved ram. (including kernel snapshot, softlookup, ftrace;
panic, even the user-space events and so on). Different products usually care about
different messages.

So we realized a mechanism named "KBOX" to provide ring-buffer alloction on reserved memory.
Kernel users can allocate and use a ring buffer. I think pstore(ramoops) may also need
this feature.

BTW, I note that "extern struct pstore_info *psinfo" locates in
fs/pstore/internal.h. So users out of directory "fs/pstore/" can not use pstore to
record messages. We do not want other kernel users to use pstore, right?  And can we
break this?

> 
> Would different users need different sized regions? I think logging of
> console messages might be able to work with a smaller ring buffer
> than the ftrace logger. So perhaps we need a "size" argument when allocating?

Yes, I will add this to my RFC patches.
> 
> Since these "regions" are in fact "ring buffers", the name of the allocation
> routine should make that clear.  So call it "pstore_alloc_ring_buffer()"
Yes, ditto..

> After the system hangs/crashes ... how would you like pstore to
> name these objects in /sys/fs/pstore/ for applications to pick them
> up for analysis?  Maybe pstore_alloc_ring_buffer() needs a "char *name"
> argument as well as a size?
> 
ditoo.. Since other backends like efi and erst may can not privide such ring buffer.
So pstore_alloc_ring_buffer should be a funciton pointer of pstore_info struct.

Thanks very much again. if pstore can accept this feature, it will be a great news for us.
we will drop our "KBOX" gradually, using pstore instead. If necessary, I will try to send
patch series to do this. What do you think about it?

Thanks,
Liu Hua



> -Tony
> 



^ permalink raw reply	[flat|nested] 10+ messages in thread

* RE: Should Pstore(ramoops) records customized information?
  2014-06-19 12:21     ` Liu hua
@ 2014-06-19 23:42       ` Luck, Tony
  2014-06-20 10:47         ` Liu hua
  0 siblings, 1 reply; 10+ messages in thread
From: Luck, Tony @ 2014-06-19 23:42 UTC (permalink / raw)
  To: Liu hua, anton, ccross, keescook, linux-kernel; +Cc: Wang Nan, peifeiyue

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 1238 bytes --]

> BTW, I note that "extern struct pstore_info *psinfo" locates in
> fs/pstore/internal.h. So users out of directory "fs/pstore/" can not use pstore to
> record messages. We do not want other kernel users to use pstore, right?  And can we
> break this?

Yes we can make some interface visible to the rest of the kernel ... probably
not the raw "*psinfo" though. Perhaps the pstore_alloc_ring_buffer() and
pstore_write_ring_buffer() functions should be the ones exported to the
rest of the kernel.

> ditoo.. Since other backends like efi and erst may can not privide such ring buffer.
> So pstore_alloc_ring_buffer should be a funciton pointer of pstore_info struct.

Yes - that allows less capable backend like ERST and efivars to not provide the
service.  Since it becomes internal, you can drop the "pstore_" prefix.  E.g.
something like:

int pstore_alloc_ring_buffer(char *name, int size)
{
	return psinfo->alloc_ring_buffer(name, size);
}
EXPORT_SYMBOL_GPL(pstore_alloc_ring_buffer);

... and you have to find/make some global header for the "extern" declaration.

-Tony


ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: Should Pstore(ramoops) records customized information?
  2014-06-19 23:42       ` Luck, Tony
@ 2014-06-20 10:47         ` Liu hua
  2014-06-25  0:41           ` Zhang, Yanmin
  0 siblings, 1 reply; 10+ messages in thread
From: Liu hua @ 2014-06-20 10:47 UTC (permalink / raw)
  To: Luck, Tony, anton, ccross, keescook, linux-kernel; +Cc: Wang Nan, peifeiyue

On 2014/6/20 7:42, Luck, Tony wrote:

>> BTW, I note that "extern struct pstore_info *psinfo" locates in
>> fs/pstore/internal.h. So users out of directory "fs/pstore/" can not use pstore to
>> record messages. We do not want other kernel users to use pstore, right?  And can we
>> break this?
> 
> Yes we can make some interface visible to the rest of the kernel ... probably
> not the raw "*psinfo" though. Perhaps the pstore_alloc_ring_buffer() and
> pstore_write_ring_buffer() functions should be the ones exported to the
> rest of the kernel.
> 
>> ditoo.. Since other backends like efi and erst may can not privide such ring buffer.
>> So pstore_alloc_ring_buffer should be a funciton pointer of pstore_info struct.
> 
> Yes - that allows less capable backend like ERST and efivars to not provide the
> service.  Since it becomes internal, you can drop the "pstore_" prefix.  E.g.
> something like:
> 
> int pstore_alloc_ring_buffer(char *name, int size)
> {
> 	return psinfo->alloc_ring_buffer(name, size);
> }
> EXPORT_SYMBOL_GPL(pstore_alloc_ring_buffer);
> 
> ... and you have to find/make some global header for the "extern" declaration.

I will make these RFC patch series according to our discussion. Thanks you very to
valuable advice.

Thanks,
Liu Hua




> 
> -Tony
> 
> 



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: Should Pstore(ramoops) records customized information?
  2014-06-20 10:47         ` Liu hua
@ 2014-06-25  0:41           ` Zhang, Yanmin
  2014-06-25 13:08             ` Liu hua
  0 siblings, 1 reply; 10+ messages in thread
From: Zhang, Yanmin @ 2014-06-25  0:41 UTC (permalink / raw)
  To: Liu hua, Luck, Tony, anton, ccross, keescook, linux-kernel
  Cc: Wang Nan, peifeiyue, Liu ShuoX


On 2014/6/20 18:47, Liu hua wrote:
> On 2014/6/20 7:42, Luck, Tony wrote:
>
>>> BTW, I note that "extern struct pstore_info *psinfo" locates in
>>> fs/pstore/internal.h. So users out of directory "fs/pstore/" can not use pstore to
>>> record messages. We do not want other kernel users to use pstore, right?  And can we
>>> break this?
>> Yes we can make some interface visible to the rest of the kernel ... probably
>> not the raw "*psinfo" though. Perhaps the pstore_alloc_ring_buffer() and
>> pstore_write_ring_buffer() functions should be the ones exported to the
>> rest of the kernel.
>>
>>> ditoo.. Since other backends like efi and erst may can not privide such ring buffer.
>>> So pstore_alloc_ring_buffer should be a funciton pointer of pstore_info struct.
>> Yes - that allows less capable backend like ERST and efivars to not provide the
>> service.  Since it becomes internal, you can drop the "pstore_" prefix.  E.g.
>> something like:
>>
>> int pstore_alloc_ring_buffer(char *name, int size)
>> {
>> 	return psinfo->alloc_ring_buffer(name, size);
>> }
>> EXPORT_SYMBOL_GPL(pstore_alloc_ring_buffer);
>>
>> ... and you have to find/make some global header for the "extern" declaration.
> I will make these RFC patch series according to our discussion. Thanks you very to
> valuable advice.

Sorry for seeing your email late.We already worked out some patches to restructure
pstore. Would you like to try patchset http://article.gmane.org/gmane.linux.kernel/1697680/?

We have more patches available to add some flags to disable/enable specific zones.

Yanmin


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: Should Pstore(ramoops) records customized information?
  2014-06-25  0:41           ` Zhang, Yanmin
@ 2014-06-25 13:08             ` Liu hua
  2014-06-26  0:57               ` Zhang, Yanmin
  0 siblings, 1 reply; 10+ messages in thread
From: Liu hua @ 2014-06-25 13:08 UTC (permalink / raw)
  To: Zhang, Yanmin, Luck, Tony, anton, ccross, keescook, linux-kernel
  Cc: Wang Nan, peifeiyue, Liu ShuoX

于 2014/6/25 8:41, Zhang, Yanmin 写道:
> 
> On 2014/6/20 18:47, Liu hua wrote:
>> On 2014/6/20 7:42, Luck, Tony wrote:
>>
>>>> BTW, I note that "extern struct pstore_info *psinfo" locates in
>>>> fs/pstore/internal.h. So users out of directory "fs/pstore/" can not use pstore to
>>>> record messages. We do not want other kernel users to use pstore, right?  And can we
>>>> break this?
>>> Yes we can make some interface visible to the rest of the kernel ... probably
>>> not the raw "*psinfo" though. Perhaps the pstore_alloc_ring_buffer() and
>>> pstore_write_ring_buffer() functions should be the ones exported to the
>>> rest of the kernel.
>>>
>>>> ditoo.. Since other backends like efi and erst may can not privide such ring buffer.
>>>> So pstore_alloc_ring_buffer should be a funciton pointer of pstore_info struct.
>>> Yes - that allows less capable backend like ERST and efivars to not provide the
>>> service.  Since it becomes internal, you can drop the "pstore_" prefix.  E.g.
>>> something like:
>>>
>>> int pstore_alloc_ring_buffer(char *name, int size)
>>> {
>>>     return psinfo->alloc_ring_buffer(name, size);
>>> }
>>> EXPORT_SYMBOL_GPL(pstore_alloc_ring_buffer);
>>>
>>> ... and you have to find/make some global header for the "extern" declaration.
>> I will make these RFC patch series according to our discussion. Thanks you very to
>> valuable advice.
> 
> Sorry for seeing your email late.We already worked out some patches to restructure
> pstore. Would you like to try patchset http://article.gmane.org/gmane.linux.kernel/1697680/?
> 
> We have more patches available to add some flags to disable/enable specific zones.

That's great! I have tried your patches. BTW, Your patches do not work on ARM platform,
before I changed linker scripts; And can we use this method in modules(I failed to do that)?

After a quick glance and try, I think my idea is a little different from yours. I will reply you
later.


> Yanmin
> 
> 
> .
> 



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: Should Pstore(ramoops) records customized information?
  2014-06-25 13:08             ` Liu hua
@ 2014-06-26  0:57               ` Zhang, Yanmin
  2014-06-27 12:06                 ` Liu hua
  0 siblings, 1 reply; 10+ messages in thread
From: Zhang, Yanmin @ 2014-06-26  0:57 UTC (permalink / raw)
  To: Liu hua, Luck, Tony, anton, ccross, keescook, linux-kernel
  Cc: Wang Nan, peifeiyue, Liu ShuoX, Rocher, Jeremy


On 2014/6/25 21:08, Liu hua wrote:
> 于 2014/6/25 8:41, Zhang, Yanmin 写道:
>> On 2014/6/20 18:47, Liu hua wrote:
>>> On 2014/6/20 7:42, Luck, Tony wrote:
>>>
>>>>> BTW, I note that "extern struct pstore_info *psinfo" locates in
>>>>> fs/pstore/internal.h. So users out of directory "fs/pstore/" can not use pstore to
>>>>> record messages. We do not want other kernel users to use pstore, right?  And can we
>>>>> break this?
>>>> Yes we can make some interface visible to the rest of the kernel ... probably
>>>> not the raw "*psinfo" though. Perhaps the pstore_alloc_ring_buffer() and
>>>> pstore_write_ring_buffer() functions should be the ones exported to the
>>>> rest of the kernel.
>>>>
>>>>> ditoo.. Since other backends like efi and erst may can not privide such ring buffer.
>>>>> So pstore_alloc_ring_buffer should be a funciton pointer of pstore_info struct.
>>>> Yes - that allows less capable backend like ERST and efivars to not provide the
>>>> service.  Since it becomes internal, you can drop the "pstore_" prefix.  E.g.
>>>> something like:
>>>>
>>>> int pstore_alloc_ring_buffer(char *name, int size)
>>>> {
>>>>      return psinfo->alloc_ring_buffer(name, size);
>>>> }
>>>> EXPORT_SYMBOL_GPL(pstore_alloc_ring_buffer);
>>>>
>>>> ... and you have to find/make some global header for the "extern" declaration.
>>> I will make these RFC patch series according to our discussion. Thanks you very to
>>> valuable advice.
>> Sorry for seeing your email late.We already worked out some patches to restructure
>> pstore. Would you like to try patchset http://article.gmane.org/gmane.linux.kernel/1697680/?
>>
>> We have more patches available to add some flags to disable/enable specific zones.
> That's great! I have tried your patches. BTW, Your patches do not work on ARM platform,
> before I changed linker scripts;

Initially, we just implemented it on x86. It's easy to extend it to ARM. Mostly change the arm
vmlinux.lds.S to add the sections. Pls. also change setup_arch to allocate memory blocks for
pstore.
In the patchset, there is an example patch, including reserve memory and zone examples.
Pls. reference it.

>   And can we use this method in modules(I failed to do that)?

It's a good question. There are many approaches to support modules.
1) Define the zone in built-in files and export it.Then, you can use it in module.
2) Define the zone and new tracer functions in built-in files and export
the tracer functions.

>
> After a quick glance and try, I think my idea is a little different from yours. I will reply you
> later.

Pls. Share your opinions. We are improving pstore to make it easier to be used.


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: Should Pstore(ramoops) records customized information?
  2014-06-26  0:57               ` Zhang, Yanmin
@ 2014-06-27 12:06                 ` Liu hua
  2014-06-30  9:05                   ` Zhang, Yanmin
  0 siblings, 1 reply; 10+ messages in thread
From: Liu hua @ 2014-06-27 12:06 UTC (permalink / raw)
  To: Zhang, Yanmin, Luck, Tony, anton, ccross, keescook, linux-kernel
  Cc: Wang Nan, peifeiyue, Liu ShuoX, Rocher, Jeremy, zhangzhiqiang.zhang

于 2014/6/26 8:57, Zhang, Yanmin 写道:
> 
> On 2014/6/25 21:08, Liu hua wrote:
>> 于 2014/6/25 8:41, Zhang, Yanmin 写道:
>>> On 2014/6/20 18:47, Liu hua wrote:
>>>> On 2014/6/20 7:42, Luck, Tony wrote:
>>>>
>>>>>> BTW, I note that "extern struct pstore_info *psinfo" locates in
>>>>>> fs/pstore/internal.h. So users out of directory "fs/pstore/" can not use pstore to
>>>>>> record messages. We do not want other kernel users to use pstore, right?  And can we
>>>>>> break this?
>>>>> Yes we can make some interface visible to the rest of the kernel ... probably
>>>>> not the raw "*psinfo" though. Perhaps the pstore_alloc_ring_buffer() and
>>>>> pstore_write_ring_buffer() functions should be the ones exported to the
>>>>> rest of the kernel.
>>>>>
>>>>>> ditoo.. Since other backends like efi and erst may can not privide such ring buffer.
>>>>>> So pstore_alloc_ring_buffer should be a funciton pointer of pstore_info struct.
>>>>> Yes - that allows less capable backend like ERST and efivars to not provide the
>>>>> service.  Since it becomes internal, you can drop the "pstore_" prefix.  E.g.
>>>>> something like:
>>>>>
>>>>> int pstore_alloc_ring_buffer(char *name, int size)
>>>>> {
>>>>>      return psinfo->alloc_ring_buffer(name, size);
>>>>> }
>>>>> EXPORT_SYMBOL_GPL(pstore_alloc_ring_buffer);
>>>>>
>>>>> ... and you have to find/make some global header for the "extern" declaration.
>>>> I will make these RFC patch series according to our discussion. Thanks you very to
>>>> valuable advice.
>>> Sorry for seeing your email late.We already worked out some patches to restructure
>>> pstore. Would you like to try patchset http://article.gmane.org/gmane.linux.kernel/1697680/?
>>>
>>> We have more patches available to add some flags to disable/enable specific zones.
>> That's great! I have tried your patches. BTW, Your patches do not work on ARM platform,
>> before I changed linker scripts;
> 
> Initially, we just implemented it on x86. It's easy to extend it to ARM. Mostly change the arm
> vmlinux.lds.S to add the sections. Pls. also change setup_arch to allocate memory blocks for
> pstore.
> In the patchset, there is an example patch, including reserve memory and zone examples.
> Pls. reference it.
> 
>>   And can we use this method in modules(I failed to do that)?
> 
> It's a good question. There are many approaches to support modules.
> 1) Define the zone in built-in files and export it.Then, you can use it in module.
> 2) Define the zone and new tracer functions in built-in files and export
> the tracer functions.
> 
>>
>> After a quick glance and try, I think my idea is a little different from yours. I will reply you
>> later.
> 
> Pls. Share your opinions. We are improving pstore to make it easier to be used.
> 

This feature can use in real products (actually we have done that), because usually several
mega-byte-size ram is enough and it is very useful for fault location. So I want that pstore
can be implemented in products, not just in labs.

These seems that there are at least two ways to make pstore visible to other kernel users:


(1) static allocation:(your way, maybe my description is not good, please correct me)

	When kernel image is made, zones are determinate;
	So if moudules want to use a zone, we should define it in kernel source before compiling
	and export it;

      (a)advantage:
	
	This method will not fail at most time if it passed at first time.

      (b)disadvantage:
	
	Engineer should change the kernel source code if he want to get a zone to record something. For lab,
        it is good enough; but for products, different products may use different kernel source codes if they
        want record different messages. It is very expensive.

(2) dynamic alloction: (ring buffer similar to your zone)
	
	(1) We should introduce metadata to describe the ring buffers in the ramoops bankend.
	    So when initializing, we just need to read metadata. then we know information of all ring buffers.
	    So we can read and manage all ring buffers in a list named "ring_buffer_list";

	(2) when we call pstore_alloc_ring_buffer(name,size). If "ring_buffer_list" contains this ring buffer,
	    we do nothing if size check passes. Else we create a ring buffer description and add it to list;

	(3) When we call pstore_write_ring_buffer(name,str). we find this ring buffer in "ring_buffer_list" by name;
	    and then copy strings to this ring buffer.
	
	
	disadvantage:

	   (1) So this alloc maybe fail when name is the same, but size isn't;
	   (2) We should find a way to format the backend. erase is not enough. (earse just clear the data of a ring buffer,
		but the ring buffer still exists).

	advantage:
	   (1) Os venders can implement this feature. Then people can use it even they can not compile the kernel.
	   (2) We can determin how to use the ramoops backend at runtime, rather than before compiling the kernel.



So which way do we really need?
And what do you think, Tony or others ?

Thanks,
Liu Hua




^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: Should Pstore(ramoops) records customized information?
  2014-06-27 12:06                 ` Liu hua
@ 2014-06-30  9:05                   ` Zhang, Yanmin
  0 siblings, 0 replies; 10+ messages in thread
From: Zhang, Yanmin @ 2014-06-30  9:05 UTC (permalink / raw)
  To: Liu hua, Luck, Tony, anton, ccross, keescook, linux-kernel
  Cc: Wang Nan, peifeiyue, Liu ShuoX, Rocher, Jeremy, zhangzhiqiang.zhang

On 2014/6/27 20:06, Liu hua wrote:
> 于 2014/6/26 8:57, Zhang, Yanmin 写道:
>> On 2014/6/25 21:08, Liu hua wrote:
>>> 于 2014/6/25 8:41, Zhang, Yanmin 写道:
>>>> On 2014/6/20 18:47, Liu hua wrote:
>>>>> On 2014/6/20 7:42, Luck, Tony wrote:
>>>>>
>>>>>>> BTW, I note that "extern struct pstore_info *psinfo" locates in
>>>>>>> fs/pstore/internal.h. So users out of directory "fs/pstore/" can not use pstore to
>>>>>>> record messages. We do not want other kernel users to use pstore, right?  And can we
>>>>>>> break this?
>>>>>> Yes we can make some interface visible to the rest of the kernel ... probably
>>>>>> not the raw "*psinfo" though. Perhaps the pstore_alloc_ring_buffer() and
>>>>>> pstore_write_ring_buffer() functions should be the ones exported to the
>>>>>> rest of the kernel.
>>>>>>
>>>>>>> ditoo.. Since other backends like efi and erst may can not privide such ring buffer.
>>>>>>> So pstore_alloc_ring_buffer should be a funciton pointer of pstore_info struct.
>>>>>> Yes - that allows less capable backend like ERST and efivars to not provide the
>>>>>> service.  Since it becomes internal, you can drop the "pstore_" prefix.  E.g.
>>>>>> something like:
>>>>>>
>>>>>> int pstore_alloc_ring_buffer(char *name, int size)
>>>>>> {
>>>>>>       return psinfo->alloc_ring_buffer(name, size);
>>>>>> }
>>>>>> EXPORT_SYMBOL_GPL(pstore_alloc_ring_buffer);
>>>>>>
>>>>>> ... and you have to find/make some global header for the "extern" declaration.
>>>>> I will make these RFC patch series according to our discussion. Thanks you very to
>>>>> valuable advice.
>>>> Sorry for seeing your email late.We already worked out some patches to restructure
>>>> pstore. Would you like to try patchset http://article.gmane.org/gmane.linux.kernel/1697680/?
>>>>
>>>> We have more patches available to add some flags to disable/enable specific zones.
>>> That's great! I have tried your patches. BTW, Your patches do not work on ARM platform,
>>> before I changed linker scripts;
>> Initially, we just implemented it on x86. It's easy to extend it to ARM. Mostly change the arm
>> vmlinux.lds.S to add the sections. Pls. also change setup_arch to allocate memory blocks for
>> pstore.
>> In the patchset, there is an example patch, including reserve memory and zone examples.
>> Pls. reference it.
>>
>>>    And can we use this method in modules(I failed to do that)?
>> It's a good question. There are many approaches to support modules.
>> 1) Define the zone in built-in files and export it.Then, you can use it in module.
>> 2) Define the zone and new tracer functions in built-in files and export
>> the tracer functions.
>>
>>> After a quick glance and try, I think my idea is a little different from yours. I will reply you
>>> later.
>> Pls. Share your opinions. We are improving pstore to make it easier to be used.
>>
> This feature can use in real products (actually we have done that), because usually several
> mega-byte-size ram is enough and it is very useful for fault location. So I want that pstore
> can be implemented in products, not just in labs.
>
> These seems that there are at least two ways to make pstore visible to other kernel users:
>
>
> (1) static allocation:(your way, maybe my description is not good, please correct me)
>
> 	When kernel image is made, zones are determinate;
> 	So if moudules want to use a zone, we should define it in kernel source before compiling
> 	and export it;
>
>        (a)advantage:
> 	
> 	This method will not fail at most time if it passed at first time.
>
>        (b)disadvantage:
> 	
> 	Engineer should change the kernel source code if he want to get a zone to record something. For lab,
>          it is good enough; but for products, different products may use different kernel source codes if they
>          want record different messages. It is very expensive.
>
> (2) dynamic alloction: (ring buffer similar to your zone)
> 	
> 	(1) We should introduce metadata to describe the ring buffers in the ramoops bankend.
> 	    So when initializing, we just need to read metadata. then we know information of all ring buffers.
> 	    So we can read and manage all ring buffers in a list named "ring_buffer_list";
>
> 	(2) when we call pstore_alloc_ring_buffer(name,size). If "ring_buffer_list" contains this ring buffer,
> 	    we do nothing if size check passes. Else we create a ring buffer description and add it to list;
>
> 	(3) When we call pstore_write_ring_buffer(name,str). we find this ring buffer in "ring_buffer_list" by name;
> 	    and then copy strings to this ring buffer.
> 	
> 	
> 	disadvantage:
>
> 	   (1) So this alloc maybe fail when name is the same, but size isn't;
> 	   (2) We should find a way to format the backend. erase is not enough. (earse just clear the data of a ring buffer,
> 		but the ring buffer still exists).
>
> 	advantage:
> 	   (1) Os venders can implement this feature. Then people can use it even they can not compile the kernel.
> 	   (2) We can determin how to use the ramoops backend at runtime, rather than before compiling the kernel.
>
>
>
> So which way do we really need?
> And what do you think, Tony or others ?

I got your pointer. It's indeed a special requirement that using pstore in modules without recompiling
kernel source codes, especially when OS vendors don't allow to recompile kernel codes.

We can extend pstore zone by below methods:
1) Add function register_dynamic_zone. It uses the same static ZONE data structure and defines most members of
ZONE like static zones.
2) Add a kernel boot cmdline parameter, which can define ZONE name and size. At booting early stage,
kernel parses the parameter and reserves memory block for these dynamic zones.
3) When a module calls register_dynamic_zone, kernel checks the reserved dynamic zone memory using
zone name as key, and uses that memory.

One issue is pstore core codes would create pstore file entries and copy old contents to cached memory. It's
hard to release the memory and files. It means we can only insmod the modules and can't rmmod the modules if
the modules use register_dynamic_zone.

Another issue is: usually we save callchain in traces in the ZONE. If you use a module and load it to kernel
randomly, the module might be in different virtual space addresses at different loading. So we need save
callchain symbol names instead of binary addresses, which might hurt performance too much.

Does it match with your requirement?

Yanmin


^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2014-06-30  9:06 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <539E6D4D.5000802@huawei.com>
2014-06-18 10:07 ` Should Pstore(ramoops) records customized information? Liu hua
2014-06-18 17:50   ` Luck, Tony
2014-06-19 12:21     ` Liu hua
2014-06-19 23:42       ` Luck, Tony
2014-06-20 10:47         ` Liu hua
2014-06-25  0:41           ` Zhang, Yanmin
2014-06-25 13:08             ` Liu hua
2014-06-26  0:57               ` Zhang, Yanmin
2014-06-27 12:06                 ` Liu hua
2014-06-30  9:05                   ` Zhang, Yanmin

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).