Re: [PATCH] linux-2.417 devfs 64bit portablility issue

[Richard Gooch]

Carsten Otte writes:
> the previous version of my patch did contain a bug,
> caused by incorrect parameter order when calling
> __devfs_unregister_major. The symptom was, that
> major numbers registered with devfs_register_*dev
> but not allocated with devfs_alloc_major were not
> freed by calling devfs_unregister_*dev. This is
> now fixed. Sorry, the patch is attached (due to Notes
> messing up with whitespace).
> (See attached file: linux-2.4.17-devfs_fixup.diff)

In future, please send patches in plain text, rather than
MIME-encoded.

> Richard, I would appreciate it if you could finally
> look into this.

I don't like your patch because:
- it replaces the bitfield with a character array. This in turn causes
  more data bloat (8 times more), and also prevents the use of the ffz
  functions

- you've mixed in a behavioural change to devfs_register_???dev() to
  call devfs_alloc_major() when the provided major is 0. While this
  might be good idea in the long run (but maybe not), it should be
  separated from the actual fix.

Unfortunately I've been busy chasing other (possible, not sure yet
what the source of the problems are) bugs, so I haven't had time to
code up a solution yet.

Is there any reason why switching from __u32 to unsigned long won't
work? Of course, the initialising values would need to be 64 bits wide
on a 64 bit machine, but that could probably be taken care of with
some clever macros (ab)use:

#if 64 bit
#define INITIALISER64(a,b) (a)<<32|(b)
#else
#define INITIALISER64(a,b) (a),(b)
#endif

static struct major_list block_major_list =
{SPIN_LOCK_UNLOCKED,
    {INITIALISER64(0xfffffb8f,0xffffffff),  /*  Majors 0   to 63    */
     INITIALISER64(0xfffffffe,0xff03ffef),  /*  Majors 64  to 127   */

and so on. Untested, and I only spent a few seconds thinking about it,
but perhaps this will solve it.

				Regards,

					Richard....
Permanent: [e-mail]
Current:   [e-mail]