lkml.org 
[lkml]   [2000]   [Jul]   [26]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [broken] Re: [patch-2.4.0-test5-pre3] struct inode shortened
On Wed, 26 Jul 2000, Jamie Lokier wrote:

> Tigran Aivazian wrote:
> > Actually, the idea of union-ing things is probably broken anyway. Imagine
> > clear_inode() is called on a pipe (because pipefs does iput(), e.g. from
> > get_pipe_inode()):
> >
> > if (inode->i_bdev) {
> > bdput(inode->i_bdev);
> > inode->i_bdev = NULL;
> > }
> >
> > in the union version, for a pipe, inode->i_bdev may well be not NULL and
> > so bdput() will be called illegally. So, my yesterday's patch appears to
> > be broken. (which is why I left Linus in cc to this message, I would not
> > have wasted his time otherwise).
>
> 1. Make the name explicit: inode->i_union->bdev.

I don't think renaming things make them any different.

>
> 2. Change all the code that refers to the newly unioned fields.
> It should be clear from the name i_union that no code should
> read a field until it knows the type of the inode.

ah, that is much better - what you are saying is the above code should
look like:

if (IS_BLK(inode->i_mode) & inode->i_bdev) {
bdput(inode->i_bdev);
inode->i_bdev = NULL;
}

Then it is a typical tradeoff between speed and memory optimizations - we
gain 4 bytes in the inode but get an extra comparison instruction in the
code. To me, speed is more important - extra memory one can always buy but
the time... hmmm redeem the time for the days are evil.

Regards,
Tigran


-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.rutgers.edu
Please read the FAQ at http://www.tux.org/lkml/

\
 
 \ /
  Last update: 2005-03-22 13:57    [W:0.069 / U:0.244 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site