mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* kill i_dev
@ 2002-11-21  0:07 Andries.Brouwer
  2002-11-21  0:39 ` Alexander Viro
  0 siblings, 1 reply; 5+ messages in thread
From: Andries.Brouwer @ 2002-11-21  0:07 UTC (permalink / raw)
  To: linux-kernel; +Cc: torvalds, viro

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


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: kill i_dev
  2002-11-21  0:07 kill i_dev Andries.Brouwer
@ 2002-11-21  0:39 ` Alexander Viro
  0 siblings, 0 replies; 5+ messages in thread
From: Alexander Viro @ 2002-11-21  0:39 UTC (permalink / raw)
  To: Andries.Brouwer; +Cc: linux-kernel, torvalds



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.


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: kill i_dev
@ 2002-11-21 18:12 Andries.Brouwer
  0 siblings, 0 replies; 5+ messages in thread
From: Andries.Brouwer @ 2002-11-21 18:12 UTC (permalink / raw)
  To: Andries.Brouwer, viro; +Cc: linux-kernel, torvalds

    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?

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: kill i_dev
  2002-11-21  1:33 Andries.Brouwer
@ 2002-11-21  2:58 ` Alexander Viro
  0 siblings, 0 replies; 5+ messages in thread
From: Alexander Viro @ 2002-11-21  2:58 UTC (permalink / raw)
  To: Andries.Brouwer; +Cc: linux-kernel, torvalds



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.


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: kill i_dev
@ 2002-11-21  1:33 Andries.Brouwer
  2002-11-21  2:58 ` Alexander Viro
  0 siblings, 1 reply; 5+ messages in thread
From: Andries.Brouwer @ 2002-11-21  1:33 UTC (permalink / raw)
  To: Andries.Brouwer, viro; +Cc: linux-kernel, torvalds

> 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


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2002-11-21 18:05 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2002-11-21  0:07 kill i_dev Andries.Brouwer
2002-11-21  0:39 ` Alexander Viro
2002-11-21  1:33 Andries.Brouwer
2002-11-21  2:58 ` Alexander Viro
2002-11-21 18:12 Andries.Brouwer

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®