Messages in this thread | | | Date | Wed, 26 Jul 2000 12:43:56 +0100 (BST) | From | Tigran Aivazian <> | Subject | Re: [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/
| |