linux-kernel - Re: SLUB defrag pull request?

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <E1KpOjX-0003dt-AY@pomaz-ex.szeredi.hu>
Date:	Mon, 13 Oct 2008 16:49:19 +0200
From:	Miklos Szeredi <miklos@...redi.hu>
To:	cl@...ux-foundation.org
CC:	miklos@...redi.hu, penberg@...helsinki.fi, nickpiggin@...oo.com.au,
	hugh@...itas.com, linux-mm@...ck.org,
	linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org,
	akpm@...ux-foundation.org
Subject: Re: SLUB defrag pull request?

On Mon, 13 Oct 2008, Christoph Lameter wrote:
> Miklos Szeredi wrote:
> > And the things kick_inodes() does without any sort of locking look
> > even more dangerous.
> >
> > It should be the other way round: first make sure nothing is
> > referencing the inode, and _then_ start cleaning it up with
> > appropriate locks held.  See prune_icache().
> >
> >   
> kick_inodes() only works on inodes that first have undergone 
> get_inodes() where we establish a refcount under inode_lock(). The final 
> cleanup in kick_inodes() is done under iprune_mutex. You are looking at 
> the loop that does writeback and invalidates attached dentries. This can 
> fail for various reasons.

Yes, but I'm not at all sure that calling remove_inode_buffers() or
invalidate_mapping_pages() is OK on a live inode.  They should be done
after checking the refcount, just like prune_icache() does.

Also, while d_invalidate() is not actually wrong here, because you
check S_ISDIR(), but it's still the wrong function to use.  You really
just want to shrink the children.  Invalidation means: the filesystem
found out that the cached inode is invalid, so we want to throw it
away.  In the future it might actually be able to do it for
directories as well, but currently it cannot because of possible
mounts on the dentry.

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