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

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

[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):
  *
  *
  * 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.

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

[Grant Grundler]

On Mon, Jul 21, 2008 at 02:51:08PM +0000, Joel Soete wrote:
> 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):
...
> 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.)

>
> 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.

> 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

>
> 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 majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Joel Soete]

Grant Grundler wrote:
> On Mon, Jul 21, 2008 at 02:51:08PM +0000, Joel Soete wrote:
>> 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):
> ...
>> 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.)
> 
Ok my worry was because other bits of this bytes was related to DMA behaviour of this U2 (Prefetch, Update, Lock, SafeDMA).

>> 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)

>> 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:
>> Well a short test seems to help but doesn't fix all issue:
>>
No that doesn't help at all :_(