2002-08-20 04:25:11

by NeilBrown

[permalink] [raw]
Subject: PATCH - SUNRPC 1 of 3 - The new "sk_flags" word in struct svc_sock must be long....


As Dave Miller reminded me, 'flags' fields must be unsigned long.


----------- Diffstat output ------------
./include/linux/sunrpc/svcsock.h | 2 +-
1 files changed, 1 insertion(+), 1 deletion(-)

--- ./include/linux/sunrpc/svcsock.h 2002/08/20 03:00:47 1.1
+++ ./include/linux/sunrpc/svcsock.h 2002/08/20 03:01:25 1.2
@@ -22,7 +22,7 @@ struct svc_sock {

struct svc_serv * sk_server; /* service for this socket */
unsigned char sk_inuse; /* use count */
- unsigned int sk_flags;
+ unsigned long sk_flags;
#define SK_BUSY 0 /* enqueued/receiving */
#define SK_CONN 1 /* conn pending */
#define SK_CLOSE 2 /* dead or dying */


-------------------------------------------------------
This sf.net email is sponsored by: OSDN - Tired of that same old
cell phone? Get a new here for FREE!
https://www.inphonic.com/r.asp?r=sourceforge1&refcode1=vs3390
_______________________________________________
NFS maillist - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs


2002-08-20 22:54:01

by Benjamin LaHaise

[permalink] [raw]
Subject: Re: PATCH - SUNRPC 1 of 3 - The new "sk_flags" word in struct svc_sock must be long....

On Tue, Aug 20, 2002 at 02:24:56PM +1000, Neil Brown wrote:
> @@ -22,7 +22,7 @@ struct svc_sock {
>
> struct svc_serv * sk_server; /* service for this socket */
> unsigned char sk_inuse; /* use count */
> - unsigned int sk_flags;
> + unsigned long sk_flags;

Cool, 7 bytes of useless padding on 64 bit architectures.

-ben
--
"You will be reincarnated as a toad; and you will be much happier."


-------------------------------------------------------
This sf.net email is sponsored by: OSDN - Tired of that same old
cell phone? Get a new here for FREE!
https://www.inphonic.com/r.asp?r=sourceforge1&refcode1=vs3390
_______________________________________________
NFS maillist - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs

2002-08-20 23:16:17

by Trond Myklebust

[permalink] [raw]
Subject: Re: PATCH - SUNRPC 1 of 3 - The new "sk_flags" word in struct svc_sock must be long....

>>>>> " " == Benjamin LaHaise <[email protected]> writes:

> On Tue, Aug 20, 2002 at 02:24:56PM +1000, Neil Brown wrote:
>> @@ -22,7 +22,7 @@ struct svc_sock {
>>
>> struct svc_serv * sk_server; /* service for this socket */
>> unsigned char sk_inuse; /* use count */
>> - unsigned int sk_flags;
>> + unsigned long sk_flags;

> Cool, 7 bytes of useless padding on 64 bit architectures.

All down to the sad inability of certain 64-bit "processor architects"
to provide for atomic bit operations on 32-bit integers.
If the relatively minor bloat in knfsd worries you, then I advise you
to take a deep breath before peeking at struct page... 8-)

Cheers,
Trond


-------------------------------------------------------
This sf.net email is sponsored by: OSDN - Tired of that same old
cell phone? Get a new here for FREE!
https://www.inphonic.com/r.asp?r=sourceforge1&refcode1=vs3390
_______________________________________________
NFS maillist - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs

2002-08-20 23:22:01

by Benjamin LaHaise

[permalink] [raw]
Subject: Re: PATCH - SUNRPC 1 of 3 - The new "sk_flags" word in struct svc_sock must be long....

On Wed, Aug 21, 2002 at 01:16:12AM +0200, Trond Myklebust wrote:
> > On Tue, Aug 20, 2002 at 02:24:56PM +1000, Neil Brown wrote:
> >> @@ -22,7 +22,7 @@ struct svc_sock {
> >>
> >> struct svc_serv * sk_server; /* service for this socket */
> >> unsigned char sk_inuse; /* use count */
> >> - unsigned int sk_flags;
> >> + unsigned long sk_flags;
>
> > Cool, 7 bytes of useless padding on 64 bit architectures.
>
> All down to the sad inability of certain 64-bit "processor architects"
> to provide for atomic bit operations on 32-bit integers.
> If the relatively minor bloat in knfsd worries you, then I advise you
> to take a deep breath before peeking at struct page... 8-)

You've missed the point. Between sk_inuse and sk_flags is 7 bytes of
padding. Yes, I'm aware that sk_flags has 7 bytes of unused space, but
that's the unavoidable part. Personally, I think the sunrpc code should
do this particular operation differently.

-ben
--
"You will be reincarnated as a toad; and you will be much happier."


-------------------------------------------------------
This sf.net email is sponsored by: OSDN - Tired of that same old
cell phone? Get a new here for FREE!
https://www.inphonic.com/r.asp?r=sourceforge1&refcode1=vs3390
_______________________________________________
NFS maillist - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs

2002-08-20 23:32:31

by NeilBrown

[permalink] [raw]
Subject: Re: PATCH - SUNRPC 1 of 3 - The new "sk_flags" word in struct svc_sock must be long....

On Tuesday August 20, [email protected] wrote:
> On Wed, Aug 21, 2002 at 01:16:12AM +0200, Trond Myklebust wrote:
> > > On Tue, Aug 20, 2002 at 02:24:56PM +1000, Neil Brown wrote:
> > >> @@ -22,7 +22,7 @@ struct svc_sock {
> > >>
> > >> struct svc_serv * sk_server; /* service for this socket */
> > >> unsigned char sk_inuse; /* use count */
> > >> - unsigned int sk_flags;
> > >> + unsigned long sk_flags;
> >
> > > Cool, 7 bytes of useless padding on 64 bit architectures.
> >
> > All down to the sad inability of certain 64-bit "processor architects"
> > to provide for atomic bit operations on 32-bit integers.
> > If the relatively minor bloat in knfsd worries you, then I advise you
> > to take a deep breath before peeking at struct page... 8-)
>
> You've missed the point. Between sk_inuse and sk_flags is 7 bytes of
> padding. Yes, I'm aware that sk_flags has 7 bytes of unused space, but
> that's the unavoidable part. Personally, I think the sunrpc code should
> do this particular operation differently.

Code is welcome. Even detailed description of "differently" is
welcome.

At the time, I was interested in "cleaner" and "works", both of which
I achieved. "best" takes a bit longer.

NeilBrown


-------------------------------------------------------
This sf.net email is sponsored by: OSDN - Tired of that same old
cell phone? Get a new here for FREE!
https://www.inphonic.com/r.asp?r=sourceforge1&refcode1=vs3390
_______________________________________________
NFS maillist - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs

2002-08-20 23:44:57

by Trond Myklebust

[permalink] [raw]
Subject: Re: PATCH - SUNRPC 1 of 3 - The new "sk_flags" word in struct svc_sock must be long....

>>>>> " " == Benjamin LaHaise <[email protected]> writes:

> You've missed the point. Between sk_inuse and sk_flags is 7
> bytes of padding. Yes, I'm aware that sk_flags has 7 bytes of
> unused space, but that's the unavoidable part. Personally, I
> think the sunrpc code should do this particular operation
> differently.

Fair enough: I agree that sk_flags could be moved to the top of the
struct. However in practice I doubt you'll see any gain whatsoever,
given that all the struct svc_sock get kmalloc()ed one by one...

Cheers,
Trond


-------------------------------------------------------
This sf.net email is sponsored by: OSDN - Tired of that same old
cell phone? Get a new here for FREE!
https://www.inphonic.com/r.asp?r=sourceforge1&refcode1=vs3390
_______________________________________________
NFS maillist - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs