NFS: set_bit on an 'int' variable OK for 64-bit?

Hello,

since test11, the NFS code uses the set_bit and related routines to manipulate the wb_flags member of the nfs_page struct (nfs_page.h). Unfortunately, wb_flags has still data type 'int'.

This is a problem (at least) on the 64-bit S/390 architecture, as our ..._bit macros assume bit 0 is the least significant bit of a 'long', which means due to big-endian byte order that bit 0 resides in the 7th byte of the variable. As an int occupies only 4 bytes, however, set_bit(0, int) clobbers memory.

Now the question is, who's correct?

At all other places (I found, at least), the ..._bit macros are indeed used only on 'long' variables (or arrays).

However, on the Alpha, the ..._bit routines assume bit 0 to be the least significant bit of an 'int'. Sparc64 on the other hand also uses 'long' :-/

What do you suggest we should do? Fix nfs_page to use a 'long'
variable, or change our bitops macros to use ints?

Mit freundlichen Gruessen / Best Regards

Ulrich Weigand

--
Dr. Ulrich Weigand
Linux for S/390 Design & Development
IBM Deutschland Entwicklung GmbH, Schoenaicher Str. 220, 71032 Boeblingen
Phone: +49-7031/16-3727 --- Email: [email protected]

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/

Ulrich> Hello,

Ulrich> since test11, the NFS code uses the set_bit and related Ulrich> routines to manipulate the wb_flags member of the nfs_page Ulrich> struct (nfs_page.h). Unfortunately, wb_flags has still data Ulrich> type 'int'.

Ulrich> This is a problem (at least) on the 64-bit S/390 architecture, Ulrich> as our ..._bit macros assume bit 0 is the least significant Ulrich> bit of a 'long', which means due to big-endian byte order that Ulrich> bit 0 resides in the 7th byte of the variable. As an int Ulrich> occupies only 4 bytes, however, set_bit(0, int) clobbers Ulrich> memory.

Ulrich> Now the question is, who's correct?

You are, the bit macros should only be used on long's. It happens to work on little endian bitfield machines like the Alpha but thats the same as when we had the problem with bit operations on char * in the file system code a couple of years ago.

The NFS code needs to be fixed for this then.

Jes
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/

NFS is wrong. Rusty did a complete audit of the code and I've been feeding some stuff to Linus. That one may have been missed

Fix NFS

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/

In message <[email protected]> you write:

Yes, didn't grep the headers. Hmm... that's the only one in include/linux/*.h though.

Yep, it's trivial.

Cheers,
Rusty.
--
Hacking time.

@@ -31,10 +31,10 @@
 	struct page		*wb_page;	/* page to read in/write out */
 	wait_queue_head_t	wb_wait;	/* wait queue */
 	unsigned long		wb_timeout;	/* when to read/write/commit */
+	unsigned long		wb_flags;	/* long req'd for set_bit */
 	unsigned int		wb_offset,	/* Offset of read/write */
 				wb_bytes,	/* Length of request */
-				wb_count,	/* reference count */
-				wb_flags;
+				wb_count;	/* reference count */
 	struct nfs_writeverf	wb_verf;	/* Commit cookie */
 };
-

To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/