Re: [rfc][patch] fs: introduce iget, mark igrab as must_check

[Nick Piggin <npiggin@suse.de>]

On Sat, Mar 20, 2010 at 01:13:38PM -0700, Brad Boyer wrote:
> On Thu, Mar 18, 2010 at 12:11:12AM +1100, Nick Piggin wrote:
> > I thought I would ask for opinions on this because it can come in handy
> > to avoid locking to use an iget() rather than igrab() if we do lazy LRU
> > (like the dcache), then iget() just increments the refcount and doesn't
> > require any more locking.
> > 
> > I thought it might be useful anyway to avoid igrab misuse by
> > filesystems? So after this patch we should use igrab for when we don't
> > already have the inode pinned. Introduce iget for when we do.
> > The interfaces don't change, but __must_check will prompt filesystems to
> > use the correct API.
> 
> I'm not sure if this matters, but I have to say I don't like the name. In
> particular, iget used to exist and be iget(sb, ino). A lot of the code
> still has leftovers from this including iget_locked. I'm not sure if
> there is a more obvious name, especially since __iget is effectively the
> unlocked version of what you added. It's something to consider if you
> haven't already.

Yeah, I can't think of a better name. It would be annoying to introduce
a worse name because of legacy reasons.

> 
> Also, are you really avoiding any locking? It looks the same to me.

Not in this patch, but I have a patch in my icache scalability series
(which can be broken out of course) that basically does a "lazy lru"
for inodes, like we do for dentries now.

So iget will become basically what dget is now. It would not have to
check inode state or move it between lists.

Whether or not we actually do that is another question, but either way
I think this patch is worthwhile.

Thanks,
Nick