[LART] inode mismanagement in hugetlb code

[LART] inode mismanagement in hugetlb code

[Alexander Viro]

[A discussion of the meanings of the terms "MUST", "SHOULD", and "MAY" appears
in RFC-1123; the terms "MUST NOT" and "SHOULD NOT" are logical extensions of
this usage]

	a) inodes MUST have an address of valid struct super_block in their
->i_sb.  Had been discussed quite a few times already.

	b) inodes MUST NOT be allocated by code that isn't called from
alloc_inode().

	c) inodes SHOULD NOT be kept around for noticable intervals without
a dentry pointing to them.

	d) people who choose variable names like htlbpagek SHOULD be sent to
produce a street map of R'Lyeh.  On site.

Re: [LART] inode mismanagement in hugetlb code

[William Lee Irwin III]

On Thu, Oct 10, 2002 at 03:00:37PM -0400, Alexander Viro wrote:
> 	a) inodes MUST have an address of valid struct super_block in their
> ->i_sb.  Had been discussed quite a few times already.
> 	b) inodes MUST NOT be allocated by code that isn't called from
> alloc_inode().
> 	c) inodes SHOULD NOT be kept around for noticable intervals without
> a dentry pointing to them.
> 	d) people who choose variable names like htlbpagek SHOULD be sent to
> produce a street map of R'Lyeh.  On site.


A wee bit of neurosurgery is in progress to repair (a) and (b) since
they're taking out my boxen dead cold and reliably on the first or
second invocation with Paul Larson's tests. (c) is very unclean, and
(d) I wholeheartedly agree with (and am amused by the wording of too =).


Bill

Re: [LART] inode mismanagement in hugetlb code

[Andrew Morton]

Alexander Viro wrote:
> 
> [A discussion of the meanings of the terms "MUST", "SHOULD", and "MAY" appears
> in RFC-1123; the terms "MUST NOT" and "SHOULD NOT" are logical extensions of
> this usage]
> 
>         a) inodes MUST have an address of valid struct super_block in their
> ->i_sb.  Had been discussed quite a few times already.
> 

afaict, that code only wants an inode because it is borrowing
the pagecache functions for page lookup.  It's using i_ino as
a search key too.  It has no superblock.

Solutions might be: 1) allocate a private <int key, radix tree>
structure or 2) require that these inodes come from hugetlbfs,
although the "key" makes that a bit tricky.

Re: [LART] inode mismanagement in hugetlb code

[William Lee Irwin III]

Alexander Viro wrote:
>>         a) inodes MUST have an address of valid struct super_block in their
>> ->i_sb.  Had been discussed quite a few times already.

On Thu, Oct 10, 2002 at 12:22:28PM -0700, Andrew Morton wrote:
> afaict, that code only wants an inode because it is borrowing
> the pagecache functions for page lookup.  It's using i_ino as
> a search key too.  It has no superblock.
> Solutions might be: 1) allocate a private <int key, radix tree>
> structure or 2) require that these inodes come from hugetlbfs,
> although the "key" makes that a bit tricky.

shm clobbers hugetlbfs-assigned inode numbers with the shp->id
somewhere around newseg(), (which doesn't matter because it's the
only user of the kern_mounted fs) so ->i_ino will need to avoid
clashing there. But there are several ways to wean hugetlbpage.c off
->i_ino so it's not a big deal.

Bill