Re: [PATCH] struct char_device

[Stephen C. Tweedie]

Hi,

On Wed, May 23, 2001 at 01:54:15PM -0400, Alexander Viro wrote:
> On Wed, 23 May 2001 [EMAIL PROTECTED] wrote:
> 
> > > But I don't want an initrd.
> > Don't be afraid of words. You wouldnt notice - it would do its
> > job and disappear just like piggyback today.
> 
> Andries, initrd code is _sick_. Our boot sequence is not a wonder of
> elegance, but that crap is the worst part. I certainly can understand
> people not wanting to use that on aesthetical reasons alone.

It's the principle of a kernel-linked boot image, not initrd, which is
important.  Unpacking a cramfs image from an __init section is much
cleaner than initrd and has almost the same effect: the boot
filesystem just ends up readonly that way.  Either way we can hook
in that default user-space code at boot time transparently.

--Stephen
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] struct char_device

[Andries . Brouwer]

> Besides, just on general principles, we'd better have clean interface
> for changing partitioning

It is not quite clear to me what you are arguing for or against.
But never mind - I'll leave few hours from now.
When the time is there I'll show you an implementation,
and if you don't like it, you'll show me a better one.

Andries
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] struct char_device

[Alexander Viro]

On Wed, 23 May 2001 [EMAIL PROTECTED] wrote:

> > Andries, initrd code is _sick_.
> 
> Oh, but the fact that there exists a bad implementation
> does not mean the idea is wrong. It is really easy to
> make an elegant implementation.

Andries, I've been doing cleanups of that logics (see namespaces-patch -
they've got merged into it) and I have to say that you are sadly mistaken.

It's not just an implementation that is ugly, it's behaviour currently
implemented (and relied upon by existing boot setups) that is extremely
ill-defined and crufty. I would rather get rid of the abortion, but that
is impossible without breaking tons of existing setups.

And that _is_ an area where "we can do something vaguely similar to
current behaviour that wouldn't take pages to describe" does not work.
You don't fsck with others' boot sequences, unless you want a free
tar-and-feather ride. I don't want it.

Besides, just on general principles, we'd better have clean interface
for changing partitioning on the kernel side and rip that crap out of
$BIGNUM fdisk implmentations. It can be made modular, so problem of
teaching it new types of partitions is not hard.

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] struct char_device

[Andries . Brouwer]

> Andries, initrd code is _sick_.

Oh, but the fact that there exists a bad implementation
does not mean the idea is wrong. It is really easy to
make an elegant implementation.

Andries
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] struct char_device

[Wayne . Brown]

[EMAIL PROTECTED] on 05/23/2001 08:34:44 AM

To:   [EMAIL PROTECTED], [EMAIL PROTECTED], [EMAIL PROTECTED]
cc:(bcc: Wayne Brown/Corporate/Altec)

Subject:  Re: [PATCH] struct char_device



>> But I don't want an initrd.
>
>Don't be afraid of words. You wouldnt notice - it would do its
>job and disappear just like piggyback today.

So nothing in the boot scripts would have to change?  (Not that it would be a
big problem if it was necessary.  However, I prefer to keep things in /etc/rc.d
as close to the distribution defaults as possible, just in case I ever need to
reinstall the distribution.)

Wayne




-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] struct char_device

[Alexander Viro]

On Wed, 23 May 2001 [EMAIL PROTECTED] wrote:

> > But I don't want an initrd.
> 
> Don't be afraid of words. You wouldnt notice - it would do its
> job and disappear just like piggyback today.

Andries, initrd code is _sick_. Our boot sequence is not a wonder of
elegance, but that crap is the worst part. I certainly can understand
people not wanting to use that on aesthetical reasons alone.

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] struct char_device

[Andries . Brouwer]

> But I don't want an initrd.

Don't be afraid of words. You wouldnt notice - it would do its
job and disappear just like piggyback today.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] struct char_device

[Helge Hafting]

[EMAIL PROTECTED] wrote:
> 
> From [EMAIL PROTECTED] Wed May 23 14:16:46 2001
> 
> > It is entirely possible to remove all partition table handling code
> > from the kernel. User space can figure out where the partitions
> > are supposed to be and tell the kernel.
> > For the initial boot this user space can be in an initrd,
> > or it could just be a boot parameter: rootdev=/dev/hda,
> > rootpartition:offset=N,length=L, rootfstype=ext3.
> 
> Not if you want compatibility.
> 
> I don't think compatibility is a problem.
> It would go like this: at configure time you get the
> choice of the default initrd or a custom initrd.

But I don't want an initrd.  I want to get the root fs directly from
disk the way I always have.  Initrd may be useful for install floppies
and such, not something I want for an ordinary installed system.

The kernel parameter way is better, just add to lilo.conf
and it works.  Forcing an initrd is also incompatibility,
because I don't use one now.

Helge Hafting
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] struct char_device

[Martin Dalecki]

Linus Torvalds wrote:
> 
> On Tue, 22 May 2001, Jeff Garzik wrote:
> >
> > IMHO it would be nice to (for 2.4) create wrappers for accessing the
> > block arrays, so that we can more easily dispose of the arrays when 2.5
> > rolls around...
> 
> No.
> 
> We do not create wrappers "so that we can easily change the implementation
> when xxx happens".
> 
> That way lies bad implementations.

However Linus please note that in the case of the bould arrays
used in device handling code we have code patterns like this:

if (blah[major]) {
size = blah[major][minor]
} else
size = some default;

And those have to by dragged throughout the whole places where
the arrays get used. Thus making some wrappers (many are already in
place):

1. Prevents typo kind of programming errors.

2. Possibly make the code more explicit.

and please don't forget:

3. Allows to change the underlying implementation in some soon point in
time.

However I agree that *without* the above arguments such kind of wrappers
would make the overall code as unreadable as C++ code frequently is,
which
tryies to preserve private: attributes at simple field cases..
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] struct char_device

[Andries . Brouwer]

From [EMAIL PROTECTED] Wed May 23 14:16:46 2001

> It is entirely possible to remove all partition table handling code
> from the kernel. User space can figure out where the partitions
> are supposed to be and tell the kernel.
> For the initial boot this user space can be in an initrd,
> or it could just be a boot parameter: rootdev=/dev/hda,
> rootpartition:offset=N,length=L, rootfstype=ext3.

Not if you want compatibility.

I don't think compatibility is a problem.
It would go like this: at configure time you get the
choice of the default initrd or a custom initrd.
If you choose the custom one you construct it yourself.
If you choose the default one, then you get something
that comes together with the kernel image, just like
the piggyback stuff today. This default initrd does
the partition parsing that up to now the kernel did.
That way nobody need to notice a difference, except for
those who use initrd already now. They can solve their
problems.

Andries


-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] struct char_device

[Alan Cox]

> It is entirely possible to remove all partition table handling code
> from the kernel. User space can figure out where the partitions
> are supposed to be and tell the kernel.
> For the initial boot this user space can be in an initrd,
> or it could just be a boot parameter: rootdev=/dev/hda,
> rootpartition:offset=N,length=L, rootfstype=ext3.

Not if you want compatibility.


-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] struct char_device

[Andries . Brouwer]

Alan writes:

The current partitioning code consists of re-reading from disk. That is 
code that has to be present anyway even without an initrd since it is
needed for mounting other filesystems

I am not quite sure what you are saying here.
(For example, the "even" was unexpected.)

It is entirely possible to remove all partition table handling code
from the kernel. User space can figure out where the partitions
are supposed to be and tell the kernel.
For the initial boot this user space can be in an initrd,
or it could just be a boot parameter: rootdev=/dev/hda,
rootpartition:offset=N,length=L, rootfstype=ext3.

mount does not need any partition reading code.

Andries


[I conjecture that we'll want to start moving partition parsing
out into userspace fairly soon. Indeed, soon we'll see EFI everywhere,
and there is no good reason to build knowledge about that into the kernel.]
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] struct char_device

[Alan Cox]

> Because we still need the partitioning code for backwards
> compatibility. There's no way I'm going to use initrd to do partition
> setup with lvmtools etc.

The current partitioning code consists of re-reading from disk. That is 
code that has to be present anyway even without an initrd since it is needed
for mounting other filesystems

> Also, lvm and friends are _heavyweight_. The partitioning stuff should be
> _one_ add (and perhaps a range check) at bh submit time. None of this
> remapping crap. We don't need no steenking overhead for something we need
> to do anyway.

Thats the kind of LVM layer I am thinking in terms of, not a large block
of LVM code. And for that matter the partition scanner and modifying code
can even be seperate dynamic loaded modules.

At the moment ponder what happens if you have I/Os in flight to /dev/hda3
when you delete /dev/hda2

Alan



-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] struct char_device

[Andries . Brouwer]

On Wed, 23 May 2001 [EMAIL PROTECTED] wrote:
>
> > why not implement partitions as simply doing block remaps
> 
> Everybody agrees.

No they don't.

We had this discussion already. We all agree.
Maybe you read in "remap" something other than a simple addition
but I don't. This is not a topic where further discussion is needed.

Andries
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] struct char_device

[Andries . Brouwer]

On Wed, 23 May 2001 [EMAIL PROTECTED] wrote:

  why not implement partitions as simply doing block remaps
 
 Everybody agrees.

No they don't.

We had this discussion already. We all agree.
Maybe you read in remap something other than a simple addition
but I don't. This is not a topic where further discussion is needed.

Andries
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] struct char_device

[Alan Cox]

 It is entirely possible to remove all partition table handling code
 from the kernel. User space can figure out where the partitions
 are supposed to be and tell the kernel.
 For the initial boot this user space can be in an initrd,
 or it could just be a boot parameter: rootdev=/dev/hda,
 rootpartition:offset=N,length=L, rootfstype=ext3.

Not if you want compatibility.


-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] struct char_device

[Martin Dalecki]

Linus Torvalds wrote:
 
 On Tue, 22 May 2001, Jeff Garzik wrote:
 
  IMHO it would be nice to (for 2.4) create wrappers for accessing the
  block arrays, so that we can more easily dispose of the arrays when 2.5
  rolls around...
 
 No.
 
 We do not create wrappers so that we can easily change the implementation
 when xxx happens.
 
 That way lies bad implementations.

However Linus please note that in the case of the bould arrays
used in device handling code we have code patterns like this:

if (blah[major]) {
size = blah[major][minor]
} else
size = some default;

And those have to by dragged throughout the whole places where
the arrays get used. Thus making some wrappers (many are already in
place):

1. Prevents typo kind of programming errors.

2. Possibly make the code more explicit.

and please don't forget:

3. Allows to change the underlying implementation in some soon point in
time.

However I agree that *without* the above arguments such kind of wrappers
would make the overall code as unreadable as C++ code frequently is,
which
tryies to preserve private: attributes at simple field cases..
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] struct char_device

[Helge Hafting]

[EMAIL PROTECTED] wrote:
 
 From [EMAIL PROTECTED] Wed May 23 14:16:46 2001
 
  It is entirely possible to remove all partition table handling code
  from the kernel. User space can figure out where the partitions
  are supposed to be and tell the kernel.
  For the initial boot this user space can be in an initrd,
  or it could just be a boot parameter: rootdev=/dev/hda,
  rootpartition:offset=N,length=L, rootfstype=ext3.
 
 Not if you want compatibility.
 
 I don't think compatibility is a problem.
 It would go like this: at configure time you get the
 choice of the default initrd or a custom initrd.

But I don't want an initrd.  I want to get the root fs directly from
disk the way I always have.  Initrd may be useful for install floppies
and such, not something I want for an ordinary installed system.

The kernel parameter way is better, just add to lilo.conf
and it works.  Forcing an initrd is also incompatibility,
because I don't use one now.

Helge Hafting
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] struct char_device

[Andries . Brouwer]

 But I don't want an initrd.

Don't be afraid of words. You wouldnt notice - it would do its
job and disappear just like piggyback today.
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] struct char_device

[Alexander Viro]

On Wed, 23 May 2001 [EMAIL PROTECTED] wrote:

  But I don't want an initrd.
 
 Don't be afraid of words. You wouldnt notice - it would do its
 job and disappear just like piggyback today.

Andries, initrd code is _sick_. Our boot sequence is not a wonder of
elegance, but that crap is the worst part. I certainly can understand
people not wanting to use that on aesthetical reasons alone.

-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] struct char_device

[Wayne . Brown]

[EMAIL PROTECTED] on 05/23/2001 08:34:44 AM

To:   [EMAIL PROTECTED], [EMAIL PROTECTED], [EMAIL PROTECTED]
cc:(bcc: Wayne Brown/Corporate/Altec)

Subject:  Re: [PATCH] struct char_device



 But I don't want an initrd.

Don't be afraid of words. You wouldnt notice - it would do its
job and disappear just like piggyback today.

So nothing in the boot scripts would have to change?  (Not that it would be a
big problem if it was necessary.  However, I prefer to keep things in /etc/rc.d
as close to the distribution defaults as possible, just in case I ever need to
reinstall the distribution.)

Wayne




-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] struct char_device

[Andries . Brouwer]

 Andries, initrd code is _sick_.

Oh, but the fact that there exists a bad implementation
does not mean the idea is wrong. It is really easy to
make an elegant implementation.

Andries
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] struct char_device

[Alexander Viro]

On Wed, 23 May 2001 [EMAIL PROTECTED] wrote:

  Andries, initrd code is _sick_.
 
 Oh, but the fact that there exists a bad implementation
 does not mean the idea is wrong. It is really easy to
 make an elegant implementation.

Andries, I've been doing cleanups of that logics (see namespaces-patch -
they've got merged into it) and I have to say that you are sadly mistaken.

It's not just an implementation that is ugly, it's behaviour currently
implemented (and relied upon by existing boot setups) that is extremely
ill-defined and crufty. I would rather get rid of the abortion, but that
is impossible without breaking tons of existing setups.

And that _is_ an area where we can do something vaguely similar to
current behaviour that wouldn't take pages to describe does not work.
You don't fsck with others' boot sequences, unless you want a free
tar-and-feather ride. I don't want it.

Besides, just on general principles, we'd better have clean interface
for changing partitioning on the kernel side and rip that crap out of
$BIGNUM fdisk implmentations. It can be made modular, so problem of
teaching it new types of partitions is not hard.

-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] struct char_device

[Andries . Brouwer]

 Besides, just on general principles, we'd better have clean interface
 for changing partitioning

It is not quite clear to me what you are arguing for or against.
But never mind - I'll leave few hours from now.
When the time is there I'll show you an implementation,
and if you don't like it, you'll show me a better one.

Andries
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] struct char_device

[Andries . Brouwer]

Alan writes:

The current partitioning code consists of re-reading from disk. That is 
code that has to be present anyway even without an initrd since it is
needed for mounting other filesystems

I am not quite sure what you are saying here.
(For example, the even was unexpected.)

It is entirely possible to remove all partition table handling code
from the kernel. User space can figure out where the partitions
are supposed to be and tell the kernel.
For the initial boot this user space can be in an initrd,
or it could just be a boot parameter: rootdev=/dev/hda,
rootpartition:offset=N,length=L, rootfstype=ext3.

mount does not need any partition reading code.

Andries


[I conjecture that we'll want to start moving partition parsing
out into userspace fairly soon. Indeed, soon we'll see EFI everywhere,
and there is no good reason to build knowledge about that into the kernel.]
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] struct char_device

[Andries . Brouwer]

From [EMAIL PROTECTED] Wed May 23 14:16:46 2001

 It is entirely possible to remove all partition table handling code
 from the kernel. User space can figure out where the partitions
 are supposed to be and tell the kernel.
 For the initial boot this user space can be in an initrd,
 or it could just be a boot parameter: rootdev=/dev/hda,
 rootpartition:offset=N,length=L, rootfstype=ext3.

Not if you want compatibility.

I don't think compatibility is a problem.
It would go like this: at configure time you get the
choice of the default initrd or a custom initrd.
If you choose the custom one you construct it yourself.
If you choose the default one, then you get something
that comes together with the kernel image, just like
the piggyback stuff today. This default initrd does
the partition parsing that up to now the kernel did.
That way nobody need to notice a difference, except for
those who use initrd already now. They can solve their
problems.

Andries


-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] struct char_device

[Alan Cox]

 Because we still need the partitioning code for backwards
 compatibility. There's no way I'm going to use initrd to do partition
 setup with lvmtools etc.

The current partitioning code consists of re-reading from disk. That is 
code that has to be present anyway even without an initrd since it is needed
for mounting other filesystems

 Also, lvm and friends are _heavyweight_. The partitioning stuff should be
 _one_ add (and perhaps a range check) at bh submit time. None of this
 remapping crap. We don't need no steenking overhead for something we need
 to do anyway.

Thats the kind of LVM layer I am thinking in terms of, not a large block
of LVM code. And for that matter the partition scanner and modifying code
can even be seperate dynamic loaded modules.

At the moment ponder what happens if you have I/Os in flight to /dev/hda3
when you delete /dev/hda2

Alan



-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] struct char_device

[Jeff Garzik]

Jeff Garzik wrote:
> /dev/sda <-> partition_blkdev <-> /dev/disk{0,1,2,3,4}
> /dev/hda <-> partition_blkdev <-> /dev/disk{5,6,7}

I also point out that handling disk partitions as a -tiny- remapping
blkdev also has the advantage of making it easy to have a single request
device per hardware device (a simple remap shouldn't require its own
request queue, right?), while remapping devices flexibility to do their
own request queue management.


> I do grant you that an offset at bh submit time is faster, but IMHO
> partitions -not- as a remapping blkdev are an ugly special case.

think of the simplifications possible, when partitions are just another
block device, just like anything else...  No special partitions arrays
in the lowlevel blkdev, etc.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] struct char_device

[Jeff Garzik]

Linus Torvalds wrote:
> 
> On Tue, 22 May 2001, Jeff Garzik wrote:
> >
> > Alan recently straightened me out with "EVMS/LVM is partitions done
> > right"
> >
> > so... why not implement partitions as simply doing block remaps to the
> > lower level device?  That's what EVMS/LVM/md are doing already.
> 
> Because we still need the partitioning code for backwards
> compatibility. There's no way I'm going to use initrd to do partition
> setup with lvmtools etc.
> 
> Also, lvm and friends are _heavyweight_. The partitioning stuff should be
> _one_ add (and perhaps a range check) at bh submit time. None of this
> remapping crap. We don't need no steenking overhead for something we need
> to do anyway.

no no no.  Not -that- heavyweight.

Partition support becomes a -peer- of LVM.

Imagine a tiny blkdev driver that understood MS-DOS (and other) hardware
partitions, and exported N block devices, representing the underlying
device (whatever it is).  In fact, that might be even a -unifying-
factor:  this tiny blkdev module -is- your /dev/disk.  For example,

/dev/sda <-> partition_blkdev <-> /dev/disk{0,1,2,3,4}
/dev/hda <-> partition_blkdev <-> /dev/disk{5,6,7}

A nice side effect:  modular partition support, since its a normal
blkdev just like anything yes.

YES there is overhead, but if partitions are just another remapping
blkdev, you get all this stuff for free.

I do grant you that an offset at bh submit time is faster, but IMHO
partitions -not- as a remapping blkdev are an ugly special case.

Remapping to an unchanging offset in the make_request_fn can be fast,
too...

-- 
Jeff Garzik  | "Are you the police?"
Building 1024| "No, ma'am.  We're musicians."
MandrakeSoft |
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] struct char_device

[Linus Torvalds]

On Wed, 23 May 2001 [EMAIL PROTECTED] wrote:
>
> > why not implement partitions as simply doing block remaps
> 
> Everybody agrees.

No they don't.

Look at the cost of lvm. Function calls, buffer head remapping, the
works. You _need_ that for a generic LVM layer, but you sure as hell don't
need it for simple partitioning.

Can LVM do it? Sure.

Do we need LVM to do it? No.

Does LVM simplify anything? No.

It doesn't get much simpler than a single line that does the
equivalent of "bh->sector += offset". Most of the bulk of the code in the
partitioning stuff is for actually parsing the partitions, and we need
that to bootstrap. Maybe we can get rid of some of the more esoteric ones
(ie pretty much everything except for the native partitioning code for
each architecture) and tell people to use LVM for the rest.

In short, there are _no_ advantages to involve LVM here.

Linus

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] struct char_device

[Linus Torvalds]

On Tue, 22 May 2001, Jeff Garzik wrote:
> 
> Alan recently straightened me out with "EVMS/LVM is partitions done
> right"
> 
> so... why not implement partitions as simply doing block remaps to the
> lower level device?  That's what EVMS/LVM/md are doing already.

Because we still need the partitioning code for backwards
compatibility. There's no way I'm going to use initrd to do partition
setup with lvmtools etc.

Also, lvm and friends are _heavyweight_. The partitioning stuff should be
_one_ add (and perhaps a range check) at bh submit time. None of this
remapping crap. We don't need no steenking overhead for something we need
to do anyway.

Linus

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] struct char_device

[Linus Torvalds]

On Tue, 22 May 2001, Jeff Garzik wrote:
>
> IMHO it would be nice to (for 2.4) create wrappers for accessing the
> block arrays, so that we can more easily dispose of the arrays when 2.5
> rolls around...

No.

We do not create wrappers "so that we can easily change the implementation
when xxx happens".

That way lies bad implementations.

We do the _good_ implementation first, and then we create the
"kcompat-2.4" file later that makes people able to use the good
implementation.

Why this way? Because wrapping for wrappings sake just makes code
unreadable (see acpi), and often makes it less obvious what you actually
_have_ to do, and what you don't. So get the design right _first_, and
once the design is right, _then_ you worry about kcompat-2.4.

Linus

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] struct char_device

[Linus Torvalds]

On Tue, 22 May 2001, Alexander Viro wrote:

> >which populate the "inode->dev" union pointer, which in turn is _only_
> >a cache of the lookup. Right now we do this purely based on "dev_t",
> >and I think that is bogus. We should never pass a "dev_t" around
> >without an inode, I think.
> 
> I doubt it. First of all, then we'd better make i_rdev dev_t. Right now
> we have it kdev_t and it makes absolutely no sense that way.

Absolutely.

In fact, the whole "kdev_t" makes no sense if we have proper char_dev and
block_dev pointers.

We have "dev_t" which is what we export to user space. And if we split up
char dev and block dev (which I'm an avid proponent for), kdev_t will be
an anachronism.

> The real thing is inode->dev. Notice that for devfs (and per-device
> filesystems) we can set it upon inode creation, since they _know_ it
> from the very beginning and there is absolutely no reason to do any
> hash lookups. Moreover, in these cases we may have no meaningful device
> number at all.

Sure. If something like devfs _knows_ the device pointers from the very
beginning, then just do: 
 - increment reference count
 - inode->dev.char = cdev;

> >Block devices will have the "request queue" pointer, and the size and
> >partitioning information. Character devices currently would not have
> 
> Do we really want a separate queue for each partition?

No. 

But the pointer is not a 1:1 thing (otherwise we'd just put the whole
request queue _into_ the block device).

The block device should have a pointer to the request queue, along with
the partitioning information. Why? Because that way it becomes very simple
indeed to do request processing: none of the current "look up the proper
queue for each request and do the partition offset magic inside each
driver". Instead, the queuing function becomes:

bh->index = bdev->index;
bh->sector += bdev->sector_offset;
submit_bh(bh, bdev->request_queue);

and you're done. Those three lines did both the queue lookup, the
"index" for the driver, and the partitioning offset. Whoa, nelly.

And THAT is why you want to have the queue pointer in the bdev structure.

Linus

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] struct char_device

[Alexander Viro]

On Wed, 23 May 2001 [EMAIL PROTECTED] wrote:

> >>dev_t rdev;
> 
> > Reasonable.
> 
> Good. We all agree.
> (But now you see what I meant in comments about mknod.)
> 
> >>kdev_t dev;
> 
> > Useless. If you hope that block_device will help to solve rmmod races
> 
> Yes. Why am I mistaken?

Because the problems begin in subsystems. Solving the situation with
block_device_operations is trivial. It's stuff on the character side that
is going to bite you big way. TTY drivers, for example. They are below
the layer where your kdev_t lives.

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] struct char_device

[Andries . Brouwer]

>>  dev_t rdev;

> Reasonable.

Good. We all agree.
(But now you see what I meant in comments about mknod.)

>>  kdev_t dev;

> Useless. If you hope that block_device will help to solve rmmod races

Yes. Why am I mistaken?

Andries
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] struct char_device

[Martin Dalecki]

[EMAIL PROTECTED] wrote:
> 
> > IMHO it would be nice to create wrappers for accessing the block arrays
> 
> Last year Linus didnt like that at all. Maybe this year.

Well... the attached patch lines up into this effort and fixes
some abuses, removes redundant code and so on. Please have a second
look.

diff -urN linux/drivers/block/ll_rw_blk.c new/drivers/block/ll_rw_blk.c
--- linux/drivers/block/ll_rw_blk.c Thu Apr 12 21:15:52 2001
+++ new/drivers/block/ll_rw_blk.c   Mon Apr 30 23:16:03 2001
@@ -85,25 +85,21 @@
 int * blk_size[MAX_BLKDEV];
 
 /*
- * blksize_size contains the size of all block-devices:
+ * blksize_size contains the block size of all block-devices:
  *
  * blksize_size[MAJOR][MINOR]
  *
- * if (!blksize_size[MAJOR]) then 1024 bytes is assumed.
+ * Access to this array should happen through the get_blksize_size() function.
+ * If (!blksize_size[MAJOR]) then 1024 bytes is assumed.
  */
 int * blksize_size[MAX_BLKDEV];
 
 /*
  * hardsect_size contains the size of the hardware sector of a device.
  *
- * hardsect_size[MAJOR][MINOR]
- *
- * if (!hardsect_size[MAJOR])
- * then 512 bytes is assumed.
- * else
- * sector_size is hardsect_size[MAJOR][MINOR]
- * This is currently set by some scsi devices and read by the msdos fs driver.
- * Other uses may appear later.
+ * Access to this array should happen through the get_hardsect_size() function.
+ * The default value is assumed to be 512 unless specified differently by the
+ * corresponding low-level driver.
  */
 int * hardsect_size[MAX_BLKDEV];
 
@@ -992,22 +988,14 @@
 
 void ll_rw_block(int rw, int nr, struct buffer_head * bhs[])
 {
-   unsigned int major;
-   int correct_size;
+   ssize_t correct_size;
int i;
 
if (!nr)
return;
 
-   major = MAJOR(bhs[0]->b_dev);
-
/* Determine correct block size for this device. */
-   correct_size = BLOCK_SIZE;
-   if (blksize_size[major]) {
-   i = blksize_size[major][MINOR(bhs[0]->b_dev)];
-   if (i)
-   correct_size = i;
-   }
+   correct_size = get_blksize_size(bhs[0]->b_dev);
 
/* Verify requested block sizes. */
for (i = 0; i < nr; i++) {
diff -urN linux/drivers/block/loop.c new/drivers/block/loop.c
--- linux/drivers/block/loop.c  Thu Apr 12 04:05:14 2001
+++ new/drivers/block/loop.cMon Apr 30 23:30:17 2001
@@ -272,22 +272,10 @@
return desc.error;
 }
 
-static inline int loop_get_bs(struct loop_device *lo)
-{
-   int bs = 0;
-
-   if (blksize_size[MAJOR(lo->lo_device)])
-   bs = blksize_size[MAJOR(lo->lo_device)][MINOR(lo->lo_device)];
-   if (!bs)
-   bs = BLOCK_SIZE;
-
-   return bs;
-}
-
 static inline unsigned long loop_get_iv(struct loop_device *lo,
unsigned long sector)
 {
-   int bs = loop_get_bs(lo);
+   int bs = get_blksize_size(lo->lo_device);
unsigned long offset, IV;
 
IV = sector / (bs >> 9) + lo->lo_offset / bs;
@@ -306,9 +294,9 @@
pos = ((loff_t) bh->b_rsector << 9) + lo->lo_offset;
 
if (rw == WRITE)
-   ret = lo_send(lo, bh, loop_get_bs(lo), pos);
+   ret = lo_send(lo, bh, get_blksize_size(lo->lo_device), pos);
else
-   ret = lo_receive(lo, bh, loop_get_bs(lo), pos);
+   ret = lo_receive(lo, bh, get_blksize_size(lo->lo_device), pos);
 
return ret;
 }
@@ -650,12 +638,7 @@
lo->old_gfp_mask = inode->i_mapping->gfp_mask;
inode->i_mapping->gfp_mask = GFP_BUFFER;
 
-   bs = 0;
-   if (blksize_size[MAJOR(inode->i_rdev)])
-   bs = blksize_size[MAJOR(inode->i_rdev)][MINOR(inode->i_rdev)];
-   if (!bs)
-   bs = BLOCK_SIZE;
-
+   bs = get_blksize_size(inode->i_rdev);
set_blocksize(dev, bs);
 
lo->lo_bh = lo->lo_bhtail = NULL;
diff -urN linux/drivers/char/raw.c new/drivers/char/raw.c
--- linux/drivers/char/raw.cFri Apr 27 23:23:25 2001
+++ new/drivers/char/raw.c  Mon Apr 30 22:57:20 2001
@@ -124,22 +124,25 @@
return err;
}
 
-   
-   /* 
-* Don't interfere with mounted devices: we cannot safely set
-* the blocksize on a device which is already mounted.  
+   /*
+* 29.04.2001 Martin Dalecki:
+*
+* The original comment here was saying:
+*
+* "Don't interfere with mounted devices: we cannot safely set the
+* blocksize on a device which is already mounted."
+*
+* However the code below was setting happily the blocksize
+* disregarding the previous check. I have fixed this, however I'm
+* quite sure, that the statement above isn't right and we should be
+* able to remove the first arm of the branch below entierly.
 */
-   
-   sector_size = 512;
if (get_super(rdev) != NULL) {
-   if 

Re: [PATCH] struct char_device

[Andries . Brouwer]

> IMHO it would be nice to create wrappers for accessing the block arrays

Last year Linus didnt like that at all. Maybe this year.

Andries
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] struct char_device

[Andries . Brouwer]

> why not implement partitions as simply doing block remaps

Everybody agrees.

Andries
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] struct char_device

[Jens Axboe]

On Tue, May 22 2001, Jeff Garzik wrote:
> IMHO it would be nice to (for 2.4) create wrappers for accessing the
> block arrays, so that we can more easily dispose of the arrays when 2.5
> rolls around...

Agreed, in fact I have a lot of stuff that could be included in the
kcompat files for 2.4 (of course still changing :)

BTW, max_sectors/max_segments/hardsect_size already in place. Still some
to go.

-- 
Jens Axboe

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] struct char_device

[Jeff Garzik]

Alexander Viro wrote:
> Do we really want a separate queue for each partition? I'd rather have
> disk_struct created when driver sees the disk and list of partitions
> (possibly represented by struct block_device) anchored in disk_struct
> and populated by grok_partitions().

Alan recently straightened me out with "EVMS/LVM is partitions done
right"

so... why not implement partitions as simply doing block remaps to the
lower level device?  That's what EVMS/LVM/md are doing already.

-- 
Jeff Garzik  | "Are you the police?"
Building 1024| "No, ma'am.  We're musicians."
MandrakeSoft |
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] struct char_device

[Alexander Viro]

On Wed, 23 May 2001 [EMAIL PROTECTED] wrote:

> I am not sure whether we agree or differ in opinion. I wouldn't mind
> 
> /* pairing for dev_t to bd_op/cd_op */
> struct bc_device {
> struct list_headbd_hash;
> atomic_tbd_count;
> dev_t   bd_dev;
> atomic_tbd_openers;
> union {
>   struct block_device_operations_and_data *bd_op;
>   struct char_device_operations_and_data *cd_op;
>   }
> struct semaphorebd_sem;
> };
> 
> typedef struct bc_device *kdev_t;

What for? What part of the kernel needs a device and doesn't know apriory
whether it's block or character one?

> and in an inode
> 
>   kdev_t dev;
Useless. If you hope that block_device will help to solve rmmod races -
sorry, it won't. Wrong layer.

>   dev_t rdev;
Reasonable.

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] struct char_device

[Andries . Brouwer]

From [EMAIL PROTECTED] Wed May 23 00:39:23 2001

On Tue, 22 May 2001 [EMAIL PROTECTED] wrote:
> 
> The operations are different, but all bdev/cdev code is identical.
> 
> So the choice is between two uglies:
> (i) have some not entirely trivial amount of code twice in the kernel
> (ii) have a union at the point where the struct operations
> is assigned.
> 
> I preferred the union.

I would much prefer a union of pointers over a pointer to a union.

Why? Because if you have a "struct inode", you also have enough
information to decide _which_ of the two types of pointers you have, so
you can do the proper dis-ambiguation of the union and properly select
either 'inode->dev.char' or 'inode->dev.block' depending on other
information in the inode.

I am not sure whether we agree or differ in opinion. I wouldn't mind

/* pairing for dev_t to bd_op/cd_op */
struct bc_device {
struct list_headbd_hash;
atomic_tbd_count;
dev_t   bd_dev;
atomic_tbd_openers;
union {
struct block_device_operations_and_data *bd_op;
struct char_device_operations_and_data *cd_op;
}
struct semaphorebd_sem;
};

typedef struct bc_device *kdev_t;

and in an inode

kdev_t dev;
dev_t rdev;

In reality we want the pair (dev_t, pointer to stuff), but then
there is all this administrative nonsense needed to make sure
that nobody uses the pointer after the module has been unloaded
that makes the pointer a bit thicker.

   And we should not depend on the "inode->dev." pointer
   being valid all the time, as there is absolutely zero point
   in initializing the pointer every time somebody does a "ls -l /dev".

Yes, that is why I want to go back and have dev_t rdev, not kdev_t.
The lookup is done when the device is opened.

Andries
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] struct char_device

[Martin Dalecki]

And if we are at the topic... Those are the places where blk_size[]
get's
abused, since it's in fact a property of a FS in fact and not the
property of
a particular device... blksect_size is the array describing the physical
access limits of a device and blk_size get's usually checked against it.
However due to the bad naming and the fact that this information is
associated with major/minor number usage same device driver writers got
*very* confused as you can see below:

./fs/block_dev.c: Here this information should be passed entierly insice
the request.

./fs/partitions/check.c: Here it basically get's reset or ignored


Here it's serving the purpose of a sector size, which is bogous!

./mm/swapfile.c:#include  /* for blk_size */
./mm/swapfile.c:if (!dev || (blk_size[MAJOR(dev)] &&
./mm/swapfile.c: !blk_size[MAJOR(dev)][MINOR(dev)]))
./mm/swapfile.c:if (blk_size[MAJOR(dev)])
./mm/swapfile.c:swapfilesize = blk_size[MAJOR(dev)][MINOR(dev)]


Here it shouldn't be needed
./drivers/block/ll_rw_blk.c: 


./drivers/block/floppy.c:   blk_size[MAJOR_NR] = floppy_sizes;
./drivers/block/nbd.c:  blk_size[MAJOR_NR] = nbd_sizes;
./drivers/block/rd.c: * and set blk_size for -ENOSPC, Werner Fink
<[EMAIL PROTECTED]>, Apr '99
./drivers/block/amiflop.c:  blk_size[MAJOR_NR] = floppy_sizes;
./drivers/block/loop.c: if (blk_size[MAJOR(lodev)])
./drivers/block/ataflop.c: *   - Set blk_size for proper size checking
./drivers/block/ataflop.c:  blk_size[MAJOR_NR] = floppy_sizes;
./drivers/block/cpqarray.c: drv->blk_size;
./drivers/block/z2ram.c:blk_size[ MAJOR_NR ] = z2_sizes;
./drivers/block/swim3.c:blk_size[MAJOR_NR] = floppy_sizes;
./drivers/block/swim_iop.c: blk_size[MAJOR_NR] = floppy_sizes;
./drivers/char/raw.c:   if (blk_size[MAJOR(dev)])
./drivers/scsi/advansys.c:ASC_DCNTblk_size;
./drivers/scsi/sd.c:blk_size[SD_MAJOR(i)] = NULL;
./drivers/scsi/sr.c:blk_size[MAJOR_NR] = sr_sizes;
./drivers/scsi/sr.c:blk_size[MAJOR_NR] = NULL;
./drivers/sbus/char/jsflash.c:  blk_size[JSFD_MAJOR] = jsfd_sizes;
./drivers/ide/ide-cd.c: blk_size[HWIF(drive)->major] =
HWIF(drive)->gd->sizes;
./drivers/ide/ide-floppy.c: *   Revalidate the new media. Should set
blk_size[]
./drivers/acorn/block/fd1772.c: blk_size[MAJOR_NR] = floppy_sizes;
./drivers/i2o/i2o_block.c:  blk_size[MAJOR_NR] = i2ob_sizes;

In the following they are REALLY confusing it and then compensating for
this misunderstanding in lvm.h by redefining the corresponding default
values.

./drivers/s390/*

And then some minor confusions follow...

./drivers/mtd/mtdblock.c:   blk_size[MAJOR_NR] = NULL;
./drivers/md/md.c:  if (blk_size[MAJOR(dev)])
./arch/m68k/atari/stram.c:blk_size[STRAM_MAJOR] = stram_sizes;

Basically one should just stop setting blk_size[][] inside *ANY* driver
and anything should still work fine unless the driver is broken...

Well that's the point for another fine kernel experiment I will do
and report whatever it works really out like this in reality 8-)
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] struct char_device

[Linus Torvalds]

On Tue, 22 May 2001 [EMAIL PROTECTED] wrote:
> 
> The operations are different, but all bdev/cdev code is identical.
> 
> So the choice is between two uglies:
> (i) have some not entirely trivial amount of code twice in the kernel
> (ii) have a union at the point where the struct operations
> is assigned.
> 
> I preferred the union.

I would much prefer a union of pointers over a pointer to a union.

So I'd much rather have the inode have a

union {
struct block_device *block;
struct char_device *char;
} dev;

and then have people do

cdev = inode->dev.char;

to get the right information, than to have 

union block_char_union {
struct block_device block;
struct char_device char;
};

.. with struct inode containing ..
union block_char_union *dev;

Why? Because if you have a "struct inode", you also have enough
information to decide _which_ of the two types of pointers you have, so
you can do the proper dis-ambiguation of the union and properly select
either 'inode->dev.char' or 'inode->dev.block' depending on other
information in the inode.

In contrast, if you have a pointer to a union, you don't have information
of which sub-type it is, and you'd have to carry that along some other way
(for example, by having common fields at the beginning). Which I think is
broken.

So my suggestion for eventual interfaces:

 - have functions like

struct block_dev *bdget(struct inode *);
struct char_dev *cdget(struct inode *);

   which populate the "inode->dev" union pointer, which in turn is _only_
   a cache of the lookup. Right now we do this purely based on "dev_t",
   and I think that is bogus. We should never pass a "dev_t" around
   without an inode, I think.

   And we should not depend on the "inode->dev." pointer being valid all
   the time, as there is absolutely zero point in initializing the pointer
   every time the inode is read just because somebody does a "ls -l /dev".
   Thus the "cache" part above.

 - NO reason to try to make "struct block_dev" and "struct char_dev" look
   similar. They will have some commonality for lookup purposes (that
   issue is similar, as Andries points out), and maybe that commonality
   can be separated out into a sub-structure or something. But apart from
   that, they have absolutely nothing to do with each other, and I'd
   rather not have them have even a _superficial_ connection.

   Block devices will have the "request queue" pointer, and the size and
   partitioning information. Character devices currently would not have
   much more than the operations pointer and name, but who knows..

But the most important thing is to be able to do this in steps. One of the
reasons Andries has had patches for a long time is that it was never very
gradual. Al's patch is gradual, and I like that.

Linus

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] struct char_device

[Martin Dalecki]

[EMAIL PROTECTED] wrote:
> 
> Martin Dalecki writes:
> 
> > Erm... I wasn't talking about the DESIRED state of affairs!
> > I was talking about the CURRENT state of affairs. OK?
> 
> Oh, but in 1995 it was quite possible to compile the kernel
> with kdev_t a pointer type, and I have done it several times since.

Yes I remember but unfortunately some big L* did ignore
your *fine* efforts entierly in favour of developing 
/proc and /dev/random and other crap maybe?

> The kernel keeps growing, so each time it is more work than
> the previous time.
> 
> > At least you have admitted that you where the one responsible
> > for the design of this MESS.
> 
> Thank you! However, you give me too much honour.

Well ... you ask for it in the corresponding header ;-).
But it isn't yours fault indeed I admit...
I know the discussions from memmory since I'm returning REGULARLY to
this
topic in intervals of about between 6 and 24 months since about
maybe already 6 years!!! Currently they have just started to hurt
seriously. And please remember the change I have mentioned above
wasn't intended as developement but just only as an experiment...

Well let's us stop throw flames at each other.
Please have a tight look at the following *EXPERIMENT* I have
already done. It's really really only intended to mark the
places where the full mess shows it's ugly head:

http://www.dalecki.de/big-002.diff
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] struct char_device

[Andries . Brouwer]

Martin Dalecki writes:

> Erm... I wasn't talking about the DESIRED state of affairs!
> I was talking about the CURRENT state of affairs. OK?

Oh, but in 1995 it was quite possible to compile the kernel
with kdev_t a pointer type, and I have done it several times since.

The kernel keeps growing, so each time it is more work than
the previous time.

> At least you have admitted that you where the one responsible
> for the design of this MESS.

Thank you! However, you give me too much honour.

Andries
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] struct char_device

[Martin Dalecki]

[EMAIL PROTECTED] wrote:
> 
> Martin Dalecki writes:
> 
> > I fully agree with you.
> 
> Good.
> 
> Unfortunately I do not fully agree with you.
> 
> > Most of the places where there kernel is passing kdev_t
> > would be entierly satisfied with only the knowlendge of
> > the minor number.
> 
> My kdev_t is a pointer to a structure with device data
> and device operations. Among the operations a routine
> that produces a name. Among the data, in the case of a
> block device, the size, and the sectorsize, ..
> 
> A minor gives no name, and no data.
> 
> Linus' minor is 20-bit if I recall correctly.
> My minor is 32-bit. Neither of the two can be
> used to index arrays.

Erm... I wasn't talking about the DESIRED state of affairs!
I was talking about the CURRENT state of affairs. OK?
The fact still remains that most of the places which a have pointed
out just need the minor nibble of whatever bits you pass to them.

Apparently nobody on this list here blabbering about a new improved
minor/major space didn't actually take the time and looked into
all those places where the kernel is CURRENTLY replying in minor/major
array enumeration. They are TON's of them. The most ugly are RAID
drivers
an all those MD LVW and whatever stuff as well as abused minor number
spaces as replacements of differnt majors.

At least you have admitted that you where the one responsible for
the design of this MESS.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] struct char_device

[Andries . Brouwer]

Martin Dalecki writes:

> I fully agree with you.

Good.

Unfortunately I do not fully agree with you.

> Most of the places where there kernel is passing kdev_t
> would be entierly satisfied with only the knowlendge of
> the minor number.

My kdev_t is a pointer to a structure with device data
and device operations. Among the operations a routine
that produces a name. Among the data, in the case of a
block device, the size, and the sectorsize, ..

A minor gives no name, and no data.

Linus' minor is 20-bit if I recall correctly.
My minor is 32-bit. Neither of the two can be
used to index arrays.

Andries
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] struct char_device

[Martin Dalecki]

[EMAIL PROTECTED] wrote:
> 
> > They are entirely different. Too different sets of operations.
> 
> Maybe you didnt understand what I meant.
> both bdev and cdev take care of the correspondence
> device number <---> struct with operations.
> 
> The operations are different, but all bdev/cdev code is identical.
> 
> So the choice is between two uglies:
> (i) have some not entirely trivial amount of code twice in the kernel
> (ii) have a union at the point where the struct operations
> is assigned.
> 
> I preferred the union.
> 
> >> And a second remark: don't forget that presently the point where
> >> bdev is introduced is not quite right. We must only introduce it
> >> when we really have a device, not when there only is a device
> >> number (like on a mknod call).
> 
> > That's simply wrong. kdev_t is used for unopened objects quite often.
> 
> Yes, but that was my design mistake in 1995.

I fully agree with you. Most of the places where there kernel is passing
kdev_t
would be entierly satisfied with only the knowlendge of the minor number
used to
distinguish between different device ranges, which is BTW an abuse by
itself as well
since minors where for encounters of instances of similiar devices in
linux...
The places where this is the case are namely:

1. literally: all character devices.

2. The whole scsi stuff.

3. most of the ide stuff.

4. md/lvm and similiar culprits.

I did "discover" this by splitting the i_dev field from stuct inode
into explicit i_minor and i_major fields and then actually "fixing" my
particular kernel configuration until it worked again. This was
*very* insigtfull, since it discovered all the places where kdev_t get's
used, where it shouldn't be of any need anylonger anyway.

The remaining places where kdev_t comes into sight are mostly
the places where the kernel is mounting the initial root
device.

In case you would like to have a look at the resulting bit huge
patch I can send it to you...

> I think you'll find if you continue on this way,
> as I found and already wrote in kdev_t.h
> that it is bad to carry pointers around for unopened and unknown devices.
> 
> So, I think that the setup must be changed a tiny little bit
> and distinguish meaningless numbers from devices.
> 
> Andries
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [EMAIL PROTECTED]
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

-- 
- phone: +49 214 8656 283
- job:   eVision-Ventures AG, LEV .de (MY OPINIONS ARE MY OWN!)
- langs: de_DE.ISO8859-1, en_US, pl_PL.ISO8859-2, last ressort:
ru_RU.KOI8-R
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] struct char_device

[Andries . Brouwer]

> They are entirely different. Too different sets of operations.

Maybe you didnt understand what I meant.
both bdev and cdev take care of the correspondence
device number <---> struct with operations.

The operations are different, but all bdev/cdev code is identical.

So the choice is between two uglies:
(i) have some not entirely trivial amount of code twice in the kernel
(ii) have a union at the point where the struct operations
is assigned.

I preferred the union.

>> And a second remark: don't forget that presently the point where
>> bdev is introduced is not quite right. We must only introduce it
>> when we really have a device, not when there only is a device
>> number (like on a mknod call).

> That's simply wrong. kdev_t is used for unopened objects quite often.

Yes, but that was my design mistake in 1995.
I think you'll find if you continue on this way,
as I found and already wrote in kdev_t.h
that it is bad to carry pointers around for unopened and unknown devices.

So, I think that the setup must be changed a tiny little bit
and distinguish meaningless numbers from devices.

Andries
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] struct char_device

[Alexander Viro]

On Tue, 22 May 2001 [EMAIL PROTECTED] wrote:

> One remark, repeating what I wrote on some web page:
> -
> A struct block_device provides the connection between a device number
> and a struct block_device_operations. 
> ...
> Clearly, we also want to associate a struct char_device_operations
> to a character device number. When we do this, all bdev code will

They are entirely different. Too different sets of operations.

> have to be duplicated for cdev, so there seems no point in having
> bdev code - kdev, for both bdev and cdev, seems more elegant. 
> -

Not really. For struct block_device you have partitioning stuff sitting
nearby. It should be handled by generic code. Nothing of that kind for
character devices.

But the main point is that for block devices ->read() and ->write() do
not belong to the natural interface. Request queue does. They are about
as far from each other as from FIFOs and sockets.

> And a second remark: don't forget that presently the point where
> bdev is introduced is not quite right. We must only introduce it
> when we really have a device, not when there only is a device
> number (like on a mknod call).

That's simply wrong. kdev_t is used for unopened objects quite often.

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] struct char_device

[Andries . Brouwer]

Alexander Viro writes:

> patch below adds the missing half of kdev_t -
> for block devices we already have a unique pointer
> and that adds a similar animal for character devices.

Very good.
(Of course I did precisely the same, but am a bit slower in
submitting things during a stable series or code freeze.)

One remark, repeating what I wrote on some web page:
-
A struct block_device provides the connection between a device number
and a struct block_device_operations. 
...
Clearly, we also want to associate a struct char_device_operations
to a character device number. When we do this, all bdev code will
have to be duplicated for cdev, so there seems no point in having
bdev code - kdev, for both bdev and cdev, seems more elegant. 
-

And a second remark: don't forget that presently the point where
bdev is introduced is not quite right. We must only introduce it
when we really have a device, not when there only is a device
number (like on a mknod call).

Andries
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] struct char_device

[Oliver Xymoron]

On Tue, 22 May 2001, Guest section DW wrote:

> On Tue, May 22, 2001 at 11:08:16AM -0500, Oliver Xymoron wrote:
>
> > > >+   struct list_headhash;
>
> > > Why not name consistently with the struct block_device?
> > >  struct list_headcd_hash;
>
> > Because foo_ is a throwback to the days when C compilers had a single
> > namespace for all structure elements, not a readability aid. If you need
> > foo_ to know what type of structure you're futzing with, you need to name
> > your variables better.
>
> One often has to go through all occurrences of a variable or
> field of a struct. That is much easier with cd_hash and cd_dev
> than with hash and dev.
>
> No, it is a good habit, these prefixes, even though it is no longer
> necessary because of the C compiler.

A better habit is encapsulating your data structures well enough that the
entire kernel doesn't feel the need to go digging through them. The fact
that you have to change many widely-scattered instances of something
points to bad modularity. Supporting that practice with verbose naming is
not doing yourself a favor in the long run.

If you must, use accessor functions instead. At best you'll be able to
make sweeping semantic changes in one spot. At worst, you'll be able to
grep for it.

--
 "Love the dolphins," she advised him. "Write by W.A.S.T.E.."


-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] struct char_device

[Alexander Viro]

On Tue, 22 May 2001, Guest section DW wrote:

> One often has to go through all occurrences of a variable or
> field of a struct. That is much easier with cd_hash and cd_dev
> than with hash and dev.
> 
> No, it is a good habit, these prefixes, even though it is no longer
> necessary because of the C compiler. 

True, except for the stuff that should remain local.

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] struct char_device

[Guest section DW]

On Tue, May 22, 2001 at 11:08:16AM -0500, Oliver Xymoron wrote:

> > >+   struct list_headhash;

> > Why not name consistently with the struct block_device?
> >  struct list_headcd_hash;

> Because foo_ is a throwback to the days when C compilers had a single
> namespace for all structure elements, not a readability aid. If you need
> foo_ to know what type of structure you're futzing with, you need to name
> your variables better.

One often has to go through all occurrences of a variable or
field of a struct. That is much easier with cd_hash and cd_dev
than with hash and dev.

No, it is a good habit, these prefixes, even though it is no longer
necessary because of the C compiler. 
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] struct char_device

[Alexander Viro]

On Tue, 22 May 2001, Oliver Xymoron wrote:

> > Not always. If the thing is used all over the tree, it'd better be
> > greppable. I hate the foo->de stuff - it's not localized and it's
> > impossible to grep for.
> 
> I'd like to say the compiler will happily find it for you, but the
> kernel's mostly conditionally compiled..
> 
> Have you tried something like:
> 
>  find -type f | xargs grep -A100 'struct_name' | grep -e '\belement\b'
> 
> Obviously not a fix, but possibly helpful.

Too many false negatives. And you apparently missed a part of fun -
in that case there's a certain TLD...

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] struct char_device

[Oliver Xymoron]

On Tue, 22 May 2001, Alexander Viro wrote:

> On Tue, 22 May 2001, Oliver Xymoron wrote:
>
> > Because foo_ is a throwback to the days when C compilers had a single
> > namespace for all structure elements, not a readability aid. If you need
> > foo_ to know what type of structure you're futzing with, you need to name
> > your variables better.
>
> Not always. If the thing is used all over the tree, it'd better be
> greppable. I hate the foo->de stuff - it's not localized and it's
> impossible to grep for.

I'd like to say the compiler will happily find it for you, but the
kernel's mostly conditionally compiled..

Have you tried something like:

 find -type f | xargs grep -A100 'struct_name' | grep -e '\belement\b'

Obviously not a fix, but possibly helpful.

--
 "Love the dolphins," she advised him. "Write by W.A.S.T.E.."



-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] struct char_device

[Alexander Viro]

On Tue, 22 May 2001, Oliver Xymoron wrote:

> Because foo_ is a throwback to the days when C compilers had a single
> namespace for all structure elements, not a readability aid. If you need
> foo_ to know what type of structure you're futzing with, you need to name
> your variables better.

Not always. If the thing is used all over the tree, it'd better be
greppable. I hate the foo->de stuff - it's not localized and it's
impossible to grep for.

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] struct char_device

[Oliver Xymoron]

On Tue, 22 May 2001, Anton Altaparmakov wrote:

> Hello,
>
> At 15:18 22/05/01, Alexander Viro wrote:
> [snip cool stuff]
> >+struct char_device {
> >+   struct list_headhash;
> >+   atomic_tcount;
> >+   dev_t   dev;
> >+   atomic_topeners;
> >+   struct semaphoresem;
>
> Why not name consistently with the struct block_device?
> I.e.:
> struct char_device {
>  struct list_headcd_hash;
>  atomic_tcd_count;
>  dev_t   cd_dev;
>  atomic_tcd_openers;
>  struct semaphorecd_sem;
> };

Because foo_ is a throwback to the days when C compilers had a single
namespace for all structure elements, not a readability aid. If you need
foo_ to know what type of structure you're futzing with, you need to name
your variables better.

--
 "Love the dolphins," she advised him. "Write by W.A.S.T.E.."

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] struct char_device

[Anton Altaparmakov]

Hello,

At 15:18 22/05/01, Alexander Viro wrote:
[snip cool stuff]
>diff -urN S5-pre4/include/linux/fs.h S5-pre4-cdev/include/linux/fs.h
>--- S5-pre4/include/linux/fs.h  Sat May 19 22:46:36 2001
>+++ S5-pre4-cdev/include/linux/fs.h Tue May 22 09:14:25 2001
>@@ -384,6 +384,14 @@
> int gfp_mask;   /* how to allocate the 
> pages */
>  };
>
>+struct char_device {
>+   struct list_headhash;
>+   atomic_tcount;
>+   dev_t   dev;
>+   atomic_topeners;
>+   struct semaphoresem;

Why not name consistently with the struct block_device?
I.e.:
struct char_device {
 struct list_headcd_hash;
 atomic_tcd_count;
 dev_t   cd_dev;
 atomic_tcd_openers;
 struct semaphorecd_sem;
};

>@@ -426,8 +434,10 @@
> struct address_space*i_mapping;
> struct address_spacei_data;
> struct dquot*i_dquot[MAXQUOTAS];
>+   /* These three should probably be a union */
> struct pipe_inode_info  *i_pipe;
> struct block_device *i_bdev;
>+   struct char_device  *i_cdev;

You could then use an (perhaps even unnamed if we require a high enough gcc 
version) union in the inode structure so you don't waste space having a 
pointer to both c_dev and b_dev, such as:

 union {
 struct block_device *i_bdev;
 struct char_device  *i_cdev;
 };

It isn't possible that both i_bdev and i_cdev are used at the same time, is 
it? If it was my suggestion is obviously bogus...

Any reasons why my proposal would be a bad idea?

Just IMVHO.

Best regards,

Anton


-- 
   "Nothing succeeds like success." - Alexandre Dumas
-- 
Anton Altaparmakov  (replace at with @)
Linux NTFS Maintainer / WWW: http://sf.net/projects/linux-ntfs/
ICQ: 8561279 / WWW: http://www-stu.christs.cam.ac.uk/~aia21/

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] struct char_device

[Tommy Hallgren]

Yes, I thought about it again, and you are right. Sorry for the noise.

Regards, Tommy

--- Mark Hahn <[EMAIL PROTECTED]> wrote:
> > + if ((flags & (SLAB_CTOR_VERIFY|SLAB_CTOR_CONSTRUCTOR)) == 
> > + SLAB_CTOR_CONSTRUCTOR) 
> ...
> > + if ((flags & SLAB_CTOR_CONSTRUCTOR) == SLAB_CTOR_CONSTRUCTOR) 
> 
> consider whether flags contains SLAB_CTOR_VERIFY.  in the first case,
> it must be zero.  in the second case, it's ignored.
> 


__
Do You Yahoo!?
Yahoo! Auctions - buy the things you want at great prices
http://auctions.yahoo.com/
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] struct char_device

[Tommy Hallgren]

Hi Alexander!

If I'm not entirely mistaken, this:

+ if ((flags & (SLAB_CTOR_VERIFY|SLAB_CTOR_CONSTRUCTOR)) == 
+ SLAB_CTOR_CONSTRUCTOR) 
+ { 
+ memset(cdev, 0, sizeof(*cdev)); 
+ sema_init(>sem, 1); 
+ } 
+} 

could be replaced with this:

+ if ((flags & SLAB_CTOR_CONSTRUCTOR) == SLAB_CTOR_CONSTRUCTOR) 
+ { 
+ memset(cdev, 0, sizeof(*cdev)); 
+ sema_init(>sem, 1); 
+ } 
+} 

Regards, Tommy


__
Do You Yahoo!?
Yahoo! Auctions - buy the things you want at great prices
http://auctions.yahoo.com/
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] struct char_device

[Tommy Hallgren]

Hi Alexander!

If I'm not entirely mistaken, this:

+ if ((flags  (SLAB_CTOR_VERIFY|SLAB_CTOR_CONSTRUCTOR)) == 
+ SLAB_CTOR_CONSTRUCTOR) 
+ { 
+ memset(cdev, 0, sizeof(*cdev)); 
+ sema_init(cdev-sem, 1); 
+ } 
+} 

could be replaced with this:

+ if ((flags  SLAB_CTOR_CONSTRUCTOR) == SLAB_CTOR_CONSTRUCTOR) 
+ { 
+ memset(cdev, 0, sizeof(*cdev)); 
+ sema_init(cdev-sem, 1); 
+ } 
+} 

Regards, Tommy


__
Do You Yahoo!?
Yahoo! Auctions - buy the things you want at great prices
http://auctions.yahoo.com/
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] struct char_device

[Tommy Hallgren]

Yes, I thought about it again, and you are right. Sorry for the noise.

Regards, Tommy

--- Mark Hahn [EMAIL PROTECTED] wrote:
  + if ((flags  (SLAB_CTOR_VERIFY|SLAB_CTOR_CONSTRUCTOR)) == 
  + SLAB_CTOR_CONSTRUCTOR) 
 ...
  + if ((flags  SLAB_CTOR_CONSTRUCTOR) == SLAB_CTOR_CONSTRUCTOR) 
 
 consider whether flags contains SLAB_CTOR_VERIFY.  in the first case,
 it must be zero.  in the second case, it's ignored.
 


__
Do You Yahoo!?
Yahoo! Auctions - buy the things you want at great prices
http://auctions.yahoo.com/
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] struct char_device

[Anton Altaparmakov]

Hello,

At 15:18 22/05/01, Alexander Viro wrote:
[snip cool stuff]
diff -urN S5-pre4/include/linux/fs.h S5-pre4-cdev/include/linux/fs.h
--- S5-pre4/include/linux/fs.h  Sat May 19 22:46:36 2001
+++ S5-pre4-cdev/include/linux/fs.h Tue May 22 09:14:25 2001
@@ -384,6 +384,14 @@
 int gfp_mask;   /* how to allocate the 
 pages */
  };

+struct char_device {
+   struct list_headhash;
+   atomic_tcount;
+   dev_t   dev;
+   atomic_topeners;
+   struct semaphoresem;

Why not name consistently with the struct block_device?
I.e.:
struct char_device {
 struct list_headcd_hash;
 atomic_tcd_count;
 dev_t   cd_dev;
 atomic_tcd_openers;
 struct semaphorecd_sem;
};

@@ -426,8 +434,10 @@
 struct address_space*i_mapping;
 struct address_spacei_data;
 struct dquot*i_dquot[MAXQUOTAS];
+   /* These three should probably be a union */
 struct pipe_inode_info  *i_pipe;
 struct block_device *i_bdev;
+   struct char_device  *i_cdev;

You could then use an (perhaps even unnamed if we require a high enough gcc 
version) union in the inode structure so you don't waste space having a 
pointer to both c_dev and b_dev, such as:

 union {
 struct block_device *i_bdev;
 struct char_device  *i_cdev;
 };

It isn't possible that both i_bdev and i_cdev are used at the same time, is 
it? If it was my suggestion is obviously bogus...

Any reasons why my proposal would be a bad idea?

Just IMVHO.

Best regards,

Anton


-- 
   Nothing succeeds like success. - Alexandre Dumas
-- 
Anton Altaparmakov aia21 at cam.ac.uk (replace at with @)
Linux NTFS Maintainer / WWW: http://sf.net/projects/linux-ntfs/
ICQ: 8561279 / WWW: http://www-stu.christs.cam.ac.uk/~aia21/

-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] struct char_device

[Oliver Xymoron]

On Tue, 22 May 2001, Anton Altaparmakov wrote:

 Hello,

 At 15:18 22/05/01, Alexander Viro wrote:
 [snip cool stuff]
 +struct char_device {
 +   struct list_headhash;
 +   atomic_tcount;
 +   dev_t   dev;
 +   atomic_topeners;
 +   struct semaphoresem;

 Why not name consistently with the struct block_device?
 I.e.:
 struct char_device {
  struct list_headcd_hash;
  atomic_tcd_count;
  dev_t   cd_dev;
  atomic_tcd_openers;
  struct semaphorecd_sem;
 };

Because foo_ is a throwback to the days when C compilers had a single
namespace for all structure elements, not a readability aid. If you need
foo_ to know what type of structure you're futzing with, you need to name
your variables better.

--
 Love the dolphins, she advised him. Write by W.A.S.T.E..

-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] struct char_device

[Alexander Viro]

On Tue, 22 May 2001, Oliver Xymoron wrote:

 Because foo_ is a throwback to the days when C compilers had a single
 namespace for all structure elements, not a readability aid. If you need
 foo_ to know what type of structure you're futzing with, you need to name
 your variables better.

Not always. If the thing is used all over the tree, it'd better be
greppable. I hate the foo-de stuff - it's not localized and it's
impossible to grep for.

-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] struct char_device

[Oliver Xymoron]

On Tue, 22 May 2001, Alexander Viro wrote:

 On Tue, 22 May 2001, Oliver Xymoron wrote:

  Because foo_ is a throwback to the days when C compilers had a single
  namespace for all structure elements, not a readability aid. If you need
  foo_ to know what type of structure you're futzing with, you need to name
  your variables better.

 Not always. If the thing is used all over the tree, it'd better be
 greppable. I hate the foo-de stuff - it's not localized and it's
 impossible to grep for.

I'd like to say the compiler will happily find it for you, but the
kernel's mostly conditionally compiled..

Have you tried something like:

 find -type f | xargs grep -A100 'struct_name' | grep -e '\belement\b'

Obviously not a fix, but possibly helpful.

--
 Love the dolphins, she advised him. Write by W.A.S.T.E..



-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] struct char_device

[Alexander Viro]

On Tue, 22 May 2001, Oliver Xymoron wrote:

  Not always. If the thing is used all over the tree, it'd better be
  greppable. I hate the foo-de stuff - it's not localized and it's
  impossible to grep for.
 
 I'd like to say the compiler will happily find it for you, but the
 kernel's mostly conditionally compiled..
 
 Have you tried something like:
 
  find -type f | xargs grep -A100 'struct_name' | grep -e '\belement\b'
 
 Obviously not a fix, but possibly helpful.

Too many false negatives. And you apparently missed a part of fun -
in that case there's a certain TLD...

-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] struct char_device

[Guest section DW]

On Tue, May 22, 2001 at 11:08:16AM -0500, Oliver Xymoron wrote:

  +   struct list_headhash;

  Why not name consistently with the struct block_device?
   struct list_headcd_hash;

 Because foo_ is a throwback to the days when C compilers had a single
 namespace for all structure elements, not a readability aid. If you need
 foo_ to know what type of structure you're futzing with, you need to name
 your variables better.

One often has to go through all occurrences of a variable or
field of a struct. That is much easier with cd_hash and cd_dev
than with hash and dev.

No, it is a good habit, these prefixes, even though it is no longer
necessary because of the C compiler. 
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] struct char_device

[Alexander Viro]

On Tue, 22 May 2001, Guest section DW wrote:

 One often has to go through all occurrences of a variable or
 field of a struct. That is much easier with cd_hash and cd_dev
 than with hash and dev.
 
 No, it is a good habit, these prefixes, even though it is no longer
 necessary because of the C compiler. 

True, except for the stuff that should remain local.

-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] struct char_device

[Oliver Xymoron]

On Tue, 22 May 2001, Guest section DW wrote:

 On Tue, May 22, 2001 at 11:08:16AM -0500, Oliver Xymoron wrote:

   +   struct list_headhash;

   Why not name consistently with the struct block_device?
struct list_headcd_hash;

  Because foo_ is a throwback to the days when C compilers had a single
  namespace for all structure elements, not a readability aid. If you need
  foo_ to know what type of structure you're futzing with, you need to name
  your variables better.

 One often has to go through all occurrences of a variable or
 field of a struct. That is much easier with cd_hash and cd_dev
 than with hash and dev.

 No, it is a good habit, these prefixes, even though it is no longer
 necessary because of the C compiler.

A better habit is encapsulating your data structures well enough that the
entire kernel doesn't feel the need to go digging through them. The fact
that you have to change many widely-scattered instances of something
points to bad modularity. Supporting that practice with verbose naming is
not doing yourself a favor in the long run.

If you must, use accessor functions instead. At best you'll be able to
make sweeping semantic changes in one spot. At worst, you'll be able to
grep for it.

--
 Love the dolphins, she advised him. Write by W.A.S.T.E..


-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] struct char_device

[Andries . Brouwer]

Alexander Viro writes:

 patch below adds the missing half of kdev_t -
 for block devices we already have a unique pointer
 and that adds a similar animal for character devices.

Very good.
(Of course I did precisely the same, but am a bit slower in
submitting things during a stable series or code freeze.)

One remark, repeating what I wrote on some web page:
-
A struct block_device provides the connection between a device number
and a struct block_device_operations. 
...
Clearly, we also want to associate a struct char_device_operations
to a character device number. When we do this, all bdev code will
have to be duplicated for cdev, so there seems no point in having
bdev code - kdev, for both bdev and cdev, seems more elegant. 
-

And a second remark: don't forget that presently the point where
bdev is introduced is not quite right. We must only introduce it
when we really have a device, not when there only is a device
number (like on a mknod call).

Andries
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] struct char_device

[Alexander Viro]

On Tue, 22 May 2001 [EMAIL PROTECTED] wrote:

 One remark, repeating what I wrote on some web page:
 -
 A struct block_device provides the connection between a device number
 and a struct block_device_operations. 
 ...
 Clearly, we also want to associate a struct char_device_operations
 to a character device number. When we do this, all bdev code will

They are entirely different. Too different sets of operations.

 have to be duplicated for cdev, so there seems no point in having
 bdev code - kdev, for both bdev and cdev, seems more elegant. 
 -

Not really. For struct block_device you have partitioning stuff sitting
nearby. It should be handled by generic code. Nothing of that kind for
character devices.

But the main point is that for block devices -read() and -write() do
not belong to the natural interface. Request queue does. They are about
as far from each other as from FIFOs and sockets.

 And a second remark: don't forget that presently the point where
 bdev is introduced is not quite right. We must only introduce it
 when we really have a device, not when there only is a device
 number (like on a mknod call).

That's simply wrong. kdev_t is used for unopened objects quite often.

-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] struct char_device

[Andries . Brouwer]

 They are entirely different. Too different sets of operations.

Maybe you didnt understand what I meant.
both bdev and cdev take care of the correspondence
device number --- struct with operations.

The operations are different, but all bdev/cdev code is identical.

So the choice is between two uglies:
(i) have some not entirely trivial amount of code twice in the kernel
(ii) have a union at the point where the struct operations
is assigned.

I preferred the union.

 And a second remark: don't forget that presently the point where
 bdev is introduced is not quite right. We must only introduce it
 when we really have a device, not when there only is a device
 number (like on a mknod call).

 That's simply wrong. kdev_t is used for unopened objects quite often.

Yes, but that was my design mistake in 1995.
I think you'll find if you continue on this way,
as I found and already wrote in kdev_t.h
that it is bad to carry pointers around for unopened and unknown devices.

So, I think that the setup must be changed a tiny little bit
and distinguish meaningless numbers from devices.

Andries
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] struct char_device

[Martin Dalecki]

[EMAIL PROTECTED] wrote:
 
  They are entirely different. Too different sets of operations.
 
 Maybe you didnt understand what I meant.
 both bdev and cdev take care of the correspondence
 device number --- struct with operations.
 
 The operations are different, but all bdev/cdev code is identical.
 
 So the choice is between two uglies:
 (i) have some not entirely trivial amount of code twice in the kernel
 (ii) have a union at the point where the struct operations
 is assigned.
 
 I preferred the union.
 
  And a second remark: don't forget that presently the point where
  bdev is introduced is not quite right. We must only introduce it
  when we really have a device, not when there only is a device
  number (like on a mknod call).
 
  That's simply wrong. kdev_t is used for unopened objects quite often.
 
 Yes, but that was my design mistake in 1995.

I fully agree with you. Most of the places where there kernel is passing
kdev_t
would be entierly satisfied with only the knowlendge of the minor number
used to
distinguish between different device ranges, which is BTW an abuse by
itself as well
since minors where for encounters of instances of similiar devices in
linux...
The places where this is the case are namely:

1. literally: all character devices.

2. The whole scsi stuff.

3. most of the ide stuff.

4. md/lvm and similiar culprits.

I did discover this by splitting the i_dev field from stuct inode
into explicit i_minor and i_major fields and then actually fixing my
particular kernel configuration until it worked again. This was
*very* insigtfull, since it discovered all the places where kdev_t get's
used, where it shouldn't be of any need anylonger anyway.

The remaining places where kdev_t comes into sight are mostly
the places where the kernel is mounting the initial root
device.

In case you would like to have a look at the resulting bit huge
patch I can send it to you...

 I think you'll find if you continue on this way,
 as I found and already wrote in kdev_t.h
 that it is bad to carry pointers around for unopened and unknown devices.
 
 So, I think that the setup must be changed a tiny little bit
 and distinguish meaningless numbers from devices.
 
 Andries
 -
 To unsubscribe from this list: send the line unsubscribe linux-kernel in
 the body of a message to [EMAIL PROTECTED]
 More majordomo info at  http://vger.kernel.org/majordomo-info.html
 Please read the FAQ at  http://www.tux.org/lkml/

-- 
- phone: +49 214 8656 283
- job:   eVision-Ventures AG, LEV .de (MY OPINIONS ARE MY OWN!)
- langs: de_DE.ISO8859-1, en_US, pl_PL.ISO8859-2, last ressort:
ru_RU.KOI8-R
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] struct char_device

[Andries . Brouwer]

Martin Dalecki writes:

 I fully agree with you.

Good.

Unfortunately I do not fully agree with you.

 Most of the places where there kernel is passing kdev_t
 would be entierly satisfied with only the knowlendge of
 the minor number.

My kdev_t is a pointer to a structure with device data
and device operations. Among the operations a routine
that produces a name. Among the data, in the case of a
block device, the size, and the sectorsize, ..

A minor gives no name, and no data.

Linus' minor is 20-bit if I recall correctly.
My minor is 32-bit. Neither of the two can be
used to index arrays.

Andries
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] struct char_device

[Martin Dalecki]

[EMAIL PROTECTED] wrote:
 
 Martin Dalecki writes:
 
  I fully agree with you.
 
 Good.
 
 Unfortunately I do not fully agree with you.
 
  Most of the places where there kernel is passing kdev_t
  would be entierly satisfied with only the knowlendge of
  the minor number.
 
 My kdev_t is a pointer to a structure with device data
 and device operations. Among the operations a routine
 that produces a name. Among the data, in the case of a
 block device, the size, and the sectorsize, ..
 
 A minor gives no name, and no data.
 
 Linus' minor is 20-bit if I recall correctly.
 My minor is 32-bit. Neither of the two can be
 used to index arrays.

Erm... I wasn't talking about the DESIRED state of affairs!
I was talking about the CURRENT state of affairs. OK?
The fact still remains that most of the places which a have pointed
out just need the minor nibble of whatever bits you pass to them.

Apparently nobody on this list here blabbering about a new improved
minor/major space didn't actually take the time and looked into
all those places where the kernel is CURRENTLY replying in minor/major
array enumeration. They are TON's of them. The most ugly are RAID
drivers
an all those MD LVW and whatever stuff as well as abused minor number
spaces as replacements of differnt majors.

At least you have admitted that you where the one responsible for
the design of this MESS.
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] struct char_device

[Martin Dalecki]

[EMAIL PROTECTED] wrote:
 
 Martin Dalecki writes:
 
  Erm... I wasn't talking about the DESIRED state of affairs!
  I was talking about the CURRENT state of affairs. OK?
 
 Oh, but in 1995 it was quite possible to compile the kernel
 with kdev_t a pointer type, and I have done it several times since.

Yes I remember but unfortunately some big L* did ignore
your *fine* efforts entierly in favour of developing 
/proc and /dev/random and other crap maybe?

 The kernel keeps growing, so each time it is more work than
 the previous time.
 
  At least you have admitted that you where the one responsible
  for the design of this MESS.
 
 Thank you! However, you give me too much honour.

Well ... you ask for it in the corresponding header ;-).
But it isn't yours fault indeed I admit...
I know the discussions from memmory since I'm returning REGULARLY to
this
topic in intervals of about between 6 and 24 months since about
maybe already 6 years!!! Currently they have just started to hurt
seriously. And please remember the change I have mentioned above
wasn't intended as developement but just only as an experiment...

Well let's us stop throw flames at each other.
Please have a tight look at the following *EXPERIMENT* I have
already done. It's really really only intended to mark the
places where the full mess shows it's ugly head:

http://www.dalecki.de/big-002.diff
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] struct char_device

[Linus Torvalds]

On Tue, 22 May 2001 [EMAIL PROTECTED] wrote:
 
 The operations are different, but all bdev/cdev code is identical.
 
 So the choice is between two uglies:
 (i) have some not entirely trivial amount of code twice in the kernel
 (ii) have a union at the point where the struct operations
 is assigned.
 
 I preferred the union.

I would much prefer a union of pointers over a pointer to a union.

So I'd much rather have the inode have a

union {
struct block_device *block;
struct char_device *char;
} dev;

and then have people do

cdev = inode-dev.char;

to get the right information, than to have 

union block_char_union {
struct block_device block;
struct char_device char;
};

.. with struct inode containing ..
union block_char_union *dev;

Why? Because if you have a struct inode, you also have enough
information to decide _which_ of the two types of pointers you have, so
you can do the proper dis-ambiguation of the union and properly select
either 'inode-dev.char' or 'inode-dev.block' depending on other
information in the inode.

In contrast, if you have a pointer to a union, you don't have information
of which sub-type it is, and you'd have to carry that along some other way
(for example, by having common fields at the beginning). Which I think is
broken.

So my suggestion for eventual interfaces:

 - have functions like

struct block_dev *bdget(struct inode *);
struct char_dev *cdget(struct inode *);

   which populate the inode-dev union pointer, which in turn is _only_
   a cache of the lookup. Right now we do this purely based on dev_t,
   and I think that is bogus. We should never pass a dev_t around
   without an inode, I think.

   And we should not depend on the inode-dev. pointer being valid all
   the time, as there is absolutely zero point in initializing the pointer
   every time the inode is read just because somebody does a ls -l /dev.
   Thus the cache part above.

 - NO reason to try to make struct block_dev and struct char_dev look
   similar. They will have some commonality for lookup purposes (that
   issue is similar, as Andries points out), and maybe that commonality
   can be separated out into a sub-structure or something. But apart from
   that, they have absolutely nothing to do with each other, and I'd
   rather not have them have even a _superficial_ connection.

   Block devices will have the request queue pointer, and the size and
   partitioning information. Character devices currently would not have
   much more than the operations pointer and name, but who knows..

But the most important thing is to be able to do this in steps. One of the
reasons Andries has had patches for a long time is that it was never very
gradual. Al's patch is gradual, and I like that.

Linus

-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] struct char_device

[Martin Dalecki]

And if we are at the topic... Those are the places where blk_size[]
get's
abused, since it's in fact a property of a FS in fact and not the
property of
a particular device... blksect_size is the array describing the physical
access limits of a device and blk_size get's usually checked against it.
However due to the bad naming and the fact that this information is
associated with major/minor number usage same device driver writers got
*very* confused as you can see below:

./fs/block_dev.c: Here this information should be passed entierly insice
the request.

./fs/partitions/check.c: Here it basically get's reset or ignored


Here it's serving the purpose of a sector size, which is bogous!

./mm/swapfile.c:#include linux/blkdev.h /* for blk_size */
./mm/swapfile.c:if (!dev || (blk_size[MAJOR(dev)] 
./mm/swapfile.c: !blk_size[MAJOR(dev)][MINOR(dev)]))
./mm/swapfile.c:if (blk_size[MAJOR(dev)])
./mm/swapfile.c:swapfilesize = blk_size[MAJOR(dev)][MINOR(dev)]


Here it shouldn't be needed
./drivers/block/ll_rw_blk.c: 


./drivers/block/floppy.c:   blk_size[MAJOR_NR] = floppy_sizes;
./drivers/block/nbd.c:  blk_size[MAJOR_NR] = nbd_sizes;
./drivers/block/rd.c: * and set blk_size for -ENOSPC, Werner Fink
[EMAIL PROTECTED], Apr '99
./drivers/block/amiflop.c:  blk_size[MAJOR_NR] = floppy_sizes;
./drivers/block/loop.c: if (blk_size[MAJOR(lodev)])
./drivers/block/ataflop.c: *   - Set blk_size for proper size checking
./drivers/block/ataflop.c:  blk_size[MAJOR_NR] = floppy_sizes;
./drivers/block/cpqarray.c: drv-blk_size;
./drivers/block/z2ram.c:blk_size[ MAJOR_NR ] = z2_sizes;
./drivers/block/swim3.c:blk_size[MAJOR_NR] = floppy_sizes;
./drivers/block/swim_iop.c: blk_size[MAJOR_NR] = floppy_sizes;
./drivers/char/raw.c:   if (blk_size[MAJOR(dev)])
./drivers/scsi/advansys.c:ASC_DCNTblk_size;
./drivers/scsi/sd.c:blk_size[SD_MAJOR(i)] = NULL;
./drivers/scsi/sr.c:blk_size[MAJOR_NR] = sr_sizes;
./drivers/scsi/sr.c:blk_size[MAJOR_NR] = NULL;
./drivers/sbus/char/jsflash.c:  blk_size[JSFD_MAJOR] = jsfd_sizes;
./drivers/ide/ide-cd.c: blk_size[HWIF(drive)-major] =
HWIF(drive)-gd-sizes;
./drivers/ide/ide-floppy.c: *   Revalidate the new media. Should set
blk_size[]
./drivers/acorn/block/fd1772.c: blk_size[MAJOR_NR] = floppy_sizes;
./drivers/i2o/i2o_block.c:  blk_size[MAJOR_NR] = i2ob_sizes;

In the following they are REALLY confusing it and then compensating for
this misunderstanding in lvm.h by redefining the corresponding default
values.

./drivers/s390/*

And then some minor confusions follow...

./drivers/mtd/mtdblock.c:   blk_size[MAJOR_NR] = NULL;
./drivers/md/md.c:  if (blk_size[MAJOR(dev)])
./arch/m68k/atari/stram.c:blk_size[STRAM_MAJOR] = stram_sizes;

Basically one should just stop setting blk_size[][] inside *ANY* driver
and anything should still work fine unless the driver is broken...

Well that's the point for another fine kernel experiment I will do
and report whatever it works really out like this in reality 8-)
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] struct char_device

[Andries . Brouwer]

From [EMAIL PROTECTED] Wed May 23 00:39:23 2001

On Tue, 22 May 2001 [EMAIL PROTECTED] wrote:
 
 The operations are different, but all bdev/cdev code is identical.
 
 So the choice is between two uglies:
 (i) have some not entirely trivial amount of code twice in the kernel
 (ii) have a union at the point where the struct operations
 is assigned.
 
 I preferred the union.

I would much prefer a union of pointers over a pointer to a union.

Why? Because if you have a struct inode, you also have enough
information to decide _which_ of the two types of pointers you have, so
you can do the proper dis-ambiguation of the union and properly select
either 'inode-dev.char' or 'inode-dev.block' depending on other
information in the inode.

I am not sure whether we agree or differ in opinion. I wouldn't mind

/* pairing for dev_t to bd_op/cd_op */
struct bc_device {
struct list_headbd_hash;
atomic_tbd_count;
dev_t   bd_dev;
atomic_tbd_openers;
union {
struct block_device_operations_and_data *bd_op;
struct char_device_operations_and_data *cd_op;
}
struct semaphorebd_sem;
};

typedef struct bc_device *kdev_t;

and in an inode

kdev_t dev;
dev_t rdev;

In reality we want the pair (dev_t, pointer to stuff), but then
there is all this administrative nonsense needed to make sure
that nobody uses the pointer after the module has been unloaded
that makes the pointer a bit thicker.

   And we should not depend on the inode-dev. pointer
   being valid all the time, as there is absolutely zero point
   in initializing the pointer every time somebody does a ls -l /dev.

Yes, that is why I want to go back and have dev_t rdev, not kdev_t.
The lookup is done when the device is opened.

Andries
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] struct char_device

[Alexander Viro]

On Wed, 23 May 2001 [EMAIL PROTECTED] wrote:

 I am not sure whether we agree or differ in opinion. I wouldn't mind
 
 /* pairing for dev_t to bd_op/cd_op */
 struct bc_device {
 struct list_headbd_hash;
 atomic_tbd_count;
 dev_t   bd_dev;
 atomic_tbd_openers;
 union {
   struct block_device_operations_and_data *bd_op;
   struct char_device_operations_and_data *cd_op;
   }
 struct semaphorebd_sem;
 };
 
 typedef struct bc_device *kdev_t;

What for? What part of the kernel needs a device and doesn't know apriory
whether it's block or character one?

 and in an inode
 
   kdev_t dev;
Useless. If you hope that block_device will help to solve rmmod races -
sorry, it won't. Wrong layer.

   dev_t rdev;
Reasonable.

-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] struct char_device

[Jeff Garzik]

Alexander Viro wrote:
 Do we really want a separate queue for each partition? I'd rather have
 disk_struct created when driver sees the disk and list of partitions
 (possibly represented by struct block_device) anchored in disk_struct
 and populated by grok_partitions().

Alan recently straightened me out with EVMS/LVM is partitions done
right

so... why not implement partitions as simply doing block remaps to the
lower level device?  That's what EVMS/LVM/md are doing already.

-- 
Jeff Garzik  | Are you the police?
Building 1024| No, ma'am.  We're musicians.
MandrakeSoft |
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] struct char_device

[Jens Axboe]

On Tue, May 22 2001, Jeff Garzik wrote:
 IMHO it would be nice to (for 2.4) create wrappers for accessing the
 block arrays, so that we can more easily dispose of the arrays when 2.5
 rolls around...

Agreed, in fact I have a lot of stuff that could be included in the
kcompat files for 2.4 (of course still changing :)

BTW, max_sectors/max_segments/hardsect_size already in place. Still some
to go.

-- 
Jens Axboe

-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] struct char_device

[Andries . Brouwer]

 why not implement partitions as simply doing block remaps

Everybody agrees.

Andries
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] struct char_device

[Andries . Brouwer]

 IMHO it would be nice to create wrappers for accessing the block arrays

Last year Linus didnt like that at all. Maybe this year.

Andries
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] struct char_device

[Andries . Brouwer]

  dev_t rdev;

 Reasonable.

Good. We all agree.
(But now you see what I meant in comments about mknod.)

  kdev_t dev;

 Useless. If you hope that block_device will help to solve rmmod races

Yes. Why am I mistaken?

Andries
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] struct char_device

[Martin Dalecki]

[EMAIL PROTECTED] wrote:
 
  IMHO it would be nice to create wrappers for accessing the block arrays
 
 Last year Linus didnt like that at all. Maybe this year.

Well... the attached patch lines up into this effort and fixes
some abuses, removes redundant code and so on. Please have a second
look.

diff -urN linux/drivers/block/ll_rw_blk.c new/drivers/block/ll_rw_blk.c
--- linux/drivers/block/ll_rw_blk.c Thu Apr 12 21:15:52 2001
+++ new/drivers/block/ll_rw_blk.c   Mon Apr 30 23:16:03 2001
@@ -85,25 +85,21 @@
 int * blk_size[MAX_BLKDEV];
 
 /*
- * blksize_size contains the size of all block-devices:
+ * blksize_size contains the block size of all block-devices:
  *
  * blksize_size[MAJOR][MINOR]
  *
- * if (!blksize_size[MAJOR]) then 1024 bytes is assumed.
+ * Access to this array should happen through the get_blksize_size() function.
+ * If (!blksize_size[MAJOR]) then 1024 bytes is assumed.
  */
 int * blksize_size[MAX_BLKDEV];
 
 /*
  * hardsect_size contains the size of the hardware sector of a device.
  *
- * hardsect_size[MAJOR][MINOR]
- *
- * if (!hardsect_size[MAJOR])
- * then 512 bytes is assumed.
- * else
- * sector_size is hardsect_size[MAJOR][MINOR]
- * This is currently set by some scsi devices and read by the msdos fs driver.
- * Other uses may appear later.
+ * Access to this array should happen through the get_hardsect_size() function.
+ * The default value is assumed to be 512 unless specified differently by the
+ * corresponding low-level driver.
  */
 int * hardsect_size[MAX_BLKDEV];
 
@@ -992,22 +988,14 @@
 
 void ll_rw_block(int rw, int nr, struct buffer_head * bhs[])
 {
-   unsigned int major;
-   int correct_size;
+   ssize_t correct_size;
int i;
 
if (!nr)
return;
 
-   major = MAJOR(bhs[0]-b_dev);
-
/* Determine correct block size for this device. */
-   correct_size = BLOCK_SIZE;
-   if (blksize_size[major]) {
-   i = blksize_size[major][MINOR(bhs[0]-b_dev)];
-   if (i)
-   correct_size = i;
-   }
+   correct_size = get_blksize_size(bhs[0]-b_dev);
 
/* Verify requested block sizes. */
for (i = 0; i  nr; i++) {
diff -urN linux/drivers/block/loop.c new/drivers/block/loop.c
--- linux/drivers/block/loop.c  Thu Apr 12 04:05:14 2001
+++ new/drivers/block/loop.cMon Apr 30 23:30:17 2001
@@ -272,22 +272,10 @@
return desc.error;
 }
 
-static inline int loop_get_bs(struct loop_device *lo)
-{
-   int bs = 0;
-
-   if (blksize_size[MAJOR(lo-lo_device)])
-   bs = blksize_size[MAJOR(lo-lo_device)][MINOR(lo-lo_device)];
-   if (!bs)
-   bs = BLOCK_SIZE;
-
-   return bs;
-}
-
 static inline unsigned long loop_get_iv(struct loop_device *lo,
unsigned long sector)
 {
-   int bs = loop_get_bs(lo);
+   int bs = get_blksize_size(lo-lo_device);
unsigned long offset, IV;
 
IV = sector / (bs  9) + lo-lo_offset / bs;
@@ -306,9 +294,9 @@
pos = ((loff_t) bh-b_rsector  9) + lo-lo_offset;
 
if (rw == WRITE)
-   ret = lo_send(lo, bh, loop_get_bs(lo), pos);
+   ret = lo_send(lo, bh, get_blksize_size(lo-lo_device), pos);
else
-   ret = lo_receive(lo, bh, loop_get_bs(lo), pos);
+   ret = lo_receive(lo, bh, get_blksize_size(lo-lo_device), pos);
 
return ret;
 }
@@ -650,12 +638,7 @@
lo-old_gfp_mask = inode-i_mapping-gfp_mask;
inode-i_mapping-gfp_mask = GFP_BUFFER;
 
-   bs = 0;
-   if (blksize_size[MAJOR(inode-i_rdev)])
-   bs = blksize_size[MAJOR(inode-i_rdev)][MINOR(inode-i_rdev)];
-   if (!bs)
-   bs = BLOCK_SIZE;
-
+   bs = get_blksize_size(inode-i_rdev);
set_blocksize(dev, bs);
 
lo-lo_bh = lo-lo_bhtail = NULL;
diff -urN linux/drivers/char/raw.c new/drivers/char/raw.c
--- linux/drivers/char/raw.cFri Apr 27 23:23:25 2001
+++ new/drivers/char/raw.c  Mon Apr 30 22:57:20 2001
@@ -124,22 +124,25 @@
return err;
}
 
-   
-   /* 
-* Don't interfere with mounted devices: we cannot safely set
-* the blocksize on a device which is already mounted.  
+   /*
+* 29.04.2001 Martin Dalecki:
+*
+* The original comment here was saying:
+*
+* Don't interfere with mounted devices: we cannot safely set the
+* blocksize on a device which is already mounted.
+*
+* However the code below was setting happily the blocksize
+* disregarding the previous check. I have fixed this, however I'm
+* quite sure, that the statement above isn't right and we should be
+* able to remove the first arm of the branch below entierly.
 */
-   
-   sector_size = 512;
if (get_super(rdev) != NULL) {
-   if (blksize_size[MAJOR(rdev)])
-

Re: [PATCH] struct char_device

[Alexander Viro]

On Wed, 23 May 2001 [EMAIL PROTECTED] wrote:

 dev_t rdev;
 
  Reasonable.
 
 Good. We all agree.
 (But now you see what I meant in comments about mknod.)
 
 kdev_t dev;
 
  Useless. If you hope that block_device will help to solve rmmod races
 
 Yes. Why am I mistaken?

Because the problems begin in subsystems. Solving the situation with
block_device_operations is trivial. It's stuff on the character side that
is going to bite you big way. TTY drivers, for example. They are below
the layer where your kdev_t lives.

-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] struct char_device

[Linus Torvalds]

On Tue, 22 May 2001, Alexander Viro wrote:

 which populate the inode-dev union pointer, which in turn is _only_
 a cache of the lookup. Right now we do this purely based on dev_t,
 and I think that is bogus. We should never pass a dev_t around
 without an inode, I think.
 
 I doubt it. First of all, then we'd better make i_rdev dev_t. Right now
 we have it kdev_t and it makes absolutely no sense that way.

Absolutely.

In fact, the whole kdev_t makes no sense if we have proper char_dev and
block_dev pointers.

We have dev_t which is what we export to user space. And if we split up
char dev and block dev (which I'm an avid proponent for), kdev_t will be
an anachronism.

 The real thing is inode-dev. Notice that for devfs (and per-device
 filesystems) we can set it upon inode creation, since they _know_ it
 from the very beginning and there is absolutely no reason to do any
 hash lookups. Moreover, in these cases we may have no meaningful device
 number at all.

Sure. If something like devfs _knows_ the device pointers from the very
beginning, then just do: 
 - increment reference count
 - inode-dev.char = cdev;

 Block devices will have the request queue pointer, and the size and
 partitioning information. Character devices currently would not have
 
 Do we really want a separate queue for each partition?

No. 

But the pointer is not a 1:1 thing (otherwise we'd just put the whole
request queue _into_ the block device).

The block device should have a pointer to the request queue, along with
the partitioning information. Why? Because that way it becomes very simple
indeed to do request processing: none of the current look up the proper
queue for each request and do the partition offset magic inside each
driver. Instead, the queuing function becomes:

bh-index = bdev-index;
bh-sector += bdev-sector_offset;
submit_bh(bh, bdev-request_queue);

and you're done. Those three lines did both the queue lookup, the
index for the driver, and the partitioning offset. Whoa, nelly.

And THAT is why you want to have the queue pointer in the bdev structure.

Linus

-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] struct char_device

[Linus Torvalds]

On Tue, 22 May 2001, Jeff Garzik wrote:
 
 Alan recently straightened me out with EVMS/LVM is partitions done
 right
 
 so... why not implement partitions as simply doing block remaps to the
 lower level device?  That's what EVMS/LVM/md are doing already.

Because we still need the partitioning code for backwards
compatibility. There's no way I'm going to use initrd to do partition
setup with lvmtools etc.

Also, lvm and friends are _heavyweight_. The partitioning stuff should be
_one_ add (and perhaps a range check) at bh submit time. None of this
remapping crap. We don't need no steenking overhead for something we need
to do anyway.

Linus

-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] struct char_device

[Linus Torvalds]

On Tue, 22 May 2001, Jeff Garzik wrote:

 IMHO it would be nice to (for 2.4) create wrappers for accessing the
 block arrays, so that we can more easily dispose of the arrays when 2.5
 rolls around...

No.

We do not create wrappers so that we can easily change the implementation
when xxx happens.

That way lies bad implementations.

We do the _good_ implementation first, and then we create the
kcompat-2.4 file later that makes people able to use the good
implementation.

Why this way? Because wrapping for wrappings sake just makes code
unreadable (see acpi), and often makes it less obvious what you actually
_have_ to do, and what you don't. So get the design right _first_, and
once the design is right, _then_ you worry about kcompat-2.4.

Linus

-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] struct char_device

[Linus Torvalds]

On Wed, 23 May 2001 [EMAIL PROTECTED] wrote:

  why not implement partitions as simply doing block remaps
 
 Everybody agrees.

No they don't.

Look at the cost of lvm. Function calls, buffer head remapping, the
works. You _need_ that for a generic LVM layer, but you sure as hell don't
need it for simple partitioning.

Can LVM do it? Sure.

Do we need LVM to do it? No.

Does LVM simplify anything? No.

It doesn't get much simpler than a single line that does the
equivalent of bh-sector += offset. Most of the bulk of the code in the
partitioning stuff is for actually parsing the partitions, and we need
that to bootstrap. Maybe we can get rid of some of the more esoteric ones
(ie pretty much everything except for the native partitioning code for
each architecture) and tell people to use LVM for the rest.

In short, there are _no_ advantages to involve LVM here.

Linus

-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] struct char_device

[Jeff Garzik]

Linus Torvalds wrote:
 
 On Tue, 22 May 2001, Jeff Garzik wrote:
 
  Alan recently straightened me out with EVMS/LVM is partitions done
  right
 
  so... why not implement partitions as simply doing block remaps to the
  lower level device?  That's what EVMS/LVM/md are doing already.
 
 Because we still need the partitioning code for backwards
 compatibility. There's no way I'm going to use initrd to do partition
 setup with lvmtools etc.
 
 Also, lvm and friends are _heavyweight_. The partitioning stuff should be
 _one_ add (and perhaps a range check) at bh submit time. None of this
 remapping crap. We don't need no steenking overhead for something we need
 to do anyway.

no no no.  Not -that- heavyweight.

Partition support becomes a -peer- of LVM.

Imagine a tiny blkdev driver that understood MS-DOS (and other) hardware
partitions, and exported N block devices, representing the underlying
device (whatever it is).  In fact, that might be even a -unifying-
factor:  this tiny blkdev module -is- your /dev/disk.  For example,

/dev/sda - partition_blkdev - /dev/disk{0,1,2,3,4}
/dev/hda - partition_blkdev - /dev/disk{5,6,7}

A nice side effect:  modular partition support, since its a normal
blkdev just like anything yes.

YES there is overhead, but if partitions are just another remapping
blkdev, you get all this stuff for free.

I do grant you that an offset at bh submit time is faster, but IMHO
partitions -not- as a remapping blkdev are an ugly special case.

Remapping to an unchanging offset in the make_request_fn can be fast,
too...

-- 
Jeff Garzik  | Are you the police?
Building 1024| No, ma'am.  We're musicians.
MandrakeSoft |
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] struct char_device

[Jeff Garzik]

Jeff Garzik wrote:
 /dev/sda - partition_blkdev - /dev/disk{0,1,2,3,4}
 /dev/hda - partition_blkdev - /dev/disk{5,6,7}

I also point out that handling disk partitions as a -tiny- remapping
blkdev also has the advantage of making it easy to have a single request
device per hardware device (a simple remap shouldn't require its own
request queue, right?), while remapping devices flexibility to do their
own request queue management.


 I do grant you that an offset at bh submit time is faster, but IMHO
 partitions -not- as a remapping blkdev are an ugly special case.

think of the simplifications possible, when partitions are just another
block device, just like anything else...  No special partitions arrays
in the lowlevel blkdev, etc.
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/