Skip to content Skip to conversation

ccio_mark_invalid(): would it have to clear a bit or byte?

DISCUSSION Joel Soete 2 participants 3 messages Jul 21, 2008 – Aug 2, 2008

Share & tools

Joel Soete

author

2008-07-21 14:51 UTC #0 lore raw

Hello all,

given this comment:
* Given a virtual address (vba, arg2) and space id, (sid, arg1),
* load the I/O PDIR entry pointed to by pdir_ptr (arg0). Each IO Pdir
* entry consists of 8 bytes as shown below (MSB == bit 0):
*
*
* WORD 0:
* +------+----------------+-----------------------------------------------+

 * | Phys | Virtual Index  |               Phys                            |
 * | 0:3  |     0:11       |               4:19                            |
 * |4 bits|   12 bits      |              16 bits                          |
 * +------+----------------+-----------------------------------------------+
 * WORD 1:
 * +-----------------------+-----------------------------------------------+
 * |      Phys    |  Rsvd  | Prefetch |Update |Rsvd  |Lock  |Safe  |Valid  |
 * |     20:39    |        | Enable   |Enable |      |Enable|DMA   |       |
 * |    20 bits   | 5 bits | 1 bit    |1 bit  |2 bits|1 bit |1 bit |1 bit  |

* +-----------------------+-----------------------------------------------+ *

and also this:

        while (byte_cnt > 0) {
                /* clear I/O Pdir entry "valid" bit first */
                ((unsigned char *) pdir_ptr)[7] = 0;

So if I well understand 'Valid' field of a pdir entry is well of 1 bit but the code cleanup a all byte?

Is coding something like:
#define PTE_VALID_BIT_MASK 0xfffffffffffffffeULL

		*pdir_ptr &= PTE_VALID_BIT_MASK;

wouldn't do better what comment says it does?

Well a short test seems to help but doesn't fix all issue:

patst007 login: end_request: I/O error, dev sdl, sector 6659574 __ratelimit: 32 messages suppressed

Buffer I/O error on device sdl9, logical block 193374

lost page write due to I/O error on sdl9

Buffer I/O error on device sdl9, logical block 193375

lost page write due to I/O error on sdl9

Buffer I/O error on device sdl9, logical block 193376

lost page write due to I/O error on sdl9

Buffer I/O error on device sdl9, logical block 193377

lost page write due to I/O error on sdl9

Buffer I/O error on device sdl9, logical block 193378

lost page write due to I/O error on sdl9

Buffer I/O error on device sdl9, logical block 193379

lost page write due to I/O error on sdl9

Buffer I/O error on device sdl9, logical block 193380

lost page write due to I/O error on sdl9 Buffer I/O error on device sdl9, logical block 193381 lost page write due to I/O error on sdl9 Buffer I/O error on device sdl9, logical block 193382 lost page write due to I/O error on sdl9 Buffer I/O error on device sdl9, logical block 193383

Tia for all advise,

J.

Grant Grundler

2008-08-01 16:12 UTC #1 lore raw

Quoted 6 lines · Joel Soete

Hello all,
given this comment:
* Given a virtual address (vba, arg2) and space id, (sid, arg1),
* load the I/O PDIR entry pointed to by pdir_ptr (arg0). Each IO Pdir
* entry consists of 8 bytes as shown below (MSB == bit 0):

...

Quoted 7 lines

and also this:
        while (byte_cnt > 0) {
                /* clear I/O Pdir entry "valid" bit first */
                ((unsigned char *) pdir_ptr)[7] = 0;
So if I well understand 'Valid' field of a pdir entry is well of 1 bit but the code cleanup a all byte?

That's just a convenient way to clobber the bit we care about. The fact that the rest of the pdir remains available is irrelevant except for debugging (when we might dump IO Pdir to see the history.)

Quoted 5 lines

Is coding something like:
#define PTE_VALID_BIT_MASK 0xfffffffffffffffeULL
		*pdir_ptr &= PTE_VALID_BIT_MASK;

That's a load/modify store of a 64-bit value.
That substantially more instructions than a single byte store.

Quoted 1 line

wouldn't do better what comment says it does?

Yes, but the comment is just describing what needs to happen, not exactly how. The "entry valid" bit just needs to be cleared and it doesn't matter how.

hth
grant

Quoted 31 lines

Well a short test seems to help but doesn't fix all issue:
patst007 login: end_request: I/O error, dev sdl, sector 6659574 __ratelimit: 32 messages suppressed
Buffer I/O error on device sdl9, logical block 193374 lost page write due to I/O error on sdl9
Buffer I/O error on device sdl9, logical block 193375 lost page write due to I/O error on sdl9
Buffer I/O error on device sdl9, logical block 193376 lost page write due to I/O error on sdl9
Buffer I/O error on device sdl9, logical block 193377 lost page write due to I/O error on sdl9
Buffer I/O error on device sdl9, logical block 193378 lost page write due to I/O error on sdl9
Buffer I/O error on device sdl9, logical block 193379 lost page write due to I/O error on sdl9
Buffer I/O error on device sdl9, logical block 193380 lost page write due to I/O error on sdl9
Buffer I/O error on device sdl9, logical block 193381 lost page write due to I/O error on sdl9
Buffer I/O error on device sdl9, logical block 193382 lost page write due to I/O error on sdl9
Buffer I/O error on device sdl9, logical block 193383
Tia for all advise,
	J.
--
To unsubscribe from this list: send the line "unsubscribe linux-parisc" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Joel Soete

author

2008-08-02 07:59 UTC #2 lore raw

Quoted 13 lines · Grant Grundler

Quoted 6 lines · Joel Soete
Hello all,
given this comment:
* Given a virtual address (vba, arg2) and space id, (sid, arg1),
* load the I/O PDIR entry pointed to by pdir_ptr (arg0). Each IO Pdir
* entry consists of 8 bytes as shown below (MSB == bit 0):
...
Quoted 5 lines
and also this:
        while (byte_cnt > 0) {
                /* clear I/O Pdir entry "valid" bit first */
                ((unsigned char *) pdir_ptr)[7] = 0;

Quoted 6 lines

Quoted 1 line
So if I well understand 'Valid' field of a pdir entry is well of 1 bit but the code cleanup a all byte?
That's just a convenient way to clobber the bit we care about. The fact that the rest of the pdir remains available is irrelevant except for debugging (when we might dump IO Pdir to see the history.)

Ok my worry was because other bits of this bytes was related to DMA behaviour of this U2 (Prefetch, Update, Lock, SafeDMA).

Quoted 8 lines

Quoted 4 lines
Is coding something like:
#define PTE_VALID_BIT_MASK 0xfffffffffffffffeULL
		*pdir_ptr &= PTE_VALID_BIT_MASK;
That's a load/modify store of a 64-bit value.
That substantially more instructions than a single byte store.

Agree (but I am looking first to make this driver reliable)

Quoted 8 lines

Quoted 1 line
wouldn't do better what comment says it does?
Yes, but the comment is just describing what needs to happen, not exactly how. The "entry valid" bit just needs to be cleared and it doesn't matter how.
hth
grant

Tx,

J.

PS:

Quoted 2 lines

Quoted 2 lines
Well a short test seems to help but doesn't fix all issue:

No that doesn't help at all :_(