Thread (14 messages) 14 messages, 4 authors, 2011-03-03

Re: [PATCH V2 0/3] drivers/staging: zcache: dynamic page cache/swap compression

From: Minchan Kim <hidden>
Date: 2011-02-16 04:36:21
Also in: linux-btrfs, lkml

On Wed, Feb 16, 2011 at 10:27 AM, Dan Magenheimer
[off-list ref] wrote:
quoted
-----Original Message-----
From: Matt [mailto:jackdachef@gmail.com]
Sent: Tuesday, February 15, 2011 5:12 PM
To: Minchan Kim
Cc: Dan Magenheimer; gregkh@suse.de; Chris Mason; linux-
kernel@vger.kernel.org; linux-mm@kvack.org; ngupta@vflare.org; linux-
btrfs@vger.kernel.org; Josef Bacik; Dan Rosenberg; Yan Zheng;
miaox@cn.fujitsu.com; Li Zefan
Subject: Re: [PATCH V2 0/3] drivers/staging: zcache: dynamic page
cache/swap compression

On Mon, Feb 14, 2011 at 4:35 AM, Minchan Kim [off-list ref]
quoted
Just my guessing. I might be wrong.

__cleancache_flush_inode calls cleancache_get_key with
cleancache_filekey.
quoted
cleancache_file_key's size is just 6 * u32.
cleancache_get_key calls btrfs_encode_fh with the key.
but btrfs_encode_fh does typecasting the key to btrfs_fid which is
bigger size than cleancache_filekey's one so it should not access
fields beyond cleancache_get_key.

I think some file systems use extend fid so in there, this problem
can
quoted
happen. I don't know why we can't find it earlier. Maybe Dan and
others test it for a long time.

Am I missing something?



--
Kind regards,
Minchan Kim
reposting Minchan's message for reference to the btrfs mailing list
while also adding

Li Zefan, Miao Xie, Yan Zheng, Dan Rosenberg and Josef Bacik to CC

Regards

Matt
Hi Matt and Minchan --

(BTRFS EXPERTS SEE *** BELOW)

I definitely see a bug in cleancache_get_key in the monolithic
zcache+cleancache+frontswap patch I posted on oss.oracle.com
that is corrected in linux-next but I don't see how it could
get provoked by btrfs.

The bug is that, in cleancache_get_key, the return value of fhfn should
be checked against 255.  If the return value is 255, cleancache_get_key
should return -1.  This should disable cleancache for any filesystem
where KEY_MAX is too large.

But cleancache_get_key always calls fhfn with connectable == 0 and
CLEANCACHE_KEY_MAX==6 should be greater than BTRFS_FID_SIZE_CONNECTABLE
(which I think should be 5?).  And the elements written into the
typecast btrfs_fid should be only writing the first 5 32-bit words.
BTRFS_FID_SIZE_NON_CONNECTALBE is 5,  not BTRFS_FID_SIZE_CONNECTABLE.
Anyway, you passed connectable with 0 so it should be only writing the
first 5 32-bit words as you said.
That's one I missed. ;-)

Thanks.
-- 
Kind regards,
Minchan Kim

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help