Re: [PATCH v1 06/11] Documentation: pstore/blk: blkoops: create document for pstore_blk

[Randy Dunlap]

On 1/20/20 9:23 PM, liaoweixiong wrote:
> hi Randy Dunlap,
> 
> On 2020/1/21 PM12:13, Randy Dunlap wrote:
>> Hi,
>>
>> I have some documentation comments for you:
>>
>>
>> On 1/19/20 5:03 PM, WeiXiong Liao wrote:
>>> The document, at Documentation/admin-guide/pstore-block.rst, tells us
>>> how to use pstore/blk and blkoops.
>>>
>>> Signed-off-by: WeiXiong Liao <liaoweixiong@xxxxxxxxxxxxxxxxx>
>>> ---
>>>  Documentation/admin-guide/pstore-block.rst | 278 +++++++++++++++++++++++++++++
>>>  MAINTAINERS                                |   1 +
>>>  fs/pstore/Kconfig                          |   2 +
>>>  3 files changed, 281 insertions(+)
>>>  create mode 100644 Documentation/admin-guide/pstore-block.rst
>>>
>>> diff --git a/Documentation/admin-guide/pstore-block.rst b/Documentation/admin-guide/pstore-block.rst
>>> new file mode 100644
>>> index 000000000000..58418d429c55
>>> --- /dev/null
>>> +++ b/Documentation/admin-guide/pstore-block.rst
>>> +
>>> +
>>> +dmesg_size
>>> +~~~~~~~~~~
>>> +
>>> +The chunk size in bytes for dmesg(oops/panic). It **MUST** be a multiple of
>>> +4096. If you don't need it, safely set it 0 or ignore it.
>>
>>                                       set it to 0 or ignore it.
>>
> 
> I will fix it, thank you.
> 
>> The example above is:  blkoops.dmesg_size=64
>> where 64 is not a multiple of 4096. (?)
>>
> 
> The module parameter dmesg_size is in unit KB.

I didn't see that documented anywhere.


>>> +Normally the number of bytes written should be returned, while for error,
>>> +negative number should be returned.
>>> +
>>> +panic_write (for block device)
>>> +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>> +
>>> +It's much similar to panic_write for non-block device, but panic_write for
>>> +block device writes alignment to SECTOR_SIZE, that's why the parameters are
>>
>>                 writes only aligned sectors of SECTOR_SIZE  (??)
>>
> 
> How about this?
> 
> It's much similar to panic_write for non-block device, but the position and
> data size of panic_write for block device must be aligned to SECTOR_SIZE,
> that's why the parameters are @sects and @start_sect. Block device driver
> should register it by ``blkoops_register_blkdev``.

OK.

>>> +@sects and @start_sect. Block device driver should register it by
>>> +``blkoops_register_blkdev``.
>>> +
>>> +The parameter @start_sect is the relative position of the block device and
>>> +partition. If block driver requires absolute position for panic_write,
>>> +``blkoops_blkdev_info`` will be helpful, which can provide the absolute
>>> +position of the block device (or partition) on the whole disk/flash.
>>> +
>>> +Normally zero should be returned, otherwise it indicates an error.
>>> +
>>> +Compression and header
>>> +----------------------
>>> +
>>> +Block device is large enough for uncompressed dmesg data. Actually we do not
>>> +recommend data compression because pstore/blk will insert some information into
>>> +the first line of dmesg data. For example::
>>> +
>>> +        Panic: Total 16 times
>>> +
>>> +It means that it's the 16th times panic log since the first booting. Sometimes
>>
>>                                time of a panic log since ...
>>
> 
> Should it be like this?
> It means the time of a panic log since the first booting.

That sounds like clock time, not the number of instances or occurrences.

> 
>>> +the oops|panic occurs since burning is very important for embedded device to
>>
>>                                ^^^^^^^ huh??
>>
> 
> How about this?
> 
> Sometimes the number of occurrences of oops|panic since the first
> booting is important
> to judge whether the system is stable.

OK.

>>> +judge whether the system is stable.
>>> +
>>> +The following line is inserted by pstore filesystem. For example::
>>> +
>>> +        Oops#2 Part1
>>> +
>>> +It means that it's the 2nd times oops log on last booting.
>>
>>                           2nd time of an oops log on the last boot. (?)
>>
> 
> How about this?
> 
> It means that it's OOPS for the 2nd time on the last boot.

OK. It's an oops counter.

>>> +#. Just use CPU to transfer.
>>> +   Do not use DMA to transfer unless you are sure that DMA will not keep lock.
>>> +#. Operate register directly.
>>
>>       Don't know what that means.
>>
> 
> How about this?
> 
> #. Control registers directly.
>     Please control registers directly rather than use Linux kernel
> resources.

OK.

>     Do I/O map while initializing rather than wait until a panic occurs.
> 
>>> +   Try not to use Linux kernel resources. Do I/O map while initializing rather
>>> +   than waiting until the panic.


-- 
~Randy