Re: kill i_dev

Re: kill i_dev

[Andries.Brouwer]

    From: Alexander Viro <viro@math.psu.edu>

    On Thu, 21 Nov 2002 Andries.Brouwer@cwi.nl wrote:

    > Yes, I looked at that but concluded that someone (you?)
    > had added the assignment just to preserve the guarantee
    > previously given by get_empty_inode() at the moment it
    > was replaced by new_inode(). But it is superfluous, I think.

    Existing userland doesn't, though.

I saw that you hit fuser last year, but this is 2.5.
Any other examples?

Re: kill i_dev

[Andries.Brouwer]

> We can kill i_dev.  
> BTW, watch out for socket.c use of ->i_dev.

Yes, I looked at that but concluded that someone (you?)
had added the assignment just to preserve the guarantee
previously given by get_empty_inode() at the moment it
was replaced by new_inode(). But it is superfluous, I think.

> However, rdev and [cb]dev will have to remain separate.

We can fight later on that one. My point of view is
that just like i_dev is a field in i_sb, also i_rdev
can be retrieved from i_[cb]dev.

Something else is kdev_t. I liked it when it was a pointer.
Now it is just garbage and the kernel is full of conversions
to and from. Is there any reason not to throw out all of kdev_t?
That is, is there a reason to have a kdev_t different from dev_t?

Andries

Re: kill i_dev

[Alexander Viro]

On Thu, 21 Nov 2002 Andries.Brouwer@cwi.nl wrote:

> Yes, I looked at that but concluded that someone (you?)
> had added the assignment just to preserve the guarantee
> previously given by get_empty_inode() at the moment it
> was replaced by new_inode(). But it is superfluous, I think.

Existing userland doesn't, though.
 
> Something else is kdev_t. I liked it when it was a pointer.
> Now it is just garbage and the kernel is full of conversions
> to and from. Is there any reason not to throw out all of kdev_t?
> That is, is there a reason to have a kdev_t different from dev_t?

Yes - dev_t has legitimate reasons to exist.  kdev_t is a garbage
and its presence means that we need to fugure out what real objects
should be used in that particular context.

Note that in many cases these objects are, indeed, different.  So
blanket replacement of kdev_t with pointer to some type is *wrong*.
E.g. on the block side, we used to use kdev_t when we actually needed
	* filesystem
	* device node in some fs
	* device number from userland POV
	* struct block_device *, with associated cache, etc.
	* "disk" from driver POV (what had become struct gendisk *)
	* pointer to driver-specific data structure.

All of the above have different lifetime rules.  That alone means that
there is no hope in hell for a natural object unifying these uses.

And this information ("what kind of object are we talking about") was
simply not there - most of the stuff I'd been doing in 2.5 was *adding*
*that* *information* *into* *source*.  Several megabytes in 400-odd
patches...

Similar work needs to be done for character devices.  Then (and only then)
kdev_t can go.  Note that dev_t has only one legitimate use - it's when
we are talking about userland-supplied number without refering to internal
objects.

And yes, these conversions serve useful purpose - they mark the places
that need work.  Simple "oh, we'll just s/kdev_t/dev_t/" is not a good
step - more often than not it would just propagate dev_t to the places
where it doesn't belong.

kill i_dev

[Andries.Brouwer]

One disadvantage of enlarging the size of dev_t is
that struct inode grows. Bad.
We used to have i_dev and i_rdev; today i_rdev has split into
i_rdev, i_bdev and i_cdev. Bad.

It looks like these four fields can be replaced by a single one,
making struct inode smaller. Not bad.

The first step would be to delete the field i_dev, and
all assignments to it, and replace all remaining occurrences
by i_sb->s_dev.

Roughly speaking, the only use of this field is in the stat
system call.

Any objections?

Andries

Re: kill i_dev

[Alexander Viro]

On Thu, 21 Nov 2002 Andries.Brouwer@cwi.nl wrote:

> One disadvantage of enlarging the size of dev_t is
> that struct inode grows. Bad.
> We used to have i_dev and i_rdev; today i_rdev has split into
> i_rdev, i_bdev and i_cdev. Bad.
> 
> It looks like these four fields can be replaced by a single one,
> making struct inode smaller. Not bad.

No, they can't.  We _can_ put i_bdev/i_cdev into union and we can
kill i_dev.  However, rdev and [cb]dev will have to remain separate.

BTW, watch out for socket.c use of ->i_dev.