Benjamin Herrenschmidt wrote:
On Thu, 2005-07-07 at 11:41 -0700, Greg KH wrote:
How about the issue of tying this into the other pci error reporting
infrastructure that is being worked on?
The other infrastructure is for asynchronous reporting and recovery.
We still need synchronous detection & reporting. So this is a bit
different.
The interesting point is that it seems that both could use for reporting.
However, it would be nice if Hidetoshi's work could be adapted a bit so
that 1) naming is a bit more consistent with the other stuff (pcierr_*
maybe) and 2) the error "token" is the same. The later is especially
important if we start adding ways to query the error token to know what
the error precisely was etc... There is no reason to have 2 different
ways of representing error details.
The naming doesn't really matter. iochk_* is just my preference.
However it would be worth a try to move generic codes from iomap.*
(historical home) to pci.* and rename iochk_* to pcierr_*.
Well, I'd like to use this opportunity to sort out my thoughts...
Now iochk_read() returns a boolean value, whether there was a error
or not. Of course I agree that it would be more useful if it can return
the detail of the error.
A quick solution is return "token" instead of boolean value 1.
-extern int iochk_read(iocookie *cookie);
+extern token iochk_read(iocookie *cookie);
The token should be a pointer or a bitmask having proper flags, not 0.
So this still work:
if(iochk_read(cookie)) return -EIO;
and now this will also work:
if((token=iochk_read(cookie))!=0){
switch(severity(token)) {
...
}}
The error "token", tentatively named "pci_error_token" in document
you posted, is now temporarily defined in recent Linas's patch using
other alias:
enum pci_channel_state {
pci_channel_io_normal = 0, /* I/O channel is in normal state */
pci_channel_io_frozen = 1, /* I/O to channel is blocked */
pci_channel_io_perm_failure, /* pci card is dead */
};
Of course this will be not enough in near future.
I have already agree with what few month ago you said:
The token should be an opaque type with accessors. You could define a
pci_error_get_severity(token) to return the severity. The idea is to
define accessors which return an error when the data requested isn't
present in the error info. The actual content of the token is to be
defined. I was thinking about a type plus a union. I was hoping Seto
could provide something here ...
For example:
/* offsets */
enum {
pcierr_io_frozen = 0, /* I/O to channel is blocked */
pcierr_io_perm_failure, /* pci card is dead */
pcierr_severity_valid, /* 1:valid */
pcierr_severity, /* 0:non-fatal 1:fatal */
};
#define pcierr_perm_failure(token) \
test_bit(pcierr_io_perm_failure, token)
int pcierr_get_severity(token)
{
if(test_bit(pcierr_severity_valid, token)) {
return test_bit(pcierr_severity, token);
}
return -1;
}
Objections?
Are there any better place to put a group of codes like above than
include/linux/pci.h?
By the way, if token says frozen but not perm_failure, is it mean
"recovery processing now"? Are there any special state which
synchronous detection have to report?
Thanks,
H.Seto
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
| |
 |